Merge pull request #9010 from visheshgubrani/8994-storage-mount-scheduler-mismatch

Reject scheduler mismatch at mount time

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,

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))
+	}
+}

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-<hash> 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}

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"))
+}

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 == "" {

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`))
+}

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
+}