-
Notifications
You must be signed in to change notification settings - Fork 85
fix(vertical-pod-autoscaler): Update secretName for the TLS cert mount #1140
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(vertical-pod-autoscaler): Update secretName for the TLS cert mount #1140
Conversation
|
Thanks for the PR @ahamlinman, do you want to add the release changes into this PR? FYI since the last VPA release I've automated the chart changes annotations so that can be skipped. |
2d670ff to
cd7e7c5
Compare
|
Those changes should be in now, and all references to 1.8.0 should be up to date: |
stevehipwell
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a minor typo to fix in the CHANGELOG.
stevehipwell
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, but the date now needs changing.
|
@ahamlinman could you also rebase? |
When a custom issuer is used, the secret name and issuer name are no longer guaranteed to align. Signed-off-by: Alex Hamlin <[email protected]>
da39a61 to
19b12c0
Compare
stevehipwell
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
When a custom issuer is used, the secret name and issuer name are no longer guaranteed to align. Fixes #1137.
Testing
I've tried deploying this internally, and it turns out our normal
ClusterIssuerrequires a common name on every certificate, which I'll have to look into separately. In the meantime, I have some sample commands to show off the diff between the chart versions.Enabling cert-manager with the default issuer
…produces no output, as expected.
Enabling cert-manager with a custom issuer
…produces the following:
…which matches what
grep -A20 'kind: Certificate' new.yamlshows is the certificate name:Release
I can cut a separate PR for a v1.8.1 release (similar to how #1111 released a fix for this chart from #1106), or if it would be easier I'm happy to inline those bumps into this PR.