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