From dccc3c4273c830e747c4e69008e4793296b743bf Mon Sep 17 00:00:00 2001 From: Curt Bushko Date: Tue, 15 Mar 2022 23:02:36 -0400 Subject: [PATCH 1/6] improve bats tests add changes to see how acceptance tests do Only do client and server turn ACL back on in vault_test and add bats tests --- acceptance/tests/vault/vault_test.go | 17 +++++++++++++++ charts/consul/templates/client-daemonset.yaml | 9 +++++--- .../consul/templates/server-statefulset.yaml | 4 ++++ charts/consul/test/unit/client-daemonset.bats | 21 ++++++++++++++++++- .../consul/test/unit/server-statefulset.bats | 19 +++++++++++++++++ 5 files changed, 66 insertions(+), 4 deletions(-) diff --git a/acceptance/tests/vault/vault_test.go b/acceptance/tests/vault/vault_test.go index ed4410999b..6216079de7 100644 --- a/acceptance/tests/vault/vault_test.go +++ b/acceptance/tests/vault/vault_test.go @@ -1,6 +1,8 @@ package vault import ( + "context" + "fmt" "testing" "github.com/hashicorp/consul-k8s/acceptance/framework/consul" @@ -9,6 +11,7 @@ import ( "github.com/hashicorp/consul-k8s/acceptance/framework/logger" "github.com/hashicorp/consul-k8s/acceptance/framework/vault" "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) const staticClientName = "static-client" @@ -111,6 +114,20 @@ func TestVault(t *testing.T) { require.NoError(t, err) require.Equal(t, caConfig.Provider, "vault") + // Validate that tls.crt is not set to the default value + logger.Log(t, "Validating that CONSUL_CACERT has been set correctly") + serverPod, err := ctx.KubernetesClient(t).CoreV1().Pods(ns).Get(context.Background(), fmt.Sprintf("%s-consul-server-0", consulReleaseName), metav1.GetOptions{}) + require.NoError(t, err) + + // Environment variables are buried in the pod spec + containerVars := serverPod.Spec.Containers[0].Env + for k := range containerVars { + if containerVars[k].Name == "CONSUL_CACERT" { + require.NotEqual(t, containerVars[k].Value, "/consul/tls/ca/tls.crt") + require.Equal(t, containerVars[k].Value, "/vault/secrets/serverca.crt") + } + } + if cfg.EnableEnterprise { // Validate that the enterprise license is set correctly. logger.Log(t, "Validating the enterprise license has been set correctly.") diff --git a/charts/consul/templates/client-daemonset.yaml b/charts/consul/templates/client-daemonset.yaml index d5d4a07de0..ea96247888 100644 --- a/charts/consul/templates/client-daemonset.yaml +++ b/charts/consul/templates/client-daemonset.yaml @@ -231,13 +231,16 @@ spec: {{- if .Values.global.tls.enabled }} - name: CONSUL_HTTP_ADDR value: https://localhost:8501 - {{- if .Values.global.tls.enableAutoEncrypt }} + {{- if .Values.global.tls.enableAutoEncrypt }} - name: CONSUL_HTTP_SSL_VERIFY value: "false" - {{- else }} + {{- end }} - name: CONSUL_CACERT + {{- if .Values.global.secretsBackend.vault.enabled }} + value: /vault/secrets/serverca.crt + {{- else }} value: /consul/tls/ca/tls.crt - {{- end }} + {{- end }} {{- end }} {{- include "consul.extraEnvironmentVars" .Values.client | nindent 12 }} command: diff --git a/charts/consul/templates/server-statefulset.yaml b/charts/consul/templates/server-statefulset.yaml index e2eda87dc5..0d669f3a81 100644 --- a/charts/consul/templates/server-statefulset.yaml +++ b/charts/consul/templates/server-statefulset.yaml @@ -223,7 +223,11 @@ spec: - name: CONSUL_HTTP_ADDR value: https://localhost:8501 - name: CONSUL_CACERT + {{- if .Values.global.secretsBackend.vault.enabled }} + value: /vault/secrets/serverca.crt + {{- else }} value: /consul/tls/ca/tls.crt + {{- end }} {{- end }} {{- if (and .Values.global.enterpriseLicense.secretName .Values.global.enterpriseLicense.enableLicenseAutoload) }} - name: CONSUL_LICENSE_PATH diff --git a/charts/consul/test/unit/client-daemonset.bats b/charts/consul/test/unit/client-daemonset.bats index eb1c2b5048..5b283df413 100755 --- a/charts/consul/test/unit/client-daemonset.bats +++ b/charts/consul/test/unit/client-daemonset.bats @@ -841,6 +841,25 @@ load _helpers [ "${actual}" = "/consul/tls/ca/tls.crt" ] } +@test "client/DaemonSet: sets Consul environment variables when global.tls.enabled and global.secretsBackend.vault.enabled " { + cd `chart_dir` + local env=$(helm template \ + -s templates/client-daemonset.yaml \ + --set 'global.tls.enabled=true' \ + --set 'global.tls.enableAutoEncrypt=true' \ + --set 'global.secretsBackend.vault.enabled=true' \ + --set 'global.secretsBackend.vault.consulClientRole=foo' \ + --set 'global.secretsBackend.vault.consulServerRole=test' \ + --set 'global.secretsBackend.vault.consulCARole=test' \ + --set 'global.tls.caCert.secretName=pki_int/cert/ca' \ + . | tee /dev/stderr | + yq -r '.spec.template.spec.containers[0].env[]' | tee /dev/stderr) + + local actual + actual=$(echo $env | jq -r '. | select(.name == "CONSUL_CACERT") | .value' | tee /dev/stderr) + [ "${actual}" = "/vault/secrets/serverca.crt" ] +} + @test "client/DaemonSet: sets verify_* flags to true by default when global.tls.enabled" { cd `chart_dir` local command=$(helm template \ @@ -1966,4 +1985,4 @@ rollingUpdate: [ "$status" -eq 1 ] [[ "$output" =~ "global.imageK8s is not a valid key, use global.imageK8S (note the capital 'S')" ]] -} \ No newline at end of file +} diff --git a/charts/consul/test/unit/server-statefulset.bats b/charts/consul/test/unit/server-statefulset.bats index 45aa6ea443..fc08cdcc3f 100755 --- a/charts/consul/test/unit/server-statefulset.bats +++ b/charts/consul/test/unit/server-statefulset.bats @@ -1113,6 +1113,25 @@ load _helpers [ "${actual}" = "/consul/tls/ca/tls.crt" ] } +@test "server/StatefulSet: sets Consul environment variables when global.tls.enabled and global.secretsBackend.vault.enabled" { + cd `chart_dir` + local env=$(helm template \ + -s templates/server-statefulset.yaml \ + --set 'global.tls.enabled=true' \ + --set 'global.tls.enableAutoEncrypt=true' \ + --set 'global.secretsBackend.vault.enabled=true' \ + --set 'global.secretsBackend.vault.consulClientRole=foo' \ + --set 'global.secretsBackend.vault.consulServerRole=test' \ + --set 'global.secretsBackend.vault.consulCARole=test' \ + --set 'global.tls.caCert.secretName=pki_int/cert/ca' \ + . | tee /dev/stderr | + yq -r '.spec.template.spec.containers[0].env[]' | tee /dev/stderr) + + local actual + actual=$(echo $env | jq -r '. | select(.name == "CONSUL_CACERT") | .value' | tee /dev/stderr) + [ "${actual}" = "/vault/secrets/serverca.crt" ] +} + @test "server/StatefulSet: sets verify_* flags to true by default when global.tls.enabled" { cd `chart_dir` local command=$(helm template \ From 5ae19c1c3bbf245ecb43331fc4771529b3543e0a Mon Sep 17 00:00:00 2001 From: Curt Bushko Date: Wed, 16 Mar 2022 13:47:13 -0400 Subject: [PATCH 2/6] fix tabs of client-daemonset.yaml --- charts/consul/templates/client-daemonset.yaml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/charts/consul/templates/client-daemonset.yaml b/charts/consul/templates/client-daemonset.yaml index ea96247888..b9f5beecf7 100644 --- a/charts/consul/templates/client-daemonset.yaml +++ b/charts/consul/templates/client-daemonset.yaml @@ -231,16 +231,16 @@ spec: {{- if .Values.global.tls.enabled }} - name: CONSUL_HTTP_ADDR value: https://localhost:8501 - {{- if .Values.global.tls.enableAutoEncrypt }} + {{- if .Values.global.tls.enableAutoEncrypt }} - name: CONSUL_HTTP_SSL_VERIFY value: "false" - {{- end }} + {{- end }} - name: CONSUL_CACERT - {{- if .Values.global.secretsBackend.vault.enabled }} + {{- if .Values.global.secretsBackend.vault.enabled }} value: /vault/secrets/serverca.crt - {{- else }} + {{- else }} value: /consul/tls/ca/tls.crt - {{- end }} + {{- end }} {{- end }} {{- include "consul.extraEnvironmentVars" .Values.client | nindent 12 }} command: From 8f99c4e273b223aad49cd5f12763d8788f1b2424 Mon Sep 17 00:00:00 2001 From: Curt Bushko Date: Fri, 18 Mar 2022 15:26:02 -0400 Subject: [PATCH 3/6] client-daemonset did not need to change and test that commands work still --- acceptance/tests/vault/vault_test.go | 19 ++++--------------- charts/consul/templates/client-daemonset.yaml | 5 +---- 2 files changed, 5 insertions(+), 19 deletions(-) diff --git a/acceptance/tests/vault/vault_test.go b/acceptance/tests/vault/vault_test.go index 6216079de7..6192da10ed 100644 --- a/acceptance/tests/vault/vault_test.go +++ b/acceptance/tests/vault/vault_test.go @@ -1,7 +1,6 @@ package vault import ( - "context" "fmt" "testing" @@ -11,7 +10,6 @@ import ( "github.com/hashicorp/consul-k8s/acceptance/framework/logger" "github.com/hashicorp/consul-k8s/acceptance/framework/vault" "github.com/stretchr/testify/require" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) const staticClientName = "static-client" @@ -68,7 +66,7 @@ func TestVault(t *testing.T) { "global.secretsBackend.vault.connectCA.rootPKIPath": "connect_root", "global.secretsBackend.vault.connectCA.intermediatePKIPath": "dc1/connect_inter", - "global.acls.manageSystemACLs": "true", + "global.acls.manageSystemACLs": "false", "global.tls.enabled": "true", "global.gossipEncryption.secretName": "consul/data/secret/gossip", "global.gossipEncryption.secretKey": "gossip", @@ -114,19 +112,10 @@ func TestVault(t *testing.T) { require.NoError(t, err) require.Equal(t, caConfig.Provider, "vault") - // Validate that tls.crt is not set to the default value - logger.Log(t, "Validating that CONSUL_CACERT has been set correctly") - serverPod, err := ctx.KubernetesClient(t).CoreV1().Pods(ns).Get(context.Background(), fmt.Sprintf("%s-consul-server-0", consulReleaseName), metav1.GetOptions{}) + // Validate that consul sever is running correctly and the consul members command works + membersOutput, err := k8s.RunKubectlAndGetOutputE(t, ctx.KubectlOptions(t), "exec", "consul members") require.NoError(t, err) - - // Environment variables are buried in the pod spec - containerVars := serverPod.Spec.Containers[0].Env - for k := range containerVars { - if containerVars[k].Name == "CONSUL_CACERT" { - require.NotEqual(t, containerVars[k].Value, "/consul/tls/ca/tls.crt") - require.Equal(t, containerVars[k].Value, "/vault/secrets/serverca.crt") - } - } + require.Contains(t, membersOutput, fmt.Sprintf("%s-consul-server-0", consulReleaseName)) if cfg.EnableEnterprise { // Validate that the enterprise license is set correctly. diff --git a/charts/consul/templates/client-daemonset.yaml b/charts/consul/templates/client-daemonset.yaml index b9f5beecf7..d5d4a07de0 100644 --- a/charts/consul/templates/client-daemonset.yaml +++ b/charts/consul/templates/client-daemonset.yaml @@ -234,11 +234,8 @@ spec: {{- if .Values.global.tls.enableAutoEncrypt }} - name: CONSUL_HTTP_SSL_VERIFY value: "false" - {{- end }} - - name: CONSUL_CACERT - {{- if .Values.global.secretsBackend.vault.enabled }} - value: /vault/secrets/serverca.crt {{- else }} + - name: CONSUL_CACERT value: /consul/tls/ca/tls.crt {{- end }} {{- end }} From cafad2f21033df13cc2734f0356e688258d1f0bf Mon Sep 17 00:00:00 2001 From: Curt Bushko Date: Fri, 18 Mar 2022 15:44:55 -0400 Subject: [PATCH 4/6] revert client-daemonset bats tests --- charts/consul/test/unit/client-daemonset.bats | 21 +------------------ 1 file changed, 1 insertion(+), 20 deletions(-) diff --git a/charts/consul/test/unit/client-daemonset.bats b/charts/consul/test/unit/client-daemonset.bats index 5b283df413..eb1c2b5048 100755 --- a/charts/consul/test/unit/client-daemonset.bats +++ b/charts/consul/test/unit/client-daemonset.bats @@ -841,25 +841,6 @@ load _helpers [ "${actual}" = "/consul/tls/ca/tls.crt" ] } -@test "client/DaemonSet: sets Consul environment variables when global.tls.enabled and global.secretsBackend.vault.enabled " { - cd `chart_dir` - local env=$(helm template \ - -s templates/client-daemonset.yaml \ - --set 'global.tls.enabled=true' \ - --set 'global.tls.enableAutoEncrypt=true' \ - --set 'global.secretsBackend.vault.enabled=true' \ - --set 'global.secretsBackend.vault.consulClientRole=foo' \ - --set 'global.secretsBackend.vault.consulServerRole=test' \ - --set 'global.secretsBackend.vault.consulCARole=test' \ - --set 'global.tls.caCert.secretName=pki_int/cert/ca' \ - . | tee /dev/stderr | - yq -r '.spec.template.spec.containers[0].env[]' | tee /dev/stderr) - - local actual - actual=$(echo $env | jq -r '. | select(.name == "CONSUL_CACERT") | .value' | tee /dev/stderr) - [ "${actual}" = "/vault/secrets/serverca.crt" ] -} - @test "client/DaemonSet: sets verify_* flags to true by default when global.tls.enabled" { cd `chart_dir` local command=$(helm template \ @@ -1985,4 +1966,4 @@ rollingUpdate: [ "$status" -eq 1 ] [[ "$output" =~ "global.imageK8s is not a valid key, use global.imageK8S (note the capital 'S')" ]] -} +} \ No newline at end of file From 1cb266c38a8f61cabf2edb3fc25a1062b2c4c006 Mon Sep 17 00:00:00 2001 From: Curt Bushko Date: Mon, 21 Mar 2022 10:29:44 -0400 Subject: [PATCH 5/6] use bootstrap token to run consul members command --- acceptance/tests/vault/vault_test.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/acceptance/tests/vault/vault_test.go b/acceptance/tests/vault/vault_test.go index 6192da10ed..14a68b27a5 100644 --- a/acceptance/tests/vault/vault_test.go +++ b/acceptance/tests/vault/vault_test.go @@ -1,6 +1,7 @@ package vault import ( + "context" "fmt" "testing" @@ -10,6 +11,7 @@ import ( "github.com/hashicorp/consul-k8s/acceptance/framework/logger" "github.com/hashicorp/consul-k8s/acceptance/framework/vault" "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) const staticClientName = "static-client" @@ -66,7 +68,7 @@ func TestVault(t *testing.T) { "global.secretsBackend.vault.connectCA.rootPKIPath": "connect_root", "global.secretsBackend.vault.connectCA.intermediatePKIPath": "dc1/connect_inter", - "global.acls.manageSystemACLs": "false", + "global.acls.manageSystemACLs": "true", "global.tls.enabled": "true", "global.gossipEncryption.secretName": "consul/data/secret/gossip", "global.gossipEncryption.secretKey": "gossip", @@ -113,7 +115,12 @@ func TestVault(t *testing.T) { require.Equal(t, caConfig.Provider, "vault") // Validate that consul sever is running correctly and the consul members command works - membersOutput, err := k8s.RunKubectlAndGetOutputE(t, ctx.KubectlOptions(t), "exec", "consul members") + tokenSecret, err := ctx.KubernetesClient(t).CoreV1().Secrets(ns).Get(context.Background(), fmt.Sprintf("%s-consul-bootstrap-acl-token", consulReleaseName), metav1.GetOptions{}) + require.NoError(t, err) + token := string(tokenSecret.Data["token"]) + + membersOutput, err := k8s.RunKubectlAndGetOutputE(t, ctx.KubectlOptions(t), "exec", fmt.Sprintf("%s-consul-server-0", consulReleaseName), "-c", "consul", "--", "sh", "-c", fmt.Sprintf("CONSUL_HTTP_TOKEN=%s consul members", token)) + logger.Log(t, "Members: ", membersOutput) require.NoError(t, err) require.Contains(t, membersOutput, fmt.Sprintf("%s-consul-server-0", consulReleaseName)) From 5a5e4f5f86e50afe242b85a7a396539b6cae7924 Mon Sep 17 00:00:00 2001 From: Curt Bushko Date: Mon, 21 Mar 2022 15:08:06 -0400 Subject: [PATCH 6/6] turning off logger so that token is not output and nicer member output --- acceptance/tests/vault/vault_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/acceptance/tests/vault/vault_test.go b/acceptance/tests/vault/vault_test.go index 14a68b27a5..dcf151f231 100644 --- a/acceptance/tests/vault/vault_test.go +++ b/acceptance/tests/vault/vault_test.go @@ -5,6 +5,7 @@ import ( "fmt" "testing" + terratestLogger "github.com/gruntwork-io/terratest/modules/logger" "github.com/hashicorp/consul-k8s/acceptance/framework/consul" "github.com/hashicorp/consul-k8s/acceptance/framework/helpers" "github.com/hashicorp/consul-k8s/acceptance/framework/k8s" @@ -119,8 +120,9 @@ func TestVault(t *testing.T) { require.NoError(t, err) token := string(tokenSecret.Data["token"]) - membersOutput, err := k8s.RunKubectlAndGetOutputE(t, ctx.KubectlOptions(t), "exec", fmt.Sprintf("%s-consul-server-0", consulReleaseName), "-c", "consul", "--", "sh", "-c", fmt.Sprintf("CONSUL_HTTP_TOKEN=%s consul members", token)) - logger.Log(t, "Members: ", membersOutput) + logger.Log(t, "Confirming that we can run Consul commands when exec'ing into server container") + membersOutput, err := k8s.RunKubectlAndGetOutputWithLoggerE(t, ctx.KubectlOptions(t), terratestLogger.Discard, "exec", fmt.Sprintf("%s-consul-server-0", consulReleaseName), "-c", "consul", "--", "sh", "-c", fmt.Sprintf("CONSUL_HTTP_TOKEN=%s consul members", token)) + logger.Logf(t, "Members: \n%s", membersOutput) require.NoError(t, err) require.Contains(t, membersOutput, fmt.Sprintf("%s-consul-server-0", consulReleaseName))