From 89afd09087a01d6d9200ceedc785ac139c4d39f4 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Wed, 29 Apr 2026 22:36:00 -0400 Subject: [PATCH] fix: storage plugin install setup and post-delete permissions apps:destroy was failing in CI because the post-delete trigger called PropertyListWrite, which tried to mkdir /var/lib/dokku/config/storage/ as the dokku user but the parent directory had been created by the install trigger as root. The install trigger now calls PropertySetup("storage") so the per-plugin config root has the correct ownership, and post-delete / post-app-clone / post-app-rename short-circuit when the app has no attachments to begin with so they can't fail on a never-touched property tree. The entry registry and migration flag directories also moved out of config/storage (where they would have collided with the property-store path config/storage/) into a dedicated data/storage-registry tree. --- plugins/storage/entry.go | 13 +++++++++++-- plugins/storage/entry_test.go | 2 +- plugins/storage/migrate.go | 7 ++++--- plugins/storage/triggers.go | 18 +++++++++++++++++- 4 files changed, 33 insertions(+), 7 deletions(-) diff --git a/plugins/storage/entry.go b/plugins/storage/entry.go index 68ef5d79f..3fae69d66 100644 --- a/plugins/storage/entry.go +++ b/plugins/storage/entry.go @@ -76,10 +76,19 @@ type Entry struct { SchemaVersion int `json:"schema_version"` } +// RegistryDirectory returns the parent directory for storage-plugin +// state that lives outside the property store. Kept under data/ rather +// than config/ so it doesn't collide with the property-list paths +// `config/storage//` that the rest of the plugin +// relies on for per-app attachments. +func RegistryDirectory() string { + root := common.GetenvWithDefault("DOKKU_LIB_ROOT", "/var/lib/dokku") + return filepath.Join(root, "data", "storage-registry") +} + // EntriesDirectory returns the directory holding per-entry JSON files. func EntriesDirectory() string { - root := common.GetenvWithDefault("DOKKU_LIB_ROOT", "/var/lib/dokku") - return filepath.Join(root, "config", "storage", "entries") + return filepath.Join(RegistryDirectory(), "entries") } // entryPath returns the on-disk path for a named entry. diff --git a/plugins/storage/entry_test.go b/plugins/storage/entry_test.go index 683032008..804245b1f 100644 --- a/plugins/storage/entry_test.go +++ b/plugins/storage/entry_test.go @@ -138,7 +138,7 @@ func TestEntryRoundTrip(t *testing.T) { Expect(loaded.Labels).To(Equal(original.Labels)) Expect(loaded.SchemaVersion).To(Equal(SchemaVersion)) - expectedPath := filepath.Join(root, "config", "storage", "entries", "demo-data.json") + expectedPath := filepath.Join(root, "data", "storage-registry", "entries", "demo-data.json") Expect(expectedPath).To(BeARegularFile()) Expect(DeleteEntry("demo-data")).To(Succeed()) diff --git a/plugins/storage/migrate.go b/plugins/storage/migrate.go index 088ad4d83..3dba56eb0 100644 --- a/plugins/storage/migrate.go +++ b/plugins/storage/migrate.go @@ -14,10 +14,11 @@ import ( // migrationFlagDir contains per-app flag files written after a successful // per-app migration; presence means the app's legacy -v lines have been -// drained into the attachment store. +// drained into the attachment store. Lives under the registry directory +// so it doesn't collide with the property-store path +// `config/storage/`. func migrationFlagDir() string { - root := common.GetenvWithDefault("DOKKU_LIB_ROOT", "/var/lib/dokku") - return filepath.Join(root, "config", "storage", ".migrated") + return filepath.Join(RegistryDirectory(), "migrations") } func migrationFlagFile(appName string) string { diff --git a/plugins/storage/triggers.go b/plugins/storage/triggers.go index 389783e42..c8f8b30a6 100644 --- a/plugins/storage/triggers.go +++ b/plugins/storage/triggers.go @@ -14,6 +14,10 @@ import ( // TriggerInstall sets up the storage plugin on installation and runs // the bulk legacy-mount migration once per upgrade. func TriggerInstall() error { + if err := common.PropertySetup(PluginName); err != nil { + return fmt.Errorf("Unable to install the storage plugin: %s", err.Error()) + } + storageDir := GetStorageDirectory() if err := os.MkdirAll(storageDir, 0755); err != nil { @@ -55,11 +59,17 @@ func TriggerInstall() error { } // TriggerPostDelete clears any attachment list for an app that's being -// destroyed. Entries are global and survive app deletion. +// destroyed. Entries are global and survive app deletion. Skipped when +// the app has no attachments to begin with so we don't trigger a +// pointless propertyTouch that could fail on installs with no +// property-list state for this plugin yet. func TriggerPostDelete(appName string) error { if appName == "" { return nil } + if !common.PropertyExists(PluginName, appName, AttachmentsProperty) { + return nil + } return common.PropertyListWrite(PluginName, appName, AttachmentsProperty, []string{}) } @@ -69,6 +79,9 @@ func TriggerPostAppCloneSetup(oldName string, newName string) error { if oldName == "" || newName == "" { return nil } + if !common.PropertyExists(PluginName, oldName, AttachmentsProperty) { + return nil + } attachments, err := LoadAttachments(oldName) if err != nil { return err @@ -82,6 +95,9 @@ func TriggerPostAppRenameSetup(oldName string, newName string) error { if oldName == "" || newName == "" { return nil } + if !common.PropertyExists(PluginName, oldName, AttachmentsProperty) { + return nil + } attachments, err := LoadAttachments(oldName) if err != nil { return err