OCPBUGS-84551: fix(ingress): set FIPS_ENABLED env var on ingress operator#8375
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
jira/valid-reference
|
@csrwng: This pull request references Jira Issue OCPBUGS-84551, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe changes update the ingress operator deployment adaptation to check 🚥 Pre-merge checks | ✅ 11 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (11 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Review rate limit: 8/10 reviews remaining, refill in 6 minutes and 4 seconds. Comment |
openshift-ci
Bot
added
area/control-plane-operator
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: csrwng The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8375 +/- ##
==========================================
+ Coverage 36.48% 36.50% +0.01%
==========================================
Files 765 765
Lines 93266 93267 +1
==========================================
+ Hits 34032 34050 +18
+ Misses 56519 56501 -18
- Partials 2715 2716 +1
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
/lgtm |
|
Scheduling tests matching the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
control-plane-operator/controllers/hostedcontrolplane/v2/ingressoperator/deployment.go (1)
25-29: MakeFIPS_ENABLEDexplicit for both FIPS states.Current logic sets
FIPS_ENABLEDonly on true, but never clears it on false. Making both branches explicit keepsadaptDeploymentidempotent even if the input deployment was previously mutated.Suggested diff
if cpContext.HCP.Spec.FIPS { podspec.UpsertEnvVar(c, corev1.EnvVar{ Name: "FIPS_ENABLED", Value: "true", }) + } else { + filtered := c.Env[:0] + for _, env := range c.Env { + if env.Name != "FIPS_ENABLED" { + filtered = append(filtered, env) + } + } + c.Env = filtered }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@control-plane-operator/controllers/hostedcontrolplane/v2/ingressoperator/deployment.go` around lines 25 - 29, The code only sets FIPS_ENABLED when cpContext.HCP.Spec.FIPS is true, leaving the variable absent when false and breaking idempotency in adaptDeployment; change the branch so podspec.UpsertEnvVar is called in both cases with corev1.EnvVar{Name: "FIPS_ENABLED", Value: "true"} when cpContext.HCP.Spec.FIPS is true and Value: "false" when false (use the same UpsertEnvVar call path so existing env is updated), referencing cpContext.HCP.Spec.FIPS and podspec.UpsertEnvVar to locate where to make the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@control-plane-operator/controllers/hostedcontrolplane/v2/ingressoperator/deployment.go`:
- Around line 25-29: The code only sets FIPS_ENABLED when
cpContext.HCP.Spec.FIPS is true, leaving the variable absent when false and
breaking idempotency in adaptDeployment; change the branch so
podspec.UpsertEnvVar is called in both cases with corev1.EnvVar{Name:
"FIPS_ENABLED", Value: "true"} when cpContext.HCP.Spec.FIPS is true and Value:
"false" when false (use the same UpsertEnvVar call path so existing env is
updated), referencing cpContext.HCP.Spec.FIPS and podspec.UpsertEnvVar to locate
where to make the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 4009eeba-d275-4785-8cf0-ae409ec640bf
📒 Files selected for processing (2)
control-plane-operator/controllers/hostedcontrolplane/v2/ingressoperator/deployment.gocontrol-plane-operator/controllers/hostedcontrolplane/v2/ingressoperator/deployment_test.go
The ingress operator determines FIPS mode by reading /proc/sys/crypto/fips_enabled on the node where it runs. In hosted clusters the ingress operator runs on the management cluster, which may have a different FIPS state than the hosted cluster. This causes the operator to deploy routers with incorrect cipher configuration when the FIPS states differ. Set FIPS_ENABLED=true on the ingress operator container when the hosted cluster has FIPS enabled, so the operator uses the correct cipher suite regardless of the management cluster's FIPS state. Co-Authored-By: Claude Opus 4.6 <[email protected]>
|
/lgtm |
|
Scheduling tests matching the |
|
/jira refresh |
openshift-ci-robot
added
the
jira/valid-bug
|
@csrwng: This pull request references Jira Issue OCPBUGS-84551, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci-robot
removed
the
jira/invalid-bug
AI Test Failure AnalysisJob: Generated by hypershift-analyze-e2e-failure post-step using Claude claude-opus-4-6 |
Test Resultse2e-aws
e2e-aks
|
|
/retest-required |
|
/verified by @csrwng Manually tested a 5.0ci release payload with the control plane operator built from this PR.
|
openshift-ci-robot
added
the
verified
|
@csrwng: This PR has been marked as verified by DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
|
I now have all the evidence needed. The failure is completely clear. Let me produce the final report. Test Failure Analysis CompleteJob Information
Test Failure AnalysisErrorSummaryThe Root CauseThe
Three workflow files were updated on
The PR itself only modifies Commit references:
Recommendations
Evidence
|
|
/override ci/prow/verify-workflows |
|
@csrwng: Overrode contexts on behalf of csrwng: ci/prow/verify-workflows DetailsIn response to this:
Instructions 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. |
|
@csrwng: all tests passed! Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
|
@csrwng: Jira Issue OCPBUGS-84551: Some pull requests linked via external trackers have merged: The following pull request, linked via external tracker, has not merged: All associated pull requests must be merged or unlinked from the Jira bug in order for it to move to the next state. Once unlinked, request a bug refresh with Jira Issue OCPBUGS-84551 has not been moved to the MODIFIED state. This PR is marked as verified. If the remaining PRs listed above are marked as verified before merging, the issue will automatically be moved to VERIFIED after all of the changes from the PRs are available in an accepted nightly payload. DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
|
/cherry-pick release-4.22 |
|
@csrwng: new pull request created: #8388 DetailsIn response to this:
Instructions 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. |
|
Fix included in release 5.0.0-0.nightly-2026-05-02-042818 |
6 participants
Summary
/proc/sys/crypto/fips_enabledon the node where it runs. In hosted clusters, the ingress operator runs on the management cluster, which may have a different FIPS state than the hosted cluster. This causes the operator to deploy routers with incorrect cipher configuration when the FIPS states differ.FIPS_ENABLED=trueon the ingress operator container when the hosted cluster has FIPS enabled, so the operator uses the correct cipher suite regardless of the management cluster's FIPS state.Test plan
go test ./control-plane-operator/controllers/hostedcontrolplane/v2/ingressoperator/ -vFIPS_ENABLED=truesetFIPS_ENABLEDset🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests