Native client-go implementation (big rewrite) by inteon · Pull Request #129 · cert-manager/webhook-cert-lib

inteon · 2026-01-06T18:19:47Z

I spent a lot of time porting this controller to client-go and iterating on the solution.

Current state of the PR:

based on client-go alone
works with very short certificate lifetime and 200+ controllers
added tests using the new testing/synctest package
much more resilient during CA rotation compared to cm/cm

You can run make test-smoke to test the example controller.

Copilot

Pull request overview

This pull request represents a major architectural rewrite of the webhook certificate authority controller, migrating from controller-runtime to a native client-go implementation. The changes introduce significant improvements in certificate rotation handling, particularly for scenarios with very short certificate lifetimes and large numbers of controllers.

Changes:

Complete migration from controller-runtime to native client-go with custom informer factories
New pending/serving CA certificate rotation mechanism with promotion delay to ensure smooth transitions
Tests now use Go's testing/synctest package instead of Ginkgo/Gomega
Removed unused internal error package and simplified PKI utilities

Reviewed changes

Copilot reviewed 72 out of 77 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
test/combined_controller_test.go	New comprehensive test suite using synctest for authority controller behavior
test/util_test.go	Helper functions for testing CA secrets and ValidatingWebhookConfigurations
test/delayed_watch.go	Simulates delayed watch events for testing race conditions
pkg/authority/authority.go	Core authority implementation with native client-go and workqueue
pkg/authority/options.go	Configuration validation and defaults for authority setup
pkg/authority/injectable/validating_webhook.go	Refactored CA bundle injection using informers and listers
internal/pki/cert_parser.go	New certificate parser with caching to avoid redundant parsing
internal/certificate/tls.go	Certificate generation utilities
examples/webhook-controller/main.go	Example webhook server demonstrating library usage

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

test/combined_controller_test.go

pkg/authority/options.go

test/combined_controller_test.go

inteon · 2026-01-16T10:33:04Z

/approve

cert-manager-prow · 2026-01-16T10:33:11Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: inteon

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~~ [inteon]

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

erikgb

Thanks, @inteon! This is a HUGE PR, and I have no chance to review it properly. Are you able to split it up? You seem to change "everything" without explaining the motivations for it. 😅 I intentionally didn't change the code copied from cert-manager (PKI) and trust-manager (CertPool) to allow us to eventually stop this copy-pasting across projects. And now you are rewriting most of it.....

I have added some initial comments/questions, but I will not be able to LGTM this with the current size of the PR.

erikgb · 2026-01-25T19:22:38Z

README.md

+                Namespace: "my-namespace",
+                Name: "my-webhook-ca",
+            },
+            Duration:             1 * time.Hour, // Optional: Default 1h


I think a default of 1 hour is too short. 😅

I believe that if we build it to renew frequently by default, this forces us to make renewals happen without downtime.

Atm. renewal does not happen often and can be a hard to debug cause for temporary downtime.

Also, by using short-lived certs, we improve our security posture.

erikgb · 2026-01-25T19:26:58Z

README.md

+
+### CA Rotation Lifecycle
+
+To ensure zero downtime, the controller uses a "Pending CA" strategy. This allows the new CA to be distributed to all clients (via the `ValidatingWebhookConfiguration` CA bundle) *before* it is used to sign the serving certificate.


Is the formulation "(via the ValidatingWebhookConfiguration CA bundle" fully correct? We also support injecting other resources with a CA bundle.

I don't think we implemented other resources yet. I would updated the README once we add support for the other resources.

erikgb · 2026-01-25T19:33:43Z

README.md

+
+- **Leader first**: The controller that issued the pending CA can perform injection and promotion directly, while other controllers have to wait between 3-6s first.
+- **Jitter**: Non-issuing controllers wait with randomized jitter (between 3 and 6 seconds) to avoid thundering-herd updates.
+- **Renewal window**: The renewal timestamp is randomized to avoid all controllers renewing the certs at the same time.


I don't know if we should reimplement the old, obsolete leader election mechanism using configs/secrets, and I'd prefer to use the standard leader election mechanism.

I added this to the TODO file: https://github.com/cert-manager/webhook-cert-lib/pull/129/changes#diff-337e547a950fc8a98592f10d964c1e79a304961790a8da0ce449a1f000cefabbR5

diagrams/architecture.mmd

diagrams/rotation.mmd

erikgb · 2026-01-25T19:42:44Z

examples/webhook-controller/deploy/05_validatingwebhook.yaml

+      # CA bundle will be injected by the Authority controller; keep empty here.
+      caBundle: ""


Suggested change

# CA bundle will be injected by the Authority controller; keep empty here.

caBundle: ""

# CA bundle will be injected by the Authority controller, so omit it here.

erikgb · 2026-01-25T19:46:13Z

examples/webhook-controller/main.go

+func main() {
+	kubeconfig := flag.String("kubeconfig", "", "Path to kubeconfig (optional)")
+	addr := flag.String("addr", ":8443", "Address to serve HTTPS on")
+	serviceName := flag.String("service-name", "example-webhook", "will be used in the DNS name for the webhook server certificate (<service-name>.<namespace>.svc)")


Maybe also add a flag for the CA secret name. Mainly to make the RBAC with resourceNames less magic?

I added this to the TODO file: https://github.com/cert-manager/webhook-cert-lib/pull/129/changes#diff-337e547a950fc8a98592f10d964c1e79a304961790a8da0ce449a1f000cefabbR12

internal/pki/parse.go

make/02_mod.mk

erikgb · 2026-01-25T20:00:30Z

pkg/authority/api/api.go

+	// TLSPendingCertKey stores a pending (new) CA cert PEM while it is being
+	// propagated to targets. It will be promoted to `tls_serving.crt` after
+	// `Options.PropagationDelay` has elapsed since the rotation timestamp.
+	TLSPendingCertKey = corev1.TLSCertKey


As discussed previously, I would find it less confusing if tls.crt contained the currently used serving CA, and instead use something like tls-pending.crt for the pending one. And OFC the same for the key.

I added this to the TODO file: https://github.com/cert-manager/webhook-cert-lib/pull/129/changes#diff-337e547a950fc8a98592f10d964c1e79a304961790a8da0ce449a1f000cefabbR6-R7

Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com>

inteon · 2026-02-06T09:57:47Z

@erikgb I added your outstanding comments to the TODO file.
I would like to move forward with this PR and merge it.

Next, I would like to figure out if we can use cert-manager as a source for our CA certificates. This will also help us make some of these design decisions.

Also, I think we should try improve our test framework further. Adding even more failure/ delay points to our simulation should help us make decisions when it comes to eg. leader election.

cert-manager-prow bot added dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jan 6, 2026

inteon force-pushed the clientgo branch 8 times, most recently from 4688445 to 69c45fe Compare January 8, 2026 13:45

cert-manager-prow bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 8, 2026

inteon force-pushed the clientgo branch from 327f0d1 to 2a371c9 Compare January 9, 2026 18:42

cert-manager-prow bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 9, 2026

inteon force-pushed the clientgo branch 2 times, most recently from 34afd96 to fbac5d7 Compare January 9, 2026 18:49

cert-manager-prow bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 11, 2026

inteon force-pushed the clientgo branch from fbac5d7 to 32a64aa Compare January 15, 2026 17:48

cert-manager-prow bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 15, 2026

inteon force-pushed the clientgo branch from 32a64aa to 4274ddf Compare January 15, 2026 17:55

inteon requested review from Copilot and erikgb January 15, 2026 18:00

Copilot started reviewing on behalf of inteon January 15, 2026 18:01 View session

Copilot AI reviewed Jan 15, 2026

View reviewed changes

test/combined_controller_test.go Show resolved Hide resolved

pkg/authority/options.go Outdated Show resolved Hide resolved

pkg/authority/options.go Outdated Show resolved Hide resolved

test/combined_controller_test.go Outdated Show resolved Hide resolved

inteon force-pushed the clientgo branch from 4274ddf to 36b68c3 Compare January 16, 2026 09:04

cert-manager-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 16, 2026

erikgb reviewed Jan 25, 2026

View reviewed changes

inteon force-pushed the clientgo branch from 917dce0 to a332c7b Compare January 27, 2026 09:10

inteon mentioned this pull request Jan 27, 2026

Update PKI logic #142

Draft

This was referenced Jan 27, 2026

Move pkg/authority/certificate to internal/certificate #144

Merged

Use renew trigger window instead of static renew time #145

Merged

PKI small code cleanup #146

Merged

cert-manager-prow bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 27, 2026

inteon force-pushed the clientgo branch from a332c7b to f4aa376 Compare February 2, 2026 13:45

cert-manager-prow bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 2, 2026

inteon force-pushed the clientgo branch from f4aa376 to 079dbcd Compare February 2, 2026 13:52

cert-manager-prow bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 5, 2026

inteon force-pushed the clientgo branch from 079dbcd to a9c55c0 Compare February 5, 2026 08:32

cert-manager-prow bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 5, 2026

inteon force-pushed the clientgo branch from a9c55c0 to 51236bb Compare February 6, 2026 09:19

native client-go implementation

7567232

Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com>

inteon force-pushed the clientgo branch from 51236bb to 7567232 Compare February 6, 2026 09:22

add TODOs based on PR feedback

6f67ad7

Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com>


		### CA Rotation Lifecycle

		To ensure zero downtime, the controller uses a "Pending CA" strategy. This allows the new CA to be distributed to all clients (via the `ValidatingWebhookConfiguration` CA bundle) before it is used to sign the serving certificate.

		# CA bundle will be injected by the Authority controller; keep empty here.
		caBundle: ""

Conversation

inteon commented Jan 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

inteon commented Jan 16, 2026

Uh oh!

cert-manager-prow bot commented Jan 16, 2026

Uh oh!

erikgb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

inteon commented Feb 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

inteon commented Jan 6, 2026 •

edited

Loading