From 5f1b56b923b45f4bfe81c35b258aa7fb6ea511b4 Mon Sep 17 00:00:00 2001 From: Onur Date: Tue, 3 Mar 2026 12:05:55 +0100 Subject: [PATCH 1/2] refactor(operator): remove pvc-based storage paths --- ...6-02-24-simplest-spritz-deployment-spec.md | 14 +- .../spritz/templates/operator-deployment.yaml | 32 -- helm/spritz/values.yaml | 17 - operator/controllers/home_mounts_test.go | 143 +++++++ operator/controllers/home_pvc_test.go | 255 ------------- .../controllers/shared_config_pvc_test.go | 39 -- operator/controllers/spritz_controller.go | 348 +----------------- 7 files changed, 166 insertions(+), 682 deletions(-) create mode 100644 operator/controllers/home_mounts_test.go delete mode 100644 operator/controllers/home_pvc_test.go delete mode 100644 operator/controllers/shared_config_pvc_test.go diff --git a/docs/2026-02-24-simplest-spritz-deployment-spec.md b/docs/2026-02-24-simplest-spritz-deployment-spec.md index a1bcdbb..c54ed78 100644 --- a/docs/2026-02-24-simplest-spritz-deployment-spec.md +++ b/docs/2026-02-24-simplest-spritz-deployment-spec.md @@ -66,7 +66,6 @@ The default installation should require only: - `global.ingress.className`: ingress class - `global.ingress.tls.enabled`: whether TLS is enabled - `global.ingress.tls.secretName` (optional): pre-provisioned TLS secret name -- `operator.homePVC.storageClass` (optional): home PVC storage class override Everything else should have working defaults. @@ -98,10 +97,6 @@ ui: apiBaseUrl: /api operator: - homePVC: - enabled: true - storageClassName: standard - sharedMounts: enabled: false @@ -150,7 +145,6 @@ File: `helm/spritz/values.yaml` - Add `global.ingress.tls.secretName` (default empty; operator-provided). - Keep `ui.ingress.enabled` default `true` for single-host installs. - Keep `ui.apiBaseUrl` default `/api`. -- Keep `operator.homePVC.enabled` default `true`. - Keep `operator.sharedMounts.enabled` and `api.sharedMounts.enabled` default `false`. - Remove compatibility-only keys from the default path: - `ui.ingress.host` @@ -224,15 +218,15 @@ Required behavior: ## Storage and Sync Defaults -- Default mode is per-devbox persistent home PVC. +- Default mode is ephemeral home storage (`EmptyDir` at `/home/dev`). - Shared cross-devbox live sync is disabled by default. - Shared mounts remain available as an opt-in advanced feature. Rationale: -- PVC-only mode has fewer failure modes. -- This is enough for most single-devbox usage. -- Operators can enable shared sync only when they need it. +- Ephemeral defaults keep install and cleanup behavior predictable. +- This is enough for stateless single-devbox usage. +- Operators can enable shared sync only for specific paths they need to persist. ## Optional Advanced Mode diff --git a/helm/spritz/templates/operator-deployment.yaml b/helm/spritz/templates/operator-deployment.yaml index 7d8809d..904353b 100644 --- a/helm/spritz/templates/operator-deployment.yaml +++ b/helm/spritz/templates/operator-deployment.yaml @@ -43,38 +43,6 @@ spec: - name: SPRITZ_HOME_SIZE_LIMIT value: {{ .Values.operator.homeSizeLimit | quote }} {{- end }} - {{- if and .Values.operator.homePVC .Values.operator.homePVC.enabled }} - - name: SPRITZ_HOME_PVC_PREFIX - value: {{ .Values.operator.homePVC.prefix | quote }} - - name: SPRITZ_HOME_PVC_SIZE - value: {{ .Values.operator.homePVC.size | quote }} - - name: SPRITZ_HOME_PVC_ACCESS_MODES - value: {{ join "," .Values.operator.homePVC.accessModes | quote }} - {{- if .Values.operator.homePVC.storageClass }} - - name: SPRITZ_HOME_PVC_STORAGE_CLASS - value: {{ .Values.operator.homePVC.storageClass | quote }} - {{- end }} - {{- if .Values.operator.homePVC.mountPaths }} - - name: SPRITZ_HOME_MOUNT_PATHS - value: {{ join "," .Values.operator.homePVC.mountPaths | quote }} - {{- end }} - {{- end }} - {{- if and .Values.operator.sharedConfigPVC .Values.operator.sharedConfigPVC.enabled }} - - name: SPRITZ_SHARED_CONFIG_PVC_PREFIX - value: {{ .Values.operator.sharedConfigPVC.prefix | quote }} - - name: SPRITZ_SHARED_CONFIG_PVC_SIZE - value: {{ .Values.operator.sharedConfigPVC.size | quote }} - - name: SPRITZ_SHARED_CONFIG_PVC_ACCESS_MODES - value: {{ join "," .Values.operator.sharedConfigPVC.accessModes | quote }} - {{- if .Values.operator.sharedConfigPVC.storageClass }} - - name: SPRITZ_SHARED_CONFIG_PVC_STORAGE_CLASS - value: {{ .Values.operator.sharedConfigPVC.storageClass | quote }} - {{- end }} - {{- if .Values.operator.sharedConfigPVC.mountPath }} - - name: SPRITZ_SHARED_CONFIG_MOUNT_PATH - value: {{ .Values.operator.sharedConfigPVC.mountPath | quote }} - {{- end }} - {{- end }} {{- if and .Values.operator.sharedMounts .Values.operator.sharedMounts.enabled }} - name: SPRITZ_SHARED_MOUNTS value: {{ toJson .Values.operator.sharedMounts.mounts | quote }} diff --git a/helm/spritz/values.yaml b/helm/spritz/values.yaml index 8e0744d..2e62f26 100644 --- a/helm/spritz/values.yaml +++ b/helm/spritz/values.yaml @@ -77,23 +77,6 @@ operator: workspaceSizeLimit: 10Gi homeSizeLimit: 5Gi podNodeSelector: "" - homePVC: - enabled: true - prefix: spritz-home - size: 5Gi - accessModes: - - ReadWriteOnce - storageClass: "" - mountPaths: - - /home/dev - sharedConfigPVC: - enabled: false - prefix: spritz-shared-config - size: 100Mi - accessModes: - - ReadWriteMany - storageClass: "" - mountPath: /shared sharedMounts: enabled: false mounts: [] diff --git a/operator/controllers/home_mounts_test.go b/operator/controllers/home_mounts_test.go new file mode 100644 index 0000000..22581c8 --- /dev/null +++ b/operator/controllers/home_mounts_test.go @@ -0,0 +1,143 @@ +package controllers + +import ( + "testing" + + corev1 "k8s.io/api/core/v1" + + spritzv1 "spritz.sh/operator/api/v1" +) + +func TestParseCSV(t *testing.T) { + if parseCSV("") != nil { + t.Fatal("expected nil for empty CSV") + } + got := parseCSV("/home/dev, /home/spritz") + if len(got) != 2 { + t.Fatalf("expected 2 entries, got %d", len(got)) + } + if got[0] != "/home/dev" || got[1] != "/home/spritz" { + t.Fatalf("unexpected values: %v", got) + } +} + +func TestParseNodeSelector(t *testing.T) { + selector, err := parseNodeSelector("spritz.sh/storage-ready=true,zone=fsn1") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if selector["spritz.sh/storage-ready"] != "true" || selector["zone"] != "fsn1" { + t.Fatalf("unexpected selector: %v", selector) + } + + if _, err := parseNodeSelector("missingequals"); err == nil { + t.Fatal("expected error for invalid selector entry") + } + if _, err := parseNodeSelector("=novalue"); err == nil { + t.Fatal("expected error for empty key") + } + if _, err := parseNodeSelector("key="); err == nil { + t.Fatal("expected error for empty value") + } +} + +func TestBuildHomeMountsDefault(t *testing.T) { + mounts := buildHomeMounts() + if len(mounts) != 1 { + t.Fatalf("expected 1 mount, got %d", len(mounts)) + } + if mounts[0].Name != "home" { + t.Fatalf("unexpected mount name: %s", mounts[0].Name) + } + if mounts[0].MountPath != repoInitHomeDir { + t.Fatalf("unexpected mount path: %s", mounts[0].MountPath) + } +} + +func TestBuildPodSecurityContext(t *testing.T) { + if ctx := buildPodSecurityContext(false, false); ctx != nil { + t.Fatal("expected nil security context when no shared mounts or repo init") + } + + ctx := buildPodSecurityContext(true, false) + if ctx == nil || ctx.FSGroup == nil || *ctx.FSGroup != repoInitGroupID { + t.Fatalf("expected fsGroup %d when shared mounts enabled, got %+v", repoInitGroupID, ctx) + } + + ctx = buildPodSecurityContext(false, true) + if ctx == nil || ctx.FSGroup == nil || *ctx.FSGroup != repoInitGroupID { + t.Fatalf("expected fsGroup %d when repo init present, got %+v", repoInitGroupID, ctx) + } +} + +func TestBuildRepoInitContainerDedupesHomeMount(t *testing.T) { + spritz := &spritzv1.Spritz{ + Spec: spritzv1.SpritzSpec{ + Repo: &spritzv1.SpritzRepo{ + URL: "https://github.com/example/repo.git", + }, + }, + } + + homeMounts := buildHomeMounts() + repos := repoEntries(spritz) + containers, _, err := buildRepoInitContainers(spritz, repos, homeMounts) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(containers) == 0 { + t.Fatal("expected repo init container") + } + + count := 0 + for _, mount := range containers[0].VolumeMounts { + if mount.MountPath == repoInitHomeDir { + count++ + } + } + if count != 1 { + t.Fatalf("expected single %s mount, got %d", repoInitHomeDir, count) + } +} + +func TestRepoDirNeedsWorkspaceMountHonorsSharedMounts(t *testing.T) { + mountRoots := []corev1.VolumeMount{ + {Name: "shared", MountPath: "/shared"}, + } + if repoDirNeedsWorkspaceMount("/shared/repo", mountRoots) { + t.Fatal("expected repo dir under shared mount to skip workspace mount") + } + if repoDirNeedsWorkspaceMount("/workspace/repo", mountRoots) { + t.Fatal("expected repo dir under /workspace to skip workspace mount") + } +} + +func TestValidateRepoDir(t *testing.T) { + cases := []struct { + name string + dir string + wantErr bool + }{ + {"empty ok", "", false}, + {"relative ok", "spritz", false}, + {"relative nested ok", "project/app", false}, + {"relative up invalid", "../etc", true}, + {"relative up nested invalid", "foo/../../etc", true}, + {"absolute workspace ok", "/workspace/spritz", false}, + {"absolute workspace nested ok", "/workspace/spritz/app", false}, + {"absolute escape invalid", "/etc", true}, + {"absolute escape via traversal invalid", "/workspace/../etc", true}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + err := validateRepoDir(tc.dir) + if tc.wantErr && err == nil { + t.Fatalf("expected error for %s", tc.dir) + } + if !tc.wantErr && err != nil { + t.Fatalf("unexpected error for %s: %v", tc.dir, err) + } + }) + } +} diff --git a/operator/controllers/home_pvc_test.go b/operator/controllers/home_pvc_test.go deleted file mode 100644 index c1d1491..0000000 --- a/operator/controllers/home_pvc_test.go +++ /dev/null @@ -1,255 +0,0 @@ -package controllers - -import ( - "context" - "strings" - "testing" - "time" - - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/resource" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "sigs.k8s.io/controller-runtime/pkg/client/fake" - - spritzv1 "spritz.sh/operator/api/v1" -) - -func TestParseCSV(t *testing.T) { - if parseCSV("") != nil { - t.Fatal("expected nil for empty CSV") - } - got := parseCSV("/home/dev, /home/spritz") - if len(got) != 2 { - t.Fatalf("expected 2 entries, got %d", len(got)) - } - if got[0] != "/home/dev" || got[1] != "/home/spritz" { - t.Fatalf("unexpected values: %v", got) - } -} - -func TestParseNodeSelector(t *testing.T) { - selector, err := parseNodeSelector("spritz.sh/storage-ready=true,zone=fsn1") - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if selector["spritz.sh/storage-ready"] != "true" || selector["zone"] != "fsn1" { - t.Fatalf("unexpected selector: %v", selector) - } - - if _, err := parseNodeSelector("missingequals"); err == nil { - t.Fatal("expected error for invalid selector entry") - } - if _, err := parseNodeSelector("=novalue"); err == nil { - t.Fatal("expected error for empty key") - } - if _, err := parseNodeSelector("key="); err == nil { - t.Fatal("expected error for empty value") - } -} - -func TestParseAccessModes(t *testing.T) { - modes := parseAccessModes("rwo,ReadWriteMany,rox") - if len(modes) != 3 { - t.Fatalf("expected 3 access modes, got %d", len(modes)) - } -} - -func TestBuildHomeMountsDefault(t *testing.T) { - mounts := buildHomeMounts(homePVCSettings{}) - if len(mounts) != 1 { - t.Fatalf("expected 1 mount, got %d", len(mounts)) - } - if mounts[0].MountPath != "/home/dev" { - t.Fatalf("unexpected mount path: %s", mounts[0].MountPath) - } -} - -func TestBuildHomeMountsDedup(t *testing.T) { - settings := homePVCSettings{mountPaths: []string{"/home/dev", "/home/dev", "/home/spritz"}} - mounts := buildHomeMounts(settings) - if len(mounts) != 2 { - t.Fatalf("expected 2 mounts, got %d", len(mounts)) - } - paths := []string{mounts[0].MountPath, mounts[1].MountPath} - if !containsPath(paths, "/home/dev") || !containsPath(paths, "/home/spritz") { - t.Fatalf("unexpected mounts: %v", paths) - } -} - -func TestValidateMountPaths(t *testing.T) { - if err := validateMountPaths([]string{"/home/dev", "/home/spritz"}); err != nil { - t.Fatalf("unexpected error: %v", err) - } - if err := validateMountPaths([]string{"/home", "/home/spritz"}); err == nil { - t.Fatal("expected overlap error") - } - if err := validateMountPaths([]string{"home"}); err == nil { - t.Fatal("expected absolute path error") - } -} - -func TestLoadHomePVCSettingsDisabled(t *testing.T) { - t.Setenv("SPRITZ_HOME_PVC_PREFIX", "") - settings := loadHomePVCSettings() - if settings.enabled { - t.Fatal("expected home PVC disabled") - } - if len(settings.mountPaths) != 0 { - t.Fatalf("expected no mount paths, got %v", settings.mountPaths) - } -} - -func TestOwnerPVCName(t *testing.T) { - name := ownerPVCName("spritz-home", "user-123") - if !strings.HasPrefix(name, "spritz-home-owner-") { - t.Fatalf("unexpected pvc name: %s", name) - } -} - -func TestEnsureHomePVCAllowsPending(t *testing.T) { - scheme := runtime.NewScheme() - if err := corev1.AddToScheme(scheme); err != nil { - t.Fatalf("failed to add core scheme: %v", err) - } - if err := spritzv1.AddToScheme(scheme); err != nil { - t.Fatalf("failed to add spritz scheme: %v", err) - } - - now := metav1.NewTime(time.Now()) - pvc := &corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Name: "spritz-home-owner-test", - Namespace: "spritz", - CreationTimestamp: now, - }, - Status: corev1.PersistentVolumeClaimStatus{ - Phase: corev1.ClaimPending, - }, - } - - client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(pvc).Build() - reconciler := &SpritzReconciler{Client: client} - - settings := homePVCSettings{ - accessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, - size: resource.MustParse("1Gi"), - } - spritz := &spritzv1.Spritz{ - ObjectMeta: metav1.ObjectMeta{Name: "devbox1", Namespace: "spritz"}, - Spec: spritzv1.SpritzSpec{ - Owner: spritzv1.SpritzOwner{ID: "user-123"}, - }, - } - - if err := reconciler.ensureHomePVC(context.Background(), spritz, pvc.Name, settings); err != nil { - t.Fatalf("expected pending PVC to be allowed, got error: %v", err) - } -} - -func TestBuildPodSecurityContext(t *testing.T) { - if ctx := buildPodSecurityContext(false, false, false, false); ctx != nil { - t.Fatal("expected nil security context when no repo init or home PVC") - } - - ctx := buildPodSecurityContext(true, false, false, false) - if ctx == nil || ctx.FSGroup == nil || *ctx.FSGroup != repoInitGroupID { - t.Fatalf("expected fsGroup %d when home PVC enabled, got %+v", repoInitGroupID, ctx) - } - - ctx = buildPodSecurityContext(false, false, false, true) - if ctx == nil || ctx.FSGroup == nil || *ctx.FSGroup != repoInitGroupID { - t.Fatalf("expected fsGroup %d when repo init present, got %+v", repoInitGroupID, ctx) - } - - ctx = buildPodSecurityContext(false, true, false, false) - if ctx == nil || ctx.FSGroup == nil || *ctx.FSGroup != repoInitGroupID { - t.Fatalf("expected fsGroup %d when shared config PVC enabled, got %+v", repoInitGroupID, ctx) - } - - ctx = buildPodSecurityContext(false, false, true, false) - if ctx == nil || ctx.FSGroup == nil || *ctx.FSGroup != repoInitGroupID { - t.Fatalf("expected fsGroup %d when shared mounts enabled, got %+v", repoInitGroupID, ctx) - } -} - -func TestBuildRepoInitContainerDedupesHomeMount(t *testing.T) { - spritz := &spritzv1.Spritz{ - Spec: spritzv1.SpritzSpec{ - Repo: &spritzv1.SpritzRepo{ - URL: "https://github.com/example/repo.git", - }, - }, - } - - homeMounts := buildHomeMounts(homePVCSettings{}) - repos := repoEntries(spritz) - containers, _, err := buildRepoInitContainers(spritz, repos, homeMounts) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if len(containers) == 0 { - t.Fatal("expected repo init container") - } - - count := 0 - for _, mount := range containers[0].VolumeMounts { - if mount.MountPath == repoInitHomeDir { - count++ - } - } - if count != 1 { - t.Fatalf("expected single %s mount, got %d", repoInitHomeDir, count) - } -} - -func TestRepoDirNeedsWorkspaceMountHonorsSharedMounts(t *testing.T) { - mountRoots := []corev1.VolumeMount{ - {Name: "shared", MountPath: "/shared"}, - } - if repoDirNeedsWorkspaceMount("/shared/repo", mountRoots) { - t.Fatal("expected repo dir under shared mount to skip workspace mount") - } - if repoDirNeedsWorkspaceMount("/workspace/repo", mountRoots) { - t.Fatal("expected repo dir under /workspace to skip workspace mount") - } -} - -func TestValidateRepoDir(t *testing.T) { - cases := []struct { - name string - dir string - wantErr bool - }{ - {"empty ok", "", false}, - {"relative ok", "spritz", false}, - {"relative nested ok", "project/app", false}, - {"relative up invalid", "../etc", true}, - {"relative up nested invalid", "foo/../../etc", true}, - {"absolute workspace ok", "/workspace/spritz", false}, - {"absolute workspace nested ok", "/workspace/spritz/app", false}, - {"absolute escape invalid", "/etc", true}, - {"absolute escape via traversal invalid", "/workspace/../etc", true}, - } - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - err := validateRepoDir(tc.dir) - if tc.wantErr && err == nil { - t.Fatalf("expected error for %s", tc.dir) - } - if !tc.wantErr && err != nil { - t.Fatalf("unexpected error for %s: %v", tc.dir, err) - } - }) - } -} - -func containsPath(paths []string, value string) bool { - for _, path := range paths { - if path == value { - return true - } - } - return false -} diff --git a/operator/controllers/shared_config_pvc_test.go b/operator/controllers/shared_config_pvc_test.go deleted file mode 100644 index da8d582..0000000 --- a/operator/controllers/shared_config_pvc_test.go +++ /dev/null @@ -1,39 +0,0 @@ -package controllers - -import ( - "testing" - - corev1 "k8s.io/api/core/v1" -) - -func TestValidateSharedConfigMountPath(t *testing.T) { - if err := validateSharedConfigMountPath(""); err == nil { - t.Fatal("expected error for empty mount path") - } - if err := validateSharedConfigMountPath("shared"); err == nil { - t.Fatal("expected error for relative mount path") - } - if err := validateSharedConfigMountPath("/"); err == nil { - t.Fatal("expected error for root mount path") - } - if err := validateSharedConfigMountPath("/shared"); err != nil { - t.Fatalf("unexpected error for /shared: %v", err) - } -} - -func TestLoadSharedConfigPVCSettingsDefaults(t *testing.T) { - t.Setenv("SPRITZ_SHARED_CONFIG_PVC_PREFIX", "spritz-shared") - t.Setenv("SPRITZ_SHARED_CONFIG_PVC_ACCESS_MODES", "") - t.Setenv("SPRITZ_SHARED_CONFIG_MOUNT_PATH", "") - - settings := loadSharedConfigPVCSettings() - if !settings.enabled { - t.Fatal("expected shared config PVC to be enabled") - } - if settings.mountPath != "/shared" { - t.Fatalf("expected default mount path /shared, got %s", settings.mountPath) - } - if len(settings.accessModes) != 1 || settings.accessModes[0] != corev1.ReadWriteMany { - t.Fatalf("expected default access mode RWX, got %+v", settings.accessModes) - } -} diff --git a/operator/controllers/spritz_controller.go b/operator/controllers/spritz_controller.go index b2cf854..2dc2b70 100644 --- a/operator/controllers/spritz_controller.go +++ b/operator/controllers/spritz_controller.go @@ -29,45 +29,25 @@ import ( ) const ( - defaultRepoDir = "/workspace/repo" - defaultWebPort = int32(8080) - defaultSSHPort = int32(22) - defaultSSHUser = "spritz" - defaultSSHMode = "service" - spritzContainerName = "spritz" - spritzFinalizer = "spritz.sh/finalizer" - defaultTTLGrace = 5 * time.Minute - defaultRepoInitImage = "alpine/git:2.45.2" - repoAuthMountPath = "/var/run/spritz/repo-auth" - repoInitHomeDir = "/home/dev" - repoInitGroupID int64 = 65532 - defaultSharedConfigMount = "/shared" + defaultRepoDir = "/workspace/repo" + defaultWebPort = int32(8080) + defaultSSHPort = int32(22) + defaultSSHUser = "spritz" + defaultSSHMode = "service" + spritzContainerName = "spritz" + spritzFinalizer = "spritz.sh/finalizer" + defaultTTLGrace = 5 * time.Minute + defaultRepoInitImage = "alpine/git:2.45.2" + repoAuthMountPath = "/var/run/spritz/repo-auth" + repoInitHomeDir = "/home/dev" + repoInitGroupID int64 = 65532 ) var ( defaultWorkspaceSizeLimit = resource.MustParse("10Gi") defaultHomeSizeLimit = resource.MustParse("5Gi") - defaultSharedConfigSize = resource.MustParse("1Gi") ) -type homePVCSettings struct { - enabled bool - prefix string - size resource.Quantity - accessModes []corev1.PersistentVolumeAccessMode - storageClass string - mountPaths []string -} - -type sharedConfigPVCSettings struct { - enabled bool - prefix string - size resource.Quantity - accessModes []corev1.PersistentVolumeAccessMode - storageClass string - mountPath string -} - type SpritzReconciler struct { client.Client Scheme *runtime.Scheme @@ -310,61 +290,20 @@ func (r *SpritzReconciler) reconcileDeployment(ctx context.Context, spritz *spri env = append(env, spritz.Spec.Env...) ports := containerPorts(spritz) - homeSettings := loadHomePVCSettings() - if err := validateMountPaths(homeSettings.mountPaths); err != nil { - return err - } - sharedSettings := loadSharedConfigPVCSettings() - if sharedSettings.enabled { - if err := validateSharedConfigMountPath(sharedSettings.mountPath); err != nil { - return err - } - } sharedMountsSettings, err := loadSharedMountsSettings() if err != nil { return err } - homeVolumeSource := corev1.VolumeSource{ - EmptyDir: &corev1.EmptyDirVolumeSource{SizeLimit: homeSizeLimit}, - } - homePVCEnabled := homeSettings.enabled && spritz.Spec.Owner.ID != "" - if homePVCEnabled { - homePVCName := ownerPVCName(homeSettings.prefix, spritz.Spec.Owner.ID) - if err := r.ensureHomePVC(ctx, spritz, homePVCName, homeSettings); err != nil { - return fmt.Errorf("failed to ensure home PVC %s: %w", homePVCName, err) - } - homeVolumeSource = corev1.VolumeSource{ - PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ - ClaimName: homePVCName, - }, - } - } - sharedPVCEnabled := sharedSettings.enabled && spritz.Spec.Owner.ID != "" - sharedVolumeSource := corev1.VolumeSource{} - if sharedPVCEnabled { - sharedPVCName := ownerPVCName(sharedSettings.prefix, spritz.Spec.Owner.ID) - if err := r.ensureSharedConfigPVC(ctx, spritz, sharedPVCName, sharedSettings); err != nil { - return fmt.Errorf("failed to ensure shared config PVC %s: %w", sharedPVCName, err) - } - sharedVolumeSource = corev1.VolumeSource{ - PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ - ClaimName: sharedPVCName, - }, - } - } nodeSelector, err := loadPodNodeSelector() if err != nil { return err } - homeMounts := buildHomeMounts(homeSettings) + homeMounts := buildHomeMounts() sharedMountRuntime, err := buildSharedMountRuntime(spritz, sharedMountsSettings) if err != nil { return err } repoMountRoots := append([]corev1.VolumeMount{}, homeMounts...) - if sharedPVCEnabled { - repoMountRoots = append(repoMountRoots, corev1.VolumeMount{Name: "shared-config", MountPath: sharedSettings.mountPath}) - } if len(sharedMountRuntime.volumeMounts) > 0 { repoMountRoots = append(repoMountRoots, sharedMountRuntime.volumeMounts...) } @@ -375,19 +314,13 @@ func (r *SpritzReconciler) reconcileDeployment(ctx context.Context, spritz *spri volumes := []corev1.Volume{ {Name: "workspace", VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{SizeLimit: workspaceSizeLimit}}}, - {Name: "home", VolumeSource: homeVolumeSource}, - } - if sharedPVCEnabled { - volumes = append(volumes, corev1.Volume{Name: "shared-config", VolumeSource: sharedVolumeSource}) + {Name: "home", VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{SizeLimit: homeSizeLimit}}}, } if len(repoAuthVolumes) > 0 { volumes = append(volumes, repoAuthVolumes...) } volumeMounts := append([]corev1.VolumeMount{{Name: "workspace", MountPath: "/workspace"}}, homeMounts...) - if sharedPVCEnabled { - volumeMounts = append(volumeMounts, corev1.VolumeMount{Name: "shared-config", MountPath: sharedSettings.mountPath}) - } if len(sharedMountRuntime.volumes) > 0 { volumes = append(volumes, sharedMountRuntime.volumes...) } @@ -415,7 +348,7 @@ func (r *SpritzReconciler) reconcileDeployment(ctx context.Context, spritz *spri }, Volumes: volumes, } - podSpec.SecurityContext = buildPodSecurityContext(homePVCEnabled, sharedPVCEnabled, sharedMountsSettings.enabled, len(repoInitContainers) > 0) + podSpec.SecurityContext = buildPodSecurityContext(sharedMountsSettings.enabled, len(repoInitContainers) > 0) initContainers := []corev1.Container{} if sharedMountRuntime.initContainer != nil { initContainers = append(initContainers, *sharedMountRuntime.initContainer) @@ -792,70 +725,6 @@ func ttlGracePeriod() time.Duration { return grace } -func loadHomePVCSettings() homePVCSettings { - prefix := strings.TrimSpace(os.Getenv("SPRITZ_HOME_PVC_PREFIX")) - if prefix == "" { - return homePVCSettings{enabled: false, mountPaths: nil} - } - - size := defaultHomeSizeLimit - sizeRaw := strings.TrimSpace(os.Getenv("SPRITZ_HOME_PVC_SIZE")) - if sizeRaw != "" { - if parsed, err := resource.ParseQuantity(sizeRaw); err == nil { - size = parsed - } - } - - accessModes := parseAccessModes(os.Getenv("SPRITZ_HOME_PVC_ACCESS_MODES")) - if len(accessModes) == 0 { - accessModes = []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce} - } - - mountPaths := parseCSV(os.Getenv("SPRITZ_HOME_MOUNT_PATHS")) - return homePVCSettings{ - enabled: true, - prefix: prefix, - size: size, - accessModes: accessModes, - storageClass: strings.TrimSpace(os.Getenv("SPRITZ_HOME_PVC_STORAGE_CLASS")), - mountPaths: mountPaths, - } -} - -func loadSharedConfigPVCSettings() sharedConfigPVCSettings { - prefix := strings.TrimSpace(os.Getenv("SPRITZ_SHARED_CONFIG_PVC_PREFIX")) - if prefix == "" { - return sharedConfigPVCSettings{enabled: false} - } - - size := defaultSharedConfigSize - sizeRaw := strings.TrimSpace(os.Getenv("SPRITZ_SHARED_CONFIG_PVC_SIZE")) - if sizeRaw != "" { - if parsed, err := resource.ParseQuantity(sizeRaw); err == nil { - size = parsed - } - } - - accessModes := parseAccessModes(os.Getenv("SPRITZ_SHARED_CONFIG_PVC_ACCESS_MODES")) - if len(accessModes) == 0 { - accessModes = []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany} - } - - mountPath := strings.TrimSpace(os.Getenv("SPRITZ_SHARED_CONFIG_MOUNT_PATH")) - if mountPath == "" { - mountPath = defaultSharedConfigMount - } - - return sharedConfigPVCSettings{ - enabled: true, - prefix: prefix, - size: size, - accessModes: accessModes, - storageClass: strings.TrimSpace(os.Getenv("SPRITZ_SHARED_CONFIG_PVC_STORAGE_CLASS")), - mountPath: mountPath, - } -} - func loadPodNodeSelector() (map[string]string, error) { raw := strings.TrimSpace(os.Getenv("SPRITZ_POD_NODE_SELECTOR")) if raw == "" { @@ -864,21 +733,6 @@ func loadPodNodeSelector() (map[string]string, error) { return parseNodeSelector(raw) } -func parseAccessModes(raw string) []corev1.PersistentVolumeAccessMode { - modes := []corev1.PersistentVolumeAccessMode{} - for _, item := range parseCSV(raw) { - switch strings.ToLower(item) { - case "readwriteonce", "rwo": - modes = append(modes, corev1.ReadWriteOnce) - case "readwritemany", "rwx": - modes = append(modes, corev1.ReadWriteMany) - case "readonlymany", "rox": - modes = append(modes, corev1.ReadOnlyMany) - } - } - return modes -} - func parseCSV(raw string) []string { if strings.TrimSpace(raw) == "" { return nil @@ -914,8 +768,8 @@ func parseNodeSelector(raw string) (map[string]string, error) { return selector, nil } -func buildPodSecurityContext(homePVCEnabled bool, sharedConfigPVCEnabled bool, sharedMountsEnabled bool, repoInitEnabled bool) *corev1.PodSecurityContext { - if !homePVCEnabled && !sharedConfigPVCEnabled && !sharedMountsEnabled && !repoInitEnabled { +func buildPodSecurityContext(sharedMountsEnabled bool, repoInitEnabled bool) *corev1.PodSecurityContext { + if !sharedMountsEnabled && !repoInitEnabled { return nil } fsGroup := repoInitGroupID @@ -993,21 +847,8 @@ func appendRepoDirMounts(mounts []corev1.VolumeMount, repoDirs []string, mountRo return mounts } -func buildHomeMounts(settings homePVCSettings) []corev1.VolumeMount { - paths := settings.mountPaths - if len(paths) == 0 { - paths = []string{"/home/dev"} - } - seen := map[string]bool{} - mounts := []corev1.VolumeMount{} - for _, path := range paths { - if seen[path] { - continue - } - seen[path] = true - mounts = append(mounts, corev1.VolumeMount{Name: "home", MountPath: path}) - } - return mounts +func buildHomeMounts() []corev1.VolumeMount { + return []corev1.VolumeMount{{Name: "home", MountPath: repoInitHomeDir}} } type repoAuthConfig struct { @@ -1279,157 +1120,6 @@ func repoInitImage() string { return defaultRepoInitImage } -func (r *SpritzReconciler) ensureHomePVC( - ctx context.Context, - spritz *spritzv1.Spritz, - name string, - settings homePVCSettings, -) error { - labels := map[string]string{ - "spritz.sh/purpose": "home", - } - if spritz.Spec.Owner.ID != "" { - labels["spritz.sh/owner"] = ownerLabelValue(spritz.Spec.Owner.ID) - } - - pvc := &corev1.PersistentVolumeClaim{ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: spritz.Namespace}} - _, err := controllerutil.CreateOrUpdate(ctx, r.Client, pvc, func() error { - // Intentionally no owner reference: per-owner PVCs should outlive individual Spritz instances. - pvc.Labels = mergeMaps(pvc.Labels, labels) - if pvc.CreationTimestamp.IsZero() { - pvc.Spec = corev1.PersistentVolumeClaimSpec{ - AccessModes: settings.accessModes, - Resources: corev1.VolumeResourceRequirements{ - Requests: corev1.ResourceList{ - corev1.ResourceStorage: settings.size, - }, - }, - } - if settings.storageClass != "" { - pvc.Spec.StorageClassName = &settings.storageClass - } - } - return nil - }) - if err != nil { - return err - } - switch pvc.Status.Phase { - case "", corev1.ClaimBound, corev1.ClaimPending: - return nil - case corev1.ClaimLost: - return fmt.Errorf("home PVC %s lost", name) - default: - return fmt.Errorf("home PVC %s not bound (status=%s)", name, pvc.Status.Phase) - } - return nil -} - -func (r *SpritzReconciler) ensureSharedConfigPVC( - ctx context.Context, - spritz *spritzv1.Spritz, - name string, - settings sharedConfigPVCSettings, -) error { - labels := map[string]string{ - "spritz.sh/purpose": "shared-config", - } - if spritz.Spec.Owner.ID != "" { - labels["spritz.sh/owner"] = ownerLabelValue(spritz.Spec.Owner.ID) - } - - pvc := &corev1.PersistentVolumeClaim{ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: spritz.Namespace}} - _, err := controllerutil.CreateOrUpdate(ctx, r.Client, pvc, func() error { - // Intentionally no owner reference: shared config PVCs should outlive individual Spritz instances. - pvc.Labels = mergeMaps(pvc.Labels, labels) - if pvc.CreationTimestamp.IsZero() { - pvc.Spec = corev1.PersistentVolumeClaimSpec{ - AccessModes: settings.accessModes, - Resources: corev1.VolumeResourceRequirements{ - Requests: corev1.ResourceList{ - corev1.ResourceStorage: settings.size, - }, - }, - } - if settings.storageClass != "" { - pvc.Spec.StorageClassName = &settings.storageClass - } - } - return nil - }) - if err != nil { - return err - } - switch pvc.Status.Phase { - case "", corev1.ClaimBound, corev1.ClaimPending: - return nil - case corev1.ClaimLost: - return fmt.Errorf("shared config PVC %s lost", name) - default: - return fmt.Errorf("shared config PVC %s not bound (status=%s)", name, pvc.Status.Phase) - } -} - -func validateMountPaths(paths []string) error { - if len(paths) == 0 { - return nil - } - cleaned := []string{} - seen := map[string]bool{} - for _, raw := range paths { - trimmed := strings.TrimSpace(raw) - if trimmed == "" { - continue - } - if !strings.HasPrefix(trimmed, "/") { - return fmt.Errorf("home mount path must be absolute: %s", trimmed) - } - path := strings.TrimRight(trimmed, "/") - if path == "" { - return fmt.Errorf("home mount path must not be root: %s", trimmed) - } - if seen[path] { - continue - } - seen[path] = true - cleaned = append(cleaned, path) - } - for i, base := range cleaned { - for j, other := range cleaned { - if i == j { - continue - } - if strings.HasPrefix(other, base+"/") { - return fmt.Errorf("home mount paths overlap: %s and %s", base, other) - } - } - } - return nil -} - -func validateSharedConfigMountPath(raw string) error { - trimmed := strings.TrimSpace(raw) - if trimmed == "" { - return fmt.Errorf("shared config mount path must be set") - } - if !strings.HasPrefix(trimmed, "/") { - return fmt.Errorf("shared config mount path must be absolute: %s", trimmed) - } - path := strings.TrimRight(trimmed, "/") - if path == "" { - return fmt.Errorf("shared config mount path must not be root: %s", trimmed) - } - return nil -} - -func ownerPVCName(prefix, id string) string { - if id == "" { - return prefix - } - sum := sha256.Sum256([]byte(id)) - return fmt.Sprintf("%s-owner-%x", prefix, sum[:]) -} - func emptyDirSizeLimit(key string, fallback resource.Quantity) *resource.Quantity { value := strings.TrimSpace(os.Getenv(key)) if value != "" { From b615ad696491171468fdd1a710c9fe1218e6128d Mon Sep 17 00:00:00 2001 From: Onur Date: Tue, 3 Mar 2026 12:09:58 +0100 Subject: [PATCH 2/2] refactor(helm): fail on removed pvc value keys --- docs/2026-02-05-shared-mount-syncer.md | 6 ------ helm/spritz/templates/operator-deployment.yaml | 6 ++++++ scripts/verify-helm.sh | 8 ++++++++ 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/docs/2026-02-05-shared-mount-syncer.md b/docs/2026-02-05-shared-mount-syncer.md index cf93398..a52cdd1 100644 --- a/docs/2026-02-05-shared-mount-syncer.md +++ b/docs/2026-02-05-shared-mount-syncer.md @@ -237,12 +237,6 @@ Alternative (Kubernetes-native): For Spritz pods, the shared mount is writable. The syncer sidecar uses a filesystem watcher to publish quickly when content changes, with a periodic safety tick. -## Deprecated: Shared Config PVC - -The shared config PVC approach is sunsetted and is **not** planned for implementation -in the near future. Any existing PVC code should be treated as legacy and not enabled -for new deployments. Use the object-storage syncer above. - ## GCS Uniform Bucket-Level Access (Important) When using GCS buckets with Uniform Bucket-Level Access (UBLA) enabled, rclone must diff --git a/helm/spritz/templates/operator-deployment.yaml b/helm/spritz/templates/operator-deployment.yaml index 904353b..89c819b 100644 --- a/helm/spritz/templates/operator-deployment.yaml +++ b/helm/spritz/templates/operator-deployment.yaml @@ -1,3 +1,9 @@ +{{- if hasKey .Values.operator "homePVC" -}} +{{ fail "operator.homePVC has been removed; use operator.homeSizeLimit and sharedMounts instead" }} +{{- end -}} +{{- if hasKey .Values.operator "sharedConfigPVC" -}} +{{ fail "operator.sharedConfigPVC has been removed; use operator.sharedMounts/api.sharedMounts instead" }} +{{- end -}} apiVersion: apps/v1 kind: Deployment metadata: diff --git a/scripts/verify-helm.sh b/scripts/verify-helm.sh index 188e4de..b614e1b 100755 --- a/scripts/verify-helm.sh +++ b/scripts/verify-helm.sh @@ -87,4 +87,12 @@ expect_failure \ "global.ingress.className must be nginx when authGateway.enabled=true" \ helm template spritz "${chart_dir}" -f "${example_values}" --set global.ingress.className=traefik +expect_failure \ + "operator.homePVC has been removed; use operator.homeSizeLimit and sharedMounts instead" \ + helm template spritz "${chart_dir}" --set operator.homePVC.enabled=true + +expect_failure \ + "operator.sharedConfigPVC has been removed; use operator.sharedMounts/api.sharedMounts instead" \ + helm template spritz "${chart_dir}" --set operator.sharedConfigPVC.enabled=true + echo "helm checks passed"