From 0e5eaf8ee32662182147f5f62c1bfebef66f5c47 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Thu, 14 Dec 2017 10:27:10 -0500 Subject: [PATCH] Ensure plugin returns correctly scoped paths Before this change, volume management was relying on the fact that everything the plugin mounts is visible on the host within the plugin's rootfs. In practice this caused some issues with mount leaks, so we changed the behavior such that mounts are not visible on the plugin's rootfs, but available outside of it, which breaks volume management. To fix the issue, allow the plugin to scope the path correctly rather than assuming that everything is visible in `p.Rootfs`. In practice this is just scoping the `PropagatedMount` paths to the correct host path. Signed-off-by: Brian Goff --- daemon/graphdriver/proxy.go | 3 +- daemon/logger/adapter.go | 5 ++- daemon/logger/plugin.go | 16 ++++----- pkg/plugingetter/getter.go | 2 +- pkg/plugins/plugins_unix.go | 8 ++--- pkg/plugins/plugins_windows.go | 9 +++-- plugin/manager_linux.go | 1 - plugin/v2/plugin.go | 12 ++++--- volume/drivers/adapter.go | 60 +++++++++++++++------------------- volume/drivers/extpoint.go | 8 ++--- volume/testutils/testutils.go | 4 +-- 11 files changed, 60 insertions(+), 68 deletions(-) diff --git a/daemon/graphdriver/proxy.go b/daemon/graphdriver/proxy.go index 81ef872ad9..c365922c1e 100644 --- a/daemon/graphdriver/proxy.go +++ b/daemon/graphdriver/proxy.go @@ -4,7 +4,6 @@ import ( "errors" "fmt" "io" - "path/filepath" "github.com/docker/docker/pkg/archive" "github.com/docker/docker/pkg/containerfs" @@ -143,7 +142,7 @@ func (d *graphDriverProxy) Get(id, mountLabel string) (containerfs.ContainerFS, if ret.Err != "" { err = errors.New(ret.Err) } - return containerfs.NewLocalContainerFS(filepath.Join(d.p.BasePath(), ret.Dir)), err + return containerfs.NewLocalContainerFS(d.p.ScopedPath(ret.Dir)), err } func (d *graphDriverProxy) Put(id string) error { diff --git a/daemon/logger/adapter.go b/daemon/logger/adapter.go index 5817913cbc..7fe49776af 100644 --- a/daemon/logger/adapter.go +++ b/daemon/logger/adapter.go @@ -3,7 +3,7 @@ package logger import ( "io" "os" - "strings" + "path/filepath" "sync" "time" @@ -19,7 +19,6 @@ type pluginAdapter struct { driverName string id string plugin logPlugin - basePath string fifoPath string capabilities Capability logInfo Info @@ -58,7 +57,7 @@ func (a *pluginAdapter) Close() error { a.mu.Lock() defer a.mu.Unlock() - if err := a.plugin.StopLogging(strings.TrimPrefix(a.fifoPath, a.basePath)); err != nil { + if err := a.plugin.StopLogging(filepath.Join("/", "run", "docker", "logging", a.id)); err != nil { return err } diff --git a/daemon/logger/plugin.go b/daemon/logger/plugin.go index bdccea5b21..f293a529a9 100644 --- a/daemon/logger/plugin.go +++ b/daemon/logger/plugin.go @@ -5,7 +5,6 @@ import ( "io" "os" "path/filepath" - "strings" "github.com/docker/docker/api/types/plugins/logdriver" getter "github.com/docker/docker/pkg/plugingetter" @@ -39,18 +38,20 @@ func getPlugin(name string, mode int) (Creator, error) { } d := &logPluginProxy{p.Client()} - return makePluginCreator(name, d, p.BasePath()), nil + return makePluginCreator(name, d, p.ScopedPath), nil } -func makePluginCreator(name string, l *logPluginProxy, basePath string) Creator { +func makePluginCreator(name string, l *logPluginProxy, scopePath func(s string) string) Creator { return func(logCtx Info) (logger Logger, err error) { defer func() { if err != nil { pluginGetter.Get(name, extName, getter.Release) } }() - root := filepath.Join(basePath, "run", "docker", "logging") - if err := os.MkdirAll(root, 0700); err != nil { + + unscopedPath := filepath.Join("/", "run", "docker", "logging") + logRoot := scopePath(unscopedPath) + if err := os.MkdirAll(logRoot, 0700); err != nil { return nil, err } @@ -59,8 +60,7 @@ func makePluginCreator(name string, l *logPluginProxy, basePath string) Creator driverName: name, id: id, plugin: l, - basePath: basePath, - fifoPath: filepath.Join(root, id), + fifoPath: filepath.Join(logRoot, id), logInfo: logCtx, } @@ -77,7 +77,7 @@ func makePluginCreator(name string, l *logPluginProxy, basePath string) Creator a.stream = stream a.enc = logdriver.NewLogEntryEncoder(a.stream) - if err := l.StartLogging(strings.TrimPrefix(a.fifoPath, basePath), logCtx); err != nil { + if err := l.StartLogging(filepath.Join(unscopedPath, id), logCtx); err != nil { return nil, errors.Wrapf(err, "error creating logger") } diff --git a/pkg/plugingetter/getter.go b/pkg/plugingetter/getter.go index b9ffa547a4..b90d4fa3f6 100644 --- a/pkg/plugingetter/getter.go +++ b/pkg/plugingetter/getter.go @@ -17,7 +17,7 @@ const ( type CompatPlugin interface { Client() *plugins.Client Name() string - BasePath() string + ScopedPath(string) string IsV1() bool } diff --git a/pkg/plugins/plugins_unix.go b/pkg/plugins/plugins_unix.go index 02f1da69a1..543f439fc7 100644 --- a/pkg/plugins/plugins_unix.go +++ b/pkg/plugins/plugins_unix.go @@ -2,8 +2,8 @@ package plugins -// BasePath returns the path to which all paths returned by the plugin are relative to. -// For v1 plugins, this always returns the host's root directory. -func (p *Plugin) BasePath() string { - return "/" +// ScopedPath returns the path scoped to the plugin's rootfs. +// For v1 plugins, this always returns the path unchanged as v1 plugins run directly on the host. +func (p *Plugin) ScopedPath(s string) string { + return s } diff --git a/pkg/plugins/plugins_windows.go b/pkg/plugins/plugins_windows.go index 3c8d8feb83..a46e3c109e 100644 --- a/pkg/plugins/plugins_windows.go +++ b/pkg/plugins/plugins_windows.go @@ -1,8 +1,7 @@ package plugins -// BasePath returns the path to which all paths returned by the plugin are relative to. -// For Windows v1 plugins, this returns an empty string, since the plugin is already aware -// of the absolute path of the mount. -func (p *Plugin) BasePath() string { - return "" +// ScopedPath returns the path scoped to the plugin's rootfs. +// For v1 plugins, this always returns the path unchanged as v1 plugins run directly on the host. +func (p *Plugin) ScopedPath(s string) string { + return s } diff --git a/plugin/manager_linux.go b/plugin/manager_linux.go index 00eaf869cc..9670cf8ae4 100644 --- a/plugin/manager_linux.go +++ b/plugin/manager_linux.go @@ -65,7 +65,6 @@ func (pm *Manager) enable(p *v2.Plugin, c *controller, force bool) error { } } } - return pm.pluginPostStart(p, c) } diff --git a/plugin/v2/plugin.go b/plugin/v2/plugin.go index a4c7c535f6..74adba680b 100644 --- a/plugin/v2/plugin.go +++ b/plugin/v2/plugin.go @@ -2,6 +2,7 @@ package v2 import ( "fmt" + "path/filepath" "strings" "sync" @@ -39,10 +40,13 @@ func (e ErrInadequateCapability) Error() string { return fmt.Sprintf("plugin does not provide %q capability", e.cap) } -// BasePath returns the path to which all paths returned by the plugin are relative to. -// For Plugin objects this returns the host path of the plugin container's rootfs. -func (p *Plugin) BasePath() string { - return p.Rootfs +// ScopedPath returns the path scoped to the plugin rootfs +func (p *Plugin) ScopedPath(s string) string { + if p.PluginObj.Config.PropagatedMount != "" && strings.HasPrefix(s, p.PluginObj.Config.PropagatedMount) { + // re-scope to the propagated mount path on the host + return filepath.Join(filepath.Dir(p.Rootfs), "propagated-mount", strings.TrimPrefix(s, p.PluginObj.Config.PropagatedMount)) + } + return filepath.Join(p.Rootfs, s) } // Client returns the plugin client. diff --git a/volume/drivers/adapter.go b/volume/drivers/adapter.go index 54d1ff2630..36aa521098 100644 --- a/volume/drivers/adapter.go +++ b/volume/drivers/adapter.go @@ -2,7 +2,6 @@ package volumedrivers import ( "errors" - "path/filepath" "strings" "time" @@ -16,7 +15,7 @@ var ( type volumeDriverAdapter struct { name string - baseHostPath string + scopePath func(s string) string capabilities *volume.Capability proxy *volumeDriverProxy } @@ -30,10 +29,10 @@ func (a *volumeDriverAdapter) Create(name string, opts map[string]string) (volum return nil, err } return &volumeAdapter{ - proxy: a.proxy, - name: name, - driverName: a.name, - baseHostPath: a.baseHostPath, + proxy: a.proxy, + name: name, + driverName: a.name, + scopePath: a.scopePath, }, nil } @@ -41,13 +40,6 @@ func (a *volumeDriverAdapter) Remove(v volume.Volume) error { return a.proxy.Remove(v.Name()) } -func hostPath(baseHostPath, path string) string { - if baseHostPath != "" { - path = filepath.Join(baseHostPath, path) - } - return path -} - func (a *volumeDriverAdapter) List() ([]volume.Volume, error) { ls, err := a.proxy.List() if err != nil { @@ -57,11 +49,11 @@ func (a *volumeDriverAdapter) List() ([]volume.Volume, error) { var out []volume.Volume for _, vp := range ls { out = append(out, &volumeAdapter{ - proxy: a.proxy, - name: vp.Name, - baseHostPath: a.baseHostPath, - driverName: a.name, - eMount: hostPath(a.baseHostPath, vp.Mountpoint), + proxy: a.proxy, + name: vp.Name, + scopePath: a.scopePath, + driverName: a.name, + eMount: a.scopePath(vp.Mountpoint), }) } return out, nil @@ -79,13 +71,13 @@ func (a *volumeDriverAdapter) Get(name string) (volume.Volume, error) { } return &volumeAdapter{ - proxy: a.proxy, - name: v.Name, - driverName: a.Name(), - eMount: v.Mountpoint, - createdAt: v.CreatedAt, - status: v.Status, - baseHostPath: a.baseHostPath, + proxy: a.proxy, + name: v.Name, + driverName: a.Name(), + eMount: v.Mountpoint, + createdAt: v.CreatedAt, + status: v.Status, + scopePath: a.scopePath, }, nil } @@ -122,13 +114,13 @@ func (a *volumeDriverAdapter) getCapabilities() volume.Capability { } type volumeAdapter struct { - proxy *volumeDriverProxy - name string - baseHostPath string - driverName string - eMount string // ephemeral host volume path - createdAt time.Time // time the directory was created - status map[string]interface{} + proxy *volumeDriverProxy + name string + scopePath func(string) string + driverName string + eMount string // ephemeral host volume path + createdAt time.Time // time the directory was created + status map[string]interface{} } type proxyVolume struct { @@ -149,7 +141,7 @@ func (a *volumeAdapter) DriverName() string { func (a *volumeAdapter) Path() string { if len(a.eMount) == 0 { mountpoint, _ := a.proxy.Path(a.name) - a.eMount = hostPath(a.baseHostPath, mountpoint) + a.eMount = a.scopePath(mountpoint) } return a.eMount } @@ -160,7 +152,7 @@ func (a *volumeAdapter) CachedPath() string { func (a *volumeAdapter) Mount(id string) (string, error) { mountpoint, err := a.proxy.Mount(a.name, id) - a.eMount = hostPath(a.baseHostPath, mountpoint) + a.eMount = a.scopePath(mountpoint) return a.eMount, err } diff --git a/volume/drivers/extpoint.go b/volume/drivers/extpoint.go index c360b37a2b..56f78b9245 100644 --- a/volume/drivers/extpoint.go +++ b/volume/drivers/extpoint.go @@ -25,9 +25,9 @@ var drivers = &driverExtpoint{ const extName = "VolumeDriver" // NewVolumeDriver returns a driver has the given name mapped on the given client. -func NewVolumeDriver(name string, baseHostPath string, c client) volume.Driver { +func NewVolumeDriver(name string, scopePath func(string) string, c client) volume.Driver { proxy := &volumeDriverProxy{c} - return &volumeDriverAdapter{name: name, baseHostPath: baseHostPath, proxy: proxy} + return &volumeDriverAdapter{name: name, scopePath: scopePath, proxy: proxy} } // volumeDriver defines the available functions that volume plugins must implement. @@ -129,7 +129,7 @@ func lookup(name string, mode int) (volume.Driver, error) { return nil, errors.Wrap(err, "error looking up volume plugin "+name) } - d := NewVolumeDriver(p.Name(), p.BasePath(), p.Client()) + d := NewVolumeDriver(p.Name(), p.ScopedPath, p.Client()) if err := validateDriver(d); err != nil { if mode > 0 { // Undo any reference count changes from the initial `Get` @@ -224,7 +224,7 @@ func GetAllDrivers() ([]volume.Driver, error) { continue } - ext := NewVolumeDriver(name, p.BasePath(), p.Client()) + ext := NewVolumeDriver(name, p.ScopedPath, p.Client()) if p.IsV1() { drivers.extensions[name] = ext } diff --git a/volume/testutils/testutils.go b/volume/testutils/testutils.go index 84ab55ff77..5a17a26b77 100644 --- a/volume/testutils/testutils.go +++ b/volume/testutils/testutils.go @@ -178,8 +178,8 @@ func (p *fakePlugin) IsV1() bool { return false } -func (p *fakePlugin) BasePath() string { - return "" +func (p *fakePlugin) ScopedPath(s string) string { + return s } type fakePluginGetter struct {