OCPBUGS-120802: fix(webhookcerts): prevent CA overwrite on transient API errors - #9507
redhat-chai-bot wants to merge 2 commits into
Conversation
…g upgrade During operator upgrade, the install path applies 93 CRDs via SSA before the new deployment rolls out. When the new operator pod starts, it calls EnsureWebhookCerts to check whether webhook cert secrets exist. Under API server load from the concurrent CRD apply, the Get call in certsExist can fail with a transient error. The old certsExist returned false on any error (not just NotFound), causing EnsureWebhookCerts to generate a brand-new CA and serving cert that overwrites the existing ones via CreateOrUpdate. This invalidates the caBundle previously patched into CRDs and webhook configurations, producing persistent "tls: bad certificate" errors from the API server and stalling the deployment rollout until the 5-minute WaitUntilAvailable timeout expires. Fix certsExist to return an error on transient API failures so the operator startup propagates the error instead of silently regenerating certs. Fix EnsureWebhookCerts to guard each CreateOrUpdate callback against overwriting a secret that already contains valid data (defending against the replica-startup race where two pods bootstrap concurrently), and to derive the serving cert from the existing CA when one is already present rather than generating a mismatched pair. Signed-off-by: Chai Bot <chai-bot@redhat.com>
Address pre-commit review findings for OCPBUGS-118883:
R1: Add transient-error tests for certsExist using fake client
interceptors. Two new subtests verify the core safety property
that distinguishes NotFound (safe to regenerate) from transient
API errors (must propagate to prevent silent CA corruption).
I2: Add subtests for serving-cert NotFound and empty-data paths
in certsExist to cover both legs of the function.
I3: Rename caObj/servingObj to caSecret/servingSecret in
EnsureWebhookCerts for naming consistency with the rest of the
file.
I6: Use requeueInterval constant in test assertions instead of
duplicating the literal 12h value.
I7: Fix misleading test name ("regenerate both" to "reuse
existing CA") and add assertion that CA data was not modified.
Signed-off-by: Chai Bot <chai-bot@redhat.com>
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
openshift-ci
Bot
added
area/hypershift-operator
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## release-4.22 #9507 +/- ##
================================================
+ Coverage 36.71% 36.73% +0.01%
================================================
Files 777 777
Lines 95484 95508 +24
================================================
+ Hits 35061 35085 +24
- Misses 57580 57582 +2
+ Partials 2843 2841 -2
🚀 New features to boost your workflow:
|
csrwng
changed the title
openshift-ci-robot
added
jira/valid-reference
|
@redhat-chai-bot: This pull request references Jira Issue OCPBUGS-120802, 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. |
csrwng
added
jira/valid-bug
csrwng
added
the
backport-risk-assessed
|
Scheduling tests matching the |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: csrwng, redhat-chai-bot 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
|
/retest-required |
|
@redhat-chai-bot: The following tests failed, say
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. |
|
/uncc @devguyio |
This branch has not been deployed
4 participants
Summary
NotFoundwhen checking webhook certificate secrets.Root Cause
During the operator upgrade, applying the CRDs can temporarily increase API server load. The certificate check previously treated any API error as if the secret were missing, which could regenerate and overwrite the CA while existing webhook configurations still referenced the previous CA bundle. The resulting certificate mismatch caused webhook TLS handshakes to fail and stalled the rollout.
Validation
make test: 192 packages, 0 failuresmake verify: passedScope
The pre-existing
hasKeysissue insupport/certs/tls.gois not changed by this pull request.AI-generated. Review for accuracy.
@devguyio requested in Slack thread