From 507f405b9ef916085042458706fbd671240184be Mon Sep 17 00:00:00 2001 From: Ashwin Venkatesh Date: Tue, 8 Mar 2022 15:14:10 -0500 Subject: [PATCH] Refactor ConnectInject to use authmethods (#1076) Refactor connect-injector to use the new auth-method workflow when ACLs are enabled so that Kubernetes secrets are not used. --- .../templates/connect-inject-clusterrole.yaml | 7 - .../templates/connect-inject-deployment.yaml | 76 ++- .../templates/controller-deployment.yaml | 4 +- .../consul/templates/server-acl-init-job.yaml | 2 +- .../test/unit/connect-inject-clusterrole.bats | 14 - .../test/unit/connect-inject-deployment.bats | 221 ++++++++- .../consul/test/unit/server-acl-init-job.bats | 18 +- .../subcommand/server-acl-init/command.go | 54 +- .../server-acl-init/command_ent_test.go | 246 +--------- .../server-acl-init/command_test.go | 460 +++++++++--------- 10 files changed, 560 insertions(+), 542 deletions(-) diff --git a/charts/consul/templates/connect-inject-clusterrole.yaml b/charts/consul/templates/connect-inject-clusterrole.yaml index ec8b288fda..3a2eb62829 100644 --- a/charts/consul/templates/connect-inject-clusterrole.yaml +++ b/charts/consul/templates/connect-inject-clusterrole.yaml @@ -41,13 +41,6 @@ rules: - use {{- end }} {{- if .Values.global.acls.manageSystemACLs }} -- apiGroups: [""] - resources: - - secrets - resourceNames: - - {{ template "consul.fullname" . }}-connect-inject-acl-token - verbs: - - get - apiGroups: [""] resources: - serviceaccounts diff --git a/charts/consul/templates/connect-inject-deployment.yaml b/charts/consul/templates/connect-inject-deployment.yaml index cd5ad9ddd2..9c79db4235 100644 --- a/charts/consul/templates/connect-inject-deployment.yaml +++ b/charts/consul/templates/connect-inject-deployment.yaml @@ -66,6 +66,10 @@ spec: valueFrom: fieldRef: fieldPath: metadata.namespace + {{- if .Values.global.acls.manageSystemACLs }} + - name: CONSUL_HTTP_TOKEN_FILE + value: "/consul/login/acl-token" + {{- end }} {{- if .Values.global.tls.enabled }} - name: CONSUL_CACERT value: /consul/tls/ca/tls.crt @@ -80,12 +84,6 @@ spec: secretKeyRef: name: {{ .Values.connectInject.aclInjectToken.secretName }} key: {{ .Values.connectInject.aclInjectToken.secretKey }} - {{- else if .Values.global.acls.manageSystemACLs }} - - name: CONSUL_HTTP_TOKEN - valueFrom: - secretKeyRef: - name: "{{ template "consul.fullname" . }}-connect-inject-acl-token" - key: "token" {{- end }} - name: CONSUL_HTTP_ADDR {{- if .Values.global.tls.enabled }} @@ -216,6 +214,16 @@ spec: -default-consul-sidecar-cpu-request={{ $consulSidecarResources.requests.cpu }} \ {{- end }} {{- end }} + {{- if .Values.global.acls.manageSystemACLs }} + lifecycle: + preStop: + exec: + command: + - "/bin/sh" + - "-ec" + - | + consul-k8s-control-plane consul-logout + {{- end }} startupProbe: httpGet: path: /readyz/ready @@ -246,6 +254,9 @@ spec: - name: certs mountPath: /etc/connect-injector/certs readOnly: true + - mountPath: /consul/login + name: consul-data + readOnly: true {{- if .Values.global.tls.enabled }} {{- if .Values.global.tls.enableAutoEncrypt }} - name: consul-auto-encrypt-ca-cert @@ -264,6 +275,9 @@ spec: secret: defaultMode: 420 secretName: {{ template "consul.fullname" . }}-connect-inject-webhook-cert + - name: consul-data + emptyDir: + medium: "Memory" {{- if .Values.global.tls.enabled }} {{- if not (and .Values.externalServers.enabled .Values.externalServers.useSystemRoots) }} - name: consul-ca-cert @@ -285,16 +299,57 @@ spec: {{- end }} {{- if or (and .Values.global.acls.manageSystemACLs) (and .Values.global.tls.enabled .Values.global.tls.enableAutoEncrypt) }} initContainers: + {{- if and .Values.global.tls.enabled .Values.global.tls.enableAutoEncrypt }} + {{- include "consul.getAutoEncryptClientCA" . | nindent 6 }} + {{- end }} {{- if .Values.global.acls.manageSystemACLs }} - - name: injector-acl-init + - name: connect-injector-acl-init + env: + - name: HOST_IP + valueFrom: + fieldRef: + fieldPath: status.hostIP + {{- if .Values.global.tls.enabled }} + - name: CONSUL_CACERT + value: /consul/tls/ca/tls.crt + {{- end }} + - name: CONSUL_HTTP_ADDR + {{- if .Values.global.tls.enabled }} + value: https://$(HOST_IP):8501 + {{- 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" - | consul-k8s-control-plane acl-init \ - -secret-name="{{ template "consul.fullname" . }}-connect-inject-acl-token" \ - -k8s-namespace={{ .Release.Namespace }} + -component-name=connect-injector \ + {{- if and .Values.global.federation.enabled .Values.global.federation.primaryDatacenter .Values.global.enableConsulNamespaces }} + -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 }} + -log-level={{ default .Values.global.logLevel .Values.controller.logLevel }} \ + -log-json={{ .Values.global.logJSON }} resources: requests: memory: "25Mi" @@ -303,9 +358,6 @@ spec: memory: "25Mi" cpu: "50m" {{- end }} - {{- if (and .Values.global.tls.enabled .Values.global.tls.enableAutoEncrypt) }} - {{- include "consul.getAutoEncryptClientCA" . | nindent 6 }} - {{- end }} {{- end }} {{- if .Values.connectInject.priorityClassName }} priorityClassName: {{ .Values.connectInject.priorityClassName | quote }} diff --git a/charts/consul/templates/controller-deployment.yaml b/charts/consul/templates/controller-deployment.yaml index 5c6f65529b..29e5aa5bda 100644 --- a/charts/consul/templates/controller-deployment.yaml +++ b/charts/consul/templates/controller-deployment.yaml @@ -47,7 +47,7 @@ spec: spec: {{- if or .Values.global.acls.manageSystemACLs (and .Values.global.tls.enabled .Values.global.tls.enableAutoEncrypt) }} initContainers: - {{- if (and .Values.global.tls.enabled .Values.global.tls.enableAutoEncrypt) }} + {{- if and .Values.global.tls.enabled .Values.global.tls.enableAutoEncrypt }} {{- include "consul.getAutoEncryptClientCA" . | nindent 6 }} {{- end }} {{- if .Values.global.acls.manageSystemACLs }} @@ -97,7 +97,7 @@ spec: -partition={{ .Values.global.adminPartitions.name }} \ {{- end }} -log-level={{ default .Values.global.logLevel .Values.controller.logLevel }} \ - -log-json={{ .Values.global.logJSON }} \ + -log-json={{ .Values.global.logJSON }} resources: requests: memory: "25Mi" diff --git a/charts/consul/templates/server-acl-init-job.yaml b/charts/consul/templates/server-acl-init-job.yaml index a6c26a0656..97916c495c 100644 --- a/charts/consul/templates/server-acl-init-job.yaml +++ b/charts/consul/templates/server-acl-init-job.yaml @@ -183,7 +183,7 @@ spec: {{- end }} {{- if (or (and (ne (.Values.connectInject.enabled | toString) "-") .Values.connectInject.enabled) (and (eq (.Values.connectInject.enabled | toString) "-") .Values.global.enabled)) }} - -create-inject-token=true \ + -connect-inject=true \ {{- if and .Values.externalServers.enabled .Values.externalServers.k8sAuthMethodHost }} -auth-method-host={{ .Values.externalServers.k8sAuthMethodHost }} \ {{- end }} diff --git a/charts/consul/test/unit/connect-inject-clusterrole.bats b/charts/consul/test/unit/connect-inject-clusterrole.bats index b2f1a35906..c7ff654cf2 100644 --- a/charts/consul/test/unit/connect-inject-clusterrole.bats +++ b/charts/consul/test/unit/connect-inject-clusterrole.bats @@ -26,17 +26,3 @@ load _helpers yq -r '.rules | map(select(.resources[0] == "podsecuritypolicies")) | length' | tee /dev/stderr) [ "${actual}" = "1" ] } - -#-------------------------------------------------------------------- -# global.acls.manageSystemACLs - -@test "connectInject/ClusterRole: secret access with global.acls.manageSystemACLs=true" { - cd `chart_dir` - local actual=$(helm template \ - -s templates/connect-inject-clusterrole.yaml \ - --set 'connectInject.enabled=true' \ - --set 'global.acls.manageSystemACLs=true' \ - . | tee /dev/stderr | - yq -r '.rules | map(select(.resources[0] == "secrets")) | length' | tee /dev/stderr) - [ "${actual}" = "1" ] -} diff --git a/charts/consul/test/unit/connect-inject-deployment.bats b/charts/consul/test/unit/connect-inject-deployment.bats index a3da403005..9ea21184ec 100755 --- a/charts/consul/test/unit/connect-inject-deployment.bats +++ b/charts/consul/test/unit/connect-inject-deployment.bats @@ -926,40 +926,188 @@ EOF #-------------------------------------------------------------------- # global.acls.manageSystemACLs -@test "connectInject/Deployment: CONSUL_HTTP_TOKEN env variable created when global.acls.manageSystemACLs=true" { +@test "connectInject/Deployment: consul-logout preStop hook is added when ACLs are enabled" { cd `chart_dir` local object=$(helm template \ -s templates/connect-inject-deployment.yaml \ --set 'connectInject.enabled=true' \ --set 'global.acls.manageSystemACLs=true' \ . | tee /dev/stderr | - yq '[.spec.template.spec.containers[0].env[].name] ' | tee /dev/stderr) + yq '[.spec.template.spec.containers[0].lifecycle.preStop.exec.command[2]] | any(contains("consul-k8s-control-plane consul-logout"))' | tee /dev/stderr) + + [ "${object}" = "true" ] +} + +@test "connectInject/Deployment: CONSUL_HTTP_TOKEN_FILE is not set when acls are disabled" { + cd `chart_dir` + local actual=$(helm template \ + -s templates/connect-inject-deployment.yaml \ + --set 'connectInject.enabled=true' \ + . | tee /dev/stderr | + yq '[.spec.template.spec.containers[0].env[0].name] | any(contains("CONSUL_HTTP_TOKEN_FILE"))' | tee /dev/stderr) + [ "${actual}" = "false" ] +} + +@test "connectInject/Deployment: CONSUL_HTTP_TOKEN_FILE is set when acls are enabled" { + cd `chart_dir` + local actual=$(helm template \ + -s templates/connect-inject-deployment.yaml \ + --set 'connectInject.enabled=true' \ + --set 'global.acls.manageSystemACLs=true' \ + . | tee /dev/stderr | + yq '[.spec.template.spec.containers[0].env[1].name] | any(contains("CONSUL_HTTP_TOKEN_FILE"))' | tee /dev/stderr) + [ "${actual}" = "true" ] +} + +@test "connectInject/Deployment: init container is created when global.acls.manageSystemACLs=true and has correct command and environment with tls disabled" { + cd `chart_dir` + local object=$(helm template \ + -s templates/connect-inject-deployment.yaml \ + --set 'connectInject.enabled=true' \ + --set 'global.acls.manageSystemACLs=true' \ + . | tee /dev/stderr | + yq '.spec.template.spec.initContainers[0]' | tee /dev/stderr) local actual=$(echo $object | - yq 'any(contains("CONSUL_HTTP_TOKEN"))' | tee /dev/stderr) + yq -r '.name' | tee /dev/stderr) + [ "${actual}" = "connect-injector-acl-init" ] + + 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 'map(select(test("CONSUL_HTTP_TOKEN"))) | length' | tee /dev/stderr) - [ "${actual}" = "1" ] + yq '[.env[1].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) + echo $actual + [ "${actual}" = "true" ] } -@test "connectInject/Deployment: init container is created when global.acls.manageSystemACLs=true" { +@test "connectInject/Deployment: init container is created when global.acls.manageSystemACLs=true and has correct command and environment with tls enabled" { cd `chart_dir` local object=$(helm template \ -s templates/connect-inject-deployment.yaml \ --set 'connectInject.enabled=true' \ + --set 'global.tls.enabled=true' \ --set 'global.acls.manageSystemACLs=true' \ . | tee /dev/stderr | - yq '.spec.template.spec.initContainers[0]' | tee /dev/stderr) + yq '.spec.template.spec.initContainers[] | select(.name == "connect-injector-acl-init")' | tee /dev/stderr) local actual=$(echo $object | - yq -r '.name' | tee /dev/stderr) - [ "${actual}" = "injector-acl-init" ] + yq -r '.command | any(contains("consul-k8s-control-plane acl-init"))' | tee /dev/stderr) + [ "${actual}" = "true" ] + + local actual=$(echo $object | + yq '[.env[1].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) + [ "${actual}" = "true" ] + + local actual=$(echo $object | + yq '[.env[2].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) + [ "${actual}" = "true" ] +} + +@test "connectInject/Deployment: init container is created when global.acls.manageSystemACLs=true and has correct command with Partitions enabled" { + cd `chart_dir` + local object=$(helm template \ + -s templates/connect-inject-deployment.yaml \ + --set 'connectInject.enabled=true' \ + --set 'global.tls.enabled=true' \ + --set 'global.enableConsulNamespaces=true' \ + --set 'global.adminPartitions.enabled=true' \ + --set 'global.adminPartitions.name=default' \ + --set 'global.acls.manageSystemACLs=true' \ + . | tee /dev/stderr | + yq '.spec.template.spec.initContainers[] | select(.name == "connect-injector-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"))' | tee /dev/stderr) + [ "${actual}" = "true" ] + + local actual=$(echo $object | + yq -r '.command | any(contains("-partition=default"))' | tee /dev/stderr) + [ "${actual}" = "true" ] + + local actual=$(echo $object | + yq '[.env[1].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) + [ "${actual}" = "true" ] + + local actual=$(echo $object | + yq '[.env[2].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) + [ "${actual}" = "true" ] +} + +@test "connectInject/Deployment: init container is created when global.acls.manageSystemACLs=true and has correct command and environment with tls enabled and autoencrypt enabled" { + cd `chart_dir` + local object=$(helm template \ + -s templates/connect-inject-deployment.yaml \ + --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 == "connect-injector-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 '[.env[1].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) + [ "${actual}" = "true" ] + + local actual=$(echo $object | + yq '[.env[2].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) + [ "${actual}" = "true" ] +} + +@test "connectInject/Deployment: auto-encrypt init container is created and is the first init-container when global.acls.manageSystemACLs=true and has correct command and environment with tls enabled and autoencrypt enabled" { + cd `chart_dir` + local object=$(helm template \ + -s templates/connect-inject-deployment.yaml \ + --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[0]' | tee /dev/stderr) + + local actual=$(echo $object | + yq -r '.name' | tee /dev/stderr) + [ "${actual}" = "get-auto-encrypt-client-ca" ] } @test "connectInject/Deployment: cross namespace policy is not added when global.acls.manageSystemACLs=false" { @@ -985,6 +1133,61 @@ EOF [ "${actual}" = "true" ] } +@test "connectInject/Deployment: init container is created when global.acls.manageSystemACLs=true and has correct command when in non-primary datacenter with Consul Namespaces disabled" { + cd `chart_dir` + local object=$(helm template \ + -s templates/connect-inject-deployment.yaml \ + --set 'connectInject.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 == "connect-injector-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"))' | tee /dev/stderr) + [ "${actual}" = "true" ] +} + +@test "connectInject/Deployment: init container is created when global.acls.manageSystemACLs=true and has correct command when in non-primary datacenter with Consul Namespaces enabled" { + cd `chart_dir` + local object=$(helm template \ + -s templates/connect-inject-deployment.yaml \ + --set 'connectInject.enabled=true' \ + --set 'global.datacenter=dc2' \ + --set 'global.enableConsulNamespaces=true' \ + --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 == "connect-injector-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" ] +} + #-------------------------------------------------------------------- # resources diff --git a/charts/consul/test/unit/server-acl-init-job.bats b/charts/consul/test/unit/server-acl-init-job.bats index 73dd3f6432..f982c47c28 100644 --- a/charts/consul/test/unit/server-acl-init-job.bats +++ b/charts/consul/test/unit/server-acl-init-job.bats @@ -961,7 +961,7 @@ load _helpers [ "${actual}" = "false" ] local actual=$(echo $object | - yq 'any(contains("create-inject-token"))' | tee /dev/stderr) + yq 'any(contains("connect-inject"))' | tee /dev/stderr) [ "${actual}" = "false" ] local actual=$(echo $object | @@ -1008,7 +1008,7 @@ load _helpers [ "${actual}" = "false" ] local actual=$(echo $object | - yq 'any(contains("create-inject-token"))' | tee /dev/stderr) + yq 'any(contains("connect-inject"))' | tee /dev/stderr) [ "${actual}" = "false" ] local actual=$(echo $object | @@ -1051,7 +1051,7 @@ load _helpers [ "${actual}" = "false" ] local actual=$(echo $object | - yq 'any(contains("create-inject-token"))' | tee /dev/stderr) + yq 'any(contains("connect-inject"))' | tee /dev/stderr) [ "${actual}" = "false" ] local actual=$(echo $object | @@ -1095,7 +1095,7 @@ load _helpers [ "${actual}" = "false" ] local actual=$(echo $object | - yq 'any(contains("create-inject-token"))' | tee /dev/stderr) + yq 'any(contains("connect-inject"))' | tee /dev/stderr) [ "${actual}" = "false" ] local actual=$(echo $object | @@ -1140,7 +1140,7 @@ load _helpers [ "${actual}" = "true" ] local actual=$(echo $object | - yq 'any(contains("create-inject-token"))' | tee /dev/stderr) + yq 'any(contains("connect-inject"))' | tee /dev/stderr) [ "${actual}" = "false" ] local actual=$(echo $object | @@ -1187,7 +1187,7 @@ load _helpers [ "${actual}" = "false" ] local actual=$(echo $object | - yq 'any(contains("create-inject-token"))' | tee /dev/stderr) + yq 'any(contains("connect-inject"))' | tee /dev/stderr) [ "${actual}" = "false" ] local actual=$(echo $object | @@ -1230,7 +1230,7 @@ load _helpers [ "${actual}" = "false" ] local actual=$(echo $object | - yq 'any(contains("create-inject-token"))' | tee /dev/stderr) + yq 'any(contains("connect-inject"))' | tee /dev/stderr) [ "${actual}" = "true" ] local actual=$(echo $object | @@ -1274,7 +1274,7 @@ load _helpers [ "${actual}" = "false" ] local actual=$(echo $object | - yq 'any(contains("create-inject-token"))' | tee /dev/stderr) + yq 'any(contains("connect-inject"))' | tee /dev/stderr) [ "${actual}" = "true" ] local actual=$(echo $object | @@ -1319,7 +1319,7 @@ load _helpers [ "${actual}" = "false" ] local actual=$(echo $object | - yq 'any(contains("create-inject-token"))' | tee /dev/stderr) + yq 'any(contains("connect-inject"))' | tee /dev/stderr) [ "${actual}" = "true" ] local actual=$(echo $object | diff --git a/control-plane/subcommand/server-acl-init/command.go b/control-plane/subcommand/server-acl-init/command.go index 6c5e0f3e4b..bb729b5137 100644 --- a/control-plane/subcommand/server-acl-init/command.go +++ b/control-plane/subcommand/server-acl-init/command.go @@ -45,7 +45,7 @@ type Command struct { flagCreateSyncToken bool flagSyncConsulNodeName string - flagCreateInjectToken bool + flagConnectInject bool flagAuthMethodHost string flagBindingRuleSelector string @@ -132,19 +132,8 @@ func (c *Command) init() { "The Consul node name to register for catalog sync. Defaults to k8s-sync. To be discoverable "+ "via DNS, the name should only contain alpha-numerics and dashes.") - // Previously when this flag was set, -enable-namespaces and -create-inject-auth-method - // were always passed, so now we just look at those flags and ignore - // this flag. We keep the flag here though so there's no error if it's - // passed. - var unused bool - c.flags.BoolVar(&unused, "create-inject-namespace-token", false, - "Toggle for creating a connect injector token. Only required when namespaces are enabled. "+ - "Deprecated: set -enable-namespaces and -create-inject-token instead.") - - c.flags.BoolVar(&c.flagCreateInjectToken, "create-inject-auth-method", false, - "Toggle for creating a connect inject auth method. Deprecated: use -create-inject-token instead.") - c.flags.BoolVar(&c.flagCreateInjectToken, "create-inject-token", false, - "Toggle for creating a connect inject auth method and an ACL token.") + c.flags.BoolVar(&c.flagConnectInject, "connect-inject", false, + "Toggle for creating a connect inject auth method and policy.") c.flags.StringVar(&c.flagAuthMethodHost, "auth-method-host", "", "Kubernetes Host config parameter for the auth method."+ "If not provided, the default cluster Kubernetes service will be used.") @@ -512,9 +501,9 @@ func (c *Command) Run(args []string) int { } } - if c.flagCreateInjectToken { - authMethodName := c.withPrefix("k8s-auth-method") - err := c.configureConnectInjectAuthMethod(consulClient, authMethodName) + if c.flagConnectInject { + connectAuthMethodName := c.withPrefix("k8s-auth-method") + err := c.configureConnectInjectAuthMethod(consulClient, connectAuthMethodName) if err != nil { c.log.Error(err.Error()) return 1 @@ -527,14 +516,22 @@ func (c *Command) Run(args []string) int { return 1 } + serviceAccountName := c.withPrefix("connect-injector") + componentAuthMethodName := localComponentAuthMethodName + // If namespaces are enabled, the policy and token need to be global // to be allowed to create namespaces. if c.flagEnableNamespaces { - err = c.createGlobalACL("connect-inject", injectRules, consulDC, primary, consulClient) + // Create the controller ACL Policy, Role and BindingRule but do not issue any ACLTokens or create Kube Secrets. + // ConnectInjector token must be global when namespaces are enabled. This means secondary datacenters need + // a token that is known by the primary datacenters. + if !primary { + componentAuthMethodName = globalComponentAuthMethodName + } + err = c.createACLPolicyRoleAndBindingRule("connect-inject", injectRules, consulDC, primaryDC, globalPolicy, primary, componentAuthMethodName, serviceAccountName, consulClient) } else { - err = c.createLocalACL("connect-inject", injectRules, consulDC, primary, consulClient) + err = c.createACLPolicyRoleAndBindingRule("connect-inject", injectRules, consulDC, primaryDC, localPolicy, primary, componentAuthMethodName, serviceAccountName, consulClient) } - if err != nil { c.log.Error(err.Error()) return 1 @@ -739,7 +736,7 @@ func (c *Command) Run(args []string) int { if !primary { authMethodName = globalComponentAuthMethodName } - err = c.createACLPolicyRoleAndBindingRule("controller", rules, consulDC, primaryDC, globalToken, primary, authMethodName, serviceAccountName, consulClient) + err = c.createACLPolicyRoleAndBindingRule("controller", rules, consulDC, primaryDC, globalPolicy, primary, authMethodName, serviceAccountName, consulClient) if err != nil { c.log.Error(err.Error()) return 1 @@ -905,7 +902,7 @@ func (c *Command) createAnonymousPolicy(isPrimary bool) bool { // on cross-dc API calls. The cross-dc API calls thus use the anonymous // token. Cross-dc API calls are needed by the Connect proxies to talk // cross-dc. - (c.flagCreateInjectToken && c.flagFederation)) + (c.flagConnectInject && c.flagFederation)) } func (c *Command) validateFlags() error { @@ -947,11 +944,13 @@ func (c *Command) validateFlags() error { return nil } -const consulDefaultNamespace = "default" -const consulDefaultPartition = "default" -const globalToken = true -const synopsis = "Initialize ACLs on Consul servers and other components." -const help = ` +const ( + consulDefaultNamespace = "default" + consulDefaultPartition = "default" + globalPolicy = true + localPolicy = false + synopsis = "Initialize ACLs on Consul servers and other components." + help = ` Usage: consul-k8s-control-plane server-acl-init [options] Bootstraps servers with ACLs and creates policies and ACL tokens for other @@ -960,3 +959,4 @@ Usage: consul-k8s-control-plane server-acl-init [options] and safe to run multiple times. ` +) diff --git a/control-plane/subcommand/server-acl-init/command_ent_test.go b/control-plane/subcommand/server-acl-init/command_ent_test.go index 3dd8c675b2..5f0c4f8da5 100644 --- a/control-plane/subcommand/server-acl-init/command_ent_test.go +++ b/control-plane/subcommand/server-acl-init/command_ent_test.go @@ -44,7 +44,7 @@ func TestRun_ConnectInject_SingleDestinationNamespace(t *testing.T) { "-server-port=" + strings.Split(testAgent.HTTPAddr, ":")[1], "-resource-prefix=" + resourcePrefix, "-k8s-namespace=" + ns, - "-create-inject-token", + "-connect-inject", "-enable-partitions", "-partition=default", "-enable-namespaces", @@ -172,7 +172,7 @@ func TestRun_ConnectInject_NamespaceMirroring(t *testing.T) { "-server-port=" + strings.Split(testAgent.HTTPAddr, ":")[1], "-resource-prefix=" + resourcePrefix, "-k8s-namespace=" + ns, - "-create-inject-token", + "-connect-inject", "-enable-partitions", "-partition=default", "-enable-namespaces", @@ -288,7 +288,7 @@ func TestRun_ACLPolicyUpdates(t *testing.T) { "-allow-dns", "-create-mesh-gateway-token", "-create-sync-token", - "-create-inject-token", + "-connect-inject", "-create-snapshot-agent-token", "-create-enterprise-license-token", "-ingress-gateway-name=gw", @@ -333,7 +333,7 @@ func TestRun_ACLPolicyUpdates(t *testing.T) { "anothergw-ingress-gateway-token", "gw-terminating-gateway-token", "anothergw-terminating-gateway-token", - "connect-inject-token", + "connect-inject-policy", "controller-policy", } policies, _, err := consul.ACL().PolicyList(nil) @@ -377,7 +377,7 @@ func TestRun_ACLPolicyUpdates(t *testing.T) { "anonymous-token-policy", "client-token", "catalog-sync-token", - "connect-inject-token", + "connect-inject-policy", "mesh-gateway-token", "client-snapshot-agent-token", "enterprise-license-token", @@ -599,7 +599,7 @@ func TestRun_ConnectInject_Updates(t *testing.T) { "-k8s-namespace=" + ns, "-enable-partitions", "-partition=default", - "-create-inject-token", + "-connect-inject", } // First run. NOTE: we don't assert anything here since we've @@ -682,13 +682,6 @@ func TestRun_TokensWithNamespacesEnabled(t *testing.T) { SecretNames: []string{resourcePrefix + "-catalog-sync-acl-token"}, LocalToken: false, }, - "connect-inject-token": { - TokenFlags: []string{"-create-inject-token", "-enable-namespaces"}, - PolicyNames: []string{"connect-inject-token"}, - PolicyDCs: nil, - SecretNames: []string{resourcePrefix + "-connect-inject-acl-token"}, - LocalToken: false, - }, "enterprise-license token": { TokenFlags: []string{"-create-enterprise-license-token"}, PolicyNames: []string{"enterprise-license-token"}, @@ -743,20 +736,6 @@ func TestRun_TokensWithNamespacesEnabled(t *testing.T) { SecretNames: []string{resourcePrefix + "-acl-replication-acl-token"}, LocalToken: false, }, - "inject token with namespaces (deprecated)": { - TokenFlags: []string{"-create-inject-auth-method", "-enable-namespaces", "-create-inject-namespace-token"}, - PolicyNames: []string{"connect-inject-token"}, - PolicyDCs: nil, - SecretNames: []string{resourcePrefix + "-connect-inject-acl-token"}, - LocalToken: false, - }, - "inject token and namespaces": { - TokenFlags: []string{"-create-inject-token", "-enable-namespaces"}, - PolicyNames: []string{"connect-inject-token"}, - PolicyDCs: nil, - SecretNames: []string{resourcePrefix + "-connect-inject-acl-token"}, - LocalToken: false, - }, "partitions token": { TokenFlags: []string{"-enable-partitions", "-partition=default"}, PolicyNames: []string{"partitions-token"}, @@ -1079,197 +1058,6 @@ partition "default" { } } -// Test creating the correct ACL policies and Binding Rules for components in the primary datacenter. -// The test works by running the command and then ensuring that: -// * An ACLBindingRule exists which references the ACLRole. -// * An ACLRole exists and has the correct PolicyName in it's ACLPolicyLinkRule list. -// * The ACLPolicy exists. -func TestRun_PrimaryDatacenter_PoliciesAndBindingRulesForACLLogin_NamespacesEnabled(t *testing.T) { - t.Parallel() - - cases := []struct { - TestName string - TokenFlags []string - PolicyNames []string - Roles []string - Namespace string - }{ - { - TestName: "Controller", - TokenFlags: []string{"-create-controller-token"}, - PolicyNames: []string{"controller-policy"}, - Roles: []string{resourcePrefix + "-controller-acl-role"}, - Namespace: ns, - }, - } - for _, c := range cases { - t.Run(c.TestName, func(t *testing.T) { - k8s, testSvr := completeSetup(t) - defer testSvr.Stop() - setUpK8sServiceAccount(t, k8s, ns) - - // Run the command. - ui := cli.NewMockUi() - cmd := Command{ - UI: ui, - clientset: k8s, - } - cmdArgs := append([]string{ - "-timeout=500ms", - "-resource-prefix=" + resourcePrefix, - "-k8s-namespace=" + c.Namespace, - "-enable-namespaces", - "-consul-inject-destination-namespace", c.Namespace, - "-server-address", strings.Split(testSvr.HTTPAddr, ":")[0], - "-server-port", strings.Split(testSvr.HTTPAddr, ":")[1], - }, c.TokenFlags...) - cmd.init() - responseCode := cmd.Run(cmdArgs) - require.Equal(t, 0, responseCode, ui.ErrorWriter.String()) - - bootToken := getBootToken(t, k8s, resourcePrefix, ns) - consul, err := api.NewClient(&api.Config{ - Namespace: ns, - Address: testSvr.HTTPAddr, - Token: bootToken, - }) - require.NoError(t, err) - - // Check that the Role exists + has correct Policy and is associated with a BindingRule. - for i := range c.Roles { - // Check that the Policy exists. - policy, _, err := consul.ACL().PolicyReadByName(c.PolicyNames[i], &api.QueryOptions{}) - require.NoError(t, err) - require.NotNil(t, policy) - - // Check that the Role exists. - role, _, err := consul.ACL().RoleReadByName(c.Roles[i], &api.QueryOptions{}) - require.NoError(t, err) - require.NotNil(t, role) - - // Check that the Role references the Policy. - found := false - for x := range role.Policies { - if role.Policies[x].Name == policy.Name { - found = true - break - } - } - require.True(t, found) - - // Check that there exists a BindingRule that references this Role. - rb, _, err := consul.ACL().BindingRuleList(fmt.Sprintf("%s-%s", resourcePrefix, componentAuthMethod), &api.QueryOptions{}) - require.NoError(t, err) - require.NotNil(t, rb) - found = false - for x := range rb { - if rb[x].BindName == c.Roles[i] { - found = true - break - } - } - require.True(t, found) - } - }) - } -} - -// Test creating the correct ACL policies and Binding Rules for components running in the secondary datacenter. -// The test works by running the command and then ensuring that: -// * An ACLBindingRule exists which references the ACLRole. -// * An ACLRole exists and has the correct PolicyName in it's ACLPolicyLinkRule list. -// * The ACLPolicy exists. -func TestRun_SecondaryDatacenter_PoliciesAndBindingRulesForACLLogin_NamespacesEnabled(t *testing.T) { - t.Parallel() - - const ( - secondaryDatacenter = "dc2" - primaryDatacenter = "dc1" - ) - cases := []struct { - TestName string - TokenFlags []string - PolicyNames []string - Roles []string - Namespace string - }{ - { - TestName: "Controller", - TokenFlags: []string{"-create-controller-token"}, - PolicyNames: []string{"controller-policy-" + secondaryDatacenter}, - Roles: []string{resourcePrefix + "-controller-acl-role-" + secondaryDatacenter}, - Namespace: ns, - }, - } - for _, c := range cases { - t.Run(c.TestName, func(t *testing.T) { - bootToken := "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" - tokenFile := common.WriteTempFile(t, bootToken) - k8s, consul, consulHTTPAddr, cleanup := mockReplicatedSetup(t, bootToken) - setUpK8sServiceAccount(t, k8s, ns) - defer cleanup() - - // Run the command. - ui := cli.NewMockUi() - cmd := Command{ - UI: ui, - clientset: k8s, - } - cmdArgs := append([]string{ - "-federation", - "-timeout=1m", - "-resource-prefix=" + resourcePrefix, - "-k8s-namespace=" + c.Namespace, - "-enable-namespaces", - "-consul-inject-destination-namespace", c.Namespace, - "-auth-method-host=" + "https://my-kube.com", - "-acl-replication-token-file", tokenFile, - "-server-address", strings.Split(consulHTTPAddr, ":")[0], - "-server-port", strings.Split(consulHTTPAddr, ":")[1], - }, c.TokenFlags...) - cmd.init() - responseCode := cmd.Run(cmdArgs) - require.Equal(t, 0, responseCode, ui.ErrorWriter.String()) - - // Check that the Role exists + has correct Policy and is associated with a BindingRule. - for i := range c.Roles { - // Check that the Policy exists. - policy, _, err := consul.ACL().PolicyReadByName(c.PolicyNames[i], &api.QueryOptions{Datacenter: primaryDatacenter}) - require.NoError(t, err) - require.NotNil(t, policy) - - // Check that the Role exists. - role, _, err := consul.ACL().RoleReadByName(c.Roles[i], &api.QueryOptions{Datacenter: primaryDatacenter}) - require.NoError(t, err) - require.NotNil(t, role) - - // Check that the Role references the Policy. - found := false - for x := range role.Policies { - if role.Policies[x].Name == policy.Name { - found = true - break - } - } - require.True(t, found) - - // Check that there exists a BindingRule that references this Role. - rb, _, err := consul.ACL().BindingRuleList(fmt.Sprintf("%s-%s-%s", resourcePrefix, componentAuthMethod, secondaryDatacenter), &api.QueryOptions{Datacenter: primaryDatacenter}) - require.NoError(t, err) - require.NotNil(t, rb) - found = false - for x := range rb { - if rb[x].BindName == c.Roles[i] { - found = true - break - } - } - require.True(t, found) - } - }) - } -} - // Test that server-acl-init used the local auth method to create the desired token in the primary datacenter. // The test works by running the login command and then ensuring that the token // returned has the correct role for the component. @@ -1282,7 +1070,15 @@ func TestRun_NamespaceEnabled_ValidateLoginToken_PrimaryDatacenter(t *testing.T) Roles []string Namespace string GlobalToken bool - }{} + }{ + { + ComponentName: "connect-injector", + TokenFlags: []string{"-connect-inject"}, + Roles: []string{resourcePrefix + "-connect-injector-acl-role"}, + Namespace: ns, + GlobalToken: false, + }, + } for _, c := range cases { t.Run(c.ComponentName, func(t *testing.T) { authMethodName := fmt.Sprintf("%s-%s", resourcePrefix, componentAuthMethod) @@ -1313,6 +1109,7 @@ func TestRun_NamespaceEnabled_ValidateLoginToken_PrimaryDatacenter(t *testing.T) cmdArgs := append([]string{ "-timeout=500ms", "-resource-prefix=" + resourcePrefix, + "-enable-namespaces", "-k8s-namespace=" + c.Namespace, "-enable-namespaces", "-consul-inject-destination-namespace", c.Namespace, @@ -1357,7 +1154,15 @@ func TestRun_NamespaceEnabled_ValidateLoginToken_SecondaryDatacenter(t *testing. Roles []string Namespace string GlobalToken bool - }{} + }{ + { + ComponentName: "connect-injector", + TokenFlags: []string{"-connect-inject"}, + Roles: []string{resourcePrefix + "-connect-injector-acl-role-dc2"}, + Namespace: ns, + GlobalToken: true, + }, + } for _, c := range cases { t.Run(c.ComponentName, func(t *testing.T) { bootToken := "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" @@ -1391,6 +1196,7 @@ func TestRun_NamespaceEnabled_ValidateLoginToken_SecondaryDatacenter(t *testing. "-federation", "-timeout=1m", "-resource-prefix=" + resourcePrefix, + "-enable-namespaces", "-k8s-namespace=" + c.Namespace, "-enable-namespaces", "-consul-inject-destination-namespace", c.Namespace, diff --git a/control-plane/subcommand/server-acl-init/command_test.go b/control-plane/subcommand/server-acl-init/command_test.go index 76671b49fa..bfd2a82d67 100644 --- a/control-plane/subcommand/server-acl-init/command_test.go +++ b/control-plane/subcommand/server-acl-init/command_test.go @@ -248,14 +248,6 @@ func TestRun_TokensPrimaryDC(t *testing.T) { SecretNames: []string{resourcePrefix + "-acl-replication-acl-token"}, LocalToken: false, }, - { - TestName: "Endpoints Controller ACL token", - TokenFlags: []string{"-create-inject-token"}, - PolicyNames: []string{"connect-inject-token"}, - PolicyDCs: []string{"dc1"}, - SecretNames: []string{resourcePrefix + "-connect-inject-acl-token"}, - LocalToken: true, - }, } for _, c := range cases { t.Run(c.TestName, func(t *testing.T) { @@ -474,14 +466,6 @@ func TestRun_TokensReplicatedDC(t *testing.T) { resourcePrefix + "-another-gateway-terminating-gateway-acl-token"}, LocalToken: true, }, - { - TestName: "Endpoints controller ACL token", - TokenFlags: []string{"-create-inject-token"}, - PolicyNames: []string{"connect-inject-token-dc2"}, - PolicyDCs: []string{"dc2"}, - SecretNames: []string{resourcePrefix + "-connect-inject-acl-token"}, - LocalToken: true, - }, } for _, c := range cases { t.Run(c.TestName, func(t *testing.T) { @@ -552,12 +536,6 @@ func TestRun_TokensWithProvidedBootstrapToken(t *testing.T) { PolicyNames: []string{"client-token"}, SecretNames: []string{resourcePrefix + "-client-acl-token"}, }, - { - TestName: "Endpoints controller ACL token", - TokenFlags: []string{"-create-inject-token"}, - PolicyNames: []string{"connect-inject-token"}, - SecretNames: []string{resourcePrefix + "-connect-inject-acl-token"}, - }, { TestName: "Sync token", TokenFlags: []string{"-create-sync-token"}, @@ -694,33 +672,13 @@ func TestRun_AnonymousTokenPolicy(t *testing.T) { SecondaryDC: true, ExpAnonymousPolicy: false, }, - "auth method, primary dc, no replication (deprecated)": { - Flags: []string{"-create-inject-auth-method"}, - SecondaryDC: false, - ExpAnonymousPolicy: false, - }, - "auth method, primary dc, with federation": { - Flags: []string{"-create-inject-auth-method", "-federation"}, - SecondaryDC: false, - ExpAnonymousPolicy: true, - }, - "auth method, secondary dc, with federation": { - Flags: []string{"-create-inject-auth-method", "-federation"}, - SecondaryDC: true, - ExpAnonymousPolicy: false, - }, - "auth method, secondary dc (deprecated)": { - Flags: []string{"-create-inject-auth-method"}, - SecondaryDC: true, - ExpAnonymousPolicy: false, - }, "auth method, primary dc, no replication": { - Flags: []string{"-create-inject-token"}, + Flags: []string{"-connect-inject"}, SecondaryDC: false, ExpAnonymousPolicy: false, }, "auth method, secondary dc": { - Flags: []string{"-create-inject-token"}, + Flags: []string{"-connect-inject"}, SecondaryDC: true, ExpAnonymousPolicy: false, }, @@ -821,24 +779,13 @@ func TestRun_ConnectInjectAuthMethod(t *testing.T) { flags []string expectedHost string }{ - "-create-inject-token flag": { - flags: []string{"-create-inject-token"}, - expectedHost: "https://kubernetes.default.svc", - }, - "-create-inject-auth-method flag": { - flags: []string{"-create-inject-auth-method"}, + "-connect-inject flag": { + flags: []string{"-connect-inject"}, expectedHost: "https://kubernetes.default.svc", }, - "-auth-method-host flag (deprecated)": { - flags: []string{ - "-create-inject-auth-method", - "-auth-method-host=https://my-kube.com", - }, - expectedHost: "https://my-kube.com", - }, "-auth-method-host flag": { flags: []string{ - "-create-inject-token", + "-connect-inject", "-auth-method-host=https://my-kube.com", }, expectedHost: "https://my-kube.com", @@ -916,177 +863,162 @@ func TestRun_ConnectInjectAuthMethod(t *testing.T) { func TestRun_ConnectInjectAuthMethodUpdates(t *testing.T) { t.Parallel() - // Test with deprecated -create-inject-auth-method flag. - cases := []string{"-create-inject-auth-method", "-create-inject-token"} - for _, flag := range cases { - t.Run(flag, func(t *testing.T) { - - k8s, testSvr := completeSetup(t) - defer testSvr.Stop() - caCert, jwtToken := setUpK8sServiceAccount(t, k8s, ns) + k8s, testSvr := completeSetup(t) + defer testSvr.Stop() + caCert, jwtToken := setUpK8sServiceAccount(t, k8s, ns) - ui := cli.NewMockUi() - cmd := Command{ - UI: ui, - clientset: k8s, - } + ui := cli.NewMockUi() + cmd := Command{ + UI: ui, + clientset: k8s, + } - bindingRuleSelector := "serviceaccount.name!=default" + bindingRuleSelector := "serviceaccount.name!=default" - // First, create an auth method using the defaults - responseCode := cmd.Run([]string{ - "-timeout=1m", - "-resource-prefix=" + resourcePrefix, - "-k8s-namespace=" + ns, - "-server-address", strings.Split(testSvr.HTTPAddr, ":")[0], - "-server-port", strings.Split(testSvr.HTTPAddr, ":")[1], - flag, - "-acl-binding-rule-selector=" + bindingRuleSelector, - }) - require.Equal(t, 0, responseCode, ui.ErrorWriter.String()) - - // Check that the auth method was created. - bootToken := getBootToken(t, k8s, resourcePrefix, ns) - consul, err := api.NewClient(&api.Config{ - Address: testSvr.HTTPAddr, - }) - require.NoError(t, err) - authMethodName := resourcePrefix + "-k8s-auth-method" - authMethod, _, err := consul.ACL().AuthMethodRead(authMethodName, - &api.QueryOptions{Token: bootToken}) - require.NoError(t, err) - require.NotNil(t, authMethod) - require.Contains(t, authMethod.Config, "Host") - require.Equal(t, authMethod.Config["Host"], defaultKubernetesHost) - require.Contains(t, authMethod.Config, "CACert") - require.Equal(t, authMethod.Config["CACert"], caCert) - require.Contains(t, authMethod.Config, "ServiceAccountJWT") - require.Equal(t, authMethod.Config["ServiceAccountJWT"], jwtToken) + // First, create an auth method using the defaults + responseCode := cmd.Run([]string{ + "-timeout=1m", + "-resource-prefix=" + resourcePrefix, + "-k8s-namespace=" + ns, + "-server-address", strings.Split(testSvr.HTTPAddr, ":")[0], + "-server-port", strings.Split(testSvr.HTTPAddr, ":")[1], + "-connect-inject", + "-acl-binding-rule-selector=" + bindingRuleSelector, + }) + require.Equal(t, 0, responseCode, ui.ErrorWriter.String()) - // Generate a new CA certificate - _, _, caCertPem, _, err := cert.GenerateCA("kubernetes") - require.NoError(t, err) + // Check that the auth method was created. + bootToken := getBootToken(t, k8s, resourcePrefix, ns) + consul, err := api.NewClient(&api.Config{ + Address: testSvr.HTTPAddr, + }) + require.NoError(t, err) + authMethodName := resourcePrefix + "-k8s-auth-method" + authMethod, _, err := consul.ACL().AuthMethodRead(authMethodName, + &api.QueryOptions{Token: bootToken}) + require.NoError(t, err) + require.NotNil(t, authMethod) + require.Contains(t, authMethod.Config, "Host") + require.Equal(t, authMethod.Config["Host"], defaultKubernetesHost) + require.Contains(t, authMethod.Config, "CACert") + require.Equal(t, authMethod.Config["CACert"], caCert) + require.Contains(t, authMethod.Config, "ServiceAccountJWT") + require.Equal(t, authMethod.Config["ServiceAccountJWT"], jwtToken) + + // Generate a new CA certificate + _, _, caCertPem, _, err := cert.GenerateCA("kubernetes") + require.NoError(t, err) - // Overwrite the default kubernetes api, service account token and CA cert - kubernetesHost := "https://kubernetes.example.com" - // This token is the base64 encoded example token from jwt.io - serviceAccountToken = "ZXlKaGJHY2lPaUpJVXpJMU5pSXNJblI1Y0NJNklrcFhWQ0o5LmV5SnpkV0lpT2lJeE1qTTBOVFkzT0Rrd0lpd2libUZ0WlNJNklrcHZhRzRnUkc5bElpd2lhV0YwSWpveE5URTJNak01TURJeWZRLlNmbEt4d1JKU01lS0tGMlFUNGZ3cE1lSmYzNlBPazZ5SlZfYWRRc3N3NWM=" - serviceAccountCACert = base64.StdEncoding.EncodeToString([]byte(caCertPem)) + // Overwrite the default kubernetes api, service account token and CA cert + kubernetesHost := "https://kubernetes.example.com" + // This token is the base64 encoded example token from jwt.io + serviceAccountToken = "ZXlKaGJHY2lPaUpJVXpJMU5pSXNJblI1Y0NJNklrcFhWQ0o5LmV5SnpkV0lpT2lJeE1qTTBOVFkzT0Rrd0lpd2libUZ0WlNJNklrcHZhRzRnUkc5bElpd2lhV0YwSWpveE5URTJNak01TURJeWZRLlNmbEt4d1JKU01lS0tGMlFUNGZ3cE1lSmYzNlBPazZ5SlZfYWRRc3N3NWM=" + serviceAccountCACert = base64.StdEncoding.EncodeToString([]byte(caCertPem)) - // Create a new service account - updatedCACert, updatedJWTToken := setUpK8sServiceAccount(t, k8s, ns) + // Create a new service account + updatedCACert, updatedJWTToken := setUpK8sServiceAccount(t, k8s, ns) - // Run command again - responseCode = cmd.Run([]string{ - "-timeout=1m", - "-resource-prefix=" + resourcePrefix, - "-k8s-namespace=" + ns, - "-server-address", strings.Split(testSvr.HTTPAddr, ":")[0], - "-server-port", strings.Split(testSvr.HTTPAddr, ":")[1], - "-acl-binding-rule-selector=" + bindingRuleSelector, - flag, - "-auth-method-host=" + kubernetesHost, - }) - require.Equal(t, 0, responseCode, ui.ErrorWriter.String()) + // Run command again + responseCode = cmd.Run([]string{ + "-timeout=1m", + "-resource-prefix=" + resourcePrefix, + "-k8s-namespace=" + ns, + "-server-address", strings.Split(testSvr.HTTPAddr, ":")[0], + "-server-port", strings.Split(testSvr.HTTPAddr, ":")[1], + "-acl-binding-rule-selector=" + bindingRuleSelector, + "-connect-inject", + "-auth-method-host=" + kubernetesHost, + }) + require.Equal(t, 0, responseCode, ui.ErrorWriter.String()) - // Check that the auth method has been updated - authMethod, _, err = consul.ACL().AuthMethodRead(authMethodName, - &api.QueryOptions{Token: bootToken}) - require.NoError(t, err) - require.NotNil(t, authMethod) - require.Contains(t, authMethod.Config, "Host") - require.Equal(t, authMethod.Config["Host"], kubernetesHost) - require.Contains(t, authMethod.Config, "CACert") - require.Equal(t, authMethod.Config["CACert"], updatedCACert) - require.Contains(t, authMethod.Config, "ServiceAccountJWT") - require.Equal(t, authMethod.Config["ServiceAccountJWT"], updatedJWTToken) - }) - } + // Check that the auth method has been updated + authMethod, _, err = consul.ACL().AuthMethodRead(authMethodName, + &api.QueryOptions{Token: bootToken}) + require.NoError(t, err) + require.NotNil(t, authMethod) + require.Contains(t, authMethod.Config, "Host") + require.Equal(t, authMethod.Config["Host"], kubernetesHost) + require.Contains(t, authMethod.Config, "CACert") + require.Equal(t, authMethod.Config["CACert"], updatedCACert) + require.Contains(t, authMethod.Config, "ServiceAccountJWT") + require.Equal(t, authMethod.Config["ServiceAccountJWT"], updatedJWTToken) } // Test that ACL binding rules are updated if the rule selector changes. -func TestRun_BindingRuleUpdates(tt *testing.T) { - tt.Parallel() +func TestRun_BindingRuleUpdates(t *testing.T) { + k8s, testSvr := completeSetup(t) + setUpK8sServiceAccount(t, k8s, ns) + defer testSvr.Stop() - // Test with deprecated -create-inject-auth-method flag. - cases := []string{"-create-inject-auth-method", "-create-inject-token"} - for _, flag := range cases { - tt.Run(flag, func(t *testing.T) { - k8s, testSvr := completeSetup(t) - setUpK8sServiceAccount(t, k8s, ns) - defer testSvr.Stop() + consul, err := api.NewClient(&api.Config{ + Address: testSvr.HTTPAddr, + }) + require.NoError(t, err) - consul, err := api.NewClient(&api.Config{ - Address: testSvr.HTTPAddr, - }) - require.NoError(t, err) + ui := cli.NewMockUi() + commonArgs := []string{ + "-resource-prefix=" + resourcePrefix, + "-k8s-namespace=" + ns, + "-server-address", strings.Split(testSvr.HTTPAddr, ":")[0], + "-server-port", strings.Split(testSvr.HTTPAddr, ":")[1], + "-connect-inject", + } + firstRunArgs := append(commonArgs, + "-acl-binding-rule-selector=serviceaccount.name!=default", + ) + // On the second run, we change the binding rule selector. + secondRunArgs := append(commonArgs, + "-acl-binding-rule-selector=serviceaccount.name!=changed", + ) - ui := cli.NewMockUi() - commonArgs := []string{ - "-resource-prefix=" + resourcePrefix, - "-k8s-namespace=" + ns, - "-server-address", strings.Split(testSvr.HTTPAddr, ":")[0], - "-server-port", strings.Split(testSvr.HTTPAddr, ":")[1], - flag, - } - firstRunArgs := append(commonArgs, - "-acl-binding-rule-selector=serviceaccount.name!=default", - ) - // On the second run, we change the binding rule selector. - secondRunArgs := append(commonArgs, - "-acl-binding-rule-selector=serviceaccount.name!=changed", - ) - - // Run the command first to populate the binding rule. - cmd := Command{ - UI: ui, - clientset: k8s, - } - responseCode := cmd.Run(firstRunArgs) - require.Equal(t, 0, responseCode, ui.ErrorWriter.String()) + // Run the command first to populate the binding rule. + cmd := Command{ + UI: ui, + clientset: k8s, + } + responseCode := cmd.Run(firstRunArgs) + require.Equal(t, 0, responseCode, ui.ErrorWriter.String()) - // Validate the binding rule. - { - queryOpts := &api.QueryOptions{Token: getBootToken(t, k8s, resourcePrefix, ns)} - authMethodName := resourcePrefix + "-k8s-auth-method" - rules, _, err := consul.ACL().BindingRuleList(authMethodName, queryOpts) - require.NoError(t, err) - require.Len(t, rules, 1) - actRule, _, err := consul.ACL().BindingRuleRead(rules[0].ID, queryOpts) - require.NoError(t, err) - require.NotNil(t, actRule) - require.Equal(t, "Kubernetes binding rule", actRule.Description) - require.Equal(t, api.BindingRuleBindTypeService, actRule.BindType) - require.Equal(t, "${serviceaccount.name}", actRule.BindName) - require.Equal(t, "serviceaccount.name!=default", actRule.Selector) - } + // Validate the binding rule. + { + queryOpts := &api.QueryOptions{Token: getBootToken(t, k8s, resourcePrefix, ns)} + authMethodName := resourcePrefix + "-k8s-auth-method" + rules, _, err := consul.ACL().BindingRuleList(authMethodName, queryOpts) + require.NoError(t, err) + require.Len(t, rules, 1) + actRule, _, err := consul.ACL().BindingRuleRead(rules[0].ID, queryOpts) + require.NoError(t, err) + require.NotNil(t, actRule) + require.Equal(t, "Kubernetes binding rule", actRule.Description) + require.Equal(t, api.BindingRuleBindTypeService, actRule.BindType) + require.Equal(t, "${serviceaccount.name}", actRule.BindName) + require.Equal(t, "serviceaccount.name!=default", actRule.Selector) + } - // Re-run the command with namespace flags. The policies should be updated. - // NOTE: We're redefining the command so that the old flag values are - // reset. - cmd = Command{ - UI: ui, - clientset: k8s, - } - responseCode = cmd.Run(secondRunArgs) - require.Equal(t, 0, responseCode, ui.ErrorWriter.String()) + // Re-run the command with namespace flags. The policies should be updated. + // NOTE: We're redefining the command so that the old flag values are + // reset. + cmd = Command{ + UI: ui, + clientset: k8s, + } + responseCode = cmd.Run(secondRunArgs) + require.Equal(t, 0, responseCode, ui.ErrorWriter.String()) - // Check the binding rule is changed expected. - { - queryOpts := &api.QueryOptions{Token: getBootToken(t, k8s, resourcePrefix, ns)} - authMethodName := resourcePrefix + "-k8s-auth-method" - rules, _, err := consul.ACL().BindingRuleList(authMethodName, queryOpts) - require.NoError(t, err) - require.Len(t, rules, 1) - actRule, _, err := consul.ACL().BindingRuleRead(rules[0].ID, queryOpts) - require.NoError(t, err) - require.NotNil(t, actRule) - require.Equal(t, "Kubernetes binding rule", actRule.Description) - require.Equal(t, api.BindingRuleBindTypeService, actRule.BindType) - require.Equal(t, "${serviceaccount.name}", actRule.BindName) - require.Equal(t, "serviceaccount.name!=changed", actRule.Selector) - } - }) + // Check the binding rule is changed expected. + { + queryOpts := &api.QueryOptions{Token: getBootToken(t, k8s, resourcePrefix, ns)} + authMethodName := resourcePrefix + "-k8s-auth-method" + rules, _, err := consul.ACL().BindingRuleList(authMethodName, queryOpts) + require.NoError(t, err) + require.Len(t, rules, 1) + actRule, _, err := consul.ACL().BindingRuleRead(rules[0].ID, queryOpts) + require.NoError(t, err) + require.NotNil(t, actRule) + require.Equal(t, "Kubernetes binding rule", actRule.Description) + require.Equal(t, api.BindingRuleBindTypeService, actRule.BindType) + require.Equal(t, "${serviceaccount.name}", actRule.BindName) + require.Equal(t, "serviceaccount.name!=changed", actRule.Selector) } } @@ -2073,7 +2005,7 @@ func TestRun_ACLReplicationTokenValid(t *testing.T) { // Test that if acl replication is enabled, we don't create an anonymous token policy. func TestRun_AnonPolicy_IgnoredWithReplication(t *testing.T) { // The anonymous policy is configured when one of these flags is set. - cases := []string{"-allow-dns", "-create-inject-auth-method"} + cases := []string{"-allow-dns", "-connect-inject"} for _, flag := range cases { t.Run(flag, func(t *testing.T) { bootToken := "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" @@ -2241,6 +2173,12 @@ func TestRun_PoliciesAndBindingRulesForACLLogin_PrimaryDatacenter(t *testing.T) PolicyNames: []string{"controller-policy"}, Roles: []string{resourcePrefix + "-controller-acl-role"}, }, + { + TestName: "Connect Inject", + TokenFlags: []string{"-connect-inject"}, + PolicyNames: []string{"connect-inject-policy"}, + Roles: []string{resourcePrefix + "-connect-injector-acl-role"}, + }, } for _, c := range cases { t.Run(c.TestName, func(t *testing.T) { @@ -2286,8 +2224,8 @@ func TestRun_PoliciesAndBindingRulesForACLLogin_PrimaryDatacenter(t *testing.T) // Check that the Role references the Policy. found := false - for x := range role.Policies { - if role.Policies[x].Name == policy.Name { + for j := range role.Policies { + if role.Policies[j].Name == policy.Name { found = true break } @@ -2299,8 +2237,8 @@ func TestRun_PoliciesAndBindingRulesForACLLogin_PrimaryDatacenter(t *testing.T) require.NoError(t, err) require.NotNil(t, rb) found = false - for x := range rb { - if rb[x].BindName == c.Roles[i] { + for j := range rb { + if rb[j].BindName == c.Roles[i] { found = true break } @@ -2325,16 +2263,25 @@ func TestRun_PoliciesAndBindingRulesACLLogin_SecondaryDatacenter(t *testing.T) { primaryDatacenter = "dc1" ) cases := []struct { - TestName string - TokenFlags []string - PolicyNames []string - Roles []string + TestName string + TokenFlags []string + PolicyNames []string + Roles []string + GlobalAuthMethod bool }{ { - TestName: "Controller", - TokenFlags: []string{"-create-controller-token"}, - PolicyNames: []string{"controller-policy-" + secondaryDatacenter}, - Roles: []string{resourcePrefix + "-controller-acl-role-" + secondaryDatacenter}, + TestName: "Controller", + TokenFlags: []string{"-create-controller-token"}, + PolicyNames: []string{"controller-policy-" + secondaryDatacenter}, + Roles: []string{resourcePrefix + "-controller-acl-role-" + secondaryDatacenter}, + GlobalAuthMethod: true, + }, + { + TestName: "Connect Inject", + TokenFlags: []string{"-connect-inject"}, + PolicyNames: []string{"connect-inject-policy-" + secondaryDatacenter}, + Roles: []string{resourcePrefix + "-connect-injector-acl-role-" + secondaryDatacenter}, + GlobalAuthMethod: false, }, } for _, c := range cases { @@ -2365,6 +2312,11 @@ func TestRun_PoliciesAndBindingRulesACLLogin_SecondaryDatacenter(t *testing.T) { responseCode := cmd.Run(cmdArgs) require.Equal(t, 0, responseCode, ui.ErrorWriter.String()) + datacenter := "" + if c.GlobalAuthMethod { + datacenter = primaryDatacenter + } + // Check that the Role exists + has correct Policy and is associated with a BindingRule. for i := range c.Roles { // Check that the Policy exists. @@ -2373,14 +2325,14 @@ func TestRun_PoliciesAndBindingRulesACLLogin_SecondaryDatacenter(t *testing.T) { require.NotNil(t, policy) // Check that the Role exists. - role, _, err := consul.ACL().RoleReadByName(c.Roles[i], &api.QueryOptions{Datacenter: primaryDatacenter}) + role, _, err := consul.ACL().RoleReadByName(c.Roles[i], &api.QueryOptions{Datacenter: datacenter}) require.NoError(t, err) require.NotNil(t, role) // Check that the Role references the Policy. found := false - for x := range role.Policies { - if role.Policies[x].Name == policy.Name { + for j := range role.Policies { + if role.Policies[j].Name == policy.Name { found = true break } @@ -2388,12 +2340,16 @@ func TestRun_PoliciesAndBindingRulesACLLogin_SecondaryDatacenter(t *testing.T) { require.True(t, found) // Check that there exists a BindingRule that references this Role. - rb, _, err := consul.ACL().BindingRuleList(fmt.Sprintf("%s-%s-%s", resourcePrefix, componentAuthMethod, secondaryDatacenter), &api.QueryOptions{Datacenter: primaryDatacenter}) + authMethodName := fmt.Sprintf("%s-%s", resourcePrefix, componentAuthMethod) + if c.GlobalAuthMethod { + authMethodName = fmt.Sprintf("%s-%s-%s", resourcePrefix, componentAuthMethod, secondaryDatacenter) + } + rb, _, err := consul.ACL().BindingRuleList(authMethodName, &api.QueryOptions{Datacenter: datacenter}) require.NoError(t, err) require.NotNil(t, rb) found = false - for x := range rb { - if rb[x].BindName == c.Roles[i] { + for j := range rb { + if rb[j].BindName == c.Roles[i] { found = true break } @@ -2422,6 +2378,12 @@ func TestRun_ValidateLoginToken_PrimaryDatacenter(t *testing.T) { Roles: []string{resourcePrefix + "-controller-acl-role"}, GlobalToken: false, }, + { + ComponentName: "connect-injector", + TokenFlags: []string{"-connect-inject"}, + Roles: []string{resourcePrefix + "-connect-injector-acl-role"}, + GlobalToken: false, + }, } for _, c := range cases { t.Run(c.ComponentName, func(t *testing.T) { @@ -2490,23 +2452,35 @@ func TestRun_ValidateLoginToken_SecondaryDatacenter(t *testing.T) { t.Parallel() cases := []struct { - ComponentName string - TokenFlags []string - Roles []string - GlobalToken bool + ComponentName string + TokenFlags []string + Roles []string + GlobalAuthMethod bool + GlobalToken bool }{ { - ComponentName: "controller", - TokenFlags: []string{"-create-controller-token"}, - Roles: []string{resourcePrefix + "-controller-acl-role-dc2"}, - GlobalToken: true, + ComponentName: "controller", + TokenFlags: []string{"-create-controller-token"}, + Roles: []string{resourcePrefix + "-controller-acl-role-dc2"}, + GlobalAuthMethod: true, + GlobalToken: true, + }, + { + ComponentName: "connect-injector", + TokenFlags: []string{"-connect-inject"}, + Roles: []string{resourcePrefix + "-connect-injector-acl-role-dc2"}, + GlobalAuthMethod: false, + GlobalToken: false, }, } for _, c := range cases { t.Run(c.ComponentName, func(t *testing.T) { bootToken := "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" tokenFile := common.WriteTempFile(t, bootToken) - authMethodName := fmt.Sprintf("%s-%s-%s", resourcePrefix, componentAuthMethod, "dc2") + authMethodName := fmt.Sprintf("%s-%s", resourcePrefix, componentAuthMethod) + if c.GlobalAuthMethod { + authMethodName = fmt.Sprintf("%s-%s-%s", resourcePrefix, componentAuthMethod, "dc2") + } serviceAccountName := fmt.Sprintf("%s-%s", resourcePrefix, c.ComponentName) k8s, _, consulHTTPAddr, cleanup := mockReplicatedSetup(t, bootToken) @@ -2545,9 +2519,13 @@ func TestRun_ValidateLoginToken_SecondaryDatacenter(t *testing.T) { responseCode := cmd.Run(cmdArgs) require.Equal(t, 0, responseCode, ui.ErrorWriter.String()) + datacenter := "" + if c.GlobalAuthMethod { + datacenter = "dc1" + } client, err := api.NewClient(&api.Config{ Address: consulHTTPAddr, - Datacenter: "dc1", + Datacenter: datacenter, }) require.NoError(t, err)