From 76cacdcda4276d849ec07b10fdc32c915b763852 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Thu, 3 Sep 2026 20:20:32 +0530 Subject: [PATCH] fix(storage): reject scheduler mismatch at mount time (#8994) --- plugins/scheduler-k3s/storage.go | 4 +- plugins/scheduler-k3s/storage_test.go | 109 ++++++++++++++++++++++++++ plugins/storage/subcommands.go | 9 +++ plugins/storage/subcommands_test.go | 59 ++++++++++++++ plugins/storage/triggers.go | 2 +- plugins/storage/triggers_test.go | 22 ++++++ tests/unit/storage-2.bats | 54 +++++++++++++ 7 files changed, 256 insertions(+), 3 deletions(-) create mode 100644 plugins/scheduler-k3s/storage_test.go diff --git a/plugins/scheduler-k3s/storage.go b/plugins/scheduler-k3s/storage.go index 9cf0e29ce..fc7b39969 100644 --- a/plugins/scheduler-k3s/storage.go +++ b/plugins/scheduler-k3s/storage.go @@ -285,8 +285,8 @@ func ToProcessVolumes(pairs []AppMountPair) ([]ProcessVolume, error) { if pair.Entry == nil || pair.Attachment == nil { continue } - if pair.Entry.Scheduler == storage.SchedulerDockerLocal { - return nil, fmt.Errorf("storage entry %q is scheduler=docker-local but is mounted on a k3s app; recreate it with --scheduler k3s", pair.Entry.Name) + if pair.Entry.Scheduler != storage.SchedulerK3s { + return nil, fmt.Errorf("storage entry %q is scheduler=%s but is mounted on a k3s app; recreate it with --scheduler k3s", pair.Entry.Name, pair.Entry.Scheduler) } volumes = append(volumes, ProcessVolume{ Name: pair.Entry.Name, diff --git a/plugins/scheduler-k3s/storage_test.go b/plugins/scheduler-k3s/storage_test.go new file mode 100644 index 000000000..aa2bb5d4f --- /dev/null +++ b/plugins/scheduler-k3s/storage_test.go @@ -0,0 +1,109 @@ +package scheduler_k3s + +import ( + "testing" + + "github.com/dokku/dokku/plugins/storage" +) + +func TestToProcessVolumesRejectsDockerLocalEntry(t *testing.T) { + pairs := []AppMountPair{ + { + Entry: &storage.Entry{ + Name: "demo-data", + Scheduler: storage.SchedulerDockerLocal, + }, + Attachment: &storage.Attachment{ + ContainerPath: "/data", + }, + }, + } + + _, err := ToProcessVolumes(pairs) + if err == nil { + t.Fatal("expected error for docker-local entry, got nil") + } + expected := `storage entry "demo-data" is scheduler=docker-local but is mounted on a k3s app; recreate it with --scheduler k3s` + if err.Error() != expected { + t.Fatalf("expected error %q, got %q", expected, err.Error()) + } +} + +func TestToProcessVolumesRejectsNonK3sEntry(t *testing.T) { + pairs := []AppMountPair{ + { + Entry: &storage.Entry{ + Name: "demo-custom", + Scheduler: "nomad", + }, + Attachment: &storage.Attachment{ + ContainerPath: "/data", + }, + }, + } + + _, err := ToProcessVolumes(pairs) + if err == nil { + t.Fatal("expected error for non-k3s entry, got nil") + } + expected := `storage entry "demo-custom" is scheduler=nomad but is mounted on a k3s app; recreate it with --scheduler k3s` + if err.Error() != expected { + t.Fatalf("expected error %q, got %q", expected, err.Error()) + } +} + +func TestToProcessVolumesAcceptsK3sEntry(t *testing.T) { + pairs := []AppMountPair{ + { + Entry: &storage.Entry{ + Name: "demo-pvc", + Scheduler: storage.SchedulerK3s, + }, + Attachment: &storage.Attachment{ + ContainerPath: "/app/data", + Subpath: "sub", + Readonly: true, + }, + }, + } + + volumes, err := ToProcessVolumes(pairs) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(volumes) != 1 { + t.Fatalf("expected 1 volume, got %d", len(volumes)) + } + v := volumes[0] + if v.Name != "demo-pvc" { + t.Errorf("expected volume name demo-pvc, got %s", v.Name) + } + if v.MountPath != "/app/data" { + t.Errorf("expected mount path /app/data, got %s", v.MountPath) + } + if v.SubPath != "sub" { + t.Errorf("expected subpath sub, got %s", v.SubPath) + } + if !v.ReadOnly { + t.Errorf("expected read only true, got %v", v.ReadOnly) + } + if v.PersistentClaim == nil || v.PersistentClaim.ClaimName != "demo-pvc" { + t.Errorf("expected persistent claim name demo-pvc, got %+v", v.PersistentClaim) + } +} + +func TestToProcessVolumesSkipsNilPairs(t *testing.T) { + pairs := []AppMountPair{ + {Entry: nil, Attachment: nil}, + {Entry: &storage.Entry{Name: "demo", Scheduler: storage.SchedulerK3s}, Attachment: nil}, + {Entry: nil, Attachment: &storage.Attachment{ContainerPath: "/data"}}, + } + + volumes, err := ToProcessVolumes(pairs) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(volumes) != 0 { + t.Fatalf("expected 0 volumes, got %d", len(volumes)) + } +} diff --git a/plugins/storage/subcommands.go b/plugins/storage/subcommands.go index 08295653e..b6d04c4ab 100644 --- a/plugins/storage/subcommands.go +++ b/plugins/storage/subcommands.go @@ -152,11 +152,16 @@ func CommandMount(input CommandMountInput) error { return err } + appScheduler := common.GetAppScheduler(input.AppName) + // Legacy colon form: synthesize a legacy- entry plus an // attachment so storage:list (now attachment-only) sees the mount. // The storage docker-args trigger emits the corresponding -v flag at // deploy time, so behavior at the docker-run boundary is unchanged. if strings.Contains(input.NameOrPath, ":") { + if appScheduler != SchedulerDockerLocal { + return fmt.Errorf("colon-form mounts are only supported on docker-local apps; %s is a %s app. Create a named entry with 'dokku storage:create --scheduler %s' and mount it instead", input.AppName, appScheduler, appScheduler) + } return mountLegacyColon(input.AppName, input.NameOrPath) } @@ -173,6 +178,10 @@ func CommandMount(input CommandMountInput) error { return err } + if entry.Scheduler != appScheduler { + return fmt.Errorf("storage entry %q is scheduler=%s but cannot be mounted on a %s app; recreate it with --scheduler %s", entry.Name, entry.Scheduler, appScheduler, appScheduler) + } + phases := input.Phases if len(phases) == 0 { phases = []string{PhaseDeploy, PhaseRun} diff --git a/plugins/storage/subcommands_test.go b/plugins/storage/subcommands_test.go index d0d5bfe4e..f3fded584 100644 --- a/plugins/storage/subcommands_test.go +++ b/plugins/storage/subcommands_test.go @@ -1,6 +1,8 @@ package storage import ( + "os" + "path/filepath" "testing" . "github.com/onsi/gomega" @@ -15,3 +17,60 @@ func TestResolveChownIDRejectsOutOfBoundsValues(t *testing.T) { Expect(err.Error()).To(ContainSubstring("Unsupported chown permissions")) } } + +func setupTestApp(t *testing.T, appName string) { + t.Helper() + libRoot := withTempLibRoot(t) + dokkuRoot := t.TempDir() + t.Setenv("DOKKU_ROOT", dokkuRoot) + t.Setenv("PLUGIN_PATH", filepath.Join(libRoot, "plugins")) + if err := os.MkdirAll(filepath.Join(dokkuRoot, appName), 0755); err != nil { + t.Fatalf("mkdir app root: %v", err) + } +} + +func TestCommandMountRejectsSchedulerMismatch(t *testing.T) { + RegisterTestingT(t) + setupTestApp(t, "demo") + + Expect(SaveEntry(&Entry{ + Name: "demo-pvc", + Scheduler: SchedulerK3s, + })).To(Succeed()) + + err := CommandMount(CommandMountInput{ + AppName: "demo", + NameOrPath: "demo-pvc", + ContainerDir: "/data", + }) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal(`storage entry "demo-pvc" is scheduler=k3s but cannot be mounted on a docker-local app; recreate it with --scheduler docker-local`)) + + attachments, err := LoadAttachments("demo") + Expect(err).NotTo(HaveOccurred()) + Expect(attachments).To(BeEmpty()) +} + +func TestCommandMountAcceptsMatchingScheduler(t *testing.T) { + RegisterTestingT(t) + setupTestApp(t, "demo") + + Expect(SaveEntry(&Entry{ + Name: "demo-data", + Scheduler: SchedulerDockerLocal, + HostPath: "/data", + })).To(Succeed()) + + err := CommandMount(CommandMountInput{ + AppName: "demo", + NameOrPath: "demo-data", + ContainerDir: "/data", + }) + Expect(err).NotTo(HaveOccurred()) + + attachments, err := LoadAttachments("demo") + Expect(err).NotTo(HaveOccurred()) + Expect(attachments).To(HaveLen(1)) + Expect(attachments[0].EntryName).To(Equal("demo-data")) + Expect(attachments[0].ContainerPath).To(Equal("/data")) +} diff --git a/plugins/storage/triggers.go b/plugins/storage/triggers.go index 7b2042f5f..2fd01d44b 100644 --- a/plugins/storage/triggers.go +++ b/plugins/storage/triggers.go @@ -252,7 +252,7 @@ func TriggerDockerArgs(appName string, phase string) error { return fmt.Errorf("attachment on %q references missing entry %q: %w", appName, attachment.EntryName, err) } if entry.Scheduler != SchedulerDockerLocal { - continue + return fmt.Errorf("storage entry %q is scheduler=%s but is mounted on a docker-local app; recreate it with --scheduler docker-local", entry.Name, entry.Scheduler) } flag := buildDockerVFlag(entry, attachment) if flag == "" { diff --git a/plugins/storage/triggers_test.go b/plugins/storage/triggers_test.go index ac4e6731e..b15d35655 100644 --- a/plugins/storage/triggers_test.go +++ b/plugins/storage/triggers_test.go @@ -60,3 +60,25 @@ func TestBuildDockerVFlagVolumeOptions(t *testing.T) { roOpts := buildDockerVFlag(entry, &Attachment{ContainerPath: "/container", Readonly: true, VolumeOptions: "noexec,nosuid"}) Expect(roOpts).To(Equal("-v /host:/container:ro,noexec,nosuid")) } + +func TestTriggerDockerArgsRejectsSchedulerMismatch(t *testing.T) { + RegisterTestingT(t) + root := withTempLibRoot(t) + + Expect(SaveEntry(&Entry{ + Name: "demo-pvc", + Scheduler: SchedulerK3s, + })).To(Succeed()) + + writeAttachmentsFile(t, root, "demo", []*Attachment{ + { + EntryName: "demo-pvc", + ContainerPath: "/data", + Phases: []string{PhaseDeploy}, + }, + }) + + err := TriggerDockerArgs("demo", PhaseDeploy) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal(`storage entry "demo-pvc" is scheduler=k3s but is mounted on a docker-local app; recreate it with --scheduler docker-local`)) +} diff --git a/tests/unit/storage-2.bats b/tests/unit/storage-2.bats index 5908732e1..94737bb78 100644 --- a/tests/unit/storage-2.bats +++ b/tests/unit/storage-2.bats @@ -842,3 +842,57 @@ teardown() { run /bin/bash -c "dokku storage:destroy rdmtest-set-annot --destroy-host-dir --force" assert_success } + +@test "(storage:mount) rejects k3s storage entry on docker-local app" { + entry_path="$DOKKU_LIB_ROOT/data/storage-registry/entries/rdmtest-k3s-entry.json" + run /bin/bash -c "echo '{\"name\":\"rdmtest-k3s-entry\",\"scheduler\":\"k3s\",\"size\":\"1Gi\",\"schema_version\":1}' | sudo tee $entry_path >/dev/null" + assert_success + run /bin/bash -c "sudo chown dokku:dokku $entry_path" + assert_success + + run /bin/bash -c "dokku storage:mount $TEST_APP rdmtest-k3s-entry --container-dir /data" + echo "output: $output" + echo "status: $status" + assert_failure + assert_output_contains "storage entry \"rdmtest-k3s-entry\" is scheduler=k3s but cannot be mounted on a docker-local app; recreate it with --scheduler docker-local" + + run /bin/bash -c "dokku storage:list $TEST_APP --format json | jq -r '.[].entry_name' | grep '^rdmtest-k3s-entry$' || true" + assert_output "" + + run /bin/bash -c "sudo rm -f $entry_path" + assert_success +} + +@test "(storage:mount) rejects docker-local storage entry on k3s app" { + run /bin/bash -c "dokku storage:create rdmtest-local-entry" + assert_success + + run /bin/bash -c "dokku scheduler:set $TEST_APP selected k3s" + assert_success + + run /bin/bash -c "dokku storage:mount $TEST_APP rdmtest-local-entry --container-dir /data" + echo "output: $output" + echo "status: $status" + assert_failure + assert_output_contains "storage entry \"rdmtest-local-entry\" is scheduler=docker-local but cannot be mounted on a k3s app; recreate it with --scheduler k3s" + + run /bin/bash -c "dokku scheduler:set $TEST_APP selected docker-local" + assert_success + + run /bin/bash -c "dokku storage:destroy rdmtest-local-entry --destroy-host-dir --force" + assert_success +} + +@test "(storage:mount) rejects colon-form mount on k3s app" { + run /bin/bash -c "dokku scheduler:set $TEST_APP selected k3s" + assert_success + + run /bin/bash -c "dokku storage:mount $TEST_APP /tmp/custom-mount:/data" + echo "output: $output" + echo "status: $status" + assert_failure + assert_output_contains "colon-form mounts are only supported on docker-local apps" + + run /bin/bash -c "dokku scheduler:set $TEST_APP selected docker-local" + assert_success +}