🐛 (helm/v2-alpha): Support webhook opt-out via env overrides by aiqubits · Pull Request #5995 · kubernetes-sigs/kubebuilder

aiqubits · 2026-08-27T15:03:13Z

Description

webhook.enabled=false skips the webhook resources rendered by the chart, but it does not stop the scaffolded manager from registering webhooks or starting its webhook server.

This change:

allows explicit manager environment overrides when the source Deployment has no env field, without exposing envOverrides in the default values or changing the default rendered manifest
adds conditional environment variable templating to manager containers without existing environment variables
allows users to stop scaffolded webhooks with manager.envOverrides.ENABLE_WEBHOOKS=false
documents the actual webhook.enabled behavior, the default 9443 listener, and conversion webhook limitations
regenerates the affected Helm chart samples

The chart does not automatically set ENABLE_WEBHOOKS=false, so existing runtime behavior remains unchanged unless the user explicitly configures the override.

Fixes #5987

Testing

make test-unit
go test -race -tags=integration -timeout 10m ./pkg/plugins/optional/helm/v2alpha/...
make generate-charts
make helm-lint
make kube-linter
make fix-docs
bin/golangci-lint fmt --diff
rendered charts with manager.envOverrides.ENABLE_WEBHOOKS=false and verified that Helm emits value: "false"

kubernetes-prow · 2026-08-27T15:03:24Z

Welcome @aiqubits!

It looks like this is your first PR to kubernetes-sigs/kubebuilder 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/kubebuilder has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

kubernetes-prow · 2026-08-27T15:03:25Z

Hi @aiqubits. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

kubernetes-prow · 2026-08-27T15:03:56Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: aiqubits
Once this PR has been reviewed and has the lgtm label, please assign camilamacedo86 for approval. For more information see the Code Review Process.

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

Details

Needs approval from an approver in each of these files:

OWNERS

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

camilamacedo86 · 2026-09-02T07:39:15Z

+		childIndent + `{{- end }}`,
+		childIndent + `{{- else }}`,
+		childIndent + "[]",
+		childIndent + `{{- end }}`,


Would be possible to use buf.WriteString ??
Or constants as we do on other places to faciliate the maintainability?

Updated. The shared Helm condition is now a named constant, and the block is generated with strings.Builder. The existing-env and optional-env paths share the same builder while preserving their different default rendering behavior.

camilamacedo86 · 2026-09-02T07:41:03Z

+  ## Environment variable overrides (--set manager.envOverrides.VAR=value)
+  ## These values take precedence over entries with the same name in manager.env.
+  ##
+  envOverrides: {}


Why that was added ??
Would with this propose we start to add it indeed when has no ENV VARs?
What would be the use cases?
Pros and cons? Maybe that is the right approach just trying to think on that to ensure that we are going in the right direction.

I added this because the workaround in #5987 currently does not work for the common case where the source Deployment has no env field: both the generated env template and envOverrides are omitted.
However, you are right that always exposing envOverrides broadens the chart API beyond what this fix needs. Helm can create a missing nested value through --set, so I can narrow the change: keep envOverrides in values.yaml only when source env entries exist, while generating a fully conditional env block for deployments without env. The default manifest then remains unchanged, but the explicit --set manager.envOverrides.ENABLE_WEBHOOKS=false workaround still works.

Always expose manager.envOverrides and template the manager env field even when the source Deployment has no environment variables. Document the limits of webhook.enabled and the conversion webhook caveat.

hexbinoct · 2026-09-03T13:56:20Z

One wording point on the new aside, since it is going into the book as a permanent warning.

No webhook.port value disables the server.

That holds today for two independent reasons: the chart does not pass --webhook-port at all when webhook.enabled=false, and controller-runtime v0.24.1 defaults any port <= 0 back to 9443 in Options.setDefaults.

The second reason is already gone upstream. controller-runtime#3481 (merged 2026-07-27, unreleased, newest tag is v0.24.1 from 2026-05-12) narrowed that to if o.Port == 0 and made a negative port disable the server. So after the scaffold's next controller-runtime bump, --set webhook.enabled=true --set webhook.port=-1 will pass --webhook-port=-1 and the server will not listen, which makes that sentence false.

Scoping it to what we pin would keep it accurate, something like: "with the controller-runtime version the scaffold currently pins, no webhook.port value disables the server." Everything else in the aside reads correctly to me.

aiqubits · 2026-09-04T02:44:21Z

@hexbinoct I scoped the statement to the controller-runtime version currently pinned by the scaffold, as suggested. Thanks for catching that.

camilamacedo86 · 2026-09-04T09:33:11Z

Hi @aiqubits

See the comment: #5987 (comment)

We will need to bump controller-runtime latest and then address this one

Copilot

🟢 Approval recommended

The implementation preserves default rendering while providing tested, documented opt-out behavior.

Pull request overview

Adds explicit Helm environment overrides so users can disable scaffolded webhook registration without changing default chart output.

Changes:

Conditionally renders manager environment overrides when no source env field exists.
Adds unit and integration coverage for default and override rendering.
Documents webhook opt-out behavior and regenerates tutorial charts.

File summaries

File	Description
`pkg/plugins/optional/helm/v2alpha/scaffolds/test/chart_generation_integration_test.go`	Tests rendered override behavior.
`pkg/plugins/optional/helm/v2alpha/scaffolds/internal/templates/values_test.go`	Tests environment values generation.
`pkg/plugins/optional/helm/v2alpha/scaffolds/internal/kustomize/templater/appliers/manager.go`	Adds conditional manager environment templating.
`pkg/plugins/optional/helm/v2alpha/scaffolds/internal/kustomize/templater/appliers/helpers_test.go`	Covers env-less and inline-env deployments.
`docs/book/src/plugins/available/helm-v2-alpha.md`	Documents webhook behavior and opt-out usage.
`docs/book/src/multiversion-tutorial/testdata/project/dist/chart/templates/manager/manager.yaml`	Regenerates the multiversion chart.
`docs/book/src/getting-started/testdata/project/dist/chart/templates/manager/manager.yaml`	Regenerates the getting-started chart.
`docs/book/src/cronjob-tutorial/testdata/project/dist/chart/templates/manager/manager.yaml`	Regenerates the CronJob chart.

Review details

Files reviewed: 5/8 changed files
Comments generated: 0
Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

camilamacedo86 · 2026-09-04T09:38:48Z

+
+Setting `webhook.enabled=false` prevents the chart from rendering the webhook `Service`, admission webhook configurations, and `NetworkPolicy`. It does not prevent the scaffolded manager from registering its webhooks or starting its webhook server.
+
+When `webhook.enabled=false`, the chart does not pass `--webhook-port` to the manager. The scaffolded manager therefore listens on its compiled default port, `9443`, regardless of `webhook.port`. The `Deployment` still declares `webhook.port` as the `webhook-server` container port, but this declaration does not configure the listener. With the controller-runtime version the scaffold currently pins, no `webhook.port` value disables the server.


@aiqubits could you please check in the latets CR release if we cannot set 0 as it is done for metrics?

@camilamacedo86 I checked controller-runtime v0.25.0. Port 0 still defaults to 9443, while Port -1 disables the webhook server: https://github.com/kubernetes-sigs/controller-runtime/blob/v0.25.0/pkg/webhook/server.go#L76-L80 and https://github.com/kubernetes-sigs/controller-runtime/blob/v0.25.0/pkg/webhook/server.go#L203-L206.

Kubebuilder still pins v0.24.1, so after the controller-runtime bump lands I will revise this PR to use --webhook-port=-1 and remove the envOverrides workaround.

aiqubits · 2026-09-04T12:04:38Z

@camilamacedo86 I confirmed that controller-runtime v0.25.0 supports disabling the webhook server with Port: -1. I agree that the version bump should land first; afterward, I will rework this PR to use --webhook-port=-1 and remove the envOverrides workaround.

kubernetes-prow · 2026-09-09T11:31:16Z

PR needs rebase.

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.

kubernetes-prow Bot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Aug 27, 2026

kubernetes-prow Bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 27, 2026

kubernetes-prow Bot requested review from varshaprasad96 and vitorfloriano August 27, 2026 15:03

camilamacedo86 reviewed Sep 2, 2026

View reviewed changes

kubernetes-prow Bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 2, 2026

aiqubits changed the title ~~fix(helm/v2-alpha): support webhook opt-out via env overrides~~ 🐛 (helm/v2-alpha): Support webhook opt-out via env overrides Sep 2, 2026

fix(helm/v2-alpha): support webhook opt-out via env overrides

e3e2825

Always expose manager.envOverrides and template the manager env field even when the source Deployment has no environment variables. Document the limits of webhook.enabled and the conversion webhook caveat.

aiqubits force-pushed the fix/helm-webhook-env-overrides-5987 branch from 3d058bd to e3e2825 Compare September 2, 2026 10:27

kubernetes-prow Bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 2, 2026

aiqubits requested a review from camilamacedo86 September 2, 2026 10:28

hexbinoct mentioned this pull request Sep 3, 2026

helm/v2-alpha: webhook.enabled=false does not stop the manager's webhook server #5987

Closed

docs: scope webhook port warning to pinned runtime

430ae19

camilamacedo86 requested a balanced review from Copilot September 4, 2026 09:32

Copilot started reviewing on behalf of camilamacedo86 September 4, 2026 09:32 View session

Copilot AI reviewed Sep 4, 2026

View reviewed changes

camilamacedo86 reviewed Sep 4, 2026

View reviewed changes

kubernetes-prow Bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 9, 2026


		Setting `webhook.enabled=false` prevents the chart from rendering the webhook `Service`, admission webhook configurations, and `NetworkPolicy`. It does not prevent the scaffolded manager from registering its webhooks or starting its webhook server.

		When `webhook.enabled=false`, the chart does not pass `--webhook-port` to the manager. The scaffolded manager therefore listens on its compiled default port, `9443`, regardless of `webhook.port`. The `Deployment` still declares `webhook.port` as the `webhook-server` container port, but this declaration does not configure the listener. With the controller-runtime version the scaffold currently pins, no `webhook.port` value disables the server.

Uh oh!

Conversation

aiqubits commented Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing

Uh oh!

kubernetes-prow Bot commented Aug 27, 2026

Uh oh!

kubernetes-prow Bot commented Aug 27, 2026

Uh oh!

kubernetes-prow Bot commented Aug 27, 2026

Uh oh!

camilamacedo86 Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

aiqubits Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

camilamacedo86 Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

aiqubits Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

hexbinoct commented Sep 3, 2026

Uh oh!

aiqubits commented Sep 4, 2026

Uh oh!

camilamacedo86 commented Sep 4, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

camilamacedo86 Sep 4, 2026

Choose a reason for hiding this comment

Uh oh!

aiqubits Sep 4, 2026

Choose a reason for hiding this comment

Uh oh!

aiqubits commented Sep 4, 2026

Uh oh!

kubernetes-prow Bot commented Sep 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

aiqubits commented Aug 27, 2026 •

edited

Loading