From 44e91ac0d3c9cea8aa381036c4f668fe55c533a2 Mon Sep 17 00:00:00 2001 From: Onur Date: Mon, 2 Mar 2026 23:08:52 +0100 Subject: [PATCH] fix(owner): make spritz ownership id-only --- api/main.go | 3 - api/main_create_owner_test.go | 103 +++++++++++++++++++++++ crd/generated/spritz.sh_spritzes.yaml | 3 - crd/spritz.sh_spritzes.yaml | 3 - helm/spritz/crds/spritz.sh_spritzes.yaml | 42 ++++++--- operator/api/v1/spritz_types.go | 4 +- 6 files changed, 134 insertions(+), 24 deletions(-) create mode 100644 api/main_create_owner_test.go diff --git a/api/main.go b/api/main.go index ce2a795..426d654 100644 --- a/api/main.go +++ b/api/main.go @@ -293,9 +293,6 @@ func (s *server) createSpritz(c echo.Context) error { return writeError(c, http.StatusBadRequest, "spec.owner.id is required") } } - if owner.Email == "" { - owner.Email = principal.Email - } if s.auth.enabled() && !principal.IsAdmin && owner.ID != principal.ID { return writeError(c, http.StatusForbidden, "owner mismatch") } diff --git a/api/main_create_owner_test.go b/api/main_create_owner_test.go new file mode 100644 index 0000000..2e8d847 --- /dev/null +++ b/api/main_create_owner_test.go @@ -0,0 +1,103 @@ +package main + +import ( + "bytes" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/labstack/echo/v4" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + spritzv1 "spritz.sh/operator/api/v1" +) + +func newTestSpritzScheme(t *testing.T) *runtime.Scheme { + t.Helper() + scheme := runtime.NewScheme() + if err := spritzv1.AddToScheme(scheme); err != nil { + t.Fatalf("failed to register spritz scheme: %v", err) + } + return scheme +} + +func newCreateSpritzTestServer(t *testing.T) *server { + t.Helper() + scheme := newTestSpritzScheme(t) + return &server{ + client: fake.NewClientBuilder().WithScheme(scheme).Build(), + scheme: scheme, + namespace: "spritz-test", + auth: authConfig{ + mode: authModeHeader, + headerID: "X-Spritz-User-Id", + headerEmail: "X-Spritz-User-Email", + }, + internalAuth: internalAuthConfig{enabled: false}, + userConfigPolicy: userConfigPolicy{}, + } +} + +func TestCreateSpritzOwnerUsesIDAndOmitsEmail(t *testing.T) { + s := newCreateSpritzTestServer(t) + e := echo.New() + secured := e.Group("", s.authMiddleware()) + secured.POST("/api/spritzes", s.createSpritz) + + body := []byte(`{"name":"tidal-ember","spec":{"image":"example.com/spritz:latest"}}`) + req := httptest.NewRequest(http.MethodPost, "/api/spritzes", bytes.NewReader(body)) + req.Header.Set(echo.HeaderContentType, echo.MIMEApplicationJSON) + req.Header.Set("X-Spritz-User-Id", "bcf6c03e-51a1-4f05-97d8-d616405b42a2") + req.Header.Set("X-Spritz-User-Email", "bcf6c03e-51a1-4f05-97d8-d616405b42a2") + rec := httptest.NewRecorder() + + e.ServeHTTP(rec, req) + + if rec.Code != http.StatusCreated { + t.Fatalf("expected status 201, got %d: %s", rec.Code, rec.Body.String()) + } + + var payload map[string]any + if err := json.Unmarshal(rec.Body.Bytes(), &payload); err != nil { + t.Fatalf("failed to decode response json: %v", err) + } + data, ok := payload["data"].(map[string]any) + if !ok { + t.Fatalf("expected data object in response, got %#v", payload["data"]) + } + spec, ok := data["spec"].(map[string]any) + if !ok { + t.Fatalf("expected spec object in response, got %#v", data["spec"]) + } + owner, ok := spec["owner"].(map[string]any) + if !ok { + t.Fatalf("expected owner object in response, got %#v", spec["owner"]) + } + if owner["id"] != "bcf6c03e-51a1-4f05-97d8-d616405b42a2" { + t.Fatalf("expected owner.id to be principal id, got %#v", owner["id"]) + } + if _, exists := owner["email"]; exists { + t.Fatalf("expected owner.email to be omitted from response, got %#v", owner["email"]) + } +} + +func TestCreateSpritzRejectsOwnerIDMismatchForNonAdmin(t *testing.T) { + s := newCreateSpritzTestServer(t) + e := echo.New() + secured := e.Group("", s.authMiddleware()) + secured.POST("/api/spritzes", s.createSpritz) + + body := []byte(`{"name":"tidal-ember","spec":{"image":"example.com/spritz:latest","owner":{"id":"someone-else"}}}`) + req := httptest.NewRequest(http.MethodPost, "/api/spritzes", bytes.NewReader(body)) + req.Header.Set(echo.HeaderContentType, echo.MIMEApplicationJSON) + req.Header.Set("X-Spritz-User-Id", "current-user") + rec := httptest.NewRecorder() + + e.ServeHTTP(rec, req) + + if rec.Code != http.StatusForbidden { + t.Fatalf("expected status 403, got %d: %s", rec.Code, rec.Body.String()) + } +} diff --git a/crd/generated/spritz.sh_spritzes.yaml b/crd/generated/spritz.sh_spritzes.yaml index 2d13b7e..344c25a 100644 --- a/crd/generated/spritz.sh_spritzes.yaml +++ b/crd/generated/spritz.sh_spritzes.yaml @@ -268,9 +268,6 @@ spec: owner: description: SpritzOwner identifies the creator of a spritz. properties: - email: - format: email - type: string id: minLength: 1 type: string diff --git a/crd/spritz.sh_spritzes.yaml b/crd/spritz.sh_spritzes.yaml index 2d13b7e..344c25a 100644 --- a/crd/spritz.sh_spritzes.yaml +++ b/crd/spritz.sh_spritzes.yaml @@ -268,9 +268,6 @@ spec: owner: description: SpritzOwner identifies the creator of a spritz. properties: - email: - format: email - type: string id: minLength: 1 type: string diff --git a/helm/spritz/crds/spritz.sh_spritzes.yaml b/helm/spritz/crds/spritz.sh_spritzes.yaml index 0337451..344c25a 100644 --- a/helm/spritz/crds/spritz.sh_spritzes.yaml +++ b/helm/spritz/crds/spritz.sh_spritzes.yaml @@ -56,9 +56,6 @@ spec: type: object spec: description: SpritzSpec defines the desired state of Spritz. - x-kubernetes-validations: - - message: spec.repo and spec.repos are mutually exclusive - rule: '!(has(self.repo) && has(self.repos) && size(self.repos) > 0)' properties: annotations: additionalProperties: @@ -271,9 +268,6 @@ spec: owner: description: SpritzOwner identifies the creator of a spritz. properties: - email: - format: email - type: string id: minLength: 1 type: string @@ -356,19 +350,17 @@ spec: - url type: object repos: - description: Repos holds multiple repositories to clone. When set, - repo is ignored. items: - description: SpritzRepo describes the repository to clone inside the - workload. + description: SpritzRepo describes the repository to clone inside + the workload. properties: auth: description: SpritzRepoAuth describes how to authenticate git clone operations. properties: netrcKey: - description: NetrcKey points to a Secret key containing a - full .netrc file. + description: NetrcKey points to a Secret key containing + a full .netrc file. type: string passwordKey: description: PasswordKey points to a Secret key containing @@ -461,6 +453,29 @@ spec: More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ type: object type: object + sharedMounts: + description: SharedMounts configures per-spritz shared directories. + items: + properties: + mode: + type: string + mountPath: + type: string + name: + type: string + pollSeconds: + type: integer + publishSeconds: + type: integer + scope: + type: string + syncMode: + type: string + required: + - mountPath + - name + type: object + type: array ssh: description: SpritzSSH configures SSH access behavior. properties: @@ -500,6 +515,9 @@ spec: - image - owner type: object + x-kubernetes-validations: + - message: spec.repo and spec.repos are mutually exclusive + rule: '!(has(self.repo) && has(self.repos) && size(self.repos) > 0)' status: description: SpritzStatus defines the observed state of Spritz. properties: diff --git a/operator/api/v1/spritz_types.go b/operator/api/v1/spritz_types.go index 9cafcab..691f5e3 100644 --- a/operator/api/v1/spritz_types.go +++ b/operator/api/v1/spritz_types.go @@ -61,9 +61,7 @@ type SpritzRepoAuth struct { type SpritzOwner struct { // +kubebuilder:validation:MinLength=1 ID string `json:"id"` - // +kubebuilder:validation:Format=email - Email string `json:"email,omitempty"` - Team string `json:"team,omitempty"` + Team string `json:"team,omitempty"` } // SpritzFeatures toggles optional capabilities.