Fix indentation in webhook-deployment when both webhook.volumes and webhook.config is provided#8664
Conversation
…ebhook.config is provided Signed-off-by: Joakim Nohlgård <joakim@nohlgard.se>
|
Hi @jnohlgard. Thanks for your PR. I'm waiting for a cert-manager member to verify that this patch is reasonable to test. If it is, they should reply with 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. |
|
/ok-to-test |
There was a problem hiding this comment.
Pull request overview
Fixes Helm chart rendering for the cert-manager webhook Deployment when both webhook.config and user-supplied webhook.volumeMounts/webhook.volumes are set, addressing the YAML parse error introduced in #8010.
Changes:
- Adjust
nindentforwebhook.volumeMountsrendering so list items correctly nest undervolumeMounts:. - Adjust
nindentforwebhook.volumesrendering so list items correctly nest undervolumes:.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
erikgb
left a comment
There was a problem hiding this comment.
@jnohlgard, thanks a lot! It looks indeed like #8010 introduced a regression. I have no idea how this apparently unrelated change slipped through our review. But Helm has a lot of sharp corners, and it is also very hard to use standard good software development principles like testing on Helm charts. 😓
I was able to reproduce the bug locally, and also that your patch fixes the issue. 🚀
We just had a patch release of cert-manager 1.20, but I am going to cherry-pick this fix into our release-1.20 branch to ensure it will be included in our first patch release.
/lgtm
/approve
/cherrypick release-1.20
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: erikgb 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 |
|
/cherrypick release-1.20 |
|
@erikgb: new pull request created: #8665 DetailsIn response to this:
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. |
|
@jnohlgard, a fix for this regression has been released as part of cert-manager 1.20.2. Sorry for the inconvenience! 🙈 |
Pull Request Motivation
#8010 broke the indentation in
webhook-deployment.yamlwhen bothwebhook.configandwebhook.volumesare defined.helm templategives the following message with my current config:Error: YAML parse error on cert-manager/templates/webhook-deployment.yaml: error converting YAML to JSON: yaml: line 99: did not find expected keySpecifically, these two unmotivated changed lines:
Kind
/kind bug
Release Note