From 87b240054f73fddc238b2356552f798a82f6914d Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Mon, 10 Aug 2026 01:23:10 -0400 Subject: [PATCH 1/2] feat: add storage directory mode and removal flags `storage:create` and `storage:set` accept a `--mode` flag that sets the octal permissions of a docker-local host directory, and `storage:destroy` accepts a `--destroy-host-dir` flag that removes the directory along with its contents. A docker-local entry also honors `--reclaim-policy Delete` at destroy time now, matching how that policy governs a k3s PersistentVolume. Both are limited to the default `/var/lib/dokku/data/storage/` location, the same restriction `--chown` already carries. `storage:set` applies `--chown` and `--mode` to the directory rather than only recording them. --- docs/advanced-usage/persistent-storage.md | 61 +++- plugins/storage/bin/chmod-storage-dir | 27 ++ plugins/storage/bin/destroy-storage-dir | 33 ++ plugins/storage/commands_entries.go | 157 ++++++--- plugins/storage/commands_entries_test.go | 85 +++++ plugins/storage/entry.go | 65 +++- plugins/storage/entry_test.go | 86 +++++ plugins/storage/src/commands/commands.go | 2 +- .../storage/src/subcommands/subcommands.go | 11 +- plugins/storage/storage.go | 45 +++ plugins/storage/subcommands.go | 14 +- plugins/storage/triggers.go | 8 +- tests.mk | 1 + tests/unit/storage.bats | 306 ++++++++++++++++++ 14 files changed, 830 insertions(+), 71 deletions(-) create mode 100755 plugins/storage/bin/chmod-storage-dir create mode 100755 plugins/storage/bin/destroy-storage-dir create mode 100644 plugins/storage/commands_entries_test.go diff --git a/docs/advanced-usage/persistent-storage.md b/docs/advanced-usage/persistent-storage.md index 2dc09dc25..1b96d5a61 100644 --- a/docs/advanced-usage/persistent-storage.md +++ b/docs/advanced-usage/persistent-storage.md @@ -7,7 +7,7 @@ The preferred method to attach persistent storage to a Dokku-managed container i ``` storage:create [] [flags] # Register a named storage entry -storage:destroy [--force] # Remove a named storage entry (must be unmounted from every app first) +storage:destroy [--force] [--destroy-host-dir] # Remove a named storage entry (must be unmounted from every app first) storage:ensure-directory [--chown option] # [DEPRECATED] use storage:create instead storage:exec [-- ...] # Run a command (or shell) in a temporary container that mounts the entry storage:info [--format text|json] # Show details for one storage entry @@ -116,6 +116,36 @@ The `--chown` flag - whether on `storage:create` or `storage:ensure-directory` - > [!WARNING] > Failing to set the correct directory ownership may result in issues in persisting files written to the mounted storage directory. +### Setting directory permissions + +> [!IMPORTANT] +> New as of 0.38.27 + +Where `--chown` states who owns the host directory, `--mode` states its permission bits. It takes a 3 or 4 digit octal mode and is available on both `storage:create` and `storage:set`: + +```shell +dokku storage:create node-js-data --mode 0777 +``` + +```shell +dokku storage:set node-js-data --mode 0770 +``` + +Without `--mode`, a newly created directory keeps the `0755` default and a pre-existing directory keeps whatever permissions it already had. The value is stored on the entry and re-applied every time `storage:create` or `storage:set` runs against it, so a declarative caller converges the directory by re-running the same command rather than reaching for `chmod` over SSH. The mode is shown by `storage:info`: + +```shell +dokku storage:info node-js-data +``` + +``` +-----> Storage entry node-js-data + Scheduler: docker-local + Host path: /var/lib/dokku/data/storage/node-js-data + Mode: 0777 +``` + +`--mode` is applied to the directory itself and does not recurse into its contents. Like `--chown`, it is docker-local only and only manages the default `/var/lib/dokku/data/storage/` location - it is refused for k3s entries and for entries created with a custom ``. + ### Mounting storage into apps Dokku supports mounting both explicit host paths as well as docker volumes via the `storage:mount` command. This takes two arguments, an app name and a `host-path:container-path` or `docker-volume:container-path` combination. @@ -199,6 +229,35 @@ The global `--force` flag is also supported: dokku --force storage:destroy rdmtest-entry ``` +#### Removing the host directory + +> [!IMPORTANT] +> New as of 0.38.27 + +By default a docker-local entry's host directory survives `storage:destroy` - the entry is deregistered but the data stays on disk. The `--destroy-host-dir` flag removes the directory and everything in it: + +```shell +dokku storage:destroy node-js-data --destroy-host-dir +``` + +``` + ! Storage entry node-js-data is backed by /var/lib/dokku/data/storage/node-js-data, which will be removed along with its contents. + ! WARNING: Potentially Destructive Action + ! This command will destroy storage entry node-js-data. + ! To proceed, type "node-js-data" +``` + +The removal is recursive, so it succeeds whether or not the directory is empty. It is only permitted for entries at the default `/var/lib/dokku/data/storage/` location; an entry created with a custom `` is refused, and the operator removes the path themselves. + +The same removal can be declared ahead of time with `--reclaim-policy`, which behaves for a docker-local host directory the way it behaves for a k3s PersistentVolume. An entry created with `Delete` has its host directory removed on `storage:destroy` without any extra flag, while `Retain` - the default when unset - keeps it: + +```shell +dokku storage:create node-js-data --reclaim-policy Delete +dokku storage:destroy node-js-data --force +``` + +`--destroy-host-dir` is docker-local only. On a k3s entry the underlying volume is already governed by the reclaim policy recorded on the entry, so passing the flag is an error. + ### Displaying storage reports for an app > [!IMPORTANT] diff --git a/plugins/storage/bin/chmod-storage-dir b/plugins/storage/bin/chmod-storage-dir new file mode 100755 index 000000000..aaa6931b2 --- /dev/null +++ b/plugins/storage/bin/chmod-storage-dir @@ -0,0 +1,27 @@ +#!/usr/bin/env bash +set -eo pipefail +[[ $DOKKU_TRACE ]] && set -x + +main() { + declare desc="chmods a storage directory" + declare DIRECTORY="$1" MODE="$2" + + if [[ -z "$DIRECTORY" ]]; then + echo " ! Please specify a directory to chmod" 1>&2 + exit 1 + fi + + if [[ ! "$DIRECTORY" =~ ^[A-Za-z0-9\\_-]+$ ]]; then + echo " ! Directory can only contain the following set of characters: [A-Za-z0-9_-]" 1>&2 + exit 1 + fi + + if [[ ! "$MODE" =~ ^[0-7]{3,4}$ ]]; then + echo " ! Unsupported directory mode. Value must be a 3 or 4 digit octal mode, such as 0755" 1>&2 + exit 1 + fi + + chmod "$MODE" "${DOKKU_LIB_ROOT}/data/storage/$DIRECTORY" +} + +main "$@" diff --git a/plugins/storage/bin/destroy-storage-dir b/plugins/storage/bin/destroy-storage-dir new file mode 100755 index 000000000..097ae3325 --- /dev/null +++ b/plugins/storage/bin/destroy-storage-dir @@ -0,0 +1,33 @@ +#!/usr/bin/env bash +set -eo pipefail +[[ $DOKKU_TRACE ]] && set -x + +main() { + declare desc="removes a storage directory and its contents" + declare DIRECTORY="$1" + + if [[ -z "$DIRECTORY" ]]; then + echo " ! Please specify a directory to destroy" 1>&2 + exit 1 + fi + + if [[ ! "$DIRECTORY" =~ ^[A-Za-z0-9\\_-]+$ ]]; then + echo " ! Directory can only contain the following set of characters: [A-Za-z0-9_-]" 1>&2 + exit 1 + fi + + local storage_path="${DOKKU_LIB_ROOT}/data/storage/$DIRECTORY" + + if [[ ! -e "$storage_path" ]]; then + exit 0 + fi + + if [[ ! -d "$storage_path" ]]; then + echo " ! $storage_path exists but is not a directory" 1>&2 + exit 1 + fi + + rm -rf "$storage_path" +} + +main "$@" diff --git a/plugins/storage/commands_entries.go b/plugins/storage/commands_entries.go index afbc459b5..86377a405 100644 --- a/plugins/storage/commands_entries.go +++ b/plugins/storage/commands_entries.go @@ -13,17 +13,18 @@ import ( // CommandCreateInput captures the flags accepted by storage:create. type CommandCreateInput struct { - Name string - Path string - Scheduler string - Size string - AccessMode string - StorageClass string - Namespace string - Chown string - ReclaimPolicy string - Annotations map[string]string - Labels map[string]string + Name string + Path string + Scheduler string + Size string + AccessMode string + StorageClass string + Namespace string + Chown string + Mode string + ReclaimPolicy string + Annotations map[string]string + Labels map[string]string } // CommandCreate registers a new storage entry. @@ -42,6 +43,11 @@ func CommandCreate(input CommandCreateInput) error { hostPath = filepath.Join(GetStorageDirectory(), input.Name) } + mode, err := NormalizeDirectoryMode(input.Mode) + if err != nil { + return err + } + entry := &Entry{ Name: input.Name, Scheduler: scheduler, @@ -51,6 +57,7 @@ func CommandCreate(input CommandCreateInput) error { StorageClass: input.StorageClass, Namespace: input.Namespace, Chown: input.Chown, + Mode: mode, ReclaimPolicy: input.ReclaimPolicy, Annotations: input.Annotations, Labels: input.Labels, @@ -98,7 +105,12 @@ func CommandCreate(input CommandCreateInput) error { // an entry that any app still has attached. Prompts for confirmation // unless force is set (or the global --force flag exported // DOKKU_APPS_FORCE_DELETE). -func CommandDestroy(name string, force bool) error { +// +// On docker-local, the host directory is removed when destroyHostDir is +// set or when the entry's reclaim policy is Delete - the same distinction +// a k3s PV reclaim policy draws between keeping and dropping the backing +// data. Removal is recursive. +func CommandDestroy(name string, force bool, destroyHostDir bool) error { if name == "" { return errors.New("storage entry name is required") } @@ -114,26 +126,56 @@ func CommandDestroy(name string, force bool) error { return fmt.Errorf("storage entry %q is still mounted by app(s): %s", name, strings.Join(using, ", ")) } + entry, err := LoadEntry(name) + if err != nil { + return err + } + + if destroyHostDir && entry.Scheduler != SchedulerDockerLocal { + return fmt.Errorf("--destroy-host-dir only applies to docker-local storage entries; %q is scheduler %q and follows --reclaim-policy", name, entry.Scheduler) + } + + removeHostDir := false + if entry.Scheduler == SchedulerDockerLocal && (destroyHostDir || entry.ReclaimPolicy == ReclaimPolicyDelete) { + err := requireDefaultHostPath(entry, "--destroy-host-dir") + if err == nil { + removeHostDir = true + } else if destroyHostDir { + return err + } else { + // An entry written before the reclaim policy was honored on + // docker-local can carry Delete on a custom path. Refusing + // here would wedge storage:destroy for that entry, so warn + // and leave the path alone. + common.LogWarn(fmt.Sprintf("Leaving %s in place; reclaim policy Delete only removes the default host path", entry.HostPath)) + } + } + if os.Getenv("DOKKU_APPS_FORCE_DELETE") == "1" { force = true } if !force { + if removeHostDir { + common.LogWarn(fmt.Sprintf("Storage entry %s is backed by %s, which will be removed along with its contents.", name, entry.HostPath)) + } if err := common.AskForDestructiveConfirmation(name, "storage entry"); err != nil { return err } } - entry, err := LoadEntry(name) - if err != nil { - return err - } - if entry.Scheduler == SchedulerK3s { if err := callSchedulerDestroyTrigger(entry); err != nil { return fmt.Errorf("scheduler refused to remove storage entry %q: %w", name, err) } } + if removeHostDir { + if err := callStorageDirScript("destroy-storage-dir", entry.Name); err != nil { + return fmt.Errorf("unable to remove %s: %w", entry.HostPath, err) + } + common.LogVerbose(fmt.Sprintf("Removed %s", entry.HostPath)) + } + if err := DeleteEntry(name); err != nil { return err } @@ -184,6 +226,9 @@ func CommandInfo(name string, format string) error { if entry.Chown != "" { common.LogVerbose(fmt.Sprintf("Chown: %s", entry.Chown)) } + if entry.Mode != "" { + common.LogVerbose(fmt.Sprintf("Mode: %s", entry.Mode)) + } if entry.ReclaimPolicy != "" { common.LogVerbose(fmt.Sprintf("Reclaim policy: %s", entry.ReclaimPolicy)) } @@ -198,6 +243,7 @@ type CommandSetInput struct { StorageClass string Namespace string Chown string + Mode string ReclaimPolicy string Annotations map[string]string Labels map[string]string @@ -230,6 +276,13 @@ func CommandSet(input CommandSetInput) error { if input.Chown != "" { entry.Chown = input.Chown } + if input.Mode != "" { + mode, err := NormalizeDirectoryMode(input.Mode) + if err != nil { + return err + } + entry.Mode = mode + } if input.ReclaimPolicy != "" { entry.ReclaimPolicy = input.ReclaimPolicy } @@ -246,6 +299,14 @@ func CommandSet(input CommandSetInput) error { if err := SaveEntry(entry); err != nil { return err } + // Only converge the directory when the caller actually asked to change + // its permissions; an unrelated storage:set should not create or touch + // anything on disk. + if entry.Scheduler == SchedulerDockerLocal && (input.Chown != "" || input.Mode != "") { + if err := ensureDockerLocalPath(entry); err != nil { + return err + } + } if entry.Scheduler == SchedulerK3s { if err := callSchedulerCreateTrigger(entry); err != nil { return fmt.Errorf("scheduler refused storage:set for %q: %w", entry.Name, err) @@ -421,8 +482,8 @@ func CommandReportGlobal(format string) error { return err } type entryWithUse struct { - Entry *Entry `json:"entry"` - MountedBy []string `json:"mounted_by"` + Entry *Entry `json:"entry"` + MountedBy []string `json:"mounted_by"` } rows := []entryWithUse{} for _, entry := range entries { @@ -492,15 +553,29 @@ func CommandListEntries(scheduler string, format string) error { } // ensureDockerLocalPath creates the host directory referenced by a -// docker-local entry if it doesn't already exist. Idempotent: a -// pre-existing directory is left in place. +// docker-local entry if it doesn't already exist, then applies the entry's +// --mode and --chown. Idempotent: a pre-existing directory keeps its +// contents, and mode/ownership are re-applied on every run so a +// declarative caller converges by re-running storage:create. func ensureDockerLocalPath(entry *Entry) error { - if entry.Chown != "" && entry.Chown != "false" { - defaultHostPath := filepath.Join(GetStorageDirectory(), entry.Name) - if entry.HostPath != defaultHostPath { - return fmt.Errorf("--chown is only supported when the storage entry uses the default host path (%s); use --chown false and chown %s manually", defaultHostPath, entry.HostPath) + wantsChown := entry.Chown != "" && entry.Chown != "false" + if wantsChown { + if err := requireDefaultHostPath(entry, "--chown"); err != nil { + return err } } + if entry.Mode != "" { + if err := requireDefaultHostPath(entry, "--mode"); err != nil { + return err + } + } + + // A docker named volume is resolved by the docker engine, not by us. + // Without this guard the stat and mkdir below run against a relative + // token and create a stray directory in the process working directory. + if !filepath.IsAbs(entry.HostPath) { + return nil + } info, err := os.Stat(entry.HostPath) if err != nil && !os.IsNotExist(err) { @@ -516,29 +591,39 @@ func ensureDockerLocalPath(entry *Entry) error { common.LogVerbose(fmt.Sprintf("Created %s", entry.HostPath)) } - if entry.Chown != "" && entry.Chown != "false" { + if entry.Mode != "" { + common.LogVerbose(fmt.Sprintf("Setting directory mode to %s", entry.Mode)) + if err := callStorageDirScript("chmod-storage-dir", entry.Name, entry.Mode); err != nil { + return fmt.Errorf("unable to chmod %s: %w", entry.HostPath, err) + } + } + + if wantsChown { chownID, err := ResolveChownID(entry.Chown) if err != nil { return err } if chownID != "false" { - pluginPath := common.MustGetEnv("PLUGIN_AVAILABLE_PATH") - chownScript := filepath.Join(pluginPath, "storage", "bin", "chown-storage-dir") - result, err := common.CallExecCommand(common.ExecCommandInput{ - Command: "sudo", - Args: []string{chownScript, entry.Name, chownID}, - }) - if err != nil { + common.LogVerbose(fmt.Sprintf("Setting directory ownership to %s:%s", chownID, chownID)) + if err := callStorageDirScript("chown-storage-dir", entry.Name, chownID); err != nil { return fmt.Errorf("unable to chown %s: %w", entry.HostPath, err) } - if result.ExitCode != 0 { - return fmt.Errorf("unable to chown %s: %s", entry.HostPath, result.StderrContents()) - } } } return nil } +// requireDefaultHostPath refuses flags that are implemented by the sudo +// helpers in bin/, which only ever operate on the default host path. An +// operator pointing an entry at their own path owns that path themselves. +func requireDefaultHostPath(entry *Entry, flagName string) error { + defaultHostPath := filepath.Join(GetStorageDirectory(), entry.Name) + if entry.HostPath == defaultHostPath { + return nil + } + return fmt.Errorf("%s is only supported when the storage entry uses the default host path (%s); omit %s and manage %s manually", flagName, defaultHostPath, flagName, entry.HostPath) +} + // callSchedulerCreateTrigger asks the scheduler plugin (k3s) to provision // the underlying PVC/PV. The scheduler is responsible for any // cluster-level validation (storage class existence, etc.). diff --git a/plugins/storage/commands_entries_test.go b/plugins/storage/commands_entries_test.go new file mode 100644 index 000000000..ac8fd7471 --- /dev/null +++ b/plugins/storage/commands_entries_test.go @@ -0,0 +1,85 @@ +package storage + +import ( + "path/filepath" + "testing" + + . "github.com/onsi/gomega" +) + +func TestEnsureDockerLocalPathCreatesDefaultPath(t *testing.T) { + RegisterTestingT(t) + withTempLibRoot(t) + + hostPath := filepath.Join(GetStorageDirectory(), "demo") + entry := &Entry{Name: "demo", Scheduler: SchedulerDockerLocal, HostPath: hostPath} + + Expect(ensureDockerLocalPath(entry)).To(Succeed()) + Expect(hostPath).To(BeADirectory()) + + // Idempotent: a second run leaves the existing directory in place. + Expect(ensureDockerLocalPath(entry)).To(Succeed()) + Expect(hostPath).To(BeADirectory()) +} + +// TestEnsureDockerLocalPathSkipsNamedVolumes guards the docker named-volume +// case: the host path is a token the docker engine resolves, not a path on +// disk, so stat'ing and creating it would produce a stray directory in +// whatever working directory the command happened to run from. +func TestEnsureDockerLocalPathSkipsNamedVolumes(t *testing.T) { + RegisterTestingT(t) + withTempLibRoot(t) + + cwd := t.TempDir() + t.Chdir(cwd) + + entry := &Entry{Name: "demo", Scheduler: SchedulerDockerLocal, HostPath: "myvolume"} + Expect(ensureDockerLocalPath(entry)).To(Succeed()) + Expect(filepath.Join(cwd, "myvolume")).NotTo(BeADirectory()) +} + +func TestEnsureDockerLocalPathRefusesModeOnCustomPath(t *testing.T) { + RegisterTestingT(t) + withTempLibRoot(t) + + entry := &Entry{ + Name: "demo", + Scheduler: SchedulerDockerLocal, + HostPath: filepath.Join(t.TempDir(), "custom"), + Mode: "0777", + } + + err := ensureDockerLocalPath(entry) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("--mode is only supported when the storage entry uses the default host path")) + Expect(entry.HostPath).NotTo(BeADirectory()) +} + +func TestEnsureDockerLocalPathRefusesChownOnCustomPath(t *testing.T) { + RegisterTestingT(t) + withTempLibRoot(t) + + entry := &Entry{ + Name: "demo", + Scheduler: SchedulerDockerLocal, + HostPath: filepath.Join(t.TempDir(), "custom"), + Chown: "herokuish", + } + + err := ensureDockerLocalPath(entry) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("--chown is only supported when the storage entry uses the default host path")) +} + +// TestEnsureDockerLocalPathAllowsChownFalseOnCustomPath documents that the +// escape hatch the refusal message points at actually works. +func TestEnsureDockerLocalPathAllowsChownFalseOnCustomPath(t *testing.T) { + RegisterTestingT(t) + withTempLibRoot(t) + + hostPath := filepath.Join(t.TempDir(), "custom") + entry := &Entry{Name: "demo", Scheduler: SchedulerDockerLocal, HostPath: hostPath, Chown: "false"} + + Expect(ensureDockerLocalPath(entry)).To(Succeed()) + Expect(hostPath).To(BeADirectory()) +} diff --git a/plugins/storage/entry.go b/plugins/storage/entry.go index 0bb5dad73..0ed9db919 100644 --- a/plugins/storage/entry.go +++ b/plugins/storage/entry.go @@ -48,6 +48,9 @@ var ( // rule docker uses to disambiguate volume names from bind paths. dockerNamedVolumeRegexp = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9_.-]+$`) + // directoryModeRegexp matches a 3 or 4 digit octal directory mode. + directoryModeRegexp = regexp.MustCompile(`^[0-7]{3,4}$`) + // supportedSchedulers lists the values accepted for --scheduler. supportedSchedulers = map[string]bool{ SchedulerDockerLocal: true, @@ -66,18 +69,19 @@ var ( // Entry is the source of truth for a storage volume. One file per entry // lives at $DOKKU_LIB_ROOT/config/storage/entries/.json. type Entry struct { - Name string `json:"name"` - Scheduler string `json:"scheduler"` - HostPath string `json:"host_path,omitempty"` - Size string `json:"size,omitempty"` - AccessMode string `json:"access_mode,omitempty"` - StorageClass string `json:"storage_class,omitempty"` - Namespace string `json:"namespace,omitempty"` - Chown string `json:"chown,omitempty"` - ReclaimPolicy string `json:"reclaim_policy,omitempty"` - Annotations map[string]string `json:"annotations,omitempty"` - Labels map[string]string `json:"labels,omitempty"` - SchemaVersion int `json:"schema_version"` + Name string `json:"name"` + Scheduler string `json:"scheduler"` + HostPath string `json:"host_path,omitempty"` + Size string `json:"size,omitempty"` + AccessMode string `json:"access_mode,omitempty"` + StorageClass string `json:"storage_class,omitempty"` + Namespace string `json:"namespace,omitempty"` + Chown string `json:"chown,omitempty"` + Mode string `json:"mode,omitempty"` + ReclaimPolicy string `json:"reclaim_policy,omitempty"` + Annotations map[string]string `json:"annotations,omitempty"` + Labels map[string]string `json:"labels,omitempty"` + SchemaVersion int `json:"schema_version"` } // RegistryDirectory returns the parent directory for storage-plugin @@ -213,6 +217,23 @@ func ValidateEntryName(name string, allowLegacyPrefix bool) error { return nil } +// NormalizeDirectoryMode canonicalizes a user-supplied octal directory +// mode to its 4 digit form so the stored value is stable regardless of +// whether the caller wrote 755 or 0755. An empty mode is passed through, +// meaning "leave the directory's permissions alone". +func NormalizeDirectoryMode(mode string) (string, error) { + if mode == "" { + return "", nil + } + if !directoryModeRegexp.MatchString(mode) { + return "", fmt.Errorf("Unsupported directory mode %q. Value must be a 3 or 4 digit octal mode, such as 0755", mode) + } + if len(mode) == 3 { + return "0" + mode, nil + } + return mode, nil +} + // Validate checks an Entry's fields against the cross-field rules for its // scheduler. It is reused by both storage:create and the legacy migration. func (e *Entry) Validate() error { @@ -227,6 +248,12 @@ func (e *Entry) Validate() error { return fmt.Errorf("storage entry %q has unsupported scheduler %q (supported: docker-local, k3s)", e.Name, e.Scheduler) } + // The reclaim policy governs whether the underlying volume survives a + // storage:destroy on both schedulers, so it is validated for both. + if e.ReclaimPolicy != "" && e.ReclaimPolicy != ReclaimPolicyRetain && e.ReclaimPolicy != ReclaimPolicyDelete { + return fmt.Errorf("storage entry %q has unsupported reclaim policy %q", e.Name, e.ReclaimPolicy) + } + switch e.Scheduler { case SchedulerDockerLocal: if e.HostPath == "" { @@ -244,6 +271,16 @@ func (e *Entry) Validate() error { if e.AccessMode != "" { return fmt.Errorf("storage entry %q (docker-local) does not accept --access-mode", e.Name) } + if _, err := NormalizeDirectoryMode(e.Mode); err != nil { + return err + } + // Removing the host path is implemented by a sudo helper that only + // ever operates on the default location, so a Delete policy on any + // other path could never be honored. + defaultHostPath := filepath.Join(GetStorageDirectory(), e.Name) + if e.ReclaimPolicy == ReclaimPolicyDelete && e.HostPath != defaultHostPath { + return fmt.Errorf("storage entry %q (docker-local) only accepts --reclaim-policy Delete on the default host path (%s)", e.Name, defaultHostPath) + } case SchedulerK3s: if e.Size == "" { return fmt.Errorf("storage entry %q (k3s) requires --size", e.Name) @@ -257,8 +294,8 @@ func (e *Entry) Validate() error { if e.HostPath != "" && !filepath.IsAbs(e.HostPath) { return fmt.Errorf("storage entry %q host_path must be absolute, got %q", e.Name, e.HostPath) } - if e.ReclaimPolicy != "" && e.ReclaimPolicy != ReclaimPolicyRetain && e.ReclaimPolicy != ReclaimPolicyDelete { - return fmt.Errorf("storage entry %q has unsupported reclaim policy %q", e.Name, e.ReclaimPolicy) + if e.Mode != "" { + return fmt.Errorf("storage entry %q (k3s) does not accept --mode", e.Name) } } diff --git a/plugins/storage/entry_test.go b/plugins/storage/entry_test.go index a09fae334..edbb4caa5 100644 --- a/plugins/storage/entry_test.go +++ b/plugins/storage/entry_test.go @@ -101,6 +101,70 @@ func TestEntryValidateDockerLocal(t *testing.T) { withClass := &Entry{Name: "foo", Scheduler: SchedulerDockerLocal, HostPath: "/data", StorageClass: "longhorn"} Expect(withClass.Validate()).To(HaveOccurred()) + + withMode := &Entry{Name: "foo", Scheduler: SchedulerDockerLocal, HostPath: "/var/lib/dokku/data/storage/foo", Mode: "0777"} + Expect(withMode.Validate()).To(Succeed()) + + withBadMode := &Entry{Name: "foo", Scheduler: SchedulerDockerLocal, HostPath: "/var/lib/dokku/data/storage/foo", Mode: "0999"} + Expect(withBadMode.Validate()).To(HaveOccurred()) +} + +// TestEntryValidateDockerLocalReclaimPolicy covers the reclaim policy now +// that it governs whether storage:destroy removes the host directory on +// docker-local, not just the k3s PV. +func TestEntryValidateDockerLocalReclaimPolicy(t *testing.T) { + RegisterTestingT(t) + + defaultPath := "/var/lib/dokku/data/storage/foo" + + retain := &Entry{Name: "foo", Scheduler: SchedulerDockerLocal, HostPath: defaultPath, ReclaimPolicy: ReclaimPolicyRetain} + Expect(retain.Validate()).To(Succeed()) + + del := &Entry{Name: "foo", Scheduler: SchedulerDockerLocal, HostPath: defaultPath, ReclaimPolicy: ReclaimPolicyDelete} + Expect(del.Validate()).To(Succeed()) + + bad := &Entry{Name: "foo", Scheduler: SchedulerDockerLocal, HostPath: defaultPath, ReclaimPolicy: "Recycle"} + err := bad.Validate() + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("reclaim policy")) + + // Delete can only be honored where the sudo helper is allowed to + // operate, so a custom host path is refused up front rather than + // silently ignored at destroy time. + custom := &Entry{Name: "foo", Scheduler: SchedulerDockerLocal, HostPath: "/mnt/custom", ReclaimPolicy: ReclaimPolicyDelete} + err = custom.Validate() + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("default host path")) + + // Retain on a custom path stays legal; nothing gets removed either way. + customRetain := &Entry{Name: "foo", Scheduler: SchedulerDockerLocal, HostPath: "/mnt/custom", ReclaimPolicy: ReclaimPolicyRetain} + Expect(customRetain.Validate()).To(Succeed()) +} + +func TestNormalizeDirectoryMode(t *testing.T) { + RegisterTestingT(t) + + accepted := map[string]string{ + "": "", + "755": "0755", + "777": "0777", + "0755": "0755", + "0777": "0777", + "2775": "2775", + "1777": "1777", + "0000": "0000", + } + for input, expected := range accepted { + normalized, err := NormalizeDirectoryMode(input) + Expect(err).NotTo(HaveOccurred(), "expected %q to be accepted", input) + Expect(normalized).To(Equal(expected), "unexpected normalization of %q", input) + } + + for _, input := range []string{"8", "88", "888", "0888", "07555", "0x1ff", "u+rwx", "-1", " 755 ", "rwx"} { + _, err := NormalizeDirectoryMode(input) + Expect(err).To(HaveOccurred(), "expected %q to be rejected", input) + Expect(err.Error()).To(ContainSubstring("Unsupported directory mode")) + } } func TestEntryValidateK3s(t *testing.T) { @@ -125,6 +189,11 @@ func TestEntryValidateK3s(t *testing.T) { badReclaim := &Entry{Name: "foo", Scheduler: SchedulerK3s, Size: "2Gi", StorageClass: "longhorn", ReclaimPolicy: "Recycle"} Expect(badReclaim.Validate()).To(HaveOccurred()) + + withMode := &Entry{Name: "foo", Scheduler: SchedulerK3s, Size: "2Gi", StorageClass: "longhorn", Mode: "0777"} + err = withMode.Validate() + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("--mode")) } func TestEntryValidateScheduler(t *testing.T) { @@ -166,6 +235,23 @@ func TestEntryRoundTrip(t *testing.T) { Expect(loaded.Labels).To(Equal(original.Labels)) Expect(loaded.SchemaVersion).To(Equal(SchemaVersion)) + // mode is docker-local only, so it round-trips through its own entry. + local := &Entry{ + Name: "demo-local", + Scheduler: SchedulerDockerLocal, + HostPath: filepath.Join(GetStorageDirectory(), "demo-local"), + Chown: "herokuish", + Mode: "0777", + ReclaimPolicy: ReclaimPolicyDelete, + } + Expect(SaveEntry(local)).To(Succeed()) + + loadedLocal, err := LoadEntry("demo-local") + Expect(err).NotTo(HaveOccurred()) + Expect(loadedLocal.Mode).To(Equal("0777")) + Expect(loadedLocal.Chown).To(Equal("herokuish")) + Expect(loadedLocal.ReclaimPolicy).To(Equal(ReclaimPolicyDelete)) + expectedPath := filepath.Join(root, "data", "storage-registry", "entries", "demo-data.json") Expect(expectedPath).To(BeARegularFile()) diff --git a/plugins/storage/src/commands/commands.go b/plugins/storage/src/commands/commands.go index a7aef6198..f95b87220 100644 --- a/plugins/storage/src/commands/commands.go +++ b/plugins/storage/src/commands/commands.go @@ -19,7 +19,7 @@ Additional commands:` helpContent = ` storage:create [] [flags], Register a named storage entry - storage:destroy [--force], Remove a named storage entry (must be unmounted from every app first) + storage:destroy [--force] [--destroy-host-dir], Remove a named storage entry (must be unmounted from every app first) storage:ensure-directory [--chown option] , [DEPRECATED] use storage:create instead storage:exec [-- ...], Run a command (or shell) in a temporary container that mounts the entry storage:info [--format text|json], Show details for one storage entry diff --git a/plugins/storage/src/subcommands/subcommands.go b/plugins/storage/src/subcommands/subcommands.go index b49509115..ddc758467 100644 --- a/plugins/storage/src/subcommands/subcommands.go +++ b/plugins/storage/src/subcommands/subcommands.go @@ -27,7 +27,8 @@ func main() { storageClass := args.String("storage-class-name", "", "--storage-class-name: PVC storage class (k3s only)") namespace := args.String("namespace", "", "--namespace: PVC namespace (k3s only)") chown := args.String("chown", "", "--chown: chown option (docker-local only)") - reclaim := args.String("reclaim-policy", "", "--reclaim-policy: PV reclaim policy (Retain or Delete, k3s only)") + mode := args.String("mode", "", "--mode: octal permissions for the host directory, such as 0755 (docker-local only)") + reclaim := args.String("reclaim-policy", "", "--reclaim-policy: reclaim policy for the underlying volume (Retain or Delete)") annotations := args.StringSlice("annotation", nil, "--annotation key=value: PVC annotation (repeatable)") labels := args.StringSlice("label", nil, "--label key=value: PVC label (repeatable)") args.Parse(os.Args[2:]) @@ -52,6 +53,7 @@ func main() { StorageClass: *storageClass, Namespace: *namespace, Chown: *chown, + Mode: *mode, ReclaimPolicy: *reclaim, Annotations: annotMap, Labels: labelMap, @@ -59,8 +61,9 @@ func main() { case "destroy": args := flag.NewFlagSet("storage:destroy", flag.ExitOnError) force := args.Bool("force", false, "--force: force destroy without confirmation") + destroyHostDir := args.Bool("destroy-host-dir", false, "--destroy-host-dir: also remove the host directory and its contents (docker-local only)") args.Parse(os.Args[2:]) - err = storage.CommandDestroy(args.Arg(0), *force) + err = storage.CommandDestroy(args.Arg(0), *force, *destroyHostDir) case "ensure-directory": args := flag.NewFlagSet("storage:ensure-directory", flag.ExitOnError) chown := args.String("chown", "herokuish", "--chown: chown option (herokuish, heroku, paketo, root, false)") @@ -86,7 +89,8 @@ func main() { storageClass := args.String("storage-class-name", "", "--storage-class-name: existing storage class (must match)") namespace := args.String("namespace", "", "--namespace: new namespace") chown := args.String("chown", "", "--chown: chown option") - reclaim := args.String("reclaim-policy", "", "--reclaim-policy: PV reclaim policy") + mode := args.String("mode", "", "--mode: octal permissions for the host directory, such as 0755 (docker-local only)") + reclaim := args.String("reclaim-policy", "", "--reclaim-policy: reclaim policy for the underlying volume (Retain or Delete)") annotations := args.StringSlice("annotation", nil, "--annotation key=value: PVC annotation (repeatable, replaces all)") labels := args.StringSlice("label", nil, "--label key=value: PVC label (repeatable, replaces all)") args.Parse(os.Args[2:]) @@ -107,6 +111,7 @@ func main() { StorageClass: *storageClass, Namespace: *namespace, Chown: *chown, + Mode: *mode, ReclaimPolicy: *reclaim, Annotations: annotMap, Labels: labelMap, diff --git a/plugins/storage/storage.go b/plugins/storage/storage.go index edeab30cd..18fc19447 100644 --- a/plugins/storage/storage.go +++ b/plugins/storage/storage.go @@ -3,6 +3,7 @@ package storage import ( "errors" "fmt" + "path/filepath" "regexp" "strings" @@ -189,6 +190,50 @@ func GetStorageDirectory() string { return fmt.Sprintf("%s/data/storage", dokkuLibRoot) } +// storageDirScriptNames lists the sudo helpers shipped in the plugin's bin +// directory. Each one takes a storage entry basename and builds the path +// under $DOKKU_LIB_ROOT/data/storage itself, so no caller can point them +// outside the storage root. +var storageDirScriptNames = []string{ + "chown-storage-dir", + "chmod-storage-dir", + "destroy-storage-dir", +} + +// StorageDirScriptPath returns the absolute path to a storage directory +// sudo helper. +func StorageDirScriptPath(name string) string { + pluginPath := common.MustGetEnv("PLUGIN_AVAILABLE_PATH") + return filepath.Join(pluginPath, "storage", "bin", name) +} + +// StorageDirScripts returns the absolute path of every storage directory +// sudo helper. TriggerInstall whitelists each one in the plugin's sudoers +// file. +func StorageDirScripts() []string { + paths := []string{} + for _, name := range storageDirScriptNames { + paths = append(paths, StorageDirScriptPath(name)) + } + return paths +} + +// callStorageDirScript runs a storage directory sudo helper, surfacing the +// helper's own stderr message when it refuses the arguments. +func callStorageDirScript(name string, args ...string) error { + result, err := common.CallExecCommand(common.ExecCommandInput{ + Command: "sudo", + Args: append([]string{StorageDirScriptPath(name)}, args...), + }) + if err != nil { + return err + } + if result.ExitCode != 0 { + return errors.New(strings.TrimSpace(result.StderrContents())) + } + return nil +} + // ValidateDirectoryName validates a storage directory name func ValidateDirectoryName(directory string) error { if directory == "" { diff --git a/plugins/storage/subcommands.go b/plugins/storage/subcommands.go index 2dec6470a..7a38b23f0 100644 --- a/plugins/storage/subcommands.go +++ b/plugins/storage/subcommands.go @@ -21,7 +21,7 @@ Additional commands:` helpContent = ` storage:create [] [flags], Register a named storage entry - storage:destroy [--force], Remove a named storage entry (must be unmounted from every app first) + storage:destroy [--force] [--destroy-host-dir], Remove a named storage entry (must be unmounted from every app first) storage:ensure-directory [--chown option] , [DEPRECATED] use storage:create instead storage:exec [-- ...], Run a command (or shell) in a temporary container that mounts the entry storage:info [--format text|json], Show details for one storage entry @@ -62,19 +62,9 @@ func CommandEnsureDirectory(directory string, chownFlag string) error { if chownID != "false" { common.LogVerboseQuiet(fmt.Sprintf("Setting directory ownership to %s:%s", chownID, chownID)) - pluginPath := common.MustGetEnv("PLUGIN_AVAILABLE_PATH") - chownScript := filepath.Join(pluginPath, "storage", "bin", "chown-storage-dir") - - result, err := common.CallExecCommand(common.ExecCommandInput{ - Command: "sudo", - Args: []string{chownScript, directory, chownID}, - }) - if err != nil { + if err := callStorageDirScript("chown-storage-dir", directory, chownID); err != nil { return fmt.Errorf("Unable to set directory ownership: %s", err.Error()) } - if result.ExitCode != 0 { - return fmt.Errorf("Unable to set directory ownership: %s", result.StderrContents()) - } } common.LogVerboseQuiet("Directory ready for mounting") diff --git a/plugins/storage/triggers.go b/plugins/storage/triggers.go index d07ce3ac2..7b2042f5f 100644 --- a/plugins/storage/triggers.go +++ b/plugins/storage/triggers.go @@ -66,11 +66,11 @@ func TriggerInstall() error { return nil } - pluginPath := common.MustGetEnv("PLUGIN_AVAILABLE_PATH") - chownScript := filepath.Join(pluginPath, "storage", "bin", "chown-storage-dir") - sudoersFile := "/etc/sudoers.d/dokku-storage" - content := fmt.Sprintf("%%dokku ALL=(ALL) NOPASSWD:%s *\n", chownScript) + content := "" + for _, script := range StorageDirScripts() { + content += fmt.Sprintf("%%dokku ALL=(ALL) NOPASSWD:%s *\n", script) + } content += "Defaults env_keep += \"DOKKU_LIB_ROOT\"\n" if err := os.WriteFile(sudoersFile, []byte(content), 0440); err != nil { diff --git a/tests.mk b/tests.mk index 7d07723f7..17c5a1aab 100644 --- a/tests.mk +++ b/tests.mk @@ -186,6 +186,7 @@ go-tests: @$(MAKE) go-test-plugin PLUGIN_NAME=network @$(MAKE) go-test-plugin PLUGIN_NAME=buildpacks @$(MAKE) go-test-plugin PLUGIN_NAME=scheduler-k3s + @$(MAKE) go-test-plugin PLUGIN_NAME=storage go-test-plugin: cd plugins/$(PLUGIN_NAME) && go get github.com/onsi/gomega && DOKKU_ROOT=/home/dokku DOKKU_LIB_ROOT=/var/lib/dokku go test -v -p 1 -race -mod=readonly || exit $$? diff --git a/tests/unit/storage.bats b/tests/unit/storage.bats index 3acc757bc..327779a9a 100644 --- a/tests/unit/storage.bats +++ b/tests/unit/storage.bats @@ -953,3 +953,309 @@ teardown() { run /bin/bash -c "dokku storage:destroy rdmtest-rpt-keys --force" assert_success } + +@test "(storage:create) --mode sets directory permissions" { + run /bin/bash -c "dokku storage:create --mode 0777 rdmtest-mode" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "stat -c '%a' $DOKKU_LIB_ROOT/data/storage/rdmtest-mode" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "777" + + # the mode is stored on the entry in its canonical 4-digit form + run /bin/bash -c "dokku storage:info rdmtest-mode --format json | jq -r '.mode'" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "0777" + + run /bin/bash -c "dokku storage:destroy rdmtest-mode --destroy-host-dir --force" + assert_success +} + +@test "(storage:create) --mode re-applies on an existing directory" { + # the default mode assertion below only holds for a freshly created directory + rm -rf "$DOKKU_LIB_ROOT/data/storage/rdmtest-mode-converge" + + run /bin/bash -c "dokku storage:create rdmtest-mode-converge" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "stat -c '%a' $DOKKU_LIB_ROOT/data/storage/rdmtest-mode-converge" + assert_success + assert_output "755" + + # re-running create against the existing entry converges the directory + run /bin/bash -c "dokku storage:create --mode 0700 rdmtest-mode-converge" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "stat -c '%a' $DOKKU_LIB_ROOT/data/storage/rdmtest-mode-converge" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "700" + + run /bin/bash -c "dokku storage:destroy rdmtest-mode-converge --destroy-host-dir --force" + assert_success +} + +@test "(storage:create) --mode accepts a 3 digit octal mode" { + run /bin/bash -c "dokku storage:create --mode 750 rdmtest-mode-short" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "stat -c '%a' $DOKKU_LIB_ROOT/data/storage/rdmtest-mode-short" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "750" + + run /bin/bash -c "dokku storage:info rdmtest-mode-short --format json | jq -r '.mode'" + assert_success + assert_output "0750" + + run /bin/bash -c "dokku storage:destroy rdmtest-mode-short --destroy-host-dir --force" + assert_success +} + +@test "(storage:create) --mode rejects an invalid value" { + run /bin/bash -c "dokku storage:create --mode 0888 rdmtest-mode-bad" + echo "output: $output" + echo "status: $status" + assert_failure + assert_output_contains "Unsupported directory mode" + + run /bin/bash -c "dokku storage:create --mode u+rwx rdmtest-mode-bad" + echo "output: $output" + echo "status: $status" + assert_failure + assert_output_contains "Unsupported directory mode" + + run /bin/bash -c "dokku storage:list-entries --format json | jq -r '.[].name' | grep '^rdmtest-mode-bad$' || true" + assert_output "" +} + +@test "(storage:create) --mode rejects a non-default host path" { + custom_path="/tmp/rdmtest-mode-custom" + rm -rf "$custom_path" + + run /bin/bash -c "dokku storage:create --mode 0777 rdmtest-mode-custom $custom_path" + echo "output: $output" + echo "status: $status" + assert_failure + assert_output_contains "--mode is only supported when the storage entry uses the default host path" + + run /bin/bash -c "dokku storage:list-entries --format json | jq -r '.[].name' | grep '^rdmtest-mode-custom$' || true" + assert_output "" + + rm -rf "$custom_path" +} + +@test "(storage:create) --mode is rejected on a k3s entry" { + run /bin/bash -c "dokku storage:create --scheduler k3s --size 1Gi --mode 0777 rdmtest-mode-k3s" + echo "output: $output" + echo "status: $status" + assert_failure + assert_output_contains "does not accept --mode" + + run /bin/bash -c "dokku storage:list-entries --format json | jq -r '.[].name' | grep '^rdmtest-mode-k3s$' || true" + assert_output "" +} + +@test "(storage:set) --mode updates permissions on an existing entry" { + run /bin/bash -c "dokku storage:create --mode 0755 rdmtest-mode-set" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku storage:set rdmtest-mode-set --mode 0770" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "stat -c '%a' $DOKKU_LIB_ROOT/data/storage/rdmtest-mode-set" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "770" + + run /bin/bash -c "dokku storage:info rdmtest-mode-set --format json | jq -r '.mode'" + assert_success + assert_output "0770" + + run /bin/bash -c "dokku storage:destroy rdmtest-mode-set --destroy-host-dir --force" + assert_success +} + +@test "(storage) chmod-storage-dir rejects invalid modes" { + run /bin/bash -c "DOKKU_LIB_ROOT=$DOKKU_LIB_ROOT $PLUGIN_AVAILABLE_PATH/storage/bin/chmod-storage-dir $TEST_APP 0888" + echo "output: $output" + echo "status: $status" + assert_failure + assert_output_contains "Unsupported directory mode" + + run /bin/bash -c "DOKKU_LIB_ROOT=$DOKKU_LIB_ROOT $PLUGIN_AVAILABLE_PATH/storage/bin/chmod-storage-dir $TEST_APP 07555" + echo "output: $output" + echo "status: $status" + assert_failure + assert_output_contains "Unsupported directory mode" + + run /bin/bash -c "DOKKU_LIB_ROOT=$DOKKU_LIB_ROOT $PLUGIN_AVAILABLE_PATH/storage/bin/chmod-storage-dir $TEST_APP u+rwx" + echo "output: $output" + echo "status: $status" + assert_failure + assert_output_contains "Unsupported directory mode" + + run /bin/bash -c "DOKKU_LIB_ROOT=$DOKKU_LIB_ROOT $PLUGIN_AVAILABLE_PATH/storage/bin/chmod-storage-dir '../escape' 0777" + echo "output: $output" + echo "status: $status" + assert_failure + assert_output_contains "Directory can only contain the following set of characters" +} + +@test "(storage) destroy-storage-dir refuses a traversing directory name" { + run /bin/bash -c "DOKKU_LIB_ROOT=$DOKKU_LIB_ROOT $PLUGIN_AVAILABLE_PATH/storage/bin/destroy-storage-dir '../escape'" + echo "output: $output" + echo "status: $status" + assert_failure + assert_output_contains "Directory can only contain the following set of characters" + + # a missing directory is a no-op rather than an error + run /bin/bash -c "DOKKU_LIB_ROOT=$DOKKU_LIB_ROOT $PLUGIN_AVAILABLE_PATH/storage/bin/destroy-storage-dir rdmtest-absent" + echo "output: $output" + echo "status: $status" + assert_success +} + +@test "(storage:destroy) leaves the host directory in place by default" { + run /bin/bash -c "dokku storage:create rdmtest-keep-dir" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku storage:destroy rdmtest-keep-dir --force" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "test -d $DOKKU_LIB_ROOT/data/storage/rdmtest-keep-dir" + echo "output: $output" + echo "status: $status" + assert_success + + rm -rf "$DOKKU_LIB_ROOT/data/storage/rdmtest-keep-dir" +} + +@test "(storage:destroy) --destroy-host-dir removes a non-empty host directory" { + run /bin/bash -c "dokku storage:create rdmtest-drop-dir" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "sudo touch $DOKKU_LIB_ROOT/data/storage/rdmtest-drop-dir/payload" + assert_success + + run /bin/bash -c "dokku storage:destroy rdmtest-drop-dir --destroy-host-dir --force" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "test -d $DOKKU_LIB_ROOT/data/storage/rdmtest-drop-dir" + echo "output: $output" + echo "status: $status" + assert_failure + + run /bin/bash -c "dokku storage:list-entries --format json | jq -r '.[].name' | grep '^rdmtest-drop-dir$' || true" + assert_output "" +} + +@test "(storage:destroy) --reclaim-policy Delete removes the host directory" { + run /bin/bash -c "dokku storage:create --reclaim-policy Delete rdmtest-reclaim" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku storage:destroy rdmtest-reclaim --force" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "test -d $DOKKU_LIB_ROOT/data/storage/rdmtest-reclaim" + echo "output: $output" + echo "status: $status" + assert_failure +} + +@test "(storage:create) --reclaim-policy Delete rejects a non-default host path" { + custom_path="/tmp/rdmtest-reclaim-custom" + rm -rf "$custom_path" + + run /bin/bash -c "dokku storage:create --reclaim-policy Delete rdmtest-reclaim-custom $custom_path" + echo "output: $output" + echo "status: $status" + assert_failure + assert_output_contains "default host path" + + run /bin/bash -c "dokku storage:list-entries --format json | jq -r '.[].name' | grep '^rdmtest-reclaim-custom$' || true" + assert_output "" + + rm -rf "$custom_path" +} + +@test "(storage:destroy) --destroy-host-dir refuses a non-default host path" { + custom_path="/tmp/rdmtest-drop-custom" + rm -rf "$custom_path" + mkdir -p "$custom_path" + + run /bin/bash -c "dokku storage:create rdmtest-drop-custom $custom_path" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku storage:destroy rdmtest-drop-custom --destroy-host-dir --force" + echo "output: $output" + echo "status: $status" + assert_failure + assert_output_contains "--destroy-host-dir is only supported when the storage entry uses the default host path" + + run /bin/bash -c "test -d $custom_path" + assert_success + + run /bin/bash -c "dokku storage:destroy rdmtest-drop-custom --force" + assert_success + + rm -rf "$custom_path" +} + +@test "(storage) install trigger whitelists the storage directory helpers" { + run /bin/bash -c "test -f /etc/sudoers.d/dokku-storage" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "sudo grep -c 'storage/bin/chown-storage-dir' /etc/sudoers.d/dokku-storage" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "1" + + run /bin/bash -c "sudo grep -c 'storage/bin/chmod-storage-dir' /etc/sudoers.d/dokku-storage" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "1" + + run /bin/bash -c "sudo grep -c 'storage/bin/destroy-storage-dir' /etc/sudoers.d/dokku-storage" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "1" +} From 74f9acf7ba846930fe16960da1629426dd687d2d Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Mon, 10 Aug 2026 15:00:45 -0400 Subject: [PATCH 2/2] refactor: align storage:set with dokku conventions `storage:set` now takes ` []` like every other `:set` command, where omitting the value unsets the property. Previously it took flags and could not distinguish an empty value from an omitted one, so nothing it set could ever be cleared. The flag form keeps working and emits a deprecation warning. Annotations and labels move to `storage:annotations:set`, `storage:annotations:report`, `storage:labels:set`, and `storage:labels:report`, matching the `scheduler-k3s` equivalents. These operate on a single key, so clearing one leaves the rest in place rather than replacing the whole map as the `--annotation` and `--label` flags do. --- docs/advanced-usage/persistent-storage.md | 98 +- docs/deployment/schedulers/k3s.md | 2 +- plugins/storage/Makefile | 2 +- plugins/storage/commands_entries.go | 109 ++- plugins/storage/commands_entries_test.go | 135 +++ plugins/storage/commands_metadata.go | 212 +++++ plugins/storage/commands_metadata_test.go | 164 ++++ plugins/storage/src/commands/commands.go | 6 +- .../storage/src/subcommands/subcommands.go | 117 ++- plugins/storage/subcommands.go | 6 +- tests/unit/{storage.bats => storage-1.bats} | 429 --------- tests/unit/storage-2.bats | 844 ++++++++++++++++++ 12 files changed, 1624 insertions(+), 500 deletions(-) create mode 100644 plugins/storage/commands_metadata.go create mode 100644 plugins/storage/commands_metadata_test.go rename tests/unit/{storage.bats => storage-1.bats} (69%) create mode 100644 tests/unit/storage-2.bats diff --git a/docs/advanced-usage/persistent-storage.md b/docs/advanced-usage/persistent-storage.md index 1b96d5a61..6943a9b48 100644 --- a/docs/advanced-usage/persistent-storage.md +++ b/docs/advanced-usage/persistent-storage.md @@ -6,18 +6,22 @@ The preferred method to attach persistent storage to a Dokku-managed container is the Dokku storage plugin. ``` +storage:annotations:report [] [] # Display annotations for one or more storage entries +storage:annotations:set [] # Set or clear a single annotation on a storage entry storage:create [] [flags] # Register a named storage entry storage:destroy [--force] [--destroy-host-dir] # Remove a named storage entry (must be unmounted from every app first) storage:ensure-directory [--chown option] # [DEPRECATED] use storage:create instead storage:exec [-- ...] # Run a command (or shell) in a temporary container that mounts the entry storage:info [--format text|json] # Show details for one storage entry +storage:labels:report [] [] # Display labels for one or more storage entries +storage:labels:set [] # Set or clear a single label on a storage entry storage:list [--format text|json] # List bind mounts for an app's container(s) (legacy host:container view) storage:list-entries [--scheduler s] [--format text|json] # List registered storage entries storage:mount --container-dir [flags] # Mount a named entry into an app storage:mount # [LEGACY] colon-form mount, docker-local only storage:report [] [] # Display a storage report for one or more apps storage:report --global # Display a cluster-wide entry inventory -storage:set [flags] # Update a storage entry in place +storage:set [] # Update a storage entry in place storage:unmount [--container-dir ] # Remove an attachment storage:wait # Block until a k3s entry's PVC is bound ``` @@ -121,17 +125,23 @@ The `--chown` flag - whether on `storage:create` or `storage:ensure-directory` - > [!IMPORTANT] > New as of 0.38.27 -Where `--chown` states who owns the host directory, `--mode` states its permission bits. It takes a 3 or 4 digit octal mode and is available on both `storage:create` and `storage:set`: +Where `--chown` states who owns the host directory, `--mode` states its permission bits. It takes a 3 or 4 digit octal mode, and is a `--mode` flag on `storage:create` and a `mode` property on `storage:set`: ```shell dokku storage:create node-js-data --mode 0777 ``` ```shell -dokku storage:set node-js-data --mode 0770 +dokku storage:set node-js-data mode 0770 ``` -Without `--mode`, a newly created directory keeps the `0755` default and a pre-existing directory keeps whatever permissions it already had. The value is stored on the entry and re-applied every time `storage:create` or `storage:set` runs against it, so a declarative caller converges the directory by re-running the same command rather than reaching for `chmod` over SSH. The mode is shown by `storage:info`: +Omitting the value clears the mode, leaving the directory's permissions alone on subsequent runs: + +```shell +dokku storage:set node-js-data mode +``` + +Without a mode, a newly created directory keeps the `0755` default and a pre-existing directory keeps whatever permissions it already had. The value is stored on the entry and re-applied every time `storage:create` or `storage:set` runs against it, so a declarative caller converges the directory by re-running the same command rather than reaching for `chmod` over SSH. The mode is shown by `storage:info`: ```shell dokku storage:info node-js-data @@ -144,7 +154,85 @@ dokku storage:info node-js-data Mode: 0777 ``` -`--mode` is applied to the directory itself and does not recurse into its contents. Like `--chown`, it is docker-local only and only manages the default `/var/lib/dokku/data/storage/` location - it is refused for k3s entries and for entries created with a custom ``. +The mode is applied to the directory itself and does not recurse into its contents. Like `--chown`, it is docker-local only and only manages the default `/var/lib/dokku/data/storage/` location - it is refused for k3s entries and for entries created with a custom ``. That refusal also covers migrated `legacy-*` entries, whose host paths come from the original colon-form mount rather than the default location. + +### Updating a storage entry + +> [!IMPORTANT] +> The property form is new as of 0.38.27. Prior versions used flags, which still work but emit a deprecation warning. + +An existing entry is edited with `storage:set`, which takes a property and a value. Omitting the value unsets the property, restoring whatever the entry defaults to: + +```shell +dokku storage:set node-js-data chown herokuish +dokku storage:set node-js-data chown +``` + +The following properties can be set: + +| Property | Description | Unsetting it means | +|---|---|---| +| `chown` | Ownership preset or numeric uid for the host directory | no chown is performed | +| `mode` | Octal permissions for the host directory | permissions are left alone | +| `namespace` | Namespace holding the PVC (k3s) | the `default` namespace | +| `reclaim-policy` | Whether the underlying volume survives `storage:destroy` | `Retain` | +| `size` | PVC size (k3s) | rejected, since k3s entries require a size | +| `access-mode` | PVC access mode (k3s) | rejected, see below | +| `storage-class-name` | PVC storage class (k3s) | rejected, see below | + +`access-mode` and `storage-class-name` cannot be changed on an entry that already exists, because Kubernetes cannot apply either to a bound PVC. Both a different value and an empty one are refused, since clearing is equally a change: + +```shell +dokku storage:set node-js-data access-mode ReadWriteMany +``` + +``` + ! storage:set cannot change access-mode in place; recreate the entry +``` + +Setting `chown` or `mode` on a docker-local entry applies the change to the host directory immediately. Every other property is a metadata write, and k3s entries re-apply their helm release so the cluster picks the change up. + +The older flag form - `dokku storage:set node-js-data --mode 0770` - continues to work and warns. It gained unset semantics too, so `--mode ""` clears the mode the same way omitting the positional value does. + +### Annotations and labels + +> [!IMPORTANT] +> New as of 0.38.27 + +Annotations and labels are attached to a storage entry one key at a time, matching the [scheduler-k3s equivalents](/docs/deployment/schedulers/k3s.md#setting-annotations). On k3s they propagate to both the PersistentVolumeClaim and the PersistentVolume, so backup tools like Velero and Longhorn can find the volume. + +```shell +dokku storage:annotations:set node-js-data backup.velero.io/backup-volumes node-js-data +dokku storage:labels:set node-js-data app.kubernetes.io/part-of billing +``` + +Keys may contain `/`, as the Kubernetes-style keys above do, and are stored verbatim. To clear a single key, omit the value. Other keys are left untouched, so a declarative caller does not need to re-send the whole set on every call: + +```shell +dokku storage:annotations:set node-js-data backup.velero.io/backup-volumes +``` + +Configured annotations and labels can be inspected with the matching report commands. Without an entry name they cover every registered entry: + +```shell +dokku storage:annotations:report +dokku storage:annotations:report node-js-data +dokku storage:labels:report node-js-data +``` + +``` +=====> node-js-data annotations information + Annotation backup.velero.io/backup-volumes: node-js-data +``` + +JSON output emits the keys flat, and a single value can be read directly with a flag of the form `--storage-annotations.` (or `--storage-labels.`), which requires an entry name: + +```shell +dokku storage:annotations:report node-js-data --format json +dokku storage:annotations:report node-js-data --storage-annotations.backup.velero.io/backup-volumes +``` + +`storage:create` still accepts repeatable `--annotation key=value` and `--label key=value` flags for setting the initial set at creation time. The same flags on `storage:set` are deprecated in favor of these commands, because they replace the entire map rather than a single key. ### Mounting storage into apps diff --git a/docs/deployment/schedulers/k3s.md b/docs/deployment/schedulers/k3s.md index a4a35a194..df1250a3b 100644 --- a/docs/deployment/schedulers/k3s.md +++ b/docs/deployment/schedulers/k3s.md @@ -1038,7 +1038,7 @@ dokku storage:wait demo-data git push dokku master ``` -For a hostPath-backed PV (no StorageClass), pass `` as the second positional argument and omit `--storage-class-name`. The plugin renders both the PV and the PVC into the entry's helm release. The `--reclaim-policy` flag (`Retain` or `Delete`) controls whether the underlying PV survives `storage:destroy`. Annotations and labels on `storage:create` / `storage:set` propagate to both the PVC and the PV so backup tools (Velero, Longhorn snapshots) can find them. +For a hostPath-backed PV (no StorageClass), pass `` as the second positional argument and omit `--storage-class-name`. The plugin renders both the PV and the PVC into the entry's helm release. The `--reclaim-policy` flag (`Retain` or `Delete`) controls whether the underlying PV survives `storage:destroy`. Annotations and labels set via `storage:annotations:set` and `storage:labels:set` propagate to both the PVC and the PV so backup tools (Velero, Longhorn snapshots) can find them. The legacy `storage:mount :` colon form is rejected on k3s apps; create a named entry instead. See [Persistent Storage](/docs/advanced-usage/persistent-storage.md) for the full command reference. diff --git a/plugins/storage/Makefile b/plugins/storage/Makefile index 11829d5ec..b430b0882 100644 --- a/plugins/storage/Makefile +++ b/plugins/storage/Makefile @@ -1,5 +1,5 @@ GOARCH ?= amd64 -SUBCOMMANDS = subcommands/default subcommands/create subcommands/destroy subcommands/ensure-directory subcommands/exec subcommands/info subcommands/list subcommands/list-entries subcommands/migrate subcommands/mount subcommands/report subcommands/set subcommands/unmount subcommands/wait +SUBCOMMANDS = subcommands/default subcommands/annotations:set subcommands/annotations:report subcommands/create subcommands/destroy subcommands/ensure-directory subcommands/exec subcommands/info subcommands/labels:set subcommands/labels:report subcommands/list subcommands/list-entries subcommands/migrate subcommands/mount subcommands/report subcommands/set subcommands/unmount subcommands/wait TRIGGERS = triggers/install triggers/storage-list triggers/storage-app-mounts triggers/docker-args-deploy triggers/docker-args-run triggers/post-delete triggers/post-app-clone-setup triggers/post-app-rename-setup BUILD = commands subcommands triggers PLUGIN_NAME = storage diff --git a/plugins/storage/commands_entries.go b/plugins/storage/commands_entries.go index 86377a405..fd8624ccc 100644 --- a/plugins/storage/commands_entries.go +++ b/plugins/storage/commands_entries.go @@ -235,23 +235,40 @@ func CommandInfo(name string, format string) error { return nil } -// CommandSetInput captures the flags accepted by storage:set. +// PropertyChange is a single assignment against a storage entry. An empty +// Value unsets the property, restoring whatever the entry defaults to. +type PropertyChange struct { + Property string + Value string +} + +// SettableProperties lists the entry fields storage:set understands, in the +// sorted order the "invalid property" error reports them. +var SettableProperties = []string{ + "access-mode", + "chown", + "mode", + "namespace", + "reclaim-policy", + "size", + "storage-class-name", +} + +// CommandSetInput captures the inputs accepted by storage:set. type CommandSetInput struct { - Name string - Size string - AccessMode string - StorageClass string - Namespace string - Chown string - Mode string - ReclaimPolicy string - Annotations map[string]string - Labels map[string]string + Name string + Changes []PropertyChange + + // Annotations and Labels back the deprecated --annotation / --label + // flags only, and replace the whole map. The property form has no + // equivalent; storage:annotations:set supersedes them. + Annotations map[string]string + Labels map[string]string } // CommandSet edits an existing entry's mutable fields and re-fires the // scheduler-side helm release. Refuses changes Kubernetes can't apply -// in place (access-mode swap, storage-class swap, size shrink). +// in place (access-mode swap, storage-class swap). func CommandSet(input CommandSetInput) error { if !EntryExists(input.Name) { return fmt.Errorf("storage entry %q does not exist", input.Name) @@ -261,31 +278,16 @@ func CommandSet(input CommandSetInput) error { return err } - if input.AccessMode != "" && input.AccessMode != entry.AccessMode { - return fmt.Errorf("storage:set cannot change access-mode in place; recreate the entry") - } - if input.StorageClass != "" && input.StorageClass != entry.StorageClass { - return fmt.Errorf("storage:set cannot change storage-class-name in place; recreate the entry") - } - if input.Size != "" { - entry.Size = input.Size - } - if input.Namespace != "" { - entry.Namespace = input.Namespace - } - if input.Chown != "" { - entry.Chown = input.Chown - } - if input.Mode != "" { - mode, err := NormalizeDirectoryMode(input.Mode) - if err != nil { + touchesDirectory := false + for _, change := range input.Changes { + if err := applyPropertyChange(entry, change); err != nil { return err } - entry.Mode = mode - } - if input.ReclaimPolicy != "" { - entry.ReclaimPolicy = input.ReclaimPolicy + if change.Property == "chown" || change.Property == "mode" { + touchesDirectory = true + } } + if input.Annotations != nil { entry.Annotations = input.Annotations } @@ -302,7 +304,7 @@ func CommandSet(input CommandSetInput) error { // Only converge the directory when the caller actually asked to change // its permissions; an unrelated storage:set should not create or touch // anything on disk. - if entry.Scheduler == SchedulerDockerLocal && (input.Chown != "" || input.Mode != "") { + if entry.Scheduler == SchedulerDockerLocal && touchesDirectory { if err := ensureDockerLocalPath(entry); err != nil { return err } @@ -316,6 +318,43 @@ func CommandSet(input CommandSetInput) error { return nil } +// applyPropertyChange writes a single property onto an entry. An empty +// value clears the field rather than being ignored, which is what lets +// storage:set undo itself the way every other :set command can. +func applyPropertyChange(entry *Entry, change PropertyChange) error { + switch change.Property { + case "access-mode": + // Kubernetes cannot swap these on a bound PVC, so any change - + // including clearing one that is set - has to be refused. + if change.Value != entry.AccessMode { + return errors.New("storage:set cannot change access-mode in place; recreate the entry") + } + case "storage-class-name": + if change.Value != entry.StorageClass { + return errors.New("storage:set cannot change storage-class-name in place; recreate the entry") + } + case "size": + entry.Size = change.Value + case "namespace": + entry.Namespace = change.Value + case "chown": + entry.Chown = change.Value + case "mode": + mode, err := NormalizeDirectoryMode(change.Value) + if err != nil { + return err + } + entry.Mode = mode + case "reclaim-policy": + entry.ReclaimPolicy = change.Value + case "": + return errors.New("No property specified") + default: + return fmt.Errorf("Invalid property specified, valid properties include: %s", strings.Join(SettableProperties, ", ")) + } + return nil +} + // CommandExecInput captures the storage:exec subcommand inputs. type CommandExecInput struct { Name string diff --git a/plugins/storage/commands_entries_test.go b/plugins/storage/commands_entries_test.go index ac8fd7471..41139a880 100644 --- a/plugins/storage/commands_entries_test.go +++ b/plugins/storage/commands_entries_test.go @@ -83,3 +83,138 @@ func TestEnsureDockerLocalPathAllowsChownFalseOnCustomPath(t *testing.T) { Expect(ensureDockerLocalPath(entry)).To(Succeed()) Expect(hostPath).To(BeADirectory()) } + +func TestApplyPropertyChangeScalars(t *testing.T) { + RegisterTestingT(t) + + entry := &Entry{Name: "demo", Scheduler: SchedulerDockerLocal} + + Expect(applyPropertyChange(entry, PropertyChange{Property: "chown", Value: "herokuish"})).To(Succeed()) + Expect(entry.Chown).To(Equal("herokuish")) + + Expect(applyPropertyChange(entry, PropertyChange{Property: "namespace", Value: "dokku"})).To(Succeed()) + Expect(entry.Namespace).To(Equal("dokku")) + + Expect(applyPropertyChange(entry, PropertyChange{Property: "reclaim-policy", Value: ReclaimPolicyDelete})).To(Succeed()) + Expect(entry.ReclaimPolicy).To(Equal(ReclaimPolicyDelete)) + + Expect(applyPropertyChange(entry, PropertyChange{Property: "size", Value: "2Gi"})).To(Succeed()) + Expect(entry.Size).To(Equal("2Gi")) + + // mode is canonicalized on the way in + Expect(applyPropertyChange(entry, PropertyChange{Property: "mode", Value: "770"})).To(Succeed()) + Expect(entry.Mode).To(Equal("0770")) +} + +// TestApplyPropertyChangeUnsets is the hole this shape exists to close: an +// empty value clears the field rather than being indistinguishable from an +// omitted flag. +func TestApplyPropertyChangeUnsets(t *testing.T) { + RegisterTestingT(t) + + entry := &Entry{ + Name: "demo", + Scheduler: SchedulerDockerLocal, + Chown: "herokuish", + Mode: "0770", + Namespace: "dokku", + ReclaimPolicy: ReclaimPolicyDelete, + Size: "2Gi", + } + + for _, property := range []string{"chown", "mode", "namespace", "reclaim-policy", "size"} { + Expect(applyPropertyChange(entry, PropertyChange{Property: property})).To(Succeed(), "unsetting %q", property) + } + + Expect(entry.Chown).To(BeEmpty()) + Expect(entry.Mode).To(BeEmpty()) + Expect(entry.Namespace).To(BeEmpty()) + Expect(entry.ReclaimPolicy).To(BeEmpty()) + Expect(entry.Size).To(BeEmpty()) +} + +func TestApplyPropertyChangeRejectsInvalidInput(t *testing.T) { + RegisterTestingT(t) + + entry := &Entry{Name: "demo", Scheduler: SchedulerDockerLocal} + + err := applyPropertyChange(entry, PropertyChange{Property: "bogus", Value: "x"}) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("Invalid property specified, valid properties include: access-mode, chown, mode, namespace, reclaim-policy, size, storage-class-name")) + + err = applyPropertyChange(entry, PropertyChange{Property: "", Value: "x"}) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("No property specified")) + + err = applyPropertyChange(entry, PropertyChange{Property: "mode", Value: "0888"}) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("Unsupported directory mode")) +} + +// TestApplyPropertyChangeRefusesInPlaceSwaps covers both a differing value +// and an empty one, since clearing a bound PVC's access-mode or storage +// class is equally a change Kubernetes cannot apply. +func TestApplyPropertyChangeRefusesInPlaceSwaps(t *testing.T) { + RegisterTestingT(t) + + entry := &Entry{ + Name: "demo", + Scheduler: SchedulerK3s, + Size: "2Gi", + AccessMode: "ReadWriteOnce", + StorageClass: "longhorn", + } + + err := applyPropertyChange(entry, PropertyChange{Property: "access-mode", Value: "ReadWriteMany"}) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("cannot change access-mode in place")) + + err = applyPropertyChange(entry, PropertyChange{Property: "access-mode"}) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("cannot change access-mode in place")) + + err = applyPropertyChange(entry, PropertyChange{Property: "storage-class-name", Value: "other"}) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("cannot change storage-class-name in place")) + + err = applyPropertyChange(entry, PropertyChange{Property: "storage-class-name"}) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("cannot change storage-class-name in place")) + + // Re-stating the current value is a no-op, not a change. + Expect(applyPropertyChange(entry, PropertyChange{Property: "access-mode", Value: "ReadWriteOnce"})).To(Succeed()) + Expect(applyPropertyChange(entry, PropertyChange{Property: "storage-class-name", Value: "longhorn"})).To(Succeed()) +} + +func TestCommandSetPersistsAndValidates(t *testing.T) { + RegisterTestingT(t) + withTempLibRoot(t) + stageDockerLocalEntry(t, "demo") + + Expect(CommandSet(CommandSetInput{ + Name: "demo", + Changes: []PropertyChange{{Property: "namespace", Value: "dokku"}}, + })).To(Succeed()) + + loaded, err := LoadEntry("demo") + Expect(err).NotTo(HaveOccurred()) + Expect(loaded.Namespace).To(Equal("dokku")) + + // An invalid change leaves the stored entry untouched. + err = CommandSet(CommandSetInput{ + Name: "demo", + Changes: []PropertyChange{{Property: "bogus", Value: "x"}}, + }) + Expect(err).To(HaveOccurred()) + + loaded, err = LoadEntry("demo") + Expect(err).NotTo(HaveOccurred()) + Expect(loaded.Namespace).To(Equal("dokku")) + + err = CommandSet(CommandSetInput{ + Name: "missing", + Changes: []PropertyChange{{Property: "namespace", Value: "dokku"}}, + }) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("does not exist")) +} diff --git a/plugins/storage/commands_metadata.go b/plugins/storage/commands_metadata.go new file mode 100644 index 000000000..7b4b6703f --- /dev/null +++ b/plugins/storage/commands_metadata.go @@ -0,0 +1,212 @@ +package storage + +import ( + "encoding/json" + "errors" + "fmt" + "sort" + "strings" + + "github.com/dokku/dokku/plugins/common" +) + +// entryMapField selects which of an Entry's string maps a metadata command +// operates on, so the annotations and labels commands can share one +// implementation the way scheduler-k3s does. +type entryMapField struct { + Name string + Singular string + ReportType string + RowLabel string + Get func(*Entry) map[string]string + Set func(*Entry, map[string]string) +} + +var annotationsField = entryMapField{ + Name: "annotations", + Singular: "annotation", + ReportType: "storage-annotations", + RowLabel: "Annotation", + Get: func(e *Entry) map[string]string { return e.Annotations }, + Set: func(e *Entry, m map[string]string) { e.Annotations = m }, +} + +var labelsField = entryMapField{ + Name: "labels", + Singular: "label", + ReportType: "storage-labels", + RowLabel: "Label", + Get: func(e *Entry) map[string]string { return e.Labels }, + Set: func(e *Entry, m map[string]string) { e.Labels = m }, +} + +// CommandAnnotationsSet sets or clears a single annotation on an entry. +func CommandAnnotationsSet(name string, key string, value string) error { + return setEntryMapKey(annotationsField, name, key, value) +} + +// CommandLabelsSet sets or clears a single label on an entry. +func CommandLabelsSet(name string, key string, value string) error { + return setEntryMapKey(labelsField, name, key, value) +} + +// CommandAnnotationsReport displays the annotations on one entry, or on +// every entry when no name is given. +func CommandAnnotationsReport(name string, format string, infoFlag string) error { + return reportEntryMap(annotationsField, name, format, infoFlag) +} + +// CommandLabelsReport displays the labels on one entry, or on every entry +// when no name is given. +func CommandLabelsReport(name string, format string, infoFlag string) error { + return reportEntryMap(labelsField, name, format, infoFlag) +} + +// setEntryMapKey writes a single key on one of an entry's metadata maps. +// An empty value deletes just that key, leaving its siblings in place - +// the behavior the wholesale --annotation flag could never express. +func setEntryMapKey(field entryMapField, name string, key string, value string) error { + if name == "" { + return errors.New("storage entry name is required") + } + if key == "" { + return fmt.Errorf("No %s key specified", field.Singular) + } + if !EntryExists(name) { + return fmt.Errorf("storage entry %q does not exist", name) + } + + entry, err := LoadEntry(name) + if err != nil { + return err + } + + values := field.Get(entry) + if value == "" { + delete(values, key) + common.LogInfo2Quiet(fmt.Sprintf("Unsetting %s %s", field.Singular, key)) + } else { + if values == nil { + values = map[string]string{} + } + values[key] = value + common.LogInfo2Quiet(fmt.Sprintf("Setting %s %s to %s", field.Singular, key, value)) + } + if len(values) == 0 { + // Drop the empty map so the omitempty tag keeps it out of the JSON. + values = nil + } + field.Set(entry, values) + + if err := entry.Validate(); err != nil { + return err + } + if err := SaveEntry(entry); err != nil { + return err + } + + // k3s renders annotations and labels onto the PVC and PV through the + // entry's helm release, so the cluster only sees this once the chart + // is re-applied. + if entry.Scheduler == SchedulerK3s { + if err := callSchedulerCreateTrigger(entry); err != nil { + return fmt.Errorf("scheduler refused %s change for %q: %w", field.Name, name, err) + } + } + return nil +} + +// reportEntryMap renders one entry's metadata map, or every entry's when +// no name is given. +func reportEntryMap(field entryMapField, name string, format string, infoFlag string) error { + if format != "stdout" && format != "text" && format != "json" { + return fmt.Errorf("Invalid format: %s", format) + } + if format == "json" && infoFlag != "" { + return errors.New("--format flag cannot be specified when specifying an info flag") + } + + if name != "" { + if !EntryExists(name) { + return fmt.Errorf("storage entry %q does not exist", name) + } + entry, err := LoadEntry(name) + if err != nil { + return err + } + return renderEntryMap(field, entry, format, infoFlag) + } + + if infoFlag != "" { + return fmt.Errorf("storage:%s:report requires a storage entry name when an info flag is specified", field.Name) + } + + entries, err := ListEntries() + if err != nil { + return err + } + if len(entries) == 0 { + common.LogInfo1Quiet("No storage entries registered") + return nil + } + for _, entry := range entries { + if err := renderEntryMap(field, entry, format, ""); err != nil { + return err + } + } + return nil +} + +// renderEntryMap prints one entry's metadata map. Keys are emitted +// verbatim rather than through common.ReportSingleApp, which rewrites +// dots and dashes into spaces and would mangle keys like +// app.kubernetes.io/part-of. +func renderEntryMap(field entryMapField, entry *Entry, format string, infoFlag string) error { + values := field.Get(entry) + + keys := []string{} + for key := range values { + keys = append(keys, key) + } + sort.Strings(keys) + + if infoFlag != "" { + flagPrefix := "--" + field.ReportType + "." + validFlags := []string{} + for _, key := range keys { + flag := flagPrefix + key + if flag == infoFlag { + fmt.Println(values[key]) + return nil + } + validFlags = append(validFlags, flag) + } + return fmt.Errorf("Invalid flag passed, valid flags: %s", strings.Join(validFlags, ", ")) + } + + if format == "json" { + flat := map[string]string{} + for _, key := range keys { + flat[key] = values[key] + } + data, err := json.Marshal(flat) + if err != nil { + return fmt.Errorf("Unable to marshal json: %w", err) + } + fmt.Println(string(data)) + return nil + } + + common.LogInfo2Quiet(fmt.Sprintf("%s %s information", entry.Name, field.Name)) + length := 31 + for _, key := range keys { + if label := fmt.Sprintf("%s %s:", field.RowLabel, key); len(label) > length { + length = len(label) + } + } + for _, key := range keys { + label := fmt.Sprintf("%s %s:", field.RowLabel, key) + common.LogVerbose(fmt.Sprintf("%s%s", common.RightPad(label, length, " "), values[key])) + } + return nil +} diff --git a/plugins/storage/commands_metadata_test.go b/plugins/storage/commands_metadata_test.go new file mode 100644 index 000000000..96b053e0a --- /dev/null +++ b/plugins/storage/commands_metadata_test.go @@ -0,0 +1,164 @@ +package storage + +import ( + "path/filepath" + "testing" + + . "github.com/onsi/gomega" +) + +// stageDockerLocalEntry writes a docker-local entry at the default host +// path so the metadata commands have something to operate on. +func stageDockerLocalEntry(t *testing.T, name string) *Entry { + t.Helper() + entry := &Entry{ + Name: name, + Scheduler: SchedulerDockerLocal, + HostPath: filepath.Join(GetStorageDirectory(), name), + } + if err := SaveEntry(entry); err != nil { + t.Fatalf("SaveEntry: %v", err) + } + return entry +} + +func TestSetEntryMapKeyAddsAndOverwrites(t *testing.T) { + RegisterTestingT(t) + withTempLibRoot(t) + stageDockerLocalEntry(t, "demo") + + Expect(setEntryMapKey(annotationsField, "demo", "first", "one")).To(Succeed()) + Expect(setEntryMapKey(annotationsField, "demo", "second", "two")).To(Succeed()) + + loaded, err := LoadEntry("demo") + Expect(err).NotTo(HaveOccurred()) + Expect(loaded.Annotations).To(Equal(map[string]string{"first": "one", "second": "two"})) + + Expect(setEntryMapKey(annotationsField, "demo", "first", "rewritten")).To(Succeed()) + loaded, err = LoadEntry("demo") + Expect(err).NotTo(HaveOccurred()) + Expect(loaded.Annotations).To(Equal(map[string]string{"first": "rewritten", "second": "two"})) +} + +// TestSetEntryMapKeyDeletesOneKey is the behavior the wholesale +// --annotation flag could never express: clearing one key without +// disturbing its siblings. +func TestSetEntryMapKeyDeletesOneKey(t *testing.T) { + RegisterTestingT(t) + withTempLibRoot(t) + stageDockerLocalEntry(t, "demo") + + Expect(setEntryMapKey(annotationsField, "demo", "first", "one")).To(Succeed()) + Expect(setEntryMapKey(annotationsField, "demo", "second", "two")).To(Succeed()) + Expect(setEntryMapKey(annotationsField, "demo", "first", "")).To(Succeed()) + + loaded, err := LoadEntry("demo") + Expect(err).NotTo(HaveOccurred()) + Expect(loaded.Annotations).To(Equal(map[string]string{"second": "two"})) +} + +// TestSetEntryMapKeyDropsEmptyMap keeps the omitempty tag honest: once the +// last key is gone the field should be absent from the JSON, not an empty +// object, and should still round-trip. +func TestSetEntryMapKeyDropsEmptyMap(t *testing.T) { + RegisterTestingT(t) + withTempLibRoot(t) + stageDockerLocalEntry(t, "demo") + + Expect(setEntryMapKey(labelsField, "demo", "only", "value")).To(Succeed()) + Expect(setEntryMapKey(labelsField, "demo", "only", "")).To(Succeed()) + + loaded, err := LoadEntry("demo") + Expect(err).NotTo(HaveOccurred()) + Expect(loaded.Labels).To(BeEmpty()) + + Expect(SaveEntry(loaded)).To(Succeed()) + reloaded, err := LoadEntry("demo") + Expect(err).NotTo(HaveOccurred()) + Expect(reloaded.Labels).To(BeEmpty()) +} + +func TestSetEntryMapKeyKeepsAnnotationsAndLabelsSeparate(t *testing.T) { + RegisterTestingT(t) + withTempLibRoot(t) + stageDockerLocalEntry(t, "demo") + + Expect(setEntryMapKey(annotationsField, "demo", "shared", "annotation")).To(Succeed()) + Expect(setEntryMapKey(labelsField, "demo", "shared", "label")).To(Succeed()) + + loaded, err := LoadEntry("demo") + Expect(err).NotTo(HaveOccurred()) + Expect(loaded.Annotations).To(Equal(map[string]string{"shared": "annotation"})) + Expect(loaded.Labels).To(Equal(map[string]string{"shared": "label"})) +} + +// TestSetEntryMapKeyPreservesSlashKeys covers Kubernetes-style keys, which +// are the common case for both annotations and labels. +func TestSetEntryMapKeyPreservesSlashKeys(t *testing.T) { + RegisterTestingT(t) + withTempLibRoot(t) + stageDockerLocalEntry(t, "demo") + + key := "backup.velero.io/backup-volumes" + Expect(setEntryMapKey(annotationsField, "demo", key, "demo")).To(Succeed()) + + loaded, err := LoadEntry("demo") + Expect(err).NotTo(HaveOccurred()) + Expect(loaded.Annotations).To(HaveKeyWithValue(key, "demo")) +} + +func TestSetEntryMapKeyValidatesInputs(t *testing.T) { + RegisterTestingT(t) + withTempLibRoot(t) + stageDockerLocalEntry(t, "demo") + + err := setEntryMapKey(annotationsField, "", "key", "value") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("name is required")) + + err = setEntryMapKey(annotationsField, "demo", "", "value") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("No annotation key specified")) + + err = setEntryMapKey(labelsField, "demo", "", "value") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("No label key specified")) + + err = setEntryMapKey(annotationsField, "missing", "key", "value") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("does not exist")) +} + +func TestReportEntryMapValidatesFormatAndInfoFlag(t *testing.T) { + RegisterTestingT(t) + withTempLibRoot(t) + stageDockerLocalEntry(t, "demo") + + err := reportEntryMap(annotationsField, "demo", "yaml", "") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("Invalid format")) + + err = reportEntryMap(annotationsField, "demo", "json", "--storage-annotations.first") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("cannot be specified when specifying an info flag")) + + // An info flag has no single answer across every entry, so it needs a name. + err = reportEntryMap(annotationsField, "", "stdout", "--storage-annotations.first") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("requires a storage entry name")) + + err = reportEntryMap(annotationsField, "missing", "stdout", "") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("does not exist")) +} + +func TestReportEntryMapRejectsUnknownInfoFlag(t *testing.T) { + RegisterTestingT(t) + withTempLibRoot(t) + stageDockerLocalEntry(t, "demo") + Expect(setEntryMapKey(annotationsField, "demo", "first", "one")).To(Succeed()) + + err := reportEntryMap(annotationsField, "demo", "stdout", "--storage-annotations.nope") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("Invalid flag passed, valid flags: --storage-annotations.first")) +} diff --git a/plugins/storage/src/commands/commands.go b/plugins/storage/src/commands/commands.go index f95b87220..7c9828af0 100644 --- a/plugins/storage/src/commands/commands.go +++ b/plugins/storage/src/commands/commands.go @@ -18,17 +18,21 @@ Manage mounted volumes Additional commands:` helpContent = ` + storage:annotations:report [] [], Displays annotations for one or more storage entries + storage:annotations:set [], Set or clear an annotation on a storage entry storage:create [] [flags], Register a named storage entry storage:destroy [--force] [--destroy-host-dir], Remove a named storage entry (must be unmounted from every app first) storage:ensure-directory [--chown option] , [DEPRECATED] use storage:create instead storage:exec [-- ...], Run a command (or shell) in a temporary container that mounts the entry storage:info [--format text|json], Show details for one storage entry + storage:labels:report [] [], Displays labels for one or more storage entries + storage:labels:set [], Set or clear a label on a storage entry storage:list [--format text|json], List bind mounts for app's container(s) (host:container) storage:list-entries [--scheduler s] [--format text|json], List registered storage entries storage:migrate [|--all], Re-run the legacy -v to attachment migration for an app storage:mount , Create a new bind mount storage:report [] [], Displays a storage report for one or more apps - storage:set [flags], Update a storage entry in place + storage:set [], Update a storage entry in place storage:unmount , Remove an existing bind mount storage:wait , Wait for a storage entry's PVC to be bound (k3s)` ) diff --git a/plugins/storage/src/subcommands/subcommands.go b/plugins/storage/src/subcommands/subcommands.go index ddc758467..59cd25408 100644 --- a/plugins/storage/src/subcommands/subcommands.go +++ b/plugins/storage/src/subcommands/subcommands.go @@ -19,6 +19,34 @@ func main() { switch subcommand { case "default": err = storage.CommandHelp() + case "annotations:set": + args := flag.NewFlagSet("storage:annotations:set", flag.ExitOnError) + args.Parse(os.Args[2:]) + err = storage.CommandAnnotationsSet(args.Arg(0), args.Arg(1), args.Arg(2)) + case "annotations:report": + args := flag.NewFlagSet("storage:annotations:report", flag.ExitOnError) + format := args.String("format", "stdout", "format: [ stdout | json ]") + reportArgs, flagErr := common.ParseReportArgs("storage", os.Args[2:]) + if flagErr != nil { + err = flagErr + break + } + args.Parse(reportArgs.OSArgs) + err = storage.CommandAnnotationsReport(args.Arg(0), *format, reportArgs.InfoFlag) + case "labels:set": + args := flag.NewFlagSet("storage:labels:set", flag.ExitOnError) + args.Parse(os.Args[2:]) + err = storage.CommandLabelsSet(args.Arg(0), args.Arg(1), args.Arg(2)) + case "labels:report": + args := flag.NewFlagSet("storage:labels:report", flag.ExitOnError) + format := args.String("format", "stdout", "format: [ stdout | json ]") + reportArgs, flagErr := common.ParseReportArgs("storage", os.Args[2:]) + if flagErr != nil { + err = flagErr + break + } + args.Parse(reportArgs.OSArgs) + err = storage.CommandLabelsReport(args.Arg(0), *format, reportArgs.InfoFlag) case "create": args := flag.NewFlagSet("storage:create", flag.ExitOnError) scheduler := args.String("scheduler", storage.SchedulerDockerLocal, "--scheduler: target scheduler (docker-local, k3s)") @@ -84,38 +112,73 @@ func main() { err = storage.CommandList(appName, *format) case "set": args := flag.NewFlagSet("storage:set", flag.ExitOnError) - size := args.String("size", "", "--size: new PVC size (k3s)") - accessMode := args.String("access-mode", "", "--access-mode: existing access mode (must match)") - storageClass := args.String("storage-class-name", "", "--storage-class-name: existing storage class (must match)") - namespace := args.String("namespace", "", "--namespace: new namespace") - chown := args.String("chown", "", "--chown: chown option") - mode := args.String("mode", "", "--mode: octal permissions for the host directory, such as 0755 (docker-local only)") - reclaim := args.String("reclaim-policy", "", "--reclaim-policy: reclaim policy for the underlying volume (Retain or Delete)") - annotations := args.StringSlice("annotation", nil, "--annotation key=value: PVC annotation (repeatable, replaces all)") - labels := args.StringSlice("label", nil, "--label key=value: PVC label (repeatable, replaces all)") + args.String("size", "", "--size: [DEPRECATED] use 'storage:set size '") + args.String("access-mode", "", "--access-mode: [DEPRECATED] use 'storage:set access-mode '") + args.String("storage-class-name", "", "--storage-class-name: [DEPRECATED] use 'storage:set storage-class-name '") + args.String("namespace", "", "--namespace: [DEPRECATED] use 'storage:set namespace '") + args.String("chown", "", "--chown: [DEPRECATED] use 'storage:set chown '") + args.String("mode", "", "--mode: [DEPRECATED] use 'storage:set mode '") + args.String("reclaim-policy", "", "--reclaim-policy: [DEPRECATED] use 'storage:set reclaim-policy '") + annotations := args.StringSlice("annotation", nil, "--annotation key=value: [DEPRECATED] use 'storage:annotations:set'") + labels := args.StringSlice("label", nil, "--label key=value: [DEPRECATED] use 'storage:labels:set'") args.Parse(os.Args[2:]) - annotMap, parseErr := parseKVPairs(*annotations) - if parseErr != nil { - err = parseErr + + input := storage.CommandSetInput{Name: args.Arg(0)} + property := args.Arg(1) + + for _, name := range storage.SettableProperties { + if !args.Changed(name) { + continue + } + value, lookupErr := args.GetString(name) + if lookupErr != nil { + err = lookupErr + break + } + input.Changes = append(input.Changes, storage.PropertyChange{Property: name, Value: value}) + } + if err != nil { break } - labelMap, parseErr := parseKVPairs(*labels) - if parseErr != nil { - err = parseErr + + usedMapFlags := args.Changed("annotation") || args.Changed("label") + if usedMapFlags { + annotMap, parseErr := parseKVPairs(*annotations) + if parseErr != nil { + err = parseErr + break + } + labelMap, parseErr := parseKVPairs(*labels) + if parseErr != nil { + err = parseErr + break + } + input.Annotations = annotMap + input.Labels = labelMap + } + + usedFlags := len(input.Changes) > 0 || usedMapFlags + if property != "" && usedFlags { + err = fmt.Errorf("storage:set accepts either a property and a value or flags, not both") break } - err = storage.CommandSet(storage.CommandSetInput{ - Name: args.Arg(0), - Size: *size, - AccessMode: *accessMode, - StorageClass: *storageClass, - Namespace: *namespace, - Chown: *chown, - Mode: *mode, - ReclaimPolicy: *reclaim, - Annotations: annotMap, - Labels: labelMap, - }) + if property == "" && !usedFlags { + err = fmt.Errorf("No property specified") + break + } + + if usedFlags { + if len(input.Changes) > 0 { + common.LogWarn("Deprecated: please use 'storage:set []' instead of flags") + } + if usedMapFlags { + common.LogWarn("Deprecated: please use 'storage:annotations:set' and 'storage:labels:set' instead of --annotation and --label") + } + } else { + input.Changes = append(input.Changes, storage.PropertyChange{Property: property, Value: args.Arg(2)}) + } + + err = storage.CommandSet(input) case "exec": args := flag.NewFlagSet("storage:exec", flag.ExitOnError) image := args.String("image", "", "--image: container image to use (default alpine:3)") diff --git a/plugins/storage/subcommands.go b/plugins/storage/subcommands.go index 7a38b23f0..08295653e 100644 --- a/plugins/storage/subcommands.go +++ b/plugins/storage/subcommands.go @@ -20,17 +20,21 @@ Manage mounted volumes Additional commands:` helpContent = ` + storage:annotations:report [] [], Displays annotations for one or more storage entries + storage:annotations:set [], Set or clear an annotation on a storage entry storage:create [] [flags], Register a named storage entry storage:destroy [--force] [--destroy-host-dir], Remove a named storage entry (must be unmounted from every app first) storage:ensure-directory [--chown option] , [DEPRECATED] use storage:create instead storage:exec [-- ...], Run a command (or shell) in a temporary container that mounts the entry storage:info [--format text|json], Show details for one storage entry + storage:labels:report [] [], Displays labels for one or more storage entries + storage:labels:set [], Set or clear a label on a storage entry storage:list [--format text|json], List bind mounts for app's container(s) (host:container) storage:list-entries [--scheduler s] [--format text|json], List registered storage entries storage:migrate [|--all], Re-run the legacy -v to attachment migration for an app storage:mount , Create a new bind mount storage:report [] [], Displays a storage report for one or more apps - storage:set [flags], Update a storage entry in place + storage:set [], Update a storage entry in place storage:unmount , Remove an existing bind mount storage:wait , Wait for a storage entry's PVC to be bound (k3s)` ) diff --git a/tests/unit/storage.bats b/tests/unit/storage-1.bats similarity index 69% rename from tests/unit/storage.bats rename to tests/unit/storage-1.bats index 327779a9a..1f702d95d 100644 --- a/tests/unit/storage.bats +++ b/tests/unit/storage-1.bats @@ -386,122 +386,6 @@ teardown() { assert_success } -@test "(storage) storage:create / storage:list-entries / storage:destroy" { - run /bin/bash -c "dokku storage:create rdmtest-entry" - echo "output: $output" - echo "status: $status" - assert_success - - run /bin/bash -c "dokku storage:list-entries --format json | jq -r '.[].name' | grep '^rdmtest-entry$'" - echo "output: $output" - echo "status: $status" - assert_success - assert_output "rdmtest-entry" - - run /bin/bash -c "dokku storage:info rdmtest-entry --format json | jq -r '.scheduler'" - echo "output: $output" - echo "status: $status" - assert_success - assert_output "docker-local" - - run /bin/bash -c "dokku storage:destroy rdmtest-entry --force" - echo "output: $output" - echo "status: $status" - assert_success -} - -@test "(storage:create) --chown sets directory ownership" { - run /bin/bash -c "dokku storage:create --chown herokuish rdmtest-chown" - echo "output: $output" - echo "status: $status" - assert_success - - run /bin/bash -c "stat -c '%u:%g' $DOKKU_LIB_ROOT/data/storage/rdmtest-chown" - echo "output: $output" - echo "status: $status" - assert_success - assert_output "32767:32767" - - run /bin/bash -c "dokku storage:destroy rdmtest-chown --force" - assert_success -} - -@test "(storage:create) --chown accepts a custom numeric uid" { - run /bin/bash -c "dokku storage:create --chown 1500 rdmtest-chown-numeric" - echo "output: $output" - echo "status: $status" - assert_success - - run /bin/bash -c "stat -c '%u:%g' $DOKKU_LIB_ROOT/data/storage/rdmtest-chown-numeric" - echo "output: $output" - echo "status: $status" - assert_success - assert_output "1500:1500" - - run /bin/bash -c "dokku storage:destroy rdmtest-chown-numeric --force" - assert_success -} - -@test "(storage:create) --chown rejects an out-of-bounds numeric uid" { - run /bin/bash -c "dokku storage:create --chown 65536 rdmtest-chown-oob" - echo "output: $output" - echo "status: $status" - assert_failure - assert_output_contains "Unsupported chown permissions" - - run /bin/bash -c "dokku storage:create --chown -1 rdmtest-chown-oob" - echo "output: $output" - echo "status: $status" - assert_failure - assert_output_contains "Unsupported chown permissions" - - run /bin/bash -c "dokku storage:list-entries --format json | jq -r '.[].name' | grep '^rdmtest-chown-oob$' || true" - assert_output "" -} - -@test "(storage:create) --chown rejects a non-default host path" { - custom_path="/tmp/rdmtest-chown-custom" - rm -rf "$custom_path" - - run /bin/bash -c "dokku storage:create --chown herokuish rdmtest-chown-custom $custom_path" - echo "output: $output" - echo "status: $status" - assert_failure - assert_output_contains "--chown is only supported when the storage entry uses the default host path" - - run /bin/bash -c "dokku storage:list-entries --format json | jq -r '.[].name' | grep '^rdmtest-chown-custom$' || true" - assert_output "" - - rm -rf "$custom_path" -} - -@test "(storage) storage:create rejects invalid names" { - # underscore: rejected - run /bin/bash -c "dokku storage:create rdmtest_invalid" - echo "output: $output" - echo "status: $status" - assert_failure - - # uppercase: rejected - run /bin/bash -c "dokku storage:create RdmTest" - echo "output: $output" - echo "status: $status" - assert_failure - - # 46 chars: too long - long_name=$(printf 'a%.0s' {1..46}) - run /bin/bash -c "dokku storage:create $long_name" - echo "output: $output" - echo "status: $status" - assert_failure - - # legacy- prefix: reserved - run /bin/bash -c "dokku storage:create legacy-foo" - echo "output: $output" - echo "status: $status" - assert_failure -} - @test "(storage) storage:create + storage:mount with named entry attaches multiple entries to one app" { run /bin/bash -c "dokku storage:create rdmtest-data" assert_success @@ -697,78 +581,6 @@ teardown() { assert_success } -@test "(storage) storage:destroy refuses to remove a still-mounted entry" { - run /bin/bash -c "dokku storage:create rdmtest-busy" - assert_success - run /bin/bash -c "dokku storage:mount $TEST_APP rdmtest-busy --container-dir /data" - assert_success - - run /bin/bash -c "dokku storage:destroy rdmtest-busy" - echo "output: $output" - echo "status: $status" - assert_failure - assert_output_contains "still mounted" - - run /bin/bash -c "dokku storage:unmount $TEST_APP rdmtest-busy" - assert_success - run /bin/bash -c "dokku storage:destroy rdmtest-busy --force" - assert_success -} - -@test "(storage:destroy) requires confirmation without --force" { - run /bin/bash -c "dokku storage:create rdmtest-confirm" - assert_success - - # No --force and no matching stdin: aborts, entry remains. - run /bin/bash -c "dokku storage:destroy rdmtest-confirm < /dev/null" - echo "output: $output" - echo "status: $status" - assert_failure - - run /bin/bash -c "dokku storage:list-entries --format json | jq -r '.[].name' | grep '^rdmtest-confirm$'" - echo "output: $output" - echo "status: $status" - assert_success - assert_output "rdmtest-confirm" - - # Matching confirmation via stdin: succeeds and removes the entry. - run /bin/bash -c "echo rdmtest-confirm | dokku storage:destroy rdmtest-confirm" - echo "output: $output" - echo "status: $status" - assert_success - - run /bin/bash -c "dokku storage:list-entries --format json | jq -r '.[].name' | grep '^rdmtest-confirm$' || true" - echo "output: $output" - echo "status: $status" - assert_output "" -} - -@test "(storage:destroy) --force skips confirmation" { - run /bin/bash -c "dokku storage:create rdmtest-force" - assert_success - - run /bin/bash -c "dokku storage:destroy rdmtest-force --force < /dev/null" - echo "output: $output" - echo "status: $status" - assert_success - - run /bin/bash -c "dokku storage:list-entries --format json | jq -r '.[].name' | grep '^rdmtest-force$' || true" - assert_output "" -} - -@test "(storage:destroy) global --force skips confirmation" { - run /bin/bash -c "dokku storage:create rdmtest-gforce" - assert_success - - run /bin/bash -c "dokku --force storage:destroy rdmtest-gforce < /dev/null" - echo "output: $output" - echo "status: $status" - assert_success - - run /bin/bash -c "dokku storage:list-entries --format json | jq -r '.[].name' | grep '^rdmtest-gforce$' || true" - assert_output "" -} - @test "(storage) storage:exec runs a non-interactive command and propagates exit code" { run /bin/bash -c "dokku storage:create rdmtest-exec" assert_success @@ -954,147 +766,6 @@ teardown() { assert_success } -@test "(storage:create) --mode sets directory permissions" { - run /bin/bash -c "dokku storage:create --mode 0777 rdmtest-mode" - echo "output: $output" - echo "status: $status" - assert_success - - run /bin/bash -c "stat -c '%a' $DOKKU_LIB_ROOT/data/storage/rdmtest-mode" - echo "output: $output" - echo "status: $status" - assert_success - assert_output "777" - - # the mode is stored on the entry in its canonical 4-digit form - run /bin/bash -c "dokku storage:info rdmtest-mode --format json | jq -r '.mode'" - echo "output: $output" - echo "status: $status" - assert_success - assert_output "0777" - - run /bin/bash -c "dokku storage:destroy rdmtest-mode --destroy-host-dir --force" - assert_success -} - -@test "(storage:create) --mode re-applies on an existing directory" { - # the default mode assertion below only holds for a freshly created directory - rm -rf "$DOKKU_LIB_ROOT/data/storage/rdmtest-mode-converge" - - run /bin/bash -c "dokku storage:create rdmtest-mode-converge" - echo "output: $output" - echo "status: $status" - assert_success - - run /bin/bash -c "stat -c '%a' $DOKKU_LIB_ROOT/data/storage/rdmtest-mode-converge" - assert_success - assert_output "755" - - # re-running create against the existing entry converges the directory - run /bin/bash -c "dokku storage:create --mode 0700 rdmtest-mode-converge" - echo "output: $output" - echo "status: $status" - assert_success - - run /bin/bash -c "stat -c '%a' $DOKKU_LIB_ROOT/data/storage/rdmtest-mode-converge" - echo "output: $output" - echo "status: $status" - assert_success - assert_output "700" - - run /bin/bash -c "dokku storage:destroy rdmtest-mode-converge --destroy-host-dir --force" - assert_success -} - -@test "(storage:create) --mode accepts a 3 digit octal mode" { - run /bin/bash -c "dokku storage:create --mode 750 rdmtest-mode-short" - echo "output: $output" - echo "status: $status" - assert_success - - run /bin/bash -c "stat -c '%a' $DOKKU_LIB_ROOT/data/storage/rdmtest-mode-short" - echo "output: $output" - echo "status: $status" - assert_success - assert_output "750" - - run /bin/bash -c "dokku storage:info rdmtest-mode-short --format json | jq -r '.mode'" - assert_success - assert_output "0750" - - run /bin/bash -c "dokku storage:destroy rdmtest-mode-short --destroy-host-dir --force" - assert_success -} - -@test "(storage:create) --mode rejects an invalid value" { - run /bin/bash -c "dokku storage:create --mode 0888 rdmtest-mode-bad" - echo "output: $output" - echo "status: $status" - assert_failure - assert_output_contains "Unsupported directory mode" - - run /bin/bash -c "dokku storage:create --mode u+rwx rdmtest-mode-bad" - echo "output: $output" - echo "status: $status" - assert_failure - assert_output_contains "Unsupported directory mode" - - run /bin/bash -c "dokku storage:list-entries --format json | jq -r '.[].name' | grep '^rdmtest-mode-bad$' || true" - assert_output "" -} - -@test "(storage:create) --mode rejects a non-default host path" { - custom_path="/tmp/rdmtest-mode-custom" - rm -rf "$custom_path" - - run /bin/bash -c "dokku storage:create --mode 0777 rdmtest-mode-custom $custom_path" - echo "output: $output" - echo "status: $status" - assert_failure - assert_output_contains "--mode is only supported when the storage entry uses the default host path" - - run /bin/bash -c "dokku storage:list-entries --format json | jq -r '.[].name' | grep '^rdmtest-mode-custom$' || true" - assert_output "" - - rm -rf "$custom_path" -} - -@test "(storage:create) --mode is rejected on a k3s entry" { - run /bin/bash -c "dokku storage:create --scheduler k3s --size 1Gi --mode 0777 rdmtest-mode-k3s" - echo "output: $output" - echo "status: $status" - assert_failure - assert_output_contains "does not accept --mode" - - run /bin/bash -c "dokku storage:list-entries --format json | jq -r '.[].name' | grep '^rdmtest-mode-k3s$' || true" - assert_output "" -} - -@test "(storage:set) --mode updates permissions on an existing entry" { - run /bin/bash -c "dokku storage:create --mode 0755 rdmtest-mode-set" - echo "output: $output" - echo "status: $status" - assert_success - - run /bin/bash -c "dokku storage:set rdmtest-mode-set --mode 0770" - echo "output: $output" - echo "status: $status" - assert_success - - run /bin/bash -c "stat -c '%a' $DOKKU_LIB_ROOT/data/storage/rdmtest-mode-set" - echo "output: $output" - echo "status: $status" - assert_success - assert_output "770" - - run /bin/bash -c "dokku storage:info rdmtest-mode-set --format json | jq -r '.mode'" - assert_success - assert_output "0770" - - run /bin/bash -c "dokku storage:destroy rdmtest-mode-set --destroy-host-dir --force" - assert_success -} - @test "(storage) chmod-storage-dir rejects invalid modes" { run /bin/bash -c "DOKKU_LIB_ROOT=$DOKKU_LIB_ROOT $PLUGIN_AVAILABLE_PATH/storage/bin/chmod-storage-dir $TEST_APP 0888" echo "output: $output" @@ -1135,106 +806,6 @@ teardown() { assert_success } -@test "(storage:destroy) leaves the host directory in place by default" { - run /bin/bash -c "dokku storage:create rdmtest-keep-dir" - echo "output: $output" - echo "status: $status" - assert_success - - run /bin/bash -c "dokku storage:destroy rdmtest-keep-dir --force" - echo "output: $output" - echo "status: $status" - assert_success - - run /bin/bash -c "test -d $DOKKU_LIB_ROOT/data/storage/rdmtest-keep-dir" - echo "output: $output" - echo "status: $status" - assert_success - - rm -rf "$DOKKU_LIB_ROOT/data/storage/rdmtest-keep-dir" -} - -@test "(storage:destroy) --destroy-host-dir removes a non-empty host directory" { - run /bin/bash -c "dokku storage:create rdmtest-drop-dir" - echo "output: $output" - echo "status: $status" - assert_success - - run /bin/bash -c "sudo touch $DOKKU_LIB_ROOT/data/storage/rdmtest-drop-dir/payload" - assert_success - - run /bin/bash -c "dokku storage:destroy rdmtest-drop-dir --destroy-host-dir --force" - echo "output: $output" - echo "status: $status" - assert_success - - run /bin/bash -c "test -d $DOKKU_LIB_ROOT/data/storage/rdmtest-drop-dir" - echo "output: $output" - echo "status: $status" - assert_failure - - run /bin/bash -c "dokku storage:list-entries --format json | jq -r '.[].name' | grep '^rdmtest-drop-dir$' || true" - assert_output "" -} - -@test "(storage:destroy) --reclaim-policy Delete removes the host directory" { - run /bin/bash -c "dokku storage:create --reclaim-policy Delete rdmtest-reclaim" - echo "output: $output" - echo "status: $status" - assert_success - - run /bin/bash -c "dokku storage:destroy rdmtest-reclaim --force" - echo "output: $output" - echo "status: $status" - assert_success - - run /bin/bash -c "test -d $DOKKU_LIB_ROOT/data/storage/rdmtest-reclaim" - echo "output: $output" - echo "status: $status" - assert_failure -} - -@test "(storage:create) --reclaim-policy Delete rejects a non-default host path" { - custom_path="/tmp/rdmtest-reclaim-custom" - rm -rf "$custom_path" - - run /bin/bash -c "dokku storage:create --reclaim-policy Delete rdmtest-reclaim-custom $custom_path" - echo "output: $output" - echo "status: $status" - assert_failure - assert_output_contains "default host path" - - run /bin/bash -c "dokku storage:list-entries --format json | jq -r '.[].name' | grep '^rdmtest-reclaim-custom$' || true" - assert_output "" - - rm -rf "$custom_path" -} - -@test "(storage:destroy) --destroy-host-dir refuses a non-default host path" { - custom_path="/tmp/rdmtest-drop-custom" - rm -rf "$custom_path" - mkdir -p "$custom_path" - - run /bin/bash -c "dokku storage:create rdmtest-drop-custom $custom_path" - echo "output: $output" - echo "status: $status" - assert_success - - run /bin/bash -c "dokku storage:destroy rdmtest-drop-custom --destroy-host-dir --force" - echo "output: $output" - echo "status: $status" - assert_failure - assert_output_contains "--destroy-host-dir is only supported when the storage entry uses the default host path" - - run /bin/bash -c "test -d $custom_path" - assert_success - - run /bin/bash -c "dokku storage:destroy rdmtest-drop-custom --force" - assert_success - - rm -rf "$custom_path" -} - @test "(storage) install trigger whitelists the storage directory helpers" { run /bin/bash -c "test -f /etc/sudoers.d/dokku-storage" echo "output: $output" diff --git a/tests/unit/storage-2.bats b/tests/unit/storage-2.bats new file mode 100644 index 000000000..5908732e1 --- /dev/null +++ b/tests/unit/storage-2.bats @@ -0,0 +1,844 @@ +#!/usr/bin/env bats + +load test_helper + +setup() { + global_setup + create_app + rm -rf "$DOKKU_LIB_ROOT/data/storage/rdmtestapp*" +} + +teardown() { + destroy_app + global_teardown +} + +@test "(storage) storage:create / storage:list-entries / storage:destroy" { + run /bin/bash -c "dokku storage:create rdmtest-entry" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku storage:list-entries --format json | jq -r '.[].name' | grep '^rdmtest-entry$'" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "rdmtest-entry" + + run /bin/bash -c "dokku storage:info rdmtest-entry --format json | jq -r '.scheduler'" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "docker-local" + + run /bin/bash -c "dokku storage:destroy rdmtest-entry --force" + echo "output: $output" + echo "status: $status" + assert_success +} + +@test "(storage:create) --chown sets directory ownership" { + run /bin/bash -c "dokku storage:create --chown herokuish rdmtest-chown" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "stat -c '%u:%g' $DOKKU_LIB_ROOT/data/storage/rdmtest-chown" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "32767:32767" + + run /bin/bash -c "dokku storage:destroy rdmtest-chown --force" + assert_success +} + +@test "(storage:create) --chown accepts a custom numeric uid" { + run /bin/bash -c "dokku storage:create --chown 1500 rdmtest-chown-numeric" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "stat -c '%u:%g' $DOKKU_LIB_ROOT/data/storage/rdmtest-chown-numeric" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "1500:1500" + + run /bin/bash -c "dokku storage:destroy rdmtest-chown-numeric --force" + assert_success +} + +@test "(storage:create) --chown rejects an out-of-bounds numeric uid" { + run /bin/bash -c "dokku storage:create --chown 65536 rdmtest-chown-oob" + echo "output: $output" + echo "status: $status" + assert_failure + assert_output_contains "Unsupported chown permissions" + + run /bin/bash -c "dokku storage:create --chown -1 rdmtest-chown-oob" + echo "output: $output" + echo "status: $status" + assert_failure + assert_output_contains "Unsupported chown permissions" + + run /bin/bash -c "dokku storage:list-entries --format json | jq -r '.[].name' | grep '^rdmtest-chown-oob$' || true" + assert_output "" +} + +@test "(storage:create) --chown rejects a non-default host path" { + custom_path="/tmp/rdmtest-chown-custom" + rm -rf "$custom_path" + + run /bin/bash -c "dokku storage:create --chown herokuish rdmtest-chown-custom $custom_path" + echo "output: $output" + echo "status: $status" + assert_failure + assert_output_contains "--chown is only supported when the storage entry uses the default host path" + + run /bin/bash -c "dokku storage:list-entries --format json | jq -r '.[].name' | grep '^rdmtest-chown-custom$' || true" + assert_output "" + + rm -rf "$custom_path" +} + +@test "(storage) storage:create rejects invalid names" { + # underscore: rejected + run /bin/bash -c "dokku storage:create rdmtest_invalid" + echo "output: $output" + echo "status: $status" + assert_failure + + # uppercase: rejected + run /bin/bash -c "dokku storage:create RdmTest" + echo "output: $output" + echo "status: $status" + assert_failure + + # 46 chars: too long + long_name=$(printf 'a%.0s' {1..46}) + run /bin/bash -c "dokku storage:create $long_name" + echo "output: $output" + echo "status: $status" + assert_failure + + # legacy- prefix: reserved + run /bin/bash -c "dokku storage:create legacy-foo" + echo "output: $output" + echo "status: $status" + assert_failure +} + +@test "(storage) storage:destroy refuses to remove a still-mounted entry" { + run /bin/bash -c "dokku storage:create rdmtest-busy" + assert_success + run /bin/bash -c "dokku storage:mount $TEST_APP rdmtest-busy --container-dir /data" + assert_success + + run /bin/bash -c "dokku storage:destroy rdmtest-busy" + echo "output: $output" + echo "status: $status" + assert_failure + assert_output_contains "still mounted" + + run /bin/bash -c "dokku storage:unmount $TEST_APP rdmtest-busy" + assert_success + run /bin/bash -c "dokku storage:destroy rdmtest-busy --force" + assert_success +} + +@test "(storage:destroy) requires confirmation without --force" { + run /bin/bash -c "dokku storage:create rdmtest-confirm" + assert_success + + # No --force and no matching stdin: aborts, entry remains. + run /bin/bash -c "dokku storage:destroy rdmtest-confirm < /dev/null" + echo "output: $output" + echo "status: $status" + assert_failure + + run /bin/bash -c "dokku storage:list-entries --format json | jq -r '.[].name' | grep '^rdmtest-confirm$'" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "rdmtest-confirm" + + # Matching confirmation via stdin: succeeds and removes the entry. + run /bin/bash -c "echo rdmtest-confirm | dokku storage:destroy rdmtest-confirm" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku storage:list-entries --format json | jq -r '.[].name' | grep '^rdmtest-confirm$' || true" + echo "output: $output" + echo "status: $status" + assert_output "" +} + +@test "(storage:destroy) --force skips confirmation" { + run /bin/bash -c "dokku storage:create rdmtest-force" + assert_success + + run /bin/bash -c "dokku storage:destroy rdmtest-force --force < /dev/null" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku storage:list-entries --format json | jq -r '.[].name' | grep '^rdmtest-force$' || true" + assert_output "" +} + +@test "(storage:destroy) global --force skips confirmation" { + run /bin/bash -c "dokku storage:create rdmtest-gforce" + assert_success + + run /bin/bash -c "dokku --force storage:destroy rdmtest-gforce < /dev/null" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku storage:list-entries --format json | jq -r '.[].name' | grep '^rdmtest-gforce$' || true" + assert_output "" +} + +@test "(storage:create) --mode sets directory permissions" { + run /bin/bash -c "dokku storage:create --mode 0777 rdmtest-mode" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "stat -c '%a' $DOKKU_LIB_ROOT/data/storage/rdmtest-mode" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "777" + + # the mode is stored on the entry in its canonical 4-digit form + run /bin/bash -c "dokku storage:info rdmtest-mode --format json | jq -r '.mode'" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "0777" + + run /bin/bash -c "dokku storage:destroy rdmtest-mode --destroy-host-dir --force" + assert_success +} + +@test "(storage:create) --mode re-applies on an existing directory" { + # the default mode assertion below only holds for a freshly created directory + rm -rf "$DOKKU_LIB_ROOT/data/storage/rdmtest-mode-converge" + + run /bin/bash -c "dokku storage:create rdmtest-mode-converge" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "stat -c '%a' $DOKKU_LIB_ROOT/data/storage/rdmtest-mode-converge" + assert_success + assert_output "755" + + # re-running create against the existing entry converges the directory + run /bin/bash -c "dokku storage:create --mode 0700 rdmtest-mode-converge" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "stat -c '%a' $DOKKU_LIB_ROOT/data/storage/rdmtest-mode-converge" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "700" + + run /bin/bash -c "dokku storage:destroy rdmtest-mode-converge --destroy-host-dir --force" + assert_success +} + +@test "(storage:create) --mode accepts a 3 digit octal mode" { + run /bin/bash -c "dokku storage:create --mode 750 rdmtest-mode-short" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "stat -c '%a' $DOKKU_LIB_ROOT/data/storage/rdmtest-mode-short" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "750" + + run /bin/bash -c "dokku storage:info rdmtest-mode-short --format json | jq -r '.mode'" + assert_success + assert_output "0750" + + run /bin/bash -c "dokku storage:destroy rdmtest-mode-short --destroy-host-dir --force" + assert_success +} + +@test "(storage:create) --mode rejects an invalid value" { + run /bin/bash -c "dokku storage:create --mode 0888 rdmtest-mode-bad" + echo "output: $output" + echo "status: $status" + assert_failure + assert_output_contains "Unsupported directory mode" + + run /bin/bash -c "dokku storage:create --mode u+rwx rdmtest-mode-bad" + echo "output: $output" + echo "status: $status" + assert_failure + assert_output_contains "Unsupported directory mode" + + run /bin/bash -c "dokku storage:list-entries --format json | jq -r '.[].name' | grep '^rdmtest-mode-bad$' || true" + assert_output "" +} + +@test "(storage:create) --mode rejects a non-default host path" { + custom_path="/tmp/rdmtest-mode-custom" + rm -rf "$custom_path" + + run /bin/bash -c "dokku storage:create --mode 0777 rdmtest-mode-custom $custom_path" + echo "output: $output" + echo "status: $status" + assert_failure + assert_output_contains "--mode is only supported when the storage entry uses the default host path" + + run /bin/bash -c "dokku storage:list-entries --format json | jq -r '.[].name' | grep '^rdmtest-mode-custom$' || true" + assert_output "" + + rm -rf "$custom_path" +} + +@test "(storage:create) --mode is rejected on a k3s entry" { + run /bin/bash -c "dokku storage:create --scheduler k3s --size 1Gi --mode 0777 rdmtest-mode-k3s" + echo "output: $output" + echo "status: $status" + assert_failure + assert_output_contains "does not accept --mode" + + run /bin/bash -c "dokku storage:list-entries --format json | jq -r '.[].name' | grep '^rdmtest-mode-k3s$' || true" + assert_output "" +} + +@test "(storage:set) mode converges the directory via the positional form" { + run /bin/bash -c "dokku storage:create --mode 0755 rdmtest-mode-set" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku storage:set rdmtest-mode-set mode 0770" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "stat -c '%a' $DOKKU_LIB_ROOT/data/storage/rdmtest-mode-set" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "770" + + run /bin/bash -c "dokku storage:info rdmtest-mode-set --format json | jq -r '.mode'" + assert_success + assert_output "0770" + + # the text renderer surfaces it too + run /bin/bash -c "dokku storage:info rdmtest-mode-set" + echo "output: $output" + echo "status: $status" + assert_success + assert_output_contains "Mode:" + assert_output_contains "0770" + + run /bin/bash -c "dokku storage:destroy rdmtest-mode-set --destroy-host-dir --force" + assert_success +} + +@test "(storage:set) the deprecated flag form still works and warns" { + run /bin/bash -c "dokku storage:create --mode 0755 rdmtest-set-flags" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku storage:set rdmtest-set-flags --mode 0770 2>&1" + echo "output: $output" + echo "status: $status" + assert_success + assert_output_contains "Deprecated:" + + run /bin/bash -c "stat -c '%a' $DOKKU_LIB_ROOT/data/storage/rdmtest-set-flags" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "770" + + run /bin/bash -c "dokku storage:destroy rdmtest-set-flags --destroy-host-dir --force" + assert_success +} + +@test "(storage:set) sets a property via the positional form" { + run /bin/bash -c "dokku storage:create rdmtest-set-prop" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku storage:set rdmtest-set-prop chown herokuish" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku storage:info rdmtest-set-prop --format json | jq -r '.chown'" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "herokuish" + + run /bin/bash -c "dokku storage:destroy rdmtest-set-prop --destroy-host-dir --force" + assert_success +} + +@test "(storage:set) unsets a property when the value is omitted" { + run /bin/bash -c "dokku storage:create --mode 0770 rdmtest-set-unset" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku storage:info rdmtest-set-unset --format json | jq -r '.mode'" + assert_success + assert_output "0770" + + run /bin/bash -c "dokku storage:set rdmtest-set-unset mode" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku storage:info rdmtest-set-unset --format json | jq -r '.mode // empty'" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "" + + run /bin/bash -c "dokku storage:destroy rdmtest-set-unset --destroy-host-dir --force" + assert_success +} + +@test "(storage:set) rejects an unknown property" { + run /bin/bash -c "dokku storage:create rdmtest-set-bad" + assert_success + + run /bin/bash -c "dokku storage:set rdmtest-set-bad bogus value" + echo "output: $output" + echo "status: $status" + assert_failure + assert_output_contains "Invalid property specified, valid properties include:" + + run /bin/bash -c "dokku storage:set rdmtest-set-bad" + echo "output: $output" + echo "status: $status" + assert_failure + assert_output_contains "No property specified" + + run /bin/bash -c "dokku storage:destroy rdmtest-set-bad --destroy-host-dir --force" + assert_success +} + +@test "(storage:set) rejects mixing a property with flags" { + run /bin/bash -c "dokku storage:create rdmtest-set-mixed" + assert_success + + run /bin/bash -c "dokku storage:set rdmtest-set-mixed mode 0770 --chown herokuish" + echo "output: $output" + echo "status: $status" + assert_failure + assert_output_contains "either a property and a value or flags, not both" + + run /bin/bash -c "dokku storage:destroy rdmtest-set-mixed --destroy-host-dir --force" + assert_success +} + +@test "(storage:set) refuses an in-place access-mode or storage-class change" { + run /bin/bash -c "dokku storage:create rdmtest-set-inplace" + assert_success + + run /bin/bash -c "dokku storage:set rdmtest-set-inplace access-mode ReadWriteMany" + echo "output: $output" + echo "status: $status" + assert_failure + assert_output_contains "cannot change access-mode in place" + + run /bin/bash -c "dokku storage:set rdmtest-set-inplace storage-class-name longhorn" + echo "output: $output" + echo "status: $status" + assert_failure + assert_output_contains "cannot change storage-class-name in place" + + run /bin/bash -c "dokku storage:destroy rdmtest-set-inplace --destroy-host-dir --force" + assert_success +} + +@test "(storage:destroy) leaves the host directory in place by default" { + run /bin/bash -c "dokku storage:create rdmtest-keep-dir" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku storage:destroy rdmtest-keep-dir --force" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "test -d $DOKKU_LIB_ROOT/data/storage/rdmtest-keep-dir" + echo "output: $output" + echo "status: $status" + assert_success + + rm -rf "$DOKKU_LIB_ROOT/data/storage/rdmtest-keep-dir" +} + +@test "(storage:destroy) --destroy-host-dir removes a non-empty host directory" { + run /bin/bash -c "dokku storage:create rdmtest-drop-dir" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "sudo touch $DOKKU_LIB_ROOT/data/storage/rdmtest-drop-dir/payload" + assert_success + + run /bin/bash -c "dokku storage:destroy rdmtest-drop-dir --destroy-host-dir --force" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "test -d $DOKKU_LIB_ROOT/data/storage/rdmtest-drop-dir" + echo "output: $output" + echo "status: $status" + assert_failure + + run /bin/bash -c "dokku storage:list-entries --format json | jq -r '.[].name' | grep '^rdmtest-drop-dir$' || true" + assert_output "" +} + +@test "(storage:destroy) --reclaim-policy Delete removes the host directory" { + run /bin/bash -c "dokku storage:create --reclaim-policy Delete rdmtest-reclaim" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku storage:destroy rdmtest-reclaim --force" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "test -d $DOKKU_LIB_ROOT/data/storage/rdmtest-reclaim" + echo "output: $output" + echo "status: $status" + assert_failure +} + +@test "(storage:create) --reclaim-policy Delete rejects a non-default host path" { + custom_path="/tmp/rdmtest-reclaim-custom" + rm -rf "$custom_path" + + run /bin/bash -c "dokku storage:create --reclaim-policy Delete rdmtest-reclaim-custom $custom_path" + echo "output: $output" + echo "status: $status" + assert_failure + assert_output_contains "default host path" + + run /bin/bash -c "dokku storage:list-entries --format json | jq -r '.[].name' | grep '^rdmtest-reclaim-custom$' || true" + assert_output "" + + rm -rf "$custom_path" +} + +@test "(storage:destroy) --destroy-host-dir refuses a non-default host path" { + custom_path="/tmp/rdmtest-drop-custom" + rm -rf "$custom_path" + mkdir -p "$custom_path" + + run /bin/bash -c "dokku storage:create rdmtest-drop-custom $custom_path" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku storage:destroy rdmtest-drop-custom --destroy-host-dir --force" + echo "output: $output" + echo "status: $status" + assert_failure + assert_output_contains "--destroy-host-dir is only supported when the storage entry uses the default host path" + + run /bin/bash -c "test -d $custom_path" + assert_success + + run /bin/bash -c "dokku storage:destroy rdmtest-drop-custom --force" + assert_success + + rm -rf "$custom_path" +} + +@test "(storage:annotations:set) sets and clears a single key" { + run /bin/bash -c "dokku storage:create rdmtest-annot" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku storage:annotations:set rdmtest-annot first one" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku storage:info rdmtest-annot --format json | jq -r '.annotations.first'" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "one" + + run /bin/bash -c "dokku storage:annotations:set rdmtest-annot first" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku storage:info rdmtest-annot --format json | jq -r '.annotations // empty'" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "" + + run /bin/bash -c "dokku storage:destroy rdmtest-annot --destroy-host-dir --force" + assert_success +} + +@test "(storage:annotations:set) leaves other keys untouched" { + run /bin/bash -c "dokku storage:create rdmtest-annot-multi" + assert_success + + run /bin/bash -c "dokku storage:annotations:set rdmtest-annot-multi first one" + assert_success + run /bin/bash -c "dokku storage:annotations:set rdmtest-annot-multi second two" + assert_success + + run /bin/bash -c "dokku storage:annotations:set rdmtest-annot-multi first" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku storage:info rdmtest-annot-multi --format json | jq -r '.annotations.second'" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "two" + + run /bin/bash -c "dokku storage:info rdmtest-annot-multi --format json | jq -r '.annotations.first // empty'" + assert_success + assert_output "" + + run /bin/bash -c "dokku storage:destroy rdmtest-annot-multi --destroy-host-dir --force" + assert_success +} + +@test "(storage:annotations:set) accepts a kubernetes-style key containing a slash" { + run /bin/bash -c "dokku storage:create rdmtest-annot-slash" + assert_success + + run /bin/bash -c "dokku storage:annotations:set rdmtest-annot-slash backup.velero.io/backup-volumes rdmtest-annot-slash" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku storage:info rdmtest-annot-slash --format json | jq -r '.annotations.\"backup.velero.io/backup-volumes\"'" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "rdmtest-annot-slash" + + run /bin/bash -c "dokku storage:destroy rdmtest-annot-slash --destroy-host-dir --force" + assert_success +} + +@test "(storage:annotations:report) reports one entry and every entry" { + run /bin/bash -c "dokku storage:create rdmtest-annot-rpt" + assert_success + run /bin/bash -c "dokku storage:annotations:set rdmtest-annot-rpt team billing" + assert_success + + run /bin/bash -c "dokku storage:annotations:report rdmtest-annot-rpt" + echo "output: $output" + echo "status: $status" + assert_success + assert_output_contains "rdmtest-annot-rpt annotations information" + assert_output_contains "Annotation team:" + assert_output_contains "billing" + + # without a name, every registered entry is covered + run /bin/bash -c "dokku storage:annotations:report" + echo "output: $output" + echo "status: $status" + assert_success + assert_output_contains "rdmtest-annot-rpt annotations information" + + run /bin/bash -c "dokku storage:destroy rdmtest-annot-rpt --destroy-host-dir --force" + assert_success +} + +@test "(storage:annotations:report) emits json and answers a single info flag" { + run /bin/bash -c "dokku storage:create rdmtest-annot-json" + assert_success + run /bin/bash -c "dokku storage:annotations:set rdmtest-annot-json team billing" + assert_success + + run /bin/bash -c "dokku storage:annotations:report rdmtest-annot-json --format json | jq -r '.team'" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "billing" + + run /bin/bash -c "dokku storage:annotations:report rdmtest-annot-json --storage-annotations.team" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "billing" + + run /bin/bash -c "dokku storage:annotations:report rdmtest-annot-json --storage-annotations.absent" + echo "output: $output" + echo "status: $status" + assert_failure + assert_output_contains "Invalid flag passed, valid flags:" + + run /bin/bash -c "dokku storage:destroy rdmtest-annot-json --destroy-host-dir --force" + assert_success +} + +@test "(storage:labels:set) sets and clears a single key" { + run /bin/bash -c "dokku storage:create rdmtest-label" + assert_success + + run /bin/bash -c "dokku storage:labels:set rdmtest-label app.kubernetes.io/part-of billing" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku storage:info rdmtest-label --format json | jq -r '.labels.\"app.kubernetes.io/part-of\"'" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "billing" + + # labels and annotations are stored separately + run /bin/bash -c "dokku storage:info rdmtest-label --format json | jq -r '.annotations // empty'" + assert_success + assert_output "" + + run /bin/bash -c "dokku storage:labels:set rdmtest-label app.kubernetes.io/part-of" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku storage:info rdmtest-label --format json | jq -r '.labels // empty'" + assert_success + assert_output "" + + run /bin/bash -c "dokku storage:destroy rdmtest-label --destroy-host-dir --force" + assert_success +} + +@test "(storage:labels:report) reports one entry" { + run /bin/bash -c "dokku storage:create rdmtest-label-rpt" + assert_success + run /bin/bash -c "dokku storage:labels:set rdmtest-label-rpt tier cache" + assert_success + + run /bin/bash -c "dokku storage:labels:report rdmtest-label-rpt" + echo "output: $output" + echo "status: $status" + assert_success + assert_output_contains "rdmtest-label-rpt labels information" + assert_output_contains "Label tier:" + assert_output_contains "cache" + + run /bin/bash -c "dokku storage:labels:report rdmtest-label-rpt --format json | jq -r '.tier'" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "cache" + + run /bin/bash -c "dokku storage:destroy rdmtest-label-rpt --destroy-host-dir --force" + assert_success +} + +@test "(storage:annotations:set) fails on a missing entry or key" { + run /bin/bash -c "dokku storage:annotations:set rdmtest-annot-absent key value" + echo "output: $output" + echo "status: $status" + assert_failure + assert_output_contains "does not exist" + + run /bin/bash -c "dokku storage:create rdmtest-annot-nokey" + assert_success + + run /bin/bash -c "dokku storage:annotations:set rdmtest-annot-nokey" + echo "output: $output" + echo "status: $status" + assert_failure + assert_output_contains "No annotation key specified" + + run /bin/bash -c "dokku storage:destroy rdmtest-annot-nokey --destroy-host-dir --force" + assert_success +} + +@test "(storage:destroy) --destroy-host-dir warns before the confirmation prompt" { + run /bin/bash -c "dokku storage:create rdmtest-drop-confirm" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "sudo touch $DOKKU_LIB_ROOT/data/storage/rdmtest-drop-confirm/payload" + assert_success + + run /bin/bash -c "echo rdmtest-drop-confirm | dokku storage:destroy rdmtest-drop-confirm --destroy-host-dir 2>&1" + echo "output: $output" + echo "status: $status" + assert_success + assert_output_contains "which will be removed along with its contents" + assert_output_contains "WARNING: Potentially Destructive Action" + + run /bin/bash -c "test -d $DOKKU_LIB_ROOT/data/storage/rdmtest-drop-confirm" + echo "output: $output" + echo "status: $status" + assert_failure +} + +@test "(storage:destroy) --destroy-host-dir is rejected on a k3s entry" { + # a k3s entry cannot be created without a cluster, since storage:create + # rolls the entry back when the scheduler trigger fails + entry_path="$DOKKU_LIB_ROOT/data/storage-registry/entries/rdmtest-drop-k3s.json" + run /bin/bash -c "echo '{\"name\":\"rdmtest-drop-k3s\",\"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:destroy rdmtest-drop-k3s --destroy-host-dir --force" + echo "output: $output" + echo "status: $status" + assert_failure + assert_output_contains "--destroy-host-dir only applies to docker-local storage entries" + + run /bin/bash -c "sudo rm -f $entry_path" + assert_success +} + +@test "(storage:set) the deprecated --annotation flag warns toward annotations:set" { + run /bin/bash -c "dokku storage:create rdmtest-set-annot" + assert_success + + run /bin/bash -c "dokku storage:set rdmtest-set-annot --annotation team=billing 2>&1" + echo "output: $output" + echo "status: $status" + assert_success + assert_output_contains "Deprecated:" + assert_output_contains "storage:annotations:set" + + run /bin/bash -c "dokku storage:info rdmtest-set-annot --format json | jq -r '.annotations.team'" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "billing" + + run /bin/bash -c "dokku storage:destroy rdmtest-set-annot --destroy-host-dir --force" + assert_success +}