Skip to content

Commit

Permalink
configure -tls-server-name when global.cloud.enabled=true so that it …
Browse files Browse the repository at this point in the history
…matches the server certificate minted from HCP (#1591)

* release 1.0.0-beta1

* configure -tls-server-name in cloud so that it matches the server certificate minted from HCP

* updating references to consul-tls-server-name to use -tls-server-name

Co-authored-by: Kyle Schochenmaier <kschoche@gmail.com>
  • Loading branch information
jmurret and kschoche authored Oct 6, 2022
1 parent 452cf93 commit 14528e1
Show file tree
Hide file tree
Showing 18 changed files with 137 additions and 10 deletions.
3 changes: 3 additions & 0 deletions charts/consul/templates/connect-inject-deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,9 @@ spec:
-default-consul-sidecar-cpu-request={{ $consulSidecarResources.requests.cpu }} \
{{- end }}
{{- end }}
{{- if .Values.global.cloud.enabled }}
-tls-server-name=server.{{ .Values.global.datacenter}}.{{ .Values.global.domain}} \
{{- end }}
startupProbe:
httpGet:
path: /readyz/ready
Expand Down
3 changes: 3 additions & 0 deletions charts/consul/templates/controller-deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,9 @@ spec:
-consul-cross-namespace-acl-policy=cross-namespace-policy \
{{- end }}
{{- end }}
{{- if .Values.global.cloud.enabled }}
-tls-server-name=server.{{ .Values.global.datacenter}}.{{ .Values.global.domain}} \
{{- end }}
env:
- name: NAMESPACE
valueFrom:
Expand Down
4 changes: 3 additions & 1 deletion charts/consul/templates/ingress-gateways-deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,9 @@ spec:
-ca-certs=/consul/tls/ca/tls.crt \
{{- end }}
{{- if and $root.Values.externalServers.enabled $root.Values.externalServers.tlsServerName }}
-tls-server-name={{$root.Values.externalServers.tlsServerName }} \
-tls-server-name={{ $root.Values.externalServers.tlsServerName }} \
{{- else if $root.Values.global.cloud.enabled }}
-tls-server-name=server.{{ $root.Values.global.datacenter}}.{{ $root.Values.global.domain}} \
{{- end }}
{{- else }}
-tls-disabled \
Expand Down
2 changes: 2 additions & 0 deletions charts/consul/templates/mesh-gateway-deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,8 @@ spec:
{{- end }}
{{- if and .Values.externalServers.enabled .Values.externalServers.tlsServerName }}
-tls-server-name={{.Values.externalServers.tlsServerName }} \
{{- else if .Values.global.cloud.enabled }}
-tls-server-name=server.{{ .Values.global.datacenter}}.{{ .Values.global.domain}} \
{{- end }}
{{- else }}
-tls-disabled \
Expand Down
3 changes: 3 additions & 0 deletions charts/consul/templates/partition-init-job.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,9 @@ spec:
consul-k8s-control-plane partition-init \
-log-level={{ .Values.global.logLevel }} \
-log-json={{ .Values.global.logJSON }} \
{{- if .Values.global.cloud.enabled }}
-tls-server-name=server.{{ .Values.global.datacenter}}.{{ .Values.global.domain}} \
{{- end }}
resources:
requests:
memory: "50Mi"
Expand Down
7 changes: 3 additions & 4 deletions charts/consul/templates/server-acl-init-job.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -149,9 +149,6 @@ spec:
-resource-prefix=${CONSUL_FULLNAME} \
-k8s-namespace={{ .Release.Namespace }} \
-set-server-tokens={{ $serverEnabled }} \
{{- if .Values.global.cloud.enabled }}
-consul-tls-server-name=server.{{ .Values.global.datacenter}}.{{ .Values.global.domain}} \
{{- end}}
-consul-api-timeout={{ .Values.global.consulAPITimeout }} \
{{- if .Values.externalServers.enabled }}
Expand Down Expand Up @@ -179,7 +176,9 @@ spec:
-server-port=8501 \
{{- end }}
{{- if .Values.externalServers.tlsServerName }}
-consul-tls-server-name={{ .Values.externalServers.tlsServerName }} \
-tls-server-name={{ .Values.externalServers.tlsServerName }} \
{{- else if .Values.global.cloud.enabled }}
-tls-server-name=server.{{ .Values.global.datacenter}}.{{ .Values.global.domain}} \
{{- end }}
{{- end }}
Expand Down
2 changes: 2 additions & 0 deletions charts/consul/templates/terminating-gateways-deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,8 @@ spec:
{{- end }}
{{- if and $root.Values.externalServers.enabled $root.Values.externalServers.tlsServerName }}
-tls-server-name={{$root.Values.externalServers.tlsServerName }} \
{{- else if $root.Values.global.cloud.enabled }}
-tls-server-name=server.{{ $root.Values.global.datacenter}}.{{ $root.Values.global.domain}} \
{{- end }}
{{- else }}
-tls-disabled \
Expand Down
14 changes: 14 additions & 0 deletions charts/consul/test/unit/connect-inject-deployment.bats
Original file line number Diff line number Diff line change
Expand Up @@ -2352,3 +2352,17 @@ reservedNameTest() {
[ "$status" -eq 1 ]
[[ "$output" =~ "When global.cloud.enabled is true, global.cloud.secretName must also be set." ]]
}

@test "connectInject/Deployment: sets TLS server name if global.cloud.enabled is set" {
cd `chart_dir`
local actual=$(helm template \
-s templates/connect-inject-deployment.yaml \
--set 'connectInject.enabled=true' \
--set 'global.tls.enabled=true' \
--set 'global.tls.enableAutoEncrypt=true' \
--set 'global.cloud.enabled=true' \
--set 'global.cloud.secretName=blah' \
. | tee /dev/stderr |
yq '.spec.template.spec.containers[0].command | any(contains("-tls-server-name=server.dc1.consul"))' | tee /dev/stderr)
[ "${actual}" = "true" ]
}
14 changes: 14 additions & 0 deletions charts/consul/test/unit/controller-deployment.bats
Original file line number Diff line number Diff line change
Expand Up @@ -859,3 +859,17 @@ load _helpers
[ "$status" -eq 1 ]
[[ "$output" =~ "When global.cloud.enabled is true, global.cloud.secretName must also be set." ]]
}

@test "controller/Deployment: sets TLS server name if global.cloud.enabled is set" {
cd `chart_dir`
local actual=$(helm template \
-s templates/controller-deployment.yaml \
--set 'controller.enabled=true' \
--set 'global.tls.enabled=true' \
--set 'global.tls.enableAutoEncrypt=true' \
--set 'global.cloud.enabled=true' \
--set 'global.cloud.secretName=blah' \
. | tee /dev/stderr |
yq '.spec.template.spec.containers[0].command | any(contains("-tls-server-name=server.dc1.consul"))' | tee /dev/stderr)
[ "${actual}" = "true" ]
}
17 changes: 17 additions & 0 deletions charts/consul/test/unit/ingress-gateways-deployment.bats
Original file line number Diff line number Diff line change
Expand Up @@ -1162,3 +1162,20 @@ key2: value2' \
[ "$status" -eq 1 ]
[[ "$output" =~ "When global.cloud.enabled is true, global.cloud.secretName must also be set." ]]
}

@test "ingressGateways/Deployment: sets TLS server name if global.cloud.enabled is set" {
cd `chart_dir`
local actual=$(helm template \
-s templates/ingress-gateways-deployment.yaml \
--set 'ingressGateways.enabled=true' \
--set 'connectInject.enabled=true' \
--set 'ingressGateways.defaults.terminationGracePeriodSeconds=5' \
--set 'ingressGateways.gateways[0].name=gateway1' \
--set 'global.tls.enabled=true' \
--set 'global.tls.enableAutoEncrypt=true' \
--set 'global.cloud.enabled=true' \
--set 'global.cloud.secretName=blah' \
. | tee /dev/stderr |
yq '.spec.template.spec.containers[0].command | any(contains("-tls-server-name=server.dc1.consul"))' | tee /dev/stderr)
[ "${actual}" = "true" ]
}
15 changes: 15 additions & 0 deletions charts/consul/test/unit/mesh-gateway-deployment.bats
Original file line number Diff line number Diff line change
Expand Up @@ -1353,3 +1353,18 @@ key2: value2' \
[ "$status" -eq 1 ]
[[ "$output" =~ "When global.cloud.enabled is true, global.cloud.secretName must also be set." ]]
}

@test "meshGateway/Deployment: sets TLS server name if global.cloud.enabled is set" {
cd `chart_dir`
local actual=$(helm template \
-s templates/mesh-gateway-deployment.yaml \
--set 'connectInject.enabled=true' \
--set 'meshGateway.enabled=true' \
--set 'global.tls.enabled=true' \
--set 'global.tls.enableAutoEncrypt=true' \
--set 'global.cloud.enabled=true' \
--set 'global.cloud.secretName=blah' \
. | tee /dev/stderr |
yq '.spec.template.spec.containers[0].command | any(contains("-tls-server-name=server.dc1.consul"))' | tee /dev/stderr)
[ "${actual}" = "true" ]
}
24 changes: 24 additions & 0 deletions charts/consul/test/unit/partition-init-job.bats
Original file line number Diff line number Diff line change
Expand Up @@ -561,3 +561,27 @@ reservedNameTest() {
yq -r '.spec.template.metadata.annotations.foo' | tee /dev/stderr)
[ "${actual}" = "bar" ]
}

#--------------------------------------------------------------------
# global.cloud

@test "partitionInit/Job: sets TLS server name if global.cloud.enabled is set" {
cd `chart_dir`
local actual=$(helm template \
-s templates/partition-init-job.yaml \
--set 'global.enabled=false' \
--set 'global.adminPartitions.enabled=true' \
--set "global.adminPartitions.name=bar" \
--set 'externalServers.enabled=true' \
--set 'externalServers.hosts[0]=foo' \
--set 'global.tls.enabled=true' \
--set 'global.tls.enableAutoEncrypt=true' \
--set 'global.tls.caCert.secretName=foo' \
--set 'global.tls.enabled=true' \
--set 'global.tls.enableAutoEncrypt=true' \
--set 'global.cloud.enabled=true' \
--set 'global.cloud.secretName=blah' \
. | tee /dev/stderr |
yq '.spec.template.spec.containers[0].command | any(contains("-tls-server-name=server.dc1.consul"))' | tee /dev/stderr)
[ "${actual}" = "true" ]
}
15 changes: 14 additions & 1 deletion charts/consul/test/unit/server-acl-init-job.bats
Original file line number Diff line number Diff line change
Expand Up @@ -1694,7 +1694,7 @@ load _helpers
--set 'global.tls.enabled=true' \
--set 'externalServers.tlsServerName=foo' \
. | tee /dev/stderr |
yq '.spec.template.spec.containers[0].command | any(contains("-consul-tls-server-name=foo"))' | tee /dev/stderr)
yq '.spec.template.spec.containers[0].command | any(contains("-tls-server-name=foo"))' | tee /dev/stderr)
[ "${actual}" = "true" ]
}

Expand Down Expand Up @@ -1911,3 +1911,16 @@ load _helpers
[ "$status" -eq 1 ]
[[ "$output" =~ "When global.cloud.enabled is true, global.cloud.secretName must also be set." ]]
}

@test "serverACLInit/Job: sets TLS server name if global.cloud.enabled is set" {
cd `chart_dir`
local actual=$(helm template \
-s templates/server-acl-init-job.yaml \
--set 'global.acls.manageSystemACLs=true' \
--set 'global.tls.enabled=true' \
--set 'global.cloud.enabled=true' \
--set 'global.cloud.secretName=blah' \
. | tee /dev/stderr |
yq '.spec.template.spec.containers[0].command | any(contains("-tls-server-name=server.dc1.consul"))' | tee /dev/stderr)
[ "${actual}" = "true" ]
}
16 changes: 16 additions & 0 deletions charts/consul/test/unit/terminating-gateways-deployment.bats
Original file line number Diff line number Diff line change
Expand Up @@ -1211,3 +1211,19 @@ key2: value2' \
[ "$status" -eq 1 ]
[[ "$output" =~ "When global.cloud.enabled is true, global.cloud.secretName must also be set." ]]
}

@test "terminatingGateways/Deployment: sets TLS server name if global.cloud.enabled is set" {
cd `chart_dir`
local actual=$(helm template \
-s templates/terminating-gateways-deployment.yaml \
--set 'terminatingGateways.enabled=true' \
--set 'global.acls.manageSystemACLs=true' \
--set 'connectInject.enabled=true' \
--set 'global.tls.enabled=true' \
--set 'global.tls.enableAutoEncrypt=true' \
--set 'global.cloud.enabled=true' \
--set 'global.cloud.secretName=blah' \
. | tee /dev/stderr |
yq '.spec.template.spec.containers[0].command | any(contains("-tls-server-name=server.dc1.consul"))' | tee /dev/stderr)
[ "${actual}" = "true" ]
}
2 changes: 1 addition & 1 deletion cli/version/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ var (
// A pre-release marker for the version. If this is "" (empty string)
// then it means that it is a final release. Otherwise, this is a pre-release
// such as "dev" (in development), "beta", "rc1", etc.
VersionPrerelease = "dev"
VersionPrerelease = "beta1"
)

// GetHumanVersion composes the parts of the version in a way that's suitable
Expand Down
2 changes: 1 addition & 1 deletion control-plane/subcommand/server-acl-init/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ func (c *Command) init() {
c.flags.UintVar(&c.flagServerPort, "server-port", 8500, "The HTTP or HTTPS port of the Consul server. Defaults to 8500.")
c.flags.StringVar(&c.flagConsulCACert, "consul-ca-cert", "",
"Path to the PEM-encoded CA certificate of the Consul cluster.")
c.flags.StringVar(&c.flagConsulTLSServerName, "consul-tls-server-name", "",
c.flags.StringVar(&c.flagConsulTLSServerName, "tls-server-name", "",
"The server name to set as the SNI header when sending HTTPS requests to Consul.")
c.flags.BoolVar(&c.flagUseHTTPS, "use-https", false,
"Toggle for using HTTPS for all API calls to Consul.")
Expand Down
2 changes: 1 addition & 1 deletion control-plane/subcommand/server-acl-init/command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1866,7 +1866,7 @@ func TestRun_HTTPS(t *testing.T) {
"-resource-prefix=" + resourcePrefix,
"-k8s-namespace=" + ns,
"-use-https",
"-consul-tls-server-name", "server.dc1.consul",
"-tls-server-name", "server.dc1.consul",
"-consul-ca-cert", caFile,
"-server-address=" + strings.Split(srv.HTTPSAddr, ":")[0],
"-server-port=" + strings.Split(srv.HTTPSAddr, ":")[1],
Expand Down
2 changes: 1 addition & 1 deletion control-plane/version/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ var (
// A pre-release marker for the version. If this is "" (empty string)
// then it means that it is a final release. Otherwise, this is a pre-release
// such as "dev" (in development), "beta", "rc1", etc.
VersionPrerelease = "dev"
VersionPrerelease = "beta1"
)

// GetHumanVersion composes the parts of the version in a way that's suitable
Expand Down

0 comments on commit 14528e1

Please sign in to comment.