fix: make dokku_storage_ensure chown genuinely optional

docs/tasks/dokku_storage_ensure.md

@@ -15,7 +15,7 @@ Not supported - deprecated; storage state is exported via dokku_storage_mount.
 | Parameter | Type | Required | Default | Choices | Description |
 | --- | --- | --- | --- | --- | --- |
 | `app` | string | yes |  |  | Name of the app |
-| `chown` | string | no |  |  | Chown value to set |
+| `chown` | string | no |  | heroku, herokuish, paketo, root, false | Chown value to set |
 | `state` | string | no | present | present, absent | Desired state of the storage |
 
 ## Examples

tasks/storage_ensure_task.go

@@ -2,6 +2,8 @@ package tasks
 
 import (
 	"errors"
+	"strings"
+
 	"github.com/dokku/docket/subprocess"
 )
 
@@ -11,7 +13,7 @@ type StorageEnsureTask struct {
 	App string `required:"true" yaml:"app" description:"Name of the app"`
 
 	// Chown is the chown value to set
-	Chown string `required:"false" yaml:"chown,omitempty" description:"Chown value to set"`
+	Chown string `required:"false" yaml:"chown,omitempty" options:"heroku,herokuish,paketo,root,false" description:"Chown value to set"`
 
 	// State is the desired state of the storage
 	State State `required:"false" yaml:"state,omitempty" default:"present" options:"present,absent" description:"Desired state of the storage"`
@@ -75,13 +77,16 @@ func (t StorageEnsureTask) Execute() TaskOutputState {
 }
 
 // Validate checks the StorageEnsureTask's inputs without contacting the server.
+// chown is optional: an omitted value lets dokku apply its default (herokuish)
+// ownership. A non-empty value must name one of the ownership presets dokku's
+// storage:ensure-directory understands so a typo is caught before dispatch.
 func (t StorageEnsureTask) Validate() error {
-	if t.State == StatePresent {
+	if t.State == StatePresent && t.Chown != "" {
 		chownValues := map[string]bool{
 			"heroku": true, "herokuish": true, "paketo": true, "root": true, "false": true,
 		}
 		if !chownValues[t.Chown] {
-			return errors.New("invalid chown value specified")
+			return errors.New("'chown' must be one of heroku, herokuish, paketo, root, false")
 		}
 	}
 	if t.State == StateAbsent {
@@ -90,6 +95,17 @@ func (t StorageEnsureTask) Validate() error {
 	return nil
 }
 
+// ensureArgs builds the storage:ensure-directory command arguments. The
+// --chown flag is omitted when no chown value is set so the field stays
+// genuinely optional and dokku applies its default ownership.
+func (t StorageEnsureTask) ensureArgs() []string {
+	args := []string{"--quiet", "storage:ensure-directory"}
+	if t.Chown != "" {
+		args = append(args, "--chown", t.Chown)
+	}
+	return append(args, t.App)
+}
+
 // Plan reports the drift the StorageEnsureTask would produce. dokku does
 // not expose a probe for storage:ensure-directory, so the plan reports
 // drift unconditionally.
@@ -99,15 +115,16 @@ func (t StorageEnsureTask) Plan() PlanResult {
 	}
 	return DispatchPlan(t.State, map[State]func() PlanResult{
 		StatePresent: func() PlanResult {
+			args := t.ensureArgs()
 			inputs := []subprocess.ExecCommandInput{{
 				Command: "dokku",
-				Args:    []string{"--quiet", "storage:ensure-directory", "--chown", t.Chown, t.App},
+				Args:    args,
 			}}
 			return PlanResult{
 				InSync:    false,
 				Status:    PlanStatusModify,
 				Reason:    "directory presence not probed",
-				Mutations: []string{"storage:ensure-directory --chown " + t.Chown + " " + t.App},
+				Mutations: []string{strings.Join(args[1:], " ")},
 				Commands:  resolveCommands(inputs),
 				apply: func() TaskOutputState {
 					return runExecInputs(TaskOutputState{State: StateAbsent}, StatePresent, inputs)

tasks/storage_ensure_task_integration_test.go

@@ -26,3 +26,26 @@ func TestIntegrationStorageEnsure(t *testing.T) {
 		t.Errorf("expected state 'present', got '%s'", result.State)
 	}
 }
+
+func TestIntegrationStorageEnsureOmittedChown(t *testing.T) {
+	skipIfNoDokkuT(t)
+
+	appName := "docket-test-storage-nochown"
+
+	destroyApp(appName)
+	createApp(appName)
+	defer destroyApp(appName)
+
+	// chown omitted: dokku applies its default (herokuish) ownership.
+	task := StorageEnsureTask{
+		App:   appName,
+		State: StatePresent,
+	}
+	result := task.Execute()
+	if result.Error != nil {
+		t.Fatalf("failed to ensure storage without chown: %v", result.Error)
+	}
+	if result.State != StatePresent {
+		t.Errorf("expected state 'present', got '%s'", result.State)
+	}
+}

tasks/storage_ensure_task_test.go

@@ -19,7 +19,7 @@ func TestStorageEnsureValidChownValues(t *testing.T) {
 		result := task.Execute()
 		// These will fail because dokku isn't running, but should NOT fail
 		// due to invalid chown value
-		if result.Error != nil && result.Error.Error() == "invalid chown value specified" {
+		if result.Error != nil && result.Error.Error() == "'chown' must be one of heroku, herokuish, paketo, root, false" {
 			t.Errorf("chown value %q should be valid but was rejected", chown)
 		}
 	}
@@ -28,7 +28,7 @@ func TestStorageEnsureValidChownValues(t *testing.T) {
 func TestStorageEnsureInvalidChownValue(t *testing.T) {
 	task := StorageEnsureTask{App: "test-app", Chown: "packeto", State: StatePresent}
 	result := task.Execute()
-	if result.Error == nil || result.Error.Error() != "invalid chown value specified" {
+	if result.Error == nil || result.Error.Error() != "'chown' must be one of heroku, herokuish, paketo, root, false" {
 		t.Errorf("chown value 'packeto' (misspelled) should be rejected as invalid")
 	}
 }
@@ -41,10 +41,32 @@ func TestStorageEnsureAbsentStateReturnsError(t *testing.T) {
 	}
 }
 
-func TestStorageEnsureEmptyChown(t *testing.T) {
-	task := StorageEnsureTask{App: "test-app", Chown: "", State: StatePresent}
-	result := task.Execute()
-	if result.Error == nil || result.Error.Error() != "invalid chown value specified" {
-		t.Errorf("expected 'invalid chown value specified' error, got: %v", result.Error)
+func TestStorageEnsureOmittedChownAllowed(t *testing.T) {
+	task := StorageEnsureTask{App: "test-app", State: StatePresent}
+	if err := task.Validate(); err != nil {
+		t.Errorf("an omitted chown should pass validation, got: %v", err)
+	}
+}
+
+func TestStorageEnsureChownCommandShape(t *testing.T) {
+	task := StorageEnsureTask{App: "node-js-app", Chown: "herokuish", State: StatePresent}
+	args := task.ensureArgs()
+	want := []string{"--quiet", "storage:ensure-directory", "--chown", "herokuish", "node-js-app"}
+	if !equalStrings(args, want) {
+		t.Errorf("ensureArgs mismatch:\n  got: %v\n want: %v", args, want)
+	}
+}
+
+func TestStorageEnsureOmittedChownCommandShape(t *testing.T) {
+	task := StorageEnsureTask{App: "node-js-app", State: StatePresent}
+	args := task.ensureArgs()
+	want := []string{"--quiet", "storage:ensure-directory", "node-js-app"}
+	if !equalStrings(args, want) {
+		t.Errorf("ensureArgs mismatch:\n  got: %v\n want: %v", args, want)
+	}
+	for _, a := range args {
+		if a == "--chown" {
+			t.Errorf("omitted chown must not emit --chown flag: %v", args)
+		}
 	}
 }

tasks/validate_test.go

@@ -564,6 +564,40 @@ func TestValidateInvalidTaskInputSatisfied(t *testing.T) {
 	}
 }
 
+func TestValidateStorageEnsureOmittedChownValid(t *testing.T) {
+	// chown is optional on dokku_storage_ensure; omitting it lets dokku apply
+	// its default ownership, so the recipe validates without an
+	// invalid_task_input problem.
+	data := []byte(`---
+- tasks:
+    - dokku_storage_ensure:
+        app: node-js-app
+`)
+	problems := Validate(data, ValidateOptions{})
+	if n := countProblems(problems, "invalid_task_input"); n != 0 {
+		t.Errorf("expected no invalid_task_input for an omitted chown, got %d: %+v", n, problems)
+	}
+}
+
+func TestValidateStorageEnsureInvalidChown(t *testing.T) {
+	// A non-empty chown that is not one of the known ownership presets is
+	// still rejected so a typo surfaces at validate time.
+	data := []byte(`---
+- tasks:
+    - dokku_storage_ensure:
+        app: node-js-app
+        chown: packeto
+`)
+	problems := Validate(data, ValidateOptions{})
+	p := findProblem(problems, "invalid_task_input")
+	if p == nil {
+		t.Fatalf("expected invalid_task_input problem, got: %+v", problems)
+	}
+	if !strings.Contains(p.Message, "'chown' must be one of") {
+		t.Errorf("expected message to mention the allowed chown values, got: %q", p.Message)
+	}
+}
+
 func TestValidateInvalidTaskInputSkippedWhenRequiredMissing(t *testing.T) {
 	// When a required field is missing, only missing_required_field is
 	// reported; the conditional Validate() check (which depends on that