feat: add Secret/ConfigMap references for JWT CA certificates#772
Open
bkhizgiy wants to merge 1 commit into
Open
feat: add Secret/ConfigMap references for JWT CA certificates#772bkhizgiy wants to merge 1 commit into
bkhizgiy wants to merge 1 commit into
Conversation
Member
Author
|
@ambient-code please review |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughJWT authenticators may reference CA PEM stored in Secrets or ConfigMaps. The controller resolves referenced CA PEM during config build, inlines it into the generated controller ConfigMap, computes a deterministic hash from the ConfigMap data, and watches referenced Secrets/ConfigMaps to trigger reconciles on rotation. ChangesJWT CA Certificate References in Jumpstarter
Sequence DiagramsequenceDiagram
participant JumpstarterCR
participant ControllerReconcile
participant buildConfig
participant resolveJWTAuthenticators
participant KubernetesAPI
participant ControllerConfigMap
JumpstarterCR->>ControllerReconcile: trigger (watch/manual)
ControllerReconcile->>buildConfig: buildConfig(ctx, jumpstarter)
buildConfig->>resolveJWTAuthenticators: resolve JWT authenticators
loop per JWT entry with CA reference
resolveJWTAuthenticators->>KubernetesAPI: GET Secret(namespace,name) or ConfigMap(namespace,name)
KubernetesAPI-->>resolveJWTAuthenticators: resource.data
resolveJWTAuthenticators->>resolveJWTAuthenticators: extract key, inline PEM into Issuer.CertificateAuthority
end
resolveJWTAuthenticators-->>buildConfig: resolved JWT authenticators
buildConfig-->>ControllerReconcile: rendered config with inlined PEM
ControllerReconcile->>ControllerReconcile: compute configMapDataHash from Data
ControllerReconcile->>ControllerConfigMap: create/update ConfigMap with hash annotation
Note over KubernetesAPI: CA Secret/ConfigMap changes
KubernetesAPI->>ControllerReconcile: watch enqueue (via Secrets/ConfigMaps watch)
ControllerReconcile->>buildConfig: rebuild config (new CA)
buildConfig->>resolveJWTAuthenticators: resolve updated CA
resolveJWTAuthenticators-->>buildConfig: resolved JWT with new PEM
ControllerReconcile->>ControllerConfigMap: update ConfigMap with new PEM
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
bkhizgiy
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
controller/deploy/operator/internal/controller/jumpstarter/jumpstarter_controller_test.go (1)
237-238: ⚡ Quick winStrengthen CA assertions to verify the expected PEM payload, not only a generic marker.
ContainSubstring("BEGIN CERTIFICATE")can pass even if the wrong certificate is rendered. AsserttestPEM(and after rotation,testPEM2plus absence oftestPEM) to lock the behavior to this feature.Proposed assertion updates
-Expect(getConfigData()).To(ContainSubstring("BEGIN CERTIFICATE")) +Expect(getConfigData()).To(ContainSubstring(testPEM))-Expect(getConfigData()).To(ContainSubstring("BEGIN CERTIFICATE")) +Expect(getConfigData()).To(ContainSubstring(testPEM))Expect(secondConfig).To(ContainSubstring("BEGIN CERTIFICATE")) -Expect(secondConfig).NotTo(Equal(firstConfig)) +Expect(secondConfig).To(ContainSubstring(testPEM2)) +Expect(secondConfig).NotTo(ContainSubstring(testPEM)) +Expect(secondConfig).NotTo(Equal(firstConfig))Also applies to: 278-279, 369-370
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controller/deploy/operator/internal/controller/jumpstarter/jumpstarter_controller_test.go` around lines 237 - 238, Replace the weak substring assertions against certificate output with exact PEM-value checks: in the test case(s) that call getConfigData() (and any related checks after rotation), assert that the returned string contains the expected test PEM payload variable testPEM; for rotation scenarios assert it contains testPEM2 and does not contain testPEM. Locate assertions around getConfigData() and update them to use positive containment of testPEM / testPEM2 and negative containment (Not/ToNot) for the old PEM after rotation so the tests validate the actual certificate payload rather than just "BEGIN CERTIFICATE".controller/deploy/operator/internal/controller/jumpstarter/ca_resolution_test.go (1)
323-326: ⚡ Quick winAdd assertion for
"jwt[0]"in the error message to maintain consistency.All other error test cases verify that the error message includes the JWT entry index (e.g.,
"jwt[0]"). This test should follow the same pattern for consistency and to confirm that error messages properly identify which JWT entry caused the failure.✅ Suggested addition
_, err := reconciler.resolveJWTAuthenticators(ctx, js) Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("jwt[0]")) Expect(err.Error()).To(ContainSubstring(`key "wrong-key" not found`)) })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controller/deploy/operator/internal/controller/jumpstarter/ca_resolution_test.go` around lines 323 - 326, Update the test assertion in ca_resolution_test.go that calls reconciler.resolveJWTAuthenticators(ctx, js) so the expected error message includes the JWT index; specifically, adjust the Expect(err.Error()).To(ContainSubstring(...)) to assert the error contains both `jwt[0]` and the existing `key "wrong-key" not found` fragment (i.e., verify the message references "jwt[0]" as well as the missing key) so resolveJWTAuthenticators' error identification remains consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@controller/deploy/operator/api/v1alpha1/jumpstarter_types.go`:
- Around line 402-425: The current cross-namespace references (fields Namespace
on Secret/ConfigMap selectors defined in types ConfigMapKeySelector and the
Secret selector that backs
certificateAuthoritySecret/certificateAuthorityConfigMap) allow arbitrary
Jumpstarter CRs to point at Secrets/ConfigMaps in other namespaces; update the
API/behavior to prevent silent cross-namespace data exfiltration by either (a)
restricting the Namespace fields so selectors are only valid when equal to the
Jumpstarter CR namespace (remove or validate external Namespace values in the
ConfigMapKeySelector and Secret selector parsing), or (b) add an explicit
authorization/allowlist check in the resolver (the code path in
controller/deploy/operator/internal/controller/jumpstarter/jumpstarter_controller.go
that reads certificateAuthoritySecret/certificateAuthorityConfigMap) to verify
the caller is permitted to read the target namespace before copying bytes;
ensure the change references ConfigMapKeySelector, the Secret selector struct,
and the certificateAuthoritySecret/certificateAuthorityConfigMap handling so
reviewers can find and update both the type validation and the controller
resolver logic.
In
`@controller/deploy/operator/internal/controller/jumpstarter/jumpstarter_controller_test.go`:
- Around line 142-160: The shared test fixture makeJumpstarterSpec returns a
JumpstarterSpec but omits cert-manager configuration; update makeJumpstarterSpec
so the returned JumpstarterSpec has CertManager.Enabled = false (i.e., set
spec.CertManager.Enabled to false on the returned struct) to ensure
envtest-compatible setup for JWT tests and avoid unrelated reconciliation
failures.
In
`@controller/deploy/operator/internal/controller/jumpstarter/jumpstarter_controller.go`:
- Around line 170-185: You built the desired ConfigMap early (createConfigMap
and configMapDataHash) which is fine for computing the pod template hash, but
you must not call reconcileConfigMaps yet—move the call to reconcileConfigMaps
so it runs after Services/networking reconciliation and before Secrets
reconciliation (i.e., keep createConfigMap and configMapDataHash where they are,
but relocate the reconcileConfigMaps(...) invocation to the documented phase
between reconcileServices/reconcileNetworking and reconcileSecrets) to restore
the required reconcile ordering for this controller.
In `@controller/deploy/operator/test/e2e/e2e_test.go`:
- Around line 2028-2058: The test currently only checks that the controller
ConfigMap changed and contains a certificate; update the eventual assertion
block (the code that fetches updatedCM) to also verify the actual rotated CA PEM
was applied by asserting that updatedCM.Data["config"] contains the rotatedPEM
value (or its string form) you set on the jwt-ca-secret; reference the
rotatedPEM variable and updatedCM.Data["config"] in the assertion so the test
fails if the ConfigMap was changed for unrelated reasons.
- Around line 1967-2026: The Jumpstarter test is missing required controller and
router fields so the operator cannot reconcile; update the Jumpstarter spec in
the test (the js variable in the It block that creates the Jumpstarter) to
include controller.image, controller.replicas, controller.grpc.endpoints and a
routers section matching other e2e tests so the controller Deployment and
ConfigMap are created, and replace the weak assertion that only checks for
"BEGIN CERTIFICATE" with an exact containment check for the fakePEM variable
(verify cm.Data["config"] contains fakePEM) to ensure the CA PEM was inlined.
---
Nitpick comments:
In
`@controller/deploy/operator/internal/controller/jumpstarter/ca_resolution_test.go`:
- Around line 323-326: Update the test assertion in ca_resolution_test.go that
calls reconciler.resolveJWTAuthenticators(ctx, js) so the expected error message
includes the JWT index; specifically, adjust the
Expect(err.Error()).To(ContainSubstring(...)) to assert the error contains both
`jwt[0]` and the existing `key "wrong-key" not found` fragment (i.e., verify the
message references "jwt[0]" as well as the missing key) so
resolveJWTAuthenticators' error identification remains consistent.
In
`@controller/deploy/operator/internal/controller/jumpstarter/jumpstarter_controller_test.go`:
- Around line 237-238: Replace the weak substring assertions against certificate
output with exact PEM-value checks: in the test case(s) that call
getConfigData() (and any related checks after rotation), assert that the
returned string contains the expected test PEM payload variable testPEM; for
rotation scenarios assert it contains testPEM2 and does not contain testPEM.
Locate assertions around getConfigData() and update them to use positive
containment of testPEM / testPEM2 and negative containment (Not/ToNot) for the
old PEM after rotation so the tests validate the actual certificate payload
rather than just "BEGIN CERTIFICATE".
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b711568e-26cd-4a30-aa39-2a76ba69bba0
📒 Files selected for processing (7)
controller/deploy/operator/api/v1alpha1/jumpstarter_types.gocontroller/deploy/operator/api/v1alpha1/zz_generated.deepcopy.gocontroller/deploy/operator/config/crd/bases/operator.jumpstarter.dev_jumpstarters.yamlcontroller/deploy/operator/internal/controller/jumpstarter/ca_resolution_test.gocontroller/deploy/operator/internal/controller/jumpstarter/jumpstarter_controller.gocontroller/deploy/operator/internal/controller/jumpstarter/jumpstarter_controller_test.gocontroller/deploy/operator/test/e2e/e2e_test.go
Comment on lines
+402
to
+425
| // Namespace of the Secret. Defaults to the namespace of the Jumpstarter CR. | ||
| // Cross-namespace references are allowed so that cluster-level CA secrets | ||
| // (e.g. openshift-ingress-operator/router-ca) can be consumed directly. | ||
| // +optional | ||
| Namespace string `json:"namespace,omitempty"` | ||
|
|
||
| // Key within the Secret that holds the PEM-encoded CA certificate. | ||
| // +kubebuilder:validation:Required | ||
| // +kubebuilder:validation:MinLength=1 | ||
| Key string `json:"key"` | ||
| } | ||
|
|
||
| // ConfigMapKeySelector references a key within a Kubernetes ConfigMap. | ||
| type ConfigMapKeySelector struct { | ||
| // Name of the ConfigMap containing the CA certificate. | ||
| // +kubebuilder:validation:Required | ||
| // +kubebuilder:validation:MinLength=1 | ||
| Name string `json:"name"` | ||
|
|
||
| // Namespace of the ConfigMap. Defaults to the namespace of the Jumpstarter CR. | ||
| // Cross-namespace references are allowed so that cluster-level CA ConfigMaps | ||
| // (e.g. kube-root-ca.crt) can be consumed directly. | ||
| // +optional | ||
| Namespace string `json:"namespace,omitempty"` |
Contributor
There was a problem hiding this comment.
Cross-namespace CA refs make arbitrary Secret reads possible.
Lines 402-425 let a namespaced Jumpstarter point certificateAuthoritySecret / certificateAuthorityConfigMap at another namespace. The resolver in controller/deploy/operator/internal/controller/jumpstarter/jumpstarter_controller.go follows that override with the operator's cluster-wide Secret/ConfigMap access and copies the fetched bytes into the controller ConfigMap without verifying that the payload is actually a CA PEM. That turns this API into a cross-namespace data exfiltration path for any principal that can create or update a Jumpstarter. Please scope these refs to the CR namespace, or add an explicit allowlist/authorization gate before keeping cross-namespace support.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@controller/deploy/operator/api/v1alpha1/jumpstarter_types.go` around lines
402 - 425, The current cross-namespace references (fields Namespace on
Secret/ConfigMap selectors defined in types ConfigMapKeySelector and the Secret
selector that backs certificateAuthoritySecret/certificateAuthorityConfigMap)
allow arbitrary Jumpstarter CRs to point at Secrets/ConfigMaps in other
namespaces; update the API/behavior to prevent silent cross-namespace data
exfiltration by either (a) restricting the Namespace fields so selectors are
only valid when equal to the Jumpstarter CR namespace (remove or validate
external Namespace values in the ConfigMapKeySelector and Secret selector
parsing), or (b) add an explicit authorization/allowlist check in the resolver
(the code path in
controller/deploy/operator/internal/controller/jumpstarter/jumpstarter_controller.go
that reads certificateAuthoritySecret/certificateAuthorityConfigMap) to verify
the caller is permitted to read the target namespace before copying bytes;
ensure the change references ConfigMapKeySelector, the Secret selector struct,
and the certificateAuthoritySecret/certificateAuthorityConfigMap handling so
reviewers can find and update both the type validation and the controller
resolver logic.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
controller/deploy/operator/config/crd/bases/operator.jumpstarter.dev_jumpstarters.yaml (2)
104-130: ⚡ Quick winClarify which ConfigMap gets updated on line 109.
Line 109 states "updates the ConfigMap" which is ambiguous—it could refer to the referenced ConfigMap or the controller's generated ConfigMap. Clarify that the operator updates the controller ConfigMap (the generated configuration), not the referenced ConfigMap.
Suggestion: "When the ConfigMap changes, the operator reconciles and updates the controller ConfigMap."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controller/deploy/operator/config/crd/bases/operator.jumpstarter.dev_jumpstarters.yaml` around lines 104 - 130, The description for certificateAuthorityConfigMap is ambiguous about which ConfigMap is updated; update the sentence in the certificateAuthorityConfigMap description (in operator.jumpstarter.dev_jumpstarters.yaml, inside the certificateAuthorityConfigMap property) to explicitly state that the operator reconciles and updates the controller ConfigMap (the generated configuration) rather than the referenced ConfigMap—for example, replace "When the ConfigMap changes, the operator reconciles and updates the ConfigMap." with a sentence like "When the referenced ConfigMap changes, the operator reconciles and updates the controller ConfigMap (the generated configuration)."
131-157: ⚡ Quick winClarify which ConfigMap gets updated on line 136.
Line 136 has the same ambiguity as line 109: "updates the ConfigMap" should specify that the operator updates the controller ConfigMap (the generated configuration), not the referenced Secret.
Suggestion: "When the Secret changes, the operator reconciles and updates the controller ConfigMap."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controller/deploy/operator/config/crd/bases/operator.jumpstarter.dev_jumpstarters.yaml` around lines 131 - 157, Update the description for the certificateAuthoritySecret object (property certificateAuthoritySecret) to explicitly state that when the referenced Secret changes the operator reconciles and updates the controller ConfigMap (the generated controller configuration), not the referenced Secret; replace the ambiguous phrase "updates the ConfigMap" with "updates the controller ConfigMap" and ensure the wording parallels the existing CertificateAuthorityConfigMap description for consistency with Jumpstarter CR and controller behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@controller/deploy/operator/config/crd/bases/operator.jumpstarter.dev_jumpstarters.yaml`:
- Around line 104-130: The description for certificateAuthorityConfigMap is
ambiguous about which ConfigMap is updated; update the sentence in the
certificateAuthorityConfigMap description (in
operator.jumpstarter.dev_jumpstarters.yaml, inside the
certificateAuthorityConfigMap property) to explicitly state that the operator
reconciles and updates the controller ConfigMap (the generated configuration)
rather than the referenced ConfigMap—for example, replace "When the ConfigMap
changes, the operator reconciles and updates the ConfigMap." with a sentence
like "When the referenced ConfigMap changes, the operator reconciles and updates
the controller ConfigMap (the generated configuration)."
- Around line 131-157: Update the description for the certificateAuthoritySecret
object (property certificateAuthoritySecret) to explicitly state that when the
referenced Secret changes the operator reconciles and updates the controller
ConfigMap (the generated controller configuration), not the referenced Secret;
replace the ambiguous phrase "updates the ConfigMap" with "updates the
controller ConfigMap" and ensure the wording parallels the existing
CertificateAuthorityConfigMap description for consistency with Jumpstarter CR
and controller behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6b13e090-5124-4256-80d2-97b59f7781d2
📒 Files selected for processing (8)
controller/deploy/operator/api/v1alpha1/jumpstarter_types.gocontroller/deploy/operator/api/v1alpha1/zz_generated.deepcopy.gocontroller/deploy/operator/config/crd/bases/operator.jumpstarter.dev_jumpstarters.yamlcontroller/deploy/operator/internal/controller/jumpstarter/ca_resolution_test.gocontroller/deploy/operator/internal/controller/jumpstarter/jumpstarter_controller.gocontroller/deploy/operator/internal/controller/jumpstarter/jumpstarter_controller_test.gocontroller/deploy/operator/test/e2e/e2e_test.gotypos.toml
✅ Files skipped from review due to trivial changes (2)
- typos.toml
- controller/deploy/operator/api/v1alpha1/zz_generated.deepcopy.go
🚧 Files skipped from review as they are similar to previous changes (5)
- controller/deploy/operator/test/e2e/e2e_test.go
- controller/deploy/operator/api/v1alpha1/jumpstarter_types.go
- controller/deploy/operator/internal/controller/jumpstarter/jumpstarter_controller_test.go
- controller/deploy/operator/internal/controller/jumpstarter/ca_resolution_test.go
- controller/deploy/operator/internal/controller/jumpstarter/jumpstarter_controller.go
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@controller/deploy/operator/internal/controller/jumpstarter/jumpstarter_controller_test.go`:
- Around line 206-245: Add a string constant named testPEM containing a
PEM-formatted certificate (including the "-----BEGIN CERTIFICATE-----" header)
and place it near the top of the test suite (e.g., after the other test-level
declarations) so the reference in the test that creates the Secret (which uses
caSecretName and caKey) compiles; ensure the constant value includes the full
PEM delimiters and newlines so the
Expect(getConfigData()).To(ContainSubstring("BEGIN CERTIFICATE")) assertion
passes.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 508ccc00-0fa6-4c22-99a2-ca3117a3633b
📒 Files selected for processing (8)
controller/deploy/operator/api/v1alpha1/jumpstarter_types.gocontroller/deploy/operator/api/v1alpha1/zz_generated.deepcopy.gocontroller/deploy/operator/config/crd/bases/operator.jumpstarter.dev_jumpstarters.yamlcontroller/deploy/operator/internal/controller/jumpstarter/ca_resolution_test.gocontroller/deploy/operator/internal/controller/jumpstarter/jumpstarter_controller.gocontroller/deploy/operator/internal/controller/jumpstarter/jumpstarter_controller_test.gocontroller/deploy/operator/test/e2e/e2e_test.gotypos.toml
✅ Files skipped from review due to trivial changes (2)
- typos.toml
- controller/deploy/operator/api/v1alpha1/zz_generated.deepcopy.go
🚧 Files skipped from review as they are similar to previous changes (5)
- controller/deploy/operator/test/e2e/e2e_test.go
- controller/deploy/operator/config/crd/bases/operator.jumpstarter.dev_jumpstarters.yaml
- controller/deploy/operator/api/v1alpha1/jumpstarter_types.go
- controller/deploy/operator/internal/controller/jumpstarter/jumpstarter_controller.go
- controller/deploy/operator/internal/controller/jumpstarter/ca_resolution_test.go
Comment on lines
+206
to
+245
| It("inlines the CA PEM from a Secret reference into the controller ConfigMap", func() { | ||
| By("creating a CA Secret") | ||
| Expect(k8sClient.Create(ctx, &corev1.Secret{ | ||
| ObjectMeta: metav1.ObjectMeta{Name: caSecretName, Namespace: crNamespace}, | ||
| Data: map[string][]byte{caKey: []byte(testPEM)}, | ||
| })).To(Succeed()) | ||
|
|
||
| By("creating a Jumpstarter CR with a certificateAuthoritySecret reference") | ||
| spec := makeJumpstarterSpec() | ||
| spec.Authentication.JWT = []operatorv1alpha1.JWTAuthenticatorConfig{ | ||
| { | ||
| JWTAuthenticator: apiserverv1beta1.JWTAuthenticator{ | ||
| Issuer: apiserverv1beta1.Issuer{ | ||
| URL: "https://issuer.example.com", | ||
| Audiences: []string{"jumpstarter-cli"}, | ||
| }, | ||
| ClaimMappings: apiserverv1beta1.ClaimMappings{ | ||
| Username: apiserverv1beta1.PrefixedClaimOrExpression{ | ||
| Claim: "preferred_username", | ||
| Prefix: strPtr("oidc:"), | ||
| }, | ||
| }, | ||
| }, | ||
| CertificateAuthoritySecret: &operatorv1alpha1.SecretKeySelector{ | ||
| Name: caSecretName, | ||
| Key: caKey, | ||
| }, | ||
| }, | ||
| } | ||
| Expect(k8sClient.Create(ctx, &operatorv1alpha1.Jumpstarter{ | ||
| ObjectMeta: metav1.ObjectMeta{Name: crName, Namespace: crNamespace}, | ||
| Spec: spec, | ||
| })).To(Succeed()) | ||
|
|
||
| By("reconciling") | ||
| doReconcile() | ||
|
|
||
| By("verifying the CA PEM is inlined in the controller ConfigMap") | ||
| Expect(getConfigData()).To(ContainSubstring("BEGIN CERTIFICATE")) | ||
| }) |
Contributor
There was a problem hiding this comment.
Define the testPEM constant.
Line 210 references testPEM, but this constant is not defined in the visible code. This will cause a compilation error.
🔧 Proposed fix
Add the constant definition near the top of the test suite (after line 137):
const caKeyPEM = "ca.crt"
const crName = "test-jwt-ca"
+
+ const testPEM = `-----BEGIN CERTIFICATE-----
+MIIBkTCB+wIJAKHHCgVZU7POMA0GCSqGSIb3DQEBCwUAMBExDzANBgNVBAMMBnRl
+c3QtY2EwHhcNMjQwMTAxMDAwMDAwWhcNMjUwMTAxMDAwMDAwWjARMQ8wDQYDVQQD
+DAZ0ZXN0LWNhMIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQC7VJTUt9Us8cKj
+MzEfYyjiWA4R4ypbHrAjfYdCw5w4UZQwpqNkHwVlN2P1pQNYaOKs0N0TJBmYn8P8
+0N5RKQvvjZx4w2rUZCdW4P4/tL/0N9vZ8TJZYwJ5J5J5J5J5J5J5J5J5J5J5J5J5
+J5J5J5J5J5J5J5J5J5J5J5J5J5J5J5J5J5J5J5J5J5JQIDAQABMA0GCSqGSIb3DQEB
+CwUAA4GBAGvLPF1gG0L0vN4wU0N0wU0N0wU0N0wU0N0wU0N0wU0N0wU0N0wU0N0w
+U0N0wU0N0wU0N0wU0N0wU0N0wU0N0wU0N0wU0N0wU0N0wU0N0wU0N0wU0N0w
+-----END CERTIFICATE-----`
+
+ const testPEM2 = `-----BEGIN CERTIFICATE-----
+MIIBkjCB/AIJALLLDhWaV8QRMA0GCSqGSIb3DQEBCwUAMBIxEDAOBgNVBAMMB3Jv
+dGF0ZWQwHhcNMjQwMjAxMDAwMDAwWhcNMjUwMjAxMDAwMDAwWjASMRAwDgYDVQQD
+DAd0b3RhdGVkMIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDMVKUVv9Vt9dLl
+NzFgZzjkXB5S5qcCjgBfYeDxz6x5V1RLrQoxF2Q2OxH1W2Q2OxH1W2Q2OxH1W2Q2
+OxH1W2Q2OxH1W2Q2OxH1W2Q2OxH1W2Q2OxH1W2Q2OxH1W2Q2OxH1W2Q2OxH1W2Q2
+OxH1W2Q2OxH1W2Q2OxH1W2Q2OxH1W2Q2OxH1W2QIDAQABMA0GCSqGSIb3DQEBCwUA
+A4GBAHwMQG2hH3N1x3N1x3N1x3N1x3N1x3N1x3N1x3N1x3N1x3N1x3N1x3N1x3N1
+x3N1x3N1x3N1x3N1x3N1x3N1x3N1x3N1x3N1x3N1x3N1x3N1x3N1x3N1x3N1
+-----END CERTIFICATE-----`🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@controller/deploy/operator/internal/controller/jumpstarter/jumpstarter_controller_test.go`
around lines 206 - 245, Add a string constant named testPEM containing a
PEM-formatted certificate (including the "-----BEGIN CERTIFICATE-----" header)
and place it near the top of the test suite (e.g., after the other test-level
declarations) so the reference in the test that creates the Secret (which uses
caSecretName and caKey) compiles; ensure the constant value includes the full
PEM delimiters and newlines so the
Expect(getConfigData()).To(ContainSubstring("BEGIN CERTIFICATE")) assertion
passes.
Signed-off-by: Bella Khizgiyaev <bkhizgiy@redhat.com> Assisted-by: Claude Sonnet 4.6 <claude@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
related to #646