Decouple conversion webhook registration from leader election by sergenyalcin · Pull Request #678 · crossplane/upjet

sergenyalcin · 2026-06-17T10:09:33Z

Description of your changes

Conversion webhook registration was accidentally gated behind leader election through the SetupGated/gate mechanism. SetupGated stores a closure that only fires on the elected leader, so follower pods never called ctrl.NewWebhookManagedBy and their webhook servers returned 404 for every conversion request. With two replicas and --leader-election, some conversion webhook calls fail.

The fix separates webhook registration from reconciler setup:

controller.go.tmpl: adds SetupWebhookWithManager — a standalone function that registers the conversion webhook for a single resource kind. Removes the if o.StartWebhooks { ... } block from Setup; webhook registration is no longer the reconciler's responsibility.
setup.go.tmpl: adds the SetupWebhookWithManager{{ .Group }} aggregator, following the same pattern as Setup{{ .Group }} and SetupGated{{ .Group }}, so providers can register all webhooks for a group in a single call.

Providers call SetupWebhookWithManager_<group> once before mgr.Start() on every pod, independent of the gate and leader election. Reconciler setup remains behind the gate and runs only on the leader.

See the generated provider with these changes: crossplane-contrib/provider-upjet-gcp#953 and crossplane-contrib/provider-upjet-aws#2122

I have:

Read and followed Upjet's contribution process.
Run make reviewable to ensure this PR is ready for review.
Added backport release-x.y labels to auto-backport this PR if necessary.

How has this code been tested

Tested locally by sending webhook calls to the webhook handlers.

Signed-off-by: Sergen Yalçın <yalcinsergen97@gmail.com>

ulucinar

Thank you @sergenyalcin for debugging this issue & fixing it, lgtm. Expressed some concerns about the breaking changes below, let's discuss.

ulucinar · 2026-06-22T08:29:16Z

+func SetupWebhookWithManager(mgr ctrl.Manager) error {
+	if err := ctrl.NewWebhookManagedBy(mgr, &{{ .TypePackageAlias }}{{ .CRD.Kind }}{}).
+		Complete(); err != nil {
+		return errors.Wrap(err, "cannot register webhook for the kind {{ .TypePackageAlias }}{{ .CRD.Kind }}")


In case the same (version, kind) exists in a different group:

Suggested change

return errors.Wrap(err, "cannot register webhook for the kind {{ .TypePackageAlias }}{{ .CRD.Kind }}")

return errors.Wrapf(err, "cannot register webhook for the kind %s", {{ .TypePackageAlias }}{{ .CRD.Kind }}_GroupVersionKind.String())

ulucinar · 2026-06-22T08:35:48Z


-	// register webhooks for the kind {{ .TypePackageAlias }}{{ .CRD.Kind }}
-	// if they're enabled.
-	if o.StartWebhooks {


One concern here is some providers out in the wild might still be relying on this condition (and using this template shipped with upjet). Maybe we need to document the change in the release notes.

nit: newly introduced SetupWebhookWithManager(mgr ctrl.Manager) can accept o tjcontroller.Options, keeping the logic intact

I'll add a detailed section about this change to the Release Notes.

ulucinar · 2026-06-22T08:38:10Z

 )

+// SetupWebhookWithManager registers the conversion webhook for {{ .CRD.Kind }}.
+func SetupWebhookWithManager(mgr ctrl.Manager) error {


Please see the comment below. We need to document the newly generated webhook setup functions and ask the provider authors to call them. Not all providers will have this bug (not all of them will be using CRD-gating). What could happen is that although they set the controller.Options.StartWebhooks to True, their webhooks won't be started. The good news is that they will get a change to see the changes (especially the removal of the previously generated condition section in Setup function for the webhooks and the newly generated SetupWebhookWithManager function) when they run make generate, so in my opinion, not a huge deal. But we had better still explain the changes and instruct the provider authors how to call the newly generated SetupWebhookWithManager_<API group> function(s) in some release notes. What do you think?

just to double-check: AFAIU, in "gated" setups, when there are MRAPs disabling a particular Kind, this will register its webhook unconditionally, although the CRD won't exist in cluster.

I think this will still work with controller-runtime, but we should test it to make sure

I'll add an instruction to the Release Notes.

@erhancagirici, I've tested this and the webhook registration doesn't be an issue for the unregistered CRDs or controller-runtime.

erhancagirici · 2026-06-22T09:31:03Z

+func SetupWebhookWithManager{{ .Group }}(mgr ctrl.Manager) error {
+	for _, setup := range []func(ctrl.Manager) error{
+		{{- range $alias := .Aliases }}
+		{{ $alias }}SetupWebhookWithManager,


nit: another alternative could be to add {{ $alias }}SetupWebhookWithManager, into Setup_<apiGroup> and SetupGated_<apiGroup> templates:
this would:

keep Setup_<apiGroup> logically the same

decouple Webhook registration for SetupGated_<apiGroup>, the actual setup is still gated, webhooks are registered without gate.

no extra call to SetupWebhookWithManager_<apiGroup> required from provider developers

On the other hand, the current approach has the advantage of keeping webhook registration clean and explicit.

Yes, this was an option, but I preferred it because of the clear separation.

Bump github.com/crossplane/upjet/v2 v2.2.0 -> v2.3.0, github.com/crossplane/crossplane-runtime/v2 v2.1.0 -> v2.2.0 and sigs.k8s.io/controller-runtime v0.22.4 -> v0.23.3 (go mod tidy also pulls controller-tools v0.20.0), then regenerate. upjet v2.3.0 decouples conversion-webhook registration from the controller Setup() function (crossplane/upjet#678): per-resource registration moves to a standalone SetupWebhookWithManager and the generated zz_setup.go gains a SetupWebhookWithManager aggregator. The hand-written providerconfig packages gain a matching SetupWebhookWithManager so the regenerated aggregator compiles. This provider does not enable conversion webhooks (StartWebhooks is never set, and no CRD uses a Webhook conversion strategy), so per the v2.3.0 migration notes the entry point is unaffected and needs no change. crossplane-runtime v2.2.0 also carries the ResolveMultiple reference-resolver fix that preserves the authored order of multi-reference lists instead of sorting by resolved UUID (refs crossplane-contrib#462). Co-Authored-By: Claude Code <noreply@anthropic.com> Signed-off-by: Gábor Somogyi <gabor.somogyi@gmail.com>

Bump github.com/crossplane/upjet/v2 v2.2.0 -> v2.3.0, github.com/crossplane/crossplane-runtime/v2 v2.1.0 -> v2.2.0 and sigs.k8s.io/controller-runtime v0.22.4 -> v0.23.3 (go mod tidy also pulls controller-tools v0.20.0), then regenerate. upjet v2.3.0 decouples conversion-webhook registration from the controller Setup() function (crossplane/upjet#678): per-resource registration moves to a standalone SetupWebhookWithManager and the generated zz_setup.go gains a SetupWebhookWithManager aggregator. The hand-written providerconfig packages gain a matching SetupWebhookWithManager so the regenerated aggregator compiles. This provider does not enable conversion webhooks (StartWebhooks is never set, and no CRD uses a Webhook conversion strategy), so per the v2.3.0 migration notes the entry point is unaffected and needs no change. crossplane-runtime v2.2.0 also carries the ResolveMultiple reference-resolver fix that preserves the authored order of multi-reference lists instead of sorting by resolved UUID (refs #462). Co-Authored-By: Claude Code <noreply@anthropic.com> Signed-off-by: Gábor Somogyi <gabor.somogyi@gmail.com>

Decouple conversion webhook registration from leader election

391acbd

Signed-off-by: Sergen Yalçın <yalcinsergen97@gmail.com>

sergenyalcin requested review from erhancagirici and ulucinar as code owners June 17, 2026 10:09

ulucinar approved these changes Jun 22, 2026

View reviewed changes

ulucinar reviewed Jun 22, 2026

View reviewed changes

erhancagirici reviewed Jun 22, 2026

View reviewed changes

sergenyalcin merged commit b02902e into crossplane:main Jun 22, 2026
9 checks passed

sergenyalcin deleted the register-webhooks-seperately branch June 22, 2026 14:37

gaborsomogyi mentioned this pull request Jun 23, 2026

chore(deps): bump upjet to v2.3.0 and regenerate crossplane-contrib/provider-keycloak#598

Merged

Navigation Menu

Uh oh!

Decouple conversion webhook registration from leader election - #678

Decouple conversion webhook registration from leader election#678
sergenyalcin merged 1 commit into
crossplane:mainfrom
sergenyalcin:register-webhooks-seperately

sergenyalcin commented Jun 17, 2026 •

edited

Loading

Uh oh!

ulucinar left a comment •

edited

Loading

Uh oh!

ulucinar Jun 22, 2026 •

edited

Loading

Uh oh!

ulucinar Jun 22, 2026 •

edited

Loading

Uh oh!

erhancagirici Jun 22, 2026

Uh oh!

sergenyalcin Jun 22, 2026

Uh oh!

ulucinar Jun 22, 2026 •

edited

Loading

Uh oh!

erhancagirici Jun 22, 2026

Uh oh!

sergenyalcin Jun 22, 2026

Uh oh!

erhancagirici Jun 22, 2026

Uh oh!

sergenyalcin Jun 22, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	return errors.Wrap(err, "cannot register webhook for the kind {{ .TypePackageAlias }}{{ .CRD.Kind }}")
	return errors.Wrapf(err, "cannot register webhook for the kind %s", {{ .TypePackageAlias }}{{ .CRD.Kind }}_GroupVersionKind.String())

Uh oh!

Conversation

sergenyalcin commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of your changes

How has this code been tested

Uh oh!

ulucinar left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ulucinar Jun 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ulucinar Jun 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

erhancagirici Jun 22, 2026

Choose a reason for hiding this comment

Uh oh!

sergenyalcin Jun 22, 2026

Choose a reason for hiding this comment

Uh oh!

ulucinar Jun 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

erhancagirici Jun 22, 2026

Choose a reason for hiding this comment

Uh oh!

sergenyalcin Jun 22, 2026

Choose a reason for hiding this comment

Uh oh!

erhancagirici Jun 22, 2026

Choose a reason for hiding this comment

Uh oh!

sergenyalcin Jun 22, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sergenyalcin commented Jun 17, 2026 •

edited

Loading

ulucinar left a comment •

edited

Loading

ulucinar Jun 22, 2026 •

edited

Loading

ulucinar Jun 22, 2026 •

edited

Loading

ulucinar Jun 22, 2026 •

edited

Loading