feat(operator): secure system server workloads

internal/operator/builders.go

@@ -18,6 +18,11 @@ import (
 // system server (see buildEnv), sourced from a Secret, never a literal.
 const envServiceToken = "FELIS_SERVICE_TOKEN"
 
+// envForwardingSecret is the environment variable a backend reads the Velocity
+// modern-forwarding secret from. Unlike the service token it goes to EVERY backend
+// (see buildEnv), because Velocity's forwarding mode is proxy-wide.
+const envForwardingSecret = "FELIS_FORWARDING_SECRET"
+
 // Workload constants shared by the builders.
 const (
 	// GamePort is the Minecraft TCP port the proxy and readiness probe target.
@@ -86,11 +91,6 @@ func rconAddress(server *v1alpha1.MinecraftServer) string {
 	return fmt.Sprintf("%s.%s.svc.cluster.local:%d", server.Name, server.Namespace, rconPort(server))
 }
 
-// gameAddress is the in-cluster game endpoint advertised when Running.
-func gameAddress(server *v1alpha1.MinecraftServer) string {
-	return fmt.Sprintf("%s.%s.svc.cluster.local:%d", server.Name, server.Namespace, GamePort)
-}
-
 // preStopScript is the operator-injected graceful-shutdown sequence (spec §7):
 // flush the world, then stop the server, both over RCON. It relies on rcon-cli
 // being present in the Felis base image and reading the RCON_* env injected
@@ -309,12 +309,13 @@ func buildEnv(server *v1alpha1.MinecraftServer) []corev1.EnvVar {
 	// link status), so it — and only it — receives the service token. Injected
 	// from a Secret in this namespace, never inlined into the CRD (the same
 	// discipline as RCON_PASSWORD above; the CRD's EnvVar type has no valueFrom
-	// precisely so a user server cannot mount an arbitrary secret). Keyed off the
-	// reserved "login" name, which naming.ValidateServerName forbids any user
-	// server from claiming — so this can never leak the token into a user's pod.
+	// precisely so a user server cannot mount an arbitrary secret). Require both
+	// the reserved name and the setup-owned system-role label: the label prevents
+	// a legacy user server named "login" from receiving the token after upgrade.
 	// The Secret must exist in this (minecraft) namespace; `felis setup` replicates
 	// it there from the control namespace before creating this server.
-	if server.Name == naming.SystemLoginServer {
+	if server.Name == naming.SystemLoginServer &&
+		server.Labels[v1alpha1.LabelSystemRole] == naming.SystemLoginServer {
 		env = append(env, corev1.EnvVar{
 			Name: envServiceToken,
 			ValueFrom: &corev1.EnvVarSource{
@@ -325,6 +326,30 @@ func buildEnv(server *v1alpha1.MinecraftServer) []corev1.EnvVar {
 			},
 		})
 	}
+	// The Velocity modern-forwarding secret goes to EVERY backend, system and user
+	// alike — not because user servers are trusted, but because Velocity's forwarding
+	// mode is one proxy-wide setting: with it on, a backend that cannot verify the
+	// signed handshake rejects every login the proxy sends it. Withholding the secret
+	// from user servers would not harden them, it would simply make them unjoinable.
+	// It is the backend's proof that a login really came from the proxy (and so that
+	// the player's UUID is Mojang-verified, not offline-derived) — the pod-level fence
+	// against bypassing the proxy is the NetworkPolicy, not this value's secrecy.
+	//
+	// A user server is built from an operator-typed Dockerfile, so Felis cannot make it
+	// consume this; the two images Felis does build (deploy/limbo, deploy/lobby) read it
+	// in their entrypoints and refuse to start without it. Optional so a cluster whose
+	// proxy is not in modern mode — no Secret provisioned — still schedules its pods
+	// instead of wedging them all in CreateContainerConfigError.
+	env = append(env, corev1.EnvVar{
+		Name: envForwardingSecret,
+		ValueFrom: &corev1.EnvVarSource{
+			SecretKeyRef: &corev1.SecretKeySelector{
+				LocalObjectReference: corev1.LocalObjectReference{Name: naming.ForwardingSecretName},
+				Key:                  naming.ForwardingSecretKey,
+				Optional:             boolPtr(true),
+			},
+		},
+	})
 	return env
 }
 

internal/operator/builders_internal_test.go

@@ -83,6 +83,7 @@ func TestBuildStatefulSetAddsHealthPort(t *testing.T) {
 func TestBuildEnvInjectsServiceTokenForLogin(t *testing.T) {
 	s := &v1alpha1.MinecraftServer{}
 	s.Name = naming.SystemLoginServer
+	s.Labels = map[string]string{v1alpha1.LabelSystemRole: naming.SystemLoginServer}
 	tok := findEnv(buildEnv(s), envServiceToken)
 	if tok == nil {
 		t.Fatalf("%s not injected for the login server", envServiceToken)
@@ -99,15 +100,60 @@ func TestBuildEnvInjectsServiceTokenForLogin(t *testing.T) {
 	}
 }
 
-// A user server (any non-login name) must NOT receive the service token — the
-// reserved-name gate is what stops the internal credential leaking into a player's
-// pod. naming.ValidateServerName forbids users from ever claiming "login".
+// A user server must NOT receive the service token. This includes a legacy,
+// unlabeled server named "login" that predates the reserved-name rule.
 func TestBuildEnvWithholdsServiceTokenFromUserServers(t *testing.T) {
-	for _, name := range []string{"survival", "creative", naming.SystemLobbyServer} {
+	tests := []struct {
+		name   string
+		labels map[string]string
+	}{
+		{name: "survival"},
+		{name: "creative"},
+		{name: naming.SystemLobbyServer, labels: map[string]string{v1alpha1.LabelSystemRole: naming.SystemLobbyServer}},
+		{name: naming.SystemLoginServer},
+		{name: naming.SystemLoginServer, labels: map[string]string{v1alpha1.LabelSystemRole: naming.SystemLobbyServer}},
+	}
+	for _, tt := range tests {
 		s := &v1alpha1.MinecraftServer{}
-		s.Name = name
+		s.Name = tt.name
+		s.Labels = tt.labels
 		if tok := findEnv(buildEnv(s), envServiceToken); tok != nil {
-			t.Errorf("%s: service token leaked into a non-login server", name)
+			t.Errorf("%s labels=%v: service token leaked into a non-system login server", tt.name, tt.labels)
+		}
+	}
+}
+
+// Every backend receives the Velocity modern-forwarding secret — system and user alike.
+// This is the opposite rule from the service token, and deliberately so: Velocity's
+// forwarding mode is proxy-wide, so a backend without the secret cannot verify the
+// signed handshake and rejects every login the proxy sends it. It is also what makes a
+// backend's view of a player's UUID trustworthy (Mojang-verified via the signed payload,
+// not offline-derived from the username) — the premise the Owner bind rests on.
+// Sourced from a Secret, never a literal, and optional so a cluster whose proxy is not
+// in modern mode still schedules its pods.
+func TestBuildEnvInjectsForwardingSecretIntoEveryBackend(t *testing.T) {
+	for _, name := range []string{naming.SystemLoginServer, naming.SystemLobbyServer, "survival"} {
+		s := &v1alpha1.MinecraftServer{}
+		s.Name = name
+		fwd := findEnv(buildEnv(s), envForwardingSecret)
+		if fwd == nil {
+			t.Errorf("%s: %s not injected — the backend would reject every proxied login", name, envForwardingSecret)
+			continue
+		}
+		if fwd.Value != "" {
+			t.Errorf("%s: %s carries a literal value %q — it must be a secretKeyRef", name, envForwardingSecret, fwd.Value)
+			continue
+		}
+		if fwd.ValueFrom == nil || fwd.ValueFrom.SecretKeyRef == nil {
+			t.Errorf("%s: %s must be sourced from a secretKeyRef", name, envForwardingSecret)
+			continue
+		}
+		ref := fwd.ValueFrom.SecretKeyRef
+		if ref.Name != naming.ForwardingSecretName || ref.Key != naming.ForwardingSecretKey {
+			t.Errorf("%s: secretKeyRef = %s/%s, want %s/%s", name, ref.Name, ref.Key, naming.ForwardingSecretName, naming.ForwardingSecretKey)
+		}
+		if ref.Optional == nil || !*ref.Optional {
+			t.Errorf("%s: secretKeyRef must be optional, or a cluster without the Secret wedges every pod in CreateContainerConfigError", name)
 		}
 	}
 }

internal/operator/reconciler.go

@@ -79,7 +79,8 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 }
 
 func (r *Reconciler) reconcileRunning(ctx context.Context, server *v1alpha1.MinecraftServer) (ctrl.Result, error) {
-	if err := r.ensureServices(ctx, server); err != nil {
+	endpointAddress, err := r.ensureServices(ctx, server)
+	if err != nil {
 		return ctrl.Result{}, err
 	}
 
@@ -112,6 +113,13 @@ func (r *Reconciler) reconcileRunning(ctx context.Context, server *v1alpha1.Mine
 		}
 		return ctrl.Result{RequeueAfter: requeueStarting}, nil
 	}
+	if endpointAddress == "" {
+		r.markStarting(server, "ServiceAddressPending", "waiting for the client Service ClusterIP")
+		if err := r.patchStatus(ctx, server); err != nil {
+			return ctrl.Result{}, err
+		}
+		return ctrl.Result{RequeueAfter: requeueStarting}, nil
+	}
 
 	// Then the operator gates true readiness on an RCON probe (spec §5), which
 	// also samples the current player tally for Status.Players.
@@ -162,7 +170,7 @@ func (r *Reconciler) reconcileRunning(ctx context.Context, server *v1alpha1.Mine
 		}
 	}
 
-	r.markRunningReady(server, players)
+	r.markRunningReady(server, players, endpointAddress)
 	return ctrl.Result{}, r.patchStatus(ctx, server)
 }
 
@@ -198,16 +206,26 @@ func (r *Reconciler) reconcileStopped(ctx context.Context, server *v1alpha1.Mine
 	return ctrl.Result{}, r.patchStatus(ctx, server)
 }
 
-func (r *Reconciler) ensureServices(ctx context.Context, server *v1alpha1.MinecraftServer) error {
+func (r *Reconciler) ensureServices(ctx context.Context, server *v1alpha1.MinecraftServer) (string, error) {
+	endpointAddress := ""
 	for _, svc := range []*corev1.Service{buildHeadlessService(server), buildClientService(server)} {
 		if err := controllerutil.SetControllerReference(server, svc, r.Scheme); err != nil {
-			return err
+			return "", err
 		}
 		if err := r.applyService(ctx, svc); err != nil {
-			return err
+			return "", err
+		}
+		if svc.Name == server.Name {
+			var current corev1.Service
+			if err := r.Get(ctx, client.ObjectKeyFromObject(svc), &current); err != nil {
+				return "", err
+			}
+			if current.Spec.ClusterIP != "" && current.Spec.ClusterIP != corev1.ClusterIPNone {
+				endpointAddress = fmt.Sprintf("%s:%d", current.Spec.ClusterIP, GamePort)
+			}
 		}
 	}
-	return nil
+	return endpointAddress, nil
 }
 
 func (r *Reconciler) rconPassword(ctx context.Context, server *v1alpha1.MinecraftServer) (string, error) {
@@ -287,7 +305,7 @@ func (r *Reconciler) markStarting(server *v1alpha1.MinecraftServer, reason, msg
 	r.setCondition(server, v1alpha1.ConditionRconReached, metav1.ConditionFalse, reason, msg)
 }
 
-func (r *Reconciler) markRunningReady(server *v1alpha1.MinecraftServer, players PlayerCount) {
+func (r *Reconciler) markRunningReady(server *v1alpha1.MinecraftServer, players PlayerCount, endpointAddress string) {
 	server.Status.Phase = v1alpha1.PhaseRunning
 	server.Status.Ready = true
 	server.Status.ObservedGeneration = server.Generation
@@ -305,7 +323,7 @@ func (r *Reconciler) markRunningReady(server *v1alpha1.MinecraftServer, players
 			metrics.StartDurationSeconds.Observe(t.Sub(server.Status.StartRequestedAt.Time).Seconds())
 		}
 	}
-	server.Status.Endpoint = v1alpha1.EndpointStatus{Mode: v1alpha1.EndpointDirect, Address: gameAddress(server)}
+	server.Status.Endpoint = v1alpha1.EndpointStatus{Mode: v1alpha1.EndpointDirect, Address: endpointAddress}
 	server.Status.LiveMotd = server.Spec.Motd.Running
 	r.setCondition(server, v1alpha1.ConditionRconReached, metav1.ConditionTrue, "Probed", "RCON probe succeeded")
 	r.setCondition(server, v1alpha1.ConditionReady, metav1.ConditionTrue, "RconReached", "server is accepting RCON")

internal/operator/reconciler_test.go

@@ -120,6 +120,15 @@ func getSTS(t *testing.T, c client.Client, name string) *appsv1.StatefulSet {
 // markPodReady simulates the kubelet flipping the StatefulSet to ready.
 func markPodReady(t *testing.T, c client.Client, name string) {
 	t.Helper()
+	var svc corev1.Service
+	if err := c.Get(context.Background(), types.NamespacedName{Namespace: "minecraft", Name: name}, &svc); err != nil {
+		t.Fatalf("get client service: %v", err)
+	}
+	svc.Spec.ClusterIP = "10.43.0.42"
+	svc.Spec.ClusterIPs = []string{"10.43.0.42"}
+	if err := c.Update(context.Background(), &svc); err != nil {
+		t.Fatalf("assign client service ClusterIP: %v", err)
+	}
 	sts := getSTS(t, c, name)
 	sts.Status.Replicas = 1
 	sts.Status.ReadyReplicas = 1
@@ -233,6 +242,9 @@ func TestReconcileRunning_RconProbeGatesReadiness(t *testing.T) {
 	if server.Status.Endpoint.Mode != v1alpha1.EndpointDirect {
 		t.Errorf("endpoint mode = %s, want direct", server.Status.Endpoint.Mode)
 	}
+	if server.Status.Endpoint.Address != "10.43.0.42:25565" {
+		t.Errorf("endpoint address = %q, want client Service ClusterIP", server.Status.Endpoint.Address)
+	}
 	if !isConditionTrue(server, v1alpha1.ConditionReady) {
 		t.Error("Ready condition should be True")
 	}
@@ -621,9 +633,9 @@ func TestReconcileRunning_StartDurationObservedOnce(t *testing.T) {
 	if err := c.Update(context.Background(), stopped); err != nil {
 		t.Fatalf("set desiredState=Stopped: %v", err)
 	}
-	reconcile(t, r, "survival")            // scales spec to 0; pods still terminating
-	markPodTerminated(t, c, "survival")    // pods finish draining
-	reconcile(t, r, "survival")            // reaches Stopped, clears startRequestedAt
+	reconcile(t, r, "survival")         // scales spec to 0; pods still terminating
+	markPodTerminated(t, c, "survival") // pods finish draining
+	reconcile(t, r, "survival")         // reaches Stopped, clears startRequestedAt
 	if s := getServer(t, c, "survival"); s.Status.StartRequestedAt != nil {
 		t.Errorf("startRequestedAt = %v after Stop, want nil so the next start re-anchors", s.Status.StartRequestedAt)
 	}