Fix cert-manager Issuer rendering annotations: null and wire documented Certificate annotations/labels#1825
Conversation
…ate annotations/labels The cert-manager Issuer in certificate.yaml referenced a non-existent `.Values.additionalAnnotations`, so with tls.type=cert-manager the Issuer rendered `metadata.annotations: null` — the same GitOps OutOfSync footgun already fixed for the APIService template (kubernetes-sigs#1752). Remove that broken block. Additionally, `tls.certManager.annotations` and `tls.certManager.labels` are declared in values.yaml and documented in the README ('Add extra annotations/labels to the Certificate resource') but were never wired to the Certificate. Apply them (guarded), so the documented values work. Per CONTRIBUTING, Chart.yaml is intentionally left unchanged; entry added under CHANGELOG [UNRELEASED]. Signed-off-by: Aman Raj <aman.yug@gmail.com>
Signed-off-by: Aman Raj <aman.yug@gmail.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: yugstar The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
This issue is currently awaiting triage. If metrics-server contributors determine this is a relevant issue, they will accept it by applying the The 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. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: yugstar The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Welcome @yugstar! |
|
Hi @yugstar. 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 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. |
Summary
Two related fixes in
charts/metrics-server/templates/certificate.yaml(only triggered whentls.type=cert-manager):1. Issuer renders
annotations: null(bug)The
Issuermetadata referenced.Values.additionalAnnotations, which does not exist anywhere invalues.yaml. Sohelm template --set tls.type=cert-managerrendered:This is the same
annotations: null→ GitOpsOutOfSyncfootgun that was already fixed for the APIService template in #1752. I removed the broken block (the value never existed, so there is no behavior to preserve).2.
tls.certManager.annotations/tls.certManager.labelswere dead valuesvalues.yamldeclarestls.certManager.annotationsandtls.certManager.labels, and the README documents them as "Add extra annotations/labels to the Certificate resource" — but they were never wired to theCertificate. This applies them (guarded withwith), so the documented values actually take effect.Verification
helm template --set tls.type=cert-manager→ Issuer no longer rendersannotations: null(no straynullanywhere).--set tls.certManager.annotations.foo=bar --set tls.certManager.labels.team=infra→ both now appear on theCertificate.helm lintpasses.CONTRIBUTING.md,Chart.yamlis intentionally left unmodified; the change is recorded underCHANGELOG.md[UNRELEASED].