Merge pull request #133 from jk-cluster/feat/template-chart-hardening-fixes

template-chart: seccomp profile, unprivileged port, and placeholder fixes

ci/excluded-charts.txt

@@ -1,4 +1,4 @@
 # Charts excluded from the PR install-test, one per line: <chart> <reason>.
 # ci/run-install-test.sh skips these with the given reason.
 satisfactory the game-server image is multiple GB and Steam downloads 10GB+ game files on first start - impractical on a CI runner
-template-chart scaffold with placeholder image "registry/image:xxx", never published
+template-chart scaffold with placeholder image "ghcr.io/ORG/IMAGE:xxx", never published

template-chart/Chart.yaml

@@ -5,7 +5,7 @@ icon: https://helm.sh/img/helm.svg
 
 type: application
 
-version: 3.0.0+upxxx
+version: 4.0.0+upxxx
 appVersion: "xxx"
 
 maintainers:

template-chart/templates/_helpers.tpl

@@ -229,7 +229,8 @@ guidelines"):
       "runAsUser" 20000
       "runAsGroup" 1000
       "fsGroup" 1000
-      "fsGroupChangePolicy" "Always" -}}
+      "fsGroupChangePolicy" "Always"
+      "seccompProfile" (dict "type" "RuntimeDefault") -}}
 {{- include "xxx.defaultedDict" (dict "defaults" $defaults "given" $given "path" "xxx.securityContext.pod") -}}
 {{- end -}}
 

template-chart/templates/xxx.configmap.yaml

@@ -3,4 +3,8 @@ kind: ConfigMap
 metadata:
   name: "{{ .Release.Name }}-{{ .Chart.Name }}-configmap"
 data:
-  key: "value"
+  # Placeholder for the chart's real configuration keys. Named for what it is:
+  # a key literally called "key" matches the patterns secret scanners use to
+  # find hardcoded credentials, so every copy of this template inherited a
+  # false positive.
+  example-setting: "value"

template-chart/templates/xxx.deployment.yaml

@@ -27,26 +27,34 @@ spec:
         {{- include "xxx.podSecurityContext" .Values.xxx.securityContext | nindent 8 }}
       containers:
         - name: {{ .Release.Name }}-{{ .Chart.Name }}-deployment
-          image: "registry/image:{{ .Chart.AppVersion }}"
+          # Fully qualified on purpose: an unqualified "registry/image" resolves
+          # to Docker Hub, which is how the repo accumulated its unqualified
+          # images. Replace host/org/image, keep the registry host.
+          image: "ghcr.io/ORG/IMAGE:{{ .Chart.AppVersion }}"
           imagePullPolicy: IfNotPresent
           env:
             {{- range $value := .Values.xxx.env }}
             {{- if $value.value }}
-            - name: {{ $value.name }}
-              value: {{ tpl $value.value $ }}
+            - name: {{ $value.name | quote }}
+              # quote is required, not cosmetic: a value like "8080" or "true"
+              # renders unquoted as a YAML number/bool, and the API server then
+              # rejects the whole manifest with "cannot unmarshal number into
+              # Go struct field EnvVar.value of type string". Every chart in
+              # this repo quotes here.
+              value: {{ tpl $value.value $ | quote }}
             {{- else if $value.valueFrom }}
             {{- if $value.valueFrom.secretKeyRef }}
             - name: {{ $value.name }}
               valueFrom:
                 secretKeyRef:
-                  name: {{ tpl $value.valueFrom.secretKeyRef.name $ }}
-                  key: {{ tpl $value.valueFrom.secretKeyRef.key $ }}
+                  name: {{ tpl $value.valueFrom.secretKeyRef.name $ | quote }}
+                  key: {{ tpl $value.valueFrom.secretKeyRef.key $ | quote }}
             {{- else if $value.valueFrom.configMapKeyRef }}
             - name: {{ $value.name }}
               valueFrom:
                 configMapKeyRef:
-                  name: {{ tpl $value.valueFrom.configMapKeyRef.name $ }}
-                  key: {{ tpl $value.valueFrom.configMapKeyRef.key $ }}
+                  name: {{ tpl $value.valueFrom.configMapKeyRef.name $ | quote }}
+                  key: {{ tpl $value.valueFrom.configMapKeyRef.key $ | quote }}
             {{- end }}
             {{- end }}
             {{- end }}

template-chart/templates/xxx.sfs.yaml

@@ -28,26 +28,34 @@ spec:
         {{- include "xxx.podSecurityContext" .Values.xxx.securityContext | nindent 8 }}
       containers:
         - name: {{ .Release.Name }}-{{ .Chart.Name }}-sfs
-          image: "registry/image:{{ .Chart.AppVersion }}"
+          # Fully qualified on purpose: an unqualified "registry/image" resolves
+          # to Docker Hub, which is how the repo accumulated its unqualified
+          # images. Replace host/org/image, keep the registry host.
+          image: "ghcr.io/ORG/IMAGE:{{ .Chart.AppVersion }}"
           imagePullPolicy: IfNotPresent
           env:
             {{- range $value := .Values.xxx.env }}
             {{- if $value.value }}
-            - name: {{ $value.name }}
-              value: {{ tpl $value.value $ }}
+            - name: {{ $value.name | quote }}
+              # quote is required, not cosmetic: a value like "8080" or "true"
+              # renders unquoted as a YAML number/bool, and the API server then
+              # rejects the whole manifest with "cannot unmarshal number into
+              # Go struct field EnvVar.value of type string". Every chart in
+              # this repo quotes here.
+              value: {{ tpl $value.value $ | quote }}
             {{- else if $value.valueFrom }}
             {{- if $value.valueFrom.secretKeyRef }}
             - name: {{ $value.name }}
               valueFrom:
                 secretKeyRef:
-                  name: {{ tpl $value.valueFrom.secretKeyRef.name $ }}
-                  key: {{ tpl $value.valueFrom.secretKeyRef.key $ }}
+                  name: {{ tpl $value.valueFrom.secretKeyRef.name $ | quote }}
+                  key: {{ tpl $value.valueFrom.secretKeyRef.key $ | quote }}
             {{- else if $value.valueFrom.configMapKeyRef }}
             - name: {{ $value.name }}
               valueFrom:
                 configMapKeyRef:
-                  name: {{ tpl $value.valueFrom.configMapKeyRef.name $ }}
-                  key: {{ tpl $value.valueFrom.configMapKeyRef.key $ }}
+                  name: {{ tpl $value.valueFrom.configMapKeyRef.name $ | quote }}
+                  key: {{ tpl $value.valueFrom.configMapKeyRef.key $ | quote }}
             {{- end }}
             {{- end }}
             {{- end }}

template-chart/values.yaml

@@ -50,6 +50,15 @@ xxx:
       runAsGroup: 1000
       fsGroup: 1000
       fsGroupChangePolicy: Always
+      # RuntimeDefault applies the container runtime's default seccomp filter,
+      # which blocks the syscalls a normal workload never makes. It sits in the
+      # pod context on purpose: from here it covers every container in the pod
+      # including init containers, while a container-level seccompProfile would
+      # have to be repeated for each one (and silently missed on the next one
+      # added). A container that genuinely needs a different filter can still
+      # override it under securityContext.container, which wins over this value.
+      seccompProfile:
+        type: RuntimeDefault
     container:
       allowPrivilegeEscalation: false
       readOnlyRootFilesystem: true
@@ -59,7 +68,29 @@ xxx:
           - ALL
         add: []
   ports:
-    - containerPort: 80
+    # An unprivileged port on purpose (AVD-KSV-0117 flags containerPort < 1024).
+    #
+    # Note what the reason is NOT: on current Kubernetes a non-root container
+    # can bind port 80 just fine. The kubelet sets
+    # net.ipv4.ip_unprivileged_port_start=0 in the pod network namespace, so
+    # the classic "ports below 1024 need NET_BIND_SERVICE" rule does not apply
+    # inside a pod. Measured in k3d (k3s v1.35.5): the sysctl reads 0, and a
+    # container running as runAsUser 20000 with capabilities.drop: [ALL] binds
+    # port 80 successfully. Anyone who "fixes" this back to 80 after testing it
+    # once will find that it works - which is exactly why the reason is written
+    # down here.
+    #
+    # The reason to stay above 1024 is that this is a runtime default, not a
+    # guarantee: a cluster that restores ip_unprivileged_port_start=1024 (node
+    # sysctl or a hardened distribution) breaks every workload that copied the
+    # 80, and the failure then appears in the copied chart, not here. Every
+    # real chart in this repo listens above 1024 (3000, 8080, 8123, 9998, ...);
+    # the one exception is samba, which needs 139/445 and adds NET_BIND_SERVICE
+    # with a documented justification.
+    #
+    # containerPort is declarative - it does not force the application onto the
+    # port. Point the application at the same port when copying this template.
+    - containerPort: 8080
       protocol: TCP
       name: http
   env: