From e50a4d213ffd92d512488c3711440fd18795fd74 Mon Sep 17 00:00:00 2001 From: Ashwin Venkatesh Date: Tue, 8 Mar 2022 18:52:38 -0500 Subject: [PATCH] Sync token acl refactor (#1081) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit • Refactor sync-catalog to use the new auth-method workflow when ACLs are enabled so that Kubernetes secrets are not used. • Create a service account and rolebinding dedicated to the component authmethod so that it no longer piggybacks on the one used by the connect-inject authmethod. --- .../templates/auth-method-clusterrole.yaml | 18 ++ .../auth-method-clusterrolebinding.yaml | 39 ++++ .../templates/auth-method-serviceaccount.yaml | 19 ++ ...-inject-authmethod-clusterrolebinding.yaml | 22 -- .../templates/connect-inject-clusterrole.yaml | 17 +- .../connect-inject-clusterrolebinding.yaml | 2 + .../connect-inject-serviceaccount.yaml | 4 +- .../consul/templates/server-acl-init-job.yaml | 2 +- .../templates/server-acl-init-role.yaml | 38 +-- .../templates/sync-catalog-clusterrole.yaml | 9 - .../templates/sync-catalog-deployment.yaml | 87 +++++-- .../test/unit/auth-method-clusterrole.bats | 20 ++ .../unit/auth-method-clusterrolebinding.bats | 20 ++ .../test/unit/auth-method-serviceaccount.bats | 41 ++++ ...-inject-authmethod-clusterrolebinding.bats | 42 ---- .../test/unit/connect-inject-clusterrole.bats | 27 +++ .../connect-inject-clusterrolebinding.bats | 26 +++ .../unit/connect-inject-serviceaccount.bats | 27 +++ .../consul/test/unit/server-acl-init-job.bats | 4 +- .../test/unit/server-acl-init-role.bats | 5 +- .../test/unit/sync-catalog-clusterrole.bats | 14 -- .../test/unit/sync-catalog-deployment.bats | 218 +++++++++++++++++- .../subcommand/server-acl-init/command.go | 26 ++- .../server-acl-init/command_ent_test.go | 27 ++- .../server-acl-init/command_test.go | 62 ++--- .../server-acl-init/connect_inject.go | 8 +- .../server-acl-init/connect_inject_test.go | 4 +- .../server-acl-init/create_or_update.go | 2 +- .../server-acl-init/create_or_update_test.go | 8 +- 29 files changed, 632 insertions(+), 206 deletions(-) create mode 100644 charts/consul/templates/auth-method-clusterrole.yaml create mode 100644 charts/consul/templates/auth-method-clusterrolebinding.yaml create mode 100644 charts/consul/templates/auth-method-serviceaccount.yaml delete mode 100644 charts/consul/templates/connect-inject-authmethod-clusterrolebinding.yaml create mode 100644 charts/consul/test/unit/auth-method-clusterrole.bats create mode 100644 charts/consul/test/unit/auth-method-clusterrolebinding.bats create mode 100644 charts/consul/test/unit/auth-method-serviceaccount.bats delete mode 100644 charts/consul/test/unit/connect-inject-authmethod-clusterrolebinding.bats diff --git a/charts/consul/templates/auth-method-clusterrole.yaml b/charts/consul/templates/auth-method-clusterrole.yaml new file mode 100644 index 0000000000..6b8f2c5451 --- /dev/null +++ b/charts/consul/templates/auth-method-clusterrole.yaml @@ -0,0 +1,18 @@ +{{- if .Values.global.acls.manageSystemACLs }} +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: {{ template "consul.fullname" . }}-auth-method + labels: + app: {{ template "consul.name" . }} + chart: {{ template "consul.chart" . }} + heritage: {{ .Release.Service }} + release: {{ .Release.Name }} + component: auth-method +rules: +- apiGroups: [ "" ] + resources: + - serviceaccounts + verbs: + - get +{{- end }} diff --git a/charts/consul/templates/auth-method-clusterrolebinding.yaml b/charts/consul/templates/auth-method-clusterrolebinding.yaml new file mode 100644 index 0000000000..9bd6c64113 --- /dev/null +++ b/charts/consul/templates/auth-method-clusterrolebinding.yaml @@ -0,0 +1,39 @@ +{{- if .Values.global.acls.manageSystemACLs }} +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + name: {{ template "consul.fullname" . }}-authdelegator + labels: + app: {{ template "consul.name" . }} + chart: {{ template "consul.chart" . }} + heritage: {{ .Release.Service }} + release: {{ .Release.Name }} + component: auth-method +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: "system:auth-delegator" +subjects: +- kind: ServiceAccount + name: {{ template "consul.fullname" . }}-auth-method + namespace: {{ .Release.Namespace }} +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + name: {{ template "consul.fullname" . }}-auth-method + labels: + app: {{ template "consul.name" . }} + chart: {{ template "consul.chart" . }} + heritage: {{ .Release.Service }} + release: {{ .Release.Name }} + component: auth-method +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: {{ template "consul.fullname" . }}-auth-method +subjects: +- kind: ServiceAccount + name: {{ template "consul.fullname" . }}-auth-method + namespace: {{ .Release.Namespace }} +{{- end }} diff --git a/charts/consul/templates/auth-method-serviceaccount.yaml b/charts/consul/templates/auth-method-serviceaccount.yaml new file mode 100644 index 0000000000..098339b8c8 --- /dev/null +++ b/charts/consul/templates/auth-method-serviceaccount.yaml @@ -0,0 +1,19 @@ +{{- if .Values.global.acls.manageSystemACLs }} +apiVersion: v1 +kind: ServiceAccount +metadata: + name: {{ template "consul.fullname" . }}-auth-method + namespace: {{ .Release.Namespace }} + labels: + app: {{ template "consul.name" . }} + chart: {{ template "consul.chart" . }} + heritage: {{ .Release.Service }} + release: {{ .Release.Name }} + component: auth-method +{{- with .Values.global.imagePullSecrets }} +imagePullSecrets: +{{- range . }} +- name: {{ .name }} +{{- end }} +{{- end }} +{{- end }} diff --git a/charts/consul/templates/connect-inject-authmethod-clusterrolebinding.yaml b/charts/consul/templates/connect-inject-authmethod-clusterrolebinding.yaml deleted file mode 100644 index 4f9d7c8083..0000000000 --- a/charts/consul/templates/connect-inject-authmethod-clusterrolebinding.yaml +++ /dev/null @@ -1,22 +0,0 @@ -{{- if or (and (ne (.Values.connectInject.enabled | toString) "-") .Values.connectInject.enabled) (and (eq (.Values.connectInject.enabled | toString) "-") .Values.global.enabled) }} -{{- if .Values.global.acls.manageSystemACLs }} -apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRoleBinding -metadata: - name: {{ template "consul.fullname" . }}-connect-injector-authdelegator - labels: - app: {{ template "consul.name" . }} - chart: {{ template "consul.chart" . }} - heritage: {{ .Release.Service }} - release: {{ .Release.Name }} - component: connect-injector -roleRef: - apiGroup: rbac.authorization.k8s.io - kind: ClusterRole - name: "system:auth-delegator" -subjects: - - kind: ServiceAccount - name: {{ template "consul.fullname" . }}-connect-injector - namespace: {{ .Release.Namespace }} -{{- end }} -{{- end }} diff --git a/charts/consul/templates/connect-inject-clusterrole.yaml b/charts/consul/templates/connect-inject-clusterrole.yaml index 3a2eb62829..683a9c6bf7 100644 --- a/charts/consul/templates/connect-inject-clusterrole.yaml +++ b/charts/consul/templates/connect-inject-clusterrole.yaml @@ -1,3 +1,4 @@ +{{- if or (and (ne (.Values.connectInject.enabled | toString) "-") .Values.connectInject.enabled) (and (eq (.Values.connectInject.enabled | toString) "-") .Values.global.enabled) }} # The ClusterRole to enable the Connect injector to get, list, watch and patch MutatingWebhookConfiguration. apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole @@ -11,14 +12,14 @@ metadata: component: connect-injector rules: {{- if .Values.global.acls.manageSystemACLs }} -- apiGroups: [""] +- apiGroups: [ "" ] resources: - serviceaccounts verbs: - get {{- end }} -- apiGroups: [""] - resources: ["pods", "endpoints", "services", "namespaces"] +- apiGroups: [ "" ] + resources: [ "pods", "endpoints", "services", "namespaces" ] verbs: - "get" - "list" @@ -33,17 +34,11 @@ rules: - list - update {{- if .Values.global.enablePodSecurityPolicies }} -- apiGroups: ["policy"] - resources: ["podsecuritypolicies"] +- apiGroups: [ "policy" ] + resources: [ "podsecuritypolicies" ] resourceNames: - {{ template "consul.fullname" . }}-connect-injector verbs: - use {{- end }} -{{- if .Values.global.acls.manageSystemACLs }} -- apiGroups: [""] - resources: - - serviceaccounts - verbs: - - get {{- end }} diff --git a/charts/consul/templates/connect-inject-clusterrolebinding.yaml b/charts/consul/templates/connect-inject-clusterrolebinding.yaml index 3a9d041852..c380adb311 100644 --- a/charts/consul/templates/connect-inject-clusterrolebinding.yaml +++ b/charts/consul/templates/connect-inject-clusterrolebinding.yaml @@ -1,3 +1,4 @@ +{{- if or (and (ne (.Values.connectInject.enabled | toString) "-") .Values.connectInject.enabled) (and (eq (.Values.connectInject.enabled | toString) "-") .Values.global.enabled) }} apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRoleBinding metadata: @@ -16,3 +17,4 @@ subjects: - kind: ServiceAccount name: {{ template "consul.fullname" . }}-connect-injector namespace: {{ .Release.Namespace }} +{{- end }} \ No newline at end of file diff --git a/charts/consul/templates/connect-inject-serviceaccount.yaml b/charts/consul/templates/connect-inject-serviceaccount.yaml index c95136914a..ea2352c7ac 100644 --- a/charts/consul/templates/connect-inject-serviceaccount.yaml +++ b/charts/consul/templates/connect-inject-serviceaccount.yaml @@ -1,3 +1,4 @@ +{{- if or (and (ne (.Values.connectInject.enabled | toString) "-") .Values.connectInject.enabled) (and (eq (.Values.connectInject.enabled | toString) "-") .Values.global.enabled) }} apiVersion: v1 kind: ServiceAccount metadata: @@ -16,6 +17,7 @@ metadata: {{- with .Values.global.imagePullSecrets }} imagePullSecrets: {{- range . }} - - name: {{ .name }} +- name: {{ .name }} +{{- end }} {{- end }} {{- end }} diff --git a/charts/consul/templates/server-acl-init-job.yaml b/charts/consul/templates/server-acl-init-job.yaml index 97916c495c..04f4e2de3e 100644 --- a/charts/consul/templates/server-acl-init-job.yaml +++ b/charts/consul/templates/server-acl-init-job.yaml @@ -169,7 +169,7 @@ spec: {{- end }} {{- if .Values.syncCatalog.enabled }} - -create-sync-token=true \ + -sync-catalog=true \ {{- if .Values.syncCatalog.consulNodeName }} -sync-consul-node-name={{ .Values.syncCatalog.consulNodeName }} \ {{- end }} diff --git a/charts/consul/templates/server-acl-init-role.yaml b/charts/consul/templates/server-acl-init-role.yaml index 43d610f7a7..eb7b6a928e 100644 --- a/charts/consul/templates/server-acl-init-role.yaml +++ b/charts/consul/templates/server-acl-init-role.yaml @@ -13,26 +13,26 @@ metadata: release: {{ .Release.Name }} component: server-acl-init rules: - - apiGroups: [""] - resources: - - secrets - verbs: - - create - - get - - apiGroups: [""] - resources: - - serviceaccounts - resourceNames: - - {{ template "consul.fullname" . }}-connect-injector - verbs: - - get +- apiGroups: [ "" ] + resources: + - secrets + verbs: + - create + - get +- apiGroups: [ "" ] + resources: + - serviceaccounts + resourceNames: + - {{ template "consul.fullname" . }}-auth-method + verbs: + - get {{- if .Values.global.enablePodSecurityPolicies }} - - apiGroups: ["policy"] - resources: ["podsecuritypolicies"] - resourceNames: - - {{ template "consul.fullname" . }}-server-acl-init - verbs: - - use +- apiGroups: [ "policy" ] + resources: [ "podsecuritypolicies" ] + resourceNames: + - {{ template "consul.fullname" . }}-server-acl-init + verbs: + - use {{- end }} {{- end }} {{- end }} diff --git a/charts/consul/templates/sync-catalog-clusterrole.yaml b/charts/consul/templates/sync-catalog-clusterrole.yaml index 5ceeb03d47..0b0837c0df 100644 --- a/charts/consul/templates/sync-catalog-clusterrole.yaml +++ b/charts/consul/templates/sync-catalog-clusterrole.yaml @@ -30,15 +30,6 @@ rules: - nodes verbs: - get -{{- if .Values.global.acls.manageSystemACLs }} - - apiGroups: [""] - resources: - - secrets - resourceNames: - - {{ template "consul.fullname" . }}-catalog-sync-acl-token - verbs: - - get -{{- end }} {{- if .Values.global.enablePodSecurityPolicies }} - apiGroups: ["policy"] resources: ["podsecuritypolicies"] diff --git a/charts/consul/templates/sync-catalog-deployment.yaml b/charts/consul/templates/sync-catalog-deployment.yaml index 2aedc54460..4fd94baf3f 100644 --- a/charts/consul/templates/sync-catalog-deployment.yaml +++ b/charts/consul/templates/sync-catalog-deployment.yaml @@ -49,8 +49,11 @@ spec: {{- end }} spec: serviceAccountName: {{ template "consul.fullname" . }}-sync-catalog - {{- if .Values.global.tls.enabled }} volumes: + - name: consul-data + emptyDir: + medium: "Memory" + {{- if .Values.global.tls.enabled }} {{- if not (and .Values.externalServers.enabled .Values.externalServers.useSystemRoots) }} - name: consul-ca-cert secret: @@ -70,9 +73,13 @@ spec: {{- end }} {{- end }} containers: - - name: consul-sync-catalog + - name: sync-catalog image: "{{ default .Values.global.imageK8S .Values.syncCatalog.image }}" env: + {{- if .Values.global.acls.manageSystemACLs }} + - name: CONSUL_HTTP_TOKEN_FILE + value: "/consul/login/acl-token" + {{- end }} - name: HOST_IP valueFrom: fieldRef: @@ -88,13 +95,6 @@ spec: name: {{ .Values.syncCatalog.aclSyncToken.secretName }} key: {{ .Values.syncCatalog.aclSyncToken.secretKey }} {{- end }} - {{- if .Values.global.acls.manageSystemACLs }} - - name: CONSUL_HTTP_TOKEN - valueFrom: - secretKeyRef: - name: "{{ template "consul.fullname" . }}-catalog-sync-acl-token" - key: "token" - {{- end}} {{- if .Values.global.tls.enabled }} {{- if .Values.client.enabled }} - name: CONSUL_HTTP_ADDR @@ -114,16 +114,19 @@ spec: value: http://{{ template "consul.fullname" . }}-server:8500 {{- end }} {{- end }} - {{- if .Values.global.tls.enabled }} volumeMounts: - {{- if (and .Values.global.tls.enableAutoEncrypt $clientEnabled) }} + - mountPath: /consul/login + name: consul-data + readOnly: true + {{- if .Values.global.tls.enabled }} + {{- if and .Values.global.tls.enableAutoEncrypt $clientEnabled }} - name: consul-auto-encrypt-ca-cert {{- else }} - name: consul-ca-cert + {{- end }} {{- end }} mountPath: /consul/tls/ca readOnly: true - {{- end }} command: - "/bin/sh" - "-ec" @@ -188,6 +191,16 @@ spec: -consul-cross-namespace-acl-policy=cross-namespace-policy \ {{- end }} {{- end }} + {{- if .Values.global.acls.manageSystemACLs }} + lifecycle: + preStop: + exec: + command: + - "/bin/sh" + - "-ec" + - | + consul-k8s-control-plane consul-logout + {{- end }} livenessProbe: httpGet: path: /health/ready @@ -214,16 +227,57 @@ spec: {{- end }} {{- if or .Values.global.acls.manageSystemACLs (and .Values.global.tls.enabled .Values.global.tls.enableAutoEncrypt $clientEnabled) }} initContainers: + {{- if (and .Values.global.tls.enabled .Values.global.tls.enableAutoEncrypt $clientEnabled) }} + {{- include "consul.getAutoEncryptClientCA" . | nindent 6 }} + {{- end }} {{- if .Values.global.acls.manageSystemACLs }} - - name: sync-acl-init + - name: sync-catalog-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" . }}-catalog-sync-acl-token" \ - -k8s-namespace={{ .Release.Namespace }} + -component-name=sync-catalog \ + {{- 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" @@ -232,9 +286,6 @@ spec: memory: "25Mi" cpu: "50m" {{- end }} - {{- if (and .Values.global.tls.enabled .Values.global.tls.enableAutoEncrypt $clientEnabled) }} - {{- include "consul.getAutoEncryptClientCA" . | nindent 6 }} - {{- end }} {{- end }} {{- if .Values.syncCatalog.priorityClassName }} priorityClassName: {{ .Values.syncCatalog.priorityClassName | quote }} diff --git a/charts/consul/test/unit/auth-method-clusterrole.bats b/charts/consul/test/unit/auth-method-clusterrole.bats new file mode 100644 index 0000000000..935a448161 --- /dev/null +++ b/charts/consul/test/unit/auth-method-clusterrole.bats @@ -0,0 +1,20 @@ +#!/usr/bin/env bats + +load _helpers + +@test "auth-method/ClusterRole: disabled by default" { + cd `chart_dir` + assert_empty helm template \ + -s templates/auth-method-clusterrole.yaml \ + . +} + +@test "auth-method/ClusterRole: enabled with global.acls.manageSystemACLs true" { + cd `chart_dir` + local actual=$(helm template \ + -s templates/auth-method-clusterrole.yaml \ + --set 'global.acls.manageSystemACLs=true' \ + . | tee /dev/stderr | + yq -s 'length > 0' | tee /dev/stderr) + [ "${actual}" = "true" ] +} \ No newline at end of file diff --git a/charts/consul/test/unit/auth-method-clusterrolebinding.bats b/charts/consul/test/unit/auth-method-clusterrolebinding.bats new file mode 100644 index 0000000000..dcb293ba14 --- /dev/null +++ b/charts/consul/test/unit/auth-method-clusterrolebinding.bats @@ -0,0 +1,20 @@ +#!/usr/bin/env bats + +load _helpers + +@test "auth-method/ClusterRoleBinding: disabled by default" { + cd `chart_dir` + assert_empty helm template \ + -s templates/auth-method-clusterrolebinding.yaml \ + . +} + +@test "auth-method/ClusterRoleBinding: enabled with global.acls.manageSystemACLs true" { + cd `chart_dir` + local actual=$(helm template \ + -s templates/auth-method-clusterrolebinding.yaml \ + --set 'global.acls.manageSystemACLs=true' \ + . | tee /dev/stderr | + yq -s 'length > 0' | tee /dev/stderr) + [ "${actual}" = "true" ] +} \ No newline at end of file diff --git a/charts/consul/test/unit/auth-method-serviceaccount.bats b/charts/consul/test/unit/auth-method-serviceaccount.bats new file mode 100644 index 0000000000..9413a03291 --- /dev/null +++ b/charts/consul/test/unit/auth-method-serviceaccount.bats @@ -0,0 +1,41 @@ +#!/usr/bin/env bats + +load _helpers + +@test "auth-method/ServiceAccount: disabled by default" { + cd `chart_dir` + assert_empty helm template \ + -s templates/auth-method-serviceaccount.yaml \ + . +} + +@test "auth-method/ServiceAccount: enabled with global.acls.manageSystemACLs.enabled true" { + cd `chart_dir` + local actual=$(helm template \ + -s templates/auth-method-serviceaccount.yaml \ + --set 'global.acls.manageSystemACLs=true' \ + . | tee /dev/stderr | + yq -s 'length > 0' | tee /dev/stderr) + [ "${actual}" = "true" ] +} + +#-------------------------------------------------------------------- +# global.imagePullSecrets + +@test "auth-method/ServiceAccount: can set image pull secrets" { + cd `chart_dir` + local object=$(helm template \ + -s templates/auth-method-serviceaccount.yaml \ + --set 'global.acls.manageSystemACLs=true' \ + --set 'global.imagePullSecrets[0].name=my-secret' \ + --set 'global.imagePullSecrets[1].name=my-secret2' \ + . | tee /dev/stderr) + + local actual=$(echo "$object" | + yq -r '.imagePullSecrets[0].name' | tee /dev/stderr) + [ "${actual}" = "my-secret" ] + + local actual=$(echo "$object" | + yq -r '.imagePullSecrets[1].name' | tee /dev/stderr) + [ "${actual}" = "my-secret2" ] +} diff --git a/charts/consul/test/unit/connect-inject-authmethod-clusterrolebinding.bats b/charts/consul/test/unit/connect-inject-authmethod-clusterrolebinding.bats deleted file mode 100644 index 7a70293f16..0000000000 --- a/charts/consul/test/unit/connect-inject-authmethod-clusterrolebinding.bats +++ /dev/null @@ -1,42 +0,0 @@ -#!/usr/bin/env bats - -load _helpers - -@test "connectInjectAuthMethod/ClusterRoleBinding: disabled by default" { - cd `chart_dir` - assert_empty helm template \ - -s templates/connect-inject-authmethod-clusterrolebinding.yaml \ - . -} - -@test "connectInjectAuthMethod/ClusterRoleBinding: enabled with global.enabled false" { - cd `chart_dir` - local actual=$(helm template \ - -s templates/connect-inject-authmethod-clusterrolebinding.yaml \ - --set 'global.enabled=false' \ - --set 'client.enabled=true' \ - --set 'connectInject.enabled=true' \ - --set 'global.acls.manageSystemACLs=true' \ - . | tee /dev/stderr | - yq -s 'length > 0' | tee /dev/stderr) - [ "${actual}" = "true" ] -} - -@test "connectInjectAuthMethod/ClusterRoleBinding: disabled with connectInject.enabled" { - cd `chart_dir` - assert_empty helm template \ - -s templates/connect-inject-authmethod-clusterrolebinding.yaml \ - --set 'connectInject.enabled=true' \ - . -} - -@test "connectInjectAuthMethod/ClusterRoleBinding: enabled with global.acls.manageSystemACLs.enabled=true" { - cd `chart_dir` - local actual=$(helm template \ - -s templates/connect-inject-authmethod-clusterrolebinding.yaml \ - --set 'connectInject.enabled=true' \ - --set 'global.acls.manageSystemACLs=true' \ - . | tee /dev/stderr | - yq -s 'length > 0' | tee /dev/stderr) - [ "${actual}" = "true" ] -} diff --git a/charts/consul/test/unit/connect-inject-clusterrole.bats b/charts/consul/test/unit/connect-inject-clusterrole.bats index c7ff654cf2..e954b8908a 100644 --- a/charts/consul/test/unit/connect-inject-clusterrole.bats +++ b/charts/consul/test/unit/connect-inject-clusterrole.bats @@ -2,6 +2,33 @@ load _helpers +@test "connectInject/ClusterRole: disabled by default" { + cd `chart_dir` + assert_empty helm template \ + -s templates/connect-inject-clusterrole.yaml \ + . +} + +@test "connectInject/ClusterRole: enabled with global.enabled false" { + cd `chart_dir` + local actual=$(helm template \ + -s templates/connect-inject-clusterrole.yaml \ + --set 'global.enabled=false' \ + --set 'client.enabled=true' \ + --set 'connectInject.enabled=true' \ + . | tee /dev/stderr | + yq -s 'length > 0' | tee /dev/stderr) + [ "${actual}" = "true" ] +} + +@test "connectInject/ClusterRole: disabled with connectInject.enabled" { + cd `chart_dir` + assert_empty helm template \ + -s templates/connect-inject-clusterrole.yaml \ + --set 'connectInject.enabled=false' \ + . +} + #-------------------------------------------------------------------- # global.enablePodSecurityPolicies diff --git a/charts/consul/test/unit/connect-inject-clusterrolebinding.bats b/charts/consul/test/unit/connect-inject-clusterrolebinding.bats index 6900cb2bd5..ccf30083f9 100644 --- a/charts/consul/test/unit/connect-inject-clusterrolebinding.bats +++ b/charts/consul/test/unit/connect-inject-clusterrolebinding.bats @@ -2,3 +2,29 @@ load _helpers +@test "connectInject/ClusterRoleBinding: disabled by default" { + cd `chart_dir` + assert_empty helm template \ + -s templates/connect-inject-clusterrolebinding.yaml \ + . +} + +@test "connectInject/ClusterRoleBinding: enabled with global.enabled false" { + cd `chart_dir` + local actual=$(helm template \ + -s templates/connect-inject-clusterrolebinding.yaml \ + --set 'global.enabled=false' \ + --set 'client.enabled=true' \ + --set 'connectInject.enabled=true' \ + . | tee /dev/stderr | + yq -s 'length > 0' | tee /dev/stderr) + [ "${actual}" = "true" ] +} + +@test "connectInject/ClusterRoleBinding: disabled with connectInject.enabled false" { + cd `chart_dir` + assert_empty helm template \ + -s templates/connect-inject-clusterrolebinding.yaml \ + --set 'connectInject.enabled=false' \ + . +} \ No newline at end of file diff --git a/charts/consul/test/unit/connect-inject-serviceaccount.bats b/charts/consul/test/unit/connect-inject-serviceaccount.bats index 4c5fb8e316..07b38c3d49 100644 --- a/charts/consul/test/unit/connect-inject-serviceaccount.bats +++ b/charts/consul/test/unit/connect-inject-serviceaccount.bats @@ -2,6 +2,33 @@ load _helpers +@test "connectInject/ServiceAccount: disabled by default" { + cd `chart_dir` + assert_empty helm template \ + -s templates/connect-inject-serviceaccount.yaml \ + . +} + +@test "connectInject/ServiceAccount: enabled with global.enabled false" { + cd `chart_dir` + local actual=$(helm template \ + -s templates/connect-inject-serviceaccount.yaml \ + --set 'global.enabled=false' \ + --set 'client.enabled=true' \ + --set 'connectInject.enabled=true' \ + . | tee /dev/stderr | + yq -s 'length > 0' | tee /dev/stderr) + [ "${actual}" = "true" ] +} + +@test "connectInject/ServiceAccount: disabled with connectInject.enabled false" { + cd `chart_dir` + assert_empty helm template \ + -s templates/connect-inject-serviceaccount.yaml \ + --set 'connectInject.enabled=false' \ + . +} + #-------------------------------------------------------------------- # global.imagePullSecrets diff --git a/charts/consul/test/unit/server-acl-init-job.bats b/charts/consul/test/unit/server-acl-init-job.bats index f982c47c28..5cbd5a7b8f 100644 --- a/charts/consul/test/unit/server-acl-init-job.bats +++ b/charts/consul/test/unit/server-acl-init-job.bats @@ -249,7 +249,7 @@ load _helpers -s templates/server-acl-init-job.yaml \ --set 'global.acls.manageSystemACLs=true' \ . | tee /dev/stderr | - yq '.spec.template.spec.containers[0].command | any(contains("-create-sync-token"))' | tee /dev/stderr) + yq '.spec.template.spec.containers[0].command | any(contains("-sync-catalog"))' | tee /dev/stderr) [ "${actual}" = "false" ] } @@ -260,7 +260,7 @@ load _helpers --set 'global.acls.manageSystemACLs=true' \ --set 'syncCatalog.enabled=true' \ . | tee /dev/stderr | - yq '.spec.template.spec.containers[0].command | any(contains("-create-sync-token"))' | tee /dev/stderr) + yq '.spec.template.spec.containers[0].command | any(contains("-sync-catalog"))' | tee /dev/stderr) [ "${actual}" = "true" ] } diff --git a/charts/consul/test/unit/server-acl-init-role.bats b/charts/consul/test/unit/server-acl-init-role.bats index 9d8d8f4573..92cdd78d16 100644 --- a/charts/consul/test/unit/server-acl-init-role.bats +++ b/charts/consul/test/unit/server-acl-init-role.bats @@ -53,14 +53,13 @@ load _helpers } #-------------------------------------------------------------------- -# connectInject.enabled +# manageSystemACLs.enabled -@test "serverACLInit/Role: allows service accounts when connectInject.enabled is true" { +@test "serverACLInit/Role: allows service accounts when manageSystemACLs.enabled is true" { cd `chart_dir` local actual=$(helm template \ -s templates/server-acl-init-role.yaml \ --set 'global.acls.manageSystemACLs=true' \ - --set 'connectInject.enabled=true' \ . | tee /dev/stderr | yq -r '.rules | map(select(.resources[0] == "serviceaccounts")) | length' | tee /dev/stderr) [ "${actual}" = "1" ] diff --git a/charts/consul/test/unit/sync-catalog-clusterrole.bats b/charts/consul/test/unit/sync-catalog-clusterrole.bats index 0688db9b93..17141e434f 100755 --- a/charts/consul/test/unit/sync-catalog-clusterrole.bats +++ b/charts/consul/test/unit/sync-catalog-clusterrole.bats @@ -60,20 +60,6 @@ load _helpers [ "${actual}" = "podsecuritypolicies" ] } -#-------------------------------------------------------------------- -# global.acls.manageSystemACLs - -@test "syncCatalog/ClusterRole: allows secret access with global.acls.manageSystemACLs=true" { - cd `chart_dir` - local actual=$(helm template \ - -s templates/sync-catalog-clusterrole.yaml \ - --set 'syncCatalog.enabled=true' \ - --set 'global.acls.manageSystemACLs=true' \ - . | tee /dev/stderr | - yq -r '.rules[2].resources[0]' | tee /dev/stderr) - [ "${actual}" = "secrets" ] -} - #-------------------------------------------------------------------- # syncCatalog.toK8S={true,false} diff --git a/charts/consul/test/unit/sync-catalog-deployment.bats b/charts/consul/test/unit/sync-catalog-deployment.bats index 8beead1564..18171942af 100755 --- a/charts/consul/test/unit/sync-catalog-deployment.bats +++ b/charts/consul/test/unit/sync-catalog-deployment.bats @@ -421,33 +421,243 @@ load _helpers #-------------------------------------------------------------------- # global.acls.manageSystemACLs -@test "syncCatalog/Deployment: CONSUL_HTTP_TOKEN env variable created when global.acls.manageSystemACLs=true" { +@test "syncCatalog/Deployment: consul-logout preStop hook is added when ACLs are enabled" { cd `chart_dir` local actual=$(helm template \ -s templates/sync-catalog-deployment.yaml \ --set 'syncCatalog.enabled=true' \ --set 'global.acls.manageSystemACLs=true' \ . | tee /dev/stderr | - yq '[.spec.template.spec.containers[0].env[].name] | any(contains("CONSUL_HTTP_TOKEN"))' | 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) + + [ "${actual}" = "true" ] +} + +@test "syncCatalog/Deployment: CONSUL_HTTP_TOKEN_FILE is not set when acls are disabled" { + cd `chart_dir` + local actual=$(helm template \ + -s templates/sync-catalog-deployment.yaml \ + --set 'syncCatalog.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 "syncCatalog/Deployment: CONSUL_HTTP_TOKEN_FILE is set when acls are enabled" { + cd `chart_dir` + local actual=$(helm template \ + -s templates/sync-catalog-deployment.yaml \ + --set 'syncCatalog.enabled=true' \ + --set 'global.acls.manageSystemACLs=true' \ + . | tee /dev/stderr | + yq '[.spec.template.spec.containers[0].env[0].name] | any(contains("CONSUL_HTTP_TOKEN_FILE"))' | tee /dev/stderr) + [ "${actual}" = "true" ] +} + +@test "syncCatalog/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/sync-catalog-deployment.yaml \ + --set 'syncCatalog.enabled=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}" = "sync-catalog-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 '[.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 "syncCatalog/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/sync-catalog-deployment.yaml \ + --set 'syncCatalog.enabled=true' \ + --set 'global.tls.enabled=true' \ + --set 'global.acls.manageSystemACLs=true' \ + . | tee /dev/stderr | + yq '.spec.template.spec.initContainers[] | select(.name == "sync-catalog-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-ca-cert"))' | tee /dev/stderr) + [ "${actual}" = "true" ] +} + +@test "syncCatalog/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/sync-catalog-deployment.yaml \ + --set 'syncCatalog.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 == "sync-catalog-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 "syncCatalog/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/sync-catalog-deployment.yaml \ + --set 'syncCatalog.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 == "sync-catalog-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 "syncCatalog/Deployment: init container is created when global.acls.manageSystemACLs=true" { +@test "syncCatalog/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/sync-catalog-deployment.yaml \ --set 'syncCatalog.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}" = "sync-acl-init" ] + [ "${actual}" = "get-auto-encrypt-client-ca" ] +} + +@test "syncCatalog/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/sync-catalog-deployment.yaml \ + --set 'syncCatalog.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 == "sync-catalog-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 "syncCatalog/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/sync-catalog-deployment.yaml \ + --set 'syncCatalog.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 == "sync-catalog-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" ] } #-------------------------------------------------------------------- diff --git a/control-plane/subcommand/server-acl-init/command.go b/control-plane/subcommand/server-acl-init/command.go index bb729b5137..b98a84e859 100644 --- a/control-plane/subcommand/server-acl-init/command.go +++ b/control-plane/subcommand/server-acl-init/command.go @@ -42,7 +42,7 @@ type Command struct { flagCreateClientToken bool - flagCreateSyncToken bool + flagSyncCatalog bool flagSyncConsulNodeName string flagConnectInject bool @@ -126,8 +126,8 @@ func (c *Command) init() { c.flags.BoolVar(&c.flagCreateClientToken, "create-client-token", true, "Toggle for creating a client agent token. Default is true.") - c.flags.BoolVar(&c.flagCreateSyncToken, "create-sync-token", false, - "Toggle for creating a catalog sync token.") + c.flags.BoolVar(&c.flagSyncCatalog, "sync-catalog", false, + "Toggle for creating a catalog sync policy.") c.flags.StringVar(&c.flagSyncConsulNodeName, "sync-consul-node-name", "k8s-sync", "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.") @@ -481,19 +481,27 @@ func (c *Command) Run(args []string) int { } } - if c.flagCreateSyncToken { + if c.flagSyncCatalog { syncRules, err := c.syncRules() if err != nil { c.log.Error("Error templating sync rules", "err", err) return 1 } - // If namespaces are enabled, the policy and token needs to be global - // to be allowed to create namespaces. + serviceAccountName := c.withPrefix("sync-catalog") + 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("catalog-sync", syncRules, consulDC, primary, consulClient) + // Create the catalog sync ACL Policy, Role and BindingRule. + // SyncCatalog 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("sync-catalog", syncRules, consulDC, primaryDC, globalPolicy, primary, componentAuthMethodName, serviceAccountName, consulClient) } else { - err = c.createLocalACL("catalog-sync", syncRules, consulDC, primary, consulClient) + err = c.createACLPolicyRoleAndBindingRule("sync-catalog", syncRules, consulDC, primaryDC, localPolicy, primary, componentAuthMethodName, serviceAccountName, consulClient) } if err != nil { c.log.Error(err.Error()) @@ -522,7 +530,7 @@ func (c *Command) Run(args []string) int { // If namespaces are enabled, the policy and token need to be global // to be allowed to create namespaces. if c.flagEnableNamespaces { - // Create the controller ACL Policy, Role and BindingRule but do not issue any ACLTokens or create Kube Secrets. + // Create the connect-inject 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 { 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 5f0c4f8da5..a7e3f71e51 100644 --- a/control-plane/subcommand/server-acl-init/command_ent_test.go +++ b/control-plane/subcommand/server-acl-init/command_ent_test.go @@ -287,7 +287,7 @@ func TestRun_ACLPolicyUpdates(t *testing.T) { "-create-client-token", "-allow-dns", "-create-mesh-gateway-token", - "-create-sync-token", + "-sync-catalog", "-connect-inject", "-create-snapshot-agent-token", "-create-enterprise-license-token", @@ -325,7 +325,7 @@ func TestRun_ACLPolicyUpdates(t *testing.T) { firstRunExpectedPolicies := []string{ "anonymous-token-policy", "client-token", - "catalog-sync-token", + "sync-catalog-policy", "mesh-gateway-token", "client-snapshot-agent-token", "enterprise-license-token", @@ -376,7 +376,7 @@ func TestRun_ACLPolicyUpdates(t *testing.T) { secondRunExpectedPolicies := []string{ "anonymous-token-policy", "client-token", - "catalog-sync-token", + "sync-catalog-policy", "connect-inject-policy", "mesh-gateway-token", "client-snapshot-agent-token", @@ -675,13 +675,6 @@ func TestRun_TokensWithNamespacesEnabled(t *testing.T) { SecretNames: []string{resourcePrefix + "-client-acl-token"}, LocalToken: true, }, - "catalog-sync token": { - TokenFlags: []string{"-create-sync-token"}, - PolicyNames: []string{"catalog-sync-token"}, - PolicyDCs: nil, - SecretNames: []string{resourcePrefix + "-catalog-sync-acl-token"}, - LocalToken: false, - }, "enterprise-license token": { TokenFlags: []string{"-create-enterprise-license-token"}, PolicyNames: []string{"enterprise-license-token"}, @@ -1078,6 +1071,13 @@ func TestRun_NamespaceEnabled_ValidateLoginToken_PrimaryDatacenter(t *testing.T) Namespace: ns, GlobalToken: false, }, + { + ComponentName: "sync-catalog", + TokenFlags: []string{"-sync-catalog"}, + Roles: []string{resourcePrefix + "-sync-catalog-acl-role"}, + Namespace: ns, + GlobalToken: false, + }, } for _, c := range cases { t.Run(c.ComponentName, func(t *testing.T) { @@ -1162,6 +1162,13 @@ func TestRun_NamespaceEnabled_ValidateLoginToken_SecondaryDatacenter(t *testing. Namespace: ns, GlobalToken: true, }, + { + ComponentName: "sync-catalog", + TokenFlags: []string{"-sync-catalog"}, + Roles: []string{resourcePrefix + "-sync-catalog-acl-role-dc2"}, + Namespace: ns, + GlobalToken: true, + }, } for _, c := range cases { t.Run(c.ComponentName, func(t *testing.T) { diff --git a/control-plane/subcommand/server-acl-init/command_test.go b/control-plane/subcommand/server-acl-init/command_test.go index bfd2a82d67..50dd929fe9 100644 --- a/control-plane/subcommand/server-acl-init/command_test.go +++ b/control-plane/subcommand/server-acl-init/command_test.go @@ -172,14 +172,6 @@ func TestRun_TokensPrimaryDC(t *testing.T) { SecretNames: []string{resourcePrefix + "-client-acl-token"}, LocalToken: true, }, - { - TestName: "Sync token", - TokenFlags: []string{"-create-sync-token"}, - PolicyNames: []string{"catalog-sync-token"}, - PolicyDCs: []string{"dc1"}, - SecretNames: []string{resourcePrefix + "-catalog-sync-acl-token"}, - LocalToken: true, - }, { TestName: "Enterprise license token", TokenFlags: []string{"-create-enterprise-license-token"}, @@ -398,14 +390,6 @@ func TestRun_TokensReplicatedDC(t *testing.T) { SecretNames: []string{resourcePrefix + "-client-acl-token"}, LocalToken: true, }, - { - TestName: "Sync token", - TokenFlags: []string{"-create-sync-token"}, - PolicyNames: []string{"catalog-sync-token-dc2"}, - PolicyDCs: []string{"dc2"}, - SecretNames: []string{resourcePrefix + "-catalog-sync-acl-token"}, - LocalToken: true, - }, { TestName: "Enterprise license token", TokenFlags: []string{"-create-enterprise-license-token"}, @@ -536,12 +520,6 @@ func TestRun_TokensWithProvidedBootstrapToken(t *testing.T) { PolicyNames: []string{"client-token"}, SecretNames: []string{resourcePrefix + "-client-acl-token"}, }, - { - TestName: "Sync token", - TokenFlags: []string{"-create-sync-token"}, - PolicyNames: []string{"catalog-sync-token"}, - SecretNames: []string{resourcePrefix + "-catalog-sync-acl-token"}, - }, { TestName: "Enterprise license token", TokenFlags: []string{"-create-enterprise-license-token"}, @@ -1035,7 +1013,7 @@ func TestRun_SyncPolicyUpdates(t *testing.T) { "-k8s-namespace=" + ns, "-server-address", strings.Split(testSvr.HTTPAddr, ":")[0], "-server-port", strings.Split(testSvr.HTTPAddr, ":")[1], - "-create-sync-token", + "-sync-catalog", } firstRunArgs := append(commonArgs, "-sync-consul-node-name=k8s-sync", @@ -1066,7 +1044,7 @@ func TestRun_SyncPolicyUpdates(t *testing.T) { require.NoError(t, err) for _, p := range firstPolicies { - if p.Name == "catalog-sync-token" { + if p.Name == "sync-catalog-policy" { policy, _, err := consul.ACL().PolicyRead(p.ID, nil) require.NoError(t, err) @@ -1089,7 +1067,7 @@ func TestRun_SyncPolicyUpdates(t *testing.T) { require.NoError(t, err) for _, p := range secondPolicies { - if p.Name == "catalog-sync-token" { + if p.Name == "sync-catalog-policy" { policy, _, err := consul.ACL().PolicyRead(p.ID, nil) require.NoError(t, err) @@ -1125,7 +1103,7 @@ func TestRun_ErrorsOnDuplicateACLPolicy(t *testing.T) { // Create the policy manually. description := "not the expected description" policy, _, err := consul.ACL().PolicyCreate(&api.ACLPolicy{ - Name: "catalog-sync-token", + Name: "sync-catalog-policy", Description: description, }, nil) require.NoError(t, err) @@ -1144,7 +1122,7 @@ func TestRun_ErrorsOnDuplicateACLPolicy(t *testing.T) { "-k8s-namespace=" + ns, "-server-address", strings.Split(testAgent.HTTPAddr, ":")[0], "-server-port", strings.Split(testAgent.HTTPAddr, ":")[1], - "-create-sync-token", + "-sync-catalog", } responseCode := cmd.Run(cmdArgs) @@ -2179,6 +2157,12 @@ func TestRun_PoliciesAndBindingRulesForACLLogin_PrimaryDatacenter(t *testing.T) PolicyNames: []string{"connect-inject-policy"}, Roles: []string{resourcePrefix + "-connect-injector-acl-role"}, }, + { + TestName: "Sync Catalog", + TokenFlags: []string{"-sync-catalog"}, + PolicyNames: []string{"sync-catalog-policy"}, + Roles: []string{resourcePrefix + "-sync-catalog-acl-role"}, + }, } for _, c := range cases { t.Run(c.TestName, func(t *testing.T) { @@ -2283,6 +2267,13 @@ func TestRun_PoliciesAndBindingRulesACLLogin_SecondaryDatacenter(t *testing.T) { Roles: []string{resourcePrefix + "-connect-injector-acl-role-" + secondaryDatacenter}, GlobalAuthMethod: false, }, + { + TestName: "Sync Catalog", + TokenFlags: []string{"-sync-catalog"}, + PolicyNames: []string{"sync-catalog-policy-" + secondaryDatacenter}, + Roles: []string{resourcePrefix + "-sync-catalog-acl-role-" + secondaryDatacenter}, + GlobalAuthMethod: false, + }, } for _, c := range cases { t.Run(c.TestName, func(t *testing.T) { @@ -2384,6 +2375,11 @@ func TestRun_ValidateLoginToken_PrimaryDatacenter(t *testing.T) { Roles: []string{resourcePrefix + "-connect-injector-acl-role"}, GlobalToken: false, }, + { + ComponentName: "sync-catalog", + TokenFlags: []string{"-sync-catalog"}, + Roles: []string{resourcePrefix + "-sync-catalog-acl-role"}, + }, } for _, c := range cases { t.Run(c.ComponentName, func(t *testing.T) { @@ -2472,6 +2468,12 @@ func TestRun_ValidateLoginToken_SecondaryDatacenter(t *testing.T) { GlobalAuthMethod: false, GlobalToken: false, }, + { + ComponentName: "sync-catalog", + TokenFlags: []string{"-sync-catalog"}, + Roles: []string{resourcePrefix + "-sync-catalog-acl-role-dc2"}, + GlobalAuthMethod: false, + }, } for _, c := range cases { t.Run(c.ComponentName, func(t *testing.T) { @@ -2801,7 +2803,7 @@ func getBootToken(t *testing.T, k8s *fake.Clientset, prefix string, k8sNamespace func setUpK8sServiceAccount(t *testing.T, k8s *fake.Clientset, namespace string) (string, string) { // Create ServiceAccount for the kubernetes auth method if it doesn't exist, // otherwise, do nothing. - serviceAccountName := resourcePrefix + "-connect-injector" + serviceAccountName := resourcePrefix + "-auth-method" sa, _ := k8s.CoreV1().ServiceAccounts(namespace).Get(context.Background(), serviceAccountName, metav1.GetOptions{}) if sa == nil { // Create a service account that references two secrets. @@ -2818,7 +2820,7 @@ func setUpK8sServiceAccount(t *testing.T, k8s *fake.Clientset, namespace string) Name: resourcePrefix + "-some-other-secret", }, { - Name: resourcePrefix + "-connect-injector", + Name: resourcePrefix + "-auth-method", }, }, }, @@ -2833,7 +2835,7 @@ func setUpK8sServiceAccount(t *testing.T, k8s *fake.Clientset, namespace string) require.NoError(t, err) // Create a Kubernetes secret if it doesn't exist, otherwise update it - secretName := resourcePrefix + "-connect-injector" + secretName := resourcePrefix + "-auth-method" secret := &v1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: secretName, diff --git a/control-plane/subcommand/server-acl-init/connect_inject.go b/control-plane/subcommand/server-acl-init/connect_inject.go index 8869fcaef3..64666b01d2 100644 --- a/control-plane/subcommand/server-acl-init/connect_inject.go +++ b/control-plane/subcommand/server-acl-init/connect_inject.go @@ -83,11 +83,11 @@ func (c *Command) configureConnectInjectAuthMethod(consulClient *api.Client, aut func (c *Command) createAuthMethodTmpl(authMethodName string, useNS bool) (api.ACLAuthMethod, error) { // Get the Secret name for the auth method ServiceAccount. var authMethodServiceAccount *apiv1.ServiceAccount - saName := c.withPrefix("connect-injector") - err := c.untilSucceeds(fmt.Sprintf("getting %s ServiceAccount", saName), + serviceAccountName := c.withPrefix("auth-method") + err := c.untilSucceeds(fmt.Sprintf("getting %s ServiceAccount", serviceAccountName), func() error { var err error - authMethodServiceAccount, err = c.clientset.CoreV1().ServiceAccounts(c.flagK8sNamespace).Get(c.ctx, saName, metav1.GetOptions{}) + authMethodServiceAccount, err = c.clientset.CoreV1().ServiceAccounts(c.flagK8sNamespace).Get(c.ctx, serviceAccountName, metav1.GetOptions{}) return err }) if err != nil { @@ -119,7 +119,7 @@ func (c *Command) createAuthMethodTmpl(authMethodName string, useNS bool) (api.A // a secret of type ServiceAccountToken. if saSecret == nil { return api.ACLAuthMethod{}, - fmt.Errorf("found no secret of type 'kubernetes.io/service-account-token' associated with the %s service account", saName) + fmt.Errorf("found no secret of type 'kubernetes.io/service-account-token' associated with the %s service account", serviceAccountName) } kubernetesHost := defaultKubernetesHost diff --git a/control-plane/subcommand/server-acl-init/connect_inject_test.go b/control-plane/subcommand/server-acl-init/connect_inject_test.go index e9c38595a0..959f02e178 100644 --- a/control-plane/subcommand/server-acl-init/connect_inject_test.go +++ b/control-plane/subcommand/server-acl-init/connect_inject_test.go @@ -30,7 +30,7 @@ func TestCommand_createAuthMethodTmpl_SecretNotFound(t *testing.T) { ctx: ctx, } - serviceAccountName := resourcePrefix + "-connect-injector" + serviceAccountName := resourcePrefix + "-auth-method" secretName := resourcePrefix + "-connect-injector" // Create a service account referencing secretName @@ -65,5 +65,5 @@ func TestCommand_createAuthMethodTmpl_SecretNotFound(t *testing.T) { require.NoError(t, err) _, err = cmd.createAuthMethodTmpl("test", true) - require.EqualError(t, err, "found no secret of type 'kubernetes.io/service-account-token' associated with the release-name-consul-connect-injector service account") + require.EqualError(t, err, "found no secret of type 'kubernetes.io/service-account-token' associated with the release-name-consul-auth-method service account") } diff --git a/control-plane/subcommand/server-acl-init/create_or_update.go b/control-plane/subcommand/server-acl-init/create_or_update.go index 954b1e83c6..291eead327 100644 --- a/control-plane/subcommand/server-acl-init/create_or_update.go +++ b/control-plane/subcommand/server-acl-init/create_or_update.go @@ -313,7 +313,7 @@ func (c *Command) createOrUpdateACLPolicy(policy api.ACLPolicy, consulClient *ap // Allowing the Consul node name to be configurable also requires any sync // policy to be updated in case the node name has changed. if isPolicyExistsErr(err, policy.Name) { - if c.flagEnableNamespaces || c.flagCreateSyncToken { + if c.flagEnableNamespaces || c.flagSyncCatalog { c.log.Info(fmt.Sprintf("Policy %q already exists, updating", policy.Name)) // The policy ID is required in any PolicyUpdate call, so first we need to diff --git a/control-plane/subcommand/server-acl-init/create_or_update_test.go b/control-plane/subcommand/server-acl-init/create_or_update_test.go index 57cdffa2a1..5cd01fac25 100644 --- a/control-plane/subcommand/server-acl-init/create_or_update_test.go +++ b/control-plane/subcommand/server-acl-init/create_or_update_test.go @@ -20,10 +20,10 @@ func TestCreateOrUpdateACLPolicy_ErrorsIfDescriptionDoesNotMatch(t *testing.T) { ui := cli.NewMockUi() k8s := fake.NewSimpleClientset() cmd := Command{ - UI: ui, - clientset: k8s, - log: hclog.NewNullLogger(), - flagCreateSyncToken: true, + UI: ui, + clientset: k8s, + log: hclog.NewNullLogger(), + flagSyncCatalog: true, } // Start Consul.