-
Notifications
You must be signed in to change notification settings - Fork 324
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
Revert 1.12 TLS config to ensure 1.11 image is also supported. #1218
Conversation
cfc93e0
to
387d27a
Compare
- Revert this commit when we want to stop supporting 1.11 style TLS config.
387d27a
to
28c4e3f
Compare
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.
👍 Nice work ashwin!
@@ -2,7 +2,7 @@ | |||
|
|||
BREAKING CHANGES: | |||
* Helm | |||
* Using the Vault integration requires Consul 1.12.0+. [[GH-1213](https://github.com/hashicorp/consul-k8s/pull/1213)] | |||
* Using the Vault integration requires Consul 1.12.0+. [[GH-1213](https://github.com/hashicorp/consul-k8s/pull/1213)], [[GH-1218](https://github.com/hashicorp/consul-k8s/pull/1218)] |
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 not trying to be nitpicky here, but should we specify if this is Vault as the connect-ca, or the vault secrets backend or both?
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.
I think most users who are using Vault for either will be ok migrating to 1.12 which is why this was left generic.
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.
Cool that makes sense!
{{- end }} | ||
} | ||
}, | ||
{{- if .Values.global.secretsBackend.vault.enabled }} |
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.
This might be my browser but it looks like the indentation is off for these two blocks. I don't think it makes a functional difference tho with JSON?
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.
Looks great!
Changes proposed in this PR:
How I've tested this PR:
How I expect reviewers to test this PR:
Checklist: