fix: expose attachment readonly and volume_options in storage:list json
The `storage:list <app> --format json` payload conflated the attachment's `Readonly` flag and its `VolumeOptions` field into a single derived `volume_options` string that rendered as `ro`, `<options>`, or `ro,<options>` depending on which fields were set on the underlying attachment. That shape is fine for the legacy `host:container[:options]` text view but it leaves drift-detection tooling unable to tell whether a `ro` token came from `Attachment.Readonly == true` or from the operator setting `Attachment.VolumeOptions = "ro"` directly. The JSON now exposes `readonly` (boolean) and `volume_options` (string) as separate omitempty keys populated straight from the attachment, and `formatStorageListEntry` combines them at format time for the colon- form text view. `ParseMountPath` was extended in lockstep so callers of the legacy `host:container:opts` form no longer have to special- case the `ro` token themselves.
This commit is contained in:
@@ -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"
|
||||
}
|
||||
]
|
||||
```
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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())
|
||||
}
|
||||
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user