Removing the gateway type suffix from the naming conventions for term…

…inating and ingress gateways (#1120) * Enable ACL Client Token (#1093) * Refactor ConsulLogin() to return the acltoken in addition to theerror. * Refactor createACLPolicyRoleAndBindingRule toappend datacenters for local tokens. Refactor updateOrCreateBindingRule to create binding rule if there are binding rules but this one does not exist * Rename -create-client-token flag to -client * set additional sans for consul server load balancer so that client will be able to use the certificate to talk to the load balancers rather than just an individual server. * Refactor server-acl-init command to create ACL Policy and Rule for client component so that client can call ConsulLogin and receive and ACL Token Call. * Enable client to talk to Consul Server to perform consul login. * Pass Auth Method to k8s al-init command. * Configure Consul address to be the Consul Server Load Balancer. * Configure CA Cert volume to be in memory rather than k8s secret when using vault. * Set consul/login volume and CONSUL_HTTP_TOKEN_FILE for use during logout. * Setup prestop command to perform consul logout. * Configure client-daemonset so that we can utilize the externalServers setting to configure clients to be able to call consul login on a server that is on a different partition. * Configuring partition-init to remove additional flags and use ones that already exist * adding missing comma * fix flakey tests by wrapping asserts in retries a la Iryna * Adding -use-https flag to client-daemonset.yaml when externalServers are enabled * Refactoring tests to cover client-acl-init changes * addressing PR comments * removing mounted tmpfs for consul-ca-cert when using vault and restoring datacenter logic because of breaking test. * addressing PR comments and only appending datacenters to a policy when its a local token, not global tokens. * completing additional dns names based on PR feedback * Do not ca-cert volume when using vault. * removing unused flagConsulCACert from partition-init command * PR Feedback. Removing unused envvars in acl-init container. changing ConsulLogin to return secretID, error instead ok token, error. * Enable terminating gateway policy to be generated via Auth Method * Filtering out failing portion of test for terminating gateway work * PR feedback.Fixing tests. Changing naming conventions for policy and roles for terminating gateways. * Correcting serviceAccount used on deployment * Making all nameshavea-ingress-gateway * Enable ingress gateway policy to be generated via Auth Method * Making all names have a -ingress-gateway suffix * Removing duplicate test * Update acceptance/tests/ingress-gateway/ingress_gateway_namespaces_test.go Co-authored-by: Nitya Dhanushkodi <[email protected]> * Removing the gateway type suffix from the naming conventions for terminating and ingress gateways * Adding check for duplicate terminating gateways and ingress gateway names * Update charts/consul/templates/ingress-gateways-deployment.yaml Co-authored-by: Luke Kysow <[email protected]> * PR Feedback - adding the duplicate name found to the check failures for duplicate ingress or terminating gateway names * Fixing rebase conflict * Merge Conflict- duplicate test * Adding a 10s sleep to flakey snapshot agent tests that were not finding a snapshot in time. Co-authored-by: Nitya Dhanushkodi <[email protected]> Co-authored-by: Luke Kysow <[email protected]>
hashicorp · Mar 30, 2022 · df6a8db · df6a8db
1 parent 8eb9902
commit df6a8db
Show file tree

Hide file tree

Showing 29 changed files with 240 additions and 144 deletions.
diff --git a/acceptance/tests/ingress-gateway/ingress_gateway_namespaces_test.go b/acceptance/tests/ingress-gateway/ingress_gateway_namespaces_test.go
@@ -121,7 +121,7 @@ func TestIngressGatewaySingleNamespace(t *testing.T) {
 			require.NoError(t, err)
 			require.Equal(t, true, created, "config entry failed")
 
-			ingressGatewayService := fmt.Sprintf("http://%s-consul-%s-ingress-gateway.%s:8080/", releaseName, igName, ctx.KubectlOptions(t).Namespace)
+			ingressGatewayService := fmt.Sprintf("http://%s-consul-%s.%s:8080/", releaseName, igName, ctx.KubectlOptions(t).Namespace)
 
 			// If ACLs are enabled, test that intentions prevent connections.
 			if c.secure {
@@ -247,7 +247,7 @@ func TestIngressGatewayNamespaceMirroring(t *testing.T) {
 			require.NoError(t, err)
 			require.Equal(t, true, created, "config entry failed")
 
-			ingressGatewayService := fmt.Sprintf("http://%s-consul-%s-ingress-gateway.%s:8080/", releaseName, igName, ctx.KubectlOptions(t).Namespace)
+			ingressGatewayService := fmt.Sprintf("http://%s-consul-%s.%s:8080/", releaseName, igName, ctx.KubectlOptions(t).Namespace)
 
 			// If ACLs are enabled, test that intentions prevent connections.
 			if c.secure {

diff --git a/acceptance/tests/ingress-gateway/ingress_gateway_test.go b/acceptance/tests/ingress-gateway/ingress_gateway_test.go
@@ -97,7 +97,7 @@ func TestIngressGateway(t *testing.T) {
 				logger.Log(t, "testing intentions prevent ingress")
 				k8s.CheckStaticServerConnectionFailing(t, k8sOptions,
 					staticClientName, "-H", "Host: static-server.ingress.consul",
-					fmt.Sprintf("http://%s-consul-%s-ingress-gateway:8080/", releaseName, igName))
+					fmt.Sprintf("http://%s-consul-%s:8080/", releaseName, igName))
 
 				// Now we create the allow intention.
 				logger.Log(t, "creating ingress-gateway => static-server intention")
@@ -119,7 +119,7 @@ func TestIngressGateway(t *testing.T) {
 			logger.Log(t, "trying calls to ingress gateway")
 			k8s.CheckStaticServerConnectionSuccessful(t, k8sOptions,
 				staticClientName, "-H", "Host: static-server.ingress.consul",
-				fmt.Sprintf("http://%s-consul-%s-ingress-gateway:8080/", releaseName, igName))
+				fmt.Sprintf("http://%s-consul-%s:8080/", releaseName, igName))
 		})
 	}
 }
diff --git a/acceptance/tests/snapshot-agent/snapshot_agent_k8s_secret_test.go b/acceptance/tests/snapshot-agent/snapshot_agent_k8s_secret_test.go
@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"strings"
 	"testing"
+	"time"
 
 	terratestLogger "github.com/gruntwork-io/terratest/modules/logger"
 	"github.com/hashicorp/consul-k8s/acceptance/framework/consul"
@@ -80,6 +81,9 @@ func TestSnapshotAgent_K8sSecret(t *testing.T) {
 	require.NoError(t, err)
 	require.True(t, len(podList.Items) > 0)
 
+	// Wait for 10seconds to allow snapsot to write.
+	time.Sleep(10 * time.Second)
+
 	// Loop through snapshot agents.  Only one will be the leader and have the snapshot files.
 	hasSnapshots := false
 	for _, pod := range podList.Items {

diff --git a/acceptance/tests/snapshot-agent/snapshot_agent_vault_test.go b/acceptance/tests/snapshot-agent/snapshot_agent_vault_test.go
@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"strings"
 	"testing"
+	"time"
 
 	terratestLogger "github.com/gruntwork-io/terratest/modules/logger"
 	"github.com/hashicorp/consul-k8s/acceptance/framework/consul"
@@ -119,6 +120,9 @@ func TestSnapshotAgent_Vault(t *testing.T) {
 	require.NoError(t, err)
 	require.True(t, len(podList.Items) > 0)
 
+	// Wait for 10seconds to allow snapsot to write.
+	time.Sleep(10 * time.Second)
+
 	// Loop through snapshot agents.  Only one will be the leader and have the snapshot files.
 	hasSnapshots := false
 	for _, pod := range podList.Items {

diff --git a/charts/consul/templates/ingress-gateways-deployment.yaml b/charts/consul/templates/ingress-gateways-deployment.yaml
@@ -9,6 +9,17 @@
 {{- $defaults := .Values.ingressGateways.defaults }}
 {{- $names := dict }}
 
+{{- /* Check if gateway names are unique. */ -}}
+{{- $gateways := .Values.ingressGateways.gateways }}
+{{- range $outerIngressIndex, $outerIngressVal := $gateways }}
+
+{{- range $innerIngressIndex, $innerIngressVal := $gateways }}
+{{- if (and (ne $outerIngressIndex $innerIngressIndex) (eq $outerIngressVal.name $innerIngressVal.name)) }}
+{{ fail (cat "ingress gateways must have unique names but found duplicate name" $innerIngressVal.name) }}
+{{ end -}}
+{{ end -}}
+{{ end -}}
+
 {{- range .Values.ingressGateways.gateways }}
 
 {{- $service := .service }}
@@ -26,15 +37,15 @@
 apiVersion: apps/v1
 kind: Deployment
 metadata:
-  name: {{ template "consul.fullname" $root }}-{{ .name }}-ingress-gateway
+  name: {{ template "consul.fullname" $root }}-{{ .name }}
   namespace: {{ $root.Release.Namespace }}
   labels:
     app: {{ template "consul.name" $root }}
     chart: {{ template "consul.chart" $root }}
     heritage: {{ $root.Release.Service }}
     release: {{ $root.Release.Name }}
     component: ingress-gateway
-    ingress-gateway-name: {{ template "consul.fullname" $root }}-{{ .name }}-ingress-gateway
+    ingress-gateway-name: {{ template "consul.fullname" $root }}-{{ .name }}
 spec:
   replicas: {{ default $defaults.replicas .replicas }}
   selector:
@@ -44,7 +55,7 @@ spec:
       heritage: {{ $root.Release.Service }}
       release: {{ $root.Release.Name }}
       component: ingress-gateway
-      ingress-gateway-name: {{ template "consul.fullname" $root }}-{{ .name }}-ingress-gateway
+      ingress-gateway-name: {{ template "consul.fullname" $root }}-{{ .name }}
   template:
     metadata:
       labels:
@@ -53,7 +64,7 @@ spec:
         heritage: {{ $root.Release.Service }}
         release: {{ $root.Release.Name }}
         component: ingress-gateway
-        ingress-gateway-name: {{ template "consul.fullname" $root }}-{{ .name }}-ingress-gateway
+        ingress-gateway-name: {{ template "consul.fullname" $root }}-{{ .name }}
       annotations:
         {{- if (and $root.Values.global.secretsBackend.vault.enabled $root.Values.global.tls.enabled) }}
         "vault.hashicorp.com/agent-init-first": "true"
@@ -92,7 +103,7 @@ spec:
         {{ tpl (default $defaults.tolerations .tolerations) $root | nindent 8 | trim }}
       {{- end }}
       terminationGracePeriodSeconds: {{ default $defaults.terminationGracePeriodSeconds .terminationGracePeriodSeconds }}
-      serviceAccountName: {{ template "consul.fullname" $root }}-{{ .name }}-ingress-gateway
+      serviceAccountName: {{ template "consul.fullname" $root }}-{{ .name }}
       volumes:
         - name: consul-bin
           emptyDir: {}
@@ -189,7 +200,7 @@ spec:
                   -log-level={{ $root.Values.global.logLevel }} \
                   -log-json={{ $root.Values.global.logJSON }} \
                   -k8s-namespace={{ $root.Release.Namespace }} \
-                  -name={{ template "consul.fullname" $root }}-{{ .name }}-ingress-gateway \
+                  -name={{ template "consul.fullname" $root }}-{{ .name }} \
                   -output-file=/tmp/address.txt
                 WAN_ADDR="$(cat /tmp/address.txt)"
                 {{- else }}

diff --git a/charts/consul/templates/ingress-gateways-podsecuritypolicy.yaml b/charts/consul/templates/ingress-gateways-podsecuritypolicy.yaml
@@ -4,15 +4,15 @@
 apiVersion: policy/v1beta1
 kind: PodSecurityPolicy
 metadata:
-  name: {{ template "consul.fullname" $root }}-{{ .name }}-ingress-gateway
+  name: {{ template "consul.fullname" $root }}-{{ .name }}
   namespace: {{ $root.Release.Namespace }}
   labels:
     app: {{ template "consul.name" $root }}
     chart: {{ template "consul.chart" $root }}
     heritage: {{ $root.Release.Service }}
     release: {{ $root.Release.Name }}
     component: ingress-gateway
-    ingress-gateway-name: {{ template "consul.fullname" $root }}-{{ .name }}-ingress-gateway
+    ingress-gateway-name: {{ template "consul.fullname" $root }}-{{ .name }}
 spec:
   privileged: false
   # Required to prevent escalations to root.

diff --git a/charts/consul/templates/ingress-gateways-role.yaml b/charts/consul/templates/ingress-gateways-role.yaml
@@ -7,28 +7,28 @@
 apiVersion: rbac.authorization.k8s.io/v1
 kind: Role
 metadata:
-  name: {{ template "consul.fullname" $root }}-{{ .name }}-ingress-gateway
+  name: {{ template "consul.fullname" $root }}-{{ .name }}
   namespace: {{ $root.Release.Namespace }}
   labels:
     app: {{ template "consul.name" $root }}
     chart: {{ template "consul.chart" $root }}
     heritage: {{ $root.Release.Service }}
     release: {{ $root.Release.Name }}
     component: ingress-gateway
-    ingress-gateway-name: {{ template "consul.fullname" $root }}-{{ .name }}-ingress-gateway
+    ingress-gateway-name: {{ template "consul.fullname" $root }}-{{ .name }}
 rules:
   - apiGroups: [""]
     resources:
       - services
     resourceNames:
-      - {{ template "consul.fullname" $root }}-{{ .name }}-ingress-gateway
+      - {{ template "consul.fullname" $root }}-{{ .name }}
     verbs:
       - get
 {{- if $root.Values.global.enablePodSecurityPolicies }}
   - apiGroups: ["policy"]
     resources: ["podsecuritypolicies"]
     resourceNames:
-      - {{ template "consul.fullname" $root }}-{{ .name }}-ingress-gateway
+      - {{ template "consul.fullname" $root }}-{{ .name }}
     verbs:
       - use
 {{- end }}
@@ -37,7 +37,7 @@ rules:
     resources:
       - secrets
     resourceNames:
-      - {{ template "consul.fullname" $root }}-{{ .name }}-ingress-gateway-acl-token
+      - {{ template "consul.fullname" $root }}-{{ .name }}-acl-token
     verbs:
       - get
 {{- end }}

diff --git a/charts/consul/templates/ingress-gateways-rolebinding.yaml b/charts/consul/templates/ingress-gateways-rolebinding.yaml
@@ -4,22 +4,22 @@
 apiVersion: rbac.authorization.k8s.io/v1
 kind: RoleBinding
 metadata:
-  name: {{ template "consul.fullname" $root }}-{{ .name }}-ingress-gateway
+  name: {{ template "consul.fullname" $root }}-{{ .name }}
   namespace: {{ $root.Release.Namespace }}
   labels:
     app: {{ template "consul.name" $root }}
     chart: {{ template "consul.chart" $root }}
     heritage: {{ $root.Release.Service }}
     release: {{ $root.Release.Name }}
     component: ingress-gateway
-    ingress-gateway-name: {{ template "consul.fullname" $root }}-{{ .name }}-ingress-gateway
+    ingress-gateway-name: {{ template "consul.fullname" $root }}-{{ .name }}
 roleRef:
   apiGroup: rbac.authorization.k8s.io
   kind: Role
-  name: {{ template "consul.fullname" $root }}-{{ .name }}-ingress-gateway
+  name: {{ template "consul.fullname" $root }}-{{ .name }}
 subjects:
   - kind: ServiceAccount
-    name: {{ template "consul.fullname" $root }}-{{ .name }}-ingress-gateway
+    name: {{ template "consul.fullname" $root }}-{{ .name }}
 ---
 {{- end }}
 {{- end }}
diff --git a/charts/consul/templates/ingress-gateways-service.yaml b/charts/consul/templates/ingress-gateways-service.yaml
@@ -9,15 +9,15 @@
 apiVersion: v1
 kind: Service
 metadata:
-  name: {{ template "consul.fullname" $root }}-{{ .name }}-ingress-gateway
+  name: {{ template "consul.fullname" $root }}-{{ .name }}
   namespace: {{ $root.Release.Namespace }}
   labels:
     app: {{ template "consul.name" $root }}
     chart: {{ template "consul.chart" $root }}
     heritage: {{ $root.Release.Service }}
     release: {{ $root.Release.Name }}
     component: ingress-gateway
-    ingress-gateway-name: {{ template "consul.fullname" $root }}-{{ .name }}-ingress-gateway
+    ingress-gateway-name: {{ template "consul.fullname" $root }}-{{ .name }}
   {{- if (or $defaults.service.annotations $service.annotations) }}
   # We allow both default annotations and gateway-specific annotations
   annotations:
@@ -33,7 +33,7 @@ spec:
     app: {{ template "consul.name" $root }}
     release: "{{ $root.Release.Name }}"
     component: ingress-gateway
-    ingress-gateway-name: {{ template "consul.fullname" $root }}-{{ .name }}-ingress-gateway
+    ingress-gateway-name: {{ template "consul.fullname" $root }}-{{ .name }}
   ports:
     {{- range $index, $ports := (default $defaults.service.ports $service.ports) }}
     - name: gateway-{{ $index }}

diff --git a/charts/consul/templates/ingress-gateways-serviceaccount.yaml b/charts/consul/templates/ingress-gateways-serviceaccount.yaml
@@ -6,15 +6,15 @@
 apiVersion: v1
 kind: ServiceAccount
 metadata:
-  name: {{ template "consul.fullname" $root }}-{{ .name }}-ingress-gateway
+  name: {{ template "consul.fullname" $root }}-{{ .name }}
   namespace: {{ $root.Release.Namespace }}
   labels:
     app: {{ template "consul.name" $root }}
     chart: {{ template "consul.chart" $root }}
     heritage: {{ $root.Release.Service }}
     release: {{ $root.Release.Name }}
     component: ingress-gateway
-    ingress-gateway-name: {{ template "consul.fullname" $root }}-{{ .name }}-ingress-gateway
+    ingress-gateway-name: {{ template "consul.fullname" $root }}-{{ .name }}
   {{- if (or $defaults.serviceAccount.annotations $serviceAccount.annotations) }}
   annotations:
     {{- if $defaults.serviceAccount.annotations }}

diff --git a/charts/consul/templates/terminating-gateways-deployment.yaml b/charts/consul/templates/terminating-gateways-deployment.yaml
@@ -9,6 +9,22 @@
 {{- $defaults := .Values.terminatingGateways.defaults }}
 {{- $names := dict }}
 
+{{- $gateways := .Values.terminatingGateways.gateways }}
+{{- range $outerTerminatingIndex, $outerTerminatingVal := $gateways }}
+
+{{- range $innerTerminatingIndex, $innerTerminatingVal := $gateways }}
+{{- if (and (ne $outerTerminatingIndex $innerTerminatingIndex) (eq $outerTerminatingVal.name $innerTerminatingVal.name)) }}
+{{ fail (cat "terminating gateways must have unique names but found duplicate name" $innerTerminatingVal.name) }}
+{{ end -}}
+{{ end -}}
+
+{{- range $outerIngressIndex, $outerIngressVal := $root.Values.ingressGateways.gateways }}
+{{- if (eq $outerTerminatingVal.name $outerIngressVal.name) }}
+{{ fail (cat "terminating gateways cannot have duplicate names of any ingress gateways but found duplicate name" $outerTerminatingVal.name) }}
+{{ end -}}
+{{ end -}}
+{{ end -}}
+
 {{- range .Values.terminatingGateways.gateways }}
 
 {{- if empty .name }}
@@ -24,15 +40,15 @@
 apiVersion: apps/v1
 kind: Deployment
 metadata:
-  name: {{ template "consul.fullname" $root }}-{{ .name }}-terminating-gateway
+  name: {{ template "consul.fullname" $root }}-{{ .name }}
   namespace: {{ $root.Release.Namespace }}
   labels:
     app: {{ template "consul.name" $root }}
     chart: {{ template "consul.chart" $root }}
     heritage: {{ $root.Release.Service }}
     release: {{ $root.Release.Name }}
     component: terminating-gateway
-    terminating-gateway-name: {{ template "consul.fullname" $root }}-{{ .name }}-terminating-gateway
+    terminating-gateway-name: {{ template "consul.fullname" $root }}-{{ .name }}
 spec:
   replicas: {{ default $defaults.replicas .replicas }}
   selector:
@@ -42,7 +58,7 @@ spec:
       heritage: {{ $root.Release.Service }}
       release: {{ $root.Release.Name }}
       component: terminating-gateway
-      terminating-gateway-name: {{ template "consul.fullname" $root }}-{{ .name }}-terminating-gateway
+      terminating-gateway-name: {{ template "consul.fullname" $root }}-{{ .name }}
   template:
     metadata:
       labels:
@@ -51,7 +67,7 @@ spec:
         heritage: {{ $root.Release.Service }}
         release: {{ $root.Release.Name }}
         component: terminating-gateway
-        terminating-gateway-name: {{ template "consul.fullname" $root }}-{{ .name }}-terminating-gateway
+        terminating-gateway-name: {{ template "consul.fullname" $root }}-{{ .name }}
       annotations:
         {{- if (and $root.Values.global.secretsBackend.vault.enabled $root.Values.global.tls.enabled) }}
         "vault.hashicorp.com/agent-init-first": "true"
@@ -90,7 +106,7 @@ spec:
         {{ tpl (default $defaults.tolerations .tolerations) $root | nindent 8 | trim }}
       {{- end }}
       terminationGracePeriodSeconds: 10
-      serviceAccountName: {{ template "consul.fullname" $root }}-{{ .name }}-terminating-gateway
+      serviceAccountName: {{ template "consul.fullname" $root }}-{{ .name }}
       volumes:
         - name: consul-bin
           emptyDir: {}

diff --git a/charts/consul/templates/terminating-gateways-podsecuritypolicy.yaml b/charts/consul/templates/terminating-gateways-podsecuritypolicy.yaml
@@ -4,15 +4,15 @@
 apiVersion: policy/v1beta1
 kind: PodSecurityPolicy
 metadata:
-  name:  {{ template "consul.fullname" $root }}-{{ .name }}-terminating-gateway
+  name:  {{ template "consul.fullname" $root }}-{{ .name }}
   namespace: {{ $root.Release.Namespace }}
   labels:
     app: {{ template "consul.name" $root }}
     chart: {{ template "consul.chart" $root }}
     heritage: {{ $root.Release.Service }}
     release: {{ $root.Release.Name }}
     component: terminating-gateway
-    terminating-gateway-name: {{ template "consul.fullname" $root }}-{{ .name }}-terminating-gateway
+    terminating-gateway-name: {{ template "consul.fullname" $root }}-{{ .name }}
 spec:
   privileged: false
   # Required to prevent escalations to root.

diff --git a/charts/consul/templates/terminating-gateways-role.yaml b/charts/consul/templates/terminating-gateways-role.yaml
@@ -7,22 +7,22 @@
 apiVersion: rbac.authorization.k8s.io/v1
 kind: Role
 metadata:
-  name:  {{ template "consul.fullname" $root }}-{{ .name }}-terminating-gateway
+  name:  {{ template "consul.fullname" $root }}-{{ .name }}
   namespace: {{ $root.Release.Namespace }}
   labels:
     app: {{ template "consul.name" $root }}
     chart: {{ template "consul.chart" $root }}
     heritage: {{ $root.Release.Service }}
     release: {{ $root.Release.Name }}
     component: terminating-gateway
-    terminating-gateway-name: {{ template "consul.fullname" $root }}-{{ .name }}-terminating-gateway
+    terminating-gateway-name: {{ template "consul.fullname" $root }}-{{ .name }}
 {{- if (or $root.Values.global.acls.manageSystemACLs $root.Values.global.enablePodSecurityPolicies) }}
 rules:
 {{- if $root.Values.global.enablePodSecurityPolicies }}
   - apiGroups: ["policy"]
     resources: ["podsecuritypolicies"]
     resourceNames:
-      -  {{ template "consul.fullname" $root }}-{{ .name }}-terminating-gateway
+      -  {{ template "consul.fullname" $root }}-{{ .name }}
     verbs:
       - use
 {{- end }}
@@ -31,7 +31,7 @@ rules:
     resources:
       - secrets
     resourceNames:
-      -  {{ template "consul.fullname" $root }}-{{ .name }}-terminating-gateway-acl-token
+      -  {{ template "consul.fullname" $root }}-{{ .name }}-acl-token
     verbs:
       - get
 {{- end }}