Repository navigation
cert-manager: add feature gate options for webhook and cainjector - #18787
kubernetes-prow[bot] merged 1 commit into
Conversation
The featureGates option only sets --feature-gates on the cert-manager controller, so gates that cert-manager also requires on the webhook (for example AdditionalCertificateOutputFormats or NameConstraints) were only half-applied: the controller accepted the feature while the webhook rejected the resources using it. Following the cert-manager Helm chart, feature gates are now configured per component. The new webhookFeatureGates and cainjectorFeatureGates fields on certManager are rendered into the args of the webhook and cainjector Deployments respectively; featureGates keeps applying to the controller only, as before.
|
Welcome @KR-Ravindra! |
|
Hi @KR-Ravindra. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
|
Round 1 self-review. Re-checked against master: the two new maps are wired through The docs table says Marking ready for review. |
|
/ok-to-test |
|
/retest The gce-cilium failure was a sum.golang.org stream error while |
|
/retest |
|
The second /test pull-kops-e2e-k8s-gce-ipalias |
|
/test pull-kops-e2e-k8s-gce-cilium |
|
Thanks for this @KR-Ravindra (and your AI). |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hakman 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 |
…-upstream-release-1.37 Automated cherry pick of #18787: cert-manager: add feature gate options for webhook and cainjector
What this PR does / why we need it:
Adds
webhookFeatureGatesandcainjectorFeatureGatestospec.certManager, so cert-manager feature gates can be set per component instead of only on the controller.Problem
spec.certManager.featureGatesis only rendered into the args of thecert-manager-controllercontainer. The webhook and cainjector Deployments never receive--feature-gates, so gates that cert-manager documents as needing both the controller and the webhook (for exampleAdditionalCertificateOutputFormatsorNameConstraints) are only half-applied: the controller supports the feature while the webhook refuses the resources that use it.Root cause
upup/models/cloudup/resources/addons/certmanager.io/k8s-1.16.yaml.template:{{ range $key, $value := .CertManager.FeatureGates }}emits--feature-gatesfor the controller onlycert-manager-webhookcontainer args have no feature gate handlingcert-manager-cainjectorcontainer args have no feature gate handlingThe three cert-manager components accept different sets of gates (see the cert-manager feature-gates documentation, linked from docs/addons.md in this PR), so blindly copying
featureGatesto every container would not be correct either. The cert-manager Helm chart solves this with per-component values (featureGates,webhook.featureGates,cainjector.featureGates), which is the approach suggested in the issue discussion.Fix
CertManagerConfiggainsWebhookFeatureGatesandCAInjectorFeatureGates(map[string]bool) inpkg/apis/kops,v1alpha2andv1alpha3; conversion and deepcopy regenerated withmake apimachinery-codegen, CRD regenerated withmake crds.rangepattern as the controller.featureGateskeeps applying to the controller only, so existing clusters are unchanged.docs/addons.mddocuments the per-component gates.many-addonsintegration fixture sets both new fields and the golden output was regenerated withhack/update-expected.sh.How tested
Golden update-cluster test (
TestManyAddons), no cloud account needed.Before the template change, with the fixture setting
webhookFeatureGates/cainjectorFeatureGatesand the expected webhook and cainjector args written into the golden file:After the template change:
hack/update-expected.sh ./cmd/kops/afterwards produced no further changes beyond themany-addonsfixture (certmanager.io manifest, itsmanifestHashin the bootstrap addon list, andcluster-completed.spec). Also ranmake apimachinery-codegen(no diff),go test ./pkg/apis/kops/...,go vetandgo build ./cmd/kops/.Links
Which issue(s) this PR fixes:
Fixes #17797
Special notes for your reviewer:
The
kops_feature_tableentry in the docs says1.37; adjust if this lands in a different release. Gate names in the fixture (AdditionalCertificateOutputFormatsfor the webhook,ServerSideApplyfor the cainjector) are taken from cert-manager's webhook and cainjector feature lists.This change was prepared with an AI agent operated by KR-Ravindra, who reviewed and tested it.