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/<name>` location, the same restriction `--chown` already carries. `storage:set` applies `--chown` and `--mode` to the directory rather than only recording them.
This commit is contained in:
Jose Diaz-Gonzalez
2026-08-10 01:23:10 -04:00
parent 39df6e3855
commit 87b240054f
14 changed files with 830 additions and 71 deletions

View File

@@ -7,7 +7,7 @@ The preferred method to attach persistent storage to a Dokku-managed container i
```
storage:create <name> [<path>] [flags] # Register a named storage entry
storage:destroy <name> [--force] # Remove a named storage entry (must be unmounted from every app first)
storage:destroy <name> [--force] [--destroy-host-dir] # Remove a named storage entry (must be unmounted from every app first)
storage:ensure-directory [--chown option] <directory> # [DEPRECATED] use storage:create instead
storage:exec <name> [-- <cmd>...] # Run a command (or shell) in a temporary container that mounts the entry
storage:info <name> [--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/<name>` location - it is refused for k3s entries and for entries created with a custom `<path>`.
### 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/<name>` location; an entry created with a custom `<path>` 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]

View File

@@ -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 "$@"

View File

@@ -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 "$@"

View File

@@ -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.).

View File

@@ -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())
}

View File

@@ -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/<name>.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)
}
}

View File

@@ -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())

View File

@@ -19,7 +19,7 @@ Additional commands:`
helpContent = `
storage:create <name> [<path>] [flags], Register a named storage entry
storage:destroy <name> [--force], Remove a named storage entry (must be unmounted from every app first)
storage:destroy <name> [--force] [--destroy-host-dir], Remove a named storage entry (must be unmounted from every app first)
storage:ensure-directory [--chown option] <directory>, [DEPRECATED] use storage:create instead
storage:exec <name> [-- <cmd>...], Run a command (or shell) in a temporary container that mounts the entry
storage:info <name> [--format text|json], Show details for one storage entry

View File

@@ -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,

View File

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

View File

@@ -21,7 +21,7 @@ Additional commands:`
helpContent = `
storage:create <name> [<path>] [flags], Register a named storage entry
storage:destroy <name> [--force], Remove a named storage entry (must be unmounted from every app first)
storage:destroy <name> [--force] [--destroy-host-dir], Remove a named storage entry (must be unmounted from every app first)
storage:ensure-directory [--chown option] <directory>, [DEPRECATED] use storage:create instead
storage:exec <name> [-- <cmd>...], Run a command (or shell) in a temporary container that mounts the entry
storage:info <name> [--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")

View File

@@ -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 {

View File

@@ -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 $$?

View File

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