Add support for injecting CA certificates into Secrets#265
Conversation
openshift-ci
Bot
added
the
needs-ok-to-test
|
Hi @mnencia. Thanks for your PR. I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
Issues go stale after 90d of inactivity. Mark the issue as fresh by commenting If this issue is safe to close now please do so with /lifecycle stale |
openshift-ci
Bot
added
the
lifecycle/stale
openshift-ci
Bot
removed
the
lifecycle/stale
|
Issues go stale after 90d of inactivity. Mark the issue as fresh by commenting If this issue is safe to close now please do so with /lifecycle stale |
openshift-ci
Bot
added
the
lifecycle/stale
WalkthroughAdds Secret-based CA bundle injection by introducing a namespaced secret injector controller that idempotently writes the Service CA bundle into annotated Secrets, wires the injector into startup, updates README to reflect ConfigMap/Secret support, and extends e2e tests to validate injection and overwrite behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant Controller as Controller (queue/Sync)
participant Informer as Secret Lister/Informer (cache)
participant API as Kubernetes API (Secrets)
rect rgba(100,150,240,0.5)
Controller->>Informer: Get Secret (namespace/name)
end
rect rgba(120,200,80,0.5)
Informer-->>Controller: Secret object (or NotFound)
end
alt Secret not found
Controller->>Controller: no-op / return
else Secret found
Controller->>Controller: apply filter? (skip if rejected)
Controller->>Controller: compare existing data to CA bundle
alt already matches
Controller-->>Controller: no update
else needs update
Controller->>API: Update Secret (replace data with CA bundle + annotations)
API-->>Controller: updated Secret / error
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 9 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (9 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (4)
README.md(1 hunks)pkg/controller/cabundleinjector/secret.go(1 hunks)pkg/controller/cabundleinjector/starter.go(1 hunks)test/e2e/e2e_test.go(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
README.mdpkg/controller/cabundleinjector/secret.gopkg/controller/cabundleinjector/starter.gotest/e2e/e2e_test.go
🧬 Code graph analysis (2)
pkg/controller/cabundleinjector/secret.go (1)
pkg/controller/api/api.go (3)
InjectCABundleAnnotationName(27-27)InjectionDataKey(29-29)OwningJiraComponent(22-22)
test/e2e/e2e_test.go (1)
pkg/controller/api/api.go (1)
InjectionDataKey(29-29)
🪛 markdownlint-cli2 (0.18.1)
README.md
9-9: Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (3)
pkg/controller/cabundleinjector/starter.go (1)
89-97: Wiring the Secret injector into the startup pipeline looks correctAdding
newSecretInjectorConfigtoconfigConstructorsmatches the existing pattern for other injectors and ensures the Secret injector runs with the shared informer factory and annotations filter.README.md (1)
7-9: Updated docs correctly mention Secret-based CA bundle injectionThe README text for the “configmap/secret cabundle injector” accurately reflects that both ConfigMaps and Secrets annotated with
inject-cabundle=trueare now handled, and explains theservice-ca.crtdata item usage clearly.test/e2e/e2e_test.go (1)
213-223: Secret CA-bundle e2e helpers and tests mirror existing ConfigMap coverageThe new helpers for creating annotated Secrets, polling for injection, editing data, and validating contents closely follow the existing ConfigMap patterns and exercise both initial injection and “stomp-on-change” behavior for dedicated injection Secrets. Timeouts and polling are consistent with the rest of the suite, so this should give good end-to-end coverage of the new Secret injector without introducing obvious flakiness.
Also applies to: 251-262, 297-313, 358-372, 1498-1558
| // skip updating when the CA bundle is already there | ||
| if data, ok := secret.Data[api.InjectionDataKey]; ok && | ||
| string(data) == bi.caBundle && len(secret.Data) == 1 { | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| klog.Infof("updating secret %s/%s with the service signing CA bundle", secret.Namespace, secret.Name) | ||
|
|
||
| // make a copy to avoid mutating cache state | ||
| secretCopy := secret.DeepCopy() | ||
| secretCopy.Data = map[string][]byte{api.InjectionDataKey: []byte(bi.caBundle)} | ||
| // set the owning-component unless someone else has claimed it. | ||
| if len(secretCopy.Annotations[apiannotations.OpenShiftComponent]) == 0 { | ||
| secretCopy.Annotations[apiannotations.OpenShiftComponent] = api.OwningJiraComponent | ||
| secretCopy.Annotations[apiannotations.OpenShiftDescription] = fmt.Sprintf("Secret is added/updated with a data item containing the CA signing bundle that can be used to verify service-serving certificates") | ||
| } | ||
|
|
||
| _, err = bi.client.Secrets(secretCopy.Namespace).Update(ctx, secretCopy, metav1.UpdateOptions{}) |
There was a problem hiding this comment.
Fix nil Annotations panic and reconsider overwriting all Secret data
Two things to flag here:
- Potential panic on nil annotations (must fix):
secretCopy.Annotationsmay benilfor a freshly-created Secret. Writing to a nil map will panic:
if len(secretCopy.Annotations[apiannotations.OpenShiftComponent]) == 0 {
secretCopy.Annotations[apiannotations.OpenShiftComponent] = api.OwningJiraComponent
secretCopy.Annotations[apiannotations.OpenShiftDescription] = fmt.Sprintf("Secret is added/updated with a data item containing the CA signing bundle that can be used to verify service-serving certificates")
}Initialize the map before writing:
- secretCopy := secret.DeepCopy()
- secretCopy.Data = map[string][]byte{api.InjectionDataKey: []byte(bi.caBundle)}
- // set the owning-component unless someone else has claimed it.
- if len(secretCopy.Annotations[apiannotations.OpenShiftComponent]) == 0 {
+ secretCopy := secret.DeepCopy()
+ secretCopy.Data = map[string][]byte{api.InjectionDataKey: []byte(bi.caBundle)}
+ // ensure annotations map is initialized before writing to it
+ if secretCopy.Annotations == nil {
+ secretCopy.Annotations = map[string]string{}
+ }
+ // set the owning-component unless someone else has claimed it.
+ if len(secretCopy.Annotations[apiannotations.OpenShiftComponent]) == 0 {
secretCopy.Annotations[apiannotations.OpenShiftComponent] = api.OwningJiraComponent
secretCopy.Annotations[apiannotations.OpenShiftDescription] = fmt.Sprintf("Secret is added/updated with a data item containing the CA signing bundle that can be used to verify service-serving certificates")
}- Secret
Datais fully overwritten (design/semantics):
secretCopy.Data = map[string][]byte{...}drops all existing keys and, together with thelen(secret.Data) == 1skip condition, forces injected Secrets to contain onlyservice-ca.crt. That’s consistent with the new e2e tests (which assertlen(Data)==1and that extra keys get “stomped”), but it is stricter than the README wording (“adds or updates a data item”) and prevents co-locating other material in the same Secret.
If you want parity with a more typical “only manage InjectionDataKey” pattern (and to better match the README text), consider instead:
- secretCopy := secret.DeepCopy()
- secretCopy.Data = map[string][]byte{api.InjectionDataKey: []byte(bi.caBundle)}
+ secretCopy := secret.DeepCopy()
+ if secretCopy.Data == nil {
+ secretCopy.Data = map[string][]byte{}
+ }
+ secretCopy.Data[api.InjectionDataKey] = []byte(bi.caBundle)and relaxing the len(secret.Data) == 1 checks in Sync/tests to only care that InjectionDataKey matches the operator’s bundle.
🤖 Prompt for AI Agents
In pkg/controller/cabundleinjector/secret.go around lines 61 to 79, initialize
secretCopy.Annotations if nil before writing to it to avoid a nil-map panic, and
stop overwriting the entire Secret.Data: instead of assigning a new map, set or
update only the api.InjectionDataKey entry on secretCopy.Data (ensuring
secretCopy.Data is non-nil first) and adjust the earlier skip condition so it
checks only that the InjectionDataKey value matches bi.caBundle rather than
relying on len(secret.Data)==1.
There was a problem hiding this comment.
Intentionally mirroring configMapCABundleInjector here; changing either behaviour would diverge the two injectors.
- Nil
Annotations: the object is only handed toSyncafter passingannotationsChecker(InjectCABundleAnnotationName), which requires the annotation to be present, soAnnotationsis always non-nil in this path.configmap.gorelies on the same invariant. - Stomping
Data: the injector's contract (documented in the README and asserted by the existing configmap "stomp on update" e2e test) is that the target object is exclusively managed by the injector. The matching behaviour for Secrets is by design, and the newca-bundle-injection-secret-updatecase verifies it.
If we want to relax this to "only manage service-ca.crt and leave other keys alone", it should be applied symmetrically to both injectors in a separate PR.
There was a problem hiding this comment.
@mnencia, thank you for the detailed explanation — both points are well-reasoned.
- Nil
Annotations: You're right thatannotationsCheckeracts as a precondition guaranteeing the annotation (and thus the map) is already present beforeSyncis ever called. I missed that invariant. - Stomping
Data: The exclusive-ownership contract makes sense, and the symmetry argument withconfigMapCABundleInjectoris a strong one. Theca-bundle-injection-secret-updatee2e case clearly documents the intended behaviour.
I've updated my understanding accordingly.
📝 Learnings were identified and not saved (knowledge base disabled). Enable
|
/remove-lifecycle stale |
openshift-ci
Bot
removed
the
lifecycle/stale
|
Issues go stale after 90d of inactivity. Mark the issue as fresh by commenting If this issue is safe to close now please do so with /lifecycle stale |
openshift-ci
Bot
added
the
lifecycle/stale
openshift-merge-robot
added
the
needs-rebase
|
PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
Stale issues rot after 30d of inactivity. Mark the issue as fresh by commenting If this issue is safe to close now please do so with /lifecycle rotten |
openshift-ci
Bot
added
lifecycle/rotten
mnencia
force-pushed
the
inject-ca-into-secrets
branch
from
f588f82 to
21e7c2a
Compare
openshift-ci
Bot
removed
the
needs-rebase
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: mnencia The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/remove-lifecycle rotten |
openshift-ci
Bot
removed
the
lifecycle/rotten
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/e2e/e2e.go`:
- Around line 853-864: pollForCABundleInjectionSecret currently returns success
as soon as the Secret exists, causing races; update
pollForCABundleInjectionSecret to, inside the wait.PollImmediate callback (after
retrieving the Secret via client.CoreV1().Secrets(...).Get), verify that
secret.Data contains api.InjectionDataKey and that the value is non-empty and
has the expected map shape by unmarshalling the value (e.g., into a
map[string]interface{}) and asserting required fields exist; keep existing
not-found and error handling and return true only when the key is present and
the unmarshalled shape checks pass.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 807ee82e-0a9f-415b-a5f0-b53fda51b96b
📒 Files selected for processing (5)
README.mdpkg/controller/cabundleinjector/secret.gopkg/controller/cabundleinjector/starter.gotest/e2e/e2e.gotest/e2e/e2e_test.go
✅ Files skipped from review due to trivial changes (1)
- README.md
🚧 Files skipped from review as they are similar to previous changes (3)
- pkg/controller/cabundleinjector/starter.go
- test/e2e/e2e_test.go
- pkg/controller/cabundleinjector/secret.go
Closes openshift#264 Signed-off-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>
mnencia
force-pushed
the
inject-ca-into-secrets
branch
from
21e7c2a to
8b76ab6
Compare
|
Rebased on current I've replied inline to the CodeRabbit threads: both flag divergences from Could a maintainer please |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
test/e2e/e2e.go (1)
853-864:⚠️ Potential issue | 🟠 MajorWait for the injected key, not just Secret creation.
This poll returns success as soon as the Secret exists, so
checkSecretCABundleInjectionDatacan still race the controller and fail intermittently. Poll untilapi.InjectionDataKeyis present instead.Suggested fix
func pollForCABundleInjectionSecret(client *kubernetes.Clientset, secretName, namespace string) error { - return wait.PollImmediate(time.Second, 10*time.Second, func() (bool, error) { - _, err := client.CoreV1().Secrets(namespace).Get(context.TODO(), secretName, metav1.GetOptions{}) + return wait.PollImmediate(time.Second, pollTimeout, func() (bool, error) { + secret, err := client.CoreV1().Secrets(namespace).Get(context.TODO(), secretName, metav1.GetOptions{}) if err != nil && errors.IsNotFound(err) { return false, nil } if err != nil { return false, err } + if len(secret.Data) != 1 { + return false, nil + } + if _, ok := secret.Data[api.InjectionDataKey]; !ok { + return false, nil + } return true, nil }) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/e2e.go` around lines 853 - 864, The pollForCABundleInjectionSecret function currently returns success as soon as the Secret exists; update it to fetch the Secret via client.CoreV1().Secrets(...).Get and inspect the returned Secret.Data to ensure the api.InjectionDataKey key exists (and is non-empty) before returning true so checkSecretCABundleInjectionData cannot race the controller; keep the same wait.PollImmediate structure but change the success condition to check for secret.Data[api.InjectionDataKey] presence (and handle nil/empty) and continue polling on not-found or missing key, returning errors only on real Get failures.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/e2e/e2e.go`:
- Around line 880-896: The test helper editSecretCABundleInjectionData currently
adds a new "foo" key, which doesn't verify the injected entry gets restored;
change it to overwrite the injected key by setting
secretCopy.Data["service-ca.crt"] = []byte("corrupt") (after ensuring
secretCopy.Data is non-nil and contains "service-ca.crt"), update the secret via
client.CoreV1().Secrets(...).Update as before, and call
pollForSecretDataChange(t, client, secretCopy, "service-ca.crt") so the test
asserts the corrupted service-ca.crt is re-stomped.
---
Duplicate comments:
In `@test/e2e/e2e.go`:
- Around line 853-864: The pollForCABundleInjectionSecret function currently
returns success as soon as the Secret exists; update it to fetch the Secret via
client.CoreV1().Secrets(...).Get and inspect the returned Secret.Data to ensure
the api.InjectionDataKey key exists (and is non-empty) before returning true so
checkSecretCABundleInjectionData cannot race the controller; keep the same
wait.PollImmediate structure but change the success condition to check for
secret.Data[api.InjectionDataKey] presence (and handle nil/empty) and continue
polling on not-found or missing key, returning errors only on real Get failures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 100f97f3-36a3-4b9c-9741-4cd95ce0f11b
📒 Files selected for processing (5)
README.mdpkg/controller/cabundleinjector/secret.gopkg/controller/cabundleinjector/starter.gotest/e2e/e2e.gotest/e2e/e2e_test.go
✅ Files skipped from review due to trivial changes (1)
- pkg/controller/cabundleinjector/secret.go
🚧 Files skipped from review as they are similar to previous changes (2)
- test/e2e/e2e_test.go
- README.md
| func editSecretCABundleInjectionData(t testing.TB, client *kubernetes.Clientset, secretName, namespace string) error { | ||
| secret, err := client.CoreV1().Secrets(namespace).Get(context.TODO(), secretName, metav1.GetOptions{}) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| secretCopy := secret.DeepCopy() | ||
| if len(secretCopy.Data) != 1 { | ||
| return fmt.Errorf("ca bundle injection secret missing data") | ||
| } | ||
| secretCopy.Data["foo"] = []byte("blah") | ||
| _, err = client.CoreV1().Secrets(namespace).Update(context.TODO(), secretCopy, metav1.UpdateOptions{}) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| return pollForSecretDataChange(t, client, secretCopy, "foo") | ||
| } |
There was a problem hiding this comment.
Overwrite the injected entry in the update test.
This helper mutates a new "foo" key, so the test only proves extra keys are cleaned up. It never verifies that a corrupted service-ca.crt value is re-stomped.
Suggested fix
func editSecretCABundleInjectionData(t testing.TB, client *kubernetes.Clientset, secretName, namespace string) error {
secret, err := client.CoreV1().Secrets(namespace).Get(context.TODO(), secretName, metav1.GetOptions{})
if err != nil {
return err
}
secretCopy := secret.DeepCopy()
if len(secretCopy.Data) != 1 {
return fmt.Errorf("ca bundle injection secret missing data")
}
- secretCopy.Data["foo"] = []byte("blah")
+ secretCopy.Data[api.InjectionDataKey] = []byte("blah")
_, err = client.CoreV1().Secrets(namespace).Update(context.TODO(), secretCopy, metav1.UpdateOptions{})
if err != nil {
return err
}
- return pollForSecretDataChange(t, client, secretCopy, "foo")
+ return pollForSecretDataChange(t, client, secretCopy, api.InjectionDataKey)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func editSecretCABundleInjectionData(t testing.TB, client *kubernetes.Clientset, secretName, namespace string) error { | |
| secret, err := client.CoreV1().Secrets(namespace).Get(context.TODO(), secretName, metav1.GetOptions{}) | |
| if err != nil { | |
| return err | |
| } | |
| secretCopy := secret.DeepCopy() | |
| if len(secretCopy.Data) != 1 { | |
| return fmt.Errorf("ca bundle injection secret missing data") | |
| } | |
| secretCopy.Data["foo"] = []byte("blah") | |
| _, err = client.CoreV1().Secrets(namespace).Update(context.TODO(), secretCopy, metav1.UpdateOptions{}) | |
| if err != nil { | |
| return err | |
| } | |
| return pollForSecretDataChange(t, client, secretCopy, "foo") | |
| } | |
| func editSecretCABundleInjectionData(t testing.TB, client *kubernetes.Clientset, secretName, namespace string) error { | |
| secret, err := client.CoreV1().Secrets(namespace).Get(context.TODO(), secretName, metav1.GetOptions{}) | |
| if err != nil { | |
| return err | |
| } | |
| secretCopy := secret.DeepCopy() | |
| if len(secretCopy.Data) != 1 { | |
| return fmt.Errorf("ca bundle injection secret missing data") | |
| } | |
| secretCopy.Data[api.InjectionDataKey] = []byte("blah") | |
| _, err = client.CoreV1().Secrets(namespace).Update(context.TODO(), secretCopy, metav1.UpdateOptions{}) | |
| if err != nil { | |
| return err | |
| } | |
| return pollForSecretDataChange(t, client, secretCopy, api.InjectionDataKey) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/e2e/e2e.go` around lines 880 - 896, The test helper
editSecretCABundleInjectionData currently adds a new "foo" key, which doesn't
verify the injected entry gets restored; change it to overwrite the injected key
by setting secretCopy.Data["service-ca.crt"] = []byte("corrupt") (after ensuring
secretCopy.Data is non-nil and contains "service-ca.crt"), update the secret via
client.CoreV1().Secrets(...).Update as before, and call
pollForSecretDataChange(t, client, secretCopy, "service-ca.crt") so the test
asserts the corrupted service-ca.crt is re-stomped.
There was a problem hiding this comment.
Same symmetry argument as the other threads: this is a verbatim mirror of editConfigMapCABundleInjectionData, which also adds a "foo" key and polls for change on "foo" rather than overwriting InjectionDataKey. The test pair asserts the injector's contract that the target object is exclusively owned by the injector, which is what "stomping" demonstrates.
Strengthening coverage to also verify that a corrupted service-ca.crt value gets re-injected is a reasonable addition, but should be done symmetrically in both helpers in a follow-up PR rather than diverging the two here.
There was a problem hiding this comment.
@mnencia, understood — the symmetry with editConfigMapCABundleInjectionData is intentional, and the "foo" key approach tests the exclusive-ownership ("stomping") contract consistently across both resource types. Keeping them in sync makes the test structure easier to reason about and avoids an asymmetric divergence mid-PR.
Would you like me to open a follow-up GitHub issue to track strengthening both editConfigMapCABundleInjectionData and editSecretCABundleInjectionData to also verify that a corrupted injected key value gets re-stomped?
There was a problem hiding this comment.
Yes please, a follow-up issue to track strengthening both helpers symmetrically would be useful. Thanks.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
gh issue create \
--repo openshift/service-ca-operator \
--title "Strengthen CA bundle injection update tests to also verify corrupted injected key is re-stomped" \
--body "## Summary
The e2e helpers \`editConfigMapCABundleInjectionData\` and \`editSecretCABundleInjectionData\` in \`test/e2e/e2e.go\` currently test the injector's exclusive-ownership contract by adding a foreign \`\"foo\"\` key and asserting it is removed. This verifies that the injector \"stomps\" the object back to a single controlled entry, but does **not** verify that a corrupted value for the injected key (\`api.InjectionDataKey\` / \`service-ca.crt\`) is also restored.
## Desired change
Both helpers should be strengthened **symmetrically** to additionally overwrite \`secretCopy.Data[api.InjectionDataKey]\` (or \`cmCopy.Data[api.InjectionDataKey]\`) with a known-bad value and poll until it is reverted to the correct CA bundle. This would cover the scenario where the injected entry itself is tampered with.
The change should be made to both helpers in the same PR to keep the two resource types in sync:
- \`editConfigMapCABundleInjectionData\` in \`test/e2e/e2e.go\`
- \`editSecretCABundleInjectionData\` in \`test/e2e/e2e.go\`
## Background
Discussed in PR `#265` (adding Secret CA bundle injection support) at:
https://github.com/openshift/service-ca-operator/pull/265#discussion_r3109380800
Requested by `@mnencia`." \
--assignee mnencia
Length of output: 76
🏁 Script executed:
gh issue create \
--repo openshift/service-ca-operator \
--title "Strengthen CA bundle injection update tests to also verify corrupted injected key is re-stomped" \
--body "## Summary
The e2e helpers \`editConfigMapCABundleInjectionData\` and \`editSecretCABundleInjectionData\` in \`test/e2e/e2e.go\` currently test the injector's exclusive-ownership contract by adding a foreign \`\"foo\"\` key and asserting it is removed. This verifies that the injector \"stomps\" the object back to a single controlled entry, but does **not** verify that a corrupted value for the injected key (\`api.InjectionDataKey\` / \`service-ca.crt\`) is also restored.
## Desired change
Both helpers should be strengthened **symmetrically** to additionally overwrite \`secretCopy.Data[api.InjectionDataKey]\` (or \`cmCopy.Data[api.InjectionDataKey]\`) with a known-bad value and poll until it is reverted to the correct CA bundle. This would cover the scenario where the injected entry itself is tampered with.
The change should be made to both helpers in the same PR to keep the two resource types in sync:
- \`editConfigMapCABundleInjectionData\` in \`test/e2e/e2e.go\`
- \`editSecretCABundleInjectionData\` in \`test/e2e/e2e.go\`
## Background
Discussed in PR `#265` (adding Secret CA bundle injection support) at:
https://github.com/openshift/service-ca-operator/pull/265#discussion_r3109380800
Requested by `@mnencia`."
Length of output: 59
@mnencia, the follow-up issue has been created: openshift/service-ca-operator#347 — Strengthen CA bundle injection update tests to also verify corrupted injected key is re-stomped
Note: I wasn't able to assign it automatically due to integration permission limits — you may want to assign it manually.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
gh issue close 347 --repo openshift/service-ca-operator --comment "Closing as duplicate of `#346` — both issues were created identically due to a retry. Tracking in `#346`."
Length of output: 60
🏁 Script executed:
gh issue close 347 --repo openshift/service-ca-operator
Length of output: 60
@mnencia, I'm sorry — I'm unable to close issue #347 programmatically. The GitHub integration returns Resource not accessible by integration for issue write operations on this repository, so I don't have the necessary permissions to close it.
Could you please close #347 manually? You can do so by navigating to #347 and clicking Close issue. Apologies for the duplicate!
3 participants
This pull request adds support for injecting the service CA bundle into Secrets to the existing functionality for ConfigMaps.
Closes #264.
Summary by CodeRabbit
New Features
Documentation
Tests