Skip to content
This repository has been archived by the owner on Aug 25, 2021. It is now read-only.

Commit

Permalink
Reorganize externalServers section
Browse files Browse the repository at this point in the history
* Consolidate client.join and externalServers.https.address
  under one value: externalServers.hosts. This value no longer accepts
  ports, and so if the serf port is non-default, it has to be provided
  separately. Since it no longer accepts ports, this value was renamed
  from 'address' to 'hosts'. This was largely based on RFC2396,
  which defined URL as '<userinfo>@<host>:<port>'.
* Deprecate 'client.join'.
* Add externalServers.serfLANPort.
* Rename externalServers.https.port -> externalServers.httpsPort
* Rename externalServers.https.tlsServerName ->
externalServers.tlsServerName
* Rename externalServers.https.useSystemRoots ->
externalServers.useSystemRoots
* Rename connectInject.overrideAuthMethodHost ->
externalServers.k8sAuthMethodHost
  • Loading branch information
ishustava committed Apr 23, 2020
1 parent 554f936 commit bb4506e
Show file tree
Hide file tree
Showing 27 changed files with 161 additions and 118 deletions.
16 changes: 8 additions & 8 deletions templates/_helpers.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -76,17 +76,17 @@ This template is for an init container.
consul-k8s get-consul-client-ca \
-output-file=/consul/tls/client/ca/tls.crt \
{{- if .Values.externalServers.enabled }}
{{- if not (or .Values.externalServers.https.address .Values.client.join)}}{{ fail "either client.join or externalServers.https.address must be set if externalServers.enabled is true" }}{{ end -}}
{{- if .Values.externalServers.https.address }}
-server-addr={{ .Values.externalServers.https.address }} \
{{- if not (or .Values.externalServers.hosts .Values.client.join)}}{{ fail "either client.join or externalServers.hosts must be set if externalServers.enabled is true" }}{{ end -}}
{{- if .Values.externalServers.hosts }}
-server-addr={{ quote (first .Values.externalServers.hosts) }} \
{{- else }}
-server-addr={{ quote (first .Values.client.join) }} \
{{- end }}
-server-port={{ .Values.externalServers.https.port }} \
{{- if .Values.externalServers.https.tlsServerName }}
-tls-server-name={{ .Values.externalServers.https.tlsServerName }} \
-server-port={{ .Values.externalServers.httpsPort }} \
{{- if .Values.externalServers.tlsServerName }}
-tls-server-name={{ .Values.externalServers.tlsServerName }} \
{{- end }}
{{- if not .Values.externalServers.https.useSystemRoots }}
{{- if not .Values.externalServers.useSystemRoots }}
-ca-file=/consul/tls/ca/tls.crt
{{- end }}
{{- else }}
Expand All @@ -95,7 +95,7 @@ This template is for an init container.
-ca-file=/consul/tls/ca/tls.crt
{{- end }}
volumeMounts:
{{- if not (and .Values.externalServers.enabled .Values.externalServers.https.useSystemRoots) }}
{{- if not (and .Values.externalServers.enabled .Values.externalServers.useSystemRoots) }}
- name: consul-ca-cert
mountPath: /consul/tls/ca
{{- end }}
Expand Down
7 changes: 7 additions & 0 deletions templates/client-daemonset.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -205,11 +205,18 @@ spec:
{{- if (and .Values.global.gossipEncryption.secretName .Values.global.gossipEncryption.secretKey) }}
-encrypt="${GOSSIP_KEY}" \
{{- end }}
{{- if or .Values.client.join (and .Values.externalServers.enabled .Values.externalServers.hosts) }}
{{- if .Values.client.join }}
{{- range $value := .Values.client.join }}
-retry-join={{ quote $value }} \
{{- end }}
{{- else }}
{{- $serfPort := (.Values.externalServers.serfLANPort | int) -}}
{{- range $value := .Values.externalServers.hosts }}
-retry-join={{ printf "%s:%d" $value $serfPort | quote}} \
{{- end }}
{{- end }}
{{- else }}
{{- if .Values.server.enabled }}
{{- range $index := until (.Values.server.replicas | int) }}
-retry-join=${CONSUL_FULLNAME}-server-{{ $index }}.${CONSUL_FULLNAME}-server.${NAMESPACE}.svc \
Expand Down
2 changes: 1 addition & 1 deletion templates/client-snapshot-agent-deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ spec:
emptyDir: {}
{{- end }}
{{- if .Values.global.tls.enabled }}
{{- if not (and .Values.externalServers.enabled .Values.externalServers.https.useSystemRoots) }}
{{- if not (and .Values.externalServers.enabled .Values.externalServers.useSystemRoots) }}
- name: consul-ca-cert
secret:
{{- if .Values.global.tls.caCert.secretName }}
Expand Down
2 changes: 1 addition & 1 deletion templates/connect-inject-deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ spec:
secretName: {{ .Values.connectInject.certs.secretName }}
{{- end }}
{{- if .Values.global.tls.enabled }}
{{- if not (and .Values.externalServers.enabled .Values.externalServers.https.useSystemRoots) }}
{{- if not (and .Values.externalServers.enabled .Values.externalServers.useSystemRoots) }}
- name: consul-ca-cert
secret:
{{- if .Values.global.tls.caCert.secretName }}
Expand Down
2 changes: 1 addition & 1 deletion templates/mesh-gateway-deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ spec:
emptyDir:
medium: "Memory"
{{- if .Values.global.tls.enabled }}
{{- if not (and .Values.externalServers.enabled .Values.externalServers.https.useSystemRoots) }}
{{- if not (and .Values.externalServers.enabled .Values.externalServers.useSystemRoots) }}
- name: consul-ca-cert
secret:
{{- if .Values.global.tls.caCert.secretName }}
Expand Down
2 changes: 1 addition & 1 deletion templates/server-acl-init-cleanup-podsecuritypolicy.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{{- $serverEnabled := (or (and (ne (.Values.server.enabled | toString) "-") .Values.server.enabled) (and (eq (.Values.server.enabled | toString) "-") .Values.global.enabled)) -}}
{{- if (or $serverEnabled .Values.externalServers.enabled) }}
{{- if (and $serverEnabled .Values.externalServers.enabled) }}{{ fail "only one of server.enabled or externalServers.enabled can be set" }}{{ end -}}
{{- if (or $serverEnabled .Values.externalServers.enabled) }}
{{- if (or .Values.global.acls.manageSystemACLs .Values.global.bootstrapACLs) }}
{{- if .Values.global.enablePodSecurityPolicies }}
apiVersion: policy/v1beta1
Expand Down
2 changes: 1 addition & 1 deletion templates/server-acl-init-clusterrole.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,4 @@ rules:
- use
{{- end }}
{{- end }}
{{- end }}
{{- end }}
16 changes: 9 additions & 7 deletions templates/server-acl-init-job.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -97,15 +97,17 @@ spec:
consul-k8s server-acl-init \
{{- if .Values.externalServers.enabled }}
{{- if not (or .Values.externalServers.https.address .Values.client.join)}}{{ fail "either client.join or externalServers.https.address must be set if externalServers.enabled is true" }}{{ end -}}
{{- if .Values.externalServers.https.address }}
-server-address={{ .Values.externalServers.https.address }} \
{{- if not (or .Values.externalServers.hosts .Values.client.join)}}{{ fail "either client.join or externalServers.hosts must be set if externalServers.enabled is true" }}{{ end -}}
{{- if .Values.externalServers.hosts }}
{{- range .Values.externalServers.hosts }}
-server-address={{ . }} \
{{- end }}
{{- else }}
{{- range .Values.client.join }}
-server-address={{ . }} \
{{- end }}
{{- end }}
-server-port={{ .Values.externalServers.https.port }} \
-server-port={{ .Values.externalServers.httpsPort }} \
{{- else }}
{{- range $index := until (.Values.server.replicas | int) }}
-server-address="${CONSUL_FULLNAME}-server-{{ $index }}.${CONSUL_FULLNAME}-server.${NAMESPACE}.svc" \
Expand All @@ -115,7 +117,7 @@ spec:
-k8s-namespace={{ .Release.Namespace }} \
{{- if .Values.global.tls.enabled }}
-use-https \
{{- if not (and .Values.externalServers.enabled .Values.externalServers.https.useSystemRoots) }}
{{- if not (and .Values.externalServers.enabled .Values.externalServers.useSystemRoots) }}
-consul-ca-cert=/consul/tls/ca/tls.crt \
{{- end }}
{{- if not .Values.externalServers.enabled }}
Expand All @@ -130,8 +132,8 @@ spec:
{{- end }}
{{- if .Values.connectInject.enabled }}
-create-inject-auth-method=true \
{{- if .Values.connectInject.overrideAuthMethodHost }}
-inject-auth-method-host={{ .Values.connectInject.overrideAuthMethodHost }} \
{{- if and .Values.externalServers.enabled .Values.externalServers.k8sAuthMethodHost }}
-inject-auth-method-host={{ .Values.externalServers.k8sAuthMethodHost }} \
{{- end }}
{{- end }}
{{- if .Values.meshGateway.enabled }}
Expand Down
2 changes: 1 addition & 1 deletion templates/sync-catalog-deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ spec:
serviceAccountName: {{ template "consul.fullname" . }}-sync-catalog
{{- if .Values.global.tls.enabled }}
volumes:
{{- if not (and .Values.externalServers.enabled .Values.externalServers.https.useSystemRoots) }}
{{- if not (and .Values.externalServers.enabled .Values.externalServers.useSystemRoots) }}
- name: consul-ca-cert
secret:
{{- if .Values.global.tls.caCert.secretName }}
Expand Down
2 changes: 1 addition & 1 deletion templates/tests/test-runner.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ metadata:
spec:
{{- if .Values.global.tls.enabled }}
volumes:
{{- if not (and .Values.externalServers.enabled .Values.externalServers.https.useSystemRoots) }}
{{- if not (and .Values.externalServers.enabled .Values.externalServers.useSystemRoots) }}
- name: consul-ca-cert
secret:
{{- if .Values.global.tls.caCert.secretName }}
Expand Down
28 changes: 27 additions & 1 deletion test/unit/client-daemonset.bats
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ load _helpers
#--------------------------------------------------------------------
# retry-join

@test "client/DaemonSet: retry join gets populated" {
@test "client/DaemonSet: retry join gets populated by default" {
cd `chart_dir`
local actual=$(helm template \
-x templates/client-daemonset.yaml \
Expand All @@ -86,6 +86,32 @@ load _helpers
[ "${actual}" = "true" ]
}

@test "client/DaemonSet: retry join gets populated when client.join is set" {
cd `chart_dir`
local actual=$(helm template \
-x templates/client-daemonset.yaml \
--set 'server.enabled=false' \
--set 'externalServers.enabled=true' \
--set 'client.join[0]=1.1.1.1' \
. | tee /dev/stderr |
yq -r '.spec.template.spec.containers[0].command | any(contains("-retry-join=\"1.1.1.1\""))' | tee /dev/stderr)

[ "${actual}" = "true" ]
}

@test "client/DaemonSet: retry join gets populated when externalServers.hosts is set" {
cd `chart_dir`
local actual=$(helm template \
-x templates/client-daemonset.yaml \
--set 'server.enabled=false' \
--set 'externalServers.enabled=true' \
--set 'externalServers.hosts[0]=1.1.1.1' \
. | tee /dev/stderr |
yq -r '.spec.template.spec.containers[0].command | any(contains("-retry-join=\"1.1.1.1:8301\""))' | tee /dev/stderr)

[ "${actual}" = "true" ]
}

#--------------------------------------------------------------------
# grpc

Expand Down
6 changes: 3 additions & 3 deletions test/unit/client-snapshot-agent-deployment.bats
Original file line number Diff line number Diff line change
Expand Up @@ -322,15 +322,15 @@ load _helpers
[ "${actual}" = "true" ]
}

@test "client/SnapshotAgentDeployment: consul-ca-cert volume is not added if externalServers.enabled=true and externalServers.https.useSystemRoots=true" {
@test "client/SnapshotAgentDeployment: consul-ca-cert volume is not added if externalServers.enabled=true and externalServers.useSystemRoots=true" {
cd `chart_dir`
local actual=$(helm template \
-x templates/client-snapshot-agent-deployment.yaml \
--set 'client.snapshotAgent.enabled=true' \
--set 'global.tls.enabled=true' \
--set 'externalServers.enabled=true' \
--set 'externalServers.https.address=foo.com' \
--set 'externalServers.https.useSystemRoots=true' \
--set 'externalServers.hosts[0]=foo.com' \
--set 'externalServers.useSystemRoots=true' \
. | tee /dev/stderr |
yq '.spec.template.spec.volumes[] | select(.name == "consul-ca-cert")' | tee /dev/stderr)
[ "${actual}" = "" ]
Expand Down
6 changes: 3 additions & 3 deletions test/unit/connect-inject-deployment.bats
Original file line number Diff line number Diff line change
Expand Up @@ -486,16 +486,16 @@ load _helpers
[ "${actual}" = "true" ]
}

@test "connectInject/Deployment: consul-ca-cert volume is not added if externalServers.enabled=true and externalServers.https.useSystemRoots=true" {
@test "connectInject/Deployment: consul-ca-cert volume is not added if externalServers.enabled=true and externalServers.useSystemRoots=true" {
cd `chart_dir`
local actual=$(helm template \
-x templates/connect-inject-deployment.yaml \
--set 'connectInject.enabled=true' \
--set 'global.tls.enabled=true' \
--set 'global.tls.enableAutoEncrypt=true' \
--set 'externalServers.enabled=true' \
--set 'externalServers.https.address=foo.com' \
--set 'externalServers.https.useSystemRoots=true' \
--set 'externalServers.hosts[0]=foo.com' \
--set 'externalServers.useSystemRoots=true' \
. | tee /dev/stderr |
yq '.spec.template.spec.volumes[] | select(.name == "consul-ca-cert")' | tee /dev/stderr)
[ "${actual}" = "" ]
Expand Down
30 changes: 15 additions & 15 deletions test/unit/helpers.bats
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ load _helpers
[ "${actual}" = "true" ]
}

@test "helper/consul.getAutoEncryptClientCA: uses client.join string if externalServers.enabled is true but the address is not provided" {
@test "helper/consul.getAutoEncryptClientCA: uses client.join string if externalServers.enabled is true but the hosts are not provided" {
cd `chart_dir`
local command=$(helm template \
-x templates/tests/test-runner.yaml \
Expand All @@ -157,20 +157,20 @@ load _helpers
[ "${actual}" = "true" ]
}

@test "helper/consul.getAutoEncryptClientCA: can set the provided server address if externalServers.enabled is true" {
@test "helper/consul.getAutoEncryptClientCA: can set the provided server hosts if externalServers.enabled is true" {
cd `chart_dir`
local command=$(helm template \
-x templates/tests/test-runner.yaml \
--set 'global.tls.enabled=true' \
--set 'global.tls.enableAutoEncrypt=true' \
--set 'server.enabled=false' \
--set 'externalServers.enabled=true' \
--set 'externalServers.https.address=consul.io' \
--set 'externalServers.hosts[0]=consul.io' \
. | tee /dev/stderr |
yq '.spec.initContainers[] | select(.name == "get-auto-encrypt-client-ca").command | join(" ")' | tee /dev/stderr)

# check server address
actual=$(echo $command | jq ' . | contains("-server-addr=consul.io")')
actual=$(echo $command | jq ' . | contains("-server-addr=\"consul.io\"")')
[ "${actual}" = "true" ]

# check the default server port is 443 if not provided
Expand All @@ -182,15 +182,15 @@ load _helpers
[ "${actual}" = "true" ]
}

@test "helper/consul.getAutoEncryptClientCA: fails if externalServers.enabled is true but neither client.join nor externalServers.https.address are provided" {
@test "helper/consul.getAutoEncryptClientCA: fails if externalServers.enabled is true but neither client.join nor externalServers.hosts[0] are provided" {
cd `chart_dir`
run helm template \
-x templates/tests/test-runner.yaml \
--set 'global.tls.enabled=true' \
--set 'global.tls.enableAutoEncrypt=true' \
--set 'externalServers.enabled=true' .
[ "$status" -eq 1 ]
[[ "$output" =~ "either client.join or externalServers.https.address must be set if externalServers.enabled is true" ]]
[[ "$output" =~ "either client.join or externalServers.hosts must be set if externalServers.enabled is true" ]]
}

@test "helper/consul.getAutoEncryptClientCA: can set the provided port if externalServers.enabled is true" {
Expand All @@ -201,13 +201,13 @@ load _helpers
--set 'global.tls.enableAutoEncrypt=true' \
--set 'server.enabled=false' \
--set 'externalServers.enabled=true' \
--set 'externalServers.https.address=consul.io' \
--set 'externalServers.https.port=8501' \
--set 'externalServers.hosts[0]=consul.io' \
--set 'externalServers.httpsPort=8501' \
. | tee /dev/stderr |
yq '.spec.initContainers[] | select(.name == "get-auto-encrypt-client-ca").command | join(" ")' | tee /dev/stderr)

# check server address
actual=$(echo $command | jq ' . | contains("-server-addr=consul.io")')
actual=$(echo $command | jq ' . | contains("-server-addr=\"consul.io\"")')
[ "${actual}" = "true" ]

# check the default server port is 443 if not provided
Expand All @@ -227,8 +227,8 @@ load _helpers
--set 'global.tls.enableAutoEncrypt=true' \
--set 'server.enabled=false' \
--set 'externalServers.enabled=true' \
--set 'externalServers.https.address=consul.io' \
--set 'externalServers.https.tlsServerName=custom-server-name' \
--set 'externalServers.hosts[0]=consul.io' \
--set 'externalServers.tlsServerName=custom-server-name' \
. | tee /dev/stderr |
yq '.spec.initContainers[] | select(.name == "get-auto-encrypt-client-ca").command | join(" ") | contains("-tls-server-name=custom-server-name")' | tee /dev/stderr)

Expand All @@ -243,8 +243,8 @@ load _helpers
--set 'global.tls.enableAutoEncrypt=true' \
--set 'server.enabled=false' \
--set 'externalServers.enabled=true' \
--set 'externalServers.https.address=consul.io' \
--set 'externalServers.https.useSystemRoots=true' \
--set 'externalServers.hosts[0]=consul.io' \
--set 'externalServers.useSystemRoots=true' \
. | tee /dev/stderr |
yq '.spec.initContainers[] | select(.name == "get-auto-encrypt-client-ca").command | join(" ") | contains("-ca-file=/consul/tls/ca/tls.crt")' | tee /dev/stderr)

Expand All @@ -259,8 +259,8 @@ load _helpers
--set 'global.tls.enableAutoEncrypt=true' \
--set 'server.enabled=false' \
--set 'externalServers.enabled=true' \
--set 'externalServers.https.address=consul.io' \
--set 'externalServers.https.useSystemRoots=true' \
--set 'externalServers.hosts[0]=consul.io' \
--set 'externalServers.useSystemRoots=true' \
. | tee /dev/stderr |
yq '.spec.initContainers[] | select(.name == "get-auto-encrypt-client-ca").volumeMounts[] | select(.name=="consul-ca-cert")' | tee /dev/stderr)

Expand Down
6 changes: 3 additions & 3 deletions test/unit/mesh-gateway-deployment.bats
Original file line number Diff line number Diff line change
Expand Up @@ -601,7 +601,7 @@ key2: value2' \
[ "${actual}" = "true" ]
}

@test "meshGateway/Deployment: consul-ca-cert volume is not added if externalServers.enabled=true and externalServers.https.useSystemRoots=true" {
@test "meshGateway/Deployment: consul-ca-cert volume is not added if externalServers.enabled=true and externalServers.useSystemRoots=true" {
cd `chart_dir`
local actual=$(helm template \
-x templates/mesh-gateway-deployment.yaml \
Expand All @@ -610,8 +610,8 @@ key2: value2' \
--set 'global.tls.enabled=true' \
--set 'global.tls.enableAutoEncrypt=true' \
--set 'externalServers.enabled=true' \
--set 'externalServers.https.address=foo.com' \
--set 'externalServers.https.useSystemRoots=true' \
--set 'externalServers.hosts[0]=foo.com' \
--set 'externalServers.useSystemRoots=true' \
. | tee /dev/stderr |
yq '.spec.template.spec.volumes[] | select(.name == "consul-ca-cert")' | tee /dev/stderr)
[ "${actual}" = "" ]
Expand Down
2 changes: 1 addition & 1 deletion test/unit/server-acl-init-cleanup-clusterrole.bats
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ load _helpers
--set 'server.enabled=false' \
--set 'global.acls.manageSystemACLs=true' \
--set 'externalServers.enabled=true' \
--set 'externalServers.https.address=foo.com' \
--set 'externalServers.hosts[0]=foo.com' \
. | tee /dev/stderr |
yq 'length > 0' | tee /dev/stderr)
[ "${actual}" = "true" ]
Expand Down
2 changes: 1 addition & 1 deletion test/unit/server-acl-init-cleanup-clusterrolebinding.bats
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ load _helpers
--set 'server.enabled=false' \
--set 'global.acls.manageSystemACLs=true' \
--set 'externalServers.enabled=true' \
--set 'externalServers.https.address=foo.com' \
--set 'externalServers.hosts[0]=foo.com' \
. | tee /dev/stderr |
yq 'length > 0' | tee /dev/stderr)
[ "${actual}" = "true" ]
Expand Down
2 changes: 1 addition & 1 deletion test/unit/server-acl-init-cleanup-job.bats
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ load _helpers
--set 'server.enabled=false' \
--set 'global.acls.manageSystemACLs=true' \
--set 'externalServers.enabled=true' \
--set 'externalServers.https.address=foo.com' \
--set 'externalServers.hosts[0]=foo.com' \
. | tee /dev/stderr |
yq 'length > 0' | tee /dev/stderr)
[ "${actual}" = "true" ]
Expand Down
2 changes: 1 addition & 1 deletion test/unit/server-acl-init-cleanup-podsecuritypolicy.bats
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ load _helpers
--set 'global.acls.manageSystemACLs=true' \
--set 'global.enablePodSecurityPolicies=true' \
--set 'externalServers.enabled=true' \
--set 'externalServers.https.address=foo.com' \
--set 'externalServers.hosts[0]=foo.com' \
. | tee /dev/stderr |
yq 'length > 0' | tee /dev/stderr)
[ "${actual}" = "true" ]
Expand Down
Loading

0 comments on commit bb4506e

Please sign in to comment.