From 7e76901434bdd0918767a052acd7c9531322530c Mon Sep 17 00:00:00 2001 From: Erik Berg Date: Wed, 29 Jun 2022 22:46:47 +0200 Subject: [PATCH 1/6] fix dataDirectoryHostPath tests Assuming that volumes will always be at a fixed positions in a list volumes won't work in the long run. Use `select(.name ...` to find them. --- charts/consul/test/unit/client-daemonset.bats | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/charts/consul/test/unit/client-daemonset.bats b/charts/consul/test/unit/client-daemonset.bats index 9338e99faf..29b11d3553 100755 --- a/charts/consul/test/unit/client-daemonset.bats +++ b/charts/consul/test/unit/client-daemonset.bats @@ -1503,14 +1503,14 @@ local actual=$(echo $object | local actual=$(helm template \ -s templates/client-daemonset.yaml \ . | tee /dev/stderr | - yq '.spec.template.spec.volumes[0].hostPath == null' | tee /dev/stderr ) + yq '.spec.template.spec.volumes[] | select(.name == "data") | .hostPath == null' | tee /dev/stderr ) [ "${actual}" = "true" ] # Test that emptyDir is set instead. local actual=$(helm template \ -s templates/client-daemonset.yaml \ . | tee /dev/stderr | - yq '.spec.template.spec.volumes[0].emptyDir == {}' | tee /dev/stderr ) + yq '.spec.template.spec.volumes[] | select(.name == "data") | .emptyDir == {}' | tee /dev/stderr ) [ "${actual}" = "true" ] } @@ -1520,7 +1520,7 @@ local actual=$(echo $object | -s templates/client-daemonset.yaml \ --set 'client.dataDirectoryHostPath=/opt/consul' \ . | tee /dev/stderr | - yq '.spec.template.spec.volumes[0].hostPath.path == "/opt/consul"' | tee /dev/stderr) + yq '.spec.template.spec.volumes[] | select(.name == "data") | .hostPath.path == "/opt/consul"' | tee /dev/stderr) [ "${actual}" = "true" ] } From fd84db2e61f22f245b03b9aae7f95be16a39f0fb Mon Sep 17 00:00:00 2001 From: Erik Berg Date: Wed, 29 Jun 2022 23:02:00 +0200 Subject: [PATCH 2/6] fix test comparison There's no ${actual} here, only ${object} to compare with. --- charts/consul/test/unit/client-daemonset.bats | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/charts/consul/test/unit/client-daemonset.bats b/charts/consul/test/unit/client-daemonset.bats index 29b11d3553..52c8c7f492 100755 --- a/charts/consul/test/unit/client-daemonset.bats +++ b/charts/consul/test/unit/client-daemonset.bats @@ -1299,7 +1299,7 @@ local actual=$(echo $object | --set 'global.acls.manageSystemACLs=true' \ --set 'global.tls.enabled=false' \ . | yq '.spec.template.spec.initContainers[0].volumeMounts[] | select(.name=="consul-ca-cert")' | tee /dev/stderr) - [ "${actual}" == "" ] + [ "${object}" == "" ] } @test "client/DaemonSet: fail when externalServers is enabled but the externalServers.hosts is not provided" { From 878741e0516dde8576fe1d5a526b7835fd9fdf44 Mon Sep 17 00:00:00 2001 From: Erik Berg Date: Wed, 29 Jun 2022 23:18:11 +0200 Subject: [PATCH 3/6] select named initContainers Assuming that initContainers will always be at a fixed positions in a list won't work in the long run. Use `select(.name ...` to find them. --- charts/consul/test/unit/client-daemonset.bats | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/charts/consul/test/unit/client-daemonset.bats b/charts/consul/test/unit/client-daemonset.bats index 52c8c7f492..c784447d0c 100755 --- a/charts/consul/test/unit/client-daemonset.bats +++ b/charts/consul/test/unit/client-daemonset.bats @@ -1277,7 +1277,7 @@ local actual=$(echo $object | -s templates/client-daemonset.yaml \ --set 'global.acls.manageSystemACLs=true' \ --set 'global.tls.enabled=true' \ - . | yq '.spec.template.spec.initContainers[0].volumeMounts[2]' | tee /dev/stderr) + . | yq '.spec.template.spec.initContainers[] | select(.name == "client-acl-init") | .volumeMounts[] | select(.name == "consul-ca-cert")' | tee /dev/stderr) local actual=$(echo $object | yq -r '.name' | tee /dev/stderr) @@ -1298,7 +1298,7 @@ local actual=$(echo $object | -s templates/client-daemonset.yaml \ --set 'global.acls.manageSystemACLs=true' \ --set 'global.tls.enabled=false' \ - . | yq '.spec.template.spec.initContainers[0].volumeMounts[] | select(.name=="consul-ca-cert")' | tee /dev/stderr) + . | yq '.spec.template.spec.initContainers[] | select(.name == "client-acl-init") | .volumeMounts[] | select(.name=="consul-ca-cert")' | tee /dev/stderr) [ "${object}" == "" ] } @@ -1325,7 +1325,7 @@ local actual=$(echo $object | --set 'externalServers.hosts[0]=foo' \ --set 'externalServers.hosts[1]=bar' \ . | tee /dev/stderr | - yq -r '.spec.template.spec.initContainers[0].command' | tee /dev/stderr) + yq -r '.spec.template.spec.initContainers[] | select(.name == "client-acl-init") | .command' | tee /dev/stderr) local actual=$(echo $command | jq -r ' . | any(contains("-server-address=\"foo\""))' | tee /dev/stderr) [ "${actual}" = "true" ] @@ -1345,7 +1345,7 @@ local actual=$(echo $object | --set 'externalServers.hosts[0]=computer' \ --set 'externalServers.tlsServerName=foo' \ . | tee /dev/stderr | - yq -r '.spec.template.spec.initContainers[0].command' | tee /dev/stderr) + yq -r '.spec.template.spec.initContainers[] | select(.name == "client-acl-init") | .command' | tee /dev/stderr) local actual=$(echo $command | jq -r ' . | any(contains("-tls-server-name=foo"))' | tee /dev/stderr) [ "${actual}" = "true" ] @@ -1360,7 +1360,7 @@ local actual=$(echo $object | --set 'server.enabled=false' \ --set 'externalServers.hosts[0]=computer' \ . | tee /dev/stderr | - yq -r '.spec.template.spec.initContainers[0].command' | tee /dev/stderr) + yq -r '.spec.template.spec.initContainers[] | select(.name == "client-acl-init") | .command' | tee /dev/stderr) local actual=$(echo $command | jq -r ' . | any(contains("-tls-server-name"))' | tee /dev/stderr) [ "${actual}" = "false" ] @@ -1375,7 +1375,7 @@ local actual=$(echo $object | --set 'server.enabled=false' \ --set 'externalServers.hosts[0]=computer' \ . | tee /dev/stderr | - yq -r '.spec.template.spec.initContainers[0].command' | tee /dev/stderr) + yq -r '.spec.template.spec.initContainers[] | select(.name == "client-acl-init") | .command' | tee /dev/stderr) local actual=$(echo $command | jq -r ' . | any(contains("-use-https"))' | tee /dev/stderr) [ "${actual}" = "false" ] @@ -1391,7 +1391,7 @@ local actual=$(echo $object | --set 'externalServers.hosts[0]=computer' \ --set 'global.tls.enabled=true' \ . | tee /dev/stderr | - yq -r '.spec.template.spec.initContainers[0].command' | tee /dev/stderr) + yq -r '.spec.template.spec.initContainers[] | select(.name == "client-acl-init") | .command' | tee /dev/stderr) local actual=$(echo $command | jq -r ' . | any(contains("-use-https"))' | tee /dev/stderr) [ "${actual}" = "true" ] @@ -1407,7 +1407,7 @@ local actual=$(echo $object | --set 'global.tls.enabled=true' \ --set 'externalServers.hosts[0]=computer' \ . | tee /dev/stderr | - yq -r '.spec.template.spec.initContainers[0].command' | tee /dev/stderr) + yq -r '.spec.template.spec.initContainers[] | select(.name == "client-acl-init") | .command' | tee /dev/stderr) local actual=$(echo $command | jq -r ' . | any(contains("-use-https"))' | tee /dev/stderr) [ "${actual}" = "false" ] @@ -1422,7 +1422,7 @@ local actual=$(echo $object | --set 'server.enabled=false' \ --set 'externalServers.hosts[0]=computer' \ . | tee /dev/stderr | - yq -r '.spec.template.spec.initContainers[0].command' | tee /dev/stderr) + yq -r '.spec.template.spec.initContainers[] | select(.name == "client-acl-init") | .command' | tee /dev/stderr) local actual=$(echo $command | jq -r ' . | any(contains("-server-port"))' | tee /dev/stderr) [ "${actual}" = "false" ] @@ -1437,7 +1437,7 @@ local actual=$(echo $object | --set 'server.enabled=false' \ --set 'externalServers.hosts[0]=computer' \ . | tee /dev/stderr | - yq -r '.spec.template.spec.initContainers[0].command' | tee /dev/stderr) + yq -r '.spec.template.spec.initContainers[] | select(.name == "client-acl-init") | .command' | tee /dev/stderr) local actual=$(echo $command | jq -r ' . | any(contains("-server-port"))' | tee /dev/stderr) [ "${actual}" = "true" ] From 09f6bbc7715161823d2d4434a47f179609fbb311 Mon Sep 17 00:00:00 2001 From: Erik Berg Date: Wed, 29 Jun 2022 23:59:08 +0200 Subject: [PATCH 4/6] simplify yq queries swap map for [] --- charts/consul/test/unit/client-daemonset.bats | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/charts/consul/test/unit/client-daemonset.bats b/charts/consul/test/unit/client-daemonset.bats index c784447d0c..da6ce020ee 100755 --- a/charts/consul/test/unit/client-daemonset.bats +++ b/charts/consul/test/unit/client-daemonset.bats @@ -1453,7 +1453,7 @@ local actual=$(echo $object | --set 'client.enabled=true' \ --set 'client.exposeGossipPorts=false' \ . | tee /dev/stderr | - yq -r '.spec.template.spec.containers | map(select(.name=="consul")) | .[0].env | map(select(.name=="ADVERTISE_IP")) | .[0] | .valueFrom.fieldRef.fieldPath' | + yq -r '.spec.template.spec.containers[] | select(.name=="consul") | .env[] | select(.name=="ADVERTISE_IP") | .valueFrom.fieldRef.fieldPath' | tee /dev/stderr) [ "${actual}" = "status.podIP" ] } @@ -1465,33 +1465,33 @@ local actual=$(echo $object | --set 'client.enabled=true' \ --set 'client.exposeGossipPorts=true' \ . | tee /dev/stderr | - yq -r '.spec.template.spec.containers | map(select(.name=="consul")) | .[0].env | map(select(.name=="ADVERTISE_IP")) | .[0] | .valueFrom.fieldRef.fieldPath' | + yq -r '.spec.template.spec.containers[] | select(.name=="consul") | .env[] | select(.name=="ADVERTISE_IP") | .valueFrom.fieldRef.fieldPath' | tee /dev/stderr) [ "${actual}" = "status.hostIP" ] } @test "client/DaemonSet: client doesn't expose hostPorts when client.exposeGossipPorts=false" { cd `chart_dir` - local actual=$(helm template \ + local has_exposed_host_ports=$(helm template \ -s templates/client-daemonset.yaml \ --set 'server.enabled=true' \ --set 'client.enabled=true' \ . | tee /dev/stderr | - yq '.spec.template.spec.containers | map(select(.name=="consul")) | .[0].ports | map(select(.containerPort==8301)) | .[0].hostPort' | + yq '[.spec.template.spec.containers[] | select(.name=="consul") | .ports[] | select(.containerPort==8301)] | any(has("hostPort"))' | tee /dev/stderr) - [ "${actual}" = "null" ] + [ "${has_exposed_host_ports}" = "false" ] } @test "client/DaemonSet: client exposes hostPorts when client.exposeGossipPorts=true" { cd `chart_dir` - local actual=$(helm template \ + local has_exposed_host_ports=$(helm template \ -s templates/client-daemonset.yaml \ --set 'client.enabled=true' \ --set 'client.exposeGossipPorts=true' \ . | tee /dev/stderr | - yq '.spec.template.spec.containers | map(select(.name=="consul")) | .[0].ports | map(select(.containerPort==8301)) | .[0].hostPort' | + yq '[.spec.template.spec.containers[] | select(.name=="consul") | .ports[] | select(.containerPort==8301)] | all(has("hostPort"))' | tee /dev/stderr) - [ "${actual}" = "8301" ] + [ "${has_exposed_host_ports}" = "true" ] } #-------------------------------------------------------------------- @@ -1682,13 +1682,13 @@ rollingUpdate: --set 'client.containerSecurityContext.tlsInit.readOnlyRootFileSystem=true' \ . | tee /dev/stderr) - local actual=$(echo "$manifest" | yq -r '.spec.template.spec.containers | map(select(.name == "consul")) | .[0].securityContext.privileged') + local actual=$(echo "$manifest" | yq -r '.spec.template.spec.containers[] | select(.name == "consul") | .securityContext.privileged') [ "${actual}" = "false" ] - local actual=$(echo "$manifest" | yq -r '.spec.template.spec.initContainers | map(select(.name == "client-acl-init")) | .[0].securityContext.allowPrivilegeEscalation') + local actual=$(echo "$manifest" | yq -r '.spec.template.spec.initContainers[] | select(.name == "client-acl-init") | .securityContext.allowPrivilegeEscalation') [ "${actual}" = "false" ] - local actual=$(echo "$manifest" | yq -r '.spec.template.spec.initContainers | map(select(.name == "client-tls-init")) | .[0].securityContext.readOnlyRootFileSystem') + local actual=$(echo "$manifest" | yq -r '.spec.template.spec.initContainers[] | select(.name == "client-tls-init") | .securityContext.readOnlyRootFileSystem') [ "${actual}" = "true" ] } @@ -1708,13 +1708,13 @@ rollingUpdate: --set 'client.containerSecurityContext.tlsInit.readOnlyRootFileSystem=true' \ . | tee /dev/stderr) - local actual=$(echo "$manifest" | yq -r '.spec.template.spec.containers | map(select(.name == "consul")) | .[0].securityContext') + local actual=$(echo "$manifest" | yq -r '.spec.template.spec.containers[] | select(.name == "consul") | .securityContext') [ "${actual}" = "null" ] - local actual=$(echo "$manifest" | yq -r '.spec.template.spec.initContainers | map(select(.name == "client-acl-init")) | .[0].securityContext') + local actual=$(echo "$manifest" | yq -r '.spec.template.spec.initContainers[] | select(.name == "client-acl-init") | .securityContext') [ "${actual}" = "null" ] - local actual=$(echo "$manifest" | yq -r '.spec.template.spec.initContainers | map(select(.name == "client-tls-init")) | .[0].securityContext') + local actual=$(echo "$manifest" | yq -r '.spec.template.spec.initContainers[] | select(.name == "client-tls-init") | .securityContext') [ "${actual}" = "null" ] } From 62edc36860c2e8cc8ca1ba4aad4b288eb2dbbc4c Mon Sep 17 00:00:00 2001 From: Erik Berg Date: Mon, 4 Jul 2022 14:44:44 +0200 Subject: [PATCH 5/6] check all of env for CONSUL_HTTP_TOKEN_FILE This test was probably meant to check all items in the env for CONSUL_HTTP_TOKEN_FILE and not just the first entry. Also, find the container named consul rather than assuming it's the first in the list. --- charts/consul/test/unit/client-daemonset.bats | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/charts/consul/test/unit/client-daemonset.bats b/charts/consul/test/unit/client-daemonset.bats index da6ce020ee..7ca8bcb63f 100755 --- a/charts/consul/test/unit/client-daemonset.bats +++ b/charts/consul/test/unit/client-daemonset.bats @@ -1192,7 +1192,7 @@ local actual=$(echo $object | -s templates/client-daemonset.yaml \ --set 'global.acls.manageSystemACLs=false' \ . | tee /dev/stderr | - yq '[.spec.template.spec.containers[0].env[0].name] | any(contains("CONSUL_HTTP_TOKEN_FILE"))' | tee /dev/stderr) + yq '[.spec.template.spec.containers[] | select(.name == "consul") | .env[] | .name] | any(contains("CONSUL_HTTP_TOKEN_FILE"))' | tee /dev/stderr) [ "${actual}" = "false" ] } @@ -1202,7 +1202,7 @@ local actual=$(echo $object | -s templates/client-daemonset.yaml \ --set 'global.acls.manageSystemACLs=true' \ . | tee /dev/stderr | - yq '[.spec.template.spec.containers[0].env[0].name] | any(contains("CONSUL_HTTP_TOKEN_FILE"))' | tee /dev/stderr) + yq '[.spec.template.spec.containers[] | select(.name == "consul") | .env[] | .name] | any(contains("CONSUL_HTTP_TOKEN_FILE"))' | tee /dev/stderr) [ "${actual}" = "true" ] } From 7178b992b1df97c2f3eb8349ab3ffb31d0e5b3e1 Mon Sep 17 00:00:00 2001 From: Erik Berg Date: Mon, 4 Jul 2022 15:10:05 +0200 Subject: [PATCH 6/6] Re-word tests with has("securityContext") I think this improves the readability of the tests. --- charts/consul/test/unit/client-daemonset.bats | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/charts/consul/test/unit/client-daemonset.bats b/charts/consul/test/unit/client-daemonset.bats index 7ca8bcb63f..7e3aaa4146 100755 --- a/charts/consul/test/unit/client-daemonset.bats +++ b/charts/consul/test/unit/client-daemonset.bats @@ -1620,12 +1620,12 @@ rollingUpdate: @test "client/DaemonSet: securityContext is not set when global.openshift.enabled=true" { cd `chart_dir` - local actual=$(helm template \ + local has_security_context=$(helm template \ -s templates/client-daemonset.yaml \ --set 'global.openshift.enabled=true' \ . | tee /dev/stderr | - yq -r '.spec.template.spec.securityContext' | tee /dev/stderr) - [ "${actual}" = "null" ] + yq -r '.spec.template.spec | has("securityContext")' | tee /dev/stderr) + [ "${has_security_context}" = "false" ] } #-------------------------------------------------------------------- @@ -1708,14 +1708,14 @@ rollingUpdate: --set 'client.containerSecurityContext.tlsInit.readOnlyRootFileSystem=true' \ . | tee /dev/stderr) - local actual=$(echo "$manifest" | yq -r '.spec.template.spec.containers[] | select(.name == "consul") | .securityContext') - [ "${actual}" = "null" ] + local has_security_context=$(echo "$manifest" | yq -r '.spec.template.spec.containers[] | select(.name == "consul") | has("securityContext")') + [ "${has_security_context}" = "false" ] - local actual=$(echo "$manifest" | yq -r '.spec.template.spec.initContainers[] | select(.name == "client-acl-init") | .securityContext') - [ "${actual}" = "null" ] + local has_security_context=$(echo "$manifest" | yq -r '.spec.template.spec.initContainers[] | select(.name == "client-acl-init") | has("securityContext")') + [ "${has_security_context}" = "false" ] - local actual=$(echo "$manifest" | yq -r '.spec.template.spec.initContainers[] | select(.name == "client-tls-init") | .securityContext') - [ "${actual}" = "null" ] + local has_security_context=$(echo "$manifest" | yq -r '.spec.template.spec.initContainers[] | select(.name == "client-tls-init") | has("securityContext")') + [ "${has_security_context}" = "false" ] } #--------------------------------------------------------------------