diff --git a/docs/advanced-usage/persistent-storage.md b/docs/advanced-usage/persistent-storage.md index 70c837123..5eac24fe7 100644 --- a/docs/advanced-usage/persistent-storage.md +++ b/docs/advanced-usage/persistent-storage.md @@ -95,6 +95,8 @@ By default, permissions are set for usage with Herokuish buildpacks. These permi Users deploying via Dockerfile will want to specify `--chown false` and manually `chown` the created directory if the user and/or group id of the runnning process in the deployed container do not correspond to any of the above options. +The `--chown` flag - whether on `storage:create` or `storage:ensure-directory` - only manages the default `/var/lib/dokku/data/storage/` location. If a custom `` is passed to `storage:create`, the chown call is refused and the operator must chown the path themselves. + > [!WARNING] > Failing to set the correct directory ownership may result in issues in persisting files written to the mounted storage directory. diff --git a/plugins/storage/commands_entries.go b/plugins/storage/commands_entries.go index 80e104468..afbc459b5 100644 --- a/plugins/storage/commands_entries.go +++ b/plugins/storage/commands_entries.go @@ -495,6 +495,13 @@ func CommandListEntries(scheduler string, format string) error { // docker-local entry if it doesn't already exist. Idempotent: a // pre-existing directory is left in place. 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) + } + } + info, err := os.Stat(entry.HostPath) if err != nil && !os.IsNotExist(err) { return fmt.Errorf("unable to stat %s: %w", entry.HostPath, err) @@ -519,7 +526,7 @@ func ensureDockerLocalPath(entry *Entry) error { chownScript := filepath.Join(pluginPath, "storage", "bin", "chown-storage-dir") result, err := common.CallExecCommand(common.ExecCommandInput{ Command: "sudo", - Args: []string{chownScript, entry.HostPath, chownID}, + Args: []string{chownScript, entry.Name, chownID}, }) if err != nil { return fmt.Errorf("unable to chown %s: %w", entry.HostPath, err) diff --git a/tests/unit/storage.bats b/tests/unit/storage.bats index 6da3920fd..1ab58c27e 100644 --- a/tests/unit/storage.bats +++ b/tests/unit/storage.bats @@ -240,6 +240,38 @@ teardown() { 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 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"