Bring minecraft-server up to the chart style guidelines (1.0.0+upjava8)

Stage 2 of the Fleet migration for minecraft-server. Major bump: runtime behaviour changes (probes, hardening) and the values interface breaks (resource renames, strict schema). Two real bugs first: - The container declared port 35565 while the Service targeted 25565. It only worked because targetPort was numeric. Both are now server.port (25565) and the Service targets the named port. - The download-serverpack init container was guarded by the map .Values.curseforgeServerpack instead of .enabled, so it rendered for every release and silently no-opped on an empty wget URL. Conventions: - Resources renamed to <release>-<chart>-<kind>, templates to <app>.<kind>.yaml. The PVC deliberately keeps its historical name - renaming it would provision a new, empty volume and orphan the worlds. - Probes (liveness/readiness/startup) via the image's own mc-health, so Ready means the server answers a server-list ping. Startup budget 20 min. - Hardened, values-configurable security contexts: non-root uid 1000, read-only root filesystem with an emptyDir at /tmp, all caps dropped. - Resources from values with the limitFactor helper; an unset memory request is derived from the JVM heap so the two cannot drift apart. - Additive env block rendered through tpl, pinned init image, storage class as a value, RCON wiring only when actually enabled. - curseforge/ftb/curseforgeServerpack are now mutually exclusive. - minecraft.version is pinned to the Java line instead of LATEST. - The mc-router auto-discovery annotation is gone; the service type it implicitly switched is now the explicit value server.service.type. Schema pilot (#68): values.schema.json with additionalProperties false on every chart-owned level, rejecting all three known dead keys.
2026-08-23 16:40:44 +02:00
1 parent b1cb1f7911
commit 227bcebeb6
@@ -1,3 +1,112 @@
+{{/*
+Render the body of a container "resources:" block from a values dict of the form
+
+  resources:
+    limitFactor: 3          # optional, default 3
+    requests: { cpu, memory, ephemeral-storage / ephemeralStorage, ... }
+    limits:   { ... }       # optional, every entry optional
+
+Rules (variant B, see issue #19):
+  - requests are rendered as given (camelCase "ephemeralStorage" is normalized
+    to the Kubernetes key "ephemeral-storage").
+  - an explicitly set limit always wins.
+  - a missing memory / ephemeral-storage limit is computed as
+    limitFactor x the corresponding request.
+  - a missing cpu limit is NOT set at all (the kubelet then treats CPU as
+    unlimited; CPU is compressible, so this cannot kill the workload).
+
+Usage (the values path to the resources dict varies per chart):
+  resources:
+    {{- include "minecraft-server.resources" .Values.<app>.resources | nindent 12 }}
+
+NOTE: charts in this repo deliberately have no dependencies, so this helper is
+duplicated into every chart's templates/_helpers.tpl with the chart's name as
+define prefix. The reference implementation lives in template-chart.
+*/}}
+{{- define "minecraft-server.resources" -}}
+{{- $resources := . | default dict -}}
+{{- $factor := 3.0 -}}
+{{- if not (kindIs "invalid" $resources.limitFactor) -}}
+{{- $factor = float64 $resources.limitFactor -}}
+{{- end -}}
+{{- $requests := dict -}}
+{{- range $key, $value := ($resources.requests | default dict) -}}
+{{- if not (kindIs "invalid" $value) -}}
+{{- $_ := set $requests (include "minecraft-server.resourceKey" $key) $value -}}
+{{- end -}}
+{{- end -}}
+{{- $limits := dict -}}
+{{- range $key, $value := ($resources.limits | default dict) -}}
+{{- if not (kindIs "invalid" $value) -}}
+{{- $_ := set $limits (include "minecraft-server.resourceKey" $key) $value -}}
+{{- end -}}
+{{- end -}}
+{{- range $key, $request := $requests -}}
+{{- if and (ne $key "cpu") (not (hasKey $limits $key)) -}}
+{{- $_ := set $limits $key (include "minecraft-server.multiplyQuantity" (dict "quantity" $request "factor" $factor)) -}}
+{{- end -}}
+{{- end -}}
+{{- $out := dict -}}
+{{- with $requests }}{{- $_ := set $out "requests" . }}{{- end -}}
+{{- with $limits }}{{- $_ := set $out "limits" . }}{{- end -}}
+{{- toYaml $out -}}
+{{- end -}}
+
+{{/*
+Normalize a resource name from values to its Kubernetes key.
+*/}}
+{{- define "minecraft-server.resourceKey" -}}
+{{- if eq . "ephemeralStorage" -}}ephemeral-storage{{- else -}}{{ . }}{{- end -}}
+{{- end -}}
+
+{{/*
+Multiply a Kubernetes quantity by a factor, keeping its suffix.
+Input: dict "quantity" <string|number> "factor" <float64>.
+Handles plain numbers ("2", "0.5", 1073741824), decimal ("1.5Gi"), binary
+("512Mi", "1Gi", "10Ki") and milli/SI suffixes ("100m", "500M"). Integer
+results are printed without a decimal point.
+*/}}
+{{- define "minecraft-server.multiplyQuantity" -}}
+{{- $quantity := printf "%v" .quantity -}}
+{{- $number := regexFind "^[0-9]+(\\.[0-9]+)?" $quantity -}}
+{{- if not $number -}}
+{{- fail (printf "cannot parse resource quantity %q" $quantity) -}}
+{{- end -}}
+{{- $suffix := trimPrefix $number $quantity -}}
+{{- if not (regexMatch "^(m|k|M|G|T|P|E|Ki|Mi|Gi|Ti|Pi|Ei)?$" $suffix) -}}
+{{- fail (printf "unsupported resource quantity suffix %q in %q" $suffix $quantity) -}}
+{{- end -}}
+{{- $product := mulf (float64 $number) .factor -}}
+{{- if eq $product (floor $product) -}}
+{{- printf "%d%s" (int64 $product) $suffix -}}
+{{- else -}}
+{{- printf "%v%s" $product $suffix -}}
+{{- end -}}
+{{- end -}}
+
+{{/*
+Resolve the server container's resources.
+
+The JVM heap (minecraft.resources.memory, in GiB) and the container's memory
+request used to be configured in two unrelated places - the heap was a value,
+the request and limit were hardcoded in the template - so raising the heap did
+not raise the container's memory. Here an unset memory request is derived from
+the heap instead, which keeps the two from drifting apart; an explicitly set
+request always wins. The limit follows from the request via the limitFactor
+rule in "minecraft-server.resources".
+
+Input: the root context.
+*/}}
+{{- define "minecraft-server.serverResources" -}}
+{{- $resources := deepCopy (.Values.server.resources | default dict) -}}
+{{- $requests := deepCopy ($resources.requests | default dict) -}}
+{{- if kindIs "invalid" $requests.memory -}}
+{{- $_ := set $requests "memory" (printf "%vGi" .Values.minecraft.resources.memory) -}}
+{{- end -}}
+{{- $_ := set $resources "requests" $requests -}}
+{{- include "minecraft-server.resources" $resources -}}
+{{- end -}}
+
 {{/*
 Render the replica count for a Deployment/StatefulSet.

@@ -15,14 +124,9 @@ Rules (issue #25):
 Usage (the values path to the workload's replicas varies per chart):
  replicas: {{ include "minecraft-server.replicas" (dict "scaleDown" .Values.scaleDown "replicas" .Values.server.replicas) }}

-NOTE: charts in this repo deliberately have no dependencies, so this helper is
-duplicated into every chart's templates/_helpers.tpl with the chart's name as
-define prefix. The reference implementation lives in template-chart.
-
-This chart is a stage-1 import (issue #67) and does not yet follow the repo
-conventions; only the replicas helper was taken over, because the scaleDown
-flag is needed before the Fleet migration. The resources helper follows in
-stage 2 together with the rest of the convention alignment.
+NOTE: like the resources helper above, this is duplicated into every chart's
+templates/_helpers.tpl with the chart's name as define prefix. The reference
+implementation lives in template-chart.
 */}}
 {{- define "minecraft-server.replicas" -}}
 {{- if .scaleDown -}}
@@ -33,3 +137,22 @@ stage 2 together with the rest of the convention alignment.
 1
 {{- end -}}
 {{- end -}}
+
+{{/*
+Guard: curseforge, ftb and curseforgeServerpack are mutually exclusive.
+
+Enabling more than one used to render a duplicate TYPE env entry, where the
+last one silently won. Fail loudly instead.
+*/}}
+{{- define "minecraft-server.validateModpackSources" -}}
+{{- $enabled := list -}}
+{{- if .Values.curseforge.enabled -}}{{- $enabled = append $enabled "curseforge" -}}{{- end -}}
+{{- if .Values.ftb.enabled -}}{{- $enabled = append $enabled "ftb" -}}{{- end -}}
+{{- if .Values.curseforgeServerpack.enabled -}}{{- $enabled = append $enabled "curseforgeServerpack" -}}{{- end -}}
+{{- if gt (len $enabled) 1 -}}
+{{- fail (printf "minecraft-server: curseforge, ftb and curseforgeServerpack are mutually exclusive, but these are enabled at the same time: %s - enable at most one modpack source" (join ", " $enabled)) -}}
+{{- end -}}
+{{- if and .Values.curseforgeServerpack.enabled (not .Values.curseforgeServerpack.downloadUrl) -}}
+{{- fail "minecraft-server: curseforgeServerpack.enabled is true but curseforgeServerpack.downloadUrl is not set - the init container would download an empty URL" -}}
+{{- end -}}
+{{- end -}}
@@ -1,11 +0,0 @@
-apiVersion: v1
-kind: PersistentVolumeClaim
-metadata:
-  name: {{ .Release.Name }}-mc-volume-claim
-spec:
-  accessModes:
-    - ReadWriteOnce
-  resources:
-    requests:
-      storage: {{ .Values.minecraft.resources.storage }}
-  storageClassName: longhorn
@@ -1,25 +0,0 @@
-apiVersion: v1
-kind: Service
-metadata:
-  name: {{ .Release.Name }}-service
-  {{- if .Values.mcRouterIntegration.enabled }}
-  annotations:
-    "mc-router.itzg.me/externalServerName": "{{ .Values.mcRouterIntegration.domain }}"
-  {{- end }}
-spec:
-  selector:
-    app: {{ .Release.Name }}-server-deployment
-  {{- if .Values.mcRouterIntegration.enabled }}
-  ports:
-    - name: mc-port
-      port: 25565
-      targetPort: 25565
-  type: ClusterIP
-  {{- else }}
-  ports:
-    - nodePort: {{ .Values.server.outwardPort }}
-      port: 25565
-      targetPort: 25565
-  type: NodePort
-  {{- end }}
-  
@@ -1,9 +1,10 @@
 apiVersion: v1
 kind: ConfigMap
 metadata:
-  name: {{ .Release.Name }}-init-config
+  name: {{ .Release.Name }}-{{ .Chart.Name }}-configmap
 data:
  init.sh: |
+    set -e
    echo "downloading serverpack"
    wget -O /modpacks/serverpack.zip {{ .Values.curseforgeServerpack.downloadUrl }}
    SERVERPACK_DIR="{{ .Values.curseforgeServerpack.workingDirectory }}"
@@ -32,8 +33,14 @@ data:
      cp $BACKUP_DIR/ops.json $SERVERPACK_DIR/ops.json
      cp $BACKUP_DIR/whitelist.json $SERVERPACK_DIR/whitelist.json

+      # The init container runs as the same uid as the server (see
+      # securityContext.pod) and cp creates the copies owned by that uid, so
+      # this is a no-op in the normal case. It is kept for volumes whose files
+      # still carry a foreign owner from an older chart version, and it must
+      # not be fatal: an unprivileged process cannot chown foreign files, and
+      # with "set -e" a failure here would keep the whole pod from starting.
      echo "change owner of files"
-      chown -R 1000:1000 $SERVERPACK_DIR
+      chown -R 1000:1000 $SERVERPACK_DIR || echo "chown skipped (not permitted); continuing"

      echo "successfully migrated server"

@@ -0,0 +1,15 @@
+apiVersion: v1
+kind: PersistentVolumeClaim
+metadata:
+  # Deliberate exception from the release-chart-kind naming scheme: this chart
+  # is already deployed with 32Gi worlds on these claims, and renaming a PVC
+  # makes Kubernetes provision a new, empty volume and orphan the existing
+  # data (see README, naming exceptions).
+  name: {{ .Release.Name }}-mc-volume-claim
+spec:
+  accessModes:
+    - ReadWriteOnce
+  resources:
+    requests:
+      storage: {{ .Values.minecraft.resources.storage }}
+  storageClassName: {{ .Values.minecraft.resources.storageClassName }}
@@ -1,6 +1,6 @@
 apiVersion: onepassword.com/v1
 kind: OnePasswordItem
 metadata:
-  name: {{ .Release.Name }}-mc-secret
+  name: {{ .Release.Name }}-{{ .Chart.Name }}-secret
 spec:
  itemPath: {{ .Values.secrets.minecraftServerRef }}
@@ -0,0 +1,22 @@
+apiVersion: v1
+kind: Service
+metadata:
+  name: {{ .Release.Name }}-{{ .Chart.Name }}-service
+  labels:
+    app: {{ .Release.Name }}-{{ .Chart.Name }}-service
+spec:
+  type: {{ .Values.server.service.type }}
+  selector:
+    app: {{ .Release.Name }}-{{ .Chart.Name }}-sfs
+  ports:
+    - name: mc-port
+      protocol: TCP
+      port: {{ .Values.server.service.port }}
+      # Named target, so a server.port override cannot break the routing.
+      # Before chart 1.0.0 this was the number 25565 while the container
+      # declared port 35565 - the mismatch went unnoticed precisely because
+      # the target was numeric.
+      targetPort: mc-port
+      {{- if eq .Values.server.service.type "NodePort" }}
+      nodePort: {{ .Values.server.service.outwardPort }}
+      {{- end }}
@@ -1,32 +1,62 @@
+{{- include "minecraft-server.validateModpackSources" . }}
 apiVersion: apps/v1
 kind: StatefulSet
 metadata:
-  name: {{ .Release.Name }}-server-deployment
+  name: {{ .Release.Name }}-{{ .Chart.Name }}-sfs
  labels:
-    app: {{ .Release.Name }}
+    app: {{ .Release.Name }}-{{ .Chart.Name }}-sfs
 spec:
  replicas: {{ include "minecraft-server.replicas" (dict "scaleDown" .Values.scaleDown "replicas" .Values.server.replicas) }}
  selector:
    matchLabels:
-      app: {{ .Release.Name }}-server-deployment
+      app: {{ .Release.Name }}-{{ .Chart.Name }}-sfs
+  serviceName: {{ .Release.Name }}-{{ .Chart.Name }}-service
  template:
    metadata:
-      name: {{ .Release.Name }}-server-deployment
      labels:
-        app: {{ .Release.Name }}-server-deployment
+        app: {{ .Release.Name }}-{{ .Chart.Name }}-sfs
    spec:
      automountServiceAccountToken: false
+      securityContext:
+        {{- toYaml .Values.securityContext.pod | nindent 8 }}
+      {{- if .Values.curseforgeServerpack.enabled }}
+      initContainers:
+        - name: download-serverpack
+          image: {{ .Values.server.initImage }}
+          command: [ 'sh', '/init/init.sh' ]
+          securityContext:
+            {{- toYaml .Values.securityContext.container | nindent 12 }}
+          volumeMounts:
+            - name: "mc-volume"
+              mountPath: /modpacks
+              subPath: mc-modpacks/
+            - name: "mc-volume"
+              mountPath: /data
+              subPath: mc-data/
+            - name: init-script
+              mountPath: /init/init.sh
+              subPath: init.sh
+            - name: tmp
+              mountPath: /tmp
+      {{- end }}
      containers:
-        - name: {{ .Release.Name }}-mc-server-container
+        - name: {{ .Release.Name }}-{{ .Chart.Name }}-sfs
          image: "itzg/minecraft-server:{{ .Chart.AppVersion }}"
          imagePullPolicy: IfNotPresent
+          securityContext:
+            {{- toYaml .Values.securityContext.container | nindent 12 }}
          ports:
            - name: mc-port
-              containerPort: 35565
+              containerPort: {{ .Values.server.port }}
+              protocol: TCP
+            {{- if .Values.server.rcon.enabled }}
            - name: minecraft-rcon
-              containerPort: 25575
+              containerPort: {{ .Values.server.rcon.port }}
+              protocol: TCP
+            {{- end }}
            - name: metrics
              containerPort: 19565
+              protocol: TCP
          env:
            - name: EULA
              value: "true"
@@ -36,21 +66,22 @@ spec:
              value: "{{ .Values.server.motd }}"
            - name: DIFFICULTY
              value: {{ .Values.minecraft.difficulty }}
+            - name: SERVER_PORT
+              value: "{{ .Values.server.port }}"
            {{- if .Values.minecraft.whitelist.enabled }}
            - name: EXISTING_WHITELIST_FILE
              value: {{ .Values.minecraft.whitelist.overridePolicy }}
+            {{- with .Values.minecraft.whitelist.players }}
            - name: WHITELIST
-              value: {{ .Values.minecraft.whitelist.players }}
+              value: "{{ . }}"
            {{- end }}
+            {{- end }}
+            {{- with .Values.minecraft.management.opPlayers }}
            - name: OPS
-              value: {{ .Values.minecraft.management.opPlayers }}
+              value: "{{ . }}"
+            {{- end }}
            - name: EXISTING_OPS_FILE
              value: "SYNCHRONIZE"
-            - name: RCON_PASSWORD
-              valueFrom:
-                secretKeyRef:
-                  key: rcon-password
-                  name: {{ .Release.Name }}-mc-secret
            - name: MAX_PLAYERS
              value: "{{ .Values.minecraft.management.maxPlayers }}"
            - name: FORCE_GAMEMODE
@@ -66,7 +97,16 @@ spec:
            - name: VERSION
              value: "{{ .Values.minecraft.version }}"
            - name: ENABLE_RCON
-              value: "false"
+              value: "{{ .Values.server.rcon.enabled }}"
+            {{- if .Values.server.rcon.enabled }}
+            - name: RCON_PORT
+              value: "{{ .Values.server.rcon.port }}"
+            - name: RCON_PASSWORD
+              valueFrom:
+                secretKeyRef:
+                  key: rcon-password
+                  name: {{ .Release.Name }}-{{ .Chart.Name }}-secret
+            {{- end }}
            - name: ENABLE_AUTOPAUSE
              value: "false"
            - name: MEMORY
@@ -78,7 +118,7 @@ spec:
              valueFrom:
                secretKeyRef:
                  key: curseforge-api-key
-                  name: {{ .Release.Name }}-mc-secret
+                  name: {{ .Release.Name }}-{{ .Chart.Name }}-secret
            - name: CF_SLUG
              value: "{{ .Values.curseforge.modpack.slug }}"
            - name: CF_FILE_ID
@@ -108,15 +148,34 @@ spec:
            - name: FTB_MODPACK_VERSION_ID
              value: "{{ .Values.ftb.versionId }}"
            {{- end }}
+            {{- range $value := .Values.env }}
+            {{- if $value.value }}
+            - name: {{ $value.name }}
+              value: {{ tpl $value.value $ }}
+            {{- 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 $ }}
+            {{- else if $value.valueFrom.configMapKeyRef }}
+            - name: {{ $value.name }}
+              valueFrom:
+                configMapKeyRef:
+                  name: {{ tpl $value.valueFrom.configMapKeyRef.name $ }}
+                  key: {{ tpl $value.valueFrom.configMapKeyRef.key $ }}
+            {{- end }}
+            {{- end }}
+            {{- end }}
+          livenessProbe:
+            {{- toYaml .Values.livenessProbe | nindent 12 }}
+          readinessProbe:
+            {{- toYaml .Values.readinessProbe | nindent 12 }}
+          startupProbe:
+            {{- toYaml .Values.startupProbe | nindent 12 }}
          resources:
-            requests:
-              cpu: "2"
-              memory: "4Gi"
-              ephemeral-storage: "100Mi"
-            limits:
-              cpu: "4"
-              memory: "12Gi"
-              ephemeral-storage: "200Mi"
+            {{- include "minecraft-server.serverResources" . | nindent 12 }}
          volumeMounts:
            - name: "mc-volume"
              mountPath: /data
@@ -124,30 +183,20 @@ spec:
            - name: "mc-volume"
              mountPath: /modpacks
              subPath: mc-modpacks/
-      {{- if .Values.curseforgeServerpack }}
-      initContainers:
-        - name: download-serverpack
-          image: alpine
-          command: [ 'sh', '/init/init.sh' ]
-          volumeMounts:
-            - name: "mc-volume"
-              mountPath: /modpacks
-              subPath: mc-modpacks/
-            - name: "mc-volume"
-              mountPath: /data
-              subPath: mc-data/
-            - name: init-script
-              mountPath: /init/init.sh
-              subPath: init.sh
-      {{- end }}
-      restartPolicy: Always
+            # Writable path required by readOnlyRootFilesystem: the image writes
+            # its server-icon scratch file and mc-server-runner's console named
+            # pipe to /tmp. Everything else goes to the PVC at /data.
+            - name: tmp
+              mountPath: /tmp
      volumes:
        - name: "mc-volume"
          persistentVolumeClaim:
+            # Deliberate exception from the naming scheme: renaming the PVC
+            # would make Kubernetes provision a new, empty volume and orphan
+            # the existing world (see README, naming exceptions).
            claimName: {{ .Release.Name }}-mc-volume-claim
        - name: init-script
          configMap:
-            name: {{ .Release.Name }}-init-config
-  serviceName: {{ .Release.Name }}-service
-
-
+            name: {{ .Release.Name }}-{{ .Chart.Name }}-configmap
+        - name: tmp
+          emptyDir: {}