Global auth method (#1075)

• Update server-acl-init to create authmethods in the primary datacenter when the job is run in a secondary datacenter during federation. This authmethod allows us to issue logins for global policies. • Update the controller workflow in server-acl-init to use this global authmethod when run in a secondary DC. • Update the mesh-gateway acceptance tests to create proxy defaults in the secondary DC to test above behavior works successfully. • Updated logout to not pass in the partition flag as it is not required. • Update server acl init tests to migrate from require := require.New(t) to require.xyz(t, ...) patterns.
hashicorp · Mar 29, 2022 · d0f1195 · d0f1195
1 parent bcd5481
commit d0f1195
Show file tree

Hide file tree

Showing 20 changed files with 1,112 additions and 509 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -9,8 +9,8 @@ executors:
       - image: docker.mirror.hashicorp.services/cimg/go:1.17.5
     environment:
       TEST_RESULTS: /tmp/test-results # path to where test results are saved
-      CONSUL_VERSION: 1.11.2 # Consul's OSS version to use in tests
-      CONSUL_ENT_VERSION: 1.11.2+ent # Consul's enterprise version to use in tests
+      CONSUL_VERSION: 1.11.4 # Consul's OSS version to use in tests
+      CONSUL_ENT_VERSION: 1.11.4+ent # Consul's enterprise version to use in tests
 
 control-plane-path: &control-plane-path control-plane
 cli-path: &cli-path cli

diff --git a/acceptance/go.mod b/acceptance/go.mod
@@ -7,6 +7,7 @@ require (
 	github.com/hashicorp/consul-k8s/control-plane v0.0.0-20211207212234-aea9efea5638
 	github.com/hashicorp/consul/api v1.12.0
 	github.com/hashicorp/consul/sdk v0.9.0
+	github.com/hashicorp/go-uuid v1.0.2
 	github.com/hashicorp/vault/api v1.2.0
 	github.com/stretchr/testify v1.7.0
 	gopkg.in/yaml.v2 v2.4.0
@@ -49,7 +50,6 @@ require (
 	github.com/hashicorp/go-secure-stdlib/parseutil v0.1.1 // indirect
 	github.com/hashicorp/go-secure-stdlib/strutil v0.1.1 // indirect
 	github.com/hashicorp/go-sockaddr v1.0.2 // indirect
-	github.com/hashicorp/go-uuid v1.0.2 // indirect
 	github.com/hashicorp/go-version v1.2.0 // indirect
 	github.com/hashicorp/golang-lru v0.5.3 // indirect
 	github.com/hashicorp/hcl v1.0.0 // indirect

diff --git a/acceptance/tests/mesh-gateway/mesh_gateway_test.go b/acceptance/tests/mesh-gateway/mesh_gateway_test.go
@@ -33,8 +33,9 @@ func TestMeshGatewayDefault(t *testing.T) {
 		"global.federation.enabled":                "true",
 		"global.federation.createFederationSecret": "true",
 
-		"connectInject.enabled": "true",
-		"controller.enabled":    "true",
+		"connectInject.enabled":  "true",
+		"connectInject.replicas": "1",
+		"controller.enabled":     "true",
 
 		"meshGateway.enabled":  "true",
 		"meshGateway.replicas": "1",
@@ -79,7 +80,9 @@ func TestMeshGatewayDefault(t *testing.T) {
 		"server.extraVolumes[0].items[0].key":  "serverConfigJSON",
 		"server.extraVolumes[0].items[0].path": "config.json",
 
-		"connectInject.enabled": "true",
+		"connectInject.enabled":  "true",
+		"connectInject.replicas": "1",
+		"controller.enabled":     "true",
 
 		"meshGateway.enabled":  "true",
 		"meshGateway.replicas": "1",
@@ -164,8 +167,9 @@ func TestMeshGatewaySecure(t *testing.T) {
 				"global.federation.enabled":                "true",
 				"global.federation.createFederationSecret": "true",
 
-				"connectInject.enabled": "true",
-				"controller.enabled":    "true",
+				"connectInject.enabled":  "true",
+				"connectInject.replicas": "1",
+				"controller.enabled":     "true",
 
 				"meshGateway.enabled":  "true",
 				"meshGateway.replicas": "1",
@@ -191,6 +195,19 @@ func TestMeshGatewaySecure(t *testing.T) {
 			_, err = secondaryContext.KubernetesClient(t).CoreV1().Secrets(secondaryContext.KubectlOptions(t).Namespace).Create(context.Background(), federationSecret, metav1.CreateOptions{})
 			require.NoError(t, err)
 
+			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 := secondaryContext.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, secondaryContext.KubectlOptions(t))
+			}
+
 			// Create secondary cluster
 			secondaryHelmValues := map[string]string{
 				"global.datacenter": "dc2",
@@ -207,15 +224,19 @@ func TestMeshGatewaySecure(t *testing.T) {
 				"global.acls.replicationToken.secretName": federationSecretName,
 				"global.acls.replicationToken.secretKey":  "replicationToken",
 
-				"global.federation.enabled": "true",
+				"global.federation.enabled":           "true",
+				"global.federation.k8sAuthMethodHost": k8sAuthMethodHost,
+				"global.federation.primaryDatacenter": "dc1",
 
 				"server.extraVolumes[0].type":          "secret",
 				"server.extraVolumes[0].name":          federationSecretName,
 				"server.extraVolumes[0].load":          "true",
 				"server.extraVolumes[0].items[0].key":  "serverConfigJSON",
 				"server.extraVolumes[0].items[0].path": "config.json",
 
-				"connectInject.enabled": "true",
+				"connectInject.enabled":  "true",
+				"connectInject.replicas": "1",
+				"controller.enabled":     "true",
 
 				"meshGateway.enabled":  "true",
 				"meshGateway.replicas": "1",
@@ -248,9 +269,9 @@ func TestMeshGatewaySecure(t *testing.T) {
 			// gateways.
 			logger.Log(t, "creating proxy-defaults config")
 			kustomizeDir := "../fixtures/bases/mesh-gateway"
-			k8s.KubectlApplyK(t, primaryContext.KubectlOptions(t), kustomizeDir)
+			k8s.KubectlApplyK(t, secondaryContext.KubectlOptions(t), kustomizeDir)
 			helpers.Cleanup(t, cfg.NoCleanupOnFailure, func() {
-				k8s.KubectlDeleteK(t, primaryContext.KubectlOptions(t), kustomizeDir)
+				k8s.KubectlDeleteK(t, secondaryContext.KubectlOptions(t), kustomizeDir)
 			})
 
 			// Check that we can connect services over the mesh gateways

diff --git a/charts/consul/templates/controller-clusterrole.yaml b/charts/consul/templates/controller-clusterrole.yaml
@@ -57,15 +57,6 @@ rules:
   - get
   - list
   - update
-{{- if .Values.global.acls.manageSystemACLs }}
-- apiGroups: [""]
-  resources:
-  - secrets
-  resourceNames:
-  - {{ template "consul.fullname" . }}-controller-acl-token
-  verbs:
-  - get
-{{- end }}
 {{- if .Values.global.enablePodSecurityPolicies }}
 - apiGroups: ["policy"]
   resources: ["podsecuritypolicies"]

diff --git a/charts/consul/templates/controller-deployment.yaml b/charts/consul/templates/controller-deployment.yaml
@@ -87,7 +87,12 @@ spec:
           - |
             consul-k8s-control-plane acl-init \
               -component-name=controller \
+              {{- 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 }} \
+              {{- else }}
               -acl-auth-method={{ template "consul.fullname" . }}-k8s-component-auth-method \
+              {{- end }}
               {{- if .Values.global.adminPartitions.enabled }}
               -partition={{ .Values.global.adminPartitions.name }} \
               {{- end }}
@@ -139,10 +144,7 @@ spec:
                 - "/bin/sh"
                 - "-ec"
                 - |
-                  consul-k8s-control-plane consul-logout \
-                  {{- if .Values.global.adminPartitions.enabled }}
-                  -partition={{ .Values.global.adminPartitions.name }} \
-                  {{- end }}
+                  consul-k8s-control-plane consul-logout
         {{- end }}
         env:
         {{- if .Values.global.acls.manageSystemACLs }}

diff --git a/charts/consul/templates/server-acl-init-job.yaml b/charts/consul/templates/server-acl-init-job.yaml
@@ -185,10 +185,14 @@ spec:
                 {{- if (or (and (ne (.Values.connectInject.enabled | toString) "-") .Values.connectInject.enabled) (and (eq (.Values.connectInject.enabled | toString) "-") .Values.global.enabled)) }}
                 -create-inject-token=true \
                 {{- if and .Values.externalServers.enabled .Values.externalServers.k8sAuthMethodHost }}
-                -inject-auth-method-host={{ .Values.externalServers.k8sAuthMethodHost }} \
+                -auth-method-host={{ .Values.externalServers.k8sAuthMethodHost }} \
                 {{- end }}
                 {{- end }}
 
+                {{- if .Values.global.federation.k8sAuthMethodHost }}
+                -auth-method-host={{ .Values.global.federation.k8sAuthMethodHost }} \
+                {{- end }}
+
                 {{- if .Values.meshGateway.enabled }}
                 -create-mesh-gateway-token=true \
                 {{- end }}

diff --git a/charts/consul/test/unit/controller-clusterrole.bats b/charts/consul/test/unit/controller-clusterrole.bats
@@ -43,17 +43,3 @@ load _helpers
       yq '.rules | map(select(.resources[0] == "podsecuritypolicies")) | length' | tee /dev/stderr)
   [ "${actual}" = "1" ]
 }
-
-#--------------------------------------------------------------------
-# global.acls.manageSystemACLs
-
-@test "controller/ClusterRole: allows secret access with global.acls.manageSystemACLs=true" {
-  cd `chart_dir`
-  local actual=$(helm template \
-      -s templates/controller-clusterrole.yaml  \
-      --set 'controller.enabled=true' \
-      --set 'global.acls.manageSystemACLs=true' \
-      . | tee /dev/stderr |
-      yq -r '.rules | map(select(.resourceNames[0] == "RELEASE-NAME-consul-controller-acl-token")) | length' | tee /dev/stderr)
-  [ "${actual}" = "1" ]
-}
diff --git a/charts/consul/test/unit/controller-deployment.bats b/charts/consul/test/unit/controller-deployment.bats
@@ -57,20 +57,6 @@ load _helpers
   [ "${actual}" = "true" ]
 }
 
-@test "controller/Deployment: consul-logout preStop hook has partition when partitions are enabled" {
-  cd `chart_dir`
-  local actual=$(helm template \
-      -s templates/controller-deployment.yaml \
-      --set 'controller.enabled=true' \
-      --set 'global.acls.manageSystemACLs=true' \
-      --set 'global.enableConsulNamespaces=true' \
-      --set 'global.adminPartitions.enabled=true' \
-      --set 'global.adminPartitions.name=default' \
-      . | tee /dev/stderr |
-      yq '[.spec.template.spec.containers[0].lifecycle.preStop.exec.command[2]] | any(contains("-partition=default"))' | tee /dev/stderr)
-  [ "${actual}" = "true" ]
-}
-
 @test "controller/Deployment: CONSUL_HTTP_TOKEN_FILE is not set when acls are disabled" {
   cd `chart_dir`
   local actual=$(helm template \
@@ -243,6 +229,35 @@ load _helpers
   [ "${actual}" = "get-auto-encrypt-client-ca" ]
 }
 
+@test "controller/Deployment: init container is created when global.acls.manageSystemACLs=true and has correct command when federation enabled in non-primary datacenter" {
+  cd `chart_dir`
+  local object=$(helm template \
+      -s templates/controller-deployment.yaml \
+      --set 'controller.enabled=true' \
+      --set 'global.datacenter=dc2' \
+      --set 'global.federation.enabled=true' \
+      --set 'global.federation.primaryDatacenter=dc1' \
+      --set 'meshGateway.enabled=true' \
+      --set 'connectInject.enabled=true' \
+      --set 'global.tls.enabled=true' \
+      --set 'global.tls.enableAutoEncrypt=true' \
+      --set 'global.acls.manageSystemACLs=true' \
+      . | tee /dev/stderr |
+      yq '.spec.template.spec.initContainers[] | select(.name == "controller-acl-init")' | tee /dev/stderr)
+
+  local actual=$(echo $object |
+      yq -r '.command | any(contains("consul-k8s-control-plane acl-init"))' | tee /dev/stderr)
+  [ "${actual}" = "true" ]
+
+  local actual=$(echo $object |
+      yq -r '.command | any(contains("-acl-auth-method=RELEASE-NAME-consul-k8s-component-auth-method-dc2"))' | tee /dev/stderr)
+  [ "${actual}" = "true" ]
+
+  local actual=$(echo $object |
+      yq -r '.command | any(contains("-primary-datacenter=dc1"))' | tee /dev/stderr)
+  [ "${actual}" = "true" ]
+}
+
 #--------------------------------------------------------------------
 # global.tls.enabled
 

diff --git a/charts/consul/test/unit/server-acl-init-job.bats b/charts/consul/test/unit/server-acl-init-job.bats
@@ -1712,7 +1712,7 @@ load _helpers
       --set 'global.acls.manageSystemACLs=true' \
       --set 'connectInject.enabled=true' \
       . | tee /dev/stderr |
-      yq '.spec.template.spec.containers[0].command | any(contains("-inject-auth-method-host"))' | tee /dev/stderr)
+      yq '.spec.template.spec.containers[0].command | any(contains("-auth-method-host"))' | tee /dev/stderr)
   [ "${actual}" = "false" ]
 }
 
@@ -1724,11 +1724,11 @@ load _helpers
       --set 'externalServers.k8sAuthMethodHost=foo.com' \
       --set 'connectInject.enabled=true' \
       . | tee /dev/stderr |
-      yq '.spec.template.spec.containers[0].command | any(contains("-inject-auth-method-host"))' | tee /dev/stderr)
+      yq '.spec.template.spec.containers[0].command | any(contains("-auth-method-host"))' | tee /dev/stderr)
   [ "${actual}" = "false" ]
 }
 
-@test "serverACLInit/Job: can provide custom auth method host" {
+@test "serverACLInit/Job: can provide custom auth method host for external servers" {
   cd `chart_dir`
   local actual=$(helm template \
       -s templates/server-acl-init-job.yaml  \
@@ -1739,7 +1739,24 @@ load _helpers
       --set 'externalServers.hosts[0]=foo.com' \
       --set 'externalServers.k8sAuthMethodHost=foo.com' \
       . | tee /dev/stderr|
-      yq '.spec.template.spec.containers[0].command | any(contains("-inject-auth-method-host=foo.com"))' | tee /dev/stderr)
+      yq '.spec.template.spec.containers[0].command | any(contains("-auth-method-host=foo.com"))' | tee /dev/stderr)
+  [ "${actual}" = "true" ]
+}
+
+@test "serverACLInit/Job: can provide custom auth method host for federation" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -s templates/server-acl-init-job.yaml  \
+      --set 'global.acls.manageSystemACLs=true' \
+      --set 'global.tls.enabled=true' \
+      --set 'global.tls.enableAutoEncrypt=true' \
+      --set 'connectInject.enabled=true' \
+      --set 'global.federation.enabled=true' \
+      --set 'global.federation.primaryDatacenter=dc1' \
+      --set 'global.federation.k8sAuthMethodHost=foo.com' \
+      --set 'meshGateway.enabled=true' \
+      . | tee /dev/stderr|
+      yq '.spec.template.spec.containers[0].command | any(contains("-auth-method-host=foo.com"))' | tee /dev/stderr)
   [ "${actual}" = "true" ]
 }
 

diff --git a/charts/consul/values.yaml b/charts/consul/values.yaml
@@ -85,7 +85,7 @@ global:
   # image: "hashicorp/consul-enterprise:1.10.0-ent"
   # ```
   # @default: hashicorp/consul:<latest version>
-  image: "hashicorp/consul:1.11.3"
+  image: "hashicorp/consul:1.11.4"
 
   # Array of objects containing image pull secret names that will be applied to each service account.
   # This can be used to reference image pull secrets if using a custom consul or consul-k8s-control-plane Docker image.
@@ -431,13 +431,31 @@ global:
     createFederationSecret: false
 
     # The name of the primary datacenter.
-    primaryDatacenter: ""
+    # @type: string
+    primaryDatacenter: null
 
     # A list of addresses of the primary mesh gateways in the form `<ip>:<port>`.
-    # (e.g. ["1.1.1.1:443", "2.3.4.5:443"] 
+    # (e.g. ["1.1.1.1:443", "2.3.4.5:443"]
     # @type: array<string>
     primaryGateways: []
 
+    # If you are setting `global.federation.enabled` to true and are in a secondary datacenter,
+    # set `k8sAuthMethodHost` to the address of the Kubernetes API server of the secondary datacenter.
+    # This address must be reachable from the Consul servers in the primary datacenter.
+    # This authmethod will be used to provision ACL tokens for Consul components and is different
+    # from the one used by the Consul Service Mesh.
+    # Please see the Kubernetes Auth Method documentation (https://consul.io/docs/acl/auth-methods/kubernetes).
+    #
+    # You could retrieve this value from your `kubeconfig` by running:
+    #
+    # ```shell-session
+    # $ kubectl config view \
+    #   -o jsonpath="{.clusters[?(@.name=='<your cluster name>')].cluster.server}"
+    # ```
+    #
+    # @type: string
+    k8sAuthMethodHost: null
+
   # Configures metrics for Consul service mesh
   metrics:
     # Configures the Helm chart’s components

diff --git a/control-plane/helper/test/test_util.go b/control-plane/helper/test/test_util.go
@@ -69,11 +69,11 @@ func SetupK8sComponentAuthMethod(t *testing.T, consulClient *api.Client, service
 	k8sMockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("content-type", "application/json")
 		if r != nil && r.URL.Path == "/apis/authentication.k8s.io/v1/tokenreviews" && r.Method == "POST" {
-			w.Write([]byte(tokenReviewsResponse(serviceAccountName, k8sComponentNS)))
+			w.Write([]byte(TokenReviewsResponse(serviceAccountName, k8sComponentNS)))
 		}
 		if r != nil && r.URL.Path == fmt.Sprintf("/api/v1/namespaces/%s/serviceaccounts/%s", k8sComponentNS, serviceAccountName) &&
 			r.Method == "GET" {
-			w.Write([]byte(serviceAccountGetResponse(serviceAccountName, k8sComponentNS)))
+			w.Write([]byte(ServiceAccountGetResponse(serviceAccountName, k8sComponentNS)))
 		}
 	}))
 	t.Cleanup(k8sMockServer.Close)
@@ -149,11 +149,11 @@ func SetupK8sAuthMethodWithNamespaces(t *testing.T, consulClient *api.Client, se
 	k8sMockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("content-type", "application/json")
 		if r != nil && r.URL.Path == "/apis/authentication.k8s.io/v1/tokenreviews" && r.Method == "POST" {
-			w.Write([]byte(tokenReviewsResponse(serviceName, k8sServiceNS)))
+			w.Write([]byte(TokenReviewsResponse(serviceName, k8sServiceNS)))
 		}
 		if r != nil && r.URL.Path == fmt.Sprintf("/api/v1/namespaces/%s/serviceaccounts/%s", k8sServiceNS, serviceName) &&
 			r.Method == "GET" {
-			w.Write([]byte(serviceAccountGetResponse(serviceName, k8sServiceNS)))
+			w.Write([]byte(ServiceAccountGetResponse(serviceName, k8sServiceNS)))
 		}
 	}))
 	t.Cleanup(k8sMockServer.Close)
@@ -196,7 +196,7 @@ func SetupK8sAuthMethodWithNamespaces(t *testing.T, consulClient *api.Client, se
 	require.NoError(t, err)
 }
 
-func tokenReviewsResponse(name, ns string) string {
+func TokenReviewsResponse(name, ns string) string {
 	return fmt.Sprintf(`{
  "kind": "TokenReview",
  "apiVersion": "authentication.k8s.io/v1",
@@ -221,7 +221,7 @@ func tokenReviewsResponse(name, ns string) string {
 }`, ns, name, ns)
 }
 
-func serviceAccountGetResponse(name, ns string) string {
+func ServiceAccountGetResponse(name, ns string) string {
 	return fmt.Sprintf(`{
  "kind": "ServiceAccount",
  "apiVersion": "v1",