Apply suggestions from code review

Co-authored-by: Kyle Schochenmaier <kschoche@gmail.com>
hashicorp · Mar 9, 2022 · fb2eb0b · fb2eb0b
1 parent a728869
commit fb2eb0b
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 36 deletions.
diff --git a/acceptance/tests/vault/vault_wan_fed_test.go b/acceptance/tests/vault/vault_wan_fed_test.go
@@ -200,12 +200,26 @@ func TestVault_WANFederationViaGateways(t *testing.T) {
 	primaryConsulCluster := consul.NewHelmCluster(t, primaryConsulHelmValues, primaryCtx, cfg, consulReleaseName)
 	primaryConsulCluster.Create(t)
 
+	var k8sAuthMethodHost string
+	// When running on kind, the kube API address in kubeconfig will have a localhost address
+	// which will not work from inside the container. That's why we need to use the endpoints address instead
+	// which will point the node IP.
+	if cfg.UseKind {
+		// The Kubernetes AuthMethod host is read from the endpoints for the Kubernetes service.
+		kubernetesEndpoint, err := secondaryCtx.KubernetesClient(t).CoreV1().Endpoints("default").Get(context.Background(), "kubernetes", metav1.GetOptions{})
+		require.NoError(t, err)
+		k8sAuthMethodHost = fmt.Sprintf("%s:%d", kubernetesEndpoint.Subsets[0].Addresses[0].IP, kubernetesEndpoint.Subsets[0].Ports[0].Port)
+	} else {
+		k8sAuthMethodHost = k8s.KubernetesAPIServerHostFromOptions(t, secondaryCtx.KubectlOptions(t))
+	}
+
 	// Get the address of the mesh gateway.
 	primaryMeshGWAddress := meshGatewayAddress(t, cfg, primaryCtx, consulReleaseName)
 	secondaryConsulHelmValues := map[string]string{
 		"global.datacenter": "dc2",
 
 		"global.federation.enabled":            "true",
+		"global.federation.k8sAuthMethodHost":  k8sAuthMethodHost,
 		"global.federation.primaryDatacenter":  "dc1",
 		"global.federation.primaryGateways[0]": primaryMeshGWAddress,
 

diff --git a/charts/consul/templates/mesh-gateway-deployment.yaml b/charts/consul/templates/mesh-gateway-deployment.yaml
@@ -122,11 +122,16 @@ spec:
         {{- include "consul.getAutoEncryptClientCA" . | nindent 8 }}
         {{- end }}
         - name: mesh-gateway-init
+          image: {{ .Values.global.imageK8S }}
           env:
           - name: HOST_IP
             valueFrom:
               fieldRef:
                 fieldPath: status.hostIP
+          - name: POD_IP
+            valueFrom:
+              fieldRef:
+                fieldPath: status.podIP
           {{- if .Values.global.tls.enabled }}
           - name: CONSUL_CACERT
             value: /consul/tls/ca/tls.crt
@@ -137,27 +142,14 @@ spec:
             {{- else }}
             value: http://$(HOST_IP):8500
             {{- end }}
-          image: {{ .Values.global.imageK8S }}
-          volumeMounts:
-          - mountPath: /consul/login
-            name: consul-data
-            readOnly: false
-          {{- if .Values.global.tls.enabled }}
-          {{- if .Values.global.tls.enableAutoEncrypt }}
-          - name: consul-auto-encrypt-ca-cert
-          {{- else }}
-          - name: consul-ca-cert
-          {{- end }}
-            mountPath: /consul/tls/ca
-            readOnly: true
-          {{- end }}
           command:
             - "/bin/sh"
             - "-ec"
             - |
                 {{- if .Values.global.acls.manageSystemACLs }}
                 consul-k8s-control-plane acl-init \
                   -component-name=mesh-gateway \
+                  -token-sink-file=/consul/service/acl-token \
                   {{- if and .Values.global.federation.enabled .Values.global.federation.primaryDatacenter }}
                   -acl-auth-method={{ template "consul.fullname" . }}-k8s-component-auth-method-{{ .Values.global.datacenter }} \
                   -primary-datacenter={{ .Values.global.federation.primaryDatacenter }} \
@@ -244,9 +236,23 @@ spec:
 
                 /consul-bin/consul services register \
                   {{- if .Values.global.acls.manageSystemACLs }}
-                  -token-file=/consul/login/acl-token \
+                  -token-file=/consul/service/acl-token \
                   {{- end }}
                   /consul/service/service.hcl
+          volumeMounts:
+            - name: consul-service
+              mountPath: /consul/service
+            - name: consul-bin
+              mountPath: /consul-bin
+            {{- if .Values.global.tls.enabled }}
+            {{- if .Values.global.tls.enableAutoEncrypt }}
+            - name: consul-auto-encrypt-ca-cert
+            {{- else }}
+            - name: consul-ca-cert
+            {{- end }}
+              mountPath: /consul/tls/ca
+              readOnly: true
+            {{- end }}
           {{- if .Values.meshGateway.initServiceInitContainer.resources }}
           resources: {{ toYaml .Values.meshGateway.initServiceInitContainer.resources | nindent 12 }}
           {{- end }}
@@ -262,8 +268,8 @@ spec:
             {{- end }}
           {{- end }}
           volumeMounts:
-          - mountPath: /consul/login
-            name: consul-data
+          - mountPath: /consul/service
+            name: consul-service
             readOnly: true
           - name: consul-bin
             mountPath: /consul-bin
@@ -293,7 +299,7 @@ spec:
             {{- end }}
             {{- if .Values.global.acls.manageSystemACLs }}
             - name: CONSUL_HTTP_TOKEN_FILE
-              value: /consul/login/acl-token
+              value: /consul/service/acl-token
             {{- end }}
             {{- if .Values.global.tls.enabled }}
             - name: CONSUL_HTTP_ADDR
@@ -341,7 +347,7 @@ spec:
           lifecycle:
             preStop:
               exec:
-                command: ["/bin/sh", "-ec", "/consul-bin/consul logout -token-file=/consul/login/acl-token"]
+                command: ["/bin/sh", "-ec", "/consul-bin/consul services deregister -id=\"{{ .Values.meshGateway.consulServiceName }}\"", "&&", "/bin/sh", "-ec", "/consul-bin/consul logout"]
 
         # consul-sidecar ensures the mesh gateway is always registered with
         # the local Consul agent, even if it loses the initial registration.
@@ -393,7 +399,7 @@ spec:
             - -service-config=/consul/service/service.hcl
             - -consul-binary=/consul-bin/consul
             {{- if .Values.global.acls.manageSystemACLs }}
-            - -token-file=/consul/login/acl-token
+            - -token-file=/consul/service/acl-token
             {{- end }}
       {{- if .Values.meshGateway.priorityClassName }}
       priorityClassName: {{ .Values.meshGateway.priorityClassName | quote }}

diff --git a/charts/consul/test/unit/mesh-gateway-deployment.bats b/charts/consul/test/unit/mesh-gateway-deployment.bats
@@ -595,7 +595,7 @@ key2: value2' \
       --set 'connectInject.enabled=true' \
       --set 'global.acls.manageSystemACLs=true' \
       . | tee /dev/stderr |
-      yq '[.spec.template.spec.containers[0].lifecycle.preStop.exec.command[2]] | any(contains("logout"))' | tee /dev/stderr)
+      yq '[.spec.template.spec.containers[0].lifecycle.preStop.exec.command[6]] | any(contains("logout"))' | tee /dev/stderr)
   [ "${actual}" = "true" ]
 }
 
@@ -641,11 +641,11 @@ key2: value2' \
   [ "${actual}" = "true" ]
 
   local actual=$(echo $object |
-      yq '[.env[1].name] | any(contains("CONSUL_HTTP_ADDR"))' | tee /dev/stderr)
+      yq '[.env[2].name] | any(contains("CONSUL_HTTP_ADDR"))' | tee /dev/stderr)
   [ "${actual}" = "true" ]
 
   local actual=$(echo $object |
-      yq '[.env[1].value] | any(contains("http://$(HOST_IP):8500"))' | tee /dev/stderr)
+      yq '[.env[2].value] | any(contains("http://$(HOST_IP):8500"))' | tee /dev/stderr)
       echo $actual
   [ "${actual}" = "true" ]
 }
@@ -666,20 +666,20 @@ key2: value2' \
   [ "${actual}" = "true" ]
 
   local actual=$(echo $object |
-      yq '[.env[1].name] | any(contains("CONSUL_CACERT"))' | tee /dev/stderr)
+      yq '[.env[2].name] | any(contains("CONSUL_CACERT"))' | tee /dev/stderr)
   [ "${actual}" = "true" ]
 
   local actual=$(echo $object |
-      yq '[.env[2].name] | any(contains("CONSUL_HTTP_ADDR"))' | tee /dev/stderr)
+      yq '[.env[3].name] | any(contains("CONSUL_HTTP_ADDR"))' | tee /dev/stderr)
   [ "${actual}" = "true" ]
 
   local actual=$(echo $object |
-      yq '[.env[2].value] | any(contains("https://$(HOST_IP):8501"))' | tee /dev/stderr)
+      yq '[.env[3].value] | any(contains("https://$(HOST_IP):8501"))' | tee /dev/stderr)
       echo $actual
   [ "${actual}" = "true" ]
 
   local actual=$(echo $object |
-      yq '.volumeMounts[1] | any(contains("consul-ca-cert"))' | tee /dev/stderr)
+      yq '.volumeMounts[2] | any(contains("consul-ca-cert"))' | tee /dev/stderr)
   [ "${actual}" = "true" ]
 }
 
@@ -710,20 +710,20 @@ key2: value2' \
   [ "${actual}" = "true" ]
 
   local actual=$(echo $object |
-      yq '[.env[1].name] | any(contains("CONSUL_CACERT"))' | tee /dev/stderr)
+      yq '[.env[2].name] | any(contains("CONSUL_CACERT"))' | tee /dev/stderr)
   [ "${actual}" = "true" ]
 
   local actual=$(echo $object |
-      yq '[.env[2].name] | any(contains("CONSUL_HTTP_ADDR"))' | tee /dev/stderr)
+      yq '[.env[3].name] | any(contains("CONSUL_HTTP_ADDR"))' | tee /dev/stderr)
   [ "${actual}" = "true" ]
 
   local actual=$(echo $object |
-      yq '[.env[2].value] | any(contains("https://$(HOST_IP):8501"))' | tee /dev/stderr)
+      yq '[.env[3].value] | any(contains("https://$(HOST_IP):8501"))' | tee /dev/stderr)
       echo $actual
   [ "${actual}" = "true" ]
 
   local actual=$(echo $object |
-      yq '.volumeMounts[1] | any(contains("consul-ca-cert"))' | tee /dev/stderr)
+      yq '.volumeMounts[2] | any(contains("consul-ca-cert"))' | tee /dev/stderr)
   [ "${actual}" = "true" ]
 }
 
@@ -744,20 +744,20 @@ key2: value2' \
   [ "${actual}" = "true" ]
 
   local actual=$(echo $object |
-      yq '[.env[1].name] | any(contains("CONSUL_CACERT"))' | tee /dev/stderr)
+      yq '[.env[2].name] | any(contains("CONSUL_CACERT"))' | tee /dev/stderr)
   [ "${actual}" = "true" ]
 
   local actual=$(echo $object |
-      yq '[.env[2].name] | any(contains("CONSUL_HTTP_ADDR"))' | tee /dev/stderr)
+      yq '[.env[3].name] | any(contains("CONSUL_HTTP_ADDR"))' | tee /dev/stderr)
   [ "${actual}" = "true" ]
 
   local actual=$(echo $object |
-      yq '[.env[2].value] | any(contains("https://$(HOST_IP):8501"))' | tee /dev/stderr)
+      yq '[.env[3].value] | any(contains("https://$(HOST_IP):8501"))' | tee /dev/stderr)
       echo $actual
   [ "${actual}" = "true" ]
 
   local actual=$(echo $object |
-      yq '.volumeMounts[1] | any(contains("consul-auto-encrypt-ca-cert"))' | tee /dev/stderr)
+      yq '.volumeMounts[2] | any(contains("consul-auto-encrypt-ca-cert"))' | tee /dev/stderr)
   [ "${actual}" = "true" ]
 }
 
@@ -1110,6 +1110,7 @@ EOF
 
   exp='consul-k8s-control-plane acl-init \
   -component-name=mesh-gateway \
+  -token-sink-file=/consul/service/acl-token \
   -acl-auth-method=RELEASE-NAME-consul-k8s-component-auth-method \
   -log-level=info \
   -log-json=false
@@ -1151,7 +1152,7 @@ service {
 EOF
 
 /consul-bin/consul services register \
-  -token-file=/consul/login/acl-token \
+  -token-file=/consul/service/acl-token \
   /consul/service/service.hcl'
 
   [ "${actual}" = "${exp}" ]