From 328e5703092eace45a338518978407df20ce8a84 Mon Sep 17 00:00:00 2001 From: Lemon-miaow Date: Wed, 23 Sep 2026 20:07:20 +0800 Subject: [PATCH] fix(setup,install): refresh the workload namespace's felis-config mirror (#51) --- cmd/felis/setup.go | 16 +++++-- cmd/felis/systemservers.go | 44 ++++++++++++++++- cmd/felis/systemservers_test.go | 83 ++++++++++++++++++++++++++++++++- deploy/bootstrap.sh | 19 ++++++-- deploy/bootstrap_test.sh | 35 ++++++++++++++ 5 files changed, 187 insertions(+), 10 deletions(-) diff --git a/cmd/felis/setup.go b/cmd/felis/setup.go index 1d88c2c..085f0fe 100644 --- a/cmd/felis/setup.go +++ b/cmd/felis/setup.go @@ -241,24 +241,28 @@ func provisionSystemServers(ctx context.Context, cfg *config.Config, out io.Writ } secretOutcomes := []systemServerOutcome{ ensureSecretReplica(ctx, cl, controlNS, cfg.K8s.Namespace, - naming.ServiceTokenSecretName, naming.ServiceTokenSecretKey, "service-token", "minecraft ns"), + naming.ServiceTokenSecretName, naming.ServiceTokenSecretKey, "service-token", "minecraft ns", false), ensureSecretReplica(ctx, cl, controlNS, cfg.K8s.Namespace, - naming.ForwardingSecretName, naming.ForwardingSecretKey, "forwarding-secret", "minecraft ns"), + naming.ForwardingSecretName, naming.ForwardingSecretKey, "forwarding-secret", "minecraft ns", false), + // refresh=true: felis-config is the rendered config, not a credential. The + // backup/restore/fileedit Jobs and the reaper mount this copy, so a re-run + // must update it when the control plane's render has moved on (a stale copy + // e.g. keeps an old database URL after a credential rotation). ensureSecretReplica(ctx, cl, controlNS, cfg.K8s.Namespace, - "felis-config", "felis.toml", "config", "minecraft ns"), + "felis-config", "felis.toml", "config", "minecraft ns", true), // The reaper's pre-reap warning emails authenticate with the same relay // password felis-api uses; the reaper pod runs in the minecraft namespace, // where a secretKeyRef resolves only against a local mirror. Skipped while // the relay is not configured yet — the "configure email" screen refreshes // both mirrors when it applies. ensureSecretReplica(ctx, cl, controlNS, cfg.K8s.Namespace, - "felis-smtp", "password", "smtp", "minecraft ns"), + "felis-smtp", "password", "smtp", "minecraft ns", false), // The build namespace needs the same token: the build Job's fetch // initContainer reads the submission context from the internal face. Best // effort — a deployment that only installs the control plane simply never // builds a user submission. ensureSecretReplica(ctx, cl, controlNS, buildNS, - naming.ServiceTokenSecretName, naming.ServiceTokenSecretKey, "service-token", "felis-build ns"), + naming.ServiceTokenSecretName, naming.ServiceTokenSecretKey, "service-token", "felis-build ns", false), } outcomes := ensureSystemServers(ctx, cl, cfg.K8s.Namespace, cfg.Velocity.LoginImage, cfg.Velocity.LobbyImage, apiBaseURL, cfg.Server.RootDomain, defaultPanelHostname(cfg.Server.RootDomain, cfg.Auth.PanelHostname)) outcomes = append(secretOutcomes, outcomes...) @@ -269,6 +273,8 @@ func provisionSystemServers(ctx context.Context, cfg *config.Config, out io.Writ fmt.Fprintf(out, " - %s: ERROR %v\n", o.name, o.err) case o.created: fmt.Fprintf(out, " - %s: created (DesiredState=Running)\n", o.name) + case o.updated: + fmt.Fprintf(out, " - %s: refreshed from the control namespace\n", o.name) default: fmt.Fprintf(out, " - %s: skipped (%s)\n", o.name, o.skipped) } diff --git a/cmd/felis/systemservers.go b/cmd/felis/systemservers.go index 97c4a47..ebd80be 100644 --- a/cmd/felis/systemservers.go +++ b/cmd/felis/systemservers.go @@ -1,6 +1,7 @@ package main import ( + "bytes" "context" "fmt" "time" @@ -256,6 +257,7 @@ func buildSystemServerClient() (client.Client, error) { type systemServerOutcome struct { name string created bool // true = we created it this run + updated bool // true = we refreshed an existing replica from the source available bool // true = the required object now exists skipped string // non-empty = why it was skipped (image unset / already exists) err error // non-nil = create failed @@ -491,7 +493,14 @@ func phaseOrPending(p v1alpha1.Phase) string { // a create failure degrades to a reported outcome, never a hard setup failure. It // copies only Type and Data — never labels/annotations/ownerRefs — so the replica // carries no accidental GC owner or managed-by lineage. -func ensureSecretReplica(ctx context.Context, cl client.Client, controlNamespace, minecraftNamespace, secretName, secretKey, label, where string) systemServerOutcome { +// +// refreshExisting switches the felis-config mirror to refresh-in-place: that Secret is +// a rendered config, never a hand-rotated credential, and the workload Jobs that mount +// it (backup/restore/fileedit) plus the reaper silently misbehave on a stale copy — +// e.g. after a database credential rotation the control plane moves on while every +// backup Job keeps failing auth. Credential Secrets keep the never-overwrite rule so a +// rotated value survives; to rotate those, delete the replica and re-run setup. +func ensureSecretReplica(ctx context.Context, cl client.Client, controlNamespace, minecraftNamespace, secretName, secretKey, label, where string, refreshExisting bool) systemServerOutcome { name := label + " (" + where + ")" validate := func(secret *corev1.Secret, location, skipped string) systemServerOutcome { if len(secret.Data[secretKey]) == 0 { @@ -500,6 +509,33 @@ func ensureSecretReplica(ctx context.Context, cl client.Client, controlNamespace } return systemServerOutcome{name: name, available: true, skipped: skipped} } + // refreshFromControl updates an existing replica from the control-namespace source + // when the rendered key differs. Only the felis-config mirror opts in. + refreshFromControl := func(existing *corev1.Secret) systemServerOutcome { + var src corev1.Secret + if err := cl.Get(ctx, client.ObjectKey{Namespace: controlNamespace, Name: secretName}, &src); err != nil { + if apierrors.IsNotFound(err) { + return systemServerOutcome{name: name, skipped: fmt.Sprintf( + "source Secret %s/%s not found — provision it (deploy/bootstrap.sh), then re-run setup", + controlNamespace, secretName)} + } + return systemServerOutcome{name: name, err: err} + } + if out := validate(&src, controlNamespace, ""); !out.available { + return out + } + if bytes.Equal(existing.Data[secretKey], src.Data[secretKey]) { + return validate(existing, minecraftNamespace, "already current") + } + if existing.Data == nil { + existing.Data = map[string][]byte{} + } + existing.Data[secretKey] = src.Data[secretKey] + if err := cl.Update(ctx, existing); err != nil { + return systemServerOutcome{name: name, err: err} + } + return systemServerOutcome{name: name, updated: true, available: true} + } if controlNamespace == minecraftNamespace { // Same namespace needs no replica, but the source still has to exist. var existing corev1.Secret @@ -518,6 +554,9 @@ func ensureSecretReplica(ctx context.Context, cl client.Client, controlNamespace var existing corev1.Secret getErr := cl.Get(ctx, client.ObjectKey{Namespace: minecraftNamespace, Name: secretName}, &existing) if getErr == nil { + if refreshExisting { + return refreshFromControl(&existing) + } return validate(&existing, minecraftNamespace, "already exists") } if !apierrors.IsNotFound(getErr) { @@ -546,6 +585,9 @@ func ensureSecretReplica(ctx context.Context, cl client.Client, controlNamespace if getErr := cl.Get(ctx, client.ObjectKey{Namespace: minecraftNamespace, Name: secretName}, &existing); getErr != nil { return systemServerOutcome{name: name, err: getErr} } + if refreshExisting { + return refreshFromControl(&existing) + } return validate(&existing, minecraftNamespace, "already exists") } return systemServerOutcome{name: name, err: err} diff --git a/cmd/felis/systemservers_test.go b/cmd/felis/systemservers_test.go index 62a0988..a52096b 100644 --- a/cmd/felis/systemservers_test.go +++ b/cmd/felis/systemservers_test.go @@ -156,7 +156,7 @@ func TestEnsureSecretReplica(t *testing.T) { } replicate := func(cl client.Client, controlNS, mcNS string) systemServerOutcome { return ensureSecretReplica(ctx, cl, controlNS, mcNS, - naming.ServiceTokenSecretName, naming.ServiceTokenSecretKey, "service-token", "minecraft ns") + naming.ServiceTokenSecretName, naming.ServiceTokenSecretKey, "service-token", "minecraft ns", false) } t.Run("replicates when absent", func(t *testing.T) { @@ -251,6 +251,87 @@ func TestEnsureSecretReplica(t *testing.T) { }) } +// The felis-config mirror is the one replica that must refresh: it is a rendered +// config, and a stale workload-side copy (backup/restore/fileedit Jobs, the reaper) +// misbehaves silently — a rotated database credential keeps the control plane moving +// while every backup Job keeps failing auth. Credential Secrets keep create-if-absent. +func TestEnsureSecretReplicaRefresh(t *testing.T) { + scheme := newSystemServerScheme(t) + ctx := context.Background() + configSecret := func(ns, body string) *corev1.Secret { + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "felis-config", Namespace: ns}, + Type: corev1.SecretTypeOpaque, + Data: map[string][]byte{"felis.toml": []byte(body)}, + } + } + refresh := func(cl client.Client) systemServerOutcome { + return ensureSecretReplica(ctx, cl, "felis", "minecraft", + "felis-config", "felis.toml", "config", "minecraft ns", true) + } + replicaBody := func(t *testing.T, cl client.Client) string { + t.Helper() + var got corev1.Secret + if err := cl.Get(ctx, client.ObjectKey{Namespace: "minecraft", Name: "felis-config"}, &got); err != nil { + t.Fatalf("get replica: %v", err) + } + return string(got.Data["felis.toml"]) + } + + t.Run("refreshes a stale config replica", func(t *testing.T) { + cl := fake.NewClientBuilder().WithScheme(scheme).WithObjects( + configSecret("felis", "current"), + configSecret("minecraft", "stale"), + ).Build() + out := refresh(cl) + if out.err != nil || !out.updated || !out.available { + t.Fatalf("outcome = %+v, want refreshed", out) + } + if got := replicaBody(t, cl); got != "current" { + t.Errorf("replica = %q, want current", got) + } + }) + + t.Run("leaves a current config replica alone", func(t *testing.T) { + cl := fake.NewClientBuilder().WithScheme(scheme).WithObjects( + configSecret("felis", "same"), + configSecret("minecraft", "same"), + ).Build() + out := refresh(cl) + if out.err != nil || out.updated || !out.available || out.skipped != "already current" { + t.Fatalf("outcome = %+v, want already current", out) + } + }) + + t.Run("fills an empty-key replica", func(t *testing.T) { + empty := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "felis-config", Namespace: "minecraft"}, + Data: map[string][]byte{}, + } + cl := fake.NewClientBuilder().WithScheme(scheme).WithObjects( + configSecret("felis", "current"), empty).Build() + out := refresh(cl) + if out.err != nil || !out.updated { + t.Fatalf("outcome = %+v, want refreshed", out) + } + if got := replicaBody(t, cl); got != "current" { + t.Errorf("replica = %q, want current", got) + } + }) + + t.Run("missing source degrades to a skip", func(t *testing.T) { + cl := fake.NewClientBuilder().WithScheme(scheme).WithObjects( + configSecret("minecraft", "stale")).Build() + out := refresh(cl) + if out.err != nil || out.updated || out.available || out.skipped == "" { + t.Fatalf("outcome = %+v, want skipped (source missing)", out) + } + if got := replicaBody(t, cl); got != "stale" { + t.Errorf("replica = %q, want untouched stale", got) + } + }) +} + func TestRequiredProvisioningError(t *testing.T) { ready := []systemServerOutcome{ {name: "service-token (minecraft ns)", available: true}, diff --git a/deploy/bootstrap.sh b/deploy/bootstrap.sh index 15c1be4..75cbe6e 100644 --- a/deploy/bootstrap.sh +++ b/deploy/bootstrap.sh @@ -344,6 +344,21 @@ apply_literal_secret() { rm -f "$tmp" } +# The control plane mounts felis-config from its own namespace; the workload +# namespace's backup/restore/fileedit Jobs and the reaper mount a local copy (a +# secretKeyRef is namespace-local). The installer owns the rendered config, so both +# copies are (re)applied on every run — unlike the create-if-absent credential +# replicas `felis setup` makes, because a stale config copy keeps an old database URL +# or archive policy after an upgrade or a credential rotation. +apply_felis_config_secrets() { + kube -n "$CONTROL_NS" create secret generic felis-config \ + --from-file=felis.toml="${STATE_DIR}/felis.pod.toml" \ + --dry-run=client -o yaml | kube apply -f - + kube -n "$MINECRAFT_NS" create secret generic felis-config \ + --from-file=felis.toml="${STATE_DIR}/felis.pod.toml" \ + --dry-run=client -o yaml | kube apply -f - +} + as_postgres() { if command -v runuser >/dev/null 2>&1; then runuser -u postgres -- "$@" @@ -2304,9 +2319,7 @@ deploy_bundle() { done log "provisioning felis-config + felis-service-token + felis-forwarding-secret + panel TLS secrets (out-of-band, never in the bundle)" - kube -n "$CONTROL_NS" create secret generic felis-config \ - --from-file=felis.toml="${STATE_DIR}/felis.pod.toml" \ - --dry-run=client -o yaml | kube apply -f - + apply_felis_config_secrets apply_literal_secret "$CONTROL_NS" felis-service-token token "$SERVICE_TOKEN" # The build namespace needs the same token: the build Job's fetch initContainer # streams a submission's build context from the felis-api internal face, and a diff --git a/deploy/bootstrap_test.sh b/deploy/bootstrap_test.sh index b2157e4..119f60e 100644 --- a/deploy/bootstrap_test.sh +++ b/deploy/bootstrap_test.sh @@ -820,6 +820,41 @@ case "$out" in *DOCKER*) echo "FAIL: a present registry:2 must not trigger a docker pull"; fails=$((fails + 1)) ;; esac +# --- installer re-runs refresh the workload namespace's felis-config copy --------------- +# The backup/restore/fileedit Jobs and the reaper mount the workload namespace's own +# felis-config (a secretKeyRef is namespace-local). `felis setup` makes that replica +# create-if-absent -- right for credentials, wrong for a rendered config -- so the +# installer must refresh it every run; a stale copy keeps old DB/archive settings. + +fcblock="$(awk '/^apply_felis_config_secrets\(\) \{/,/^}/' "$BS")" +[ -n "$fcblock" ] || { echo "FAIL: no apply_felis_config_secrets found in $BS"; exit 1; } +[ "$(printf '%s\n' "$fcblock" | wc -l)" -lt 20 ] \ + || { echo "FAIL: the extracted block is not the function -- did its closing brace move?"; exit 1; } + +kubcalls="$(mktemp)" +run_fc() { + : > "$kubcalls" + CONTROL_NS=felis MINECRAFT_NS=minecraft STATE_DIR=/tmp/fc KUBCALLS="$kubcalls" bash -c ' + kube() { printf "%s\n" "$*" >> "$KUBCALLS"; } + '"$fcblock"' + apply_felis_config_secrets' + cat "$kubcalls" +} +out="$(run_fc)" +expect "the control plane's felis-config is applied" \ + "-n felis create secret generic felis-config" "$out" +expect "the workload namespace's copy is applied too" \ + "-n minecraft create secret generic felis-config" "$out" +expect "both copies render from the pod config" \ + "felis.toml=/tmp/fc/felis.pod.toml" "$out" +applies="$(printf '%s\n' "$out" | grep -c '^apply -f -$')" +if [ "$applies" -eq 2 ]; then + echo "PASS both rendered copies are piped to kubectl apply" +else + echo "FAIL: expected 2 applies, got $applies:"; printf '%s\n' "$out"; fails=$((fails + 1)) +fi +rm -f "$kubcalls" + # --- installer re-runs keep the operator's [registry] overrides -------------------------- # §15's upgrade path is re-running the installer, but the build-lane mirrors and the # uploads backend live in [registry] as hand-written keys (docs/troubleshooting.md §8e or