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

Enable TLS #313

Merged
merged 26 commits into from
Jan 10, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
452eb4b
Initial TLS implementation
Mar 18, 2019
224c8ce
Improvements to the TLS implementation
ishustava Dec 6, 2019
b81632a
Update CHANGELOG
ishustava Dec 13, 2019
9a71a8e
tls-init-cleanup job fixes
ishustava Dec 14, 2019
46e8ab9
Client and server cluster roles don't need secret permissions
ishustava Dec 16, 2019
e57223f
Enable TLS for Consul Connect
ishustava Dec 16, 2019
2470fc3
Improvements from code review
ishustava Dec 18, 2019
6ff8db1
Enable TLS for sync-catalog
ishustava Dec 18, 2019
ef97b83
Enable TLS for server-acl-init
ishustava Dec 20, 2019
eb70d6e
Update CHANGELOG and set httpsOnly to true by default
ishustava Dec 20, 2019
458ce81
Update consul-k8s image
ishustava Dec 23, 2019
19d325d
Enable TLS for the Mesh gateway deployment
ishustava Dec 23, 2019
68c12b7
Support TLS for the snapshot agent deployment
ishustava Dec 23, 2019
c1a341e
Update CHANGELOG.md
ishustava Dec 23, 2019
d5ab090
Make sure helm test passes if TLS is enabled
ishustava Dec 24, 2019
64c57a3
tls-init service account, cluster role, and clusterrole binding shoul…
ishustava Jan 2, 2020
224b7d1
Support incremental rollout of TLS
ishustava Jan 3, 2020
b339938
Update CHANGELOG and enterprise licence TLS tests
ishustava Jan 8, 2020
81efca1
Update with changes from code review
ishustava Jan 9, 2020
c9700b8
Update Changelog and fix a few typos
ishustava Jan 9, 2020
08b4a24
Fix whitespace; combine/remove redundant tests
ishustava Jan 10, 2020
3d08818
GRPC address should not have 'http'
ishustava Jan 10, 2020
357c48e
Delete incorrect comments
ishustava Jan 10, 2020
3870aeb
Merge branch 'master' into enable-tls
ishustava Jan 10, 2020
bb34bc7
Fix syntax error
ishustava Jan 10, 2020
278da9a
Fix a bug in the enterprise license job
ishustava Jan 10, 2020
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
Prev Previous commit
Next Next commit
Fix whitespace; combine/remove redundant tests
  • Loading branch information
ishustava committed Jan 10, 2020
commit 08b4a24c88d405fb2db667eb7eb34a0c7ea24e67
1 change: 0 additions & 1 deletion templates/enterprise-license-job.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ spec:
volumeMounts:
- name: tls-ca-cert
mountPath: /consul/tls/ca/tls.crt
subPath: tls.crt
readOnly: true
{{- end }}
{{- if .Values.global.bootstrapACLs }}
Expand Down
2 changes: 1 addition & 1 deletion templates/tls-init-cleanup-job.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,4 @@ spec:
https://${KUBERNETES_SERVICE_HOST}:${KUBERNETES_SERVICE_PORT}/api/v1/namespaces/${NAMESPACE}/secrets/{{ template "consul.fullname" . }}-server-cert \
-H "Authorization: Bearer $( cat /var/run/secrets/kubernetes.io/serviceaccount/token )"
{{- end }}
{{- end }}
{{- end }}
2 changes: 1 addition & 1 deletion templates/tls-init-job.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -79,4 +79,4 @@ spec:
-H "Accept: application/json" \
-d "{ \"kind\": \"Secret\", \"apiVersion\": \"v1\", \"metadata\": { \"name\": \"{{ template "consul.fullname" . }}-server-cert\", \"namespace\": \"${NAMESPACE}\" }, \"type\": \"kubernetes.io/tls\", \"data\": { \"tls.crt\": \"$( cat {{ .Values.global.datacenter }}-server-{{ .Values.global.domain }}-0.pem | base64 | tr -d '\n' )\", \"tls.key\": \"$( cat {{ .Values.global.datacenter }}-server-{{ .Values.global.domain }}-0-key.pem | base64 | tr -d '\n' )\" } }" > /dev/null
Copy link
Member

Choose a reason for hiding this comment

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

There's one cert for all the servers right? So isn't this weird it's suffixed with -0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is what consul CLI does by default. It doesn't matter so much though because we're uploading only the contents here.

It's maybe weird for client certs. Each client cert has the -0 suffix, and so it could be a bit confusing. We could rename them, but with auto_encrypt we won't need this anymore.

Copy link
Member

Choose a reason for hiding this comment

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

gotcha, yeah for servers this is only place we see -0 since the key is tls.crt so this is cool

{{- end }}
{{- end }}
{{- end }}
2 changes: 1 addition & 1 deletion test/unit/client-clusterrole.bats
Original file line number Diff line number Diff line change
Expand Up @@ -101,4 +101,4 @@ load _helpers
. | tee /dev/stderr |
yq -r '.rules[1].resources[0]' | tee /dev/stderr)
[ "${actual}" = "secrets" ]
}
}
2 changes: 1 addition & 1 deletion test/unit/client-snapshot-agent-deployment.bats
Original file line number Diff line number Diff line change
Expand Up @@ -245,4 +245,4 @@ load _helpers
. | tee /dev/stderr |
yq '.spec.template.spec.containers[0].volumeMounts | length > 0' | tee /dev/stderr)
[ "${actual}" = "true" ]
}
}
2 changes: 1 addition & 1 deletion test/unit/connect-inject-deployment.bats
Original file line number Diff line number Diff line change
Expand Up @@ -408,4 +408,4 @@ load _helpers
. | tee /dev/stderr |
yq '.spec.template.spec.containers[0].volumeMounts | length' | tee /dev/stderr)
[ "${actual}" = "2" ]
}
}
34 changes: 5 additions & 29 deletions test/unit/enterprise-license-job.bats
Original file line number Diff line number Diff line change
Expand Up @@ -170,20 +170,8 @@ load _helpers
--set 'server.enterpriseLicense.secretKey=bar' \
--set 'global.tls.enabled=false' \
. | tee /dev/stderr |
yq '.spec.template.spec.containers[0].env[] | select(.name == "CONSUL_HTTP_ADDR") | .value | contains("http://")' | tee /dev/stderr)
[ "${actual}" = "true" ]
}

@test "server/EnterpriseLicense: Port is 8500 when TLS is disabled" {
cd `chart_dir`
local actual=$(helm template \
-x templates/enterprise-license-job.yaml \
--set 'server.enterpriseLicense.secretName=foo' \
--set 'server.enterpriseLicense.secretKey=bar' \
--set 'global.tls.enabled=false' \
. | tee /dev/stderr |
yq '.spec.template.spec.containers[0].env[] | select(.name == "CONSUL_HTTP_ADDR") | .value | contains(":8500")' | tee /dev/stderr)
[ "${actual}" = "true" ]
yq -r '.spec.template.spec.containers[0].env[] | select(.name == "CONSUL_HTTP_ADDR") | .value' | tee /dev/stderr)
[ "${actual}" = "http://release-name-consul-server:8500" ]
}

@test "server/EnterpriseLicense: URL is https when TLS is enabled" {
Expand All @@ -194,20 +182,8 @@ load _helpers
--set 'server.enterpriseLicense.secretKey=bar' \
--set 'global.tls.enabled=true' \
. | tee /dev/stderr |
yq '.spec.template.spec.containers[0].env[] | select(.name == "CONSUL_HTTP_ADDR") | .value | contains("https://")' | tee /dev/stderr)
[ "${actual}" = "true" ]
}

@test "server/EnterpriseLicense: Port is 8501 when TLS is enabled" {
cd `chart_dir`
local actual=$(helm template \
-x templates/enterprise-license-job.yaml \
--set 'server.enterpriseLicense.secretName=foo' \
--set 'server.enterpriseLicense.secretKey=bar' \
--set 'global.tls.enabled=true' \
. | tee /dev/stderr |
yq '.spec.template.spec.containers[0].env[] | select(.name == "CONSUL_HTTP_ADDR") | .value | contains(":8501")' | tee /dev/stderr)
[ "${actual}" = "true" ]
yq -r '.spec.template.spec.containers[0].env[] | select(.name == "CONSUL_HTTP_ADDR") | .value' | tee /dev/stderr)
[ "${actual}" = "https://release-name-consul-server:8501" ]
}

@test "server/EnterpriseLicense: CA certificate is specified when TLS is enabled" {
Expand All @@ -220,4 +196,4 @@ load _helpers
. | tee /dev/stderr |
yq -r '.spec.template.spec.containers[0].env[] | select(.name == "CONSUL_CACERT") | .value' | tee /dev/stderr)
[ "${actual}" = "/consul/tls/ca/tls.crt" ]
}
}
2 changes: 1 addition & 1 deletion test/unit/server-acl-init-job.bats
Original file line number Diff line number Diff line change
Expand Up @@ -232,4 +232,4 @@ load _helpers

actual=$(echo $command | jq -r '. | any(contains("-consul-tls-server-name=server.dc1.consul"))' | tee /dev/stderr)
[ "${actual}" = "true" ]
}
}
5 changes: 3 additions & 2 deletions test/unit/server-clusterrole.bats
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,14 @@ load _helpers

#--------------------------------------------------------------------
# global.enablePodSecurityPolicies

@test "server/ClusterRole: podsecuritypolicies are added when global.enablePodSecurityPolicies is true" {
cd `chart_dir`
local actual=$(helm template \
-x templates/server-clusterrole.yaml \
--set 'server.enabled=true' \
--set 'global.enablePodSecurityPolicies=true' \
. | tee /dev/stderr |
yq -r '.rules[0].resources[0]' | tee /dev/stderr)
[ "${actual}" = "podsecuritypolicies" ]
yq -r '.rules | map(select(.resources[0] == "podsecuritypolicies")) | length' | tee /dev/stderr)
[ "${actual}" = "1" ]
}
2 changes: 1 addition & 1 deletion test/unit/server-statefulset.bats
Original file line number Diff line number Diff line change
Expand Up @@ -639,4 +639,4 @@ load _helpers

actual=$(echo $command | jq -r '. | contains("verify_server_hostname = true")' | tee /dev/stderr)
[ "${actual}" = "false" ]
}
}
2 changes: 1 addition & 1 deletion test/unit/tls-init-cleanup-clusterrolebinding.bats
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,4 @@ load _helpers
. | tee /dev/stderr |
yq 'length > 0' | tee /dev/stderr)
[ "${actual}" = "true" ]
}
}
2 changes: 1 addition & 1 deletion test/unit/tls-init-cleanup-job.bats
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,4 @@ load _helpers
. | tee /dev/stderr |
yq 'length > 0' | tee /dev/stderr)
[ "${actual}" = "true" ]
}
}
2 changes: 1 addition & 1 deletion test/unit/tls-init-cleanup-serviceaccount.bats
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,4 @@ load _helpers
. | tee /dev/stderr |
yq 'length > 0' | tee /dev/stderr)
[ "${actual}" = "true" ]
}
}
2 changes: 1 addition & 1 deletion test/unit/tls-init-clusterrolebinding.bats
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,4 @@ load _helpers
. | tee /dev/stderr |
yq 'length > 0' | tee /dev/stderr)
[ "${actual}" = "true" ]
}
}
2 changes: 1 addition & 1 deletion test/unit/tls-init-serviceaccount.bats
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,4 @@ load _helpers
. | tee /dev/stderr |
yq 'length > 0' | tee /dev/stderr)
[ "${actual}" = "true" ]
}
}