fix(api): refuse backup/restore before a missing world volume
A server whose world PVC does not exist yet (never started) or no longer exists (the world was already reaped) accepted the backup/restore POST, answered 202, and the Job sat Pending on the missing claim until its deadline with nothing recorded anywhere — a silent no-op from the operator's seat. The live drill on the reaped `resolvecheck` world reproduced exactly that. Both handlers now read the world PVC (Cluster.WorldVolumeExists, over the same naming.WorldPVCName the Jobs mount) and answer a specific 409 no_world_volume with "start it once to create it, then retry". The felis-api Role gains the matching get-only PVC grant — the first live run surfaced the missing RBAC as a 403 behind a 500, so the fix ships with it. Live (auditfix38): resolvecheck -> 409 no_world_volume on both faces; test-one (which has a world) still backs up through the new gate end to end.
This commit is contained in:
8 files changed
+127
-3
No files matched your search
@@ -1451,6 +1451,7 @@ type fakeCluster struct {
|
|||||||
desired map[string]v1alpha1.DesiredState
|
desired map[string]v1alpha1.DesiredState
|
||||||
created map[string]CreateServerInput // name -> the validated input it was created from
|
created map[string]CreateServerInput // name -> the validated input it was created from
|
||||||
patched map[string]ServerSpecPatch // name -> the validated spec patch it received
|
patched map[string]ServerSpecPatch // name -> the validated spec patch it received
|
||||||
|
noWorld map[string]bool // server names modeled WITHOUT a world volume (never started / reaped)
|
||||||
createErr error
|
createErr error
|
||||||
pingErr error
|
pingErr error
|
||||||
}
|
}
|
||||||
@@ -1458,7 +1459,7 @@ type fakeCluster struct {
|
|||||||
func newFakeCluster() *fakeCluster {
|
func newFakeCluster() *fakeCluster {
|
||||||
return &fakeCluster{byName: map[string]*ServerInfo{}, bySub: map[string]*ServerInfo{},
|
return &fakeCluster{byName: map[string]*ServerInfo{}, bySub: map[string]*ServerInfo{},
|
||||||
desired: map[string]v1alpha1.DesiredState{}, created: map[string]CreateServerInput{},
|
desired: map[string]v1alpha1.DesiredState{}, created: map[string]CreateServerInput{},
|
||||||
patched: map[string]ServerSpecPatch{}}
|
patched: map[string]ServerSpecPatch{}, noWorld: map[string]bool{}}
|
||||||
}
|
}
|
||||||
func (c *fakeCluster) GetServer(_ context.Context, n string) (*ServerInfo, error) {
|
func (c *fakeCluster) GetServer(_ context.Context, n string) (*ServerInfo, error) {
|
||||||
if s, ok := c.byName[n]; ok {
|
if s, ok := c.byName[n]; ok {
|
||||||
@@ -1474,6 +1475,13 @@ func (c *fakeCluster) GetBySubdomain(_ context.Context, s string) (*ServerInfo,
|
|||||||
}
|
}
|
||||||
func (c *fakeCluster) ListServers(_ context.Context) ([]ServerInfo, error) { return c.list, nil }
|
func (c *fakeCluster) ListServers(_ context.Context) ([]ServerInfo, error) { return c.list, nil }
|
||||||
func (c *fakeCluster) Ping(_ context.Context) error { return c.pingErr }
|
func (c *fakeCluster) Ping(_ context.Context) error { return c.pingErr }
|
||||||
|
|
||||||
|
// WorldVolumeExists models the world PVC: present unless the test named the
|
||||||
|
// server in noWorld (never started / already reaped).
|
||||||
|
func (c *fakeCluster) WorldVolumeExists(_ context.Context, n string) (bool, error) {
|
||||||
|
return !c.noWorld[n], nil
|
||||||
|
}
|
||||||
|
|
||||||
func (c *fakeCluster) SetDesiredState(_ context.Context, n string, s v1alpha1.DesiredState) error {
|
func (c *fakeCluster) SetDesiredState(_ context.Context, n string, s v1alpha1.DesiredState) error {
|
||||||
c.desired[n] = s
|
c.desired[n] = s
|
||||||
return nil
|
return nil
|
||||||
|
|||||||
@@ -81,6 +81,12 @@ type Cluster interface {
|
|||||||
|
|
||||||
// GetServer reads one MinecraftServer's lifecycle view, or ErrNotFound.
|
// GetServer reads one MinecraftServer's lifecycle view, or ErrNotFound.
|
||||||
GetServer(ctx context.Context, name string) (*ServerInfo, error)
|
GetServer(ctx context.Context, name string) (*ServerInfo, error)
|
||||||
|
// WorldVolumeExists reports whether the server's world PVC exists in the
|
||||||
|
// server namespace. A server that never started — or whose world the
|
||||||
|
// retention reaper already archived and deleted — has no claim, and a
|
||||||
|
// backup/restore Job would hang Pending on the missing volume with nothing
|
||||||
|
// ever recorded, so both handlers refuse those up front.
|
||||||
|
WorldVolumeExists(ctx context.Context, name string) (bool, error)
|
||||||
// GetBySubdomain finds the MinecraftServer whose spec.subdomain matches, or
|
// GetBySubdomain finds the MinecraftServer whose spec.subdomain matches, or
|
||||||
// ErrNotFound.
|
// ErrNotFound.
|
||||||
GetBySubdomain(ctx context.Context, subdomain string) (*ServerInfo, error)
|
GetBySubdomain(ctx context.Context, subdomain string) (*ServerInfo, error)
|
||||||
|
|||||||
@@ -134,6 +134,22 @@ func TestBackupNow(t *testing.T) {
|
|||||||
}
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
|
t.Run("no world volume -> 409 no_world_volume, no backup", func(t *testing.T) {
|
||||||
|
// A never-started (or reaped) server has no world PVC: the Job would hang
|
||||||
|
// Pending on the missing claim with nothing recorded, so the gate must
|
||||||
|
// refuse before the backuper is reached.
|
||||||
|
api, _, cl, backuper := mk()
|
||||||
|
cl.noWorld["survival"] = true
|
||||||
|
api.External = staticExternal{p: owner}
|
||||||
|
w := do(api.ExternalHandler(), "POST", path, "", nil)
|
||||||
|
if w.Code != http.StatusConflict || decodeErr(t, w) != "no_world_volume" {
|
||||||
|
t.Fatalf("code = %d body %s", w.Code, w.Body.String())
|
||||||
|
}
|
||||||
|
if backuper.calls != 0 {
|
||||||
|
t.Fatal("a world-less server must not reach the backuper")
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
t.Run("nil Backuper -> 503 backup_unavailable", func(t *testing.T) {
|
t.Run("nil Backuper -> 503 backup_unavailable", func(t *testing.T) {
|
||||||
api, _, _, _ := mk()
|
api, _, _, _ := mk()
|
||||||
api.Backuper = nil
|
api.Backuper = nil
|
||||||
@@ -240,6 +256,20 @@ func TestInternalBackup(t *testing.T) {
|
|||||||
}
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
|
t.Run("no world volume -> 409 no_world_volume, no backup", func(t *testing.T) {
|
||||||
|
// The break-glass face shares enqueueBackup, so the world-volume gate must
|
||||||
|
// hold here too — this is the face the TUI's Sync picker drives.
|
||||||
|
api, _, cl, backuper := mk()
|
||||||
|
cl.noWorld["survival"] = true
|
||||||
|
w := do(api.InternalHandler(), "POST", path, "", jsonHeader)
|
||||||
|
if w.Code != http.StatusConflict || decodeErr(t, w) != "no_world_volume" {
|
||||||
|
t.Fatalf("code = %d body %s", w.Code, w.Body.String())
|
||||||
|
}
|
||||||
|
if backuper.calls != 0 {
|
||||||
|
t.Fatal("a world-less server must not reach the backuper")
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
t.Run("nil Backuper -> 503 backup_unavailable", func(t *testing.T) {
|
t.Run("nil Backuper -> 503 backup_unavailable", func(t *testing.T) {
|
||||||
api, _, _, _ := mk()
|
api, _, _, _ := mk()
|
||||||
api.Backuper = nil
|
api.Backuper = nil
|
||||||
|
|||||||
@@ -9,6 +9,16 @@ import (
|
|||||||
"felis.lolicon.best/internal/naming"
|
"felis.lolicon.best/internal/naming"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
// errNoWorldVolume is the shared 409 for backup and restore when the server's
|
||||||
|
// world PVC does not exist: the Job would only hang Pending on the missing
|
||||||
|
// claim — invisible to the caller and to the backups list — so the handlers
|
||||||
|
// refuse up front. Starting the server once (which creates the claim via the
|
||||||
|
// StatefulSet volumeClaimTemplate) unlocks both ops.
|
||||||
|
func errNoWorldVolume() error {
|
||||||
|
return newError(http.StatusConflict, "no_world_volume",
|
||||||
|
"this server has no world volume yet — start it once to create it, then retry")
|
||||||
|
}
|
||||||
|
|
||||||
// handleListBackups lists the world backups visible to the caller (spec §7 GET
|
// handleListBackups lists the world backups visible to the caller (spec §7 GET
|
||||||
// /api/v1/backups; world_backups in §22). It is app-tier: an admin sees every
|
// /api/v1/backups; world_backups in §22). It is app-tier: an admin sees every
|
||||||
// present backup; a regular user sees only the backups of worlds they formerly
|
// present backup; a regular user sees only the backups of worlds they formerly
|
||||||
@@ -154,6 +164,19 @@ func (a *API) handleRestoreBackup(w http.ResponseWriter, r *http.Request) {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// World-volume gate: the restore Job mounts the world PVC read-write to unpack
|
||||||
|
// the archive into it, so a missing claim means a Pod stuck Pending — a 202
|
||||||
|
// "restoring" with nothing ever written. Same refusal as the backup face
|
||||||
|
// (shared errNoWorldVolume): the operator starts the server once to create the
|
||||||
|
// claim, then restores into it.
|
||||||
|
if exists, err := a.Cluster.WorldVolumeExists(r.Context(), name); err != nil {
|
||||||
|
writeError(w, r, err)
|
||||||
|
return
|
||||||
|
} else if !exists {
|
||||||
|
writeError(w, r, errNoWorldVolume())
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
// Restorer is optional: when unwired the endpoint reports 503 rather than
|
// Restorer is optional: when unwired the endpoint reports 503 rather than
|
||||||
// panicking, so the authorization boundary above is exercised even before the
|
// panicking, so the authorization boundary above is exercised even before the
|
||||||
// restore-Job executor is wired (see Restorer).
|
// restore-Job executor is wired (see Restorer).
|
||||||
@@ -285,6 +308,18 @@ func (a *API) enqueueBackup(w http.ResponseWriter, r *http.Request, name string,
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// World-volume gate: the Job mounts the world PVC by claim name, and a missing
|
||||||
|
// claim would leave its Pod Pending — a 202 "backing_up" with nothing ever
|
||||||
|
// recorded anywhere. A never-started or already-reaped server is refused with
|
||||||
|
// the same specificity as the stopped gate.
|
||||||
|
if exists, err := a.Cluster.WorldVolumeExists(r.Context(), name); err != nil {
|
||||||
|
writeError(w, r, err)
|
||||||
|
return
|
||||||
|
} else if !exists {
|
||||||
|
writeError(w, r, errNoWorldVolume())
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
// Backuper is optional: when unwired the endpoint reports 503 rather than
|
// Backuper is optional: when unwired the endpoint reports 503 rather than
|
||||||
// panicking, so the authorization boundary above is exercised even before the
|
// panicking, so the authorization boundary above is exercised even before the
|
||||||
// backup-Job executor is wired (see Backuper).
|
// backup-Job executor is wired (see Backuper).
|
||||||
|
|||||||
@@ -269,6 +269,21 @@ func TestRestoreBackup(t *testing.T) {
|
|||||||
}
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
|
t.Run("no world volume -> 409 no_world_volume, no restore", func(t *testing.T) {
|
||||||
|
// Restoring into a missing world PVC would leave the Job Pending on the
|
||||||
|
// missing claim — a 202 "restoring" that never writes anything.
|
||||||
|
api, _, cl, restorer := mk()
|
||||||
|
cl.noWorld["survival"] = true
|
||||||
|
api.External = staticExternal{p: owner}
|
||||||
|
w := do(api.ExternalHandler(), "POST", path, "", nil)
|
||||||
|
if w.Code != http.StatusConflict || decodeErr(t, w) != "no_world_volume" {
|
||||||
|
t.Fatalf("code = %d body %s", w.Code, w.Body.String())
|
||||||
|
}
|
||||||
|
if restorer.calls != 0 {
|
||||||
|
t.Fatal("a world-less server must not reach the restorer")
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
t.Run("nil Restorer -> 503 restore_unavailable", func(t *testing.T) {
|
t.Run("nil Restorer -> 503 restore_unavailable", func(t *testing.T) {
|
||||||
api, _, _, _ := mk()
|
api, _, _, _ := mk()
|
||||||
api.Restorer = nil
|
api.Restorer = nil
|
||||||
|
|||||||
@@ -43,6 +43,22 @@ func (k *K8sCluster) GetServer(ctx context.Context, name string) (*ServerInfo, e
|
|||||||
return serverInfo(&ms), nil
|
return serverInfo(&ms), nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// WorldVolumeExists reads the world PVC the operator's StatefulSet
|
||||||
|
// volumeClaimTemplate creates (naming.WorldPVCName — the same name the backup
|
||||||
|
// and restore Jobs mount), so existence here is exactly existence at Job mount
|
||||||
|
// time. NotFound is (false, nil): the caller refuses with a specific 409.
|
||||||
|
func (k *K8sCluster) WorldVolumeExists(ctx context.Context, name string) (bool, error) {
|
||||||
|
var pvc corev1.PersistentVolumeClaim
|
||||||
|
err := k.c.Get(ctx, types.NamespacedName{Namespace: k.namespace, Name: naming.WorldPVCName(name)}, &pvc)
|
||||||
|
if apierrors.IsNotFound(err) {
|
||||||
|
return false, nil
|
||||||
|
}
|
||||||
|
if err != nil {
|
||||||
|
return false, err
|
||||||
|
}
|
||||||
|
return true, nil
|
||||||
|
}
|
||||||
|
|
||||||
func (k *K8sCluster) GetBySubdomain(ctx context.Context, subdomain string) (*ServerInfo, error) {
|
func (k *K8sCluster) GetBySubdomain(ctx context.Context, subdomain string) (*ServerInfo, error) {
|
||||||
var list v1alpha1.MinecraftServerList
|
var list v1alpha1.MinecraftServerList
|
||||||
if err := k.c.List(ctx, &list, client.InNamespace(k.namespace)); err != nil {
|
if err := k.c.List(ctx, &list, client.InNamespace(k.namespace)); err != nil {
|
||||||
|
|||||||
@@ -84,8 +84,11 @@ func ControlPlaneRBAC(p Params) RBAC {
|
|||||||
// FINISHED Job whose name still blocks a retry can be replaced), and stream the
|
// FINISHED Job whose name still blocks a retry can be replaced), and stream the
|
||||||
// live console for the read side (internal/api.logstream — pods:list to find
|
// live console for the read side (internal/api.logstream — pods:list to find
|
||||||
// the server's running pod, then pods/log:get to follow it; spec §8 读=pods/log
|
// the server's running pod, then pods/log:get to follow it; spec §8 读=pods/log
|
||||||
// follow). felis-api uses a DIRECT client, so it needs no list/watch beyond the
|
// follow). It also Gets the world PVC before backup/restore
|
||||||
// explicit List calls.
|
// (internal/api.k8scluster.WorldVolumeExists) so a never-started or reaped
|
||||||
|
// world is refused up front instead of leaving a Job Pending on a missing
|
||||||
|
// claim. felis-api uses a DIRECT client, so it needs no list/watch beyond the
|
||||||
|
// explicit List calls — and the PVC grant is get-only, mirroring that.
|
||||||
//
|
//
|
||||||
// The read-side grant is deliberately minimal: pods:list + pods/log:get, NOT
|
// The read-side grant is deliberately minimal: pods:list + pods/log:get, NOT
|
||||||
// pods:get — the streamer lists pods by the server label then reads the chosen
|
// pods:get — the streamer lists pods by the server label then reads the chosen
|
||||||
@@ -97,6 +100,9 @@ func APIMinecraftRole(p Params) *rbacv1.Role {
|
|||||||
return role(p.MinecraftNamespace, "felis-api", ComponentAPI, []rbacv1.PolicyRule{
|
return role(p.MinecraftNamespace, "felis-api", ComponentAPI, []rbacv1.PolicyRule{
|
||||||
rule([]string{groupFelis}, []string{"minecraftservers"}, []string{"get", "list", "create", "patch"}),
|
rule([]string{groupFelis}, []string{"minecraftservers"}, []string{"get", "list", "create", "patch"}),
|
||||||
rule([]string{groupCore}, []string{"secrets"}, []string{"get"}),
|
rule([]string{groupCore}, []string{"secrets"}, []string{"get"}),
|
||||||
|
// get-only: WorldVolumeExists does a single direct Get of the world PVC;
|
||||||
|
// nothing in felis-api lists or deletes PVCs.
|
||||||
|
rule([]string{groupCore}, []string{"persistentvolumeclaims"}, []string{"get"}),
|
||||||
// list backs GET /servers/{name}/jobs — the async status outlet reads the
|
// list backs GET /servers/{name}/jobs — the async status outlet reads the
|
||||||
// backup/restore Jobs back by the server label (read-only).
|
// backup/restore Jobs back by the server label (read-only).
|
||||||
rule([]string{groupBatch}, []string{"jobs"}, []string{"create", "get", "delete", "list"}),
|
rule([]string{groupBatch}, []string{"jobs"}, []string{"create", "get", "delete", "list"}),
|
||||||
|
|||||||
@@ -121,6 +121,14 @@ func TestAPIRole_MinecraftPowersExact(t *testing.T) {
|
|||||||
if !hasRule(mc, groupCore, "secrets", "get") {
|
if !hasRule(mc, groupCore, "secrets", "get") {
|
||||||
t.Error("felis-api must read RCON secrets (secrets:get) for console writes")
|
t.Error("felis-api must read RCON secrets (secrets:get) for console writes")
|
||||||
}
|
}
|
||||||
|
// WorldVolumeExists (backup/restore pre-gate) does a single direct PVC Get;
|
||||||
|
// nothing in felis-api lists or deletes claims.
|
||||||
|
if !hasRule(mc, groupCore, "persistentvolumeclaims", "get") {
|
||||||
|
t.Error("felis-api must get the world PVC (persistentvolumeclaims:get) for the backup/restore world-volume gate")
|
||||||
|
}
|
||||||
|
if hasRule(mc, groupCore, "persistentvolumeclaims", "list") || hasRule(mc, groupCore, "persistentvolumeclaims", "delete") {
|
||||||
|
t.Error("felis-api must NOT list or delete PVCs (the gate is a single direct Get)")
|
||||||
|
}
|
||||||
// Read-side console (spec §8 读=pods/log follow): list pods to find the
|
// Read-side console (spec §8 读=pods/log follow): list pods to find the
|
||||||
// running pod, then read its log subresource — and nothing wider.
|
// running pod, then read its log subresource — and nothing wider.
|
||||||
if !hasRule(mc, groupCore, "pods", "list") {
|
if !hasRule(mc, groupCore, "pods", "list") {
|
||||||
|
|||||||
Reference in new issue
Block a user