From 5521e498a96b0030ac080474ecb647d69967f5e1 Mon Sep 17 00:00:00 2001 From: Lemon-miaow Date: Thu, 24 Sep 2026 16:28:08 +0800 Subject: [PATCH] =?UTF-8?q?fix(platform):=20minecraft=20=E5=91=BD=E5=90=8D?= =?UTF-8?q?=E7=A9=BA=E9=97=B4=E5=BC=BA=E5=88=B6=20PodSecurity=20baseline?= =?UTF-8?q?=EF=BC=8Creaper=20=E4=B8=96=E7=95=8C=E6=A0=B9=E7=9B=AE=E5=BD=95?= =?UTF-8?q?=E6=94=B9=E8=B5=B0=E9=9D=99=E6=80=81=20hostPath=20PV?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- cmd/felis/manifests.go | 4 +- deploy/bootstrap.sh | 16 ++--- deploy/bootstrap_test.sh | 10 ++- docs/troubleshooting.md | 51 +++++++++---- internal/platform/bundle.go | 40 ++++++++++- internal/platform/bundle_test.go | 59 ++++++++++++++++ internal/platform/workloads.go | 106 ++++++++++++++++++++++++---- internal/platform/workloads_test.go | 80 +++++++++++++++++++-- 8 files changed, 315 insertions(+), 51 deletions(-) diff --git a/cmd/felis/manifests.go b/cmd/felis/manifests.go index cab3d64..947e1e2 100644 --- a/cmd/felis/manifests.go +++ b/cmd/felis/manifests.go @@ -124,8 +124,8 @@ func cmdManifests(args []string, stdout, stderr io.Writer) int { "multi-node cluster you MUST pass --reaper-node (or add a nodeSelector) for the node holding the " + "worlds, or the reaper may schedule where the hostPath is empty" if *reaperNode != "" { - pin = fmt.Sprintf("the CronJob is pinned to node %q via kubernetes.io/hostname — keep this pointed at the "+ - "node that actually holds the world volumes", *reaperNode) + pin = fmt.Sprintf("the CronJob and its worlds-root PV are pinned to node %q via kubernetes.io/hostname — "+ + "keep this pointed at the node that actually holds the world volumes", *reaperNode) } fmt.Fprintf(stderr, "felis manifests: note: rendering the retention reaper CronJob (worlds hostPath %q). "+ "These points are NOT verified here:\n"+ diff --git a/deploy/bootstrap.sh b/deploy/bootstrap.sh index 7daebc7..550a814 100644 --- a/deploy/bootstrap.sh +++ b/deploy/bootstrap.sh @@ -2496,18 +2496,10 @@ deploy_bundle() { # always travels with it because it must equal the [archive] local_path written above. if [ -n "$FELIS_WORLDS_HOST_PATH" ]; then log "retention enabled: the daily reaper will read worlds from ${FELIS_WORLDS_HOST_PATH}" - # The reaper pod runs as the tree's non-root uid (1000, platform.workloads.nonRootUID) - # and must traverse into the per-volume directories under this root. k3s's own storage - # root ships 0700 root:root, so grant traverse — an ACL entry when the host has setfacl, - # otherwise the equivalent o+x. Traverse only: no listing either way, and the per-volume - # directories themselves are world-accessible (local-path creates them 0777). - if [ -d "$FELIS_WORLDS_HOST_PATH" ]; then - if command -v setfacl >/dev/null 2>&1; then - setfacl -m u:1000:x "$FELIS_WORLDS_HOST_PATH" || chmod o+x "$FELIS_WORLDS_HOST_PATH" - else - chmod o+x "$FELIS_WORLDS_HOST_PATH" - fi - else + # The reaper reads this root as root with DAC_OVERRIDE (platform.reaperPodSecurityContext) + # through a static hostPath PV, so the host directory keeps k3s's own 0700 root:root and + # needs no extra grant. It must exist, though: the PV declares type Directory. + if [ ! -d "$FELIS_WORLDS_HOST_PATH" ]; then warn "worlds root ${FELIS_WORLDS_HOST_PATH} does not exist yet; the reaper CronJob cannot start until it does (hostPath type Directory)" fi manifest_args+=(--worlds-host-path "$FELIS_WORLDS_HOST_PATH" --archive-local-path "$FELIS_ARCHIVE_LOCAL_PATH") diff --git a/deploy/bootstrap_test.sh b/deploy/bootstrap_test.sh index c69a2a4..295b9b2 100644 --- a/deploy/bootstrap_test.sh +++ b/deploy/bootstrap_test.sh @@ -670,6 +670,7 @@ run_bundle_flags() { # backup-pvc worlds-host-path kube() { cat; } myManifests() { printf "%s\n" "$@"; } setfacl() { printf "SETFACL %s\n" "$*"; } + chmod() { printf "CHMOD %s\n" "$*"; } node_global_cidrs() { printf "203.0.113.7/32\n2001:db8::7/128\n"; } run_bundle() { '"$mblock"' @@ -723,11 +724,14 @@ missing="/tmp/felis-worlds-root-must-not-exist-$$" out="$(run_bundle_flags felis-backups "$missing")" expect "a missing worlds root is warned about, not silently skipped" "WARN: worlds root $missing does not exist yet" "$out" -# The reaper pod is non-root (uid 1000) and k3s ships the storage root 0700 root:root, so -# the installer must grant traverse or every archive dies with permission denied. +# The reaper reads the root as root with DAC_OVERRIDE through a static PV, so an existing +# root is left exactly as k3s shipped it: uid 1000 is now the game servers' uid, and a +# traverse grant for it on the node's storage root would serve nothing but them. wdir="$(mktemp -d)" out="$(run_bundle_flags felis-backups "$wdir")" -expect "enabling retention grants the reaper uid traverse on the worlds root" "SETFACL -m u:1000:x $wdir" "$out" +case "$out" in + *SETFACL*|*CHMOD*|*WARN*) echo "FAIL: an existing worlds root must get no grant and no warning: $out"; fails=$((fails + 1)) ;; +esac # --- the registry mirror writer ----------------------------------------------------------- # k3s only consults registries.yaml at agent start, so a CONTENT change must restart k3s and diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 3ffc021..cf7ab85 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -91,6 +91,26 @@ kubectl describe pod # look at Events + container State Fix the StorageClass name or capacity. [INTEGRATION-ONLY.] - **Container crash-looping before the readiness port opens** → check container logs; this is a backend/entrypoint problem, not a Felis problem. +- **Stuck in `Init:` or `AccessDeniedException` / `Permission denied` in the + log** → the pod runs as uid/gid **1000** (`naming.GameUID`) with every + capability dropped, whatever `USER` the image declares. Before the server + starts, the `prepare-data` initContainer (`felis init-volume`, root with only + `CHOWN` + `DAC_OVERRIDE`) hands every world entry not yet owned by 1000:1000 + to that uid, so a world written by an older root-run release or extracted by a + restore Job is fixed on its next start: + `kubectl logs -c prepare-data` prints how many entries it changed and + lists up to 20 it could not. An image that writes outside `/data` and `/tmp` + (a directory baked into the image as root) cannot run as uid 1000; rebuild it + to keep its state under `/data`. [GO-TESTED: `TestBuildStatefulSetRunsGameAsNonRoot`, + `TestChownTreeHandsOverMismatchedEntries`; INTEGRATION-ONLY for the walk on a + live volume.] +- **`FailedCreate … violates PodSecurity "baseline"`** on the StatefulSet or a + Job → the minecraft namespace enforces the PodSecurity `baseline` profile + (`pod-security.kubernetes.io/enforce=baseline`, set by the install bundle). + Everything Felis renders there fits it; a pod that is refused was edited or + created outside Felis (hostPath, hostPort, privileged, extra capabilities). + `kubectl get events -n minecraft --field-selector reason=FailedCreate` names + the field. [GO-TESTED: `TestObjects_MinecraftNamespaceEnforcesBaseline`.] ### 1b. `RconSecretUnavailable` — RCON secret missing or malformed @@ -681,7 +701,13 @@ store paths are [INTEGRATION-ONLY]. ### Where worlds are read from (hostPath resolution) -The CronJob mounts `--worlds-host-path` read-only at `/worlds`; the resolver +The CronJob mounts `--worlds-host-path` read-only at `/worlds` through a static +PersistentVolume (`felis-worlds-root-`, hostPath type `Directory`, +`Retain`, pre-bound to the same-named PVC in the minecraft namespace), since the +namespace's PodSecurity baseline refuses an inline hostPath in any pod. A +re-install with a different worlds root or `--reaper-node` renders a new pair +under a new digest; the old PV/PVC pair is left behind unused and can be +deleted by hand (Retain: deleting it never touches the directory). The resolver runs `cmd/felis/reaper.resolveWorldDir`: it looks for `/`, then for the stock local-path directory `/__` derived from the live PVC's `spec.volumeName` (never a glob — a leftover directory of a @@ -691,19 +717,18 @@ supported way to enable retention on a stock install. Two deployment facts the resolver cannot fix: - **Permissions.** The reaper Pod runs as **root** and carries `DAC_OVERRIDE`: - worlds are written by the game image's own UID (root for every Paper image we - ship), and Paper saves `level.dat` mode-0600, so any fixed non-root identity - (the previous uid-1000 convention, and the ACL setup that went with it) could - neither walk the tree nor read the files — every archive failed - `open …/level.dat: permission denied` and the same defect failed on-demand - backups/restores. Root is the same identity the game container itself runs as - (see the operator's forwarding-init note); `DAC_OVERRIDE` extends the archive - to game images with a different UID. If a world is still **preserved** while a - reap was expected, it is now a different cause: check the run's ERROR logs for - the resolver's `lstat` messages before suspecting permissions. + k3s's storage root is `0700 root:root`, worlds are written by the game uid + (1000) — or by root, for a world an older release wrote — and Paper saves + `level.dat` mode-0600, so a fixed non-root identity (the previous uid-1000 + convention, and the ACL setup that went with it) could neither walk the tree + nor read the files — every archive failed `open …/level.dat: permission + denied`. The installer no longer grants uid 1000 any access to the storage + root: that uid is now the game servers'. If a world is still **preserved** + while a reap was expected, it is a different cause: check the run's ERROR + logs for the resolver's `lstat` messages before suspecting permissions. - **Node placement.** Multi-node clusters: the world's directory exists only on - the node holding its volume, and the CronJob sets no `nodeSelector`, so add - one (single-node starters are pinned implicitly). + the node holding its volume, so pass `--reaper-node`; it pins both the + CronJob's pod and the PV (single-node starters are pinned implicitly). --- diff --git a/internal/platform/bundle.go b/internal/platform/bundle.go index e2551dc..d7cdd17 100644 --- a/internal/platform/bundle.go +++ b/internal/platform/bundle.go @@ -3,6 +3,7 @@ package platform import ( "bytes" "fmt" + "maps" "felis.lolicon.best/internal/build" "felis.lolicon.best/internal/restore" @@ -51,7 +52,11 @@ func Objects(p Params) []Object { // namespaceSelectors match on. (K8s ≥1.21 adds this label automatically, but // rendering it makes the bundle self-contained and the selectors provable.) for _, ns := range distinctNamespaces(p) { - objs = append(objs, namespaceObject(ns)) + nsObj := namespaceObject(ns) + if ns == p.MinecraftNamespace && minecraftNamespaceIsOwn(p) { + maps.Copy(nsObj.Labels, minecraftPodSecurityLabels) + } + objs = append(objs, nsObj) } // Control-plane RBAC: SAs, then Roles, then RoleBindings. @@ -143,6 +148,39 @@ func distinctNamespaces(p Params) []string { return out } +// minecraftPodSecurityLabels put the minecraft namespace under the PodSecurity +// admission baseline profile. Everything Felis runs there fits it: game servers run +// as naming.GameUID with every capability dropped, their prepare-data init and the +// file/backup/restore Jobs run as root holding at most CHOWN and DAC_OVERRIDE (both +// on baseline's allow-list), and the reaper reaches the node's worlds-root through a +// static PV rather than an inline hostPath. What baseline then refuses — privileged +// containers, host namespaces and ports, inline hostPath, extra capabilities — is +// exactly what a pod smuggled in through any other write path to this namespace +// would need to reach the node. +// +// warn repeats the enforced level so a StatefulSet or Job that would render a +// refused pod reports it at apply time, instead of the controller failing to create +// pods quietly. audit records restricted-profile violations for the path toward +// restricted (only the root prepare-data init and root Jobs stand in its way). +var minecraftPodSecurityLabels = map[string]string{ + "pod-security.kubernetes.io/enforce": "baseline", + "pod-security.kubernetes.io/enforce-version": "latest", + "pod-security.kubernetes.io/warn": "baseline", + "pod-security.kubernetes.io/warn-version": "latest", + "pod-security.kubernetes.io/audit": "restricted", + "pod-security.kubernetes.io/audit-version": "latest", +} + +// minecraftNamespaceIsOwn reports whether the minecraft namespace is shared with +// no other component. The registry (hostPort) and the build Jobs do not fit the +// baseline profile, so the labels go on only when neither lives there, and the +// control plane's namespace is never labelled from here. +func minecraftNamespaceIsOwn(p Params) bool { + return p.MinecraftNamespace != p.ControlNamespace && + p.MinecraftNamespace != p.BuildNamespace && + p.MinecraftNamespace != p.RegistryNamespace +} + // namespaceObject renders a Namespace carrying the immutable name label the // NetworkPolicy namespaceSelectors key on. func namespaceObject(name string) *corev1.Namespace { diff --git a/internal/platform/bundle_test.go b/internal/platform/bundle_test.go index 67fd08a..e89c861 100644 --- a/internal/platform/bundle_test.go +++ b/internal/platform/bundle_test.go @@ -4,6 +4,8 @@ import ( "strings" "testing" + appsv1 "k8s.io/api/apps/v1" + batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" networkingv1 "k8s.io/api/networking/v1" "sigs.k8s.io/yaml" @@ -72,6 +74,63 @@ func TestObjects_NamespacesLabeled(t *testing.T) { } } +// TestObjects_MinecraftNamespaceEnforcesBaseline pins the PodSecurity labels on the +// minecraft namespace, and proves nothing the bundle renders into it would be +// refused by them: no pod template there mounts an inline hostPath or uses a host +// port. The control namespace (registry hostPort) stays unlabelled, and a layout +// that folds the minecraft namespace into another one drops the labels. +func TestObjects_MinecraftNamespaceEnforcesBaseline(t *testing.T) { + p := reaperParams() + for _, obj := range Objects(p) { + if ns, ok := obj.(*corev1.Namespace); ok { + enforce := ns.Labels["pod-security.kubernetes.io/enforce"] + switch ns.Name { + case "minecraft": + if enforce != "baseline" || ns.Labels["pod-security.kubernetes.io/warn"] != "baseline" { + t.Errorf("minecraft namespace labels = %v, want enforce+warn baseline", ns.Labels) + } + default: + if enforce != "" { + t.Errorf("namespace %s must not be labelled by the bundle, got enforce=%q", ns.Name, enforce) + } + } + } + if obj.GetNamespace() != "minecraft" { + continue + } + var spec *corev1.PodSpec + switch o := obj.(type) { + case *batchv1.CronJob: + spec = &o.Spec.JobTemplate.Spec.Template.Spec + case *appsv1.Deployment: + spec = &o.Spec.Template.Spec + } + if spec == nil { + continue + } + for _, v := range spec.Volumes { + if v.HostPath != nil { + t.Errorf("%s/%s mounts inline hostPath %q, which baseline refuses", obj.GetNamespace(), obj.GetName(), v.HostPath.Path) + } + } + for _, c := range append(append([]corev1.Container{}, spec.InitContainers...), spec.Containers...) { + for _, port := range c.Ports { + if port.HostPort != 0 { + t.Errorf("%s/%s container %s uses hostPort %d, which baseline refuses", obj.GetNamespace(), obj.GetName(), c.Name, port.HostPort) + } + } + } + } + + shared := testParams() + shared.MinecraftNamespace = shared.withDefaults().ControlNamespace + for _, obj := range Objects(shared) { + if ns, ok := obj.(*corev1.Namespace); ok && ns.Labels["pod-security.kubernetes.io/enforce"] != "" { + t.Errorf("a minecraft namespace shared with the control plane must stay unlabelled, got %v", ns.Labels) + } + } +} + // TestWeakJobSAs_Isolated proves the build/restore SAs are present, disable token // auto-mounting, and — the key isolation invariant — are referenced by NO // RoleBinding anywhere. Their powerlessness is the absence of any binding. diff --git a/internal/platform/workloads.go b/internal/platform/workloads.go index 2e3380e..3fb7d18 100644 --- a/internal/platform/workloads.go +++ b/internal/platform/workloads.go @@ -1,6 +1,8 @@ package platform import ( + "crypto/sha256" + "encoding/hex" "fmt" "felis.lolicon.best/internal/naming" @@ -28,8 +30,8 @@ import ( // because archiving idle worlds means mounting where the worlds physically live, // and the spec keeps that open (§18/§19: tarLocal-on-local-path is the starter, // Longhorn/snapshot the documented evolution, and they do not share a mount -// model). The starter model — a node-local hostPath worlds-root mounted -// read-only — is the only one coherent with the operator's per-server +// model). The starter model — a node-local worlds-root, exposed through a static +// hostPath PV and mounted read-only — is the only one coherent with the operator's per-server // ReadWriteOnce world PVCs (a shared RWX worlds mount would contradict them), so // that is what renders; when the trio is absent no CronJob is emitted, which is // the fail-safe choice for a workload that deletes PVCs. The reaper resolver @@ -41,8 +43,8 @@ import ( // the hosting node, and is not provable without a cluster. No nodeSelector is set // unless ReaperNode names one: the single-node starter pins the worlds to one node // implicitly, while a multi-node deployment passes --reaper-node (rendered as a -// kubernetes.io/hostname selector) or the CronJob could schedule on a node where -// the hostPath is empty. +// kubernetes.io/hostname selector and PV node affinity) or the CronJob could +// schedule on a node where the worlds-root is empty. const ( // configSecretName / serviceTokenSecretName are referenced BY NAME and NEVER // rendered into the bundle: felis.toml carries the database URL (a credential) @@ -213,7 +215,7 @@ func Workloads(p Params) []Object { objs = append(objs, backupPVC(p)) } if reaperEnabled(p) { - objs = append(objs, reaperCronJob(p)) + objs = append(objs, worldsRootPV(p), worldsRootPVC(p), reaperCronJob(p)) } return objs } @@ -522,23 +524,24 @@ func OperatorDeployment(p Params) *appsv1.Deployment { // workloads_test.go — the reaper never opens an RCON connection. // // Mounts (the storage crux). felis.toml is mounted read-only from the config -// Secret (it carries the DB URL). The worlds-root is a node-local hostPath mounted -// READ-ONLY at /worlds: the reaper only reads worlds to tar them; deleting a world +// Secret (it carries the DB URL). The worlds-root is the node directory behind the +// static worldsRootPV, claimed by worldsRootPVC and mounted READ-ONLY at /worlds +// (why a PV rather than an inline hostPath: see worldsRootPV). The reaper only +// reads worlds to tar them; deleting a world // is a K8s API call (DeletePVC), never an rm, so the mount never needs write. The // backup PVC is mounted READ-WRITE at p.ArchiveLocalPath — which MUST equal // felis.toml [archive] local_path, because tarLocal writes archive refs as absolute // paths under it and the restore Job later mounts the same PVC at the same path to // resolve them (see the ArchiveLocalPath field doc). A /tmp emptyDir absorbs writes -// under the read-only root filesystem. hostPath type Directory fails the pod loud -// if the worlds-root is absent, rather than silently creating an empty dir and -// archiving nothing. +// under the read-only root filesystem. The PV's hostPath type Directory fails the +// pod loud if the worlds-root is absent, rather than silently creating an empty dir +// and archiving nothing. // // Pre-conditions are the caller's: reaperCronJob assumes reaperEnabled(p) — it // dereferences WorldsHostPath / BackupPVC / ArchiveLocalPath without re-checking. func reaperCronJob(p Params) *batchv1.CronJob { p = p.withDefaults() labels := controlPlanePodLabels(ComponentReaper) - hostPathDir := corev1.HostPathDirectory container := corev1.Container{ Name: ComponentReaper, @@ -582,7 +585,9 @@ func reaperCronJob(p Params) *batchv1.CronJob { { Name: worldsVolume, VolumeSource: corev1.VolumeSource{ - HostPath: &corev1.HostPathVolumeSource{Path: p.WorldsHostPath, Type: &hostPathDir}, + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: worldsRootName(p), ReadOnly: true, + }, }, }, { @@ -625,6 +630,81 @@ func reaperCronJob(p Params) *batchv1.CronJob { } } +// worldsRootName names the static PV and its PVC. The PV's hostPath and node +// affinity are immutable once created, so the name carries a digest of them: a +// re-install that moves the worlds-root renders a fresh pair (and the reaper +// follows it) instead of an apply the API server would refuse. The namespace is in +// the digest because a PV is cluster-scoped and two installs must not collide. +func worldsRootName(p Params) string { + sum := sha256.Sum256([]byte(p.MinecraftNamespace + "\x00" + p.WorldsHostPath + "\x00" + p.ReaperNode)) + return "felis-worlds-root-" + hex.EncodeToString(sum[:4]) +} + +// worldsRootStorage is the nominal size both halves of the static pair declare. A +// hostPath volume enforces no quota; the PVC only has to request no more than the +// PV offers for the two to bind. +const worldsRootStorage = "1Gi" + +// worldsRootPV exposes the node's worlds-root to the reaper as a pre-bound static +// PersistentVolume. The reaper's pod then mounts a PVC, not a hostPath, and that is +// what lets the minecraft namespace enforce the PodSecurity baseline profile (see +// minecraftPodSecurityLabels): baseline refuses any pod with an inline hostPath +// volume, whoever submits it. The node path is still reachable, but only through +// this PV, which is cluster-scoped and so something only a cluster admin — the +// installer applying this bundle — can create. A game server, file-editor or +// backup pod in the namespace cannot name a node path of its own. +// +// Retain keeps a deleted claim from ever handing the path to a reclaimer, the +// empty storageClassName keeps the default provisioner out of it, and the claimRef +// reserves it for exactly worldsRootPVC. With ReaperNode set the PV carries the +// same node pin as the reaper pod, since the directory exists only on that node. +func worldsRootPV(p Params) *corev1.PersistentVolume { + p = p.withDefaults() + name := worldsRootName(p) + hostPathDir := corev1.HostPathDirectory + pv := &corev1.PersistentVolume{ + TypeMeta: metav1.TypeMeta{APIVersion: "v1", Kind: "PersistentVolume"}, + ObjectMeta: metav1.ObjectMeta{Name: name, Labels: controlPlanePodLabels(ComponentReaper)}, + Spec: corev1.PersistentVolumeSpec{ + Capacity: corev1.ResourceList{corev1.ResourceStorage: resource.MustParse(worldsRootStorage)}, + AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, + PersistentVolumeReclaimPolicy: corev1.PersistentVolumeReclaimRetain, + StorageClassName: "", + ClaimRef: &corev1.ObjectReference{Namespace: p.MinecraftNamespace, Name: name}, + PersistentVolumeSource: corev1.PersistentVolumeSource{ + HostPath: &corev1.HostPathVolumeSource{Path: p.WorldsHostPath, Type: &hostPathDir}, + }, + }, + } + if p.ReaperNode != "" { + pv.Spec.NodeAffinity = &corev1.VolumeNodeAffinity{Required: &corev1.NodeSelector{ + NodeSelectorTerms: []corev1.NodeSelectorTerm{{MatchExpressions: []corev1.NodeSelectorRequirement{{ + Key: "kubernetes.io/hostname", Operator: corev1.NodeSelectorOpIn, Values: []string{p.ReaperNode}, + }}}}, + }} + } + return pv +} + +// worldsRootPVC is the reaper's claim on worldsRootPV, bound by name both ways. +func worldsRootPVC(p Params) *corev1.PersistentVolumeClaim { + p = p.withDefaults() + name := worldsRootName(p) + noClass := "" + return &corev1.PersistentVolumeClaim{ + TypeMeta: metav1.TypeMeta{APIVersion: "v1", Kind: "PersistentVolumeClaim"}, + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: p.MinecraftNamespace, Labels: controlPlanePodLabels(ComponentReaper)}, + Spec: corev1.PersistentVolumeClaimSpec{ + AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, + StorageClassName: &noClass, + VolumeName: name, + Resources: corev1.VolumeResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceStorage: resource.MustParse(worldsRootStorage)}, + }, + }, + } +} + // reaperPodSpec is the reaper Job's pod template. It lives apart from the CronJob // literal only so the optional node pin is one visible branch: with ReaperNode // set the pod carries a kubernetes.io/hostname selector, keeping the reaper on @@ -976,7 +1056,7 @@ func hardenedPodSecurityContext() *corev1.PodSecurityContext { } // reaperPodSecurityContext is the reaper's Pod identity: ROOT, deliberately NOT -// the control-plane's non-root uid. Its HostPath mount IS the live storage root, +// the control-plane's non-root uid. Its worlds mount IS the live storage root, // and the world directories beneath it (and the files inside them) are written // by the game uid (naming.GameUID) — or by root, in a world an older release // wrote — with Paper's mode-0600 saves (level.dat) included, while the storage diff --git a/internal/platform/workloads_test.go b/internal/platform/workloads_test.go index 6c87957..1ff8f02 100644 --- a/internal/platform/workloads_test.go +++ b/internal/platform/workloads_test.go @@ -862,14 +862,13 @@ func TestReaperCronJob_Shape(t *testing.T) { t.Error("config must be mounted read-only") } - // worlds: node hostPath at WorldsHostPath, type Directory, mounted READ-ONLY at - // /worlds — the reaper only reads worlds to tar them (deletion is a PVC API call). + // worlds: the static worlds-root claim (never an inline hostPath, which the + // namespace's PodSecurity baseline refuses), mounted READ-ONLY at /worlds — the + // reaper only reads worlds to tar them (deletion is a PVC API call). wVol := volumeByName(ps.Volumes, worldsVolume) - if wVol == nil || wVol.HostPath == nil || wVol.HostPath.Path != p.WorldsHostPath { - t.Errorf("worlds volume must be hostPath %q, got %+v", p.WorldsHostPath, wVol) - } - if wVol != nil && (wVol.HostPath == nil || wVol.HostPath.Type == nil || *wVol.HostPath.Type != corev1.HostPathDirectory) { - t.Error("worlds hostPath must be type Directory (fail loud if the dir is absent)") + if wVol == nil || wVol.HostPath != nil || wVol.PersistentVolumeClaim == nil || + wVol.PersistentVolumeClaim.ClaimName != worldsRootName(p) || !wVol.PersistentVolumeClaim.ReadOnly { + t.Errorf("worlds volume must be the read-only claim %q, got %+v", worldsRootName(p), wVol) } if m := mountByName(c.VolumeMounts, worldsVolume); m == nil || m.MountPath != worldsMountPath || !m.ReadOnly { t.Errorf("worlds must be mounted read-only at %s, got %+v", worldsMountPath, m) @@ -948,3 +947,70 @@ func containsSeq(seq, sub []string) bool { } return true } + +// TestWorldsRootStaticPV pins the pair that replaced the reaper's inline hostPath: +// the PV names the node directory (type Directory, so a missing root fails loud), +// is kept by Retain, sits outside every StorageClass and is reserved for exactly +// its PVC, which in turn names it back. Both render only alongside the reaper. +func TestWorldsRootStaticPV(t *testing.T) { + p := reaperParams() + var pv *corev1.PersistentVolume + var pvc *corev1.PersistentVolumeClaim + for _, obj := range Workloads(p) { + switch o := obj.(type) { + case *corev1.PersistentVolume: + pv = o + case *corev1.PersistentVolumeClaim: + if o.Name == worldsRootName(p) { + pvc = o + } + } + } + if pv == nil || pvc == nil { + t.Fatalf("reaper-enabled bundle must render the worlds-root PV and PVC (pv=%v pvc=%v)", pv != nil, pvc != nil) + } + hp := pv.Spec.HostPath + if hp == nil || hp.Path != p.WorldsHostPath || hp.Type == nil || *hp.Type != corev1.HostPathDirectory { + t.Errorf("PV hostPath = %+v, want %s type Directory", hp, p.WorldsHostPath) + } + if pv.Spec.PersistentVolumeReclaimPolicy != corev1.PersistentVolumeReclaimRetain { + t.Errorf("PV reclaim policy = %q, want Retain", pv.Spec.PersistentVolumeReclaimPolicy) + } + if pv.Spec.StorageClassName != "" || pvc.Spec.StorageClassName == nil || *pvc.Spec.StorageClassName != "" { + t.Error("PV and PVC must both opt out of every StorageClass") + } + if ref := pv.Spec.ClaimRef; ref == nil || ref.Namespace != p.withDefaults().MinecraftNamespace || ref.Name != pvc.Name { + t.Errorf("PV claimRef = %+v, want %s/%s", ref, p.withDefaults().MinecraftNamespace, pvc.Name) + } + if pvc.Spec.VolumeName != pv.Name || pvc.Namespace != p.withDefaults().MinecraftNamespace { + t.Errorf("PVC %s/%s volumeName = %q, want %q", pvc.Namespace, pvc.Name, pvc.Spec.VolumeName, pv.Name) + } + if pv.Spec.NodeAffinity != nil { + t.Error("no ReaperNode: the PV must carry no node affinity") + } + + // A pinned reaper pins its PV to the same node, and a moved root gets a new name + // because hostPath and node affinity are immutable on a live PV. + pinned := reaperParams() + pinned.ReaperNode = "node-a" + ppv := worldsRootPV(pinned) + terms := ppv.Spec.NodeAffinity + if terms == nil || terms.Required == nil || len(terms.Required.NodeSelectorTerms) != 1 || + terms.Required.NodeSelectorTerms[0].MatchExpressions[0].Values[0] != "node-a" { + t.Errorf("pinned PV node affinity = %+v, want kubernetes.io/hostname in [node-a]", terms) + } + if ppv.Name == pv.Name { + t.Error("a different node pin must render a differently named PV") + } + moved := reaperParams() + moved.WorldsHostPath = "/srv/worlds" + if worldsRootName(moved) == pv.Name { + t.Error("a different worlds-root must render a differently named PV") + } + + for _, obj := range Workloads(testParams()) { + if _, ok := obj.(*corev1.PersistentVolume); ok { + t.Error("without the reaper trio no PV may render") + } + } +}