Merge pull request #8712 from dokku/8708-storage-list-format-json-include-volume-options

Expose attachment readonly and volume_options in storage:list json

docs/advanced-usage/persistent-storage.md

@@ -57,9 +57,23 @@ dokku storage:list node-js-app --format json
 ```
 [
   {
+    "entry_name": "node-js-app",
     "host_path": "/var/lib/dokku/data/storage/node-js-app",
+    "container_path": "/app/storage"
+  }
+]
+```
+
+Each entry mirrors the underlying attachment. `readonly` (boolean) and `volume_options` (string) reflect `Attachment.Readonly` and `Attachment.VolumeOptions` directly and are only present when set, so external tooling can drift-detect attachments against the raw attachment fields. For example, a mount created with `--volume-options noexec,nosuid --volume-readonly` renders as:
+
+```
+[
+  {
+    "entry_name": "node-js-data",
+    "host_path": "/var/lib/dokku/data/storage/node-js-data",
     "container_path": "/app/storage",
-    "volume_options": ""
+    "readonly": true,
+    "volume_options": "noexec,nosuid"
   }
 ]
 ```

plugins/storage/attachment_test.go

@@ -85,7 +85,8 @@ func TestListAppMountEntriesDockerLocal(t *testing.T) {
 	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"))
+	Expect(rows[0].Readonly).To(BeTrue())
+	Expect(rows[0].VolumeOptions).To(BeEmpty())
 
 	// Run phase shows it too because the attachment includes both phases.
 	runRows, err := ListAppMountEntries("demo", PhaseRun)
@@ -164,11 +165,26 @@ func TestListAppMountEntriesVolumeOptions(t *testing.T) {
 		byPath[row.ContainerPath] = row
 	}
 
+	Expect(byPath["/data"].Readonly).To(BeFalse())
 	Expect(byPath["/data"].VolumeOptions).To(Equal("Z"))
 	Expect(formatStorageListEntry(byPath["/data"])).To(Equal("/var/lib/dokku/data/storage/demo-data:/data:Z"))
 
-	Expect(byPath["/ro"].VolumeOptions).To(Equal("ro,noexec,nosuid"))
+	Expect(byPath["/ro"].Readonly).To(BeTrue())
+	Expect(byPath["/ro"].VolumeOptions).To(Equal("noexec,nosuid"))
 	Expect(formatStorageListEntry(byPath["/ro"])).To(Equal("/var/lib/dokku/data/storage/demo-data:/ro:ro,noexec,nosuid"))
+
+	// The wire shape exposes Readonly and VolumeOptions as separate
+	// keys so external drift-detection tooling can compare them
+	// against the underlying Attachment fields one for one.
+	dataJSON, err := json.Marshal(byPath["/data"])
+	Expect(err).NotTo(HaveOccurred())
+	Expect(string(dataJSON)).To(ContainSubstring(`"volume_options":"Z"`))
+	Expect(string(dataJSON)).NotTo(ContainSubstring(`"readonly"`))
+
+	roJSON, err := json.Marshal(byPath["/ro"])
+	Expect(err).NotTo(HaveOccurred())
+	Expect(string(roJSON)).To(ContainSubstring(`"readonly":true`))
+	Expect(string(roJSON)).To(ContainSubstring(`"volume_options":"noexec,nosuid"`))
 }
 
 func TestListAppMountEntriesPhaseFilter(t *testing.T) {

plugins/storage/migrate.go

@@ -174,13 +174,8 @@ func migrateMount(appName string, mount string, phases []string) error {
 		ContainerPath: containerPath,
 		Phases:        phases,
 		ProcessType:   DefaultProcessType,
-	}
-	if parsed.VolumeOptions != "" {
-		if parsed.VolumeOptions == "ro" {
-			attachment.Readonly = true
-		} else {
-			attachment.VolumeOptions = parsed.VolumeOptions
-		}
+		Readonly:      parsed.Readonly,
+		VolumeOptions: parsed.VolumeOptions,
 	}
 
 	existing, err := LoadAttachments(appName)

plugins/storage/storage.go

@@ -82,11 +82,17 @@ func GetBindMountsForDisplay(appName string, phase string) string {
 }
 
 // StorageListEntry represents a storage mount entry for JSON output.
+// Readonly and VolumeOptions mirror the underlying Attachment fields one
+// for one so external drift-detection tooling can compare against them
+// directly; the combined "ro,<opts>" colon-form string used by the text
+// view is derived at format time by formatStorageListEntry rather than
+// cached on the struct.
 type StorageListEntry struct {
 	EntryName     string `json:"entry_name,omitempty"`
 	HostPath      string `json:"host_path"`
 	ContainerPath string `json:"container_path"`
-	VolumeOptions string `json:"volume_options"`
+	Readonly      bool   `json:"readonly,omitempty"`
+	VolumeOptions string `json:"volume_options,omitempty"`
 }
 
 // ListAppMountEntries returns one StorageListEntry per attachment on
@@ -117,36 +123,41 @@ func ListAppMountEntries(appName string, phase string) ([]StorageListEntry, erro
 			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,
+			Readonly:      attachment.Readonly,
+			VolumeOptions: attachment.VolumeOptions,
 		})
 	}
 	return rows, nil
 }
 
 // formatStorageListEntry renders a StorageListEntry into the legacy
-// host:container[:options] colon form for textual output.
+// host:container[:options] colon form for textual output. Combines
+// Readonly and VolumeOptions into a single "ro,<opts>" token to match
+// the historical shape consumers expect.
 func formatStorageListEntry(entry StorageListEntry) string {
-	if entry.VolumeOptions == "" {
+	options := ""
+	switch {
+	case entry.Readonly && entry.VolumeOptions != "":
+		options = "ro," + entry.VolumeOptions
+	case entry.Readonly:
+		options = "ro"
+	case entry.VolumeOptions != "":
+		options = entry.VolumeOptions
+	}
+	if options == "" {
 		return fmt.Sprintf("%s:%s", entry.HostPath, entry.ContainerPath)
 	}
-	return fmt.Sprintf("%s:%s:%s", entry.HostPath, entry.ContainerPath, entry.VolumeOptions)
+	return fmt.Sprintf("%s:%s:%s", entry.HostPath, entry.ContainerPath, options)
 }
 
-// ParseMountPath parses a mount path into its components
+// ParseMountPath parses a mount path into its components. The optional
+// third colon-separated section is a comma-separated mount-options list;
+// any "ro" token is hoisted into the Readonly field and the remaining
+// tokens (preserving order) are rejoined into VolumeOptions.
 func ParseMountPath(mountPath string) StorageListEntry {
 	parts := strings.SplitN(mountPath, ":", 3)
 	entry := StorageListEntry{}
@@ -157,8 +168,16 @@ func ParseMountPath(mountPath string) StorageListEntry {
 	if len(parts) >= 2 {
 		entry.ContainerPath = parts[1]
 	}
-	if len(parts) >= 3 {
-		entry.VolumeOptions = parts[2]
+	if len(parts) >= 3 && parts[2] != "" {
+		remaining := []string{}
+		for _, token := range strings.Split(parts[2], ",") {
+			if token == "ro" {
+				entry.Readonly = true
+				continue
+			}
+			remaining = append(remaining, token)
+		}
+		entry.VolumeOptions = strings.Join(remaining, ",")
 	}
 
 	return entry

plugins/storage/storage_test.go

@@ -69,16 +69,35 @@ func TestParseMountPath(t *testing.T) {
 	entry := ParseMountPath("/host/path:/container/path")
 	Expect(entry.HostPath).To(Equal("/host/path"))
 	Expect(entry.ContainerPath).To(Equal("/container/path"))
+	Expect(entry.Readonly).To(BeFalse())
 	Expect(entry.VolumeOptions).To(BeEmpty())
 
 	entry = ParseMountPath("/host/path:/container/path:ro")
 	Expect(entry.HostPath).To(Equal("/host/path"))
 	Expect(entry.ContainerPath).To(Equal("/container/path"))
-	Expect(entry.VolumeOptions).To(Equal("ro"))
+	Expect(entry.Readonly).To(BeTrue())
+	Expect(entry.VolumeOptions).To(BeEmpty())
+
+	entry = ParseMountPath("/host/path:/container/path:Z")
+	Expect(entry.HostPath).To(Equal("/host/path"))
+	Expect(entry.ContainerPath).To(Equal("/container/path"))
+	Expect(entry.Readonly).To(BeFalse())
+	Expect(entry.VolumeOptions).To(Equal("Z"))
+
+	entry = ParseMountPath("/host/path:/container/path:ro,Z")
+	Expect(entry.HostPath).To(Equal("/host/path"))
+	Expect(entry.ContainerPath).To(Equal("/container/path"))
+	Expect(entry.Readonly).To(BeTrue())
+	Expect(entry.VolumeOptions).To(Equal("Z"))
+
+	entry = ParseMountPath("/host/path:/container/path:noexec,nosuid,ro")
+	Expect(entry.Readonly).To(BeTrue())
+	Expect(entry.VolumeOptions).To(Equal("noexec,nosuid"))
 
 	entry = ParseMountPath("volume_name:/container/path")
 	Expect(entry.HostPath).To(Equal("volume_name"))
 	Expect(entry.ContainerPath).To(Equal("/container/path"))
+	Expect(entry.Readonly).To(BeFalse())
 	Expect(entry.VolumeOptions).To(BeEmpty())
 }
 

plugins/storage/subcommands.go

@@ -242,13 +242,8 @@ func mountLegacyColon(appName string, mountPath string) error {
 		ContainerPath: parsed.ContainerPath,
 		Phases:        []string{PhaseDeploy, PhaseRun},
 		ProcessType:   DefaultProcessType,
-	}
-	switch parsed.VolumeOptions {
-	case "":
-	case "ro":
-		attachment.Readonly = true
-	default:
-		attachment.VolumeOptions = parsed.VolumeOptions
+		Readonly:      parsed.Readonly,
+		VolumeOptions: parsed.VolumeOptions,
 	}
 
 	if err := AddAttachment(appName, attachment); err != nil {

tests/unit/storage.bats

@@ -137,11 +137,11 @@ teardown() {
   assert_success
   assert_output "/mount"
 
-  run /bin/bash -c "dokku storage:list $TEST_APP --format json | jq -r '.[].volume_options'"
+  run /bin/bash -c "dokku storage:list $TEST_APP --format json | jq -r 'map(has(\"volume_options\") or has(\"readonly\")) | any'"
   echo "output: $output"
   echo "status: $status"
   assert_success
-  assert_output_not_exists
+  assert_output "false"
 
   run /bin/bash -c "dokku storage:mount $TEST_APP /tmp/mount:/mount"
   echo "output: $output"
@@ -386,6 +386,28 @@ teardown() {
   assert_success
   assert_output_contains ":ro,noexec,nosuid"
 
+  # The JSON output of storage:list exposes readonly and volume_options
+  # as separate keys so external drift-detection tooling can compare
+  # them against the underlying attachment fields one for one.
+  run /bin/bash -c "dokku storage:list $TEST_APP --format json | jq -r '.[] | select(.container_path == \"/ro\") | .readonly'"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+  assert_output "true"
+
+  run /bin/bash -c "dokku storage:list $TEST_APP --format json | jq -r '.[] | select(.container_path == \"/ro\") | .volume_options'"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+  assert_output "noexec,nosuid"
+
+  # The /data mount has no readonly flag, so the key is absent.
+  run /bin/bash -c "dokku storage:list $TEST_APP --format json | jq -r '.[] | select(.container_path == \"/data\") | has(\"readonly\")'"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+  assert_output "false"
+
   # cleanup
   run /bin/bash -c "dokku storage:unmount $TEST_APP rdmtest-opts --container-dir /data"
   assert_success