Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable ACL Client Token #1093

Merged
merged 19 commits into from
Mar 15, 2022
Merged

Enable ACL Client Token #1093

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
4dc7dfa
Refactor ConsulLogin() to return the acltoken in addition to theerror.
jmurret Mar 10, 2022
7c5223f
Refactor createACLPolicyRoleAndBindingRule toappend datacenters for l…
jmurret Mar 10, 2022
88379a1
Rename -create-client-token flag to -client
jmurret Mar 10, 2022
7746ad7
set additional sans for consul server load balancer so that client wi…
jmurret Mar 10, 2022
492c8a0
Refactor server-acl-init command to create ACL Policy and Rule for cl…
jmurret Mar 10, 2022
be58c6d
Enable client to talk to Consul Server to perform consul login.
jmurret Mar 10, 2022
b7d9fa3
Configure client-daemonset so that we can utilize the externalServers…
jmurret Mar 10, 2022
de48da0
Configuring partition-init to remove additional flags and use ones th…
jmurret Mar 10, 2022
f2b59af
adding missing comma
jmurret Mar 10, 2022
1cac8f7
fix flakey tests by wrapping asserts in retries a la Iryna
jmurret Mar 11, 2022
19d0064
Adding -use-https flag to client-daemonset.yaml when externalServers …
jmurret Mar 14, 2022
2c76ee6
Refactoring tests to cover client-acl-init changes
jmurret Mar 14, 2022
eb41a15
addressing PR comments
jmurret Mar 15, 2022
14200d1
removing mounted tmpfs for consul-ca-cert when using vault and restor…
jmurret Mar 15, 2022
10c9b8c
addressing PR comments and only appending datacenters to a policy whe…
jmurret Mar 15, 2022
c690c57
completing additional dns names based on PR feedback
jmurret Mar 15, 2022
653f819
Do not ca-cert volume when using vault.
jmurret Mar 15, 2022
1fadcb4
removing unused flagConsulCACert from partition-init command
jmurret Mar 15, 2022
b3a3ac9
PR Feedback. Removing unused envvars in acl-init container. changin…
jmurret Mar 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion charts/consul/templates/_helpers.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ as well as the global.name setting.
{{- define "consul.serverTLSAltNames" -}}
{{- $name := include "consul.fullname" . -}}
{{- $ns := .Release.Namespace -}}
{{ printf "localhost,%s-server,*.%s-server,*.%s-server.%s,*.%s-server.%s.svc,*.server.%s.%s" $name $name $name $ns $name $ns (.Values.global.datacenter ) (.Values.global.domain) }}{{ include "consul.serverAdditionalDNSSANs" . }}
{{ printf "localhost,%s-server,*.%s-server,*.%s-server.%s,%s-server.%s,*.%s-server.%s.svc,%s-server.%s.svc,*.server.%s.%s" $name $name $name $ns $name $ns $name $ns $name $ns (.Values.global.datacenter ) (.Values.global.domain) }}{{ include "consul.serverAdditionalDNSSANs" . }}
{{- end -}}

{{- define "consul.serverAdditionalDNSSANs" -}}
Expand Down
70 changes: 66 additions & 4 deletions charts/consul/templates/client-daemonset.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
{{- if and .Values.global.federation.enabled .Values.global.adminPartitions.enabled }}{{ fail "If global.federation.enabled is true, global.adminPartitions.enabled must be false because they are mutually exclusive" }}{{ end }}
{{- if (and .Values.global.enterpriseLicense.secretName (not .Values.global.enterpriseLicense.secretKey)) }}{{fail "enterpriseLicense.secretKey and secretName must both be specified." }}{{ end -}}
{{- if (and (not .Values.global.enterpriseLicense.secretName) .Values.global.enterpriseLicense.secretKey) }}{{fail "enterpriseLicense.secretKey and secretName must both be specified." }}{{ end -}}
{{- if and .Values.externalServers.enabled (not .Values.externalServers.hosts) }}{{ fail "externalServers.hosts must be set if externalServers.enabled is true" }}{{ end -}}
# DaemonSet to run the Consul clients on every node.
apiVersion: apps/v1
kind: DaemonSet
Expand Down Expand Up @@ -48,6 +49,7 @@ spec:
annotations:
{{- if .Values.global.secretsBackend.vault.enabled }}
"vault.hashicorp.com/agent-inject": "true"
"vault.hashicorp.com/agent-init-first": "true"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need vault agent injector to fun before client-acl-init initContainer.

"vault.hashicorp.com/role": "{{ .Values.global.secretsBackend.vault.consulClientRole }}"
{{- if and .Values.global.secretsBackend.vault.ca.secretName .Values.global.secretsBackend.vault.ca.secretKey }}
"vault.hashicorp.com/agent-extra-secret": "{{ .Values.global.secretsBackend.vault.ca.secretName }}"
Expand Down Expand Up @@ -124,6 +126,9 @@ spec:
- name: config
configMap:
name: {{ template "consul.fullname" . }}-client-config
- name: consul-data
emptyDir:
medium: "Memory"
{{- if .Values.global.tls.enabled }}
{{- if not .Values.global.secretsBackend.vault.enabled }}
- name: consul-ca-cert
Expand All @@ -136,7 +141,8 @@ spec:
items:
- key: {{ default "tls.crt" .Values.global.tls.caCert.secretKey }}
path: tls.crt
{{ if not .Values.global.tls.enableAutoEncrypt }}
{{- end }}
{{- if (and (not .Values.global.secretsBackend.vault.enabled) (not .Values.global.tls.enableAutoEncrypt)) }}
- name: consul-ca-key
secret:
{{- if .Values.global.tls.caKey.secretName }}
Expand All @@ -154,7 +160,6 @@ spec:
medium: "Memory"
{{- end }}
{{- end }}
{{- end }}
{{- range .Values.client.extraVolumes }}
- name: userconfig-{{ .name }}
{{ .type }}:
Expand All @@ -177,7 +182,21 @@ spec:
containers:
- name: consul
image: "{{ default .Values.global.image .Values.client.image }}"
{{- if .Values.global.acls.manageSystemACLs }}
lifecycle:
preStop:
exec:
command:
- "/bin/sh"
- "-ec"
- |
consul logout
{{- end }}
env:
{{- if .Values.global.acls.manageSystemACLs }}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need this also for the logout command in the preStop above.

- name: CONSUL_HTTP_TOKEN_FILE
value: "/consul/login/acl-token"
{{- end }}
- name: ADVERTISE_IP
valueFrom:
fieldRef:
Expand Down Expand Up @@ -340,6 +359,9 @@ spec:
mountPath: /consul/data
- name: config
mountPath: /consul/config
- mountPath: /consul/login
name: consul-data
readOnly: true
{{- if .Values.global.tls.enabled }}
{{- if not .Values.global.secretsBackend.vault.enabled }}
- name: consul-ca-cert
Expand Down Expand Up @@ -435,17 +457,57 @@ spec:
{{- if .Values.global.acls.manageSystemACLs }}
- name: client-acl-init
image: {{ .Values.global.imageK8S }}
env:
- name: CONSUL_HTTP_ADDR
{{- if .Values.global.tls.enabled }}
value: https://{{ template "consul.fullname" . }}-server.{{ .Release.Namespace }}.svc:8501
{{- else }}
value: http://{{ template "consul.fullname" . }}-server.{{ .Release.Namespace }}.svc:8500
{{- end }}
{{- if (and .Values.global.tls.enabled (not .Values.externalServers.useSystemRoots)) }}
- name: CONSUL_CACERT
{{- if .Values.global.secretsBackend.vault.enabled }}
value: "/vault/secrets/serverca.crt"
{{- else }}
value: "/consul/tls/ca/tls.crt"
{{- end }}
{{- end }}
command:
- "/bin/sh"
- "-ec"
- |
consul-k8s-control-plane acl-init \
-secret-name="{{ template "consul.fullname" . }}-client-acl-token" \
-k8s-namespace={{ .Release.Namespace }} \
-component-name=client \
-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.client.logLevel }} \
-log-json={{ .Values.global.logJSON }} \
{{- if .Values.externalServers.enabled }}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this follows prior art of partition-init-job and tls-init-job. In a partition scenario, the client container needs to talk directly to the servers in another DC and external servers will be configured. When configured, client-acl-init should use them.

{{- if .Values.global.tls.enabled }}
-use-https \
{{- end }}
{{- range .Values.externalServers.hosts }}
-server-address={{ quote . }} \
{{- end }}
-server-port={{ .Values.externalServers.httpsPort }} \
{{- if .Values.externalServers.tlsServerName }}
-tls-server-name={{ .Values.externalServers.tlsServerName }} \
{{- end }}
{{- end }}
-init-type="client"
volumeMounts:
- name: aclconfig
mountPath: /consul/aclconfig
- mountPath: /consul/login
name: consul-data
readOnly: false
{{- if (and (not .Values.global.secretsBackend.vault.enabled) (not .Values.externalServers.useSystemRoots)) }}
- name: consul-ca-cert
mountPath: /consul/tls/ca
readOnly: false
{{- end }}
resources:
requests:
memory: "25Mi"
Expand Down
4 changes: 2 additions & 2 deletions charts/consul/templates/partition-init-job.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,10 @@ spec:
{{- if .Values.global.tls.enabled }}
-use-https \
{{- if not .Values.externalServers.useSystemRoots }}
-consul-ca-cert=/consul/tls/ca/tls.crt \
-ca-file=/consul/tls/ca/tls.crt \
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two flags are already available in c.http so we will use those instead of having two similar, unnecessary flags.

{{- end }}
{{- if .Values.externalServers.tlsServerName }}
-consul-tls-server-name={{ .Values.externalServers.tlsServerName }} \
-tls-server-name={{ .Values.externalServers.tlsServerName }} \
{{- end }}
{{- end }}
-partition-name={{ .Values.global.adminPartitions.name }}
Expand Down
2 changes: 1 addition & 1 deletion charts/consul/templates/server-acl-init-job.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ spec:
{{- end }}

{{- if not (or (and (ne (.Values.client.enabled | toString) "-") .Values.client.enabled) (and (eq (.Values.client.enabled | toString) "-") .Values.global.enabled)) }}
-create-client-token=false \
-client=false \
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing flag to the new flag convention.

{{- end }}

{{- if .Values.global.acls.createReplicationToken }}
Expand Down
2 changes: 2 additions & 0 deletions charts/consul/templates/tls-init-job.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,9 @@ spec:
-additional-dnsname="{{ template "consul.fullname" . }}-server" \
-additional-dnsname="*.{{ template "consul.fullname" . }}-server" \
-additional-dnsname="*.{{ template "consul.fullname" . }}-server.${NAMESPACE}" \
-additional-dnsname="{{ template "consul.fullname" . }}-server.${NAMESPACE}" \
-additional-dnsname="*.{{ template "consul.fullname" . }}-server.${NAMESPACE}.svc" \
-additional-dnsname="{{ template "consul.fullname" . }}-server.${NAMESPACE}.svc" \
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Modifying SANs in cert so client pod can talk to server load balancer.

ishustava marked this conversation as resolved.
Show resolved Hide resolved
-additional-dnsname="*.server.{{ .Values.global.datacenter }}.{{ .Values.global.domain }}" \
{{- range .Values.global.tls.serverAdditionalIPSANs }}
-additional-ipaddress={{ . }} \
Expand Down
Loading