OCPBUGS-85580: Fix webhook TLS failure after service-ca to self-managed cert migration by joshbranham · Pull Request #8504 · openshift/hypershift

joshbranham · 2026-05-13T16:54:41Z

Summary

During upgrade from service-ca managed certs to self-managed certs, the WebhookCertReconciler removed the serving-cert-secret-name annotation from the operator Service but did not remove the service.beta.openshift.io/inject-cabundle annotation from the MutatingWebhookConfiguration
This caused the service-ca operator to continuously overwrite the caBundle with its own CA (openshift-service-serving-signer), while the operator was serving with a certificate signed by the self-managed CA (hypershift-webhook-ca), resulting in x509: certificate signed by unknown authority errors
Adds removeInjectCABundleAnnotation() to strip the annotation from both Mutating and Validating webhook configurations during the service-ca migration path

Test plan

Unit test added covering annotation removal from both MutatingWebhookConfiguration and ValidatingWebhookConfiguration
All existing webhook cert tests continue to pass
Verified workaround on live management cluster — removing the annotation and restarting the operator resolved the TLS errors

🤖 Generated with Claude Code

Summary by CodeRabbit

Bug Fixes
- Improved webhook certificate handling during upgrades to avoid CA bundle being overwritten by external certificate management, ensuring webhook CA bundles remain correct.
Tests
- Added and extended tests covering webhook configuration updates and the upgrade scenario that removes conflicting annotations from webhook resources.

openshift-merge-bot · 2026-05-13T16:54:50Z

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

openshift-ci · 2026-05-13T16:54:51Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

coderabbitai · 2026-05-13T16:55:05Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: bd462ecf-279b-4f6d-9580-28ccf0814551

📥 Commits

Reviewing files that changed from the base of the PR and between bc2224b and b9fbac4.

📒 Files selected for processing (2)

hypershift-operator/controllers/webhookcerts/webhookcerts_controller.go
hypershift-operator/controllers/webhookcerts/webhookcerts_controller_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

hypershift-operator/controllers/webhookcerts/webhookcerts_controller.go

📝 Walkthrough

Walkthrough

The reconciler adds two constants: the inject-cabundle annotation key and a shared webhook configuration name. It introduces removeInjectCABundleAnnotation, which fetches and deletes the annotation from both the MutatingWebhookConfiguration and ValidatingWebhookConfiguration named by the new constant. patchWebhookConfigsCABundle now uses the shared webhook name when retrieving configs. The upgrade path in removeServiceCAResources calls removeInjectCABundleAnnotation before removing service-ca–managed serving cert secrets. Tests were added to verify annotation removal during reconciliation.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Controller as Controller
  participant API as Kubernetes API
  participant Mutating as MutatingWebhookConfiguration
  participant Validating as ValidatingWebhookConfiguration
  participant Secret as ServiceCA Secret
  participant ServiceCA as service-ca Operator

  Controller->>API: Get MutatingWebhookConfiguration (webhookConfigName)
  API-->>Controller: MutatingWebhookConfiguration (may contain inject-cabundle)
  Controller->>API: Get ValidatingWebhookConfiguration (webhookConfigName)
  API-->>Controller: ValidatingWebhookConfiguration (may contain inject-cabundle)
  Controller->>Mutating: Remove inject-cabundle annotation (if present)
  Controller->>Validating: Remove inject-cabundle annotation (if present)
  Controller->>API: Patch webhook configs CABundle
  API-->>Controller: Patch result
  Controller->>API: Delete service-ca managed Secret
  API-->>Controller: Secret deleted
  ServiceCA->>API: Would re-inject CABundle if annotation present
  Note right of ServiceCA: Removing annotation prevents re-injection

🚥 Pre-merge checks | ✅ 10 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Test Structure And Quality	⚠️ Warning	Assertion messages are missing from all Expect() calls. Tests lack diagnostic messages that would help identify test failures, violating criterion 4.	Add meaningful failure messages to all Expect calls. Example: `g.Expect(err).ToNot(HaveOccurred(), "failed to reconcile webhook")` and `g.Expect(mwc.Annotations).ToNot(HaveKey(annotation), "annotation should be removed")`.

✅ Passed checks (10 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly addresses the main change: preventing webhook TLS failures during cert migration by removing the inject-cabundle annotation.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names	✅ Passed	All test names use hardcoded strings without dynamic values. The 12 test titles are stable, deterministic, and will not change between runs.
Microshift Test Compatibility	✅ Passed	The PR adds only standard Go unit tests using the testing package, not Ginkgo e2e tests. The custom check targets Ginkgo e2e tests specifically; therefore it is not applicable to this PR.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	No Ginkgo e2e tests were added. The PR modifies unit tests using Go's standard testing.T framework with t.Run subtests. The SNO compatibility check is not applicable.
Topology-Aware Scheduling Compatibility	✅ Passed	Changes are limited to webhook certificate controller logic and do not introduce any scheduling constraints, deployment manifests, or topology-related configurations.
Ote Binary Stdout Contract	✅ Passed	PR has no OTE stdout contract violations. No process-level stdout writes detected in modified controller or test code.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	Not applicable. This PR contains only standard Go unit tests using testing.T, not Ginkgo e2e tests (which require It(), Describe(), Context()). The custom check applies only to Ginkgo e2e tests.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

codecov · 2026-05-13T17:01:50Z

Codecov Report

❌ Patch coverage is 48.27586% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 40.00%. Comparing base (bded456) to head (b9fbac4).
⚠️ Report is 68 commits behind head on main.

Files with missing lines	Patch %	Lines
...ontrollers/webhookcerts/webhookcerts_controller.go	48.27%	10 Missing and 5 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8504      +/-   ##
==========================================
+ Coverage   37.53%   40.00%   +2.46%     
==========================================
  Files         751      751              
  Lines       92026    92863     +837     
==========================================
+ Hits        34544    37147    +2603     
+ Misses      54841    53024    -1817     
- Partials     2641     2692      +51

Files with missing lines	Coverage Δ
...ontrollers/webhookcerts/webhookcerts_controller.go	`63.11% <48.27%> (-2.89%)`	⬇️

... and 53 files with indirect coverage changes

Flag	Coverage Δ
cmd-support	`34.09% <ø> (+1.32%)`	⬆️
cpo-hostedcontrolplane	`40.56% <ø> (+3.79%)`	⬆️
cpo-other	`40.14% <ø> (+2.38%)`	⬆️
hypershift-operator	`50.52% <48.27%> (+2.58%)`	⬆️
other	`31.54% <ø> (+3.76%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

openshift-ci-robot · 2026-05-13T19:58:28Z

@joshbranham: This pull request references Jira Issue OCPBUGS-85580, which is invalid:

expected the bug to target the "5.0.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Summary

During upgrade from service-ca managed certs to self-managed certs, the WebhookCertReconciler removed the serving-cert-secret-name annotation from the operator Service but did not remove the service.beta.openshift.io/inject-cabundle annotation from the MutatingWebhookConfiguration

This caused the service-ca operator to continuously overwrite the caBundle with its own CA (openshift-service-serving-signer), while the operator was serving with a certificate signed by the self-managed CA (hypershift-webhook-ca), resulting in x509: certificate signed by unknown authority errors

Adds removeInjectCABundleAnnotation() to strip the annotation from both Mutating and Validating webhook configurations during the service-ca migration path

Test plan

Unit test added covering annotation removal from both MutatingWebhookConfiguration and ValidatingWebhookConfiguration

All existing webhook cert tests continue to pass

Verified workaround on live management cluster — removing the annotation and restarting the operator resolved the TLS errors

🤖 Generated with Claude Code

Summary by CodeRabbit

Bug Fixes

Improved webhook certificate handling to prevent conflicts with external certificate management services during upgrades, ensuring CA bundle configurations remain properly maintained.

Tests

Added test coverage for webhook configuration updates during system upgrades.

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.

joshbranham · 2026-05-13T19:58:58Z

/jira refresh

openshift-ci-robot · 2026-05-13T19:59:02Z

@joshbranham: This pull request references Jira Issue OCPBUGS-85580, which is invalid:

expected the bug to be in one of the following states: NEW, ASSIGNED, POST, but it is MODIFIED instead

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

Details

In response to this:

/jira refresh

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.

joshbranham · 2026-05-13T20:01:51Z

/jira refresh

openshift-ci-robot · 2026-05-13T20:01:59Z

@joshbranham: This pull request references Jira Issue OCPBUGS-85580, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (5.0.0) matches configured target version for branch (5.0.0)
bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, POST)

Details

In response to this:

/jira refresh

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.

cblecker

Fix looks correct — the annotation removal is well-placed within removeServiceCAResources and the ordering with respect to patchWebhookConfigsCABundle is sound. Nice catch on this one.

/lgtm

cblecker · 2026-05-13T20:40:35Z

+
+		mwc := &admissionregistrationv1.MutatingWebhookConfiguration{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: webhookConfigName,


nit: Since you introduced webhookConfigName, it might be worth updating the existing tests in this file to use it too — lines 184, 194, 213, and 218 still hardcode "hypershift.openshift.io". No big deal either way.

Good call, silly robot missed that. Fixed, needs another lgtm if you are up for it.

openshift-merge-bot · 2026-05-13T20:41:12Z

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-aks-4-22
/test e2e-aws-4-22
/test e2e-aks
/test e2e-aws
/test e2e-aws-upgrade-hypershift-operator
/test e2e-azure-self-managed
/test e2e-kubevirt-aws-ovn-reduced
/test e2e-v2-aws
/test e2e-v2-gke

cwbotbot · 2026-05-13T21:15:02Z

Test Results

e2e-aws

Status: ✅ PASS
Started: 2026-05-14T02:55:02Z
View Job
View Job History

e2e-aks

Status: ✅ PASS
Started: 2026-05-13T22:52:55Z
View Job
View Job History

…figs during service-ca migration When upgrading from service-ca managed certs to self-managed certs, the reconciler removed the serving-cert-secret-name annotation from the Service but did not remove the inject-cabundle annotation from the MutatingWebhookConfiguration. This caused the service-ca operator to continuously overwrite the caBundle with its own CA, creating a mismatch with the self-managed serving certificate and breaking webhook TLS verification. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

openshift-ci-robot · 2026-05-13T21:38:59Z

@joshbranham: This pull request references Jira Issue OCPBUGS-85580, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (5.0.0) matches configured target version for branch (5.0.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Details

In response to this:

Summary

During upgrade from service-ca managed certs to self-managed certs, the WebhookCertReconciler removed the serving-cert-secret-name annotation from the operator Service but did not remove the service.beta.openshift.io/inject-cabundle annotation from the MutatingWebhookConfiguration

This caused the service-ca operator to continuously overwrite the caBundle with its own CA (openshift-service-serving-signer), while the operator was serving with a certificate signed by the self-managed CA (hypershift-webhook-ca), resulting in x509: certificate signed by unknown authority errors

Adds removeInjectCABundleAnnotation() to strip the annotation from both Mutating and Validating webhook configurations during the service-ca migration path

Test plan

Unit test added covering annotation removal from both MutatingWebhookConfiguration and ValidatingWebhookConfiguration

All existing webhook cert tests continue to pass

Verified workaround on live management cluster — removing the annotation and restarting the operator resolved the TLS errors

🤖 Generated with Claude Code

Summary by CodeRabbit

Bug Fixes

Improved webhook certificate handling during upgrades to avoid CA bundle being overwritten by external certificate management, ensuring webhook CA bundles remain correct.

Tests

Added and extended tests covering webhook configuration updates and the upgrade scenario that removes conflicting annotations from webhook resources.

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.

cblecker · 2026-05-13T22:52:12Z

/lgtm

openshift-merge-bot · 2026-05-13T22:52:26Z

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-aks-4-22
/test e2e-aws-4-22
/test e2e-aks
/test e2e-aws
/test e2e-aws-upgrade-hypershift-operator
/test e2e-azure-self-managed
/test e2e-kubevirt-aws-ovn-reduced
/test e2e-v2-aws
/test e2e-v2-gke

joshbranham · 2026-05-14T02:54:51Z

/test e2e-aws

enxebre · 2026-05-14T07:42:38Z

/approve
/cherry-pick release-4.22

openshift-cherrypick-robot · 2026-05-14T07:42:40Z

@enxebre: once the present PR merges, I will cherry-pick it on top of release-4.22 in a new PR and assign it to you.

Details

In response to this:

/approve
/cherry-pick release-4.22

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.

openshift-ci · 2026-05-14T07:43:00Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: enxebre, joshbranham

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [enxebre]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

enxebre · 2026-05-14T08:44:16Z

/verified by @joshbranham

openshift-ci-robot · 2026-05-14T08:44:28Z

@enxebre: This PR has been marked as verified by @joshbranham.

Details

In response to this:

/verified by @joshbranham

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 · 2026-05-14T09:12:33Z

@joshbranham: all tests passed!

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

openshift-ci-robot · 2026-05-14T09:17:34Z

@joshbranham: Jira Issue Verification Checks: Jira Issue OCPBUGS-85580
✔️ This pull request was pre-merge verified.
✔️ All associated pull requests have merged.
✔️ All associated, merged pull requests were pre-merge verified.

Jira Issue OCPBUGS-85580 has been moved to the MODIFIED state and will move to the VERIFIED state when the change is available in an accepted nightly payload. 🕓

Details

In response to this:

Summary

During upgrade from service-ca managed certs to self-managed certs, the WebhookCertReconciler removed the serving-cert-secret-name annotation from the operator Service but did not remove the service.beta.openshift.io/inject-cabundle annotation from the MutatingWebhookConfiguration

This caused the service-ca operator to continuously overwrite the caBundle with its own CA (openshift-service-serving-signer), while the operator was serving with a certificate signed by the self-managed CA (hypershift-webhook-ca), resulting in x509: certificate signed by unknown authority errors

Adds removeInjectCABundleAnnotation() to strip the annotation from both Mutating and Validating webhook configurations during the service-ca migration path

Test plan

Unit test added covering annotation removal from both MutatingWebhookConfiguration and ValidatingWebhookConfiguration

All existing webhook cert tests continue to pass

Verified workaround on live management cluster — removing the annotation and restarting the operator resolved the TLS errors

🤖 Generated with Claude Code

Summary by CodeRabbit

Bug Fixes

Improved webhook certificate handling during upgrades to avoid CA bundle being overwritten by external certificate management, ensuring webhook CA bundles remain correct.

Tests

Added and extended tests covering webhook configuration updates and the upgrade scenario that removes conflicting annotations from webhook resources.

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-cherrypick-robot · 2026-05-14T09:18:30Z

@enxebre: new pull request created: #8513

Details

In response to this:

/approve
/cherry-pick release-4.22

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.

openshift-merge-robot · 2026-05-14T22:04:58Z

Fix included in release 5.0.0-0.nightly-2026-05-14-123934

openshift-ci Bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. do-not-merge/needs-area labels May 13, 2026

openshift-ci Bot added area/hypershift-operator Indicates the PR includes changes for the hypershift operator and API - outside an OCP release and removed do-not-merge/needs-area labels May 13, 2026

joshbranham changed the title ~~OCPBUGS-TBD: Fix webhook TLS failure after service-ca to self-managed cert migration~~ OCPBUGS-85580: Fix webhook TLS failure after service-ca to self-managed cert migration May 13, 2026

openshift-ci-robot added jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels May 13, 2026

openshift-ci-robot added jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. and removed jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels May 13, 2026

joshbranham marked this pull request as ready for review May 13, 2026 20:05

openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 13, 2026

openshift-ci Bot requested review from cblecker and csrwng May 13, 2026 20:06

cblecker reviewed May 13, 2026

View reviewed changes

openshift-ci Bot assigned cblecker May 13, 2026

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label May 13, 2026

joshbranham force-pushed the fix-webhook-inject-cabundle-annotation branch from bc2224b to b9fbac4 Compare May 13, 2026 21:37

openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label May 13, 2026

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label May 13, 2026

openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 14, 2026

openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label May 14, 2026

openshift-merge-bot Bot merged commit 294fa41 into openshift:main May 14, 2026
42 checks passed

openshift-cherrypick-robot mentioned this pull request May 14, 2026

[release-4.22] OCPBUGS-85620: Fix webhook TLS failure after service-ca to self-managed cert migration #8513

Open

Conversation

joshbranham commented May 13, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Summary by CodeRabbit

Uh oh!

openshift-merge-bot Bot commented May 13, 2026

Uh oh!

openshift-ci Bot commented May 13, 2026

Uh oh!

coderabbitai Bot commented May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Sequence Diagram(s)

❌ Failed checks (2 warnings)

Uh oh!

codecov Bot commented May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

openshift-ci-robot commented May 13, 2026

Summary

Test plan

Summary by CodeRabbit

Uh oh!

joshbranham commented May 13, 2026

Uh oh!

openshift-ci-robot commented May 13, 2026

Uh oh!

joshbranham commented May 13, 2026

Uh oh!

openshift-ci-robot commented May 13, 2026

Uh oh!

cblecker left a comment

Choose a reason for hiding this comment

Uh oh!

cblecker May 13, 2026

Choose a reason for hiding this comment

Uh oh!

joshbranham May 13, 2026

Choose a reason for hiding this comment

Uh oh!

openshift-merge-bot Bot commented May 13, 2026

Uh oh!

cwbotbot commented May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

e2e-aws

e2e-aks

Uh oh!

openshift-ci-robot commented May 13, 2026

Summary

Test plan

Summary by CodeRabbit

Uh oh!

cblecker commented May 13, 2026

Uh oh!

openshift-merge-bot Bot commented May 13, 2026

Uh oh!

joshbranham commented May 14, 2026

Uh oh!

enxebre commented May 14, 2026

Uh oh!

openshift-cherrypick-robot commented May 14, 2026

Uh oh!

openshift-ci Bot commented May 14, 2026

Uh oh!

enxebre commented May 14, 2026

Uh oh!

openshift-ci-robot commented May 14, 2026

Uh oh!

openshift-ci Bot commented May 14, 2026

Uh oh!

Uh oh!

openshift-ci-robot commented May 14, 2026

Summary

Test plan

Summary by CodeRabbit

Uh oh!

joshbranham commented May 13, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 13, 2026 •

edited

Loading

codecov Bot commented May 13, 2026 •

edited

Loading

cwbotbot commented May 13, 2026 •

edited

Loading