From 3dee6850d5010c4a97d663df635f5866e2e44d43 Mon Sep 17 00:00:00 2001 From: Ashwin Venkatesh Date: Sun, 6 Mar 2022 17:42:09 -0500 Subject: [PATCH 1/4] Create component-authmethod service account and rolebinding --- ...mponent-authmethod-clusterrolebinding.yaml | 20 +++++++++ .../component-authmethod-serviceaccount.yaml | 19 +++++++++ ...mponent-authmethod-clusterrolebinding.bats | 20 +++++++++ .../component-authmethod-serviceaccount.bats | 41 +++++++++++++++++++ .../test/unit/server-acl-init-role.bats | 13 ++++++ .../subcommand/server-acl-init/command.go | 4 +- 6 files changed, 115 insertions(+), 2 deletions(-) create mode 100644 charts/consul/templates/component-authmethod-clusterrolebinding.yaml create mode 100644 charts/consul/templates/component-authmethod-serviceaccount.yaml create mode 100644 charts/consul/test/unit/component-authmethod-clusterrolebinding.bats create mode 100644 charts/consul/test/unit/component-authmethod-serviceaccount.bats diff --git a/charts/consul/templates/component-authmethod-clusterrolebinding.yaml b/charts/consul/templates/component-authmethod-clusterrolebinding.yaml new file mode 100644 index 0000000000..3e80bf6800 --- /dev/null +++ b/charts/consul/templates/component-authmethod-clusterrolebinding.yaml @@ -0,0 +1,20 @@ +{{- if .Values.global.acls.manageSystemACLs }} +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + name: {{ template "consul.fullname" . }}-component-authdelegator + labels: + app: {{ template "consul.name" . }} + chart: {{ template "consul.chart" . }} + heritage: {{ .Release.Service }} + release: {{ .Release.Name }} + component: component-authmethod +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: "system:auth-delegator" +subjects: +- kind: ServiceAccount + name: {{ template "consul.fullname" . }}-component-authmethod + namespace: {{ .Release.Namespace }} +{{- end }} diff --git a/charts/consul/templates/component-authmethod-serviceaccount.yaml b/charts/consul/templates/component-authmethod-serviceaccount.yaml new file mode 100644 index 0000000000..0efd4b26f8 --- /dev/null +++ b/charts/consul/templates/component-authmethod-serviceaccount.yaml @@ -0,0 +1,19 @@ +{{- if .Values.global.acls.manageSystemACLs }} +apiVersion: v1 +kind: ServiceAccount +metadata: + name: {{ template "consul.fullname" . }}-component-authmethod + namespace: {{ .Release.Namespace }} + labels: + app: {{ template "consul.name" . }} + chart: {{ template "consul.chart" . }} + heritage: {{ .Release.Service }} + release: {{ .Release.Name }} + component: component-authmethod +{{- with .Values.global.imagePullSecrets }} +imagePullSecrets: +{{- range . }} +- name: {{ .name }} +{{- end }} +{{- end }} +{{- end }} diff --git a/charts/consul/test/unit/component-authmethod-clusterrolebinding.bats b/charts/consul/test/unit/component-authmethod-clusterrolebinding.bats new file mode 100644 index 0000000000..7d07185573 --- /dev/null +++ b/charts/consul/test/unit/component-authmethod-clusterrolebinding.bats @@ -0,0 +1,20 @@ +#!/usr/bin/env bats + +load _helpers + +@test "componentAuthmethod/ClusterRoleBinding: disabled by default" { + cd `chart_dir` + assert_empty helm template \ + -s templates/component-authmethod-clusterrolebinding.yaml \ + . +} + +@test "componentAuthmethod/ClusterRoleBinding: enabled with global.acls.manageSystemACLs true" { + cd `chart_dir` + local actual=$(helm template \ + -s templates/component-authmethod-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/component-authmethod-serviceaccount.bats b/charts/consul/test/unit/component-authmethod-serviceaccount.bats new file mode 100644 index 0000000000..7635e8cc1d --- /dev/null +++ b/charts/consul/test/unit/component-authmethod-serviceaccount.bats @@ -0,0 +1,41 @@ +#!/usr/bin/env bats + +load _helpers + +@test "componentAuthMethod/ServiceAccount: disabled by default" { + cd `chart_dir` + assert_empty helm template \ + -s templates/component-authmethod-serviceaccount.yaml \ + . +} + +@test "componentAuthMethod/ServiceAccount: enabled with global.acls.manageSystemACLs.enabled true" { + cd `chart_dir` + local actual=$(helm template \ + -s templates/component-authmethod-serviceaccount.yaml \ + --set 'global.acls.manageSystemACLs=true' \ + . | tee /dev/stderr | + yq -s 'length > 0' | tee /dev/stderr) + [ "${actual}" = "true" ] +} + +#-------------------------------------------------------------------- +# global.imagePullSecrets + +@test "componentAuthMethod/ServiceAccount: can set image pull secrets" { + cd `chart_dir` + local object=$(helm template \ + -s templates/component-authmethod-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/server-acl-init-role.bats b/charts/consul/test/unit/server-acl-init-role.bats index 92cdd78d16..05872258a8 100644 --- a/charts/consul/test/unit/server-acl-init-role.bats +++ b/charts/consul/test/unit/server-acl-init-role.bats @@ -55,6 +55,19 @@ load _helpers #-------------------------------------------------------------------- # manageSystemACLs.enabled +@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' \ + . | tee /dev/stderr | + yq -r '.rules | map(select(.resources[0] == "serviceaccounts")) | length' | tee /dev/stderr) + [ "${actual}" = "2" ] +} + +#-------------------------------------------------------------------- +# manageSystemACLs.enabled + @test "serverACLInit/Role: allows service accounts when manageSystemACLs.enabled is true" { cd `chart_dir` local actual=$(helm template \ diff --git a/control-plane/subcommand/server-acl-init/command.go b/control-plane/subcommand/server-acl-init/command.go index b98a84e859..962ed73dc5 100644 --- a/control-plane/subcommand/server-acl-init/command.go +++ b/control-plane/subcommand/server-acl-init/command.go @@ -758,7 +758,7 @@ func (c *Command) Run(args []string) int { // that the Consul components will use to issue global ACL tokens with. func (c *Command) configureGlobalComponentAuthMethod(consulClient *api.Client, authMethodName, primaryDC string) error { // Create the auth method template. This requires calls to the kubernetes environment. - authMethod, err := c.createAuthMethodTmpl(authMethodName, false) + authMethod, err := c.createAuthMethodTmpl(authMethodName, c.withPrefix("component-authmethod"), false) if err != nil { return err } @@ -771,7 +771,7 @@ func (c *Command) configureGlobalComponentAuthMethod(consulClient *api.Client, a // that the Consul components will use to issue local ACL tokens with. func (c *Command) configureLocalComponentAuthMethod(consulClient *api.Client, authMethodName string) error { // Create the auth method template. This requires calls to the kubernetes environment. - authMethod, err := c.createAuthMethodTmpl(authMethodName, false) + authMethod, err := c.createAuthMethodTmpl(authMethodName, c.withPrefix("component-authmethod"), false) if err != nil { return err } From bf309e01418fdae375eb8a135a8e51d6d86cfa3e Mon Sep 17 00:00:00 2001 From: Ashwin Venkatesh Date: Mon, 7 Mar 2022 09:47:46 -0500 Subject: [PATCH 2/4] Use a single serviceaccount and rolebinding for all authmethods. --- .../templates/authmethod-clusterrole.yaml | 18 +++++++++ .../authmethod-clusterrolebinding.yaml | 39 +++++++++++++++++++ ...nt.yaml => authmethod-serviceaccount.yaml} | 4 +- ...mponent-authmethod-clusterrolebinding.yaml | 20 ---------- .../test/unit/authmethod-clusterrole.bats | 20 ++++++++++ .../unit/authmethod-clusterrolebinding.bats | 20 ++++++++++ ...nt.bats => authmethod-serviceaccount.bats} | 12 +++--- ...mponent-authmethod-clusterrolebinding.bats | 20 ---------- .../subcommand/server-acl-init/command.go | 4 +- 9 files changed, 107 insertions(+), 50 deletions(-) create mode 100644 charts/consul/templates/authmethod-clusterrole.yaml create mode 100644 charts/consul/templates/authmethod-clusterrolebinding.yaml rename charts/consul/templates/{component-authmethod-serviceaccount.yaml => authmethod-serviceaccount.yaml} (80%) delete mode 100644 charts/consul/templates/component-authmethod-clusterrolebinding.yaml create mode 100644 charts/consul/test/unit/authmethod-clusterrole.bats create mode 100644 charts/consul/test/unit/authmethod-clusterrolebinding.bats rename charts/consul/test/unit/{component-authmethod-serviceaccount.bats => authmethod-serviceaccount.bats} (67%) delete mode 100644 charts/consul/test/unit/component-authmethod-clusterrolebinding.bats diff --git a/charts/consul/templates/authmethod-clusterrole.yaml b/charts/consul/templates/authmethod-clusterrole.yaml new file mode 100644 index 0000000000..30385aa25f --- /dev/null +++ b/charts/consul/templates/authmethod-clusterrole.yaml @@ -0,0 +1,18 @@ +{{- if .Values.global.acls.manageSystemACLs }} +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: {{ template "consul.fullname" . }}-authmethod + labels: + app: {{ template "consul.name" . }} + chart: {{ template "consul.chart" . }} + heritage: {{ .Release.Service }} + release: {{ .Release.Name }} + component: authmethod +rules: +- apiGroups: [ "" ] + resources: + - serviceaccounts + verbs: + - get +{{- end }} diff --git a/charts/consul/templates/authmethod-clusterrolebinding.yaml b/charts/consul/templates/authmethod-clusterrolebinding.yaml new file mode 100644 index 0000000000..89bc44dea4 --- /dev/null +++ b/charts/consul/templates/authmethod-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: authmethod +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: "system:auth-delegator" +subjects: +- kind: ServiceAccount + name: {{ template "consul.fullname" . }}-authmethod + namespace: {{ .Release.Namespace }} +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + name: {{ template "consul.fullname" . }}-authmethod + labels: + app: {{ template "consul.name" . }} + chart: {{ template "consul.chart" . }} + heritage: {{ .Release.Service }} + release: {{ .Release.Name }} + component: authmethod +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: {{ template "consul.fullname" . }}-authmethod +subjects: +- kind: ServiceAccount + name: {{ template "consul.fullname" . }}-authmethod + namespace: {{ .Release.Namespace }} +{{- end }} diff --git a/charts/consul/templates/component-authmethod-serviceaccount.yaml b/charts/consul/templates/authmethod-serviceaccount.yaml similarity index 80% rename from charts/consul/templates/component-authmethod-serviceaccount.yaml rename to charts/consul/templates/authmethod-serviceaccount.yaml index 0efd4b26f8..bfb50dc0dd 100644 --- a/charts/consul/templates/component-authmethod-serviceaccount.yaml +++ b/charts/consul/templates/authmethod-serviceaccount.yaml @@ -2,14 +2,14 @@ apiVersion: v1 kind: ServiceAccount metadata: - name: {{ template "consul.fullname" . }}-component-authmethod + name: {{ template "consul.fullname" . }}-authmethod namespace: {{ .Release.Namespace }} labels: app: {{ template "consul.name" . }} chart: {{ template "consul.chart" . }} heritage: {{ .Release.Service }} release: {{ .Release.Name }} - component: component-authmethod + component: authmethod {{- with .Values.global.imagePullSecrets }} imagePullSecrets: {{- range . }} diff --git a/charts/consul/templates/component-authmethod-clusterrolebinding.yaml b/charts/consul/templates/component-authmethod-clusterrolebinding.yaml deleted file mode 100644 index 3e80bf6800..0000000000 --- a/charts/consul/templates/component-authmethod-clusterrolebinding.yaml +++ /dev/null @@ -1,20 +0,0 @@ -{{- if .Values.global.acls.manageSystemACLs }} -apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRoleBinding -metadata: - name: {{ template "consul.fullname" . }}-component-authdelegator - labels: - app: {{ template "consul.name" . }} - chart: {{ template "consul.chart" . }} - heritage: {{ .Release.Service }} - release: {{ .Release.Name }} - component: component-authmethod -roleRef: - apiGroup: rbac.authorization.k8s.io - kind: ClusterRole - name: "system:auth-delegator" -subjects: -- kind: ServiceAccount - name: {{ template "consul.fullname" . }}-component-authmethod - namespace: {{ .Release.Namespace }} -{{- end }} diff --git a/charts/consul/test/unit/authmethod-clusterrole.bats b/charts/consul/test/unit/authmethod-clusterrole.bats new file mode 100644 index 0000000000..0888ea373b --- /dev/null +++ b/charts/consul/test/unit/authmethod-clusterrole.bats @@ -0,0 +1,20 @@ +#!/usr/bin/env bats + +load _helpers + +@test "authmethod/ClusterRole: disabled by default" { + cd `chart_dir` + assert_empty helm template \ + -s templates/authmethod-clusterrole.yaml \ + . +} + +@test "authmethod/ClusterRole: enabled with global.acls.manageSystemACLs true" { + cd `chart_dir` + local actual=$(helm template \ + -s templates/authmethod-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/authmethod-clusterrolebinding.bats b/charts/consul/test/unit/authmethod-clusterrolebinding.bats new file mode 100644 index 0000000000..21b96d0bbb --- /dev/null +++ b/charts/consul/test/unit/authmethod-clusterrolebinding.bats @@ -0,0 +1,20 @@ +#!/usr/bin/env bats + +load _helpers + +@test "authmethod/ClusterRoleBinding: disabled by default" { + cd `chart_dir` + assert_empty helm template \ + -s templates/authmethod-clusterrolebinding.yaml \ + . +} + +@test "authmethod/ClusterRoleBinding: enabled with global.acls.manageSystemACLs true" { + cd `chart_dir` + local actual=$(helm template \ + -s templates/authmethod-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/component-authmethod-serviceaccount.bats b/charts/consul/test/unit/authmethod-serviceaccount.bats similarity index 67% rename from charts/consul/test/unit/component-authmethod-serviceaccount.bats rename to charts/consul/test/unit/authmethod-serviceaccount.bats index 7635e8cc1d..8cf9633cae 100644 --- a/charts/consul/test/unit/component-authmethod-serviceaccount.bats +++ b/charts/consul/test/unit/authmethod-serviceaccount.bats @@ -2,17 +2,17 @@ load _helpers -@test "componentAuthMethod/ServiceAccount: disabled by default" { +@test "authMethod/ServiceAccount: disabled by default" { cd `chart_dir` assert_empty helm template \ - -s templates/component-authmethod-serviceaccount.yaml \ + -s templates/authmethod-serviceaccount.yaml \ . } -@test "componentAuthMethod/ServiceAccount: enabled with global.acls.manageSystemACLs.enabled true" { +@test "authMethod/ServiceAccount: enabled with global.acls.manageSystemACLs.enabled true" { cd `chart_dir` local actual=$(helm template \ - -s templates/component-authmethod-serviceaccount.yaml \ + -s templates/authmethod-serviceaccount.yaml \ --set 'global.acls.manageSystemACLs=true' \ . | tee /dev/stderr | yq -s 'length > 0' | tee /dev/stderr) @@ -22,10 +22,10 @@ load _helpers #-------------------------------------------------------------------- # global.imagePullSecrets -@test "componentAuthMethod/ServiceAccount: can set image pull secrets" { +@test "authMethod/ServiceAccount: can set image pull secrets" { cd `chart_dir` local object=$(helm template \ - -s templates/component-authmethod-serviceaccount.yaml \ + -s templates/authmethod-serviceaccount.yaml \ --set 'global.acls.manageSystemACLs=true' \ --set 'global.imagePullSecrets[0].name=my-secret' \ --set 'global.imagePullSecrets[1].name=my-secret2' \ diff --git a/charts/consul/test/unit/component-authmethod-clusterrolebinding.bats b/charts/consul/test/unit/component-authmethod-clusterrolebinding.bats deleted file mode 100644 index 7d07185573..0000000000 --- a/charts/consul/test/unit/component-authmethod-clusterrolebinding.bats +++ /dev/null @@ -1,20 +0,0 @@ -#!/usr/bin/env bats - -load _helpers - -@test "componentAuthmethod/ClusterRoleBinding: disabled by default" { - cd `chart_dir` - assert_empty helm template \ - -s templates/component-authmethod-clusterrolebinding.yaml \ - . -} - -@test "componentAuthmethod/ClusterRoleBinding: enabled with global.acls.manageSystemACLs true" { - cd `chart_dir` - local actual=$(helm template \ - -s templates/component-authmethod-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/control-plane/subcommand/server-acl-init/command.go b/control-plane/subcommand/server-acl-init/command.go index 962ed73dc5..b98a84e859 100644 --- a/control-plane/subcommand/server-acl-init/command.go +++ b/control-plane/subcommand/server-acl-init/command.go @@ -758,7 +758,7 @@ func (c *Command) Run(args []string) int { // that the Consul components will use to issue global ACL tokens with. func (c *Command) configureGlobalComponentAuthMethod(consulClient *api.Client, authMethodName, primaryDC string) error { // Create the auth method template. This requires calls to the kubernetes environment. - authMethod, err := c.createAuthMethodTmpl(authMethodName, c.withPrefix("component-authmethod"), false) + authMethod, err := c.createAuthMethodTmpl(authMethodName, false) if err != nil { return err } @@ -771,7 +771,7 @@ func (c *Command) configureGlobalComponentAuthMethod(consulClient *api.Client, a // that the Consul components will use to issue local ACL tokens with. func (c *Command) configureLocalComponentAuthMethod(consulClient *api.Client, authMethodName string) error { // Create the auth method template. This requires calls to the kubernetes environment. - authMethod, err := c.createAuthMethodTmpl(authMethodName, c.withPrefix("component-authmethod"), false) + authMethod, err := c.createAuthMethodTmpl(authMethodName, false) if err != nil { return err } From 95389d9319a84f97623d781ea7fc17fdcb86ee3f Mon Sep 17 00:00:00 2001 From: Ashwin Venkatesh Date: Mon, 7 Mar 2022 16:16:13 -0500 Subject: [PATCH 3/4] wip --- .../templates/authmethod-clusterrole.yaml | 18 -------- .../authmethod-clusterrolebinding.yaml | 39 ------------------ .../templates/authmethod-serviceaccount.yaml | 19 --------- .../test/unit/authmethod-clusterrole.bats | 20 --------- .../unit/authmethod-clusterrolebinding.bats | 20 --------- .../test/unit/authmethod-serviceaccount.bats | 41 ------------------- 6 files changed, 157 deletions(-) delete mode 100644 charts/consul/templates/authmethod-clusterrole.yaml delete mode 100644 charts/consul/templates/authmethod-clusterrolebinding.yaml delete mode 100644 charts/consul/templates/authmethod-serviceaccount.yaml delete mode 100644 charts/consul/test/unit/authmethod-clusterrole.bats delete mode 100644 charts/consul/test/unit/authmethod-clusterrolebinding.bats delete mode 100644 charts/consul/test/unit/authmethod-serviceaccount.bats diff --git a/charts/consul/templates/authmethod-clusterrole.yaml b/charts/consul/templates/authmethod-clusterrole.yaml deleted file mode 100644 index 30385aa25f..0000000000 --- a/charts/consul/templates/authmethod-clusterrole.yaml +++ /dev/null @@ -1,18 +0,0 @@ -{{- if .Values.global.acls.manageSystemACLs }} -apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRole -metadata: - name: {{ template "consul.fullname" . }}-authmethod - labels: - app: {{ template "consul.name" . }} - chart: {{ template "consul.chart" . }} - heritage: {{ .Release.Service }} - release: {{ .Release.Name }} - component: authmethod -rules: -- apiGroups: [ "" ] - resources: - - serviceaccounts - verbs: - - get -{{- end }} diff --git a/charts/consul/templates/authmethod-clusterrolebinding.yaml b/charts/consul/templates/authmethod-clusterrolebinding.yaml deleted file mode 100644 index 89bc44dea4..0000000000 --- a/charts/consul/templates/authmethod-clusterrolebinding.yaml +++ /dev/null @@ -1,39 +0,0 @@ -{{- 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: authmethod -roleRef: - apiGroup: rbac.authorization.k8s.io - kind: ClusterRole - name: "system:auth-delegator" -subjects: -- kind: ServiceAccount - name: {{ template "consul.fullname" . }}-authmethod - namespace: {{ .Release.Namespace }} ---- -apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRoleBinding -metadata: - name: {{ template "consul.fullname" . }}-authmethod - labels: - app: {{ template "consul.name" . }} - chart: {{ template "consul.chart" . }} - heritage: {{ .Release.Service }} - release: {{ .Release.Name }} - component: authmethod -roleRef: - apiGroup: rbac.authorization.k8s.io - kind: ClusterRole - name: {{ template "consul.fullname" . }}-authmethod -subjects: -- kind: ServiceAccount - name: {{ template "consul.fullname" . }}-authmethod - namespace: {{ .Release.Namespace }} -{{- end }} diff --git a/charts/consul/templates/authmethod-serviceaccount.yaml b/charts/consul/templates/authmethod-serviceaccount.yaml deleted file mode 100644 index bfb50dc0dd..0000000000 --- a/charts/consul/templates/authmethod-serviceaccount.yaml +++ /dev/null @@ -1,19 +0,0 @@ -{{- if .Values.global.acls.manageSystemACLs }} -apiVersion: v1 -kind: ServiceAccount -metadata: - name: {{ template "consul.fullname" . }}-authmethod - namespace: {{ .Release.Namespace }} - labels: - app: {{ template "consul.name" . }} - chart: {{ template "consul.chart" . }} - heritage: {{ .Release.Service }} - release: {{ .Release.Name }} - component: authmethod -{{- with .Values.global.imagePullSecrets }} -imagePullSecrets: -{{- range . }} -- name: {{ .name }} -{{- end }} -{{- end }} -{{- end }} diff --git a/charts/consul/test/unit/authmethod-clusterrole.bats b/charts/consul/test/unit/authmethod-clusterrole.bats deleted file mode 100644 index 0888ea373b..0000000000 --- a/charts/consul/test/unit/authmethod-clusterrole.bats +++ /dev/null @@ -1,20 +0,0 @@ -#!/usr/bin/env bats - -load _helpers - -@test "authmethod/ClusterRole: disabled by default" { - cd `chart_dir` - assert_empty helm template \ - -s templates/authmethod-clusterrole.yaml \ - . -} - -@test "authmethod/ClusterRole: enabled with global.acls.manageSystemACLs true" { - cd `chart_dir` - local actual=$(helm template \ - -s templates/authmethod-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/authmethod-clusterrolebinding.bats b/charts/consul/test/unit/authmethod-clusterrolebinding.bats deleted file mode 100644 index 21b96d0bbb..0000000000 --- a/charts/consul/test/unit/authmethod-clusterrolebinding.bats +++ /dev/null @@ -1,20 +0,0 @@ -#!/usr/bin/env bats - -load _helpers - -@test "authmethod/ClusterRoleBinding: disabled by default" { - cd `chart_dir` - assert_empty helm template \ - -s templates/authmethod-clusterrolebinding.yaml \ - . -} - -@test "authmethod/ClusterRoleBinding: enabled with global.acls.manageSystemACLs true" { - cd `chart_dir` - local actual=$(helm template \ - -s templates/authmethod-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/authmethod-serviceaccount.bats b/charts/consul/test/unit/authmethod-serviceaccount.bats deleted file mode 100644 index 8cf9633cae..0000000000 --- a/charts/consul/test/unit/authmethod-serviceaccount.bats +++ /dev/null @@ -1,41 +0,0 @@ -#!/usr/bin/env bats - -load _helpers - -@test "authMethod/ServiceAccount: disabled by default" { - cd `chart_dir` - assert_empty helm template \ - -s templates/authmethod-serviceaccount.yaml \ - . -} - -@test "authMethod/ServiceAccount: enabled with global.acls.manageSystemACLs.enabled true" { - cd `chart_dir` - local actual=$(helm template \ - -s templates/authmethod-serviceaccount.yaml \ - --set 'global.acls.manageSystemACLs=true' \ - . | tee /dev/stderr | - yq -s 'length > 0' | tee /dev/stderr) - [ "${actual}" = "true" ] -} - -#-------------------------------------------------------------------- -# global.imagePullSecrets - -@test "authMethod/ServiceAccount: can set image pull secrets" { - cd `chart_dir` - local object=$(helm template \ - -s templates/authmethod-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" ] -} From 8323783b122a8d1bdf63a962a2c828d52c5739c6 Mon Sep 17 00:00:00 2001 From: Ashwin Venkatesh Date: Tue, 8 Mar 2022 09:44:05 -0500 Subject: [PATCH 4/4] Refactor Consul API Gateway Controller to use AuthMethod workflow. --- .../api-gateway-controller-deployment.yaml | 155 ++++++--- .../consul/templates/server-acl-init-job.yaml | 2 +- .../api-gateway-controller-deployment.bats | 294 +++++++++++++++++- .../test/unit/server-acl-init-role.bats | 13 - charts/consul/values.yaml | 23 ++ .../subcommand/server-acl-init/command.go | 23 +- .../server-acl-init/command_test.go | 68 ++-- 7 files changed, 466 insertions(+), 112 deletions(-) diff --git a/charts/consul/templates/api-gateway-controller-deployment.yaml b/charts/consul/templates/api-gateway-controller-deployment.yaml index 492d9f3302..9766793e37 100644 --- a/charts/consul/templates/api-gateway-controller-deployment.yaml +++ b/charts/consul/templates/api-gateway-controller-deployment.yaml @@ -60,11 +60,8 @@ spec: fieldRef: fieldPath: status.hostIP {{- if .Values.global.acls.manageSystemACLs }} - - name: CONSUL_HTTP_TOKEN - valueFrom: - secretKeyRef: - name: "{{ template "consul.fullname" . }}-api-gateway-controller-acl-token" - key: "token" + - name: CONSUL_HTTP_TOKEN_FILE + value: "/consul/login/acl-token" {{- end }} - name: CONSUL_HTTP_ADDR {{- if .Values.global.tls.enabled }} @@ -73,35 +70,57 @@ spec: value: http://$(HOST_IP):8500 {{- end }} command: - - "/bin/sh" - - "-ec" - - | - consul-api-gateway server \ - -sds-server-host {{ template "consul.fullname" . }}-api-gateway-controller.{{ .Release.Namespace }}.svc \ - -k8s-namespace {{ .Release.Namespace }} \ - {{- if .Values.global.enableConsulNamespaces }} - {{- if .Values.apiGateway.consulNamespaces.consulDestinationNamespace }} - -consul-destination-namespace={{ .Values.apiGateway.consulNamespaces.consulDestinationNamespace }} \ - {{- end }} - {{- if .Values.apiGateway.consulNamespaces.mirroringK8S }} - -mirroring-k8s=true \ - {{- if .Values.apiGateway.consulNamespaces.mirroringK8SPrefix }} - -mirroring-k8s-prefix={{ .Values.apiGateway.consulNamespaces.mirroringK8SPrefix }} \ - {{- end }} - {{- end }} - {{- end }} - -log-level {{ default .Values.global.logLevel .Values.apiGateway.logLevel }} \ + - "/bin/sh" + - "-ec" + - | + consul-api-gateway server \ + -sds-server-host {{ template "consul.fullname" . }}-api-gateway-controller.{{ .Release.Namespace }}.svc \ + -k8s-namespace {{ .Release.Namespace }} \ + {{- if .Values.global.enableConsulNamespaces }} + {{- if .Values.apiGateway.consulNamespaces.consulDestinationNamespace }} + -consul-destination-namespace={{ .Values.apiGateway.consulNamespaces.consulDestinationNamespace }} \ + {{- end }} + {{- if .Values.apiGateway.consulNamespaces.mirroringK8S }} + -mirroring-k8s=true \ + {{- if .Values.apiGateway.consulNamespaces.mirroringK8SPrefix }} + -mirroring-k8s-prefix={{ .Values.apiGateway.consulNamespaces.mirroringK8SPrefix }} \ + {{- end }} + {{- end }} + {{- end }} + -log-level {{ default .Values.global.logLevel .Values.apiGateway.logLevel }} \ + -log-json={{ .Values.global.logJSON }} volumeMounts: - {{- if .Values.global.tls.enabled }} - {{- if .Values.global.tls.enableAutoEncrypt }} - - name: consul-auto-encrypt-ca-cert - {{- else }} - - name: consul-ca-cert - {{- end }} - mountPath: /consul/tls/ca - readOnly: true - {{- end }} + {{- if .Values.global.acls.manageSystemACLs }} + - name: consul-bin + mountPath: /consul-bin + {{- end }} + {{- 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 }} + - mountPath: /consul/login + name: consul-data + readOnly: true + {{- if .Values.apiGateway.resources }} + resources: + {{- toYaml .Values.apiGateway.resources | nindent 12 }} + {{- end }} + {{- if .Values.global.acls.manageSystemACLs }} + lifecycle: + preStop: + exec: + command: [ "/bin/sh", "-ec", "/consul-bin/consul logout" ] + {{- end }} volumes: + {{- if .Values.global.acls.manageSystemACLs }} + - name: consul-bin + emptyDir: { } + {{- end }} {{- if .Values.global.tls.enabled }} {{- if not (and .Values.externalServers.enabled .Values.externalServers.useSystemRoots) }} - name: consul-ca-cert @@ -121,18 +140,73 @@ spec: medium: "Memory" {{- end }} {{- end }} - {{- if or (and .Values.global.acls.manageSystemACLs) (and .Values.global.tls.enabled .Values.global.tls.enableAutoEncrypt) }} + - name: consul-data + emptyDir: + medium: "Memory" + {{- if or .Values.global.acls.manageSystemACLs (and .Values.global.tls.enabled .Values.global.tls.enableAutoEncrypt) }} initContainers: {{- if .Values.global.acls.manageSystemACLs }} + - name: copy-consul-bin + image: {{ .Values.global.image | quote }} + command: + - cp + - /bin/consul + - /consul-bin/consul + volumeMounts: + - name: consul-bin + mountPath: /consul-bin + {{- if .Values.apiGateway.initCopyConsulContainer }} + {{- if .Values.apiGateway.initCopyConsulContainer.resources }} + resources: {{ toYaml .Values.apiGateway.initCopyConsulContainer.resources | nindent 12 }} + {{- end }} + {{- end }} + {{- end }} + {{- if (and .Values.global.tls.enabled .Values.global.tls.enableAutoEncrypt) }} + {{- include "consul.getAutoEncryptClientCA" . | nindent 6 }} + {{- end }} + {{- if .Values.global.acls.manageSystemACLs }} - name: api-gateway-controller-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" . }}-api-gateway-controller-acl-token" \ - -k8s-namespace={{ .Release.Namespace }} + - "/bin/sh" + - "-ec" + - | + consul-k8s-control-plane acl-init \ + -component-name=api-gateway-controller \ + -acl-auth-method={{ template "consul.fullname" . }}-k8s-component-auth-method \ + {{- 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" @@ -141,9 +215,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.apiGateway.controller.priorityClassName }} priorityClassName: {{ .Values.apiGateway.controller.priorityClassName | quote }} diff --git a/charts/consul/templates/server-acl-init-job.yaml b/charts/consul/templates/server-acl-init-job.yaml index 04f4e2de3e..45e3c6089f 100644 --- a/charts/consul/templates/server-acl-init-job.yaml +++ b/charts/consul/templates/server-acl-init-job.yaml @@ -275,7 +275,7 @@ spec: {{- end }} {{- if .Values.apiGateway.enabled }} - -create-api-gateway-token=true \ + -api-gateway-controller=true \ {{- end }} {{- if .Values.global.enableConsulNamespaces }} diff --git a/charts/consul/test/unit/api-gateway-controller-deployment.bats b/charts/consul/test/unit/api-gateway-controller-deployment.bats index 6810c5dde0..6e9686d12f 100755 --- a/charts/consul/test/unit/api-gateway-controller-deployment.bats +++ b/charts/consul/test/unit/api-gateway-controller-deployment.bats @@ -227,7 +227,7 @@ load _helpers --set 'global.tls.enabled=true' \ --set 'global.tls.enableAutoEncrypt=true' \ . | tee /dev/stderr | - yq '.spec.template.spec.initContainers | length == 2' | tee /dev/stderr) + yq '.spec.template.spec.initContainers | length == 3' | tee /dev/stderr) [ "${actual}" = "true" ] } @@ -250,7 +250,7 @@ load _helpers #-------------------------------------------------------------------- # global.acls.manageSystemACLs -@test "apiGateway/Deployment: CONSUL_HTTP_TOKEN env variable created when global.acls.manageSystemACLs=true" { +@test "apiGateway/Deployment: consul-logout preStop hook is added when ACLs are enabled" { cd `chart_dir` local object=$(helm template \ -s templates/api-gateway-controller-deployment.yaml \ @@ -258,15 +258,31 @@ load _helpers --set 'apiGateway.image=foo' \ --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 logout"))' | tee /dev/stderr) + [ "${object}" = "true" ] +} - local actual=$(echo $object | - yq 'any(contains("CONSUL_HTTP_TOKEN"))' | tee /dev/stderr) - [ "${actual}" = "true" ] +@test "apiGateway/Deployment: CONSUL_HTTP_TOKEN_FILE is not set when acls are disabled" { + cd `chart_dir` + local actual=$(helm template \ + -s templates/api-gateway-controller-deployment.yaml \ + --set 'apiGateway.enabled=true' \ + --set 'apiGateway.image=foo' \ + . | tee /dev/stderr | + yq '[.spec.template.spec.containers[0].env[1].name] | any(contains("CONSUL_HTTP_TOKEN_FILE"))' | tee /dev/stderr) + [ "${actual}" = "false" ] +} - local actual=$(echo $object | - yq 'map(select(test("CONSUL_HTTP_TOKEN"))) | length' | tee /dev/stderr) - [ "${actual}" = "1" ] +@test "apiGateway/Deployment: CONSUL_HTTP_TOKEN_FILE is set when acls are enabled" { + cd `chart_dir` + local actual=$(helm template \ + -s templates/api-gateway-controller-deployment.yaml \ + --set 'apiGateway.enabled=true' \ + --set 'apiGateway.image=foo' \ + --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 "apiGateway/Deployment: init container is created when global.acls.manageSystemACLs=true" { @@ -277,7 +293,7 @@ load _helpers --set 'apiGateway.image=foo' \ --set 'global.acls.manageSystemACLs=true' \ . | tee /dev/stderr | - yq '.spec.template.spec.initContainers[0]' | tee /dev/stderr) + yq '.spec.template.spec.initContainers[1]' | tee /dev/stderr) local actual=$(echo $object | yq -r '.name' | tee /dev/stderr) @@ -286,6 +302,264 @@ load _helpers 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 "apiGateway/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/api-gateway-controller-deployment.yaml \ + --set 'apiGateway.enabled=true' \ + --set 'apiGateway.image=foo' \ + --set 'global.tls.enabled=true' \ + --set 'global.acls.manageSystemACLs=true' \ + . | tee /dev/stderr | + yq '.spec.template.spec.initContainers[] | select(.name == "api-gateway-controller-acl-init")' | tee /dev/stderr) + + local actual=$(echo $object | + yq -r '.command | any(contains("consul-k8s-control-plane acl-init"))' | tee /dev/stderr) + [ "${actual}" = "true" ] + + local actual=$(echo $object | + yq '[.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 "apiGateway/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/api-gateway-controller-deployment.yaml \ + --set 'apiGateway.enabled=true' \ + --set 'apiGateway.image=foo' \ + --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 == "api-gateway-controller-acl-init")' | tee /dev/stderr) + + local actual=$(echo $object | + yq -r '.command | any(contains("consul-k8s-control-plane acl-init"))' | tee /dev/stderr) + [ "${actual}" = "true" ] + + local actual=$(echo $object | + yq -r '.command | any(contains("-acl-auth-method=RELEASE-NAME-consul-k8s-component-auth-method"))' | 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 "apiGateway/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/api-gateway-controller-deployment.yaml \ + --set 'apiGateway.enabled=true' \ + --set 'apiGateway.image=foo' \ + --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 == "api-gateway-controller-acl-init")' | tee /dev/stderr) + + local actual=$(echo $object | + yq -r '.command | any(contains("consul-k8s-control-plane acl-init"))' | tee /dev/stderr) + [ "${actual}" = "true" ] + + local actual=$(echo $object | + yq '[.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 "apiGateway/Deployment: init container for copy consul is created when global.acls.manageSystemACLs=true" { + cd `chart_dir` + local object=$(helm template \ + -s templates/api-gateway-controller-deployment.yaml \ + --set 'apiGateway.enabled=true' \ + --set 'apiGateway.image=foo' \ + --set 'global.acls.manageSystemACLs=true' \ + . | tee /dev/stderr | + yq '.spec.template.spec.initContainers[] | select(.name == "copy-consul-bin")' | tee /dev/stderr) + + local actual=$(echo $object | + yq -r '.command | any(contains("cp"))' | tee /dev/stderr) + [ "${actual}" = "true" ] + + local actual=$(echo $object | + yq -r '.volumeMounts[0] | any(contains("consul-bin"))' | tee /dev/stderr) + [ "${actual}" = "true" ] +} + +@test "apiGateway/Deployment: volumeMount for copy consul is created on container when global.acls.manageSystemACLs=true" { + cd `chart_dir` + local object=$(helm template \ + -s templates/api-gateway-controller-deployment.yaml \ + --set 'apiGateway.enabled=true' \ + --set 'apiGateway.image=foo' \ + --set 'global.acls.manageSystemACLs=true' \ + . | tee /dev/stderr | + yq '.spec.template.spec.containers[0].volumeMounts[0] | any(contains("consul-bin"))' | tee /dev/stderr) + + [ "${object}" = "true" ] +} + +@test "apiGateway/Deployment: volume for copy consul is created when global.acls.manageSystemACLs=true" { + cd `chart_dir` + local object=$(helm template \ + -s templates/api-gateway-controller-deployment.yaml \ + --set 'apiGateway.enabled=true' \ + --set 'apiGateway.image=foo' \ + --set 'global.acls.manageSystemACLs=true' \ + . | tee /dev/stderr | + yq '.spec.template.spec.volumes[0] | any(contains("consul-bin"))' | tee /dev/stderr) + + [ "${object}" = "true" ] +} + +@test "apiGateway/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/api-gateway-controller-deployment.yaml \ + --set 'apiGateway.enabled=true' \ + --set 'apiGateway.image=foo' \ + --set 'global.tls.enabled=true' \ + --set 'global.tls.enableAutoEncrypt=true' \ + --set 'global.acls.manageSystemACLs=true' \ + . | tee /dev/stderr | + yq '.spec.template.spec.initContainers[1]' | tee /dev/stderr) + + local actual=$(echo $object | + yq -r '.name' | tee /dev/stderr) + [ "${actual}" = "get-auto-encrypt-client-ca" ] +} + +#-------------------------------------------------------------------- +# resources + +@test "apiGateway/Deployment: resources has default" { + cd `chart_dir` + local actual=$(helm template \ + -s templates/api-gateway-controller-deployment.yaml \ + --set 'apiGateway.enabled=true' \ + --set 'apiGateway.image=foo' \ + . | tee /dev/stderr | + yq -r '.spec.template.spec.containers[0].resources' | tee /dev/stderr) + + [ $(echo "${actual}" | yq -r '.requests.memory') = "100Mi" ] + [ $(echo "${actual}" | yq -r '.requests.cpu') = "100m" ] + [ $(echo "${actual}" | yq -r '.limits.memory') = "100Mi" ] + [ $(echo "${actual}" | yq -r '.limits.cpu') = "100m" ] +} + +@test "apiGateway/Deployment: resources can be overridden" { + cd `chart_dir` + local actual=$(helm template \ + -s templates/api-gateway-controller-deployment.yaml \ + --set 'apiGateway.enabled=true' \ + --set 'apiGateway.image=foo' \ + --set 'apiGateway.resources.foo=bar' \ + . | tee /dev/stderr | + yq -r '.spec.template.spec.containers[0].resources.foo' | tee /dev/stderr) + [ "${actual}" = "bar" ] +} + +#-------------------------------------------------------------------- +# init container resources + +@test "apiGateway/Deployment: init container has default resources" { + cd `chart_dir` + local actual=$(helm template \ + -s templates/api-gateway-controller-deployment.yaml \ + --set 'apiGateway.enabled=true' \ + --set 'apiGateway.image=foo' \ + --set 'global.acls.manageSystemACLs=true' \ + . | tee /dev/stderr | + yq -r '.spec.template.spec.initContainers[0].resources' | tee /dev/stderr) + + [ $(echo "${actual}" | yq -r '.requests.memory') = "25Mi" ] + [ $(echo "${actual}" | yq -r '.requests.cpu') = "50m" ] + [ $(echo "${actual}" | yq -r '.limits.memory') = "150Mi" ] + [ $(echo "${actual}" | yq -r '.limits.cpu') = "50m" ] +} + +@test "apiGateway/Deployment: init container resources can be set" { + cd `chart_dir` + local object=$(helm template \ + -s templates/api-gateway-controller-deployment.yaml \ + --set 'apiGateway.enabled=true' \ + --set 'apiGateway.image=foo' \ + --set 'global.acls.manageSystemACLs=true' \ + --set 'apiGateway.initCopyConsulContainer.resources.requests.memory=memory' \ + --set 'apiGateway.initCopyConsulContainer.resources.requests.cpu=cpu' \ + --set 'apiGateway.initCopyConsulContainer.resources.limits.memory=memory2' \ + --set 'apiGateway.initCopyConsulContainer.resources.limits.cpu=cpu2' \ + . | tee /dev/stderr | + yq -r '.spec.template.spec.initContainers[0].resources' | tee /dev/stderr) + + local actual=$(echo $object | yq -r '.requests.memory' | tee /dev/stderr) + [ "${actual}" = "memory" ] + + local actual=$(echo $object | yq -r '.requests.cpu' | tee /dev/stderr) + [ "${actual}" = "cpu" ] + + local actual=$(echo $object | yq -r '.limits.memory' | tee /dev/stderr) + [ "${actual}" = "memory2" ] + + local actual=$(echo $object | yq -r '.limits.cpu' | tee /dev/stderr) + [ "${actual}" = "cpu2" ] } #-------------------------------------------------------------------- diff --git a/charts/consul/test/unit/server-acl-init-role.bats b/charts/consul/test/unit/server-acl-init-role.bats index 05872258a8..92cdd78d16 100644 --- a/charts/consul/test/unit/server-acl-init-role.bats +++ b/charts/consul/test/unit/server-acl-init-role.bats @@ -55,19 +55,6 @@ load _helpers #-------------------------------------------------------------------- # manageSystemACLs.enabled -@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' \ - . | tee /dev/stderr | - yq -r '.rules | map(select(.resources[0] == "serviceaccounts")) | length' | tee /dev/stderr) - [ "${actual}" = "2" ] -} - -#-------------------------------------------------------------------- -# manageSystemACLs.enabled - @test "serverACLInit/Role: allows service accounts when manageSystemACLs.enabled is true" { cd `chart_dir` local actual=$(helm template \ diff --git a/charts/consul/values.yaml b/charts/consul/values.yaml index 15f38105b5..85a7ed016c 100644 --- a/charts/consul/values.yaml +++ b/charts/consul/values.yaml @@ -2660,6 +2660,29 @@ apiGateway: # @type: string annotations: null + # Resource settings for api gateway pods. + # @recurse: false + # @type: map + resources: + requests: + memory: "100Mi" + cpu: "100m" + limits: + memory: "100Mi" + cpu: "100m" + + # Resource settings for the `copy-consul-bin` init container. + # @recurse: false + # @type: map + initCopyConsulContainer: + resources: + requests: + memory: "25Mi" + cpu: "50m" + limits: + memory: "150Mi" + cpu: "50m" + # Configuration settings for the webhook-cert-manager # `webhook-cert-manager` ensures that cert bundles are up to date for the mutating webhook. webhookCertManager: diff --git a/control-plane/subcommand/server-acl-init/command.go b/control-plane/subcommand/server-acl-init/command.go index b98a84e859..ca1e12a307 100644 --- a/control-plane/subcommand/server-acl-init/command.go +++ b/control-plane/subcommand/server-acl-init/command.go @@ -11,6 +11,11 @@ import ( "sync" "time" + "github.com/hashicorp/consul-k8s/control-plane/consul" + "github.com/hashicorp/consul-k8s/control-plane/subcommand" + "github.com/hashicorp/consul-k8s/control-plane/subcommand/common" + "github.com/hashicorp/consul-k8s/control-plane/subcommand/flags" + k8sflags "github.com/hashicorp/consul-k8s/control-plane/subcommand/flags" "github.com/hashicorp/consul/api" "github.com/hashicorp/go-discover" "github.com/hashicorp/go-hclog" @@ -19,12 +24,6 @@ import ( k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" - - "github.com/hashicorp/consul-k8s/control-plane/consul" - "github.com/hashicorp/consul-k8s/control-plane/subcommand" - "github.com/hashicorp/consul-k8s/control-plane/subcommand/common" - "github.com/hashicorp/consul-k8s/control-plane/subcommand/flags" - k8sflags "github.com/hashicorp/consul-k8s/control-plane/subcommand/flags" ) type Command struct { @@ -59,7 +58,7 @@ type Command struct { flagIngressGatewayNames []string flagTerminatingGatewayNames []string - flagCreateAPIGatewayToken bool + flagAPIGatewayController bool // Flags to configure Consul connection. flagServerAddresses []string @@ -157,7 +156,7 @@ func (c *Command) init() { "Name of a terminating gateway that needs an acl token. May be specified multiple times. "+ "[Enterprise Only] If using Consul namespaces and registering the gateway outside of the "+ "default namespace, specify the value in the form ..") - c.flags.BoolVar(&c.flagCreateAPIGatewayToken, "create-api-gateway-token", false, + c.flags.BoolVar(&c.flagAPIGatewayController, "api-gateway-controller", false, "Toggle for creating a token for the API Gateway controller integration.") c.flags.Var((*flags.AppendSliceValue)(&c.flagServerAddresses), "server-address", @@ -567,14 +566,14 @@ func (c *Command) Run(args []string) int { } } - if c.flagCreateAPIGatewayToken { - apigwRules, err := c.apiGatewayControllerRules() + if c.flagAPIGatewayController { + rules, err := c.apiGatewayControllerRules() if err != nil { c.log.Error("Error templating api gateway rules", "err", err) return 1 } - err = c.createLocalACL("api-gateway-controller", apigwRules, consulDC, primary, consulClient) - if err != nil { + serviceAccountName := c.withPrefix("api-gateway-controller") + if err := c.createACLPolicyRoleAndBindingRule("api-gateway-controller", rules, consulDC, primaryDC, localPolicy, primary, localComponentAuthMethodName, serviceAccountName, consulClient); err != nil { c.log.Error(err.Error()) return 1 } diff --git a/control-plane/subcommand/server-acl-init/command_test.go b/control-plane/subcommand/server-acl-init/command_test.go index 50dd929fe9..770e74012b 100644 --- a/control-plane/subcommand/server-acl-init/command_test.go +++ b/control-plane/subcommand/server-acl-init/command_test.go @@ -16,6 +16,16 @@ import ( "testing" "time" + "github.com/hashicorp/consul-k8s/control-plane/helper/cert" + "github.com/hashicorp/consul-k8s/control-plane/helper/go-discover/mocks" + "github.com/hashicorp/consul-k8s/control-plane/helper/test" + "github.com/hashicorp/consul-k8s/control-plane/subcommand/common" + "github.com/hashicorp/consul/api" + "github.com/hashicorp/consul/sdk/freeport" + "github.com/hashicorp/consul/sdk/testutil" + "github.com/hashicorp/consul/sdk/testutil/retry" + "github.com/hashicorp/go-discover" + "github.com/hashicorp/go-hclog" "github.com/mitchellh/cli" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -23,18 +33,6 @@ import ( k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/fake" - - "github.com/hashicorp/consul/api" - "github.com/hashicorp/consul/sdk/freeport" - "github.com/hashicorp/consul/sdk/testutil" - "github.com/hashicorp/consul/sdk/testutil/retry" - "github.com/hashicorp/go-discover" - "github.com/hashicorp/go-hclog" - - "github.com/hashicorp/consul-k8s/control-plane/helper/cert" - "github.com/hashicorp/consul-k8s/control-plane/helper/go-discover/mocks" - "github.com/hashicorp/consul-k8s/control-plane/helper/test" - "github.com/hashicorp/consul-k8s/control-plane/subcommand/common" ) var ns = "default" @@ -188,14 +186,6 @@ func TestRun_TokensPrimaryDC(t *testing.T) { SecretNames: []string{resourcePrefix + "-client-snapshot-agent-acl-token"}, LocalToken: true, }, - { - TestName: "API gateway token", - TokenFlags: []string{"-create-api-gateway-token"}, - PolicyNames: []string{"api-gateway-controller-token"}, - PolicyDCs: []string{"dc1"}, - SecretNames: []string{resourcePrefix + "-api-gateway-controller-acl-token"}, - LocalToken: true, - }, { TestName: "Mesh gateway token", TokenFlags: []string{"-create-mesh-gateway-token"}, @@ -406,14 +396,6 @@ func TestRun_TokensReplicatedDC(t *testing.T) { SecretNames: []string{resourcePrefix + "-client-snapshot-agent-acl-token"}, LocalToken: true, }, - { - TestName: "API Gateway token", - TokenFlags: []string{"-create-api-gateway-token"}, - PolicyNames: []string{"api-gateway-controller-token-dc2"}, - PolicyDCs: []string{"dc2"}, - SecretNames: []string{resourcePrefix + "-api-gateway-controller-acl-token"}, - LocalToken: true, - }, { TestName: "Mesh gateway token", TokenFlags: []string{"-create-mesh-gateway-token"}, @@ -532,12 +514,6 @@ func TestRun_TokensWithProvidedBootstrapToken(t *testing.T) { PolicyNames: []string{"client-snapshot-agent-token"}, SecretNames: []string{resourcePrefix + "-client-snapshot-agent-acl-token"}, }, - { - TestName: "API Gateway token", - TokenFlags: []string{"-create-api-gateway-token"}, - PolicyNames: []string{"api-gateway-controller-token"}, - SecretNames: []string{resourcePrefix + "-api-gateway-controller-acl-token"}, - }, { TestName: "Mesh gateway token", TokenFlags: []string{"-create-mesh-gateway-token"}, @@ -2163,6 +2139,12 @@ func TestRun_PoliciesAndBindingRulesForACLLogin_PrimaryDatacenter(t *testing.T) PolicyNames: []string{"sync-catalog-policy"}, Roles: []string{resourcePrefix + "-sync-catalog-acl-role"}, }, + { + TestName: "API Gateway Controller", + TokenFlags: []string{"-api-gateway-controller"}, + PolicyNames: []string{"api-gateway-controller-policy"}, + Roles: []string{resourcePrefix + "-api-gateway-controller-acl-role"}, + }, } for _, c := range cases { t.Run(c.TestName, func(t *testing.T) { @@ -2274,6 +2256,13 @@ func TestRun_PoliciesAndBindingRulesACLLogin_SecondaryDatacenter(t *testing.T) { Roles: []string{resourcePrefix + "-sync-catalog-acl-role-" + secondaryDatacenter}, GlobalAuthMethod: false, }, + { + TestName: "API Gateway Controller", + TokenFlags: []string{"-api-gateway-controller"}, + PolicyNames: []string{"api-gateway-controller-policy-" + secondaryDatacenter}, + Roles: []string{resourcePrefix + "-api-gateway-controller-acl-role-" + secondaryDatacenter}, + GlobalAuthMethod: false, + }, } for _, c := range cases { t.Run(c.TestName, func(t *testing.T) { @@ -2380,6 +2369,11 @@ func TestRun_ValidateLoginToken_PrimaryDatacenter(t *testing.T) { TokenFlags: []string{"-sync-catalog"}, Roles: []string{resourcePrefix + "-sync-catalog-acl-role"}, }, + { + ComponentName: "api-gateway-controller", + TokenFlags: []string{"-api-gateway-controller"}, + Roles: []string{resourcePrefix + "-api-gateway-controller-acl-role"}, + }, } for _, c := range cases { t.Run(c.ComponentName, func(t *testing.T) { @@ -2474,6 +2468,12 @@ func TestRun_ValidateLoginToken_SecondaryDatacenter(t *testing.T) { Roles: []string{resourcePrefix + "-sync-catalog-acl-role-dc2"}, GlobalAuthMethod: false, }, + { + ComponentName: "api-gateway-controller", + TokenFlags: []string{"-api-gateway-controller"}, + Roles: []string{resourcePrefix + "-api-gateway-controller-acl-role-dc2"}, + GlobalAuthMethod: false, + }, } for _, c := range cases { t.Run(c.ComponentName, func(t *testing.T) {