fix: storage:list reads from attachments, deprecate storage-list trigger

storage:list was calling the storage-list plugn trigger which read -v lines from docker-options. After the install-time migration drains those lines into the attachment store, that source is empty for every migrated app and for any app that only ever used storage:create + storage:mount. The fix moves CommandList to call a new in-process ListAppMountEntries helper that reads attachments directly, surfaces the entry name in JSON output via a new entry_name field, and falls back to the entry name as the host token for k3s entries with no host path so the colon form remains well-formed. The storage-list plugn trigger is kept for back-compat with external callers but emits a deprecation warning and now reads from the same attachment-driven source.

docs/development/plugin-triggers.md

@@ -2904,21 +2904,17 @@ DOKKU_SCHEDULER="$1"; APP="$2"; REMOVE_CONTAINERS="$3";
 # TODO
 ```
 
-### `storage-list`
+### `storage-list` (deprecated)
 
-- Description: Returns a list of storage mounts
-- Invoked by: `dokku storage:list` and `dokku deploy`
+> [!WARNING]
+> Deprecated as of 0.38.0. The trigger handler still functions for back-compat
+> with external plugins but emits a deprecation warning on every invocation.
+> In-process callers should use the `storage-app-mounts` trigger instead, or
+> the `storage` Go package directly.
+
+- Description: Returns a list of storage mounts for an app
+- Invoked by: external plugins (the core `storage:list` subcommand no longer uses it)
 - Arguments: `$APP $PHASE $FORMAT`
-- Example:
-
-```shell
-#!/usr/bin/env bash
-
-set -eo pipefail; [[ $DOKKU_TRACE ]] && set -x
-APP="$1"
-
-# TODO
-```
 
 ### `storage-app-mounts`
 

plugins/storage/attachment_test.go

@@ -1,11 +1,39 @@
 package storage
 
 import (
+	"encoding/json"
+	"os"
+	"path/filepath"
 	"testing"
 
 	. "github.com/onsi/gomega"
 )
 
+// writeAttachmentsFile drops the JSON-encoded attachment list into the
+// per-app property file directly, bypassing common.PropertyListWrite (which
+// calls SetPermissions and chowns to dokku:dokku, which a developer machine
+// or CI runner generally doesn't have).
+func writeAttachmentsFile(t *testing.T, root string, appName string, attachments []*Attachment) {
+	t.Helper()
+	dir := filepath.Join(root, "config", "storage", appName)
+	if err := os.MkdirAll(dir, 0755); err != nil {
+		t.Fatalf("mkdir attachment dir: %v", err)
+	}
+	path := filepath.Join(dir, AttachmentsProperty)
+	f, err := os.Create(path)
+	if err != nil {
+		t.Fatalf("create attachment file: %v", err)
+	}
+	defer f.Close()
+	for _, a := range attachments {
+		data, err := json.Marshal(a)
+		if err != nil {
+			t.Fatalf("marshal attachment: %v", err)
+		}
+		f.Write(append(data, '\n'))
+	}
+}
+
 func TestAttachmentValidate(t *testing.T) {
 	RegisterTestingT(t)
 
@@ -31,3 +59,103 @@ func TestAttachmentValidate(t *testing.T) {
 	badPhase := &Attachment{EntryName: "demo-data", ContainerPath: "/data", Phases: []string{"build"}}
 	Expect(badPhase.Validate()).To(HaveOccurred())
 }
+
+func TestListAppMountEntriesDockerLocal(t *testing.T) {
+	RegisterTestingT(t)
+	root := withTempLibRoot(t)
+
+	Expect(SaveEntry(&Entry{
+		Name:      "demo-data",
+		Scheduler: SchedulerDockerLocal,
+		HostPath:  "/var/lib/dokku/data/storage/demo-data",
+	})).To(Succeed())
+
+	writeAttachmentsFile(t, root, "demo", []*Attachment{
+		{
+			EntryName:     "demo-data",
+			ContainerPath: "/data",
+			Phases:        []string{PhaseDeploy, PhaseRun},
+			Readonly:      true,
+		},
+	})
+
+	rows, err := ListAppMountEntries("demo", PhaseDeploy)
+	Expect(err).NotTo(HaveOccurred())
+	Expect(rows).To(HaveLen(1))
+	Expect(rows[0].EntryName).To(Equal("demo-data"))
+	Expect(rows[0].HostPath).To(Equal("/var/lib/dokku/data/storage/demo-data"))
+	Expect(rows[0].ContainerPath).To(Equal("/data"))
+	Expect(rows[0].VolumeOptions).To(Equal("ro"))
+
+	// Run phase shows it too because the attachment includes both phases.
+	runRows, err := ListAppMountEntries("demo", PhaseRun)
+	Expect(err).NotTo(HaveOccurred())
+	Expect(runRows).To(HaveLen(1))
+
+	// formatStorageListEntry produces the legacy colon form including options.
+	Expect(formatStorageListEntry(rows[0])).To(Equal("/var/lib/dokku/data/storage/demo-data:/data:ro"))
+}
+
+func TestListAppMountEntriesK3sUsesEntryName(t *testing.T) {
+	RegisterTestingT(t)
+	root := withTempLibRoot(t)
+
+	// k3s entry has no host path because the cluster provisions it.
+	Expect(SaveEntry(&Entry{
+		Name:         "demo-pvc",
+		Scheduler:    SchedulerK3s,
+		Size:         "2Gi",
+		StorageClass: "longhorn",
+		AccessMode:   "ReadWriteOnce",
+	})).To(Succeed())
+
+	writeAttachmentsFile(t, root, "demo", []*Attachment{
+		{
+			EntryName:     "demo-pvc",
+			ContainerPath: "/data",
+			Phases:        []string{PhaseDeploy},
+			Subpath:       "uploads",
+		},
+	})
+
+	rows, err := ListAppMountEntries("demo", PhaseDeploy)
+	Expect(err).NotTo(HaveOccurred())
+	Expect(rows).To(HaveLen(1))
+	// HostPath falls back to the entry name so the colon form remains
+	// well-formed for callers that only know about the legacy shape.
+	Expect(rows[0].HostPath).To(Equal("demo-pvc"))
+	Expect(rows[0].EntryName).To(Equal("demo-pvc"))
+	Expect(rows[0].ContainerPath).To(Equal("/data"))
+	Expect(formatStorageListEntry(rows[0])).To(Equal("demo-pvc:/data"))
+}
+
+func TestListAppMountEntriesPhaseFilter(t *testing.T) {
+	RegisterTestingT(t)
+	root := withTempLibRoot(t)
+
+	Expect(SaveEntry(&Entry{
+		Name:      "demo-deploy-only",
+		Scheduler: SchedulerDockerLocal,
+		HostPath:  "/srv/deploy",
+	})).To(Succeed())
+	Expect(SaveEntry(&Entry{
+		Name:      "demo-run-only",
+		Scheduler: SchedulerDockerLocal,
+		HostPath:  "/srv/run",
+	})).To(Succeed())
+
+	writeAttachmentsFile(t, root, "demo", []*Attachment{
+		{EntryName: "demo-deploy-only", ContainerPath: "/d", Phases: []string{PhaseDeploy}},
+		{EntryName: "demo-run-only", ContainerPath: "/r", Phases: []string{PhaseRun}},
+	})
+
+	deployRows, err := ListAppMountEntries("demo", PhaseDeploy)
+	Expect(err).NotTo(HaveOccurred())
+	Expect(deployRows).To(HaveLen(1))
+	Expect(deployRows[0].EntryName).To(Equal("demo-deploy-only"))
+
+	runRows, err := ListAppMountEntries("demo", PhaseRun)
+	Expect(err).NotTo(HaveOccurred())
+	Expect(runRows).To(HaveLen(1))
+	Expect(runRows[0].EntryName).To(Equal("demo-run-only"))
+}

plugins/storage/storage.go

@@ -51,19 +51,18 @@ func CheckIfPathExists(appName string, mountPath string, phases []string) bool {
 	return false
 }
 
-// GetBindMounts returns the bind mounts for an app and phase
+// GetBindMounts returns the bind mounts for an app and phase, synthesized
+// from the attachment store. The returned strings use the legacy colon
+// form (host:container[:options]) so existing display paths keep
+// working unchanged.
 func GetBindMounts(appName string, phase string) ([]string, error) {
-	mounts := []string{}
-	options, err := dockeroptions.GetDockerOptionsForPhase(appName, phase)
+	entries, err := ListAppMountEntries(appName, phase)
 	if err != nil {
-		return mounts, err
+		return nil, err
 	}
-
-	for _, option := range options {
-		if strings.HasPrefix(option, "-v ") {
-			mount := strings.TrimPrefix(option, "-v ")
-			mounts = append(mounts, mount)
-		}
+	mounts := make([]string, 0, len(entries))
+	for _, entry := range entries {
+		mounts = append(mounts, formatStorageListEntry(entry))
 	}
 	return mounts, nil
 }
@@ -82,13 +81,71 @@ func GetBindMountsForDisplay(appName string, phase string) string {
 	return strings.Join(result, " ")
 }
 
-// StorageListEntry represents a storage mount entry for JSON output
+// StorageListEntry represents a storage mount entry for JSON output.
 type StorageListEntry struct {
+	EntryName     string `json:"entry_name,omitempty"`
 	HostPath      string `json:"host_path"`
 	ContainerPath string `json:"container_path"`
 	VolumeOptions string `json:"volume_options"`
 }
 
+// ListAppMountEntries returns one StorageListEntry per attachment on
+// an app for the requested phase, joining each attachment with its
+// referenced storage entry. Used by storage:list and the deprecated
+// storage-list trigger.
+func ListAppMountEntries(appName string, phase string) ([]StorageListEntry, error) {
+	if phase == "" {
+		phase = PhaseDeploy
+	}
+	attachments, err := AttachmentsForPhase(appName, phase)
+	if err != nil {
+		return nil, err
+	}
+
+	rows := make([]StorageListEntry, 0, len(attachments))
+	for _, attachment := range attachments {
+		entry, err := LoadEntry(attachment.EntryName)
+		if err != nil {
+			return nil, fmt.Errorf("attachment on %q references missing entry %q: %w", appName, attachment.EntryName, err)
+		}
+
+		host := entry.HostPath
+		if host == "" {
+			// k3s-only entries with no host path: surface the entry
+			// name as the host token so the colon-form output is
+			// well-formed and parseable.
+			host = entry.Name
+		}
+
+		options := ""
+		switch {
+		case attachment.Readonly && attachment.VolumeOptions != "":
+			options = "ro," + attachment.VolumeOptions
+		case attachment.Readonly:
+			options = "ro"
+		case attachment.VolumeOptions != "":
+			options = attachment.VolumeOptions
+		}
+
+		rows = append(rows, StorageListEntry{
+			EntryName:     entry.Name,
+			HostPath:      host,
+			ContainerPath: attachment.ContainerPath,
+			VolumeOptions: options,
+		})
+	}
+	return rows, nil
+}
+
+// formatStorageListEntry renders a StorageListEntry into the legacy
+// host:container[:options] colon form for textual output.
+func formatStorageListEntry(entry StorageListEntry) string {
+	if entry.VolumeOptions == "" {
+		return fmt.Sprintf("%s:%s", entry.HostPath, entry.ContainerPath)
+	}
+	return fmt.Sprintf("%s:%s:%s", entry.HostPath, entry.ContainerPath, entry.VolumeOptions)
+}
+
 // ParseMountPath parses a mount path into its components
 func ParseMountPath(mountPath string) StorageListEntry {
 	parts := strings.SplitN(mountPath, ":", 3)

plugins/storage/subcommands.go

@@ -1,6 +1,7 @@
 package storage
 
 import (
+	"encoding/json"
 	"errors"
 	"fmt"
 	"os"
@@ -226,7 +227,9 @@ func CommandUnmount(input CommandUnmountInput) error {
 	return RemoveAttachment(input.AppName, input.NameOrPath, input.ContainerDir)
 }
 
-// CommandList lists all bind mounts for an app
+// CommandList lists all bind mounts for an app. Reads attachments
+// directly from the storage plugin's own state rather than going through
+// the deprecated `storage-list` plugn trigger.
 func CommandList(appName string, format string) error {
 	if err := common.VerifyAppName(appName); err != nil {
 		return err
@@ -236,34 +239,29 @@ func CommandList(appName string, format string) error {
 		return errors.New("Invalid --format value specified")
 	}
 
-	if format == "text" {
-		common.LogInfo1Quiet(fmt.Sprintf("%s volume bind-mounts:", appName))
-	}
-
-	results, err := common.CallPlugnTrigger(common.PlugnTriggerInput{
-		Trigger: "storage-list",
-		Args:    []string{appName, "deploy", format},
-	})
+	rows, err := ListAppMountEntries(appName, PhaseDeploy)
 	if err != nil {
 		return err
 	}
 
-	output := results.StdoutContents()
-	if format == "text" && output != "" {
-		lines := strings.Split(strings.TrimSpace(output), "\n")
-		for _, line := range lines {
-			if line != "" {
-				if os.Getenv("DOKKU_QUIET_OUTPUT") != "" {
-					fmt.Println(line)
-				} else {
-					common.LogVerbose(line)
-				}
-			}
+	if format == "json" {
+		output, err := json.Marshal(rows)
+		if err != nil {
+			return err
 		}
-	} else if output != "" {
-		fmt.Println(output)
+		fmt.Println(string(output))
+		return nil
 	}
 
+	common.LogInfo1Quiet(fmt.Sprintf("%s volume bind-mounts:", appName))
+	for _, row := range rows {
+		line := formatStorageListEntry(row)
+		if os.Getenv("DOKKU_QUIET_OUTPUT") != "" {
+			fmt.Println(line)
+		} else {
+			common.LogVerbose(line)
+		}
+	}
 	return nil
 }
 

plugins/storage/triggers.go

@@ -125,30 +125,31 @@ func detectDistro() string {
 	return ""
 }
 
-// TriggerStorageList outputs storage mounts for an app
+// TriggerStorageList outputs storage mounts for an app.
+//
+// Deprecated: the storage-list plugn trigger is retained for back-compat
+// with external plugins that may still call it, but in-process callers
+// should use storage.ListAppMountEntries directly. A deprecation warning
+// is emitted on every invocation.
 func TriggerStorageList(appName string, phase string, format string) error {
-	mounts, err := GetBindMounts(appName, phase)
+	common.LogWarn("the storage-list plugn trigger is deprecated; use the storage-app-mounts trigger or the storage Go package directly")
+
+	rows, err := ListAppMountEntries(appName, phase)
 	if err != nil {
 		return err
 	}
 
 	if format == "json" {
-		entries := []StorageListEntry{}
-		for _, mount := range mounts {
-			entries = append(entries, ParseMountPath(mount))
-		}
-
-		output, err := json.Marshal(entries)
+		output, err := json.Marshal(rows)
 		if err != nil {
 			return err
 		}
 		fmt.Println(string(output))
-	} else {
-		for _, mount := range mounts {
-			fmt.Println(mount)
-		}
+		return nil
+	}
+	for _, row := range rows {
+		fmt.Println(formatStorageListEntry(row))
 	}
-
 	return nil
 }
 

tests/unit/storage.bats

@@ -251,11 +251,30 @@ teardown() {
   echo "status: $status"
   assert_success
 
+  run /bin/bash -c "dokku storage:list $TEST_APP --format json | jq -r '. | length'"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+  assert_output "2"
+
+  run /bin/bash -c "dokku storage:list $TEST_APP --format json | jq -r '.[].entry_name' | sort | xargs"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+  assert_output "rdmtest-cache rdmtest-data"
+
   # cleanup
   run /bin/bash -c "dokku storage:unmount $TEST_APP rdmtest-data"
   assert_success
   run /bin/bash -c "dokku storage:unmount $TEST_APP rdmtest-cache"
   assert_success
+
+  run /bin/bash -c "dokku storage:list $TEST_APP --format json | jq -r '. | length'"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+  assert_output "0"
+
   run /bin/bash -c "dokku storage:destroy rdmtest-data"
   assert_success
   run /bin/bash -c "dokku storage:destroy rdmtest-cache"