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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable snapshot agent configuration to be retrieved from vault #1113

Merged
merged 13 commits into from
Mar 30, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ IMPROVEMENTS:
* Upgrade Docker image Alpine version from 3.14 to 3.15. [[GH-1058](https://github.com/hashicorp/consul-k8s/pull/1058)]
* Helm
* API Gateway: Allow controller to read Kubernetes namespaces in order to determine if route is allowed for gateway. [[GH-1092](https://github.com/hashicorp/consul-k8s/pull/1092)]
* Vault: Enable snapshot agent configuration to be retrieved from vault. [[GH-1113](https://github.com/hashicorp/consul-k8s/pull/1113)]

BUG FIXES:
* Helm
Expand Down
7 changes: 7 additions & 0 deletions charts/consul/templates/_helpers.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,13 @@ as well as the global.name setting.
{{ "{{" }}- end -{{ "}}" }}
{{- end -}}

{{- define "consul.vaultDecodedSecretTemplate" -}}
|
{{ "{{" }}- with secret "{{ .secretName }}" -{{ "}}" }}
{{ "{{" }}- {{ printf "base64Decode .Data.data.%s" .secretKey }} -{{ "}}" }}
{{ "{{" }}- end -{{ "}}" }}
{{- end -}}
Comment on lines +25 to +30
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to base64 decode it? Does vault kv engine not work with not-encoded config? Or does it encode it for you if similar to k8s secrets?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great question! Let me go back and check that I did not encode as part of the test set up.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I totally was. I used json.Marshall() and then saving that to Vault instead of wrapping that result in string() and then saving it. I have a PR with the acceptance test and made the modification there ...since I can test it. This is the commit: 634164a


{{- define "consul.serverTLSCATemplate" -}}
|
{{ "{{" }}- with secret "{{ .Values.global.tls.caCert.secretName }}" -{{ "}}" }}
Expand Down
22 changes: 19 additions & 3 deletions charts/consul/templates/client-snapshot-agent-deployment.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{{- if (or (and (ne (.Values.client.enabled | toString) "-") .Values.client.enabled) (and (eq (.Values.client.enabled | toString) "-") .Values.global.enabled)) }}
{{- if .Values.client.snapshotAgent.enabled }}
{{- if or (and .Values.client.snapshotAgent.configSecret.secretName (not .Values.client.snapshotAgent.configSecret.secretKey)) (and (not .Values.client.snapshotAgent.configSecret.secretName) .Values.client.snapshotAgent.configSecret.secretKey) }}{{fail "client.snapshotAgent.configSecret.secretKey and client.snapshotAgent.configSecret.secretName must both be specified." }}{{ end -}}
apiVersion: apps/v1
kind: Deployment
metadata:
Expand Down Expand Up @@ -29,10 +30,14 @@ spec:
annotations:
"consul.hashicorp.com/connect-inject": "false"
{{- if .Values.global.secretsBackend.vault.enabled }}
{{- if .Values.client.snapshotAgent.configSecret.secretName }}
"vault.hashicorp.com/role": {{ .Values.global.secretsBackend.vault.consulSnapshotAgentRole }}
{{- else if and .Values.global.tls.enabled }}
"vault.hashicorp.com/role": {{ .Values.global.secretsBackend.vault.consulCARole }}
{{- end }}
{{- if .Values.global.tls.enabled }}
"vault.hashicorp.com/agent-init-first": "true"
"vault.hashicorp.com/agent-inject": "true"
"vault.hashicorp.com/role": {{ .Values.global.secretsBackend.vault.consulCARole }}
"vault.hashicorp.com/agent-inject-secret-serverca.crt": {{ .Values.global.tls.caCert.secretName }}
"vault.hashicorp.com/agent-inject-template-serverca.crt": {{ template "consul.serverTLSCATemplate" . }}
{{- if and .Values.global.secretsBackend.vault.ca.secretName .Values.global.secretsBackend.vault.ca.secretKey }}
Expand All @@ -49,6 +54,12 @@ spec:
"vault.hashicorp.com/agent-inject-template-enterpriselicense.txt": {{ template "consul.vaultSecretTemplate" . }}
{{- end }}
{{- end }}
{{- if .Values.client.snapshotAgent.configSecret.secretName }}
{{- with .Values.client.snapshotAgent.configSecret }}
"vault.hashicorp.com/agent-inject-secret-snapshot-agent-config.json": "{{ .secretName }}"
"vault.hashicorp.com/agent-inject-template-snapshot-agent-config.json": {{ template "consul.vaultDecodedSecretTemplate" . }}
{{- end }}
{{- end }}
{{- end }}
spec:
{{- if .Values.client.tolerations }}
Expand All @@ -65,7 +76,7 @@ spec:
- name: consul-data
emptyDir:
medium: "Memory"
{{- if (and .Values.client.snapshotAgent.configSecret.secretName .Values.client.snapshotAgent.configSecret.secretKey) }}
{{- if (and .Values.client.snapshotAgent.configSecret.secretName .Values.client.snapshotAgent.configSecret.secretKey (not .Values.global.secretsBackend.vault.enabled)) }}
jmurret marked this conversation as resolved.
Show resolved Hide resolved
- name: snapshot-config
secret:
secretName: {{ .Values.client.snapshotAgent.configSecret.secretName }}
Expand Down Expand Up @@ -139,8 +150,12 @@ spec:
{{- end }}
exec /bin/consul snapshot agent \
{{- if (and .Values.client.snapshotAgent.configSecret.secretName .Values.client.snapshotAgent.configSecret.secretKey) }}
{{- if .Values.global.secretsBackend.vault.enabled }}
-config-file=/vault/secrets/snapshot-agent-config.json \
jmurret marked this conversation as resolved.
Show resolved Hide resolved
{{- else }}
-config-dir=/consul/config \
{{- end }}
{{- end }}
{{- if .Values.global.acls.manageSystemACLs }}
-config-dir=/consul/login \
{{- end }}
Expand All @@ -156,7 +171,7 @@ spec:
/bin/consul logout
{{- end }}
volumeMounts:
{{- if (and .Values.client.snapshotAgent.configSecret.secretName .Values.client.snapshotAgent.configSecret.secretKey) }}
{{- if (and .Values.client.snapshotAgent.configSecret.secretName .Values.client.snapshotAgent.configSecret.secretKey (not .Values.global.secretsBackend.vault.enabled)) }}
jmurret marked this conversation as resolved.
Show resolved Hide resolved
- name: snapshot-config
readOnly: true
mountPath: /consul/config
Expand Down Expand Up @@ -229,6 +244,7 @@ spec:
{{- if .Values.global.adminPartitions.enabled }}
-partition={{ .Values.global.adminPartitions.name }} \
{{- end }}
-token-sink-file=/consul/login/acl-token \
jmurret marked this conversation as resolved.
Show resolved Hide resolved
-log-level={{ default .Values.global.logLevel }} \
-log-json={{ .Values.global.logJSON }}
resources:
Expand Down
216 changes: 215 additions & 1 deletion charts/consul/test/unit/client-snapshot-agent-deployment.bats
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,85 @@ load _helpers
.
}

@test "client/SnapshotAgentDeployment: when client.snapshotAgent.configSecret.secretKey!=null and client.snapshotAgent.configSecret.secretName=null, fail" {
curtbushko marked this conversation as resolved.
Show resolved Hide resolved
cd `chart_dir`
run helm template \
-s templates/client-snapshot-agent-deployment.yaml \
--set 'client.snapshotAgent.enabled=true' \
--set 'client.snapshotAgent.configSecret.secretName=' \
--set 'client.snapshotAgent.configSecret.secretKey=bar' \
.
[ "$status" -eq 1 ]
[[ "$output" =~ "client.snapshotAgent.configSecret.secretKey and client.snapshotAgent.configSecret.secretName must both be specified." ]]
}

@test "client/SnapshotAgentDeployment: when client.snapshotAgent.configSecret.secretName!=null and client.snapshotAgent.configSecret.secretKey=null, fail" {
cd `chart_dir`
run helm template \
-s templates/client-snapshot-agent-deployment.yaml \
--set 'client.snapshotAgent.enabled=true' \
--set 'client.snapshotAgent.configSecret.secretName=foo' \
--set 'client.snapshotAgent.configSecret.secretKey=' \
.
[ "$status" -eq 1 ]
[[ "$output" =~ "client.snapshotAgent.configSecret.secretKey and client.snapshotAgent.configSecret.secretName must both be specified." ]]
}

@test "client/SnapshotAgentDeployment: adds volume for snapshot agent config secret when secret is configured" {
cd `chart_dir`
local vol=$(helm template \
-s templates/client-snapshot-agent-deployment.yaml \
--set 'client.snapshotAgent.enabled=true' \
--set 'client.snapshotAgent.configSecret.secretName=a/b/c/d' \
--set 'client.snapshotAgent.configSecret.secretKey=snapshot-agent-config' \
. | tee /dev/stderr |
yq -r -c '.spec.template.spec.volumes[] | select(.name == "snapshot-config")' | tee /dev/stderr)
local actual
actual=$(echo $vol | jq -r '. .name' | tee /dev/stderr)
[ "${actual}" = 'snapshot-config' ]

actual=$(echo $vol | jq -r '. .secret.secretName' | tee /dev/stderr)
[ "${actual}" = 'a/b/c/d' ]

actual=$(echo $vol | jq -r '. .secret.items[0].key' | tee /dev/stderr)
[ "${actual}" = 'snapshot-agent-config' ]

actual=$(echo $vol | jq -r '. .secret.items[0].path' | tee /dev/stderr)
[ "${actual}" = 'snapshot-config.json' ]
}

@test "client/SnapshotAgentDeployment: adds volume mount to snapshot container for snapshot agent config secret when secret is configured" {
cd `chart_dir`
local vol=$(helm template \
-s templates/client-snapshot-agent-deployment.yaml \
--set 'client.snapshotAgent.enabled=true' \
--set 'client.snapshotAgent.configSecret.secretName=a/b/c/d' \
--set 'client.snapshotAgent.configSecret.secretKey=snapshot-agent-config' \
. | tee /dev/stderr |
yq -r -c '.spec.template.spec.containers[0].volumeMounts[] | select(.name == "snapshot-config")' | tee /dev/stderr)
local actual
actual=$(echo $vol | jq -r '. .name' | tee /dev/stderr)
[ "${actual}" = 'snapshot-config' ]

actual=$(echo $vol | jq -r '. .readOnly' | tee /dev/stderr)
[ "${actual}" = 'true' ]

actual=$(echo $vol | jq -r '. .mountPath' | tee /dev/stderr)
[ "${actual}" = '/consul/config' ]
}

@test "client/SnapshotAgentDeployment: set config-dir argument on snapshot agent command to volume mount" {
cd `chart_dir`
local actual=$(helm template \
-s templates/client-snapshot-agent-deployment.yaml \
--set 'client.snapshotAgent.enabled=true' \
--set 'client.snapshotAgent.configSecret.secretName=a/b/c/d' \
--set 'client.snapshotAgent.configSecret.secretKey=snapshot-agent-config' \
. | tee /dev/stderr |
yq -r '.spec.template.spec.containers[0].command[2] | contains("-config-dir=/consul/config")' | tee /dev/stderr)
[ "${actual}" = 'true' ]
}

#--------------------------------------------------------------------
# tolerations

Expand Down Expand Up @@ -746,6 +825,78 @@ MIICFjCCAZsCCQCdwLtdjbzlYzAKBggqhkjOPQQDAjB0MQswCQYDVQQGEwJDQTEL' \
[ "${actual}" = "" ]
}

@test "client/SnapshotAgentDeployment: vault snapshot agent config annotations are correct when enabled" {
cd `chart_dir`
local object=$(helm template \
-s templates/client-snapshot-agent-deployment.yaml \
--set 'global.secretsBackend.vault.enabled=true' \
--set 'global.secretsBackend.vault.consulClientRole=foo' \
--set 'global.secretsBackend.vault.consulServerRole=test' \
--set 'global.secretsBackend.vault.consulSnapshotAgentRole=bar' \
--set 'client.snapshotAgent.enabled=true' \
--set 'client.snapshotAgent.configSecret.secretName=path/to/secret' \
--set 'client.snapshotAgent.configSecret.secretKey=config' \
. | tee /dev/stderr |
yq -r '.spec.template.metadata' | tee /dev/stderr)

local actual=$(echo $object |
yq -r '.annotations["vault.hashicorp.com/agent-inject-secret-snapshot-agent-config.json"]' | tee /dev/stderr)
[ "${actual}" = "path/to/secret" ]

actual=$(echo $object |
yq -r '.annotations["vault.hashicorp.com/agent-inject-template-snapshot-agent-config.json"]' | tee /dev/stderr)
local expected=$'{{- with secret \"path/to/secret\" -}}\n{{- base64Decode .Data.data.config -}}\n{{- end -}}'
[ "${actual}" = "${expected}" ]

actual=$(echo $object | jq -r '.annotations["vault.hashicorp.com/role"]' | tee /dev/stderr)
[ "${actual}" = "bar" ]
}
ishustava marked this conversation as resolved.
Show resolved Hide resolved

@test "client/SnapshotAgentDeployment: vault does not add volume for snapshot agent config secret" {
cd `chart_dir`
local actual=$(helm template \
-s templates/client-snapshot-agent-deployment.yaml \
--set 'global.secretsBackend.vault.enabled=true' \
--set 'global.secretsBackend.vault.consulClientRole=foo' \
--set 'global.secretsBackend.vault.consulServerRole=test' \
--set 'client.snapshotAgent.enabled=true' \
--set 'client.snapshotAgent.configSecret.secretName=a/b/c/d' \
--set 'client.snapshotAgent.configSecret.secretKey=snapshot-agent-config' \
. | tee /dev/stderr |
yq -r -c '.spec.template.spec.volumes[] | select(.name == "snapshot-config")' | tee /dev/stderr)
[ "${actual}" = "" ]
}

@test "client/SnapshotAgentDeployment: vault does not add volume mount for snapshot agent config secret" {
cd `chart_dir`
local actual=$(helm template \
-s templates/client-snapshot-agent-deployment.yaml \
--set 'global.secretsBackend.vault.enabled=true' \
--set 'global.secretsBackend.vault.consulClientRole=foo' \
--set 'global.secretsBackend.vault.consulServerRole=test' \
--set 'client.snapshotAgent.enabled=true' \
--set 'client.snapshotAgent.configSecret.secretName=a/b/c/d' \
--set 'client.snapshotAgent.configSecret.secretKey=snapshot-agent-config' \
. | tee /dev/stderr |
yq -r -c '.spec.template.spec.containers[0].volumeMounts[] | select(.name == "snapshot-config")' | tee /dev/stderr)
[ "${actual}" = "" ]
}

@test "client/SnapshotAgentDeployment: vault sets config-file argument on snapshot agent command to config downloaded by vault agent injector" {
cd `chart_dir`
local actual=$(helm template \
-s templates/client-snapshot-agent-deployment.yaml \
--set 'global.secretsBackend.vault.enabled=true' \
--set 'global.secretsBackend.vault.consulClientRole=foo' \
--set 'global.secretsBackend.vault.consulServerRole=test' \
--set 'client.snapshotAgent.enabled=true' \
--set 'client.snapshotAgent.configSecret.secretName=a/b/c/d' \
--set 'client.snapshotAgent.configSecret.secretKey=snapshot-agent-config' \
. | tee /dev/stderr |
yq -r '.spec.template.spec.containers[0].command[2] | contains("-config-file=/vault/secrets/snapshot-agent-config.json")' | tee /dev/stderr)
[ "${actual}" = 'true' ]
}

#--------------------------------------------------------------------
# Vault agent annotations

Expand Down Expand Up @@ -780,4 +931,67 @@ MIICFjCCAZsCCQCdwLtdjbzlYzAKBggqhkjOPQQDAjB0MQswCQYDVQQGEwJDQTEL' \
. | tee /dev/stderr |
yq -r '.spec.template.metadata.annotations.foo' | tee /dev/stderr)
[ "${actual}" = "bar" ]
}
}


@test "client/SnapshotAgentDeployment: vault properly sets vault role when global.secretsBackend.vault.consulCARole is set but global.secretsBackend.vault.consulSnapshotAgentRole is not set" {
cd `chart_dir`
local object=$(helm template \
-s templates/client-snapshot-agent-deployment.yaml \
--set 'client.snapshotAgent.enabled=true' \
--set 'global.tls.enabled=true' \
--set 'global.tls.enableAutoEncrypt=true' \
--set 'global.tls.caCert.secretName=foo' \
--set 'global.secretsBackend.vault.enabled=true' \
--set 'global.secretsBackend.vault.consulClientRole=test' \
--set 'global.secretsBackend.vault.consulServerRole=foo' \
--set 'global.secretsBackend.vault.consulCARole=ca-role' \
. | tee /dev/stderr |
yq -r '.spec.template' | tee /dev/stderr)

local actual
actual=$(echo $object | jq -r '.metadata.annotations["vault.hashicorp.com/role"]' | tee /dev/stderr)
[ "${actual}" = "ca-role" ]
}

@test "client/SnapshotAgentDeployment: vault properly sets vault role when global.secretsBackend.vault.consulSnapshotAgentRole is set but global.secretsBackend.vault.consulCARole is not set" {
cd `chart_dir`
local object=$(helm template \
-s templates/client-snapshot-agent-deployment.yaml \
--set 'client.snapshotAgent.enabled=true' \
--set 'global.secretsBackend.vault.enabled=true' \
--set 'global.secretsBackend.vault.consulClientRole=test' \
--set 'global.secretsBackend.vault.consulServerRole=foo' \
--set 'global.secretsBackend.vault.consulSnapshotAgentRole=sa-role' \
--set 'client.snapshotAgent.configSecret.secretName=a/b/c/d' \
--set 'client.snapshotAgent.configSecret.secretKey=snapshot-agent-config' \
. | tee /dev/stderr |
yq -r '.spec.template' | tee /dev/stderr)

local actual
actual=$(echo $object | jq -r '.metadata.annotations["vault.hashicorp.com/role"]' | tee /dev/stderr)
[ "${actual}" = "sa-role" ]
}

@test "client/SnapshotAgentDeployment: vault properly sets vault role to global.secretsBackend.vault.consulSnapshotAgentRole value when both global.secretsBackend.vault.consulSnapshotAgentRole and global.secretsBackend.vault.consulCARole are set" {
cd `chart_dir`
local object=$(helm template \
-s templates/client-snapshot-agent-deployment.yaml \
--set 'client.snapshotAgent.enabled=true' \
--set 'global.tls.enabled=true' \
--set 'global.tls.enableAutoEncrypt=true' \
--set 'global.tls.caCert.secretName=foo' \
--set 'global.secretsBackend.vault.enabled=true' \
--set 'global.secretsBackend.vault.consulClientRole=test' \
--set 'global.secretsBackend.vault.consulServerRole=foo' \
--set 'global.secretsBackend.vault.consulSnapshotAgentRole=sa-role' \
--set 'client.snapshotAgent.configSecret.secretName=a/b/c/d' \
--set 'client.snapshotAgent.configSecret.secretKey=snapshot-agent-config' \
--set 'global.secretsBackend.vault.consulCARole=ca-role' \
. | tee /dev/stderr |
yq -r '.spec.template' | tee /dev/stderr)

local actual
actual=$(echo $object | jq -r '.metadata.annotations["vault.hashicorp.com/role"]' | tee /dev/stderr)
[ "${actual}" = "sa-role" ]
}
14 changes: 12 additions & 2 deletions charts/consul/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,16 @@ global:
# and check the name of `metadata.name`.
consulClientRole: ""

# [Enterprise Only] The Vault role for the Consul client snapshot agent.
# The role must be connected to the Consul client snapshot agent's service account and
# have a policy with read capabilities for the snapshot agent config defined by `client.snapshotAgent.configSecret.secretName`.
# To discover the service account name of the Consul client, run
# ```shell-session
# $ helm template --show-only templates/client-snapshot-agent-serviceaccount.yaml --set client.snapshotAgent.enabled=true <release-name> hashicorp/consul
# ```
# and check the name of `metadata.name`.
consulSnapshotAgentRole: ""
Copy link
Contributor Author

Choose a reason for hiding this comment

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

User has to set up a vault policy and auth role that maps the service account to the policy that has access to the snapshot agent config secret. Configuring it here will assign it in a vault annotation in the snapshot agent deployment so that the vault injector for the snapshot agent config will be authorized.


# A Vault role to allow Kubernetes job that manages ACLs for this Helm chart (`server-acl-init`)
# to read and update Vault secrets for the Consul's bootstrap, replication or partition tokens.
# This role must be bound the `server-acl-init`'s service account.
Expand Down Expand Up @@ -1274,9 +1284,9 @@ client:
# credentials present. Please see Snapshot agent config (https://consul.io/commands/snapshot/agent#config-file-options)
# for details.
configSecret:
# The name of the Kubernetes secret.
# secretName is the name of the Kubernetes secret or Vault secret path that holds the snapshot agentconfig.
secretName: null
# The key of the Kubernetes secret.
# secretKey is the key within the Kubernetes secret or Vault secret key that holds the snapshot agentconfig.
secretKey: null

serviceAccount:
Expand Down