Skip to content

feat(dgraph)!: synthesize the --tls superflag and switch probes to HTTPS when TLS is enabled #152

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

Open
mlwelles wants to merge 1 commit into
base: feat-chart-ergonomics
Choose a base branch
from
Open

feat(dgraph)!: synthesize the --tls superflag and switch probes to HTTPS when TLS is enabled #152

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 10 additions & 0 deletions charts/dgraph/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ No manual intervention is required. Also review the [additional breaking changes

**ACL and encryption flags now auto-activate**: Setting `alpha.acl.enabled: true` (or `alpha.encryption.enabled: true`) now synthesizes the matching `--acl` (or `--encryption`) superflag onto the Alpha command automatically; previously these flags had to be added by hand through `alpha.extraFlags`. If you already pass `--acl` or `--encryption` through `alpha.extraFlags`, remove it — the chart fails rendering rather than pass the flag twice. The chart points the flag at `/dgraph/acl/<alpha.acl.secretFile>` and `/dgraph/enc/<alpha.encryption.keyFile>`, which default to `hmac_secret_file` and `enc_key_file`; override those keys if your Secret stores the file under a different name.

**TLS now activates from the tls block**: Setting `alpha.tls.enabled: true` (or `zero.tls.enabled: true`) now synthesizes the `--tls` superflag onto the Alpha (or Zero) command; previously the flag had to be added by hand through `extraFlags`. If you already pass `--tls` through `alpha.extraFlags` or `zero.extraFlags`, remove it — the chart fails rendering rather than pass the flag twice. Configure TLS through the new `tls.internalPort`, `tls.clientName`, and `tls.clientAuthType` keys; the chart reads the cert files from `/dgraph/tls` (`ca.crt`, `node.crt`, `node.key`, and `client.<clientName>.crt`/`.key`).

**Health probes use HTTPS when TLS is enabled**: With `alpha.tls.enabled: true` (or `zero.tls.enabled: true`), the built-in httpGet startup, liveness, and readiness probes switch to `scheme: HTTPS`. A cert-requiring `clientAuthType` (`REQUIREANY` or `REQUIREANDVERIFY`) makes every client present a certificate, which the kubelet's probes cannot; the chart fails rendering in that case. Set `clientAuthType: VERIFYIFGIVEN`, or supply exec probes through `customStartupProbe`/`customLivenessProbe`/`customReadinessProbe`. A cert-requiring `clientAuthType` also requires `clientName` so in-cluster callers (inter-node TLS) can present a client cert.

### Installing the Chart

To install the chart with the release name `my-release`:
Expand Down Expand Up @@ -144,6 +148,9 @@ The following table lists the configurable parameters of the `dgraph` chart and
| `zero.customStartupProbe` | Zero custom startup probes (if `zero.startupProbe` not enabled) | `{}` |
| `zero.customLivenessProbe` | Zero custom liveness probes (if `zero.livenessProbe` not enabled) | `{}` |
| `zero.customReadinessProbe` | Zero custom readiness probes (if `zero.readinessProbe` not enabled) | `{}` |
| `zero.tls.internalPort` | Enable TLS on Zero's internal gRPC port (synthesized into `--tls`) | `true` |
| `zero.tls.clientName` | Client cert basename for Zero `--tls` (empty omits the client cert) | `""` |
| `zero.tls.clientAuthType` | Zero `--tls` client-auth-type, e.g. `REQUIREANDVERIFY` (empty omits) | `""` |
| `alpha.name` | Alpha component name | `alpha` |
| `alpha.metrics.enabled` | Add annotations for Prometheus metric scraping | `true` |
| `alpha.extraAnnotations` | Specify annotations for template metadata | `{}` |
Expand Down Expand Up @@ -185,6 +192,9 @@ The following table lists the configurable parameters of the `dgraph` chart and
| `alpha.securityContext.runAsUser` | User ID for the Alpha container | `1001` |
| `alpha.tls.enabled` | Alpha service TLS enabled | `false` |
| `alpha.tls.files` | Alpha service TLS key and certificate files stored as secrets | `false` |
| `alpha.tls.internalPort` | Enable TLS on Alpha's internal gRPC port (synthesized into `--tls`) | `true` |
| `alpha.tls.clientName` | Client cert basename for Alpha `--tls` (empty omits the client cert) | `""` |
| `alpha.tls.clientAuthType` | Alpha `--tls` client-auth-type, e.g. `REQUIREANDVERIFY` (empty omits) | `""` |
| `alpha.encryption.enabled` | Alpha Encryption at Rest enabled (auto-adds `--encryption`) | `false` |
| `alpha.encryption.keyFile` | Filename/key of the encryption key within the mounted Secret | `enc_key_file` |
| `alpha.encryption.existingSecret` | Name of a pre-created Secret holding the encryption key (suppresses the chart's own) | `""` |
Expand Down
45 changes: 45 additions & 0 deletions charts/dgraph/templates/_helpers.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -328,3 +328,48 @@ leading space, "-v=<n> --logtostderr=<bool>" plus --vmodule / --alsologtostderr
{{- if .logDir }}{{ printf " --log_dir=%s" .logDir }}{{ end -}}
{{- end -}}
{{- end -}}

{{/*
Does this tls block force every client to present a certificate?

client-auth-type mirrors Go's crypto/tls.ClientAuthType, which separates
"require" from "verify". REQUIREANY and REQUIREANDVERIFY are the only values
that force a cert (REQUIREANY never verifies it, REQUIREANDVERIFY does).
VERIFYIFGIVEN leaves the cert optional but verifies one that is presented;
REQUEST asks for a cert and neither requires nor verifies it; "" and OFF
disable client auth outright.

Returns the STRING "true" or "" (empty) -- NOT a boolean. Compare it as a
string: eq (include "dgraph.tls.certRequired" (dict "tls" .Values.alpha.tls)) "true".

Single source of truth for the alpha/zero probe guards. Keep them reading from
here: an inline re-derivation drifts.
*/}}
{{- define "dgraph.tls.certRequired" -}}
{{- $t := .tls.clientAuthType | default "" -}}
{{- if or (eq $t "REQUIREANDVERIFY") (eq $t "REQUIREANY") -}}true{{- end -}}
{{- end -}}

{{/*
Compose Dgraph's --tls superflag from a tier's tls map. Pass a dict
{"tls": .Values.alpha.tls, "path": "/dgraph/tls"}. Filenames follow the output
of scripts/make_tls_secrets.sh (ca.crt, node.crt, node.key,
client.<name>.crt/.key). client-cert/key and client-auth-type are emitted only
when the corresponding values are set.
*/}}
{{- define "dgraph.tlsFlag" -}}
{{- /* internalPort defaults to true (values.yaml), but Helm's `default` treats a
boolean false as empty, so an explicit `false` would be flipped back to the
default. Use a nil check so nil -> true while honoring an explicit false. */}}
{{- $ip := .tls.internalPort -}}
{{- if kindIs "invalid" $ip -}}{{- $ip = true -}}{{- end -}}
{{- $opts := list (printf "ca-cert=%s/ca.crt" .path) (printf "server-cert=%s/node.crt" .path) (printf "server-key=%s/node.key" .path) (printf "internal-port=%v" $ip) -}}
{{- if .tls.clientName -}}
{{- $opts = append $opts (printf "client-cert=%s/client.%s.crt" .path .tls.clientName) -}}
{{- $opts = append $opts (printf "client-key=%s/client.%s.key" .path .tls.clientName) -}}
{{- end -}}
{{- if .tls.clientAuthType -}}
{{- $opts = append $opts (printf "client-auth-type=%s" .tls.clientAuthType) -}}
{{- end -}}
{{- printf "--tls \"%s;\"" (join "; " $opts) -}}
{{- end -}}
31 changes: 30 additions & 1 deletion charts/dgraph/templates/alpha/statefulset.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@
{{- $hasMinioKeys := include "dgraph.backups.keys.minio.enabled" . -}}
{{- $backupsEnabled := or .Values.backups.full.enabled .Values.backups.incremental.enabled }}
{{- $initContainerEnabled := or .Values.alpha.initContainers.init.enabled .Values.alpha.extraInitContainers }}
{{- /* native-TLS is active for alpha when alpha.tls is on. Computed once here (a
bool) and reused for the --tls superflag, the HTTPS probe scheme, and the
extraFlags/clientAuthType guards below. */}}
{{- $nativeTLS := .Values.alpha.tls.enabled }}
{{- /* ACL and encryption-at-rest activate from `enabled` below by synthesizing the
matching superflag onto the alpha command. Fail rather than emit a duplicate
flag if the operator also hand-set it in extraFlags. */}}
Expand All @@ -32,6 +36,22 @@
{{- if and .Values.alpha.encryption.enabled (contains "--encryption" (.Values.alpha.extraFlags | default "")) }}
{{- fail "alpha.encryption.enabled synthesizes the --encryption superflag, but alpha.extraFlags already contains --encryption. Remove --encryption from alpha.extraFlags (configure alpha.encryption.keyFile / alpha.encryption.existingSecret instead) so the flag is not passed twice." }}
{{- end }}
{{- /* native TLS synthesizes --tls from alpha.tls; a hand-set --tls in extraFlags
would be passed twice. */}}
{{- if and $nativeTLS (contains "--tls" (.Values.alpha.extraFlags | default "")) }}
{{- fail "alpha.extraFlags contains --tls, but the chart synthesizes it when alpha.tls.enabled=true. Remove --tls from alpha.extraFlags and set alpha.tls.internalPort / clientName / clientAuthType instead." }}
{{- end }}
{{- /* client-auth-type applies to the external ports the built-in probes hit, so a
cert-requiring mode breaks the certless kubelet probe handshake. */}}
{{- $certRequiringAuth := eq (include "dgraph.tls.certRequired" (dict "tls" .Values.alpha.tls)) "true" }}
{{- if and $nativeTLS $certRequiringAuth }}
{{- if not .Values.alpha.tls.clientName }}
{{- fail (printf "alpha.tls.clientAuthType=%s requires alpha.tls.clientName so in-cluster callers (inter-node TLS) can present a client certificate." .Values.alpha.tls.clientAuthType) }}
{{- end }}
{{- if or (and .Values.alpha.startupProbe.enabled (not .Values.alpha.customStartupProbe)) (and .Values.alpha.livenessProbe.enabled (not .Values.alpha.customLivenessProbe)) (and .Values.alpha.readinessProbe.enabled (not .Values.alpha.customReadinessProbe)) }}
{{- fail (printf "alpha.tls.clientAuthType=%s forces every client to present a certificate, which the built-in httpGet probes cannot do (the kubelet has no client cert). Relax alpha.tls.clientAuthType (e.g. VERIFYIFGIVEN) on the external ports, or supply alpha.customStartupProbe / customLivenessProbe / customReadinessProbe (exec probes that present the client cert)." .Values.alpha.tls.clientAuthType) }}
{{- end }}
{{- end }}
apiVersion: apps/v1
kind: StatefulSet
metadata:
Expand Down Expand Up @@ -223,14 +243,17 @@ spec:
{{- /* TODO: Remove awk-gsub once dgraph-io/dgraph#6837 is merged and back-ported. */}}
- |
set -ex
exec dgraph alpha --my=$(hostname -f | awk '{gsub(/\.$/,""); print $0}'):7080 --zero {{ template "multi_zeros" . }}{{ template "dgraph.logFlags" .Values.alpha }} {{ .Values.alpha.extraFlags }}{{- if .Values.alpha.acl.enabled }} --acl "secret-file=/dgraph/acl/{{ .Values.alpha.acl.secretFile }};"{{- end }}{{- if .Values.alpha.encryption.enabled }} --encryption "key-file=/dgraph/enc/{{ .Values.alpha.encryption.keyFile }};"{{- end }}
exec dgraph alpha --my=$(hostname -f | awk '{gsub(/\.$/,""); print $0}'):7080 --zero {{ template "multi_zeros" . }}{{ template "dgraph.logFlags" .Values.alpha }} {{ .Values.alpha.extraFlags }}{{- if .Values.alpha.acl.enabled }} --acl "secret-file=/dgraph/acl/{{ .Values.alpha.acl.secretFile }};"{{- end }}{{- if .Values.alpha.encryption.enabled }} --encryption "key-file=/dgraph/enc/{{ .Values.alpha.encryption.keyFile }};"{{- end }}{{- if $nativeTLS }} {{ include "dgraph.tlsFlag" (dict "tls" .Values.alpha.tls "path" "/dgraph/tls") }}{{- end }}
resources:
{{ toYaml .Values.alpha.resources | indent 10 }}
{{- if .Values.alpha.startupProbe.enabled }}
startupProbe:
httpGet:
port: {{ .Values.alpha.startupProbe.port }}
path: {{ .Values.alpha.startupProbe.path }}
{{- if $nativeTLS }}
scheme: HTTPS
{{- end }}
periodSeconds: {{ .Values.alpha.startupProbe.periodSeconds }}
timeoutSeconds: {{ .Values.alpha.startupProbe.timeoutSeconds }}
successThreshold: {{ .Values.alpha.startupProbe.successThreshold }}
Expand All @@ -243,6 +266,9 @@ spec:
httpGet:
port: {{ .Values.alpha.livenessProbe.port }}
path: {{ .Values.alpha.livenessProbe.path }}
{{- if $nativeTLS }}
scheme: HTTPS
{{- end }}
initialDelaySeconds: {{ .Values.alpha.livenessProbe.initialDelaySeconds }}
periodSeconds: {{ .Values.alpha.livenessProbe.periodSeconds }}
timeoutSeconds: {{ .Values.alpha.livenessProbe.timeoutSeconds }}
Expand All @@ -256,6 +282,9 @@ spec:
httpGet:
port: {{ .Values.alpha.readinessProbe.port }}
path: {{ .Values.alpha.readinessProbe.path }}
{{- if $nativeTLS }}
scheme: HTTPS
{{- end }}
initialDelaySeconds: {{ .Values.alpha.readinessProbe.initialDelaySeconds }}
periodSeconds: {{ .Values.alpha.readinessProbe.periodSeconds }}
timeoutSeconds: {{ .Values.alpha.readinessProbe.timeoutSeconds }}
Expand Down
33 changes: 31 additions & 2 deletions charts/dgraph/templates/zero/statefulset.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,26 @@
{{- printf "--idx " -}}
{{- end -}}
{{- end -}}
{{- /* native-TLS is active for zero when zero.tls is on. Computed once here (a
bool) and reused for the --tls superflag, the HTTPS probe scheme, and the
extraFlags/clientAuthType guards below. */}}
{{- $nativeTLS := .Values.zero.tls.enabled }}
{{- /* native TLS synthesizes --tls from zero.tls; a hand-set --tls in extraFlags
would be passed twice. */}}
{{- if and $nativeTLS (contains "--tls" (.Values.zero.extraFlags | default "")) }}
{{- fail "zero.extraFlags contains --tls, but the chart synthesizes it when zero.tls.enabled=true. Remove --tls from zero.extraFlags and set zero.tls.internalPort / clientName / clientAuthType instead." }}
{{- end }}
{{- /* client-auth-type applies to the external ports the built-in probes hit, so a
cert-requiring mode breaks the certless kubelet probe handshake. */}}
{{- $certRequiringAuth := eq (include "dgraph.tls.certRequired" (dict "tls" .Values.zero.tls)) "true" }}
{{- if and $nativeTLS $certRequiringAuth }}
{{- if not .Values.zero.tls.clientName }}
{{- fail (printf "zero.tls.clientAuthType=%s requires zero.tls.clientName so inter-node TLS can present a client certificate." .Values.zero.tls.clientAuthType) }}
{{- end }}
{{- if or (and .Values.zero.startupProbe.enabled (not .Values.zero.customStartupProbe)) (and .Values.zero.livenessProbe.enabled (not .Values.zero.customLivenessProbe)) (and .Values.zero.readinessProbe.enabled (not .Values.zero.customReadinessProbe)) }}
{{- fail (printf "zero.tls.clientAuthType=%s forces every client to present a certificate, which the built-in httpGet probes cannot do (the kubelet has no client cert). Relax zero.tls.clientAuthType (e.g. VERIFYIFGIVEN) on the external ports, or supply zero.customStartupProbe / customLivenessProbe / customReadinessProbe (exec probes that present the client cert)." .Values.zero.tls.clientAuthType) }}
{{- end }}
{{- end }}
apiVersion: apps/v1
kind: StatefulSet
metadata:
Expand Down Expand Up @@ -145,9 +165,9 @@ spec:
ordinal=${BASH_REMATCH[1]}
idx=$(($ordinal + 1))
if [[ $ordinal -eq 0 ]]; then
exec dgraph zero --my=$(hostname -f | awk '{gsub(/\.$/,""); print $0}'):5080 {{ template "raft_index_flag" . }}$idx --replicas {{ .Values.zero.shardReplicaCount }}{{ template "dgraph.logFlags" .Values.zero }} {{ .Values.zero.extraFlags }}
exec dgraph zero --my=$(hostname -f | awk '{gsub(/\.$/,""); print $0}'):5080 {{ template "raft_index_flag" . }}$idx --replicas {{ .Values.zero.shardReplicaCount }}{{ template "dgraph.logFlags" .Values.zero }} {{ .Values.zero.extraFlags }}{{- if $nativeTLS }} {{ include "dgraph.tlsFlag" (dict "tls" .Values.zero.tls "path" "/dgraph/tls") }}{{- end }}
else
exec dgraph zero --my=$(hostname -f | awk '{gsub(/\.$/,""); print $0}'):5080 --peer {{ template "peer_zero" . }} {{ template "raft_index_flag" . }}$idx --replicas {{ .Values.zero.shardReplicaCount }}{{ template "dgraph.logFlags" .Values.zero }} {{ .Values.zero.extraFlags }}
exec dgraph zero --my=$(hostname -f | awk '{gsub(/\.$/,""); print $0}'):5080 --peer {{ template "peer_zero" . }} {{ template "raft_index_flag" . }}$idx --replicas {{ .Values.zero.shardReplicaCount }}{{ template "dgraph.logFlags" .Values.zero }} {{ .Values.zero.extraFlags }}{{- if $nativeTLS }} {{ include "dgraph.tlsFlag" (dict "tls" .Values.zero.tls "path" "/dgraph/tls") }}{{- end }}
fi
resources:
{{ toYaml .Values.zero.resources | indent 10 }}
Expand All @@ -156,6 +176,9 @@ spec:
httpGet:
port: {{ .Values.zero.startupProbe.port }}
path: {{ .Values.zero.startupProbe.path }}
{{- if $nativeTLS }}
scheme: HTTPS
{{- end }}
periodSeconds: {{ .Values.zero.startupProbe.periodSeconds }}
timeoutSeconds: {{ .Values.zero.startupProbe.timeoutSeconds }}
successThreshold: {{ .Values.zero.startupProbe.successThreshold }}
Expand All @@ -168,6 +191,9 @@ spec:
httpGet:
port: {{ .Values.zero.livenessProbe.port }}
path: {{ .Values.zero.livenessProbe.path }}
{{- if $nativeTLS }}
scheme: HTTPS
{{- end }}
initialDelaySeconds: {{ .Values.zero.livenessProbe.initialDelaySeconds }}
periodSeconds: {{ .Values.zero.livenessProbe.periodSeconds }}
timeoutSeconds: {{ .Values.zero.livenessProbe.timeoutSeconds }}
Expand All @@ -181,6 +207,9 @@ spec:
httpGet:
port: {{ .Values.zero.readinessProbe.port }}
path: {{ .Values.zero.readinessProbe.path }}
{{- if $nativeTLS }}
scheme: HTTPS
{{- end }}
initialDelaySeconds: {{ .Values.zero.readinessProbe.initialDelaySeconds }}
periodSeconds: {{ .Values.zero.readinessProbe.periodSeconds }}
timeoutSeconds: {{ .Values.zero.readinessProbe.timeoutSeconds }}
Expand Down
Loading