diff --git a/patchmon/templates/_helpers.tpl b/patchmon/templates/_helpers.tpl index 7472cc3..a1947e0 100644 --- a/patchmon/templates/_helpers.tpl +++ b/patchmon/templates/_helpers.tpl @@ -114,3 +114,210 @@ template-chart is the reference implementation. 1 {{- end -}} {{- end -}} + +{{/* +Merge a user-supplied values sub-tree onto a dict of chart defaults so that a +null / empty override falls back to the defaults instead of erasing them. + +Input: dict "defaults" "given" "path" . Output: YAML of the merged dict (consume +it with fromYaml). + +Why this exists (issue #99): Helm merges values maps recursively, but an +override key written without children — e.g. a leftover + + securityContext: + +after deleting its last remaining sub-key — is YAML null and replaces the +whole default sub-tree. The container then renders with "securityContext: +null": no readOnlyRootFilesystem, no dropped capabilities, no +allowPrivilegeEscalation: false. The workload starts and reports healthy while +being unhardened, so nothing points at the mistake. The same shape erases a +probe (it disappears) or the resources block (no requests, no computed +limits). This helper defines the opposite semantics: an empty block means +"chart defaults apply". + +Rules: + - a nil "given" is treated as an empty dict (all defaults apply). + - a key whose override value is null keeps the default (nulls never delete). + - a key present in both as a map is merged recursively, so a partially + filled block only overrides the keys it actually sets. + - a scalar override for a key the defaults define as a map is rejected with + a message naming the values path, instead of failing later with a template + field error. + - any other set value wins as given, including an explicit false or 0 — + Helm's "default" and sprig's "mergeOverwrite" would both swallow those + zero values, hence the explicit kindIs "invalid" nil check. + - keys only present in the override are passed through unchanged. + +NOTE: like the helpers above, this is duplicated into every chart's +templates/_helpers.tpl with the chart's name as define prefix, because charts +in this repo deliberately have no dependencies. +*/}} +{{- define "patchmon.defaultedDict" -}} +{{- $defaults := .defaults | default dict -}} +{{- $given := .given -}} +{{- $path := .path | default "value" -}} +{{- if kindIs "invalid" $given -}} +{{- $given = dict -}} +{{- end -}} +{{- if not (kindIs "map" $given) -}} +{{- fail (printf "%s must be a mapping or null, got %s (%v)" $path (kindOf $given) $given) -}} +{{- end -}} +{{- $out := deepCopy $defaults -}} +{{- range $key, $value := $given -}} +{{- if not (kindIs "invalid" $value) -}} +{{- $default := index $defaults $key -}} +{{- if and (kindIs "map" $value) (kindIs "map" $default) -}} +{{- $_ := set $out $key (fromYaml (include "patchmon.defaultedDict" (dict "defaults" $default "given" $value "path" (printf "%s.%s" $path $key)))) -}} +{{- else if kindIs "map" $default -}} +{{- fail (printf "%s.%s must be a mapping or null, got %s (%v)" $path $key (kindOf $value) $value) -}} +{{- else -}} +{{- $_ := set $out $key $value -}} +{{- end -}} +{{- end -}} +{{- end -}} +{{- toYaml $out -}} +{{- end -}} + +{{/* +Pick one sub-block out of a values block that may itself be null. + +Input: dict "block" "key" "path" . +Returns YAML of dict "value" , after rejecting a +non-mapping block with a message that names the values path. Needed because +".Values.patchmon..securityContext" can itself be erased. +*/}} +{{- define "patchmon.subBlock" -}} +{{- $block := .block -}} +{{- if kindIs "invalid" $block -}} +{{- $block = dict -}} +{{- end -}} +{{- if not (kindIs "map" $block) -}} +{{- fail (printf "%s must be a mapping or null, got %s (%v)" .path (kindOf $block) $block) -}} +{{- end -}} +{{- toYaml (dict "value" (index $block .key)) -}} +{{- end -}} + +{{/* +The effective security contexts, probes and resources of both workloads +(issue #99). + +The defaults below MUST stay in sync with the corresponding blocks in +values.yaml, which stays the documented, user-facing place for them; this copy +is only the fallback for the case where an override has erased the block. + +The server probes carry a Host header, because the 2.x server rejects requests +whose Host is neither loopback nor a CORS_ORIGIN host. The default is built +from .Values.ingress.domain via the passed-in root context, so the probe +helpers take dict "given" "root" $. +*/}} +{{- define "patchmon.server.podSecurityContext" -}} +{{- $given := (fromYaml (include "patchmon.subBlock" (dict "block" . "key" "pod" "path" "patchmon.server.securityContext"))).value -}} +{{- $defaults := dict + "runAsNonRoot" true + "runAsUser" 65532 + "runAsGroup" 65532 -}} +{{- include "patchmon.defaultedDict" (dict "defaults" $defaults "given" $given "path" "patchmon.server.securityContext.pod") -}} +{{- end -}} + +{{- define "patchmon.server.containerSecurityContext" -}} +{{- $given := (fromYaml (include "patchmon.subBlock" (dict "block" . "key" "container" "path" "patchmon.server.securityContext"))).value -}} +{{- $defaults := dict + "allowPrivilegeEscalation" false + "readOnlyRootFilesystem" true + "runAsNonRoot" true + "capabilities" (dict "drop" (list "ALL")) -}} +{{- include "patchmon.defaultedDict" (dict "defaults" $defaults "given" $given "path" "patchmon.server.securityContext.container") -}} +{{- end -}} + +{{- define "patchmon.guacd.containerSecurityContext" -}} +{{- $given := (fromYaml (include "patchmon.subBlock" (dict "block" . "key" "container" "path" "patchmon.guacd.securityContext"))).value -}} +{{- $defaults := dict + "allowPrivilegeEscalation" false + "readOnlyRootFilesystem" true + "runAsNonRoot" true + "runAsUser" 1000 + "runAsGroup" 1000 + "capabilities" (dict "drop" (list "ALL")) -}} +{{- include "patchmon.defaultedDict" (dict "defaults" $defaults "given" $given "path" "patchmon.guacd.securityContext.container") -}} +{{- end -}} + +{{- define "patchmon.server.httpGet" -}} +{{- dict "path" "/health" "port" 3000 "httpHeaders" (list (dict "name" "Host" "value" (.root.Values.ingress.domain | default ""))) | toYaml -}} +{{- end -}} + +{{- define "patchmon.server.livenessProbe" -}} +{{- $defaults := dict + "httpGet" (fromYaml (include "patchmon.server.httpGet" .)) + "periodSeconds" 10 + "timeoutSeconds" 5 + "failureThreshold" 3 -}} +{{- include "patchmon.defaultedDict" (dict "defaults" $defaults "given" .given "path" "patchmon.server.livenessProbe") -}} +{{- end -}} + +{{- define "patchmon.server.readinessProbe" -}} +{{- $defaults := dict + "httpGet" (fromYaml (include "patchmon.server.httpGet" .)) + "periodSeconds" 10 + "timeoutSeconds" 5 + "failureThreshold" 3 -}} +{{- include "patchmon.defaultedDict" (dict "defaults" $defaults "given" .given "path" "patchmon.server.readinessProbe") -}} +{{- end -}} + +{{- define "patchmon.server.startupProbe" -}} +{{- $defaults := dict + "httpGet" (fromYaml (include "patchmon.server.httpGet" .)) + "periodSeconds" 10 + "timeoutSeconds" 5 + "failureThreshold" 60 -}} +{{- include "patchmon.defaultedDict" (dict "defaults" $defaults "given" .given "path" "patchmon.server.startupProbe") -}} +{{- end -}} + +{{- define "patchmon.guacd.livenessProbe" -}} +{{- $defaults := dict + "tcpSocket" (dict "port" 4822) + "periodSeconds" 10 + "timeoutSeconds" 5 + "failureThreshold" 3 -}} +{{- include "patchmon.defaultedDict" (dict "defaults" $defaults "given" . "path" "patchmon.guacd.livenessProbe") -}} +{{- end -}} + +{{- define "patchmon.guacd.readinessProbe" -}} +{{- $defaults := dict + "tcpSocket" (dict "port" 4822) + "periodSeconds" 10 + "timeoutSeconds" 5 + "failureThreshold" 3 -}} +{{- include "patchmon.defaultedDict" (dict "defaults" $defaults "given" . "path" "patchmon.guacd.readinessProbe") -}} +{{- end -}} + +{{- define "patchmon.guacd.startupProbe" -}} +{{- $defaults := dict + "tcpSocket" (dict "port" 4822) + "periodSeconds" 5 + "timeoutSeconds" 5 + "failureThreshold" 30 -}} +{{- include "patchmon.defaultedDict" (dict "defaults" $defaults "given" . "path" "patchmon.guacd.startupProbe") -}} +{{- end -}} + +{{/* +The effective resources blocks, fed into the resources helper above so that an +erased "resources:" still yields the chart's requests and computed limits +instead of a container without any resource accounting. +*/}} +{{- define "patchmon.server.effectiveResources" -}} +{{- $defaults := dict + "limitFactor" 3 + "requests" (dict "cpu" 0.1 "memory" "256Mi" "ephemeral-storage" "10Mi") -}} +{{- $merged := fromYaml (include "patchmon.defaultedDict" (dict "defaults" $defaults "given" . "path" "patchmon.server.resources")) -}} +{{- include "patchmon.resources" $merged -}} +{{- end -}} + +{{- define "patchmon.guacd.effectiveResources" -}} +{{- $defaults := dict + "limitFactor" 3 + "requests" (dict "cpu" 0.05 "memory" "64Mi" "ephemeral-storage" "10Mi") -}} +{{- $merged := fromYaml (include "patchmon.defaultedDict" (dict "defaults" $defaults "given" . "path" "patchmon.guacd.resources")) -}} +{{- include "patchmon.resources" $merged -}} +{{- end -}} diff --git a/patchmon/templates/patchmon.deployment.yaml b/patchmon/templates/patchmon.deployment.yaml index 1f8436e..9b694a9 100644 --- a/patchmon/templates/patchmon.deployment.yaml +++ b/patchmon/templates/patchmon.deployment.yaml @@ -23,13 +23,13 @@ spec: # for the same reason). hostname: {{ .Release.Name }}-{{ .Chart.Name }}-server securityContext: - {{- toYaml .Values.patchmon.server.securityContext.pod | nindent 8 }} + {{- include "patchmon.server.podSecurityContext" .Values.patchmon.server.securityContext | nindent 8 }} containers: - name: {{ .Release.Name }}-{{ .Chart.Name }}-server image: "ghcr.io/patchmon/patchmon-server:{{ .Chart.AppVersion }}" imagePullPolicy: IfNotPresent securityContext: - {{- toYaml .Values.patchmon.server.securityContext.container | nindent 12 }} + {{- include "patchmon.server.containerSecurityContext" .Values.patchmon.server.securityContext | nindent 12 }} ports: - containerPort: 3000 protocol: TCP @@ -166,13 +166,13 @@ spec: # Probe values are rendered through tpl (see values.yaml for the # defaults and why the probes send a Host header). livenessProbe: - {{- tpl (toYaml .Values.patchmon.server.livenessProbe) . | nindent 12 }} + {{- tpl (include "patchmon.server.livenessProbe" (dict "given" .Values.patchmon.server.livenessProbe "root" $)) . | nindent 12 }} readinessProbe: - {{- tpl (toYaml .Values.patchmon.server.readinessProbe) . | nindent 12 }} + {{- tpl (include "patchmon.server.readinessProbe" (dict "given" .Values.patchmon.server.readinessProbe "root" $)) . | nindent 12 }} startupProbe: - {{- tpl (toYaml .Values.patchmon.server.startupProbe) . | nindent 12 }} + {{- tpl (include "patchmon.server.startupProbe" (dict "given" .Values.patchmon.server.startupProbe "root" $)) . | nindent 12 }} resources: - {{- include "patchmon.resources" .Values.patchmon.server.resources | nindent 12 }} + {{- include "patchmon.server.effectiveResources" .Values.patchmon.server.resources | nindent 12 }} volumeMounts: # The image is stateless, but the Go server needs a writable /tmp # for scratch files with a read-only root filesystem. @@ -206,19 +206,19 @@ spec: {{- end }} {{- end }} securityContext: - {{- toYaml .Values.patchmon.guacd.securityContext.container | nindent 12 }} + {{- include "patchmon.guacd.containerSecurityContext" .Values.patchmon.guacd.securityContext | nindent 12 }} ports: - containerPort: 4822 protocol: TCP name: guacd livenessProbe: - {{- tpl (toYaml .Values.patchmon.guacd.livenessProbe) . | nindent 12 }} + {{- tpl (include "patchmon.guacd.livenessProbe" .Values.patchmon.guacd.livenessProbe) . | nindent 12 }} readinessProbe: - {{- tpl (toYaml .Values.patchmon.guacd.readinessProbe) . | nindent 12 }} + {{- tpl (include "patchmon.guacd.readinessProbe" .Values.patchmon.guacd.readinessProbe) . | nindent 12 }} startupProbe: - {{- tpl (toYaml .Values.patchmon.guacd.startupProbe) . | nindent 12 }} + {{- tpl (include "patchmon.guacd.startupProbe" .Values.patchmon.guacd.startupProbe) . | nindent 12 }} resources: - {{- include "patchmon.resources" .Values.patchmon.guacd.resources | nindent 12 }} + {{- include "patchmon.guacd.effectiveResources" .Values.patchmon.guacd.resources | nindent 12 }} volumeMounts: - name: guacd-tmp mountPath: /tmp diff --git a/patchmon/values.schema.json b/patchmon/values.schema.json index f7bac62..0fb0597 100644 --- a/patchmon/values.schema.json +++ b/patchmon/values.schema.json @@ -57,13 +57,13 @@ "type": "string" }, "securityContext": { - "description": "guacd is a sidecar in the server pod, so only a container context applies - the pod context is the server's.", - "type": "object", + "description": "guacd is a sidecar in the server pod, so only a container context applies - the pod context is the server's. May be null / empty: the chart then falls back to its hardening defaults (issue #99).", + "type": ["object", "null"], "additionalProperties": false, "properties": { "container": { - "description": "Rendered verbatim as the container's securityContext; kept open because the chart does not interpret it.", - "type": "object" + "description": "Merged onto the chart's container hardening defaults and rendered as the container's securityContext; kept open because the chart does not interpret the individual fields.", + "type": ["object", "null"] } } }, @@ -130,7 +130,8 @@ }, "definitions": { "resources": { - "type": "object", + "description": "May be null / empty: the chart then falls back to its default requests and computed limits (issue #99) instead of rendering a container without resource accounting.", + "type": ["object", "null"], "additionalProperties": false, "properties": { "limitFactor": { @@ -146,7 +147,7 @@ } }, "resourceList": { - "type": "object", + "type": ["object", "null"], "additionalProperties": false, "properties": { "cpu": { @@ -205,22 +206,23 @@ } }, "securityContext": { - "type": "object", + "description": "May be null / empty at any level: the chart then falls back to its hardening defaults (issue #99) instead of rendering an unhardened workload.", + "type": ["object", "null"], "additionalProperties": false, "properties": { "pod": { - "description": "Rendered verbatim as the pod's securityContext; kept open because the chart does not interpret it.", - "type": "object" + "description": "Merged onto the chart's pod hardening defaults and rendered as the pod's securityContext; kept open because the chart does not interpret the individual fields.", + "type": ["object", "null"] }, "container": { - "description": "Rendered verbatim as the container's securityContext; kept open because the chart does not interpret it.", - "type": "object" + "description": "Merged onto the chart's container hardening defaults and rendered as the container's securityContext; kept open because the chart does not interpret the individual fields.", + "type": ["object", "null"] } } }, "probe": { - "description": "Rendered verbatim as a container probe; kept open because the chart does not interpret it.", - "type": "object" + "description": "Merged onto the chart's probe defaults and rendered as a container probe; kept open because the chart does not interpret the individual fields. May be null / empty, which means the defaults apply (issue #99) rather than the probe disappearing.", + "type": ["object", "null"] } } }