diff --git a/cmd/felis/systemservers.go b/cmd/felis/systemservers.go index 276e63e..b001135 100644 --- a/cmd/felis/systemservers.go +++ b/cmd/felis/systemservers.go @@ -48,6 +48,13 @@ type systemServerSpec struct { storage string // world PVC size fallbackServer string // "" = none (refuse when down); never the lobby healthHTTPPort int32 // > 0 → gate readiness on an HTTP health endpoint + // rcon opts a system service into the RCON write channel. It is per-service and + // NOT a default, because enabling it on a backend that runs no RCON listener is + // actively destructive rather than merely useless: the operator gates readiness + // on the probe, so the server would never leave Starting and would eventually be + // marked Failed. The login limbo is exactly that case (LOOHP/Limbo has no RCON), + // and it is the front door — taking it down locks everyone out. + rcon bool // env are extra plain (non-secret) environment variables baked into the pod. // System-service configuration derived from the deployment (the internal API // URL, root domain, lobby name) rides here; secrets never do — the service @@ -55,6 +62,24 @@ type systemServerSpec struct { env []v1alpha1.EnvVar } +// systemRcon renders the RCON block for a system service. The secret name comes +// from naming.RconSecretName — the same convention felis-api writes for user +// servers and the operator provisions against — so a system service is not a +// second, parallel way of doing this. Port is left 0 so the operator's default is +// the only place the number lives. +func systemRcon(in systemServerSpec) v1alpha1.RconSpec { + if !in.rcon { + return v1alpha1.RconSpec{Enabled: false} + } + return v1alpha1.RconSpec{ + Enabled: true, + SecretRef: v1alpha1.SecretKeyRef{ + Name: naming.RconSecretName(in.name), + Key: naming.RconSecretKey, + }, + } +} + // felisLimboHealthPort is the port the felis-limbo readiness plugin serves its // HTTP health endpoint on. The login system service gates pod readiness on it so // "the limbo has finished starting" — not merely "the game socket is bound" — @@ -82,8 +107,10 @@ const ( // - permits reserved names (login/lobby) via ValidateSystemServerName, // - sets DesiredState=Running (the service is up the moment it exists), // - sets ReaperExempt=true and AutostartPolicy=public, -// - leaves RCON disabled (LOOHP/Limbo has none; readiness is gated on pod -// TCP/HTTP health, not an RCON probe — see the operator reconciler). +// - enables RCON only where the image actually serves it (in.rcon): the lobby +// is Paper and needs the write channel like any user server, while the login +// limbo has no RCON listener at all and gates readiness on pod TCP/HTTP +// health instead — see the operator reconciler. func buildSystemServer(in systemServerSpec, namespace string) (*v1alpha1.MinecraftServer, error) { if err := naming.ValidateSystemServerName(in.name); err != nil { return nil, fmt.Errorf("invalid name: %w", err) @@ -141,7 +168,7 @@ func buildSystemServer(in systemServerSpec, namespace string) (*v1alpha1.Minecra // forwarding), backends run offline-mode; the proxy is the one place // online-mode is true (spec §8, §11). OnlineMode: false, - Rcon: v1alpha1.RconSpec{Enabled: false}, + Rcon: systemRcon(in), Storage: v1alpha1.StorageSpec{Size: storageQ.String()}, Resources: corev1.ResourceRequirements{Limits: limits, Requests: requests}, Startup: v1alpha1.StartupSpec{HealthHTTPPort: in.healthHTTPPort}, @@ -192,6 +219,9 @@ func lobbySystemServer(image, namespace string) (*v1alpha1.MinecraftServer, erro memory: "1Gi", storage: "2Gi", fallbackServer: naming.SystemLoginServer, + // Paper serves RCON, and the lobby is administered through the panel like any + // other server — online players, console, permissions all ride this channel. + rcon: true, }, namespace) } diff --git a/cmd/felis/systemservers_test.go b/cmd/felis/systemservers_test.go index 1c23584..b053b25 100644 --- a/cmd/felis/systemservers_test.go +++ b/cmd/felis/systemservers_test.go @@ -439,3 +439,40 @@ func TestEnsureSystemServersSkipsUnsetImage(t *testing.T) { t.Errorf("lobby: skipped = %q, want %q", byName[naming.SystemLobbyServer].skipped, "image not configured") } } + +// TestSystemServerRconPolicy pins which system service gets the RCON write +// channel. This is not a preference: the operator gates readiness on the RCON +// probe, so enabling it on a backend that serves no RCON listener would hold that +// server in Starting until it was marked Failed. The login limbo is exactly that +// backend (LOOHP/Limbo has no RCON) AND it is the front door, so getting this +// backwards locks every player out of the deployment. +func TestSystemServerRconPolicy(t *testing.T) { + login, err := loginSystemServer("reg/limbo:1", "minecraft", "http://api:8081", "mc.example.net", "console.mc.example.net") + if err != nil { + t.Fatalf("loginSystemServer: %v", err) + } + if login.Spec.Rcon.Enabled { + t.Fatal("the login limbo must not enable RCON: it serves no RCON listener, so the " + + "operator's readiness probe would never succeed and the login gate would be marked Failed") + } + + lobby, err := lobbySystemServer("reg/lobby:1", "minecraft") + if err != nil { + t.Fatalf("lobbySystemServer: %v", err) + } + if !lobby.Spec.Rcon.Enabled { + t.Fatal("the lobby runs Paper and is administered through the panel; without RCON its " + + "console, online-player list and permission changes are all unavailable") + } + if got, want := lobby.Spec.Rcon.SecretRef.Name, naming.RconSecretName("lobby"); got != want { + t.Fatalf("lobby rcon secret = %q, want %q — the operator provisions against this name", got, want) + } + if got, want := lobby.Spec.Rcon.SecretRef.Key, naming.RconSecretKey; got != want { + t.Fatalf("lobby rcon secret key = %q, want %q", got, want) + } + // Port stays unset so the operator's DefaultRconPort is the only place the + // number is written down. + if lobby.Spec.Rcon.Port != 0 { + t.Fatalf("lobby rcon port = %d, want 0 (operator default)", lobby.Spec.Rcon.Port) + } +} diff --git a/deploy/lobby/entrypoint.sh b/deploy/lobby/entrypoint.sh index acd3f1b..0641ae4 100644 --- a/deploy/lobby/entrypoint.sh +++ b/deploy/lobby/entrypoint.sh @@ -58,6 +58,33 @@ set_prop() { set_prop server-port "$PORT" set_prop online-mode false +# RCON is the control plane's write channel (spec §8 写=RCON): the operator probes it +# for readiness and the player tally, and felis-api runs console/permission commands over +# it. Paper only reads these three keys from server.properties, so the operator's injected +# RCON_PASSWORD has to be written here to take effect — env alone does nothing. +# +# Rewritten on EVERY boot from the Secret, deliberately. That makes the value in the world +# volume derived state rather than the source of truth: an owner who edits (or clobbers) +# these lines through the panel's file editor cannot lock the control plane out of their +# own server, because the next restart restores the real password. The editor is also kept +# from reading the password back out — see internal/fileedit/exec.go (spec §286: RCON +# 密码绝不下发前端). +# +# No password, no RCON: an empty enable-rcon=true would let anything that reaches the port +# in unauthenticated. Unlike the forwarding secret this is not fatal — a server without the +# write channel still serves players — so it warns and starts rather than refusing. +if [ -n "${RCON_PASSWORD:-}" ]; then + set_prop enable-rcon true + set_prop rcon.port "${RCON_PORT:-25575}" + set_prop rcon.password "$RCON_PASSWORD" + echo "felis-lobby: rcon enabled on port ${RCON_PORT:-25575}" +else + set_prop enable-rcon false + echo "felis-lobby: WARNING — RCON_PASSWORD is empty, so the console, the online-player" >&2 + echo " list and permission changes will be unavailable for this server. The operator" >&2 + echo " injects it from the -rcon Secret when spec.rcon.enabled is true." >&2 +fi + # ponytail: rewritten whole, not merged. Paper loads this file and fills every key it does # not find with the default, then writes the full tree back — so a proxies-only file is a # complete, stable input, and the lobby's other globals are simply always the defaults. diff --git a/internal/api/k8scluster.go b/internal/api/k8scluster.go index bc3e461..ce45d94 100644 --- a/internal/api/k8scluster.go +++ b/internal/api/k8scluster.go @@ -96,6 +96,24 @@ func (k *K8sCluster) CreateServer(ctx context.Context, in CreateServerInput) err FallbackServer: naming.SystemLoginServer, Storage: v1alpha1.StorageSpec{Size: in.StorageSize}, Resources: in.Resources, + // RCON is what makes a server manageable at all: the operator gates + // phase=Running on the probe and samples the player tally from it (spec + // §5), and every write — console commands, the LuckPerms grants behind the + // permissions UI — travels over it (spec §8 写=RCON). Leaving it unset + // produced a server that looked Running, reported nobody online, and + // answered the console with 503; enabling it here is the fix for all + // three. Port stays 0 so the operator applies its own default rather than + // this package pinning a second copy of it. The password is not set (and + // felis-api could not set it — it holds secrets:get, not create): the + // operator mints it into this Secret on first reconcile, and felis-api + // only ever reads it back at command time. + Rcon: v1alpha1.RconSpec{ + Enabled: true, + SecretRef: v1alpha1.SecretKeyRef{ + Name: naming.RconSecretName(in.Name), + Key: naming.RconSecretKey, + }, + }, }, } if err := k.c.Create(ctx, ms); err != nil { diff --git a/internal/api/k8scluster_test.go b/internal/api/k8scluster_test.go new file mode 100644 index 0000000..9259558 --- /dev/null +++ b/internal/api/k8scluster_test.go @@ -0,0 +1,65 @@ +package api + +import ( + "context" + "testing" + + "felis.lolicon.best/internal/apis/felis/v1alpha1" + "felis.lolicon.best/internal/naming" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +// K8sCluster is documented as integration-tested against a live cluster rather +// than covered by the hermetic suite, and for most of it that is the right call — +// merge-patch semantics are not worth faking. This one test departs from it +// deliberately: "CreateServer forgot to set spec.rcon" is precisely the kind of +// defect a live-cluster test catches only if someone runs it, and it shipped. It +// produced servers that reported Running, listed nobody online, and answered the +// console with 503, and the cause was a struct literal missing a field. A fake +// client is enough to pin a struct literal. +func TestCreateServerEnablesRcon(t *testing.T) { + scheme := runtime.NewScheme() + if err := v1alpha1.AddToScheme(scheme); err != nil { + t.Fatalf("scheme: %v", err) + } + c := fake.NewClientBuilder().WithScheme(scheme).Build() + k := NewK8sCluster(c, "minecraft") + + if err := k.CreateServer(context.Background(), CreateServerInput{ + Name: "survival", + Subdomain: "survival", + DisplayName: "Survival", + Image: "reg/paper:1", + JavaMemory: "2G", + StorageSize: "10Gi", + }); err != nil { + t.Fatalf("CreateServer: %v", err) + } + + var ms v1alpha1.MinecraftServer + if err := c.Get(context.Background(), + types.NamespacedName{Namespace: "minecraft", Name: "survival"}, &ms); err != nil { + t.Fatalf("get created server: %v", err) + } + + if !ms.Spec.Rcon.Enabled { + t.Fatal("spec.rcon.enabled is false: the operator will skip the readiness probe and " + + "never sample a player count, and every console write will fail with 503") + } + // The name is the contract with internal/operator.ensureRconSecret, which + // provisions the password against exactly this key. A mismatch leaves the pod + // stuck on a secretKeyRef that nothing ever creates. + if got, want := ms.Spec.Rcon.SecretRef.Name, naming.RconSecretName("survival"); got != want { + t.Fatalf("rcon secretRef name = %q, want %q", got, want) + } + if got, want := ms.Spec.Rcon.SecretRef.Key, naming.RconSecretKey; got != want { + t.Fatalf("rcon secretRef key = %q, want %q", got, want) + } + // felis-api holds secrets:get, not create — it must never try to mint the + // password itself, and nothing here should carry one. + if ms.Spec.Rcon.Port != 0 { + t.Fatalf("rcon port = %d, want 0 so the operator default is the only copy", ms.Spec.Rcon.Port) + } +} diff --git a/internal/fileedit/exec.go b/internal/fileedit/exec.go index 9a63c4b..cb8ace0 100644 --- a/internal/fileedit/exec.go +++ b/internal/fileedit/exec.go @@ -1,6 +1,7 @@ package fileedit import ( + "bytes" "encoding/json" "errors" "fmt" @@ -265,7 +266,50 @@ func read(r *os.Root, name string) Result { if err != nil { return failure(err, name) } - return Result{Content: b} + return Result{Content: redactSecretProps(name, b)} +} + +// propsPath is the server's main config file, and rconPasswordKey the one line in +// it the editor must not hand back (spec §286: RCON 密码绝不下发前端). +const ( + propsPath = "server.properties" + rconPasswordKey = "rcon.password" + // redactedValue is deliberately not empty: a blank value would read as "RCON has + // no password", which is a very different and much more alarming claim than "you + // are not being shown it". + redactedValue = "" +) + +// redactSecretProps blanks the RCON password when server.properties is read. +// +// Unlike secretConfigPath this is a value redaction rather than a whole-file +// denial, because the file is not platform material that merely happens to sit in +// the volume — it is the single most-edited config a server owner has (MOTD, +// view-distance, difficulty, gamemode), and refusing it outright would cost real +// repair to hide one line. The password is also per-server and garbage-collected +// with it, so unlike the cluster-wide forwarding secret it leaks nothing about +// anyone else's server; it is withheld because §286 draws the line at the frontend +// regardless of blast radius, and because the console already gives an owner every +// capability the password would. +// +// The write path is left alone on purpose, mirroring the reasoning at +// secretConfigPath: felis-lobby's entrypoint rewrites all three rcon keys from the +// injected Secret on every boot, so saving the placeholder back cannot lock the +// control plane out — the next restart restores the real value. That is what makes +// redaction safe here; without the boot-time rewrite this would be a footgun. +func redactSecretProps(name string, content []byte) []byte { + if path.Clean(name) != propsPath { + return content + } + lines := bytes.Split(content, []byte("\n")) + for i, line := range lines { + // TrimSpace before matching: a properties key may be indented, and the + // trailing \r of a CRLF file would otherwise ride along into the value. + if bytes.HasPrefix(bytes.TrimSpace(line), []byte(rconPasswordKey+"=")) { + lines[i] = []byte(rconPasswordKey + "=" + redactedValue) + } + } + return bytes.Join(lines, []byte("\n")) } // write replaces a file's contents. It truncates rather than appends, and it does diff --git a/internal/fileedit/exec_test.go b/internal/fileedit/exec_test.go index c570f20..8a04d13 100644 --- a/internal/fileedit/exec_test.go +++ b/internal/fileedit/exec_test.go @@ -355,3 +355,51 @@ func TestReadRefusesTheForwardingSecret(t *testing.T) { t.Errorf("write code = %q, want success — only the read is denied", res.Code) } } + +// TestReadRedactsRconPassword pins spec §286 (RCON 密码绝不下发前端) on the one +// path that could leak it: server.properties is the file owners edit most, so it +// is readable — but the RCON password in it is the control plane's command +// credential for that server, and the editor must not hand it back. +func TestReadRedactsRconPassword(t *testing.T) { + root := t.TempDir() + props := "motd=hello\nrcon.password=hunter2\nrcon.port=25575\nenable-rcon=true\n" + if err := os.WriteFile(filepath.Join(root, "server.properties"), []byte(props), 0o644); err != nil { + t.Fatalf("write server.properties: %v", err) + } + // A same-named file in a subdirectory must NOT be treated as the real one: the + // redaction keys off the cleaned path, and a plugin is free to keep its own + // server.properties anywhere in the volume. + if err := os.MkdirAll(filepath.Join(root, "plugins"), 0o755); err != nil { + t.Fatalf("mkdir plugins: %v", err) + } + if err := os.WriteFile(filepath.Join(root, "plugins", "server.properties"), []byte("rcon.password=notmine\n"), 0o644); err != nil { + t.Fatalf("write nested server.properties: %v", err) + } + + res, err := Execute(root, OpRead, "server.properties", nil) + if err != nil { + t.Fatalf("Execute: %v", err) + } + got := string(res.Content) + if strings.Contains(got, "hunter2") { + t.Fatalf("read returned the RCON password (spec §286):\n%s", got) + } + if !strings.Contains(got, redactedValue) { + t.Fatalf("read did not mark the password as withheld:\n%s", got) + } + // Redaction must not cost the owner the rest of the file — that is the whole + // reason this is a value redaction and not a whole-file denial. + for _, keep := range []string{"motd=hello", "rcon.port=25575", "enable-rcon=true"} { + if !strings.Contains(got, keep) { + t.Fatalf("redaction dropped %q from the file:\n%s", keep, got) + } + } + + nested, err := Execute(root, OpRead, "plugins/server.properties", nil) + if err != nil { + t.Fatalf("Execute nested: %v", err) + } + if !strings.Contains(string(nested.Content), "notmine") { + t.Fatalf("a nested server.properties was redacted; only the world root's is the real one:\n%s", nested.Content) + } +} diff --git a/internal/naming/naming.go b/internal/naming/naming.go index 7ef4267..e6e60ac 100644 --- a/internal/naming/naming.go +++ b/internal/naming/naming.go @@ -127,6 +127,21 @@ func WorldPVCName(server string) string { return worldVolumeName + "-" + server + "-0" } +// RconSecretKey is the key holding the password inside a server's RCON Secret. +const RconSecretKey = "password" + +// RconSecretName returns the per-server RCON password Secret name. Like +// WorldPVCName this convention is shared rather than duplicated: felis-api writes +// it into spec.rcon.secretRef when creating a server, `felis setup` writes the +// same for the system lobby, and the operator both provisions the Secret under +// that name and reads it back for the readiness probe. One password per server, +// never a shared one — the console grants whoever holds it full command authority +// over that server, so a single cluster-wide value would make every owner an +// operator of everyone else's world. +func RconSecretName(server string) string { + return server + "-rcon" +} + // Hostname composes subdomain.rootDomain after validating the subdomain. func Hostname(subdomain, rootDomain string) (string, error) { if err := ValidateServerName(subdomain); err != nil { diff --git a/internal/operator/reconciler.go b/internal/operator/reconciler.go index 4ba608b..11b9400 100644 --- a/internal/operator/reconciler.go +++ b/internal/operator/reconciler.go @@ -6,6 +6,8 @@ package operator import ( "context" + "crypto/rand" + "encoding/hex" "fmt" "time" @@ -84,6 +86,18 @@ func (r *Reconciler) reconcileRunning(ctx context.Context, server *v1alpha1.Mine return ctrl.Result{}, err } + // Before the StatefulSet, not after: buildEnv wires RCON_PASSWORD as a + // secretKeyRef, so a pod created ahead of its Secret never starts — it sits in + // CreateContainerConfigError, which reads like a broken image rather than a + // missing key. + if err := r.ensureRconSecret(ctx, server); err != nil { + r.markStarting(server, "RconSecretUnavailable", err.Error()) + if perr := r.patchStatus(ctx, server); perr != nil { + return ctrl.Result{}, perr + } + return ctrl.Result{RequeueAfter: requeueSecret}, nil + } + desired, err := buildStatefulSet(server, 1) if err != nil { // A malformed spec (e.g. bad storage quantity) is terminal until edited. @@ -151,7 +165,10 @@ func (r *Reconciler) reconcileRunning(ctx context.Context, server *v1alpha1.Mine // player tally is zero, track the empty duration and auto-stop when the // configured timeout expires. The existing RCON probe already supplies // the player count — no extra network cost. - if server.Spec.Idle.AutoStopEnabled && server.Spec.Idle.EmptySecondsBeforeStop > 0 { + // Rcon.Enabled is part of the condition because `players` is only a real tally + // when the probe above ran: with RCON off it keeps its zero value, which this + // branch would read as "empty" and use to stop a server full of people. + if server.Spec.Rcon.Enabled && server.Spec.Idle.AutoStopEnabled && server.Spec.Idle.EmptySecondsBeforeStop > 0 { if players.Online == 0 { if server.Status.EmptySince == nil { t := r.now() @@ -228,6 +245,84 @@ func (r *Reconciler) ensureServices(ctx context.Context, server *v1alpha1.Minecr return endpointAddress, nil } +// ensureRconSecret creates the per-server RCON password Secret named by +// spec.rcon.secretRef the first time a server with RCON enabled reconciles, and +// leaves it alone afterwards. Provisioning lives here rather than in felis-api's +// CreateServer for three reasons: it is declarative (a server whose Secret was +// deleted heals on the next pass instead of staying permanently unreachable), the +// controller reference makes Kubernetes garbage-collect the Secret with the server +// so no delete path has to remember it, and it backfills — a server created before +// RCON existed only needs spec.rcon filled in, and the password appears without +// anyone handling it. felis-api never mints the password and never needs to: it +// reads the Secret at command time (internal/api/console.go rconPassword). +// +// The password is 32 hex chars from crypto/rand. It is generated once and never +// rotated here: rewriting it would leave the running server authenticating with +// the old value until its pod restarts, so rotation belongs to an explicit +// operation, not to a reconcile that runs every few seconds. +func (r *Reconciler) ensureRconSecret(ctx context.Context, server *v1alpha1.MinecraftServer) error { + if !server.Spec.Rcon.Enabled { + return nil + } + ref := server.Spec.Rcon.SecretRef + if ref.Name == "" || ref.Key == "" { + return fmt.Errorf("rcon.secretRef.name and .key are required when rcon is enabled") + } + var existing corev1.Secret + err := r.Get(ctx, types.NamespacedName{Namespace: server.Namespace, Name: ref.Name}, &existing) + if err == nil { + if _, ok := existing.Data[ref.Key]; ok { + return nil + } + return fmt.Errorf("secret %s exists but has no key %q", ref.Name, ref.Key) + } + if !apierrors.IsNotFound(err) { + return err + } + + password, err := randomRconPassword() + if err != nil { + return err + } + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: ref.Name, + Namespace: server.Namespace, + Labels: map[string]string{v1alpha1.LabelServer: server.Name}, + }, + Type: corev1.SecretTypeOpaque, + // Data, not StringData: StringData is a write-only convenience the API server + // folds into Data, so anything reading the object back — including this + // package's own tests — would see an empty value. Encoding here keeps the + // object self-consistent the moment it is built. + Data: map[string][]byte{ref.Key: []byte(password)}, + } + if err := controllerutil.SetControllerReference(server, secret, r.Scheme); err != nil { + return err + } + if err := r.Create(ctx, secret); err != nil { + // Another reconcile (or a racing replica) won: that Secret is as good as + // this one, so treat the collision as success rather than thrashing. + if apierrors.IsAlreadyExists(err) { + return nil + } + return err + } + return nil +} + +// randomRconPassword returns 16 crypto/rand bytes as hex. Hex, not base64: the +// value is written verbatim into server.properties, whose parser treats the line +// as raw text to end-of-line, and hex avoids every character (=, :, \, whitespace) +// that a properties file or a shell round-trip could interpret. +func randomRconPassword() (string, error) { + var b [16]byte + if _, err := rand.Read(b[:]); err != nil { + return "", fmt.Errorf("generate rcon password: %w", err) + } + return hex.EncodeToString(b[:]), nil +} + func (r *Reconciler) rconPassword(ctx context.Context, server *v1alpha1.MinecraftServer) (string, error) { ref := server.Spec.Rcon.SecretRef if ref.Name == "" || ref.Key == "" { diff --git a/internal/operator/reconciler_test.go b/internal/operator/reconciler_test.go index bb47d69..fecc6d5 100644 --- a/internal/operator/reconciler_test.go +++ b/internal/operator/reconciler_test.go @@ -640,3 +640,73 @@ func TestReconcileRunning_StartDurationObservedOnce(t *testing.T) { t.Errorf("startRequestedAt = %v after Stop, want nil so the next start re-anchors", s.Status.StartRequestedAt) } } + +// A server whose RCON Secret does not exist yet is the normal case on first +// reconcile — felis-api writes spec.rcon.secretRef but holds secrets:get, not +// create, so the name it points at is a promise the operator has to keep. Before +// this the server sat in Starting/RconSecretUnavailable forever. +func TestReconcileProvisionsMissingRconSecret(t *testing.T) { + r, c := newReconciler(t, fakeProber{}, runningServer()) + reconcile(t, r, "survival") + + var secret corev1.Secret + if err := c.Get(context.Background(), + types.NamespacedName{Namespace: "minecraft", Name: "survival-rcon"}, &secret); err != nil { + t.Fatalf("expected the operator to create the rcon Secret: %v", err) + } + pw := string(secret.Data["password"]) + if pw == "" { + t.Fatal("rcon Secret was created with an empty password") + } + // 16 random bytes as hex. An assertion on the shape, not the value: a password + // short enough to guess is the failure this is guarding, and it cannot be + // checked by comparing against a fixture. + if len(pw) != 32 { + t.Fatalf("password = %d chars, want 32 hex chars", len(pw)) + } + if strings.Trim(pw, "0123456789abcdef") != "" { + t.Fatalf("password %q is not hex; it is written verbatim into server.properties", pw) + } + // The controller reference is what makes Kubernetes delete the Secret with the + // server. Without it every deleted server leaves its password behind. + if len(secret.OwnerReferences) != 1 || secret.OwnerReferences[0].Name != "survival" { + t.Fatalf("owner references = %+v, want one referring to the server", secret.OwnerReferences) + } +} + +// Reconcile runs every few seconds; regenerating the password on each pass would +// leave the running server authenticating with a value the control plane no longer +// has, breaking the console until the pod happened to restart. +func TestReconcileKeepsAnExistingRconPassword(t *testing.T) { + r, c := newReconciler(t, fakeProber{}, runningServer(), rconSecret()) + reconcile(t, r, "survival") + reconcile(t, r, "survival") + + var secret corev1.Secret + if err := c.Get(context.Background(), + types.NamespacedName{Namespace: "minecraft", Name: "survival-rcon"}, &secret); err != nil { + t.Fatalf("get rcon secret: %v", err) + } + if got := string(secret.Data["password"]); got != "hunter2" { + t.Fatalf("password = %q, want the original %q — the operator rotated it", got, "hunter2") + } +} + +// The idle auto-stop reads Status.Players, which is only sampled by the RCON +// probe. With RCON off the tally stays at its zero value, and a bare +// "players == 0" test would read that as an empty server and stop one full of +// people. +func TestIdleAutoStopIsInertWithoutRcon(t *testing.T) { + server := runningServer() + server.Spec.Rcon = v1alpha1.RconSpec{Enabled: false} + server.Spec.Idle = v1alpha1.IdleSpec{AutoStopEnabled: true, EmptySecondsBeforeStop: 1} + + r, c := newReconciler(t, fakeProber{}, server) + reconcile(t, r, "survival") + reconcile(t, r, "survival") + + if got := getServer(t, c, "survival").Spec.DesiredState; got != v1alpha1.DesiredRunning { + t.Fatalf("desiredState = %q, want %q — idle auto-stop fired on an unprobed player count", + got, v1alpha1.DesiredRunning) + } +} diff --git a/internal/platform/rbac.go b/internal/platform/rbac.go index b3cbc38..3cfd9c5 100644 --- a/internal/platform/rbac.go +++ b/internal/platform/rbac.go @@ -136,7 +136,13 @@ func OperatorRole(p Params) *rbacv1.Role { rule([]string{groupFelis}, []string{"minecraftservers/status"}, []string{"update"}), rule([]string{groupApps}, []string{"statefulsets"}, []string{"get", "list", "watch", "create", "update"}), rule([]string{groupCore}, []string{"services"}, []string{"get", "list", "watch", "create", "update"}), - rule([]string{groupCore}, []string{"secrets"}, []string{"get", "list", "watch"}), + // create is here for the per-server RCON password Secret the operator + // provisions on first reconcile (internal/operator.ensureRconSecret). It is a + // smaller grant than it looks: this identity already holds get/list/watch on + // every Secret in this namespace, so being able to add one grants no read it + // did not already have. No update/delete — the password is written once and + // removed by garbage collection through its controller reference. + rule([]string{groupCore}, []string{"secrets"}, []string{"get", "list", "watch", "create"}), }) }