From 9ac29e3b352a2b7f8e76edd5c6e2d1b7cb3e4a34 Mon Sep 17 00:00:00 2001 From: phatlc Date: Sat, 5 Sep 2026 22:10:29 +0700 Subject: [PATCH] serve docker: fix volume path being lost when the plugin restarts applyOptions consumes the "path" option into vol.Path rather than leaving it in vol.Options, but restoreState rebuilt the options with only fs and type. The explicit path was therefore dropped when the plugin restarted, and since fsString is rebuilt from those options the volume was remounted at the root of the remote instead of at its subpath. Before this change a volume created with type + path lost its path completely, and one created with remote + path silently fell back to the path of the connection string. With a backend whose credentials are scoped to the subpath the restored mount then failed every operation rather than serving the wrong directory. Feed the persisted path back like fs and type, so applyOptions applies the same precedence on restore that it applies when the volume is first created. Fixes #9853 --- cmd/serve/docker/volume.go | 3 + cmd/serve/docker/volume_test.go | 99 +++++++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+) create mode 100644 cmd/serve/docker/volume_test.go diff --git a/cmd/serve/docker/volume.go b/cmd/serve/docker/volume.go index 6555d5909..9279070cd 100644 --- a/cmd/serve/docker/volume.go +++ b/cmd/serve/docker/volume.go @@ -140,6 +140,9 @@ func (vol *Volume) restoreState(ctx context.Context, drv *Driver) error { volOpt := vol.Options volOpt["fs"] = vol.Fs volOpt["type"] = vol.Type + // applyOptions consumes "path" into vol.Path rather than leaving it in + // vol.Options, so it must be fed back explicitly like fs and type. + volOpt["path"] = vol.Path if err := vol.applyOptions(volOpt); err != nil { return err } diff --git a/cmd/serve/docker/volume_test.go b/cmd/serve/docker/volume_test.go new file mode 100644 index 000000000..078f4b5ea --- /dev/null +++ b/cmd/serve/docker/volume_test.go @@ -0,0 +1,99 @@ +package docker + +import ( + "context" + "encoding/json" + "fmt" + "os" + "path/filepath" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + _ "github.com/rclone/rclone/backend/local" +) + +// persistedCopy returns vol as it comes back from the plugin state file: +// only the exported fields survive being marshalled and reloaded. +func persistedCopy(t *testing.T, vol *Volume) *Volume { + t.Helper() + vol.prepareState() + data, err := json.Marshal(vol) + require.NoError(t, err) + restored := &Volume{} + require.NoError(t, json.Unmarshal(data, restored)) + return restored +} + +// TestRestoreStatePath checks that the on-remote path of a volume survives +// being reloaded from the plugin state file, for each of the option +// combinations that can carry one. +func TestRestoreStatePath(t *testing.T) { + ctx := context.Background() + for _, test := range []struct { + name string + volOpt VolOpts + fsString string + }{{ + name: "type and path", + volOpt: VolOpts{"type": "local", "path": "/tmp/path"}, + fsString: ":local:/tmp/path", + }, { + name: "remote only", + volOpt: VolOpts{"remote": "/tmp/remote"}, + fsString: ":local:/tmp/remote", + }, { + // an explicit path overrides the path of the connection string, + // and must keep doing so after a restart + name: "remote and path", + volOpt: VolOpts{"remote": "/tmp/remote", "path": "/tmp/path"}, + fsString: ":local:/tmp/path", + }, { + name: "type only", + volOpt: VolOpts{"type": "local"}, + fsString: ":local:", + }} { + t.Run(test.name, func(t *testing.T) { + drv := &Driver{ + root: t.TempDir(), + volumes: map[string]*Volume{}, + dummy: true, + } + vol, err := newVolume(ctx, "vol1", test.volOpt, drv) + require.NoError(t, err) + require.Equal(t, test.fsString, vol.fsString) + + restored := persistedCopy(t, vol) + require.NoError(t, restored.restoreState(ctx, drv)) + assert.Equal(t, test.fsString, restored.fsString) + assert.Equal(t, vol.Path, restored.Path) + }) + } +} + +// TestDriverRestoreStatePath checks the same through the real state file: a +// volume persisted by a previous plugin instance is reloaded with its path. +func TestDriverRestoreStatePath(t *testing.T) { + ctx := context.Background() + testDir := t.TempDir() + + state := fmt.Sprintf(`[{"name":"vol1","mountpoint":%q,"created":%q,"fs":"","type":"local","path":"/tmp/path","options":{},"mounts":[]}]`, + filepath.Join(testDir, "vol1"), time.Now().Format(time.RFC3339)) + statePath := filepath.Join(testDir, stateFile) + require.NoError(t, os.WriteFile(statePath, []byte(state), 0600)) + + drv := &Driver{ + root: testDir, + statePath: statePath, + volumes: map[string]*Volume{}, + dummy: true, + } + require.NoError(t, drv.restoreState(ctx)) + + vol, err := drv.getVolume("vol1") + require.NoError(t, err) + assert.Equal(t, ":local:/tmp/path", vol.fsString) + assert.Equal(t, "/tmp/path", vol.Path) +}