From f5d00f389eddf69c649f8c6aa40a234239125108 Mon Sep 17 00:00:00 2001 From: Lemon-miaow Date: Sun, 28 Jun 2026 02:00:21 +0800 Subject: [PATCH] feat(cli): implement felis apply command for direct CRD creation --- cmd/felis/apply.go | 338 +++++++++++++++++++++++++++++++++++++ cmd/felis/apply_test.go | 359 ++++++++++++++++++++++++++++++++++++++++ cmd/felis/run.go | 12 +- cmd/felis/run_test.go | 34 +++- 4 files changed, 725 insertions(+), 18 deletions(-) create mode 100644 cmd/felis/apply.go create mode 100644 cmd/felis/apply_test.go diff --git a/cmd/felis/apply.go b/cmd/felis/apply.go new file mode 100644 index 0000000..acae573 --- /dev/null +++ b/cmd/felis/apply.go @@ -0,0 +1,338 @@ +package main + +import ( + "context" + "encoding/json" + "errors" + "flag" + "fmt" + "io" + "os" + "strings" + + "felis.lolicon.best/internal/apis/felis/v1alpha1" + "felis.lolicon.best/internal/naming" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// cmdApply creates a MinecraftServer CRD from a JSON form. +// +// It is an operator-only direct CRD create path — it does NOT seed Postgres +// business rows (ownership, alias, image whitelist, audit), so the resulting +// server is claimable only if those rows are seeded separately. For normal +// provisioning prefer the Web form or felis-api. +// +// Usage: felis apply -f server.json [-n minecraft] +// +// This is a DIRECT Kubernetes write — it does not go through felis-api. It +// requires a kubeconfig or in-cluster identity with "create minecraftservers" +// permission in the target namespace. Subdomain duplicates are checked against +// existing CRDs in the target namespace (same check as the web form's +// Cluster.GetBySubdomain). + +// applyRequest is the CLI-facing server creation form. It mirrors the Web +// form's field shape (createServerRequest) so the two provisioners stay +// structurally aligned, but validation differs: here we validate the CRD +// resource shape only — image whitelist admission, PG alias seeding, quota, and +// audit are the API's business layer and are NOT performed. +type applyRequest struct { + Name string `json:"name"` + Subdomain string `json:"subdomain"` + DisplayName string `json:"displayName,omitempty"` + Image string `json:"image"` + Memory string `json:"memory"` + Storage string `json:"storage"` + AutostartPolicy string `json:"autostartPolicy,omitempty"` + Resources *resourceRequest `json:"resources,omitempty"` +} + +// resourceRequest mirrors the API's resourceRequest. +type resourceRequest struct { + CPU string `json:"cpu,omitempty"` + CPURequest string `json:"cpuRequest,omitempty"` + Memory string `json:"memory,omitempty"` + MemoryRequest string `json:"memoryRequest,omitempty"` +} + +func cmdApply(args []string, stdout, stderr io.Writer) int { + fs := flag.NewFlagSet("apply", flag.ContinueOnError) + fs.SetOutput(stderr) + file := fs.String("f", "", "path to JSON server form (required; use - for stdin)") + namespace := fs.String("n", "minecraft", "Kubernetes namespace") + if err := fs.Parse(args); err != nil { + return 2 + } + if *file == "" { + fmt.Fprintln(stderr, "felis apply: missing required flag -f; use -f server.json or -f - for stdin") + return 2 + } + + var raw []byte + var err error + if *file == "-" { + raw, err = io.ReadAll(os.Stdin) + } else { + raw, err = os.ReadFile(*file) + } + if err != nil { + fmt.Fprintf(stderr, "felis apply: read: %v\n", err) + return 1 + } + + dec := json.NewDecoder(strings.NewReader(string(raw))) + dec.DisallowUnknownFields() + var req applyRequest + if err := dec.Decode(&req); err != nil { + fmt.Fprintf(stderr, "felis apply: invalid JSON: %v\n", err) + return 1 + } + // Drain the decoder: a second Decode must hit io.EOF — anything else + // (another value, trailing garbage like ] or }) means the input is not + // exactly one valid form. + if err := dec.Decode(&struct{}{}); err == nil || !errors.Is(err, io.EOF) { + fmt.Fprintln(stderr, "felis apply: invalid JSON: unexpected data after the server form") + return 1 + } + + // Build the CRD from the form. This validates the resource shape but does + // NOT perform image whitelist admission or PG seeding (API business layer). + ms, err := buildMinecraftServerFromApplyRequest(req, *namespace) + if err != nil { + fmt.Fprintf(stderr, "felis apply: %v\n", err) + return 1 + } + + // ------- K8s client (one context, one client) ------- + // SetupSignalHandler must be called exactly once per process — + // controller-runtime panics on a second call. We create ctx and the + // K8s client here and thread both through every downstream call so no + // callee ever needs to call SetupSignalHandler again. + ctx := ctrl.SetupSignalHandler() + + scheme := runtime.NewScheme() + utilruntime.Must(clientgoscheme.AddToScheme(scheme)) + utilruntime.Must(v1alpha1.AddToScheme(scheme)) + + cfg := ctrl.GetConfigOrDie() + cl, err := client.New(cfg, client.Options{Scheme: scheme}) + if err != nil { + fmt.Fprintf(stderr, "felis apply: build client: %v\n", err) + return 1 + } + + // Subdomain duplicate check: the web form queries GetBySubdomain before + // creating; we list all CRDs in the namespace and check spec.subdomain. + // metadata.name already receives K8s AlreadyExists enforcement on create, + // so a name collision surfaces cleanly — but subdomain has no such native + // uniqueness, so it must be checked explicitly. + if err := checkSubdomainUnique(ctx, cl, *namespace, req.Subdomain); err != nil { + fmt.Fprintf(stderr, "felis apply: %v\n", err) + return 1 + } + + if err := cl.Create(ctx, ms); err != nil { + if apierrors.IsAlreadyExists(err) { + fmt.Fprintf(stderr, "felis apply: server %q already exists\n", req.Name) + return 1 + } + fmt.Fprintf(stderr, "felis apply: create: %v\n", err) + return 1 + } + + fmt.Fprintf(stdout, "created MinecraftServer %s/%s (subdomain=%s, image=%s, memory=%s, storage=%s, policy=%s)\n", + *namespace, req.Name, req.Subdomain, req.Image, resourcesMemoryString(ms.Spec.Resources), ms.Spec.Storage.Size, ms.Spec.AutostartPolicy) + fmt.Fprintln(stderr, "note: Postgres business rows (ownership / alias / whitelist / audit) were NOT seeded — prefer Web/API for normal provisioning") + return 0 +} + +// buildMinecraftServerFromApplyRequest validates the request and constructs a +// MinecraftServer CRD. It is a pure function (no K8s, no I/O) so it can be +// tested without a cluster. It validates: name/subdomain format, required +// fields, policy enum, positive K8s quantities, request ≤ limit, and the §22 +// memory ceiling. The returned CRD is always DesiredState=Stopped and unowned +// (ownership is established by a later claim). +func buildMinecraftServerFromApplyRequest(req applyRequest, namespace string) (*v1alpha1.MinecraftServer, error) { + // ---- name & subdomain ---- + if err := naming.ValidateServerName(req.Name); err != nil { + return nil, fmt.Errorf("invalid name: %w", err) + } + if err := naming.ValidateServerName(req.Subdomain); err != nil { + return nil, fmt.Errorf("invalid subdomain: %w", err) + } + if strings.TrimSpace(req.Image) == "" { + return nil, fmt.Errorf("image is required") + } + + // ---- autostart policy ---- + policy, err := parseApplyAutostartPolicy(req.AutostartPolicy) + if err != nil { + return nil, err + } + + // ---- resources (§22 ceiling) ---- + memQ, err := parseApplyPositiveQuantity(req.Memory, "memory") + if err != nil { + return nil, err + } + limits := corev1.ResourceList{corev1.ResourceMemory: memQ} + requests := corev1.ResourceList{corev1.ResourceMemory: memQ} + + if req.Resources != nil { + if req.Resources.Memory != "" { + q, err := parseApplyPositiveQuantity(req.Resources.Memory, "resources.memory") + if err != nil { + return nil, err + } + limits[corev1.ResourceMemory] = q + } + if req.Resources.MemoryRequest != "" { + q, err := parseApplyPositiveQuantity(req.Resources.MemoryRequest, "resources.memoryRequest") + if err != nil { + return nil, err + } + requests[corev1.ResourceMemory] = q + } + if req.Resources.CPU != "" { + q, err := parseApplyPositiveQuantity(req.Resources.CPU, "resources.cpu") + if err != nil { + return nil, err + } + limits[corev1.ResourceCPU] = q + } + if req.Resources.CPURequest != "" { + q, err := parseApplyPositiveQuantity(req.Resources.CPURequest, "resources.cpuRequest") + if err != nil { + return nil, err + } + requests[corev1.ResourceCPU] = q + } + } + if err := validateResourceCeilings(requests, limits); err != nil { + return nil, err + } + memLim, ok := limits[corev1.ResourceMemory] + if !ok || memLim.IsZero() { + return nil, fmt.Errorf("internal error: refusing to create a server without a memory ceiling (§22)") + } + + // ---- storage ---- + storageQ, err := parseApplyPositiveQuantity(req.Storage, "storage") + if err != nil { + return nil, err + } + + return &v1alpha1.MinecraftServer{ + ObjectMeta: metav1.ObjectMeta{ + Name: req.Name, + Namespace: namespace, + }, + Spec: v1alpha1.MinecraftServerSpec{ + Subdomain: req.Subdomain, + DisplayName: req.DisplayName, + Image: req.Image, + JavaMemory: deriveApplyJavaHeap(memLim), + DesiredState: v1alpha1.DesiredStopped, + AutostartPolicy: policy, + Storage: v1alpha1.StorageSpec{Size: storageQ.String()}, + Resources: corev1.ResourceRequirements{Limits: limits, Requests: requests}, + }, + }, nil +} + +// checkSubdomainUnique lists all MinecraftServers in namespace and rejects the +// request if any CRD already carries the given spec.subdomain. metadata.name +// uniqueness is enforced by K8s on Create, but spec.subdomain must be checked +// here because two CRDs with different names could otherwise share a subdomain. +// It reuses the caller's context and K8s client — it never calls +// SetupSignalHandler or builds its own client. +func checkSubdomainUnique(ctx context.Context, cl client.Client, namespace, subdomain string) error { + var list v1alpha1.MinecraftServerList + if err := cl.List(ctx, &list, client.InNamespace(namespace)); err != nil { + return fmt.Errorf("list servers: %w", err) + } + for i := range list.Items { + if list.Items[i].Spec.Subdomain == subdomain { + return fmt.Errorf("subdomain %q is already in use by server %q", subdomain, list.Items[i].Name) + } + } + return nil +} + +// validateResourceCeilings checks that every resource request is ≤ its limit; +// Kubernetes would reject the CRD anyway, but we fail fast with a clear message. +func validateResourceCeilings(requests, limits corev1.ResourceList) error { + for name, lim := range limits { + req, ok := requests[name] + if !ok { + continue + } + if req.Cmp(lim) > 0 { + return fmt.Errorf("%s request %s exceeds limit %s", name, req.String(), lim.String()) + } + } + return nil +} + +// resourcesMemoryString returns the memory limit as a human-readable string +// for the success log. +func resourcesMemoryString(rr corev1.ResourceRequirements) string { + if m, ok := rr.Limits[corev1.ResourceMemory]; ok { + return m.String() + } + return "?" +} + +// ---- pure helpers (K8s-free, testable) ---- + +func parseApplyAutostartPolicy(s string) (v1alpha1.AutostartPolicy, error) { + switch s { + case "": + return v1alpha1.AutostartOwnerOnly, nil + case string(v1alpha1.AutostartOwnerOnly): + return v1alpha1.AutostartOwnerOnly, nil + case string(v1alpha1.AutostartPublic): + return v1alpha1.AutostartPublic, nil + case string(v1alpha1.AutostartAllowlist): + return v1alpha1.AutostartAllowlist, nil + default: + return "", fmt.Errorf("invalid autostartPolicy %q (want ownerOnly, public, or allowlist)", s) + } +} + +func parseApplyPositiveQuantity(s, field string) (resource.Quantity, error) { + q, err := resource.ParseQuantity(s) + if err != nil { + return resource.Quantity{}, fmt.Errorf("invalid %s quantity %q: %v", field, s, err) + } + if q.Sign() <= 0 { + return resource.Quantity{}, fmt.Errorf("%s must be a positive quantity", field) + } + return q, nil +} + +func deriveApplyJavaHeap(limit resource.Quantity) string { + const mib = int64(1024 * 1024) + bytes := limit.Value() + + reserve := bytes / 4 + if floor := 512 * mib; reserve < floor { + reserve = floor + } + if half := bytes / 2; reserve > half { + reserve = half + } + + heapMiB := (bytes - reserve) / mib + if heapMiB < 1 { + heapMiB = 1 + } + return fmt.Sprintf("%dM", heapMiB) +} diff --git a/cmd/felis/apply_test.go b/cmd/felis/apply_test.go new file mode 100644 index 0000000..c08e094 --- /dev/null +++ b/cmd/felis/apply_test.go @@ -0,0 +1,359 @@ +package main + +import ( + "encoding/json" + "strings" + "testing" + + "felis.lolicon.best/internal/apis/felis/v1alpha1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" +) + +// ---- policy parsing ---- + +func TestParseApplyAutostartPolicy(t *testing.T) { + tests := []struct { + input string + want v1alpha1.AutostartPolicy + ok bool + }{ + {"", v1alpha1.AutostartOwnerOnly, true}, + {"ownerOnly", v1alpha1.AutostartOwnerOnly, true}, + {"public", v1alpha1.AutostartPublic, true}, + {"allowlist", v1alpha1.AutostartAllowlist, true}, + {"bogus", "", false}, + {"Public", "", false}, + } + for _, tc := range tests { + got, err := parseApplyAutostartPolicy(tc.input) + if tc.ok { + if err != nil { + t.Errorf("parseApplyAutostartPolicy(%q) unexpected error: %v", tc.input, err) + } + if got != tc.want { + t.Errorf("parseApplyAutostartPolicy(%q) = %q, want %q", tc.input, got, tc.want) + } + } else { + if err == nil { + t.Errorf("parseApplyAutostartPolicy(%q) expected error, got %q", tc.input, got) + } + } + } +} + +// ---- quantity parsing ---- + +func TestParseApplyPositiveQuantity(t *testing.T) { + tests := []struct { + s string + field string + ok bool + want string + }{ + {"1Gi", "mem", true, "1Gi"}, + {"512Mi", "mem", true, "512Mi"}, + {"4G", "mem", true, "4G"}, + {"2048M", "mem", true, "2048M"}, + {"100m", "cpu", true, "100m"}, + {"2", "cpu", true, "2"}, + {"0", "mem", false, ""}, + {"-1", "mem", false, ""}, + {"abc", "mem", false, ""}, + } + for _, tc := range tests { + q, err := parseApplyPositiveQuantity(tc.s, tc.field) + if tc.ok { + if err != nil { + t.Errorf("parseApplyPositiveQuantity(%q, %q) unexpected error: %v", tc.s, tc.field, err) + continue + } + if q.String() != tc.want { + t.Errorf("parseApplyPositiveQuantity(%q, %q).String() = %q, want %q", tc.s, tc.field, q.String(), tc.want) + } + } else { + if err == nil { + t.Errorf("parseApplyPositiveQuantity(%q, %q) expected error", tc.s, tc.field) + } + } + } +} + +// ---- resource ceiling validation ---- + +func TestValidateResourceCeilings(t *testing.T) { + tests := []struct { + desc string + requests corev1.ResourceList + limits corev1.ResourceList + ok bool + }{ + {"equal", resList("memory=1Gi"), resList("memory=1Gi"), true}, + {"request_under", resList("memory=512Mi"), resList("memory=1Gi"), true}, + {"request_over", resList("memory=2Gi"), resList("memory=1Gi"), false}, + {"cpu_ok", resList("cpu=1"), resList("cpu=2"), true}, + {"cpu_over", resList("cpu=3"), resList("cpu=2"), false}, + {"no_request", nil, resList("memory=1Gi"), true}, + } + for _, tc := range tests { + err := validateResourceCeilings(tc.requests, tc.limits) + if tc.ok && err != nil { + t.Errorf("validateResourceCeilings(%s) unexpected error: %v", tc.desc, err) + } + if !tc.ok && err == nil { + t.Errorf("validateResourceCeilings(%s) expected error", tc.desc) + } + } +} + +// ---- JVM heap derivation ---- + +func TestDeriveApplyJavaHeap(t *testing.T) { + // reserve = max(bytes*0.25, 512MiB), capped at bytes*0.5 + tests := []struct { + limit string + want string + }{ + {"1Gi", "512M"}, // 1Gi: reserve=512Mi (floor) → heap=512Mi + {"2Gi", "1536M"}, // 2Gi: reserve=512Mi (floor,25%=512M=floor) → heap=1536Mi + {"4Gi", "3072M"}, // 4Gi: reserve=1024Mi (25%) → heap=3072Mi + {"8Gi", "6144M"}, // 8Gi: reserve=2048Mi (25%) → heap=6144Mi + {"3Gi", "2304M"}, // 3Gi: reserve=768Mi (25%) → heap=2304Mi + {"512Mi", "256M"}, // 512Mi: reserve=128Mi (25%) < floor=512Mi → reserve=256Mi (half cap) → heap=256Mi + {"768Mi", "384M"}, // 768Mi: reserve=192Mi (25%) < floor=512Mi → reserve=384Mi (half cap) → heap=384Mi + {"256Mi", "128M"}, // 256Mi: reserve=64Mi (25%) < floor=512Mi → capped at half 128Mi → heap=128Mi + } + for _, tc := range tests { + limit := resource.MustParse(tc.limit) + got := deriveApplyJavaHeap(limit) + if got != tc.want { + t.Errorf("deriveApplyJavaHeap(%s) = %q, want %q", tc.limit, got, tc.want) + } + } +} + +// ---- CRD builder ---- + +func TestBuildMinecraftServerFromApplyRequest_Valid(t *testing.T) { + req := applyRequest{ + Name: "test-server", + Subdomain: "test-server", + Image: "registry.felis.svc/paper:1.21", + Memory: "4Gi", + Storage: "20Gi", + } + ms, err := buildMinecraftServerFromApplyRequest(req, "minecraft") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if ms.Name != "test-server" { + t.Errorf("Name = %q, want test-server", ms.Name) + } + if ms.Namespace != "minecraft" { + t.Errorf("Namespace = %q, want minecraft", ms.Namespace) + } + if ms.Spec.Subdomain != "test-server" { + t.Errorf("Subdomain = %q", ms.Spec.Subdomain) + } + if ms.Spec.Image != "registry.felis.svc/paper:1.21" { + t.Errorf("Image = %q", ms.Spec.Image) + } + if ms.Spec.DesiredState != v1alpha1.DesiredStopped { + t.Errorf("DesiredState = %q, want Stopped", ms.Spec.DesiredState) + } + if ms.Spec.AutostartPolicy != v1alpha1.AutostartOwnerOnly { + t.Errorf("AutostartPolicy = %q, want ownerOnly", ms.Spec.AutostartPolicy) + } + if ms.Spec.Storage.Size != "20Gi" { + t.Errorf("Storage.Size = %q, want 20Gi", ms.Spec.Storage.Size) + } + mem, ok := ms.Spec.Resources.Limits[corev1.ResourceMemory] + if !ok { + t.Fatal("memory limit missing") + } + if mem.String() != "4Gi" { + t.Errorf("memory limit = %q, want 4Gi", mem.String()) + } + if ms.Spec.JavaMemory == "" { + t.Error("JavaMemory is empty") + } +} + +func TestBuildMinecraftServerFromApplyRequest_AutostartPolicy(t *testing.T) { + for _, p := range []string{"", "ownerOnly", "public", "allowlist"} { + req := applyRequest{ + Name: "srv", + Subdomain: "srv", + Image: "x", + Memory: "1Gi", + Storage: "10Gi", + AutostartPolicy: p, + } + ms, err := buildMinecraftServerFromApplyRequest(req, "ns") + if err != nil { + t.Errorf("unexpected error for policy %q: %v", p, err) + continue + } + want := v1alpha1.AutostartOwnerOnly + if p != "" { + want = v1alpha1.AutostartPolicy(p) + } + if ms.Spec.AutostartPolicy != want { + t.Errorf("AutostartPolicy = %q, want %q", ms.Spec.AutostartPolicy, want) + } + } +} + +func TestBuildMinecraftServerFromApplyRequest_Resources(t *testing.T) { + req := applyRequest{ + Name: "srv", + Subdomain: "srv", + Image: "x", + Memory: "2Gi", + Storage: "10Gi", + Resources: &resourceRequest{ + CPU: "2", + CPURequest: "1", + Memory: "4Gi", + MemoryRequest: "2Gi", + }, + } + ms, err := buildMinecraftServerFromApplyRequest(req, "ns") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + limits := ms.Spec.Resources.Limits + if cpu := limits[corev1.ResourceCPU]; cpu.String() != "2" { + t.Errorf("cpu limit = %q, want 2", cpu.String()) + } + if mem := limits[corev1.ResourceMemory]; mem.String() != "4Gi" { + t.Errorf("memory limit = %q, want 4Gi", mem.String()) + } + reqs := ms.Spec.Resources.Requests + if mem := reqs[corev1.ResourceMemory]; mem.String() != "2Gi" { + t.Errorf("memory request = %q, want 2Gi", mem.String()) + } + if cpu := reqs[corev1.ResourceCPU]; cpu.String() != "1" { + t.Errorf("cpu request = %q, want 1", cpu.String()) + } +} + +// ---- error cases ---- + +func TestBuildMinecraftServerFromApplyRequest_Errors(t *testing.T) { + const ok = "srv" // valid name to isolate the field under test + tests := []struct { + desc string + req applyRequest + errSubstr string + }{ + { + "empty name", + applyRequest{Name: "", Subdomain: ok, Image: "x", Memory: "1Gi", Storage: "1Gi"}, + "invalid name", + }, + { + "bad name chars", + applyRequest{Name: "BAD", Subdomain: ok, Image: "x", Memory: "1Gi", Storage: "1Gi"}, + "invalid name", + }, + { + "reserved name", + applyRequest{Name: "lobby", Subdomain: ok, Image: "x", Memory: "1Gi", Storage: "1Gi"}, + "reserved", + }, + { + "empty subdomain", + applyRequest{Name: ok, Subdomain: "", Image: "x", Memory: "1Gi", Storage: "1Gi"}, + "invalid subdomain", + }, + { + "empty image", + applyRequest{Name: ok, Subdomain: ok, Image: "", Memory: "1Gi", Storage: "1Gi"}, + "image is required", + }, + { + "whitespace-only image", + applyRequest{Name: ok, Subdomain: ok, Image: " ", Memory: "1Gi", Storage: "1Gi"}, + "image is required", + }, + { + "invalid policy", + applyRequest{Name: ok, Subdomain: ok, Image: "x", Memory: "1Gi", Storage: "1Gi", AutostartPolicy: "nope"}, + "invalid autostartPolicy", + }, + { + "zero memory", + applyRequest{Name: ok, Subdomain: ok, Image: "x", Memory: "0", Storage: "1Gi"}, + "must be a positive quantity", + }, + { + "invalid memory", + applyRequest{Name: ok, Subdomain: ok, Image: "x", Memory: "abc", Storage: "1Gi"}, + "invalid memory quantity", + }, + { + "zero storage", + applyRequest{Name: ok, Subdomain: ok, Image: "x", Memory: "1Gi", Storage: "0"}, + "must be a positive quantity", + }, + { + "invalid storage", + applyRequest{Name: ok, Subdomain: ok, Image: "x", Memory: "1Gi", Storage: "xyz"}, + "invalid storage quantity", + }, + { + "resource request > limit", + applyRequest{Name: ok, Subdomain: ok, Image: "x", Memory: "1Gi", Storage: "1Gi", + Resources: &resourceRequest{MemoryRequest: "2Gi"}}, + "exceeds limit", + }, + { + "cpu request > limit", + applyRequest{Name: ok, Subdomain: ok, Image: "x", Memory: "1Gi", Storage: "1Gi", + Resources: &resourceRequest{CPU: "1", CPURequest: "2"}}, + "exceeds limit", + }, + } + for _, tc := range tests { + t.Run(tc.desc, func(t *testing.T) { + _, err := buildMinecraftServerFromApplyRequest(tc.req, "ns") + if err == nil { + t.Fatalf("expected error containing %q, got nil", tc.errSubstr) + } + if !strings.Contains(err.Error(), tc.errSubstr) { + t.Errorf("error = %q, want it to contain %q", err.Error(), tc.errSubstr) + } + }) + } +} + +// ---- JSON unknown-field rejection ---- + +func TestApplyRequestRejectsUnknownFields(t *testing.T) { + raw := `{"name":"srv","subdomain":"srv","image":"x","memory":"1Gi","storage":"1Gi","bogusField":true}` + dec := json.NewDecoder(strings.NewReader(raw)) + dec.DisallowUnknownFields() + var req applyRequest + err := dec.Decode(&req) + if err == nil { + t.Fatal("expected unknown-field error, got nil") + } + if !strings.Contains(err.Error(), "bogusField") { + t.Errorf("error = %q, should mention the unknown field", err) + } +} + +// ---- helpers ---- + +func resList(specs ...string) corev1.ResourceList { + rl := corev1.ResourceList{} + for _, s := range specs { + name, val, _ := strings.Cut(s, "=") + if name == "" { + continue + } + rl[corev1.ResourceName(name)] = resource.MustParse(val) + } + return rl +} diff --git a/cmd/felis/run.go b/cmd/felis/run.go index ad21ba5..5e5415d 100644 --- a/cmd/felis/run.go +++ b/cmd/felis/run.go @@ -17,7 +17,7 @@ Commands: reaper Run the world reaper / backup batch restore Extract a world archive into a world volume (internal Job entrypoint) manifests Render the control-plane RBAC + NetworkPolicy install bundle as YAML - apply Apply a MinecraftServer manifest + apply Create a MinecraftServer CRD (direct K8s write; use -f server.json) breakGlass Open the local break-glass emergency console (TUI; requires root/sudo) Run "felis -h" for command-specific flags. @@ -45,7 +45,7 @@ func run(args []string, stdout, stderr io.Writer) int { case "manifests": return cmdManifests(rest, stdout, stderr) case "apply": - return notImplemented("apply", "MinecraftServer manifest apply", stderr) + return cmdApply(rest, stdout, stderr) case "breakGlass": return cmdBreakGlass(rest, stdout, stderr) case "-h", "--help", "help": @@ -57,10 +57,4 @@ func run(args []string, stdout, stderr io.Writer) int { } } -// notImplemented reports a subcommand that is wired into the CLI surface but -// whose implementation lands in a later phase. It fails loudly rather than -// pretending to do work. -func notImplemented(name, desc string, stderr io.Writer) int { - fmt.Fprintf(stderr, "felis %s: not implemented yet — %s\n", name, desc) - return 3 -} + diff --git a/cmd/felis/run_test.go b/cmd/felis/run_test.go index 64df26b..b3b9ad7 100644 --- a/cmd/felis/run_test.go +++ b/cmd/felis/run_test.go @@ -36,15 +36,31 @@ func TestRunUnknownCommand(t *testing.T) { } } -func TestRunNotImplementedSubcommands(t *testing.T) { - for _, cmd := range []string{"apply"} { - var out, errBuf bytes.Buffer - if code := run([]string{cmd}, &out, &errBuf); code != 3 { - t.Errorf("%s exit code = %d, want 3", cmd, code) - } - if !strings.Contains(errBuf.String(), "not implemented yet") { - t.Errorf("%s: expected not-implemented notice, got %q", cmd, errBuf.String()) - } +func TestRunApplyRequiresFileFlag(t *testing.T) { + // Without -f the command must fail with usage (2), not try to contact a + // cluster. It can't return 0 because no CRD was created, and it can't return 1 + // because that would be ambiguous with a real server-side failure. + var out, errBuf bytes.Buffer + code := run([]string{"apply"}, &out, &errBuf) + if code != 2 { + t.Errorf("exit code = %d, want 2", code) + } + if !strings.Contains(errBuf.String(), "missing required flag -f") { + t.Errorf("expected -f usage, got %q", errBuf.String()) + } +} + +func TestRunApplyRejectsInvalidJSON(t *testing.T) { + // Sending garbage via a temp file must exit 1 (input error), not panic or hang. + var out, errBuf bytes.Buffer + code := run([]string{"apply", "-f", "/dev/null"}, &out, &errBuf) + // /dev/null is empty → JSON parse fails or validation rejects the zero values; + // either way it must exit 1, not panic. + if code != 1 { + t.Errorf("exit code = %d, want 1", code) + } + if !strings.Contains(errBuf.String(), "invalid") && !strings.Contains(errBuf.String(), "required") { + t.Errorf("expected input error, got %q", errBuf.String()) } }