Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

vault: add support for wan federation with Vault when ACLs are enabled #1025

Merged
merged 3 commits into from
Feb 17, 2022

Conversation

ishustava
Copy link
Contributor

@ishustava ishustava commented Feb 8, 2022

Changes proposed in this PR:

  • This PR adds support for storing ACL replication token in Vault

How I've tested this PR:
acceptance tests

How I expect reviewers to test this PR:
馃憖

Checklist:

  • Tests added
  • CHANGELOG entry added

    HashiCorp engineers only, community PRs should not add a changelog entry.
    Entries should use present tense (e.g. Add support for...)

@@ -70,7 +70,7 @@ commands:
type: string
consul-k8s-image:
type: string
default: "docker.mirror.hashicorp.services/hashicorpdev/consul-k8s-control-plane:latest"
default: "ishustava/consul-k8s-dev:02-08-2022-d21554d8"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

todo: remove before merging

@@ -4,7 +4,9 @@
{{- if and .Values.global.acls.createReplicationToken (not .Values.global.acls.manageSystemACLs) }}{{ fail "if global.acls.createReplicationToken is true, global.acls.manageSystemACLs must be true" }}{{ end -}}
{{- if .Values.global.bootstrapACLs }}{{ fail "global.bootstrapACLs was removed, use global.acls.manageSystemACLs instead" }}{{ end -}}
{{- if .Values.global.acls.manageSystemACLs }}
{{- /* We don't render this job when server.updatePartition > 0 because that
{{- if and .Values.global.secretsBackend.vault.enabled .Values.global.acls.replicationToken.secretName (not .Values.global.secretsBackend.vault.manageSystemACLsRole) }}{{ fail "global.secretsBackend.vault.manageSystemACLsRole must be set if global.secretsBackend.vault.enabled is true and global.acls.replicationToken is provided" }}{{ end -}}
{{- if or (and .Values.global.acls.replicationToken.secretName (not .Values.global.acls.replicationToken.secretKey)) (and .Values.global.acls.replicationToken.secretKey (not .Values.global.acls.replicationToken.secretName))}}{{ fail "both global.acls.replicationToken.secretKey and global.acls.replicationToken.secretName must be set if one of them is provided" }}{{ end -}}
Copy link
Contributor Author

@ishustava ishustava Feb 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This simplifies logic and testing across our templates since we don't have to test for the combinations when replication token secret name is provided but not secret key and vice versa.

@ishustava ishustava force-pushed the ishustava/vault-wan-fed-acls branch 3 times, most recently from fe6b041 to 55a28c6 Compare February 9, 2022 15:37
@ishustava ishustava changed the title vault: add support for wan federation when ACLs are enabled vault: add support for wan federation with Vault when ACLs are enabled Feb 9, 2022
@ishustava ishustava force-pushed the ishustava/vault-wan-fed-acls branch 2 times, most recently from 13c9839 to ec7e05e Compare February 9, 2022 19:29
@ishustava ishustava marked this pull request as ready for review February 9, 2022 19:30
@ishustava ishustava requested review from a team, kschoche and t-eckert and removed request for a team February 9, 2022 19:30
@@ -75,6 +75,10 @@ spec:
"vault.hashicorp.com/agent-inject-secret-serverca.crt": {{ .Values.global.tls.caCert.secretName }}
"vault.hashicorp.com/agent-inject-template-serverca.crt": {{ include "consul.serverTLSCATemplate" . }}
{{- end }}
{{- if (and .Values.global.acls.replicationToken.secretName (not .Values.global.acls.createReplicationToken)) }}
"vault.hashicorp.com/agent-inject-secret-replication-token-config.hcl": "{{ .Values.global.acls.replicationToken.secretName }}"
"vault.hashicorp.com/agent-inject-template-replication-token-config.hcl": {{ template "consul.vaultReplicationTokenConfigTemplate" . }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome!

Copy link
Contributor

@kschoche kschoche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

馃敟

Base automatically changed from ishustava/vault-wan-fed to main February 17, 2022 16:20
@ishustava ishustava force-pushed the ishustava/vault-wan-fed-acls branch 2 times, most recently from 8e0e4e3 to 3f5383c Compare February 17, 2022 16:36
@@ -51,7 +51,7 @@ func NewCLICluster(
ctx environment.TestContext,
cfg *config.TestConfig,
releaseName string,
) Cluster {
) *CLICluster {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

馃檹馃徎 Thank you!

Copy link
Contributor

@t-eckert t-eckert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spectacular! I always enjoy how clear your code is, Iryna. Great work.

@ishustava ishustava merged commit b4756c3 into main Feb 17, 2022
@ishustava ishustava deleted the ishustava/vault-wan-fed-acls branch February 17, 2022 19:08
@jmurret jmurret added the vault label Mar 24, 2022
geobeau pushed a commit to geobeau/consul-k8s that referenced this pull request May 20, 2022
hashicorp#1025)

* Revamp issue template and provide verbiage on +1 for tracking interest
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants