From c932e5d2a59cfcc7222dacb115dadb857a0554b8 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Wed, 13 Dec 2017 15:24:51 -0500 Subject: [PATCH 01/22] Use runtime spec modifier for metrics plugin hook Currently the metrics plugin uses a really hackish host mount with propagated mounts to get the metrics socket into a plugin after the plugin is alreay running. This approach ends up leaking mounts which requires setting the plugin manager root to private, which causes some other issues. With this change, plugin subsystems can register a set of modifiers to apply to the plugin's runtime spec before the plugin is ever started. This will help to generalize some of the customization work that needs to happen for various plugin subsystems (and future ones). Specifically it lets the metrics plugin subsystem append a mount to the runtime spec to mount the metrics socket in the plugin's mount namespace rather than the host's and prevetns any leaking due to this mount. Signed-off-by: Brian Goff Upstream-commit: 426e610e43179d58b29c496bc79a53f410a4b1e1 Component: engine --- components/engine/daemon/metrics.go | 22 --------- components/engine/daemon/metrics_unix.go | 52 +++++---------------- components/engine/plugin/defs.go | 15 +++++- components/engine/plugin/store.go | 43 +++++++++++++++-- components/engine/plugin/v2/plugin.go | 11 +++++ components/engine/plugin/v2/plugin_linux.go | 5 ++ 6 files changed, 81 insertions(+), 67 deletions(-) diff --git a/components/engine/daemon/metrics.go b/components/engine/daemon/metrics.go index 92439ad148..fef0b1d18f 100644 --- a/components/engine/daemon/metrics.go +++ b/components/engine/daemon/metrics.go @@ -1,10 +1,8 @@ package daemon import ( - "path/filepath" "sync" - "github.com/docker/docker/pkg/mount" "github.com/docker/docker/pkg/plugingetter" metrics "github.com/docker/go-metrics" "github.com/pkg/errors" @@ -132,18 +130,6 @@ func (d *Daemon) cleanupMetricsPlugins() { } } -type metricsPlugin struct { - plugingetter.CompatPlugin -} - -func (p metricsPlugin) sock() string { - return "metrics.sock" -} - -func (p metricsPlugin) sockBase() string { - return filepath.Join(p.BasePath(), "run", "docker") -} - func pluginStartMetricsCollection(p plugingetter.CompatPlugin) error { type metricsPluginResponse struct { Err string @@ -162,12 +148,4 @@ func pluginStopMetricsCollection(p plugingetter.CompatPlugin) { if err := p.Client().Call(metricsPluginType+".StopMetrics", nil, nil); err != nil { logrus.WithError(err).WithField("name", p.Name()).Error("error stopping metrics collector") } - - mp := metricsPlugin{p} - sockPath := filepath.Join(mp.sockBase(), mp.sock()) - if err := mount.Unmount(sockPath); err != nil { - if mounted, _ := mount.Mounted(sockPath); mounted { - logrus.WithError(err).WithField("name", p.Name()).WithField("socket", sockPath).Error("error unmounting metrics socket for plugin") - } - } } diff --git a/components/engine/daemon/metrics_unix.go b/components/engine/daemon/metrics_unix.go index 6045939a58..6df00c6541 100644 --- a/components/engine/daemon/metrics_unix.go +++ b/components/engine/daemon/metrics_unix.go @@ -5,13 +5,13 @@ package daemon import ( "net" "net/http" - "os" "path/filepath" - "github.com/docker/docker/pkg/mount" "github.com/docker/docker/pkg/plugingetter" "github.com/docker/docker/pkg/plugins" + "github.com/docker/docker/plugin" metrics "github.com/docker/go-metrics" + specs "github.com/opencontainers/runtime-spec/specs-go" "github.com/pkg/errors" "github.com/sirupsen/logrus" "golang.org/x/sys/unix" @@ -34,52 +34,22 @@ func (daemon *Daemon) listenMetricsSock() (string, error) { return path, nil } -func registerMetricsPluginCallback(getter plugingetter.PluginGetter, sockPath string) { - getter.Handle(metricsPluginType, func(name string, client *plugins.Client) { +func registerMetricsPluginCallback(store *plugin.Store, sockPath string) { + store.RegisterRuntimeOpt(metricsPluginType, func(s *specs.Spec) { + f := plugin.WithSpecMounts([]specs.Mount{ + {Type: "bind", Source: sockPath, Destination: "/run/docker/metrics.sock", Options: []string{"bind", "ro"}}, + }) + f(s) + }) + store.Handle(metricsPluginType, func(name string, client *plugins.Client) { // Use lookup since nothing in the system can really reference it, no need // to protect against removal - p, err := getter.Get(name, metricsPluginType, plugingetter.Lookup) + p, err := store.Get(name, metricsPluginType, plugingetter.Lookup) if err != nil { return } - mp := metricsPlugin{p} - sockBase := mp.sockBase() - if err := os.MkdirAll(sockBase, 0755); err != nil { - logrus.WithError(err).WithField("name", name).WithField("path", sockBase).Error("error creating metrics plugin base path") - return - } - - defer func() { - if err != nil { - os.RemoveAll(sockBase) - } - }() - - pluginSockPath := filepath.Join(sockBase, mp.sock()) - _, err = os.Stat(pluginSockPath) - if err == nil { - mount.Unmount(pluginSockPath) - } else { - logrus.WithField("path", pluginSockPath).Debugf("creating plugin socket") - f, err := os.OpenFile(pluginSockPath, os.O_CREATE, 0600) - if err != nil { - return - } - f.Close() - } - - if err := mount.Mount(sockPath, pluginSockPath, "none", "bind,ro"); err != nil { - logrus.WithError(err).WithField("name", name).Error("could not mount metrics socket to plugin") - return - } - if err := pluginStartMetricsCollection(p); err != nil { - if err := mount.Unmount(pluginSockPath); err != nil { - if mounted, _ := mount.Mounted(pluginSockPath); mounted { - logrus.WithError(err).WithField("sock_path", pluginSockPath).Error("error unmounting metrics socket from plugin during cleanup") - } - } logrus.WithError(err).WithField("name", name).Error("error while initializing metrics plugin") } }) diff --git a/components/engine/plugin/defs.go b/components/engine/plugin/defs.go index 3e930de048..3bbb5f93f9 100644 --- a/components/engine/plugin/defs.go +++ b/components/engine/plugin/defs.go @@ -5,12 +5,14 @@ import ( "github.com/docker/docker/pkg/plugins" "github.com/docker/docker/plugin/v2" + specs "github.com/opencontainers/runtime-spec/specs-go" ) // Store manages the plugin inventory in memory and on-disk type Store struct { sync.RWMutex - plugins map[string]*v2.Plugin + plugins map[string]*v2.Plugin + specOpts map[string][]SpecOpt /* handlers are necessary for transition path of legacy plugins * to the new model. Legacy plugins use Handle() for registering an * activation callback.*/ @@ -21,10 +23,14 @@ type Store struct { func NewStore() *Store { return &Store{ plugins: make(map[string]*v2.Plugin), + specOpts: make(map[string][]SpecOpt), handlers: make(map[string][]func(string, *plugins.Client)), } } +// SpecOpt is used for subsystems that need to modify the runtime spec of a plugin +type SpecOpt func(*specs.Spec) + // CreateOpt is used to configure specific plugin details when created type CreateOpt func(p *v2.Plugin) @@ -35,3 +41,10 @@ func WithSwarmService(id string) CreateOpt { p.SwarmServiceID = id } } + +// WithSpecMounts is a SpecOpt which appends the provided mounts to the runtime spec +func WithSpecMounts(mounts []specs.Mount) SpecOpt { + return func(s *specs.Spec) { + s.Mounts = append(s.Mounts, mounts...) + } +} diff --git a/components/engine/plugin/store.go b/components/engine/plugin/store.go index 9768c25068..b30fe55339 100644 --- a/components/engine/plugin/store.go +++ b/components/engine/plugin/store.go @@ -9,6 +9,7 @@ import ( "github.com/docker/docker/pkg/plugingetter" "github.com/docker/docker/pkg/plugins" "github.com/docker/docker/plugin/v2" + specs "github.com/opencontainers/runtime-spec/specs-go" "github.com/pkg/errors" "github.com/sirupsen/logrus" ) @@ -64,6 +65,10 @@ func (ps *Store) GetAll() map[string]*v2.Plugin { func (ps *Store) SetAll(plugins map[string]*v2.Plugin) { ps.Lock() defer ps.Unlock() + + for _, p := range plugins { + ps.setSpecOpts(p) + } ps.plugins = plugins } @@ -90,6 +95,22 @@ func (ps *Store) SetState(p *v2.Plugin, state bool) { p.PluginObj.Enabled = state } +func (ps *Store) setSpecOpts(p *v2.Plugin) { + var specOpts []SpecOpt + for _, typ := range p.GetTypes() { + opts, ok := ps.specOpts[typ.String()] + if ok { + specOpts = append(specOpts, opts...) + } + } + + p.SetSpecOptModifier(func(s *specs.Spec) { + for _, o := range specOpts { + o(s) + } + }) +} + // Add adds a plugin to memory and plugindb. // An error will be returned if there is a collision. func (ps *Store) Add(p *v2.Plugin) error { @@ -99,6 +120,9 @@ func (ps *Store) Add(p *v2.Plugin) error { if v, exist := ps.plugins[p.GetID()]; exist { return fmt.Errorf("plugin %q has the same ID %s as %q", p.Name(), p.GetID(), v.Name()) } + + ps.setSpecOpts(p) + ps.plugins[p.GetID()] = p return nil } @@ -182,20 +206,24 @@ func (ps *Store) GetAllByCap(capability string) ([]plugingetter.CompatPlugin, er return result, nil } +func pluginType(cap string) string { + return fmt.Sprintf("docker.%s/%s", strings.ToLower(cap), defaultAPIVersion) +} + // Handle sets a callback for a given capability. It is only used by network // and ipam drivers during plugin registration. The callback registers the // driver with the subsystem (network, ipam). func (ps *Store) Handle(capability string, callback func(string, *plugins.Client)) { - pluginType := fmt.Sprintf("docker.%s/%s", strings.ToLower(capability), defaultAPIVersion) + typ := pluginType(capability) // Register callback with new plugin model. ps.Lock() - handlers, ok := ps.handlers[pluginType] + handlers, ok := ps.handlers[typ] if !ok { handlers = []func(string, *plugins.Client){} } handlers = append(handlers, callback) - ps.handlers[pluginType] = handlers + ps.handlers[typ] = handlers ps.Unlock() // Register callback with legacy plugin model. @@ -204,6 +232,15 @@ func (ps *Store) Handle(capability string, callback func(string, *plugins.Client } } +// RegisterRuntimeOpt stores a list of SpecOpts for the provided capability. +// These options are applied to the runtime spec before a plugin is started for the specified capability. +func (ps *Store) RegisterRuntimeOpt(cap string, opts ...SpecOpt) { + ps.Lock() + defer ps.Unlock() + typ := pluginType(cap) + ps.specOpts[typ] = append(ps.specOpts[typ], opts...) +} + // CallHandler calls the registered callback. It is invoked during plugin enable. func (ps *Store) CallHandler(p *v2.Plugin) { for _, typ := range p.GetTypes() { diff --git a/components/engine/plugin/v2/plugin.go b/components/engine/plugin/v2/plugin.go index ce3257c0cb..a3e95f6cad 100644 --- a/components/engine/plugin/v2/plugin.go +++ b/components/engine/plugin/v2/plugin.go @@ -9,6 +9,7 @@ import ( "github.com/docker/docker/pkg/plugingetter" "github.com/docker/docker/pkg/plugins" "github.com/opencontainers/go-digest" + specs "github.com/opencontainers/runtime-spec/specs-go" ) // Plugin represents an individual plugin. @@ -23,6 +24,8 @@ type Plugin struct { Config digest.Digest Blobsums []digest.Digest + modifyRuntimeSpec func(*specs.Spec) + SwarmServiceID string } @@ -250,3 +253,11 @@ func (p *Plugin) Acquire() { func (p *Plugin) Release() { p.AddRefCount(plugingetter.Release) } + +// SetSpecOptModifier sets the function to use to modify the the generated +// runtime spec. +func (p *Plugin) SetSpecOptModifier(f func(*specs.Spec)) { + p.mu.Lock() + p.modifyRuntimeSpec = f + p.mu.Unlock() +} diff --git a/components/engine/plugin/v2/plugin_linux.go b/components/engine/plugin/v2/plugin_linux.go index 9590df4f7d..b31a4b4c23 100644 --- a/components/engine/plugin/v2/plugin_linux.go +++ b/components/engine/plugin/v2/plugin_linux.go @@ -16,6 +16,7 @@ import ( // InitSpec creates an OCI spec from the plugin's config. func (p *Plugin) InitSpec(execRoot string) (*specs.Spec, error) { s := oci.DefaultSpec() + s.Root = &specs.Root{ Path: p.Rootfs, Readonly: false, // TODO: all plugins should be readonly? settable in config? @@ -126,5 +127,9 @@ func (p *Plugin) InitSpec(execRoot string) (*specs.Spec, error) { caps.Inheritable = append(caps.Inheritable, p.PluginObj.Config.Linux.Capabilities...) caps.Effective = append(caps.Effective, p.PluginObj.Config.Linux.Capabilities...) + if p.modifyRuntimeSpec != nil { + p.modifyRuntimeSpec(&s) + } + return &s, nil } From 3928c278e5a9c08fd95d0da130e1f0915ae80087 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Thu, 14 Dec 2017 09:29:11 -0500 Subject: [PATCH 02/22] Plugins perform propagated mount in runtime spec Setting up the mounts on the host increases chances of mount leakage and makes for more cleanup after the plugin has stopped. With this change all mounts for the plugin are performed by the container runtime and automatically cleaned up when the container exits. Signed-off-by: Brian Goff Upstream-commit: a53930a04fa81b082aa78e66b342ff19cc63cc5f Component: engine --- .../engine/daemon/graphdriver/plugin.go | 2 +- components/engine/plugin/manager.go | 28 ++----------------- components/engine/plugin/manager_linux.go | 17 ++++------- components/engine/plugin/v2/plugin.go | 11 ++++---- components/engine/plugin/v2/plugin_linux.go | 21 ++++++++++---- 5 files changed, 30 insertions(+), 49 deletions(-) diff --git a/components/engine/daemon/graphdriver/plugin.go b/components/engine/daemon/graphdriver/plugin.go index 5d433e5196..9f560f0381 100644 --- a/components/engine/daemon/graphdriver/plugin.go +++ b/components/engine/daemon/graphdriver/plugin.go @@ -23,7 +23,7 @@ func newPluginDriver(name string, pl plugingetter.CompatPlugin, config Options) home := config.Root if !pl.IsV1() { if p, ok := pl.(*v2.Plugin); ok { - if p.PropagatedMount != "" { + if p.PluginObj.Config.PropagatedMount != "" { home = p.PluginObj.Config.PropagatedMount } } diff --git a/components/engine/plugin/manager.go b/components/engine/plugin/manager.go index d686443c6f..a163a2aa7e 100644 --- a/components/engine/plugin/manager.go +++ b/components/engine/plugin/manager.go @@ -19,7 +19,6 @@ import ( "github.com/docker/docker/layer" "github.com/docker/docker/pkg/authorization" "github.com/docker/docker/pkg/ioutils" - "github.com/docker/docker/pkg/mount" "github.com/docker/docker/pkg/pubsub" "github.com/docker/docker/pkg/system" "github.com/docker/docker/plugin/v2" @@ -151,16 +150,6 @@ func (pm *Manager) HandleExitEvent(id string) error { os.RemoveAll(filepath.Join(pm.config.ExecRoot, id)) - if p.PropagatedMount != "" { - if err := mount.Unmount(p.PropagatedMount); err != nil { - logrus.Warnf("Could not unmount %s: %v", p.PropagatedMount, err) - } - propRoot := filepath.Join(filepath.Dir(p.Rootfs), "propagated-mount") - if err := mount.Unmount(propRoot); err != nil { - logrus.Warn("Could not unmount %s: %v", propRoot, err) - } - } - pm.mu.RLock() c := pm.cMap[p] if c.exitChan != nil { @@ -239,28 +228,17 @@ func (pm *Manager) reload() error { // todo: restore // check if we need to migrate an older propagated mount from before // these mounts were stored outside the plugin rootfs if _, err := os.Stat(propRoot); os.IsNotExist(err) { - if _, err := os.Stat(p.PropagatedMount); err == nil { - // make sure nothing is mounted here - // don't care about errors - mount.Unmount(p.PropagatedMount) - if err := os.Rename(p.PropagatedMount, propRoot); err != nil { + rootfsProp := filepath.Join(p.Rootfs, p.PluginObj.Config.PropagatedMount) + if _, err := os.Stat(rootfsProp); err == nil { + if err := os.Rename(rootfsProp, propRoot); err != nil { logrus.WithError(err).WithField("dir", propRoot).Error("error migrating propagated mount storage") } - if err := os.MkdirAll(p.PropagatedMount, 0755); err != nil { - logrus.WithError(err).WithField("dir", p.PropagatedMount).Error("error migrating propagated mount storage") - } } } if err := os.MkdirAll(propRoot, 0755); err != nil { logrus.Errorf("failed to create PropagatedMount directory at %s: %v", propRoot, err) } - // TODO: sanitize PropagatedMount and prevent breakout - p.PropagatedMount = filepath.Join(p.Rootfs, p.PluginObj.Config.PropagatedMount) - if err := os.MkdirAll(p.PropagatedMount, 0755); err != nil { - logrus.Errorf("failed to create PropagatedMount directory at %s: %v", p.PropagatedMount, err) - return - } } } } diff --git a/components/engine/plugin/manager_linux.go b/components/engine/plugin/manager_linux.go index 65380af06f..00eaf869cc 100644 --- a/components/engine/plugin/manager_linux.go +++ b/components/engine/plugin/manager_linux.go @@ -22,7 +22,7 @@ import ( "golang.org/x/sys/unix" ) -func (pm *Manager) enable(p *v2.Plugin, c *controller, force bool) (err error) { +func (pm *Manager) enable(p *v2.Plugin, c *controller, force bool) error { p.Rootfs = filepath.Join(pm.config.Root, p.PluginObj.ID, "rootfs") if p.IsEnabled() && !force { return errors.Wrap(enabledError(p.Name()), "plugin already enabled") @@ -40,20 +40,16 @@ func (pm *Manager) enable(p *v2.Plugin, c *controller, force bool) (err error) { pm.mu.Unlock() var propRoot string - if p.PropagatedMount != "" { + if p.PluginObj.Config.PropagatedMount != "" { propRoot = filepath.Join(filepath.Dir(p.Rootfs), "propagated-mount") - if err = os.MkdirAll(propRoot, 0755); err != nil { + if err := os.MkdirAll(propRoot, 0755); err != nil { logrus.Errorf("failed to create PropagatedMount directory at %s: %v", propRoot, err) } - if err = mount.MakeRShared(propRoot); err != nil { + if err := mount.MakeRShared(propRoot); err != nil { return errors.Wrap(err, "error setting up propagated mount dir") } - - if err = mount.Mount(propRoot, p.PropagatedMount, "none", "rbind"); err != nil { - return errors.Wrap(err, "error creating mount for propagated mount") - } } rootFS := containerfs.NewLocalContainerFS(filepath.Join(pm.config.Root, p.PluginObj.ID, rootFSFileName)) @@ -63,10 +59,7 @@ func (pm *Manager) enable(p *v2.Plugin, c *controller, force bool) (err error) { stdout, stderr := makeLoggerStreams(p.GetID()) if err := pm.executor.Create(p.GetID(), *spec, stdout, stderr); err != nil { - if p.PropagatedMount != "" { - if err := mount.Unmount(p.PropagatedMount); err != nil { - logrus.Warnf("Could not unmount %s: %v", p.PropagatedMount, err) - } + if p.PluginObj.Config.PropagatedMount != "" { if err := mount.Unmount(propRoot); err != nil { logrus.Warnf("Could not unmount %s: %v", propRoot, err) } diff --git a/components/engine/plugin/v2/plugin.go b/components/engine/plugin/v2/plugin.go index a3e95f6cad..a4c7c535f6 100644 --- a/components/engine/plugin/v2/plugin.go +++ b/components/engine/plugin/v2/plugin.go @@ -14,12 +14,11 @@ import ( // Plugin represents an individual plugin. type Plugin struct { - mu sync.RWMutex - PluginObj types.Plugin `json:"plugin"` // todo: embed struct - pClient *plugins.Client - refCount int - PropagatedMount string // TODO: make private - Rootfs string // TODO: make private + mu sync.RWMutex + PluginObj types.Plugin `json:"plugin"` // todo: embed struct + pClient *plugins.Client + refCount int + Rootfs string // TODO: make private Config digest.Digest Blobsums []digest.Digest diff --git a/components/engine/plugin/v2/plugin_linux.go b/components/engine/plugin/v2/plugin_linux.go index b31a4b4c23..fa4be7a32c 100644 --- a/components/engine/plugin/v2/plugin_linux.go +++ b/components/engine/plugin/v2/plugin_linux.go @@ -4,6 +4,7 @@ import ( "os" "path/filepath" "runtime" + "sort" "strings" "github.com/docker/docker/api/types" @@ -32,6 +33,17 @@ func (p *Plugin) InitSpec(execRoot string) (*specs.Spec, error) { return nil, errors.WithStack(err) } + if p.PluginObj.Config.PropagatedMount != "" { + pRoot := filepath.Join(filepath.Dir(p.Rootfs), "propagated-mount") + s.Mounts = append(s.Mounts, specs.Mount{ + Source: pRoot, + Destination: p.PluginObj.Config.PropagatedMount, + Type: "bind", + Options: []string{"rbind", "rw", "rshared"}, + }) + s.Linux.RootfsPropagation = "rshared" + } + mounts := append(p.PluginObj.Config.Mounts, types.PluginMount{ Source: &execRoot, Destination: defaultPluginRuntimeDestination, @@ -89,11 +101,6 @@ func (p *Plugin) InitSpec(execRoot string) (*specs.Spec, error) { } } - if p.PluginObj.Config.PropagatedMount != "" { - p.PropagatedMount = filepath.Join(p.Rootfs, p.PluginObj.Config.PropagatedMount) - s.Linux.RootfsPropagation = "rshared" - } - if p.PluginObj.Config.Linux.AllowAllDevices { s.Linux.Resources.Devices = []specs.LinuxDeviceCgroup{{Allow: true, Access: "rwm"}} } @@ -131,5 +138,9 @@ func (p *Plugin) InitSpec(execRoot string) (*specs.Spec, error) { p.modifyRuntimeSpec(&s) } + sort.Slice(s.Mounts, func(i, j int) bool { + return s.Mounts[i].Destination < s.Mounts[j].Destination + }) + return &s, nil } From 90450b204476ffe0bd332d49238a6798386211bb Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Thu, 14 Dec 2017 10:27:10 -0500 Subject: [PATCH 03/22] 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 Upstream-commit: 0e5eaf8ee32662182147f5f62c1bfebef66f5c47 Component: engine --- components/engine/daemon/graphdriver/proxy.go | 3 +- components/engine/daemon/logger/adapter.go | 5 +- components/engine/daemon/logger/plugin.go | 16 ++--- components/engine/pkg/plugingetter/getter.go | 2 +- components/engine/pkg/plugins/plugins_unix.go | 8 +-- .../engine/pkg/plugins/plugins_windows.go | 9 ++- components/engine/plugin/manager_linux.go | 1 - components/engine/plugin/v2/plugin.go | 12 ++-- components/engine/volume/drivers/adapter.go | 60 ++++++++----------- components/engine/volume/drivers/extpoint.go | 8 +-- .../engine/volume/testutils/testutils.go | 4 +- 11 files changed, 60 insertions(+), 68 deletions(-) diff --git a/components/engine/daemon/graphdriver/proxy.go b/components/engine/daemon/graphdriver/proxy.go index 81ef872ad9..c365922c1e 100644 --- a/components/engine/daemon/graphdriver/proxy.go +++ b/components/engine/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/components/engine/daemon/logger/adapter.go b/components/engine/daemon/logger/adapter.go index 5817913cbc..7fe49776af 100644 --- a/components/engine/daemon/logger/adapter.go +++ b/components/engine/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/components/engine/daemon/logger/plugin.go b/components/engine/daemon/logger/plugin.go index bdccea5b21..f293a529a9 100644 --- a/components/engine/daemon/logger/plugin.go +++ b/components/engine/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/components/engine/pkg/plugingetter/getter.go b/components/engine/pkg/plugingetter/getter.go index b9ffa547a4..b90d4fa3f6 100644 --- a/components/engine/pkg/plugingetter/getter.go +++ b/components/engine/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/components/engine/pkg/plugins/plugins_unix.go b/components/engine/pkg/plugins/plugins_unix.go index 02f1da69a1..543f439fc7 100644 --- a/components/engine/pkg/plugins/plugins_unix.go +++ b/components/engine/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/components/engine/pkg/plugins/plugins_windows.go b/components/engine/pkg/plugins/plugins_windows.go index 3c8d8feb83..a46e3c109e 100644 --- a/components/engine/pkg/plugins/plugins_windows.go +++ b/components/engine/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/components/engine/plugin/manager_linux.go b/components/engine/plugin/manager_linux.go index 00eaf869cc..9670cf8ae4 100644 --- a/components/engine/plugin/manager_linux.go +++ b/components/engine/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/components/engine/plugin/v2/plugin.go b/components/engine/plugin/v2/plugin.go index a4c7c535f6..74adba680b 100644 --- a/components/engine/plugin/v2/plugin.go +++ b/components/engine/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/components/engine/volume/drivers/adapter.go b/components/engine/volume/drivers/adapter.go index 54d1ff2630..36aa521098 100644 --- a/components/engine/volume/drivers/adapter.go +++ b/components/engine/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/components/engine/volume/drivers/extpoint.go b/components/engine/volume/drivers/extpoint.go index c360b37a2b..56f78b9245 100644 --- a/components/engine/volume/drivers/extpoint.go +++ b/components/engine/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/components/engine/volume/testutils/testutils.go b/components/engine/volume/testutils/testutils.go index 84ab55ff77..5a17a26b77 100644 --- a/components/engine/volume/testutils/testutils.go +++ b/components/engine/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 { From eb861a7ae9784392cb98b6960566f9575537e127 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Wed, 13 Dec 2017 15:33:15 -0500 Subject: [PATCH 04/22] Revert "Make plugins dir private." This reverts commit 0c2821d6f2de692d105e50a399daa65169697cca. Due to other changes this is no longer needed and resolves some other issues with plugins. Signed-off-by: Brian Goff Upstream-commit: 37d7b7ddc332541f516d0c41d9ad30b28f2d3e22 Component: engine --- components/engine/plugin/manager.go | 5 ----- components/engine/plugin/manager_linux.go | 8 -------- components/engine/plugin/manager_windows.go | 2 -- 3 files changed, 15 deletions(-) diff --git a/components/engine/plugin/manager.go b/components/engine/plugin/manager.go index a163a2aa7e..2bdf82e56f 100644 --- a/components/engine/plugin/manager.go +++ b/components/engine/plugin/manager.go @@ -111,11 +111,6 @@ func NewManager(config ManagerConfig) (*Manager, error) { return nil, errors.Wrapf(err, "failed to mkdir %v", dirName) } } - - if err := setupRoot(manager.config.Root); err != nil { - return nil, err - } - var err error manager.executor, err = config.CreateExecutor(manager) if err != nil { diff --git a/components/engine/plugin/manager_linux.go b/components/engine/plugin/manager_linux.go index 9670cf8ae4..1def28c2a0 100644 --- a/components/engine/plugin/manager_linux.go +++ b/components/engine/plugin/manager_linux.go @@ -159,13 +159,6 @@ func shutdownPlugin(p *v2.Plugin, c *controller, executor Executor) { } } -func setupRoot(root string) error { - if err := mount.MakePrivate(root); err != nil { - return errors.Wrap(err, "error setting plugin manager root to private") - } - return nil -} - func (pm *Manager) disable(p *v2.Plugin, c *controller) error { if !p.IsEnabled() { return errors.Wrap(errDisabled(p.Name()), "plugin is already disabled") @@ -194,7 +187,6 @@ func (pm *Manager) Shutdown() { shutdownPlugin(p, c, pm.executor) } } - mount.Unmount(pm.config.Root) } func (pm *Manager) upgradePlugin(p *v2.Plugin, configDigest digest.Digest, blobsums []digest.Digest, tmpRootFSDir string, privileges *types.PluginPrivileges) (err error) { diff --git a/components/engine/plugin/manager_windows.go b/components/engine/plugin/manager_windows.go index ac03d6e639..72ccae72d3 100644 --- a/components/engine/plugin/manager_windows.go +++ b/components/engine/plugin/manager_windows.go @@ -26,5 +26,3 @@ func (pm *Manager) restore(p *v2.Plugin) error { // Shutdown plugins func (pm *Manager) Shutdown() { } - -func setupRoot(root string) error { return nil } From 32c5b539601a099e320a4dc6959f9eb025048cf9 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Mon, 18 Dec 2017 21:28:16 -0500 Subject: [PATCH 05/22] Make sure plugin mounts are cleaned up Signed-off-by: Brian Goff Upstream-commit: e6f784e3df2095366fd99fd42ab5a2d0b451bd07 Component: engine --- components/engine/plugin/manager.go | 5 +++++ components/engine/plugin/manager_linux.go | 3 +++ 2 files changed, 8 insertions(+) diff --git a/components/engine/plugin/manager.go b/components/engine/plugin/manager.go index 2bdf82e56f..750e3a5349 100644 --- a/components/engine/plugin/manager.go +++ b/components/engine/plugin/manager.go @@ -19,6 +19,7 @@ import ( "github.com/docker/docker/layer" "github.com/docker/docker/pkg/authorization" "github.com/docker/docker/pkg/ioutils" + "github.com/docker/docker/pkg/mount" "github.com/docker/docker/pkg/pubsub" "github.com/docker/docker/pkg/system" "github.com/docker/docker/plugin/v2" @@ -155,6 +156,10 @@ func (pm *Manager) HandleExitEvent(id string) error { if restart { pm.enable(p, c, true) + } else { + if err := mount.RecursiveUnmount(filepath.Join(pm.config.Root, id)); err != nil { + return errors.Wrap(err, "error cleaning up plugin mounts") + } } return nil } diff --git a/components/engine/plugin/manager_linux.go b/components/engine/plugin/manager_linux.go index 1def28c2a0..a98702802f 100644 --- a/components/engine/plugin/manager_linux.go +++ b/components/engine/plugin/manager_linux.go @@ -187,6 +187,9 @@ func (pm *Manager) Shutdown() { shutdownPlugin(p, c, pm.executor) } } + if err := mount.RecursiveUnmount(pm.config.Root); err != nil { + logrus.WithError(err).Warn("error cleaning up plugin mounts") + } } func (pm *Manager) upgradePlugin(p *v2.Plugin, configDigest digest.Digest, blobsums []digest.Digest, tmpRootFSDir string, privileges *types.PluginPrivileges) (err error) { From a6e6cffaed2a5b6b96210b607f805adbee2fa68a Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Thu, 15 Jun 2017 11:21:05 -0700 Subject: [PATCH 06/22] executor: Use a TemplatedDependencyGetter to support template expansion Signed-off-by: Aaron Lehmann Upstream-commit: 56da5fd7d31c9a627fc6a3c482cb0bf0ffb2d26e Component: engine --- .../engine/daemon/cluster/executor/container/executor.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/components/engine/daemon/cluster/executor/container/executor.go b/components/engine/daemon/cluster/executor/container/executor.go index 48ec7e0d25..2c4f619cf0 100644 --- a/components/engine/daemon/cluster/executor/container/executor.go +++ b/components/engine/daemon/cluster/executor/container/executor.go @@ -19,6 +19,7 @@ import ( "github.com/docker/swarmkit/agent/exec" "github.com/docker/swarmkit/api" "github.com/docker/swarmkit/api/naming" + "github.com/docker/swarmkit/template" "github.com/sirupsen/logrus" "golang.org/x/net/context" ) @@ -191,7 +192,7 @@ func (e *executor) Configure(ctx context.Context, node *api.Node) error { // Controller returns a docker container runner. func (e *executor) Controller(t *api.Task) (exec.Controller, error) { - dependencyGetter := agent.Restrict(e.dependencies, t) + dependencyGetter := template.NewTemplatedDependencyGetter(agent.Restrict(e.dependencies, t), t, nil) // Get the node description from the executor field e.mutex.Lock() From fc6a93f926fc35f01245f9a586df57c8c238c1d7 Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Thu, 15 Jun 2017 11:28:41 -0700 Subject: [PATCH 07/22] api: Add Templating parameter to SecretSpec and ConfigSpec Signed-off-by: Aaron Lehmann Upstream-commit: c5df7235f6a4811f26b37441db401f6b04858504 Component: engine --- components/engine/api/swagger.yaml | 24 +++++++++++++++++++ components/engine/api/types/swarm/config.go | 4 ++++ components/engine/api/types/swarm/secret.go | 4 ++++ .../engine/daemon/cluster/convert/config.go | 19 ++++++++++++++- .../engine/daemon/cluster/convert/secret.go | 19 ++++++++++++++- 5 files changed, 68 insertions(+), 2 deletions(-) diff --git a/components/engine/api/swagger.yaml b/components/engine/api/swagger.yaml index 8174ce387c..12fdd57537 100644 --- a/components/engine/api/swagger.yaml +++ b/components/engine/api/swagger.yaml @@ -3338,6 +3338,18 @@ definitions: Driver: description: "Name of the secrets driver used to fetch the secret's value from an external secret store" $ref: "#/definitions/Driver" + Templating: + description: "Templating driver, if applicable" + type: "object" + properties: + Name: + description: "Name of the templating driver (i.e. 'golang')" + type: "string" + Options: + description: "key/value map of driver specific options." + type: "object" + additionalProperties: + type: "string" Secret: type: "object" @@ -3374,6 +3386,18 @@ definitions: Base64-url-safe-encoded ([RFC 4648](https://tools.ietf.org/html/rfc4648#section-3.2)) config data. type: "string" + Templating: + description: "Templating driver, if applicable" + type: "object" + properties: + Name: + description: "Name of the templating driver (i.e. 'golang')" + type: "string" + Options: + description: "key/value map of driver specific options." + type: "object" + additionalProperties: + type: "string" Config: type: "object" diff --git a/components/engine/api/types/swarm/config.go b/components/engine/api/types/swarm/config.go index c1fdf3b3e4..a1555cf43e 100644 --- a/components/engine/api/types/swarm/config.go +++ b/components/engine/api/types/swarm/config.go @@ -13,6 +13,10 @@ type Config struct { type ConfigSpec struct { Annotations Data []byte `json:",omitempty"` + + // Templating controls whether and how to evaluate the config payload as + // a template. If it is not set, no templating is used. + Templating *Driver `json:",omitempty"` } // ConfigReferenceFileTarget is a file target in a config reference diff --git a/components/engine/api/types/swarm/secret.go b/components/engine/api/types/swarm/secret.go index cfba1141d8..d5213ec981 100644 --- a/components/engine/api/types/swarm/secret.go +++ b/components/engine/api/types/swarm/secret.go @@ -14,6 +14,10 @@ type SecretSpec struct { Annotations Data []byte `json:",omitempty"` Driver *Driver `json:",omitempty"` // name of the secrets driver used to fetch the secret's value from an external secret store + + // Templating controls whether and how to evaluate the secret payload as + // a template. If it is not set, no templating is used. + Templating *Driver `json:",omitempty"` } // SecretReferenceFileTarget is a file target in a secret reference diff --git a/components/engine/daemon/cluster/convert/config.go b/components/engine/daemon/cluster/convert/config.go index ba7920ec94..16b3475af8 100644 --- a/components/engine/daemon/cluster/convert/config.go +++ b/components/engine/daemon/cluster/convert/config.go @@ -2,6 +2,7 @@ package convert // import "github.com/docker/docker/daemon/cluster/convert" import ( swarmtypes "github.com/docker/docker/api/types/swarm" + types "github.com/docker/docker/api/types/swarm" swarmapi "github.com/docker/swarmkit/api" gogotypes "github.com/gogo/protobuf/types" ) @@ -21,18 +22,34 @@ func ConfigFromGRPC(s *swarmapi.Config) swarmtypes.Config { config.CreatedAt, _ = gogotypes.TimestampFromProto(s.Meta.CreatedAt) config.UpdatedAt, _ = gogotypes.TimestampFromProto(s.Meta.UpdatedAt) + if s.Spec.Templating != nil { + config.Spec.Templating = &types.Driver{ + Name: s.Spec.Templating.Name, + Options: s.Spec.Templating.Options, + } + } + return config } // ConfigSpecToGRPC converts Config to a grpc Config. func ConfigSpecToGRPC(s swarmtypes.ConfigSpec) swarmapi.ConfigSpec { - return swarmapi.ConfigSpec{ + spec := swarmapi.ConfigSpec{ Annotations: swarmapi.Annotations{ Name: s.Name, Labels: s.Labels, }, Data: s.Data, } + + if s.Templating != nil { + spec.Templating = &swarmapi.Driver{ + Name: s.Templating.Name, + Options: s.Templating.Options, + } + } + + return spec } // ConfigReferencesFromGRPC converts a slice of grpc ConfigReference to ConfigReference diff --git a/components/engine/daemon/cluster/convert/secret.go b/components/engine/daemon/cluster/convert/secret.go index 3ec2a353dd..d0e5ac45d2 100644 --- a/components/engine/daemon/cluster/convert/secret.go +++ b/components/engine/daemon/cluster/convert/secret.go @@ -2,6 +2,7 @@ package convert // import "github.com/docker/docker/daemon/cluster/convert" import ( swarmtypes "github.com/docker/docker/api/types/swarm" + types "github.com/docker/docker/api/types/swarm" swarmapi "github.com/docker/swarmkit/api" gogotypes "github.com/gogo/protobuf/types" ) @@ -22,12 +23,19 @@ func SecretFromGRPC(s *swarmapi.Secret) swarmtypes.Secret { secret.CreatedAt, _ = gogotypes.TimestampFromProto(s.Meta.CreatedAt) secret.UpdatedAt, _ = gogotypes.TimestampFromProto(s.Meta.UpdatedAt) + if s.Spec.Templating != nil { + secret.Spec.Templating = &types.Driver{ + Name: s.Spec.Templating.Name, + Options: s.Spec.Templating.Options, + } + } + return secret } // SecretSpecToGRPC converts Secret to a grpc Secret. func SecretSpecToGRPC(s swarmtypes.SecretSpec) swarmapi.SecretSpec { - return swarmapi.SecretSpec{ + spec := swarmapi.SecretSpec{ Annotations: swarmapi.Annotations{ Name: s.Name, Labels: s.Labels, @@ -35,6 +43,15 @@ func SecretSpecToGRPC(s swarmtypes.SecretSpec) swarmapi.SecretSpec { Data: s.Data, Driver: driverToGRPC(s.Driver), } + + if s.Templating != nil { + spec.Templating = &swarmapi.Driver{ + Name: s.Templating.Name, + Options: s.Templating.Options, + } + } + + return spec } // SecretReferencesFromGRPC converts a slice of grpc SecretReference to SecretReference From 82ebb2a6fde351faada2ddcbd5edc5d3f4ca2072 Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Thu, 15 Jun 2017 12:06:08 -0700 Subject: [PATCH 08/22] integration-cli: Add secret/config templating tests Signed-off-by: Aaron Lehmann Upstream-commit: cdd2e6efdbf402c629844cb20955f160327917b9 Component: engine --- .../engine/integration/config/config_test.go | 130 ++++++++++++++++++ .../integration/internal/swarm/service.go | 122 ++++++++++++++++ .../engine/integration/secret/secret_test.go | 130 ++++++++++++++++++ 3 files changed, 382 insertions(+) diff --git a/components/engine/integration/config/config_test.go b/components/engine/integration/config/config_test.go index fa2a205953..6a96986e4a 100644 --- a/components/engine/integration/config/config_test.go +++ b/components/engine/integration/config/config_test.go @@ -1,8 +1,10 @@ package config import ( + "bytes" "sort" "testing" + "time" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/filters" @@ -10,6 +12,7 @@ import ( "github.com/docker/docker/client" "github.com/docker/docker/integration/internal/swarm" "github.com/docker/docker/internal/testutil" + "github.com/docker/docker/pkg/stdcopy" "github.com/gotestyourself/gotestyourself/skip" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -188,3 +191,130 @@ func TestConfigsUpdate(t *testing.T) { err = client.ConfigUpdate(ctx, configID, insp.Version, insp.Spec) testutil.ErrorContains(t, err, "only updates to Labels are allowed") } + +func TestTemplatedConfig(t *testing.T) { + d := swarm.NewSwarm(t, testEnv) + defer d.Stop(t) + + ctx := context.Background() + client := swarm.GetClient(t, d) + + referencedSecretSpec := swarmtypes.SecretSpec{ + Annotations: swarmtypes.Annotations{ + Name: "referencedsecret", + }, + Data: []byte("this is a secret"), + } + referencedSecret, err := client.SecretCreate(ctx, referencedSecretSpec) + assert.NoError(t, err) + + referencedConfigSpec := swarmtypes.ConfigSpec{ + Annotations: swarmtypes.Annotations{ + Name: "referencedconfig", + }, + Data: []byte("this is a config"), + } + referencedConfig, err := client.ConfigCreate(ctx, referencedConfigSpec) + assert.NoError(t, err) + + configSpec := swarmtypes.ConfigSpec{ + Annotations: swarmtypes.Annotations{ + Name: "templated_config", + }, + Templating: &swarmtypes.Driver{ + Name: "golang", + }, + Data: []byte("SERVICE_NAME={{.Service.Name}}\n" + + "{{secret \"referencedsecrettarget\"}}\n" + + "{{config \"referencedconfigtarget\"}}\n"), + } + + templatedConfig, err := client.ConfigCreate(ctx, configSpec) + assert.NoError(t, err) + + serviceID := swarm.CreateService(t, d, + swarm.ServiceWithConfig( + &swarmtypes.ConfigReference{ + File: &swarmtypes.ConfigReferenceFileTarget{ + Name: "/templated_config", + UID: "0", + GID: "0", + Mode: 0600, + }, + ConfigID: templatedConfig.ID, + ConfigName: "templated_config", + }, + ), + swarm.ServiceWithConfig( + &swarmtypes.ConfigReference{ + File: &swarmtypes.ConfigReferenceFileTarget{ + Name: "referencedconfigtarget", + UID: "0", + GID: "0", + Mode: 0600, + }, + ConfigID: referencedConfig.ID, + ConfigName: "referencedconfig", + }, + ), + swarm.ServiceWithSecret( + &swarmtypes.SecretReference{ + File: &swarmtypes.SecretReferenceFileTarget{ + Name: "referencedsecrettarget", + UID: "0", + GID: "0", + Mode: 0600, + }, + SecretID: referencedSecret.ID, + SecretName: "referencedsecret", + }, + ), + swarm.ServiceWithName("svc"), + ) + + var tasks []swarmtypes.Task + waitAndAssert(t, 60*time.Second, func(t *testing.T) bool { + tasks = swarm.GetRunningTasks(t, d, serviceID) + return len(tasks) > 0 + }) + + task := tasks[0] + waitAndAssert(t, 60*time.Second, func(t *testing.T) bool { + if task.NodeID == "" || (task.Status.ContainerStatus == nil || task.Status.ContainerStatus.ContainerID == "") { + task, _, _ = client.TaskInspectWithRaw(context.Background(), task.ID) + } + return task.NodeID != "" && task.Status.ContainerStatus != nil && task.Status.ContainerStatus.ContainerID != "" + }) + + attach := swarm.ExecTask(t, d, task, types.ExecConfig{ + Cmd: []string{"/bin/cat", "/templated_config"}, + AttachStdout: true, + AttachStderr: true, + }) + + buf := bytes.NewBuffer(nil) + _, err = stdcopy.StdCopy(buf, buf, attach.Reader) + require.NoError(t, err) + + expect := "SERVICE_NAME=svc\n" + + "this is a secret\n" + + "this is a config\n" + + assert.Equal(t, expect, buf.String()) +} + +func waitAndAssert(t *testing.T, timeout time.Duration, f func(*testing.T) bool) { + t.Helper() + after := time.After(timeout) + for { + select { + case <-after: + t.Fatalf("timed out waiting for condition") + default: + } + if f(t) { + return + } + time.Sleep(100 * time.Millisecond) + } +} diff --git a/components/engine/integration/internal/swarm/service.go b/components/engine/integration/internal/swarm/service.go index bd001ba0b4..a46b02e146 100644 --- a/components/engine/integration/internal/swarm/service.go +++ b/components/engine/integration/internal/swarm/service.go @@ -1,10 +1,14 @@ package swarm import ( + "context" "fmt" "testing" + "github.com/docker/docker/api/types" + "github.com/docker/docker/api/types/filters" swarmtypes "github.com/docker/docker/api/types/swarm" + "github.com/docker/docker/client" "github.com/docker/docker/integration-cli/daemon" "github.com/docker/docker/internal/test/environment" "github.com/stretchr/testify/require" @@ -34,3 +38,121 @@ func NewSwarm(t *testing.T, testEnv *environment.Execution) *daemon.Swarm { require.NoError(t, d.Init(swarmtypes.InitRequest{})) return d } + +// ServiceSpecOpt is used with `CreateService` to pass in service spec modifiers +type ServiceSpecOpt func(*swarmtypes.ServiceSpec) + +// CreateService creates a service on the passed in swarm daemon. +func CreateService(t *testing.T, d *daemon.Swarm, opts ...ServiceSpecOpt) string { + spec := defaultServiceSpec() + for _, o := range opts { + o(&spec) + } + + client := GetClient(t, d) + + resp, err := client.ServiceCreate(context.Background(), spec, types.ServiceCreateOptions{}) + require.NoError(t, err, "error creating service") + return resp.ID +} + +func defaultServiceSpec() swarmtypes.ServiceSpec { + var spec swarmtypes.ServiceSpec + ServiceWithImage("busybox:latest")(&spec) + ServiceWithCommand([]string{"/bin/top"})(&spec) + ServiceWithReplicas(1)(&spec) + return spec +} + +// ServiceWithImage sets the image to use for the service +func ServiceWithImage(image string) func(*swarmtypes.ServiceSpec) { + return func(spec *swarmtypes.ServiceSpec) { + ensureContainerSpec(spec) + spec.TaskTemplate.ContainerSpec.Image = image + } +} + +// ServiceWithCommand sets the command to use for the service +func ServiceWithCommand(cmd []string) ServiceSpecOpt { + return func(spec *swarmtypes.ServiceSpec) { + ensureContainerSpec(spec) + spec.TaskTemplate.ContainerSpec.Command = cmd + } +} + +// ServiceWithConfig adds the config reference to the service +func ServiceWithConfig(configRef *swarmtypes.ConfigReference) ServiceSpecOpt { + return func(spec *swarmtypes.ServiceSpec) { + ensureContainerSpec(spec) + spec.TaskTemplate.ContainerSpec.Configs = append(spec.TaskTemplate.ContainerSpec.Configs, configRef) + } +} + +// ServiceWithSecret adds the secret reference to the service +func ServiceWithSecret(secretRef *swarmtypes.SecretReference) ServiceSpecOpt { + return func(spec *swarmtypes.ServiceSpec) { + ensureContainerSpec(spec) + spec.TaskTemplate.ContainerSpec.Secrets = append(spec.TaskTemplate.ContainerSpec.Secrets, secretRef) + } +} + +// ServiceWithReplicas sets the replicas for the service +func ServiceWithReplicas(n uint64) ServiceSpecOpt { + return func(spec *swarmtypes.ServiceSpec) { + spec.Mode = swarmtypes.ServiceMode{ + Replicated: &swarmtypes.ReplicatedService{ + Replicas: &n, + }, + } + } +} + +// ServiceWithName sets the name of the service +func ServiceWithName(name string) ServiceSpecOpt { + return func(spec *swarmtypes.ServiceSpec) { + spec.Annotations.Name = name + } +} + +// GetRunningTasks gets the list of running tasks for a service +func GetRunningTasks(t *testing.T, d *daemon.Swarm, serviceID string) []swarmtypes.Task { + client := GetClient(t, d) + + filterArgs := filters.NewArgs() + filterArgs.Add("desired-state", "running") + filterArgs.Add("service", serviceID) + + options := types.TaskListOptions{ + Filters: filterArgs, + } + tasks, err := client.TaskList(context.Background(), options) + require.NoError(t, err) + return tasks +} + +// ExecTask runs the passed in exec config on the given task +func ExecTask(t *testing.T, d *daemon.Swarm, task swarmtypes.Task, config types.ExecConfig) types.HijackedResponse { + client := GetClient(t, d) + + ctx := context.Background() + resp, err := client.ContainerExecCreate(ctx, task.Status.ContainerStatus.ContainerID, config) + require.NoError(t, err, "error creating exec") + + startCheck := types.ExecStartCheck{} + attach, err := client.ContainerExecAttach(ctx, resp.ID, startCheck) + require.NoError(t, err, "error attaching to exec") + return attach +} + +func ensureContainerSpec(spec *swarmtypes.ServiceSpec) { + if spec.TaskTemplate.ContainerSpec == nil { + spec.TaskTemplate.ContainerSpec = &swarmtypes.ContainerSpec{} + } +} + +// GetClient creates a new client for the passed in swarm daemon. +func GetClient(t *testing.T, d *daemon.Swarm) client.APIClient { + client, err := client.NewClientWithOpts(client.WithHost((d.Sock()))) + require.NoError(t, err) + return client +} diff --git a/components/engine/integration/secret/secret_test.go b/components/engine/integration/secret/secret_test.go index a6e9983e23..e26d8a698d 100644 --- a/components/engine/integration/secret/secret_test.go +++ b/components/engine/integration/secret/secret_test.go @@ -1,8 +1,10 @@ package secret import ( + "bytes" "sort" "testing" + "time" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/filters" @@ -10,6 +12,7 @@ import ( "github.com/docker/docker/client" "github.com/docker/docker/integration/internal/swarm" "github.com/docker/docker/internal/testutil" + "github.com/docker/docker/pkg/stdcopy" "github.com/gotestyourself/gotestyourself/skip" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -232,3 +235,130 @@ func TestSecretsUpdate(t *testing.T) { err = client.SecretUpdate(ctx, secretID, insp.Version, insp.Spec) testutil.ErrorContains(t, err, "only updates to Labels are allowed") } + +func TestTemplatedSecret(t *testing.T) { + d := swarm.NewSwarm(t, testEnv) + defer d.Stop(t) + + ctx := context.Background() + client := swarm.GetClient(t, d) + + referencedSecretSpec := swarmtypes.SecretSpec{ + Annotations: swarmtypes.Annotations{ + Name: "referencedsecret", + }, + Data: []byte("this is a secret"), + } + referencedSecret, err := client.SecretCreate(ctx, referencedSecretSpec) + assert.NoError(t, err) + + referencedConfigSpec := swarmtypes.ConfigSpec{ + Annotations: swarmtypes.Annotations{ + Name: "referencedconfig", + }, + Data: []byte("this is a config"), + } + referencedConfig, err := client.ConfigCreate(ctx, referencedConfigSpec) + assert.NoError(t, err) + + secretSpec := swarmtypes.SecretSpec{ + Annotations: swarmtypes.Annotations{ + Name: "templated_secret", + }, + Templating: &swarmtypes.Driver{ + Name: "golang", + }, + Data: []byte("SERVICE_NAME={{.Service.Name}}\n" + + "{{secret \"referencedsecrettarget\"}}\n" + + "{{config \"referencedconfigtarget\"}}\n"), + } + + templatedSecret, err := client.SecretCreate(ctx, secretSpec) + assert.NoError(t, err) + + serviceID := swarm.CreateService(t, d, + swarm.ServiceWithSecret( + &swarmtypes.SecretReference{ + File: &swarmtypes.SecretReferenceFileTarget{ + Name: "templated_secret", + UID: "0", + GID: "0", + Mode: 0600, + }, + SecretID: templatedSecret.ID, + SecretName: "templated_secret", + }, + ), + swarm.ServiceWithConfig( + &swarmtypes.ConfigReference{ + File: &swarmtypes.ConfigReferenceFileTarget{ + Name: "referencedconfigtarget", + UID: "0", + GID: "0", + Mode: 0600, + }, + ConfigID: referencedConfig.ID, + ConfigName: "referencedconfig", + }, + ), + swarm.ServiceWithSecret( + &swarmtypes.SecretReference{ + File: &swarmtypes.SecretReferenceFileTarget{ + Name: "referencedsecrettarget", + UID: "0", + GID: "0", + Mode: 0600, + }, + SecretID: referencedSecret.ID, + SecretName: "referencedsecret", + }, + ), + swarm.ServiceWithName("svc"), + ) + + var tasks []swarmtypes.Task + waitAndAssert(t, 60*time.Second, func(t *testing.T) bool { + tasks = swarm.GetRunningTasks(t, d, serviceID) + return len(tasks) > 0 + }) + + task := tasks[0] + waitAndAssert(t, 60*time.Second, func(t *testing.T) bool { + if task.NodeID == "" || (task.Status.ContainerStatus == nil || task.Status.ContainerStatus.ContainerID == "") { + task, _, _ = client.TaskInspectWithRaw(context.Background(), task.ID) + } + return task.NodeID != "" && task.Status.ContainerStatus != nil && task.Status.ContainerStatus.ContainerID != "" + }) + + attach := swarm.ExecTask(t, d, task, types.ExecConfig{ + Cmd: []string{"/bin/cat", "/run/secrets/templated_secret"}, + AttachStdout: true, + AttachStderr: true, + }) + + buf := bytes.NewBuffer(nil) + _, err = stdcopy.StdCopy(buf, buf, attach.Reader) + require.NoError(t, err) + + expect := "SERVICE_NAME=svc\n" + + "this is a secret\n" + + "this is a config\n" + + assert.Equal(t, expect, buf.String()) +} + +func waitAndAssert(t *testing.T, timeout time.Duration, f func(*testing.T) bool) { + t.Helper() + after := time.After(timeout) + for { + select { + case <-after: + t.Fatalf("timed out waiting for condition") + default: + } + if f(t) { + return + } + time.Sleep(100 * time.Millisecond) + } +} From 599f92e497b8772f603264b94e1559d38adcba39 Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Mon, 26 Jun 2017 18:46:30 -0700 Subject: [PATCH 09/22] Store configs that contain secrets on tmpfs Signed-off-by: Aaron Lehmann Upstream-commit: cd3d0486a6f62afac50f2cf74e2b9d8728848c97 Component: engine --- components/engine/container/container.go | 19 ++- components/engine/container/container_unix.go | 18 ++- components/engine/daemon/configs.go | 5 +- .../daemon/container_operations_unix.go | 112 ++++++++++++------ .../daemon/container_operations_windows.go | 2 +- components/engine/daemon/oci_linux.go | 19 ++- 6 files changed, 131 insertions(+), 44 deletions(-) diff --git a/components/engine/container/container.go b/components/engine/container/container.go index 938d66f889..32389e3804 100644 --- a/components/engine/container/container.go +++ b/components/engine/container/container.go @@ -68,6 +68,13 @@ type ExitStatus struct { ExitedAt time.Time } +// ConfigReference wraps swarmtypes.ConfigReference to add a Sensitive flag. +type ConfigReference struct { + *swarmtypes.ConfigReference + // Sensitive is set if this config should not be written to disk. + Sensitive bool +} + // Container holds the structure defining a container object. type Container struct { StreamConfig *stream.Config @@ -99,7 +106,7 @@ type Container struct { ExecCommands *exec.Store `json:"-"` DependencyStore agentexec.DependencyGetter `json:"-"` SecretReferences []*swarmtypes.SecretReference - ConfigReferences []*swarmtypes.ConfigReference + ConfigReferences []*ConfigReference // logDriver for closing LogDriver logger.Logger `json:"-"` LogCopier *logger.Copier `json:"-"` @@ -1064,6 +1071,16 @@ func (container *Container) ConfigFilePath(configRef swarmtypes.ConfigReference) return filepath.Join(configs, configRef.ConfigID), nil } +// SensitiveConfigFilePath returns the path to the location of a config mounted +// as a secret. +func (container *Container) SensitiveConfigFilePath(configRef swarmtypes.ConfigReference) (string, error) { + secretMountPath, err := container.SecretMountPath() + if err != nil { + return "", err + } + return filepath.Join(secretMountPath, configRef.ConfigID+"c"), nil +} + // CreateDaemonEnvironment creates a new environment variable slice for this container. func (container *Container) CreateDaemonEnvironment(tty bool, linkedEnv []string) []string { // Setup environment diff --git a/components/engine/container/container_unix.go b/components/engine/container/container_unix.go index 6f4d91b919..ec90921acc 100644 --- a/components/engine/container/container_unix.go +++ b/components/engine/container/container_unix.go @@ -233,6 +233,20 @@ func (container *Container) SecretMounts() ([]Mount, error) { Writable: false, }) } + for _, r := range container.ConfigReferences { + if !r.Sensitive || r.File == nil { + continue + } + fPath, err := container.SensitiveConfigFilePath(*r.ConfigReference) + if err != nil { + return nil, err + } + mounts = append(mounts, Mount{ + Source: fPath, + Destination: r.File.Name, + Writable: false, + }) + } return mounts, nil } @@ -257,10 +271,10 @@ func (container *Container) UnmountSecrets() error { func (container *Container) ConfigMounts() ([]Mount, error) { var mounts []Mount for _, configRef := range container.ConfigReferences { - if configRef.File == nil { + if configRef.Sensitive || configRef.File == nil { continue } - src, err := container.ConfigFilePath(*configRef) + src, err := container.ConfigFilePath(*configRef.ConfigReference) if err != nil { return nil, err } diff --git a/components/engine/daemon/configs.go b/components/engine/daemon/configs.go index f85c51db2e..c19b7ff3fd 100644 --- a/components/engine/daemon/configs.go +++ b/components/engine/daemon/configs.go @@ -2,6 +2,7 @@ package daemon // import "github.com/docker/docker/daemon" import ( swarmtypes "github.com/docker/docker/api/types/swarm" + "github.com/docker/docker/container" "github.com/sirupsen/logrus" ) @@ -17,7 +18,9 @@ func (daemon *Daemon) SetContainerConfigReferences(name string, refs []*swarmtyp return err } - c.ConfigReferences = refs + for _, ref := range refs { + c.ConfigReferences = append(c.ConfigReferences, &container.ConfigReference{ConfigReference: ref}) + } return nil } diff --git a/components/engine/daemon/container_operations_unix.go b/components/engine/daemon/container_operations_unix.go index abd47c807f..1523ac067c 100644 --- a/components/engine/daemon/container_operations_unix.go +++ b/components/engine/daemon/container_operations_unix.go @@ -19,6 +19,7 @@ import ( "github.com/docker/docker/pkg/stringid" "github.com/docker/docker/runconfig" "github.com/docker/libnetwork" + "github.com/docker/swarmkit/template" "github.com/opencontainers/selinux/go-selinux/label" "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -160,44 +161,23 @@ func (daemon *Daemon) setupIpcDirs(c *container.Container) error { return nil } -func (daemon *Daemon) setupSecretDir(c *container.Container) (setupErr error) { +func (daemon *Daemon) setupSecretDir(c *container.Container, hasSecretDir *bool) (setupErr error) { if len(c.SecretReferences) == 0 { return nil } - localMountPath, err := c.SecretMountPath() - if err != nil { - return errors.Wrap(err, "error getting secrets mount dir") - } - logrus.Debugf("secrets: setting up secret dir: %s", localMountPath) - - // retrieve possible remapped range start for root UID, GID - rootIDs := daemon.idMappings.RootPair() - // create tmpfs - if err := idtools.MkdirAllAndChown(localMountPath, 0700, rootIDs); err != nil { - return errors.Wrap(err, "error creating secret local mount path") - } - - defer func() { - if setupErr != nil { - // cleanup - _ = detachMounted(localMountPath) - - if err := os.RemoveAll(localMountPath); err != nil { - logrus.Errorf("error cleaning up secret mount: %s", err) - } - } - }() - - tmpfsOwnership := fmt.Sprintf("uid=%d,gid=%d", rootIDs.UID, rootIDs.GID) - if err := mount.Mount("tmpfs", localMountPath, "tmpfs", "nodev,nosuid,noexec,"+tmpfsOwnership); err != nil { - return errors.Wrap(err, "unable to setup secret mount") + if !*hasSecretDir { + daemon.createSecretDir(c) + *hasSecretDir = true } if c.DependencyStore == nil { return fmt.Errorf("secret store is not initialized") } + // retrieve possible remapped range start for root UID, GID + rootIDs := daemon.idMappings.RootPair() + for _, s := range c.SecretReferences { // TODO (ehazlett): use type switch when more are supported if s.File == nil { @@ -244,8 +224,42 @@ func (daemon *Daemon) setupSecretDir(c *container.Container) (setupErr error) { } } + return nil +} + +func (daemon *Daemon) createSecretDir(c *container.Container) error { + localMountPath, err := c.SecretMountPath() + if err != nil { + return err + } + logrus.Debugf("secrets: setting up secret dir: %s", localMountPath) + + // retrieve possible remapped range start for root UID, GID + rootIDs := daemon.idMappings.RootPair() + // create tmpfs + if err := idtools.MkdirAllAndChown(localMountPath, 0700, rootIDs); err != nil { + return errors.Wrap(err, "error creating secret local mount path") + } + + tmpfsOwnership := fmt.Sprintf("uid=%d,gid=%d", rootIDs.UID, rootIDs.GID) + if err := mount.Mount("tmpfs", localMountPath, "tmpfs", "nodev,nosuid,noexec,"+tmpfsOwnership); err != nil { + return errors.Wrap(err, "unable to setup secret mount") + } + + return nil +} + +func (daemon *Daemon) remountSecretDir(c *container.Container) error { + localMountPath, err := c.SecretMountPath() + if err != nil { + return err + } + label.Relabel(localMountPath, c.MountLabel, false) + rootIDs := daemon.idMappings.RootPair() + tmpfsOwnership := fmt.Sprintf("uid=%d,gid=%d", rootIDs.UID, rootIDs.GID) + // remount secrets ro if err := mount.Mount("tmpfs", localMountPath, "tmpfs", "remount,ro,"+tmpfsOwnership); err != nil { return errors.Wrap(err, "unable to remount secret dir as readonly") @@ -254,7 +268,20 @@ func (daemon *Daemon) setupSecretDir(c *container.Container) (setupErr error) { return nil } -func (daemon *Daemon) setupConfigDir(c *container.Container) (setupErr error) { +func (daemon *Daemon) cleanupSecretDir(c *container.Container) { + localMountPath, err := c.SecretMountPath() + if err != nil { + logrus.WithError(err).WithField("container", c.ID).Errorf("error getting secrets mounth path for cleanup") + } + + detachMounted(localMountPath) + + if err := os.RemoveAll(localMountPath); err != nil { + logrus.Errorf("error cleaning up secret mount: %s", err) + } +} + +func (daemon *Daemon) setupConfigDir(c *container.Container, hasSecretDir *bool) (setupErr error) { if len(c.ConfigReferences) == 0 { return nil } @@ -291,22 +318,35 @@ func (daemon *Daemon) setupConfigDir(c *container.Container) (setupErr error) { continue } - fPath, err := c.ConfigFilePath(*configRef) + getter := c.DependencyStore.Configs().(template.TemplatedConfigGetter) + config, sensitive, err := getter.GetAndFlagSecretData(configRef.ConfigID) if err != nil { - return err + return errors.Wrap(err, "unable to get config from config store") + } + + var fPath string + if sensitive { + configRef.Sensitive = true + fPath, err = c.SensitiveConfigFilePath(*configRef.ConfigReference) + if !*hasSecretDir { + daemon.createSecretDir(c) + *hasSecretDir = true + } + } else { + fPath, err = c.ConfigFilePath(*configRef.ConfigReference) + } + if err != nil { + return errors.Wrap(err, "error getting config file path") } log := logrus.WithFields(logrus.Fields{"name": configRef.File.Name, "path": fPath}) + log.Debug("injecting config") + if err := idtools.MkdirAllAndChown(filepath.Dir(fPath), 0700, rootIDs); err != nil { return errors.Wrap(err, "error creating config path") } - log.Debug("injecting config") - config, err := c.DependencyStore.Configs().Get(configRef.ConfigID) - if err != nil { - return errors.Wrap(err, "unable to get config from config store") - } if err := ioutil.WriteFile(fPath, config.Spec.Data, configRef.File.Mode); err != nil { return errors.Wrap(err, "error injecting config") } diff --git a/components/engine/daemon/container_operations_windows.go b/components/engine/daemon/container_operations_windows.go index e3914f9410..fe9795b8ba 100644 --- a/components/engine/daemon/container_operations_windows.go +++ b/components/engine/daemon/container_operations_windows.go @@ -51,7 +51,7 @@ func (daemon *Daemon) setupConfigDir(c *container.Container) (setupErr error) { continue } - fPath, err := c.ConfigFilePath(*configRef) + fPath, err := c.ConfigFilePath(*configRef.ConfigReference) if err != nil { return err } diff --git a/components/engine/daemon/oci_linux.go b/components/engine/daemon/oci_linux.go index 2a19dd5db8..e0d93d5ca4 100644 --- a/components/engine/daemon/oci_linux.go +++ b/components/engine/daemon/oci_linux.go @@ -760,7 +760,7 @@ func (daemon *Daemon) populateCommonSpec(s *specs.Spec, c *container.Container) return nil } -func (daemon *Daemon) createSpec(c *container.Container) (*specs.Spec, error) { +func (daemon *Daemon) createSpec(c *container.Container) (retSpec *specs.Spec, err error) { s := oci.DefaultSpec() if err := daemon.populateCommonSpec(&s, c); err != nil { return nil, err @@ -842,14 +842,27 @@ func (daemon *Daemon) createSpec(c *container.Container) (*specs.Spec, error) { return nil, err } - if err := daemon.setupSecretDir(c); err != nil { + var hasSecretDir bool + defer func() { + if hasSecretDir && err != nil { + daemon.cleanupSecretDir(c) + } + }() + + if err := daemon.setupSecretDir(c, &hasSecretDir); err != nil { return nil, err } - if err := daemon.setupConfigDir(c); err != nil { + if err := daemon.setupConfigDir(c, &hasSecretDir); err != nil { return nil, err } + if hasSecretDir { + if err := daemon.remountSecretDir(c); err != nil { + return nil, err + } + } + ms, err := daemon.setupMounts(c) if err != nil { return nil, err From 40e1524cb370b92118c5721614653ffbb31af01a Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Wed, 26 Jul 2017 10:37:32 -0700 Subject: [PATCH 10/22] daemon: Check return value of createSecretDir Signed-off-by: Aaron Lehmann Upstream-commit: 426f4e48e3e53b2445835585d7957043a5fe6ab3 Component: engine --- components/engine/daemon/container_operations_unix.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/components/engine/daemon/container_operations_unix.go b/components/engine/daemon/container_operations_unix.go index 1523ac067c..d875be73b3 100644 --- a/components/engine/daemon/container_operations_unix.go +++ b/components/engine/daemon/container_operations_unix.go @@ -167,7 +167,9 @@ func (daemon *Daemon) setupSecretDir(c *container.Container, hasSecretDir *bool) } if !*hasSecretDir { - daemon.createSecretDir(c) + if err := daemon.createSecretDir(c); err != nil { + return err + } *hasSecretDir = true } @@ -329,7 +331,9 @@ func (daemon *Daemon) setupConfigDir(c *container.Container, hasSecretDir *bool) configRef.Sensitive = true fPath, err = c.SensitiveConfigFilePath(*configRef.ConfigReference) if !*hasSecretDir { - daemon.createSecretDir(c) + if err := daemon.createSecretDir(c); err != nil { + return err + } *hasSecretDir = true } } else { From 850e2bff8c7c49a0990ca3c8c540d8df81d9c426 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Thu, 11 Jan 2018 17:28:56 -0500 Subject: [PATCH 11/22] Always mount configs with tmpfs This makes configs and secrets behavior identical. Signed-off-by: Brian Goff Upstream-commit: 8e8f5f4457d8e1b02031576dbc18c903be4bcfb6 Component: engine --- components/engine/api/swagger.yaml | 34 ++-- .../daemon/container_operations_unix.go | 151 ++++++++---------- components/engine/daemon/oci_linux.go | 24 +-- 3 files changed, 88 insertions(+), 121 deletions(-) diff --git a/components/engine/api/swagger.yaml b/components/engine/api/swagger.yaml index 12fdd57537..cb4201be90 100644 --- a/components/engine/api/swagger.yaml +++ b/components/engine/api/swagger.yaml @@ -3339,17 +3339,12 @@ definitions: description: "Name of the secrets driver used to fetch the secret's value from an external secret store" $ref: "#/definitions/Driver" Templating: - description: "Templating driver, if applicable" - type: "object" - properties: - Name: - description: "Name of the templating driver (i.e. 'golang')" - type: "string" - Options: - description: "key/value map of driver specific options." - type: "object" - additionalProperties: - type: "string" + description: | + Templating driver, if applicable + + Templating controls whether and how to evaluate the config payload as + a template. If no driver is set, no templating is used. + $ref: "#/definitions/Driver" Secret: type: "object" @@ -3387,17 +3382,12 @@ definitions: config data. type: "string" Templating: - description: "Templating driver, if applicable" - type: "object" - properties: - Name: - description: "Name of the templating driver (i.e. 'golang')" - type: "string" - Options: - description: "key/value map of driver specific options." - type: "object" - additionalProperties: - type: "string" + description: | + Templating driver, if applicable + + Templating controls whether and how to evaluate the config payload as + a template. If no driver is set, no templating is used. + $ref: "#/definitions/Driver" Config: type: "object" diff --git a/components/engine/daemon/container_operations_unix.go b/components/engine/daemon/container_operations_unix.go index d875be73b3..8c23b36e3c 100644 --- a/components/engine/daemon/container_operations_unix.go +++ b/components/engine/daemon/container_operations_unix.go @@ -19,7 +19,6 @@ import ( "github.com/docker/docker/pkg/stringid" "github.com/docker/docker/runconfig" "github.com/docker/libnetwork" - "github.com/docker/swarmkit/template" "github.com/opencontainers/selinux/go-selinux/label" "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -161,17 +160,23 @@ func (daemon *Daemon) setupIpcDirs(c *container.Container) error { return nil } -func (daemon *Daemon) setupSecretDir(c *container.Container, hasSecretDir *bool) (setupErr error) { +func (daemon *Daemon) setupSecretDir(c *container.Container) (setupErr error) { if len(c.SecretReferences) == 0 { return nil } - if !*hasSecretDir { - if err := daemon.createSecretDir(c); err != nil { - return err - } - *hasSecretDir = true + localMountPath, err := c.SecretMountPath() + if err != nil { + return errors.Wrap(err, "error getting secrets mount path for container") } + if err := daemon.createSecretsDir(localMountPath); err != nil { + return err + } + defer func() { + if setupErr != nil { + daemon.cleanupSecretDir(localMountPath) + } + }() if c.DependencyStore == nil { return fmt.Errorf("secret store is not initialized") @@ -226,64 +231,52 @@ func (daemon *Daemon) setupSecretDir(c *container.Container, hasSecretDir *bool) } } - return nil + return daemon.remountSecretDir(c.MountLabel, localMountPath) } -func (daemon *Daemon) createSecretDir(c *container.Container) error { - localMountPath, err := c.SecretMountPath() - if err != nil { - return err - } - logrus.Debugf("secrets: setting up secret dir: %s", localMountPath) - +// createSecretsDir is used to create a dir suitable for storing container secrets. +// In practice this is using a tmpfs mount and is used for both "configs" and "secrets" +func (daemon *Daemon) createSecretsDir(dir string) error { // retrieve possible remapped range start for root UID, GID rootIDs := daemon.idMappings.RootPair() // create tmpfs - if err := idtools.MkdirAllAndChown(localMountPath, 0700, rootIDs); err != nil { + if err := idtools.MkdirAllAndChown(dir, 0700, rootIDs); err != nil { return errors.Wrap(err, "error creating secret local mount path") } tmpfsOwnership := fmt.Sprintf("uid=%d,gid=%d", rootIDs.UID, rootIDs.GID) - if err := mount.Mount("tmpfs", localMountPath, "tmpfs", "nodev,nosuid,noexec,"+tmpfsOwnership); err != nil { + if err := mount.Mount("tmpfs", dir, "tmpfs", "nodev,nosuid,noexec,"+tmpfsOwnership); err != nil { return errors.Wrap(err, "unable to setup secret mount") } return nil } -func (daemon *Daemon) remountSecretDir(c *container.Container) error { - localMountPath, err := c.SecretMountPath() - if err != nil { - return err +func (daemon *Daemon) remountSecretDir(mountLabel, dir string) error { + if err := label.Relabel(dir, mountLabel, false); err != nil { + logrus.WithError(err).WithField("dir", dir).Warn("Error while attempting to set selinux label") } - - label.Relabel(localMountPath, c.MountLabel, false) - rootIDs := daemon.idMappings.RootPair() tmpfsOwnership := fmt.Sprintf("uid=%d,gid=%d", rootIDs.UID, rootIDs.GID) // remount secrets ro - if err := mount.Mount("tmpfs", localMountPath, "tmpfs", "remount,ro,"+tmpfsOwnership); err != nil { - return errors.Wrap(err, "unable to remount secret dir as readonly") + if err := mount.Mount("tmpfs", dir, "tmpfs", "remount,ro,"+tmpfsOwnership); err != nil { + return errors.Wrap(err, "unable to remount dir as readonly") } return nil } -func (daemon *Daemon) cleanupSecretDir(c *container.Container) { - localMountPath, err := c.SecretMountPath() - if err != nil { - logrus.WithError(err).WithField("container", c.ID).Errorf("error getting secrets mounth path for cleanup") +func (daemon *Daemon) cleanupSecretDir(dir string) { + if err := mount.RecursiveUnmount(dir); err != nil { + logrus.WithField("dir", dir).WithError(err).Warn("Error while attmepting to unmount dir, this may prevent removal of container.") } - - detachMounted(localMountPath) - - if err := os.RemoveAll(localMountPath); err != nil { - logrus.Errorf("error cleaning up secret mount: %s", err) + if err := os.RemoveAll(dir); err != nil && !os.IsNotExist(err) { + logrus.WithField("dir", dir).WithError(err).Error("Error removing dir.") } } -func (daemon *Daemon) setupConfigDir(c *container.Container, hasSecretDir *bool) (setupErr error) { +func (daemon *Daemon) setupConfigDir(c *container.Container) (setupErr error) { if len(c.ConfigReferences) == 0 { return nil } @@ -293,73 +286,55 @@ func (daemon *Daemon) setupConfigDir(c *container.Container, hasSecretDir *bool) return err } logrus.Debugf("configs: setting up config dir: %s", localPath) - - // retrieve possible remapped range start for root UID, GID - rootIDs := daemon.idMappings.RootPair() - // create tmpfs - if err := idtools.MkdirAllAndChown(localPath, 0700, rootIDs); err != nil { - return errors.Wrap(err, "error creating config dir") + if err := daemon.createSecretsDir(localPath); err != nil { + return err } - defer func() { if setupErr != nil { - if err := os.RemoveAll(localPath); err != nil { - logrus.Errorf("error cleaning up config dir: %s", err) - } + daemon.cleanupSecretDir(localPath) } }() if c.DependencyStore == nil { - return fmt.Errorf("config store is not initialized") + return errors.New("config store is not initialized") } - for _, configRef := range c.ConfigReferences { + // retrieve possible remapped range start for root UID, GID + rootIDs := daemon.idMappings.RootPair() + + for _, ref := range c.ConfigReferences { // TODO (ehazlett): use type switch when more are supported - if configRef.File == nil { + if ref.File == nil { logrus.Error("config target type is not a file target") continue } - - getter := c.DependencyStore.Configs().(template.TemplatedConfigGetter) - config, sensitive, err := getter.GetAndFlagSecretData(configRef.ConfigID) - if err != nil { - return errors.Wrap(err, "unable to get config from config store") - } - - var fPath string - if sensitive { - configRef.Sensitive = true - fPath, err = c.SensitiveConfigFilePath(*configRef.ConfigReference) - if !*hasSecretDir { - if err := daemon.createSecretDir(c); err != nil { - return err - } - *hasSecretDir = true - } - } else { - fPath, err = c.ConfigFilePath(*configRef.ConfigReference) - } - if err != nil { - return errors.Wrap(err, "error getting config file path") - } - - log := logrus.WithFields(logrus.Fields{"name": configRef.File.Name, "path": fPath}) - - log.Debug("injecting config") - - if err := idtools.MkdirAllAndChown(filepath.Dir(fPath), 0700, rootIDs); err != nil { - return errors.Wrap(err, "error creating config path") - } - - if err := ioutil.WriteFile(fPath, config.Spec.Data, configRef.File.Mode); err != nil { - return errors.Wrap(err, "error injecting config") - } - - uid, err := strconv.Atoi(configRef.File.UID) + // configs are created in the ConfigsDirPath on the host, at a + // single level + fPath, err := c.ConfigFilePath(*ref.ConfigReference) if err != nil { return err } - gid, err := strconv.Atoi(configRef.File.GID) + if err := idtools.MkdirAllAndChown(filepath.Dir(fPath), 0700, rootIDs); err != nil { + return errors.Wrap(err, "error creating config mount path") + } + + logrus.WithFields(logrus.Fields{ + "name": ref.File.Name, + "path": fPath, + }).Debug("injecting config") + config, err := c.DependencyStore.Configs().Get(ref.ConfigID) + if err != nil { + return errors.Wrap(err, "unable to get config from config store") + } + if err := ioutil.WriteFile(fPath, config.Spec.Data, ref.File.Mode); err != nil { + return errors.Wrap(err, "error injecting config") + } + + uid, err := strconv.Atoi(ref.File.UID) + if err != nil { + return err + } + gid, err := strconv.Atoi(ref.File.GID) if err != nil { return err } @@ -374,7 +349,7 @@ func (daemon *Daemon) setupConfigDir(c *container.Container, hasSecretDir *bool) label.Relabel(fPath, c.MountLabel, false) } - return nil + return daemon.remountSecretDir(c.MountLabel, localPath) } func killProcessDirectly(cntr *container.Container) error { diff --git a/components/engine/daemon/oci_linux.go b/components/engine/daemon/oci_linux.go index e0d93d5ca4..52a944f2a2 100644 --- a/components/engine/daemon/oci_linux.go +++ b/components/engine/daemon/oci_linux.go @@ -842,27 +842,29 @@ func (daemon *Daemon) createSpec(c *container.Container) (retSpec *specs.Spec, e return nil, err } - var hasSecretDir bool + secretMountPath, err := c.SecretMountPath() + if err != nil { + return nil, err + } + configsMountPath, err := c.ConfigsDirPath() + if err != nil { + return nil, err + } defer func() { - if hasSecretDir && err != nil { - daemon.cleanupSecretDir(c) + if err != nil { + daemon.cleanupSecretDir(secretMountPath) + daemon.cleanupSecretDir(configsMountPath) } }() - if err := daemon.setupSecretDir(c, &hasSecretDir); err != nil { + if err := daemon.setupSecretDir(c); err != nil { return nil, err } - if err := daemon.setupConfigDir(c, &hasSecretDir); err != nil { + if err := daemon.setupConfigDir(c); err != nil { return nil, err } - if hasSecretDir { - if err := daemon.remountSecretDir(c); err != nil { - return nil, err - } - } - ms, err := daemon.setupMounts(c) if err != nil { return nil, err From f68c84b9a0bbdc45deb0f5b51ac711d391fc9c87 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Wed, 17 Jan 2018 10:49:58 -0500 Subject: [PATCH 12/22] Merge configs/secrets in unix implementation On unix, merge secrets/configs handling. This is important because configs can contain secrets (via templating) and potentially a config could just simply have secret information "by accident" from the user. This just make sure that configs are as secure as secrets and de-dups a lot of code. Generally this makes everything simpler and configs more secure. Signed-off-by: Brian Goff Upstream-commit: c02171802b788fb2d4d48bebcee2a57c8eabeeaa Component: engine --- components/engine/container/container.go | 34 +--- components/engine/container/container_unix.go | 38 ++--- .../engine/container/container_windows.go | 25 +-- components/engine/daemon/configs.go | 7 +- .../daemon/container_operations_unix.go | 149 ++++++++---------- .../daemon/container_operations_windows.go | 11 +- components/engine/daemon/oci_linux.go | 21 +-- components/engine/daemon/oci_windows.go | 5 +- .../engine/integration/config/config_test.go | 19 ++- .../engine/integration/secret/secret_test.go | 19 ++- 10 files changed, 126 insertions(+), 202 deletions(-) diff --git a/components/engine/container/container.go b/components/engine/container/container.go index 32389e3804..46e592ab45 100644 --- a/components/engine/container/container.go +++ b/components/engine/container/container.go @@ -68,13 +68,6 @@ type ExitStatus struct { ExitedAt time.Time } -// ConfigReference wraps swarmtypes.ConfigReference to add a Sensitive flag. -type ConfigReference struct { - *swarmtypes.ConfigReference - // Sensitive is set if this config should not be written to disk. - Sensitive bool -} - // Container holds the structure defining a container object. type Container struct { StreamConfig *stream.Config @@ -106,7 +99,7 @@ type Container struct { ExecCommands *exec.Store `json:"-"` DependencyStore agentexec.DependencyGetter `json:"-"` SecretReferences []*swarmtypes.SecretReference - ConfigReferences []*ConfigReference + ConfigReferences []*swarmtypes.ConfigReference // logDriver for closing LogDriver logger.Logger `json:"-"` LogCopier *logger.Copier `json:"-"` @@ -1056,31 +1049,6 @@ func getSecretTargetPath(r *swarmtypes.SecretReference) string { return filepath.Join(containerSecretMountPath, r.File.Name) } -// ConfigsDirPath returns the path to the directory where configs are stored on -// disk. -func (container *Container) ConfigsDirPath() (string, error) { - return container.GetRootResourcePath("configs") -} - -// ConfigFilePath returns the path to the on-disk location of a config. -func (container *Container) ConfigFilePath(configRef swarmtypes.ConfigReference) (string, error) { - configs, err := container.ConfigsDirPath() - if err != nil { - return "", err - } - return filepath.Join(configs, configRef.ConfigID), nil -} - -// SensitiveConfigFilePath returns the path to the location of a config mounted -// as a secret. -func (container *Container) SensitiveConfigFilePath(configRef swarmtypes.ConfigReference) (string, error) { - secretMountPath, err := container.SecretMountPath() - if err != nil { - return "", err - } - return filepath.Join(secretMountPath, configRef.ConfigID+"c"), nil -} - // CreateDaemonEnvironment creates a new environment variable slice for this container. func (container *Container) CreateDaemonEnvironment(tty bool, linkedEnv []string) []string { // Setup environment diff --git a/components/engine/container/container_unix.go b/components/engine/container/container_unix.go index ec90921acc..e5cecf3166 100644 --- a/components/engine/container/container_unix.go +++ b/components/engine/container/container_unix.go @@ -5,11 +5,13 @@ package container // import "github.com/docker/docker/container" import ( "io/ioutil" "os" + "path/filepath" "github.com/containerd/continuity/fs" "github.com/docker/docker/api/types" containertypes "github.com/docker/docker/api/types/container" mounttypes "github.com/docker/docker/api/types/mount" + swarmtypes "github.com/docker/docker/api/types/swarm" "github.com/docker/docker/pkg/mount" "github.com/docker/docker/pkg/stringid" "github.com/docker/docker/volume" @@ -234,10 +236,7 @@ func (container *Container) SecretMounts() ([]Mount, error) { }) } for _, r := range container.ConfigReferences { - if !r.Sensitive || r.File == nil { - continue - } - fPath, err := container.SensitiveConfigFilePath(*r.ConfigReference) + fPath, err := container.ConfigFilePath(*r) if err != nil { return nil, err } @@ -267,27 +266,6 @@ func (container *Container) UnmountSecrets() error { return mount.RecursiveUnmount(p) } -// ConfigMounts returns the mounts for configs. -func (container *Container) ConfigMounts() ([]Mount, error) { - var mounts []Mount - for _, configRef := range container.ConfigReferences { - if configRef.Sensitive || configRef.File == nil { - continue - } - src, err := container.ConfigFilePath(*configRef.ConfigReference) - if err != nil { - return nil, err - } - mounts = append(mounts, Mount{ - Source: src, - Destination: configRef.File.Name, - Writable: false, - }) - } - - return mounts, nil -} - type conflictingUpdateOptions string func (e conflictingUpdateOptions) Error() string { @@ -471,3 +449,13 @@ func (container *Container) GetMountPoints() []types.MountPoint { } return mountPoints } + +// ConfigFilePath returns the path to the on-disk location of a config. +// On unix, configs are always considered secret +func (container *Container) ConfigFilePath(configRef swarmtypes.ConfigReference) (string, error) { + mounts, err := container.SecretMountPath() + if err != nil { + return "", err + } + return filepath.Join(mounts, configRef.ConfigID), nil +} diff --git a/components/engine/container/container_windows.go b/components/engine/container/container_windows.go index 44b646a1ad..b5bdb5bc34 100644 --- a/components/engine/container/container_windows.go +++ b/components/engine/container/container_windows.go @@ -7,6 +7,7 @@ import ( "github.com/docker/docker/api/types" containertypes "github.com/docker/docker/api/types/container" + swarmtypes "github.com/docker/docker/api/types/swarm" "github.com/docker/docker/pkg/system" ) @@ -102,23 +103,20 @@ func (container *Container) CreateConfigSymlinks() error { } // ConfigMounts returns the mount for configs. -// All configs are stored in a single mount on Windows. Target symlinks are -// created for each config, pointing to the files in this mount. -func (container *Container) ConfigMounts() ([]Mount, error) { +// TODO: Right now Windows doesn't really have a "secure" storage for secrets, +// however some configs may contain secrets. Once secure storage is worked out, +// configs and secret handling should be merged. +func (container *Container) ConfigMounts() []Mount { var mounts []Mount if len(container.ConfigReferences) > 0 { - src, err := container.ConfigsDirPath() - if err != nil { - return nil, err - } mounts = append(mounts, Mount{ - Source: src, + Source: container.ConfigsDirPath(), Destination: containerInternalConfigsDirPath, Writable: false, }) } - return mounts, nil + return mounts } // DetachAndUnmount unmounts all volumes. @@ -204,3 +202,12 @@ func (container *Container) GetMountPoints() []types.MountPoint { } return mountPoints } + +func (container *Container) ConfigsDirPath() string { + return filepath.Join(container.Root, "configs") +} + +// ConfigFilePath returns the path to the on-disk location of a config. +func (container *Container) ConfigFilePath(configRef swarmtypes.ConfigReference) string { + return filepath.Join(container.ConfigsDirPath(), configRef.ConfigID) +} diff --git a/components/engine/daemon/configs.go b/components/engine/daemon/configs.go index c19b7ff3fd..4fd0d2272c 100644 --- a/components/engine/daemon/configs.go +++ b/components/engine/daemon/configs.go @@ -2,7 +2,6 @@ package daemon // import "github.com/docker/docker/daemon" import ( swarmtypes "github.com/docker/docker/api/types/swarm" - "github.com/docker/docker/container" "github.com/sirupsen/logrus" ) @@ -17,10 +16,6 @@ func (daemon *Daemon) SetContainerConfigReferences(name string, refs []*swarmtyp if err != nil { return err } - - for _, ref := range refs { - c.ConfigReferences = append(c.ConfigReferences, &container.ConfigReference{ConfigReference: ref}) - } - + c.ConfigReferences = append(c.ConfigReferences, refs...) return nil } diff --git a/components/engine/daemon/container_operations_unix.go b/components/engine/daemon/container_operations_unix.go index 8c23b36e3c..4e92b6392e 100644 --- a/components/engine/daemon/container_operations_unix.go +++ b/components/engine/daemon/container_operations_unix.go @@ -161,20 +161,16 @@ func (daemon *Daemon) setupIpcDirs(c *container.Container) error { } func (daemon *Daemon) setupSecretDir(c *container.Container) (setupErr error) { - if len(c.SecretReferences) == 0 { + if len(c.SecretReferences) == 0 && len(c.ConfigReferences) == 0 { return nil } - localMountPath, err := c.SecretMountPath() - if err != nil { - return errors.Wrap(err, "error getting secrets mount path for container") - } - if err := daemon.createSecretsDir(localMountPath); err != nil { + if err := daemon.createSecretsDir(c); err != nil { return err } defer func() { if setupErr != nil { - daemon.cleanupSecretDir(localMountPath) + daemon.cleanupSecretDir(c) } }() @@ -231,88 +227,16 @@ func (daemon *Daemon) setupSecretDir(c *container.Container) (setupErr error) { } } - return daemon.remountSecretDir(c.MountLabel, localMountPath) -} - -// createSecretsDir is used to create a dir suitable for storing container secrets. -// In practice this is using a tmpfs mount and is used for both "configs" and "secrets" -func (daemon *Daemon) createSecretsDir(dir string) error { - // retrieve possible remapped range start for root UID, GID - rootIDs := daemon.idMappings.RootPair() - // create tmpfs - if err := idtools.MkdirAllAndChown(dir, 0700, rootIDs); err != nil { - return errors.Wrap(err, "error creating secret local mount path") - } - - tmpfsOwnership := fmt.Sprintf("uid=%d,gid=%d", rootIDs.UID, rootIDs.GID) - if err := mount.Mount("tmpfs", dir, "tmpfs", "nodev,nosuid,noexec,"+tmpfsOwnership); err != nil { - return errors.Wrap(err, "unable to setup secret mount") - } - - return nil -} - -func (daemon *Daemon) remountSecretDir(mountLabel, dir string) error { - if err := label.Relabel(dir, mountLabel, false); err != nil { - logrus.WithError(err).WithField("dir", dir).Warn("Error while attempting to set selinux label") - } - rootIDs := daemon.idMappings.RootPair() - tmpfsOwnership := fmt.Sprintf("uid=%d,gid=%d", rootIDs.UID, rootIDs.GID) - - // remount secrets ro - if err := mount.Mount("tmpfs", dir, "tmpfs", "remount,ro,"+tmpfsOwnership); err != nil { - return errors.Wrap(err, "unable to remount dir as readonly") - } - - return nil -} - -func (daemon *Daemon) cleanupSecretDir(dir string) { - if err := mount.RecursiveUnmount(dir); err != nil { - logrus.WithField("dir", dir).WithError(err).Warn("Error while attmepting to unmount dir, this may prevent removal of container.") - } - if err := os.RemoveAll(dir); err != nil && !os.IsNotExist(err) { - logrus.WithField("dir", dir).WithError(err).Error("Error removing dir.") - } -} - -func (daemon *Daemon) setupConfigDir(c *container.Container) (setupErr error) { - if len(c.ConfigReferences) == 0 { - return nil - } - - localPath, err := c.ConfigsDirPath() - if err != nil { - return err - } - logrus.Debugf("configs: setting up config dir: %s", localPath) - if err := daemon.createSecretsDir(localPath); err != nil { - return err - } - defer func() { - if setupErr != nil { - daemon.cleanupSecretDir(localPath) - } - }() - - if c.DependencyStore == nil { - return errors.New("config store is not initialized") - } - - // retrieve possible remapped range start for root UID, GID - rootIDs := daemon.idMappings.RootPair() - for _, ref := range c.ConfigReferences { // TODO (ehazlett): use type switch when more are supported if ref.File == nil { logrus.Error("config target type is not a file target") continue } - // configs are created in the ConfigsDirPath on the host, at a - // single level - fPath, err := c.ConfigFilePath(*ref.ConfigReference) + + fPath, err := c.ConfigFilePath(*ref) if err != nil { - return err + return errors.Wrap(err, "error getting config file path for container") } if err := idtools.MkdirAllAndChown(filepath.Dir(fPath), 0700, rootIDs); err != nil { return errors.Wrap(err, "error creating config mount path") @@ -342,14 +266,67 @@ func (daemon *Daemon) setupConfigDir(c *container.Container) (setupErr error) { if err := os.Chown(fPath, rootIDs.UID+uid, rootIDs.GID+gid); err != nil { return errors.Wrap(err, "error setting ownership for config") } - if err := os.Chmod(fPath, configRef.File.Mode); err != nil { + if err := os.Chmod(fPath, ref.File.Mode); err != nil { return errors.Wrap(err, "error setting file mode for config") } - - label.Relabel(fPath, c.MountLabel, false) } - return daemon.remountSecretDir(c.MountLabel, localPath) + return daemon.remountSecretDir(c) +} + +// createSecretsDir is used to create a dir suitable for storing container secrets. +// In practice this is using a tmpfs mount and is used for both "configs" and "secrets" +func (daemon *Daemon) createSecretsDir(c *container.Container) error { + // retrieve possible remapped range start for root UID, GID + rootIDs := daemon.idMappings.RootPair() + dir, err := c.SecretMountPath() + if err != nil { + return errors.Wrap(err, "error getting container secrets dir") + } + + // create tmpfs + if err := idtools.MkdirAllAndChown(dir, 0700, rootIDs); err != nil { + return errors.Wrap(err, "error creating secret local mount path") + } + + tmpfsOwnership := fmt.Sprintf("uid=%d,gid=%d", rootIDs.UID, rootIDs.GID) + if err := mount.Mount("tmpfs", dir, "tmpfs", "nodev,nosuid,noexec,"+tmpfsOwnership); err != nil { + return errors.Wrap(err, "unable to setup secret mount") + } + + return nil +} + +func (daemon *Daemon) remountSecretDir(c *container.Container) error { + dir, err := c.SecretMountPath() + if err != nil { + return errors.Wrap(err, "error getting container secrets path") + } + if err := label.Relabel(dir, c.MountLabel, false); err != nil { + logrus.WithError(err).WithField("dir", dir).Warn("Error while attempting to set selinux label") + } + rootIDs := daemon.idMappings.RootPair() + tmpfsOwnership := fmt.Sprintf("uid=%d,gid=%d", rootIDs.UID, rootIDs.GID) + + // remount secrets ro + if err := mount.Mount("tmpfs", dir, "tmpfs", "remount,ro,"+tmpfsOwnership); err != nil { + return errors.Wrap(err, "unable to remount dir as readonly") + } + + return nil +} + +func (daemon *Daemon) cleanupSecretDir(c *container.Container) { + dir, err := c.SecretMountPath() + if err != nil { + logrus.WithError(err).WithField("container", c.ID).Warn("error getting secrets mount path for container") + } + if err := mount.RecursiveUnmount(dir); err != nil { + logrus.WithField("dir", dir).WithError(err).Warn("Error while attmepting to unmount dir, this may prevent removal of container.") + } + if err := os.RemoveAll(dir); err != nil && !os.IsNotExist(err) { + logrus.WithField("dir", dir).WithError(err).Error("Error removing dir.") + } } func killProcessDirectly(cntr *container.Container) error { diff --git a/components/engine/daemon/container_operations_windows.go b/components/engine/daemon/container_operations_windows.go index fe9795b8ba..0559b8ac3e 100644 --- a/components/engine/daemon/container_operations_windows.go +++ b/components/engine/daemon/container_operations_windows.go @@ -21,10 +21,7 @@ func (daemon *Daemon) setupConfigDir(c *container.Container) (setupErr error) { return nil } - localPath, err := c.ConfigsDirPath() - if err != nil { - return err - } + localPath := c.ConfigsDirPath() logrus.Debugf("configs: setting up config dir: %s", localPath) // create local config root @@ -51,11 +48,7 @@ func (daemon *Daemon) setupConfigDir(c *container.Container) (setupErr error) { continue } - fPath, err := c.ConfigFilePath(*configRef.ConfigReference) - if err != nil { - return err - } - + fPath := c.ConfigFilePath(*configRef) log := logrus.WithFields(logrus.Fields{"name": configRef.File.Name, "path": fPath}) log.Debug("injecting config") diff --git a/components/engine/daemon/oci_linux.go b/components/engine/daemon/oci_linux.go index 52a944f2a2..68311bd605 100644 --- a/components/engine/daemon/oci_linux.go +++ b/components/engine/daemon/oci_linux.go @@ -842,18 +842,9 @@ func (daemon *Daemon) createSpec(c *container.Container) (retSpec *specs.Spec, e return nil, err } - secretMountPath, err := c.SecretMountPath() - if err != nil { - return nil, err - } - configsMountPath, err := c.ConfigsDirPath() - if err != nil { - return nil, err - } defer func() { if err != nil { - daemon.cleanupSecretDir(secretMountPath) - daemon.cleanupSecretDir(configsMountPath) + daemon.cleanupSecretDir(c) } }() @@ -861,10 +852,6 @@ func (daemon *Daemon) createSpec(c *container.Container) (retSpec *specs.Spec, e return nil, err } - if err := daemon.setupConfigDir(c); err != nil { - return nil, err - } - ms, err := daemon.setupMounts(c) if err != nil { return nil, err @@ -886,12 +873,6 @@ func (daemon *Daemon) createSpec(c *container.Container) (retSpec *specs.Spec, e } ms = append(ms, secretMounts...) - configMounts, err := c.ConfigMounts() - if err != nil { - return nil, err - } - ms = append(ms, configMounts...) - sort.Sort(mounts(ms)) if err := setMounts(daemon, &s, c, ms); err != nil { return nil, fmt.Errorf("linux mounts: %v", err) diff --git a/components/engine/daemon/oci_windows.go b/components/engine/daemon/oci_windows.go index 47b1301eee..64c651c4af 100644 --- a/components/engine/daemon/oci_windows.go +++ b/components/engine/daemon/oci_windows.go @@ -102,10 +102,7 @@ func (daemon *Daemon) createSpec(c *container.Container) (*specs.Spec, error) { mounts = append(mounts, secretMounts...) } - configMounts, err := c.ConfigMounts() - if err != nil { - return nil, err - } + configMounts := c.ConfigMounts() if configMounts != nil { mounts = append(mounts, configMounts...) } diff --git a/components/engine/integration/config/config_test.go b/components/engine/integration/config/config_test.go index 6a96986e4a..c152be59bf 100644 --- a/components/engine/integration/config/config_test.go +++ b/components/engine/integration/config/config_test.go @@ -292,15 +292,24 @@ func TestTemplatedConfig(t *testing.T) { AttachStderr: true, }) - buf := bytes.NewBuffer(nil) - _, err = stdcopy.StdCopy(buf, buf, attach.Reader) - require.NoError(t, err) - expect := "SERVICE_NAME=svc\n" + "this is a secret\n" + "this is a config\n" + assertAttachedStream(t, attach, expect) - assert.Equal(t, expect, buf.String()) + attach = swarm.ExecTask(t, d, task, types.ExecConfig{ + Cmd: []string{"mount"}, + AttachStdout: true, + AttachStderr: true, + }) + assertAttachedStream(t, attach, "tmpfs on /templated_config type tmpfs") +} + +func assertAttachedStream(t *testing.T, attach types.HijackedResponse, expect string) { + buf := bytes.NewBuffer(nil) + _, err := stdcopy.StdCopy(buf, buf, attach.Reader) + require.NoError(t, err) + assert.Contains(t, buf.String(), expect) } func waitAndAssert(t *testing.T, timeout time.Duration, f func(*testing.T) bool) { diff --git a/components/engine/integration/secret/secret_test.go b/components/engine/integration/secret/secret_test.go index e26d8a698d..3b5e66a5bf 100644 --- a/components/engine/integration/secret/secret_test.go +++ b/components/engine/integration/secret/secret_test.go @@ -336,15 +336,24 @@ func TestTemplatedSecret(t *testing.T) { AttachStderr: true, }) - buf := bytes.NewBuffer(nil) - _, err = stdcopy.StdCopy(buf, buf, attach.Reader) - require.NoError(t, err) - expect := "SERVICE_NAME=svc\n" + "this is a secret\n" + "this is a config\n" + assertAttachedStream(t, attach, expect) - assert.Equal(t, expect, buf.String()) + attach = swarm.ExecTask(t, d, task, types.ExecConfig{ + Cmd: []string{"mount"}, + AttachStdout: true, + AttachStderr: true, + }) + assertAttachedStream(t, attach, "tmpfs on /run/secrets/templated_secret type tmpfs") +} + +func assertAttachedStream(t *testing.T, attach types.HijackedResponse, expect string) { + buf := bytes.NewBuffer(nil) + _, err := stdcopy.StdCopy(buf, buf, attach.Reader) + require.NoError(t, err) + assert.Contains(t, buf.String(), expect) } func waitAndAssert(t *testing.T, timeout time.Duration, f func(*testing.T) bool) { From e537ce0b314ee0e590a8a09ffd87ddd301aa9a18 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Wed, 14 Feb 2018 12:45:28 -0500 Subject: [PATCH 13/22] Error out on secret/config templates for older API Makes sure if the user specifies an older API version that we don't pass through templating options for versions that templating was not supported. Signed-off-by: Brian Goff Upstream-commit: a407761e483d9c5ea425a6fd5e55fec03a90485c Component: engine --- .../engine/api/server/router/swarm/cluster_routes.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/components/engine/api/server/router/swarm/cluster_routes.go b/components/engine/api/server/router/swarm/cluster_routes.go index 33e52a3af1..865ed6add6 100644 --- a/components/engine/api/server/router/swarm/cluster_routes.go +++ b/components/engine/api/server/router/swarm/cluster_routes.go @@ -372,6 +372,10 @@ func (sr *swarmRouter) createSecret(ctx context.Context, w http.ResponseWriter, if err := json.NewDecoder(r.Body).Decode(&secret); err != nil { return err } + version := httputils.VersionFromContext(ctx) + if secret.Templating != nil && versions.LessThan(version, "1.36") { + return errdefs.InvalidParameter(errors.Errorf("secret templating is not supported on the specified API version: %s", version)) + } id, err := sr.backend.CreateSecret(secret) if err != nil { @@ -440,6 +444,11 @@ func (sr *swarmRouter) createConfig(ctx context.Context, w http.ResponseWriter, return err } + version := httputils.VersionFromContext(ctx) + if config.Templating != nil && versions.LessThan(version, "1.36") { + return errdefs.InvalidParameter(errors.Errorf("config templating is not supported on the specified API version: %s", version)) + } + id, err := sr.backend.CreateConfig(config) if err != nil { return err From f4580247c7ecfc985bef29c92b5c93abc6d19836 Mon Sep 17 00:00:00 2001 From: Brett Randall Date: Fri, 29 Dec 2017 14:28:46 +1100 Subject: [PATCH 14/22] test.md improvements and corrections: - Mentioned integration-cli test-suite deprecation. - Removed mentions of removed in-container hack/make.sh target test-unit, replaced with hack/test/unit. Signed-off-by: Brett Randall Upstream-commit: acaa53bc35ab8fa97d75e90da393d39204a86a15 Component: engine --- components/engine/docs/contributing/test.md | 22 +++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/components/engine/docs/contributing/test.md b/components/engine/docs/contributing/test.md index 7e9107d116..71a8a91387 100644 --- a/components/engine/docs/contributing/test.md +++ b/components/engine/docs/contributing/test.md @@ -29,7 +29,10 @@ Depending on your contribution, you may need to add _integration tests_. These are tests that combine two or more work units into one component. These work units each have unit tests and then, together, integration tests that test the interface between the components. The `integration` and `integration-cli` -directories in the Docker repository contain integration test code. +directories in the Docker repository contain integration test code. Note that +`integration-cli` tests are now deprecated in the Moby project, and new tests +cannot be added to this suite - add `integration` tests instead using the API +client. Testing is its own specialty. If you aren't familiar with testing techniques, there is a lot of information available to you on the Web. For now, you should @@ -93,7 +96,8 @@ hour. To run the test suite, do the following: ## Run targets inside a development container If you are working inside a development container, you use the -`hack/make.sh` script to run tests. The `hack/make.sh` script doesn't +`hack/test/unit` script to run unit-tests, and `hack/make.sh` script to run +integration and other tests. The `hack/make.sh` script doesn't have a single target that runs all the tests. Instead, you provide a single command line with multiple targets that does the same thing. @@ -110,19 +114,25 @@ Try this now. $ docker run --privileged --rm -ti -v `pwd`:/go/src/github.com/docker/docker dry-run-test /bin/bash ``` -3. Run the tests using the `hack/make.sh` script. +3. Run the unit tests using the `hack/test/unit` script. ```bash - root@5f8630b873fe:/go/src/github.com/docker/docker# hack/make.sh dynbinary binary cross test-unit test-integration test-docker-py + root@5f8630b873fe:/go/src/github.com/docker/docker# hack/test/unit + ``` + +4. Run the tests using the `hack/make.sh` script. + + ```bash + root@5f8630b873fe:/go/src/github.com/docker/docker# hack/make.sh dynbinary binary cross test-integration test-docker-py ``` The tests run just as they did within your local host. Of course, you can also run a subset of these targets too. For example, to run - just the unit tests: + just the integration tests: ```bash - root@5f8630b873fe:/go/src/github.com/docker/docker# hack/make.sh dynbinary binary cross test-unit + root@5f8630b873fe:/go/src/github.com/docker/docker# hack/make.sh dynbinary binary cross test-integration ``` Most test targets require that you build these precursor targets first: From df67d5ea1379886eb664937d911ed4d4b3c41b81 Mon Sep 17 00:00:00 2001 From: Brett Randall Date: Tue, 20 Feb 2018 11:28:54 +1100 Subject: [PATCH 15/22] Removed root@... PS1 from in-container root prompts, retaining #. Signed-off-by: Brett Randall Upstream-commit: bef0cd70a62eff156d99ddd15933b658a1af8893 Component: engine --- .../engine/docs/contributing/set-up-dev-env.md | 14 +++++++------- components/engine/docs/contributing/test.md | 8 ++++---- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/components/engine/docs/contributing/set-up-dev-env.md b/components/engine/docs/contributing/set-up-dev-env.md index 7f8a38119f..311edf8951 100644 --- a/components/engine/docs/contributing/set-up-dev-env.md +++ b/components/engine/docs/contributing/set-up-dev-env.md @@ -131,7 +131,7 @@ can take over 15 minutes to complete. Successfully built 3d872560918e Successfully tagged docker-dev:dry-run-test docker run --rm -i --privileged -e BUILDFLAGS -e KEEPBUNDLE -e DOCKER_BUILD_GOGC -e DOCKER_BUILD_PKGS -e DOCKER_CLIENTONLY -e DOCKER_DEBUG -e DOCKER_EXPERIMENTAL -e DOCKER_GITCOMMIT -e DOCKER_GRAPHDRIVER=devicemapper -e DOCKER_INCREMENTAL_BINARY -e DOCKER_REMAP_ROOT -e DOCKER_STORAGE_OPTS -e DOCKER_USERLANDPROXY -e TESTDIRS -e TESTFLAGS -e TIMEOUT -v "home/ubuntu/repos/docker/bundles:/go/src/github.com/docker/docker/bundles" -t "docker-dev:dry-run-test" bash - root@f31fa223770f:/go/src/github.com/docker/docker# + # ``` At this point, your prompt reflects the container's BASH shell. @@ -146,7 +146,7 @@ can take over 15 minutes to complete. 6. Make a `dockerd` binary. ```none - root@a8b2885ab900:/go/src/github.com/docker/docker# hack/make.sh binary + # hack/make.sh binary Removing bundles/ ---> Making bundle: binary (in bundles/binary) @@ -160,13 +160,13 @@ can take over 15 minutes to complete. `/usr/local/bin/` directory. ```none - root@a8b2885ab900:/go/src/github.com/docker/docker# make install + # make install ``` 8. Start the Engine daemon running in the background. ```none - root@a8b2885ab900:/go/src/github.com/docker/docker# dockerd -D & + # dockerd -D & ...output snipped... DEBU[0001] Registering POST, /networks/{id:.*}/connect DEBU[0001] Registering POST, /networks/{id:.*}/disconnect @@ -252,13 +252,13 @@ can take over 15 minutes to complete. 10. Run the `hello-world` image. ```none - root@5f8630b873fe:/go/src/github.com/docker/docker# docker run hello-world + # docker run hello-world ``` 11. List the image you just downloaded. ```none - root@5f8630b873fe:/go/src/github.com/docker/docker# docker images + # docker images REPOSITORY TAG IMAGE ID CREATED SIZE hello-world latest c54a2cc56cbb 3 months ago 1.85 kB ``` @@ -347,7 +347,7 @@ example, you'll edit the help for the `attach` subcommand. 10. To view your change, run the `dockerd --help` command in the docker development container shell. ```bash - root@b0cb4f22715d:/go/src/github.com/docker/docker# dockerd --help + # dockerd --help Usage: dockerd COMMAND diff --git a/components/engine/docs/contributing/test.md b/components/engine/docs/contributing/test.md index 71a8a91387..fdcee328a9 100644 --- a/components/engine/docs/contributing/test.md +++ b/components/engine/docs/contributing/test.md @@ -117,13 +117,13 @@ Try this now. 3. Run the unit tests using the `hack/test/unit` script. ```bash - root@5f8630b873fe:/go/src/github.com/docker/docker# hack/test/unit + # hack/test/unit ``` 4. Run the tests using the `hack/make.sh` script. ```bash - root@5f8630b873fe:/go/src/github.com/docker/docker# hack/make.sh dynbinary binary cross test-integration test-docker-py + # hack/make.sh dynbinary binary cross test-integration test-docker-py ``` The tests run just as they did within your local host. @@ -132,7 +132,7 @@ Try this now. just the integration tests: ```bash - root@5f8630b873fe:/go/src/github.com/docker/docker# hack/make.sh dynbinary binary cross test-integration + # hack/make.sh dynbinary binary cross test-integration ``` Most test targets require that you build these precursor targets first: @@ -180,7 +180,7 @@ $ TESTFLAGS='-check.f DockerSuite.TestBuild*' make test-integration To run the same test inside your Docker development container, you do this: ```bash -root@5f8630b873fe:/go/src/github.com/docker/docker# TESTFLAGS='-check.f TestBuild*' hack/make.sh binary test-integration +# TESTFLAGS='-check.f TestBuild*' hack/make.sh binary test-integration ``` ## Test the Windows binary against a Linux daemon From 9a83d9fd53ad4b70b331423a2876d2ae07fc78c6 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Thu, 28 Sep 2017 17:10:06 -0400 Subject: [PATCH 16/22] Use gotestyourself env patching Signed-off-by: Daniel Nephin Upstream-commit: 2b445a53c17ae6526c11a729cb6d1d1dc57490ff Component: engine --- components/engine/client/client_test.go | 68 +++++-------------------- 1 file changed, 13 insertions(+), 55 deletions(-) diff --git a/components/engine/client/client_test.go b/components/engine/client/client_test.go index fe452c7aa2..5ae3a51239 100644 --- a/components/engine/client/client_test.go +++ b/components/engine/client/client_test.go @@ -6,18 +6,19 @@ import ( "net/url" "os" "runtime" - "strings" "testing" "github.com/docker/docker/api" "github.com/docker/docker/api/types" "github.com/docker/docker/internal/testutil" + "github.com/gotestyourself/gotestyourself/env" "github.com/gotestyourself/gotestyourself/skip" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestNewEnvClient(t *testing.T) { - skip.IfCondition(t, runtime.GOOS == "windows") + skip.If(t, runtime.GOOS == "windows") testcases := []struct { doc string @@ -83,10 +84,9 @@ func TestNewEnvClient(t *testing.T) { }, } - env := envToMap() - defer mapToEnv(env) + defer env.PatchAll(t, nil)() for _, c := range testcases { - mapToEnv(c.envs) + env.PatchAll(t, c.envs) apiclient, err := NewEnvClient() if c.expectedError != "" { assert.Error(t, err, c.doc) @@ -196,16 +196,12 @@ func TestParseHostURL(t *testing.T) { } func TestNewEnvClientSetsDefaultVersion(t *testing.T) { - env := envToMap() - defer mapToEnv(env) - - envMap := map[string]string{ + defer env.PatchAll(t, map[string]string{ "DOCKER_HOST": "", "DOCKER_API_VERSION": "", "DOCKER_TLS_VERIFY": "", "DOCKER_CERT_PATH": "", - } - mapToEnv(envMap) + })() client, err := NewEnvClient() if err != nil { @@ -225,18 +221,10 @@ func TestNewEnvClientSetsDefaultVersion(t *testing.T) { // TestNegotiateAPIVersionEmpty asserts that client.Client can // negotiate a compatible APIVersion when omitted func TestNegotiateAPIVersionEmpty(t *testing.T) { - env := envToMap() - defer mapToEnv(env) - - envMap := map[string]string{ - "DOCKER_API_VERSION": "", - } - mapToEnv(envMap) + defer env.PatchAll(t, map[string]string{"DOCKER_API_VERSION": ""}) client, err := NewEnvClient() - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) ping := types.Ping{ APIVersion: "", @@ -260,12 +248,9 @@ func TestNegotiateAPIVersionEmpty(t *testing.T) { // negotiate a compatible APIVersion with the server func TestNegotiateAPIVersion(t *testing.T) { client, err := NewEnvClient() - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) expected := "1.21" - ping := types.Ping{ APIVersion: expected, OSType: "linux", @@ -291,18 +276,11 @@ func TestNegotiateAPIVersion(t *testing.T) { // TestNegotiateAPIVersionOverride asserts that we honor // the environment variable DOCKER_API_VERSION when negotianing versions func TestNegotiateAPVersionOverride(t *testing.T) { - env := envToMap() - defer mapToEnv(env) - - envMap := map[string]string{ - "DOCKER_API_VERSION": "9.99", - } - mapToEnv(envMap) + expected := "9.99" + defer env.PatchAll(t, map[string]string{"DOCKER_API_VERSION": expected})() client, err := NewEnvClient() - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) ping := types.Ping{ APIVersion: "1.24", @@ -310,31 +288,11 @@ func TestNegotiateAPVersionOverride(t *testing.T) { Experimental: false, } - expected := envMap["DOCKER_API_VERSION"] - // test that we honored the env var client.NegotiateAPIVersionPing(ping) assert.Equal(t, expected, client.version) } -// mapToEnv takes a map of environment variables and sets them -func mapToEnv(env map[string]string) { - for k, v := range env { - os.Setenv(k, v) - } -} - -// envToMap returns a map of environment variables -func envToMap() map[string]string { - env := make(map[string]string) - for _, e := range os.Environ() { - kv := strings.SplitAfterN(e, "=", 2) - env[kv[0]] = kv[1] - } - - return env -} - type roundTripFunc func(*http.Request) (*http.Response, error) func (rtf roundTripFunc) RoundTrip(req *http.Request) (*http.Response, error) { From 180ce350666476a5e285ff145ed5e8c5fb42c4bf Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 16 Feb 2018 13:24:57 -0500 Subject: [PATCH 17/22] Improve docstrings and small cleanup in client Use client instead of helpers for TLS in integration test Signed-off-by: Daniel Nephin Upstream-commit: a68ae4a2d95b1ff143025a435195af0f1ab30ace Component: engine --- components/engine/client/client.go | 83 ++++++++++++------- .../integration/internal/request/client.go | 34 -------- .../plugin/authz/authz_plugin_test.go | 14 +++- 3 files changed, 66 insertions(+), 65 deletions(-) diff --git a/components/engine/client/client.go b/components/engine/client/client.go index 6ce0cdba1f..f838501760 100644 --- a/components/engine/client/client.go +++ b/components/engine/client/client.go @@ -42,8 +42,8 @@ For example, to list running containers (the equivalent of "docker ps"): package client // import "github.com/docker/docker/client" import ( - "errors" "fmt" + "net" "net/http" "net/url" "os" @@ -56,6 +56,7 @@ import ( "github.com/docker/docker/api/types/versions" "github.com/docker/go-connections/sockets" "github.com/docker/go-connections/tlsconfig" + "github.com/pkg/errors" "golang.org/x/net/context" ) @@ -103,18 +104,21 @@ func CheckRedirect(req *http.Request, via []*http.Request) error { } // NewEnvClient initializes a new API client based on environment variables. -// Use DOCKER_HOST to set the url to the docker server. -// Use DOCKER_API_VERSION to set the version of the API to reach, leave empty for latest. -// Use DOCKER_CERT_PATH to load the TLS certificates from. -// Use DOCKER_TLS_VERIFY to enable or disable TLS verification, off by default. -// deprecated: use NewClientWithOpts(FromEnv) +// See FromEnv for a list of support environment variables. +// +// Deprecated: use NewClientWithOpts(FromEnv) func NewEnvClient() (*Client, error) { return NewClientWithOpts(FromEnv) } -// FromEnv enhance the default client with values from environment variables +// FromEnv configures the client with values from environment variables. +// +// Supported environment variables: +// DOCKER_HOST to set the url to the docker server. +// DOCKER_API_VERSION to set the version of the API to reach, leave empty for latest. +// DOCKER_CERT_PATH to load the TLS certificates from. +// DOCKER_TLS_VERIFY to enable or disable TLS verification, off by default. func FromEnv(c *Client) error { - var httpClient *http.Client if dockerCertPath := os.Getenv("DOCKER_CERT_PATH"); dockerCertPath != "" { options := tlsconfig.Options{ CAFile: filepath.Join(dockerCertPath, "ca.pem"), @@ -127,30 +131,58 @@ func FromEnv(c *Client) error { return err } - httpClient = &http.Client{ - Transport: &http.Transport{ - TLSClientConfig: tlsc, - }, + c.client = &http.Client{ + Transport: &http.Transport{TLSClientConfig: tlsc}, CheckRedirect: CheckRedirect, } - WithHTTPClient(httpClient)(c) } - host := os.Getenv("DOCKER_HOST") - if host != "" { - // WithHost will create an API client if it doesn't exist + if host := os.Getenv("DOCKER_HOST"); host != "" { if err := WithHost(host)(c); err != nil { return err } } - version := os.Getenv("DOCKER_API_VERSION") - if version != "" { + + if version := os.Getenv("DOCKER_API_VERSION"); version != "" { c.version = version c.manualOverride = true } return nil } +// WithTLSClientConfig applies a tls config to the client transport. +func WithTLSClientConfig(cacertPath, certPath, keyPath string) func(*Client) error { + return func(c *Client) error { + opts := tlsconfig.Options{ + CAFile: cacertPath, + CertFile: certPath, + KeyFile: keyPath, + ExclusiveRootPools: true, + } + config, err := tlsconfig.Client(opts) + if err != nil { + return errors.Wrap(err, "failed to create tls config") + } + if transport, ok := c.client.Transport.(*http.Transport); ok { + transport.TLSClientConfig = config + return nil + } + return errors.Errorf("cannot apply tls config to transport: %T", c.client.Transport) + } +} + +// WithDialer applies the dialer.DialContext to the client transport. This can be +// used to set the Timeout and KeepAlive settings of the client. +func WithDialer(dialer *net.Dialer) func(*Client) error { + return func(c *Client) error { + if transport, ok := c.client.Transport.(*http.Transport); ok { + transport.DialContext = dialer.DialContext + return nil + } + return errors.Errorf("cannot apply dialer to transport: %T", c.client.Transport) + } +} + // WithVersion overrides the client version with the specified one func WithVersion(version string) func(*Client) error { return func(c *Client) error { @@ -159,8 +191,7 @@ func WithVersion(version string) func(*Client) error { } } -// WithHost overrides the client host with the specified one, creating a new -// http client if one doesn't exist +// WithHost overrides the client host with the specified one. func WithHost(host string) func(*Client) error { return func(c *Client) error { hostURL, err := ParseHostURL(host) @@ -171,17 +202,10 @@ func WithHost(host string) func(*Client) error { c.proto = hostURL.Scheme c.addr = hostURL.Host c.basePath = hostURL.Path - if c.client == nil { - client, err := defaultHTTPClient(host) - if err != nil { - return err - } - return WithHTTPClient(client)(c) - } if transport, ok := c.client.Transport.(*http.Transport); ok { return sockets.ConfigureTransport(transport, c.proto, c.addr) } - return fmt.Errorf("cannot apply host to http transport") + return errors.Errorf("cannot apply host to transport: %T", c.client.Transport) } } @@ -266,7 +290,7 @@ func defaultHTTPClient(host string) (*http.Client, error) { // It won't send any version information if the version number is empty. It is // highly recommended that you set a version or your client may break if the // server is upgraded. -// deprecated: use NewClientWithOpts +// Deprecated: use NewClientWithOpts func NewClient(host string, version string, client *http.Client, httpHeaders map[string]string) (*Client, error) { return NewClientWithOpts(WithHost(host), WithVersion(version), WithHTTPClient(client), WithHTTPHeaders(httpHeaders)) } @@ -378,6 +402,7 @@ func (cli *Client) CustomHTTPHeaders() map[string]string { } // SetCustomHTTPHeaders that will be set on every HTTP request made by the client. +// Deprecated: use WithHTTPHeaders when creating the client. func (cli *Client) SetCustomHTTPHeaders(headers map[string]string) { cli.customHTTPHeaders = headers } diff --git a/components/engine/integration/internal/request/client.go b/components/engine/integration/internal/request/client.go index 4bfb5c7673..367db14c59 100644 --- a/components/engine/integration/internal/request/client.go +++ b/components/engine/integration/internal/request/client.go @@ -2,8 +2,6 @@ package request // import "github.com/docker/docker/integration/internal/request import ( "fmt" - "net" - "net/http" "testing" "time" @@ -11,8 +9,6 @@ import ( "github.com/docker/docker/client" "github.com/docker/docker/internal/test/environment" - "github.com/docker/go-connections/sockets" - "github.com/docker/go-connections/tlsconfig" "github.com/stretchr/testify/require" ) @@ -24,36 +20,6 @@ func NewAPIClient(t *testing.T, ops ...func(*client.Client) error) client.APICli return clt } -// NewTLSAPIClient returns a docker API client configured with the -// provided TLS settings -func NewTLSAPIClient(t *testing.T, host, cacertPath, certPath, keyPath string) (client.APIClient, error) { - opts := tlsconfig.Options{ - CAFile: cacertPath, - CertFile: certPath, - KeyFile: keyPath, - ExclusiveRootPools: true, - } - config, err := tlsconfig.Client(opts) - require.Nil(t, err) - tr := &http.Transport{ - TLSClientConfig: config, - DialContext: (&net.Dialer{ - KeepAlive: 30 * time.Second, - Timeout: 30 * time.Second, - }).DialContext, - } - proto, addr, _, err := client.ParseHost(host) - require.Nil(t, err) - - sockets.ConfigureTransport(tr, proto, addr) - - httpClient := &http.Client{ - Transport: tr, - CheckRedirect: client.CheckRedirect, - } - return client.NewClientWithOpts(client.WithHost(host), client.WithHTTPClient(httpClient)) -} - // daemonTime provides the current time on the daemon host func daemonTime(ctx context.Context, t *testing.T, client client.APIClient, testEnv *environment.Execution) time.Time { if testEnv.IsLocalDaemon() { diff --git a/components/engine/integration/plugin/authz/authz_plugin_test.go b/components/engine/integration/plugin/authz/authz_plugin_test.go index 94f7b896a7..667fc3d3cc 100644 --- a/components/engine/integration/plugin/authz/authz_plugin_test.go +++ b/components/engine/integration/plugin/authz/authz_plugin_test.go @@ -22,7 +22,6 @@ import ( eventtypes "github.com/docker/docker/api/types/events" "github.com/docker/docker/client" "github.com/docker/docker/integration/internal/container" - "github.com/docker/docker/integration/internal/request" "github.com/docker/docker/internal/test/environment" "github.com/docker/docker/pkg/authorization" "github.com/gotestyourself/gotestyourself/skip" @@ -126,7 +125,7 @@ func TestAuthZPluginTLS(t *testing.T) { ctrl.reqRes.Allow = true ctrl.resRes.Allow = true - client, err := request.NewTLSAPIClient(t, testDaemonHTTPSAddr, cacertPath, clientCertPath, clientKeyPath) + client, err := newTLSAPIClient(testDaemonHTTPSAddr, cacertPath, clientCertPath, clientKeyPath) require.Nil(t, err) _, err = client.ServerVersion(context.Background()) @@ -136,6 +135,17 @@ func TestAuthZPluginTLS(t *testing.T) { require.Equal(t, "client", ctrl.resUser) } +func newTLSAPIClient(host, cacertPath, certPath, keyPath string) (client.APIClient, error) { + dialer := &net.Dialer{ + KeepAlive: 30 * time.Second, + Timeout: 30 * time.Second, + } + return client.NewClientWithOpts( + client.WithTLSClientConfig(cacertPath, certPath, keyPath), + client.WithDialer(dialer), + client.WithHost(host)) +} + func TestAuthZPluginDenyRequest(t *testing.T) { defer setupTestV1(t)() d.Start(t, "--authorization-plugin="+testAuthZPlugin) From 133cf88cacd431525e1a206d99cfad3c05177066 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 20 Feb 2018 13:31:03 -0800 Subject: [PATCH 18/22] integration/TestUpdateMemory: fix false failure This fixes the following test failure: > --- FAIL: TestUpdateMemory (0.53s) > assertions.go:226: > Error Trace: update_linux_test.go:52 > Error: Not equal: > expected: int(524288000) > received: int64(524288000) Fixes: 0f9da07b569f0d9 Signed-off-by: Kir Kolyshkin Upstream-commit: cc866470981a1e6a839004f24eb30bb708078068 Component: engine --- components/engine/integration/container/update_linux_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/engine/integration/container/update_linux_test.go b/components/engine/integration/container/update_linux_test.go index bc8011c1d3..2c8f847ffd 100644 --- a/components/engine/integration/container/update_linux_test.go +++ b/components/engine/integration/container/update_linux_test.go @@ -35,7 +35,7 @@ func TestUpdateMemory(t *testing.T) { const ( setMemory int64 = 314572800 - setMemorySwap = 524288000 + setMemorySwap int64 = 524288000 ) _, err := client.ContainerUpdate(ctx, cID, containertypes.UpdateConfig{ From 088ad71eb5154ba0068392eea68ce82719af4f38 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 20 Feb 2018 13:36:27 -0800 Subject: [PATCH 19/22] integration/testUpdateCPUQuota: fix name The function name should be TestUpdateCPUQuota and not TestUpdateCPUQUota. Signed-off-by: Kir Kolyshkin Upstream-commit: 31825081d4c5e643a150b515547cb2a2ea223de4 Component: engine --- components/engine/integration/container/update_linux_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/engine/integration/container/update_linux_test.go b/components/engine/integration/container/update_linux_test.go index 2c8f847ffd..9028f30386 100644 --- a/components/engine/integration/container/update_linux_test.go +++ b/components/engine/integration/container/update_linux_test.go @@ -66,7 +66,7 @@ func TestUpdateMemory(t *testing.T) { assert.Equal(t, strconv.FormatInt(setMemorySwap, 10), strings.TrimSpace(res.Stdout())) } -func TestUpdateCPUQUota(t *testing.T) { +func TestUpdateCPUQuota(t *testing.T) { t.Parallel() defer setupTest(t)() From e0869be245ab8487c9d9558233f112351f56697e Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Tue, 20 Feb 2018 16:57:20 -0500 Subject: [PATCH 20/22] Cleanup volume plugin test with bad assumptions Test made some bad assumptions about on-disk state of volume data. This updates the test to only test based on what the volume API is designed to provide. Signed-off-by: Brian Goff Upstream-commit: 0df654f3d61d8691ee113a8429bcc8ef65786bc7 Component: engine --- .../docker_cli_daemon_plugins_test.go | 27 ++----------------- 1 file changed, 2 insertions(+), 25 deletions(-) diff --git a/components/engine/integration-cli/docker_cli_daemon_plugins_test.go b/components/engine/integration-cli/docker_cli_daemon_plugins_test.go index 10aa514fe0..c527cb1d8e 100644 --- a/components/engine/integration-cli/docker_cli_daemon_plugins_test.go +++ b/components/engine/integration-cli/docker_cli_daemon_plugins_test.go @@ -3,8 +3,6 @@ package main import ( - "os" - "path/filepath" "strings" "github.com/docker/docker/integration-cli/checker" @@ -199,12 +197,6 @@ func (s *DockerDaemonSuite) TestVolumePlugin(c *check.C) { if err != nil { c.Fatalf("Could not install plugin: %v %s", err, out) } - pluginID, err := s.d.Cmd("plugin", "inspect", "-f", "{{.Id}}", pName) - pluginID = strings.TrimSpace(pluginID) - if err != nil { - c.Fatalf("Could not retrieve plugin ID: %v %s", err, pluginID) - } - mountpointPrefix := filepath.Join(s.d.RootDir(), "plugins", pluginID, "rootfs") defer func() { if out, err := s.d.Cmd("plugin", "disable", pName); err != nil { c.Fatalf("Could not disable plugin: %v %s", err, out) @@ -213,11 +205,6 @@ func (s *DockerDaemonSuite) TestVolumePlugin(c *check.C) { if out, err := s.d.Cmd("plugin", "remove", pName); err != nil { c.Fatalf("Could not remove plugin: %v %s", err, out) } - - exists, err := existsMountpointWithPrefix(mountpointPrefix) - c.Assert(err, checker.IsNil) - c.Assert(exists, checker.Equals, false) - }() out, err = s.d.Cmd("volume", "create", "-d", pName, volName) @@ -237,21 +224,11 @@ func (s *DockerDaemonSuite) TestVolumePlugin(c *check.C) { c.Assert(out, checker.Contains, volName) c.Assert(out, checker.Contains, pName) - mountPoint, err := s.d.Cmd("volume", "inspect", volName, "--format", "{{.Mountpoint}}") - if err != nil { - c.Fatalf("Could not inspect volume: %v %s", err, mountPoint) - } - mountPoint = strings.TrimSpace(mountPoint) - out, err = s.d.Cmd("run", "--rm", "-v", volName+":"+destDir, "busybox", "touch", destDir+destFile) c.Assert(err, checker.IsNil, check.Commentf(out)) - path := filepath.Join(s.d.RootDir(), "plugins", pluginID, "rootfs", mountPoint, destFile) - _, err = os.Lstat(path) - c.Assert(err, checker.IsNil) - exists, err := existsMountpointWithPrefix(mountpointPrefix) - c.Assert(err, checker.IsNil) - c.Assert(exists, checker.Equals, true) + out, err = s.d.Cmd("run", "--rm", "-v", volName+":"+destDir, "busybox", "ls", destDir+destFile) + c.Assert(err, checker.IsNil, check.Commentf(out)) } func (s *DockerDaemonSuite) TestGraphdriverPlugin(c *check.C) { From 49adb54d7197eb5a5bd814f68080079e1c59cc04 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Tue, 19 Sep 2017 16:12:29 -0400 Subject: [PATCH 21/22] Remove duplicate calls for getting an APIClient Remove request.SockRequest Remove request.SockRequestHijack Remove request.SockRequestRaw() Remove deprecated ParseHost Deprecate and unexport more helpers Signed-off-by: Daniel Nephin Upstream-commit: 0a91ba2d8cfe16df0ba37c1e283c8e3dbbb086d4 Component: engine --- components/engine/client/client.go | 11 -- components/engine/client/client_test.go | 26 ---- components/engine/client/interface.go | 1 + components/engine/client/request.go | 5 +- components/engine/client/request_test.go | 17 ++- .../engine/integration-cli/daemon/daemon.go | 33 +---- .../integration-cli/daemon/daemon_swarm.go | 35 +++--- .../integration-cli/docker_api_attach_test.go | 58 +++++++-- .../integration-cli/docker_api_build_test.go | 16 +-- .../docker_api_containers_windows_test.go | 49 ++++---- .../docker_api_exec_resize_test.go | 2 +- .../integration-cli/docker_api_images_test.go | 24 +--- .../docker_api_ipcmode_test.go | 11 +- .../integration-cli/docker_api_swarm_test.go | 42 +++---- .../docker_cli_plugins_test.go | 16 +-- .../engine/integration-cli/request/request.go | 117 +++--------------- .../integration-cli/trust_server_test.go | 6 +- 17 files changed, 158 insertions(+), 311 deletions(-) diff --git a/components/engine/client/client.go b/components/engine/client/client.go index f838501760..e129bb20f3 100644 --- a/components/engine/client/client.go +++ b/components/engine/client/client.go @@ -356,17 +356,6 @@ func (cli *Client) DaemonHost() string { return cli.host } -// ParseHost parses a url string, validates the strings is a host url, and returns -// the parsed host as: protocol, address, and base path -// Deprecated: use ParseHostURL -func ParseHost(host string) (string, string, string, error) { - hostURL, err := ParseHostURL(host) - if err != nil { - return "", "", "", err - } - return hostURL.Scheme, hostURL.Host, hostURL.Path, nil -} - // ParseHostURL parses a url string, validates the string is a host url, and // returns the parsed URL func ParseHostURL(host string) (*url.URL, error) { diff --git a/components/engine/client/client_test.go b/components/engine/client/client_test.go index 5ae3a51239..db394e24b5 100644 --- a/components/engine/client/client_test.go +++ b/components/engine/client/client_test.go @@ -132,32 +132,6 @@ func TestGetAPIPath(t *testing.T) { } } -func TestParseHost(t *testing.T) { - cases := []struct { - host string - proto string - addr string - base string - err bool - }{ - {"", "", "", "", true}, - {"foobar", "", "", "", true}, - {"foo://bar", "foo", "bar", "", false}, - {"tcp://localhost:2476", "tcp", "localhost:2476", "", false}, - {"tcp://localhost:2476/path", "tcp", "localhost:2476", "/path", false}, - } - - for _, cs := range cases { - p, a, b, e := ParseHost(cs.host) - if cs.err { - assert.Error(t, e) - } - assert.Equal(t, cs.proto, p) - assert.Equal(t, cs.addr, a) - assert.Equal(t, cs.base, b) - } -} - func TestParseHostURL(t *testing.T) { testcases := []struct { host string diff --git a/components/engine/client/interface.go b/components/engine/client/interface.go index e928e647a7..8517546abd 100644 --- a/components/engine/client/interface.go +++ b/components/engine/client/interface.go @@ -37,6 +37,7 @@ type CommonAPIClient interface { NegotiateAPIVersion(ctx context.Context) NegotiateAPIVersionPing(types.Ping) DialSession(ctx context.Context, proto string, meta map[string][]string) (net.Conn, error) + Close() error } // ContainerAPIClient defines API client methods for the containers diff --git a/components/engine/client/request.go b/components/engine/client/request.go index 986b512dda..302f599f5c 100644 --- a/components/engine/client/request.go +++ b/components/engine/client/request.go @@ -123,10 +123,7 @@ func (cli *Client) sendRequest(ctx context.Context, method, path string, query u if err != nil { return resp, err } - if err := cli.checkResponseErr(resp); err != nil { - return resp, err - } - return resp, nil + return resp, cli.checkResponseErr(resp) } func (cli *Client) doRequest(ctx context.Context, req *http.Request) (serverResponse, error) { diff --git a/components/engine/client/request_test.go b/components/engine/client/request_test.go index f3c136a04f..1dbfed62c7 100644 --- a/components/engine/client/request_test.go +++ b/components/engine/client/request_test.go @@ -9,6 +9,7 @@ import ( "testing" "github.com/docker/docker/api/types" + "github.com/stretchr/testify/require" "golang.org/x/net/context" ) @@ -44,10 +45,8 @@ func TestSetHostHeader(t *testing.T) { } for c, test := range testCases { - proto, addr, basePath, err := ParseHost(test.host) - if err != nil { - t.Fatal(err) - } + hostURL, err := ParseHostURL(test.host) + require.NoError(t, err) client := &Client{ client: newMockClient(func(req *http.Request) (*http.Response, error) { @@ -66,15 +65,13 @@ func TestSetHostHeader(t *testing.T) { }, nil }), - proto: proto, - addr: addr, - basePath: basePath, + proto: hostURL.Scheme, + addr: hostURL.Host, + basePath: hostURL.Path, } _, err = client.sendRequest(context.Background(), "GET", testURL, nil, nil, nil) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) } } diff --git a/components/engine/integration-cli/daemon/daemon.go b/components/engine/integration-cli/daemon/daemon.go index 69a87d9d9c..9672d160f8 100644 --- a/components/engine/integration-cli/daemon/daemon.go +++ b/components/engine/integration-cli/daemon/daemon.go @@ -1,7 +1,6 @@ package daemon // import "github.com/docker/docker/integration-cli/daemon" import ( - "bytes" "encoding/json" "fmt" "io" @@ -14,7 +13,6 @@ import ( "strings" "time" - "github.com/docker/docker/api" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/events" "github.com/docker/docker/client" @@ -596,28 +594,6 @@ func (d *Daemon) PrependHostArg(args []string) []string { return append([]string{"--host", d.Sock()}, args...) } -// SockRequest executes a socket request on a daemon and returns statuscode and output. -func (d *Daemon) SockRequest(method, endpoint string, data interface{}) (int, []byte, error) { - jsonData := bytes.NewBuffer(nil) - if err := json.NewEncoder(jsonData).Encode(data); err != nil { - return -1, nil, err - } - - res, body, err := d.SockRequestRaw(method, endpoint, jsonData, "application/json") - if err != nil { - return -1, nil, err - } - b, err := request.ReadBody(body) - return res.StatusCode, b, err -} - -// SockRequestRaw executes a socket request on a daemon and returns an http -// response and a reader for the output data. -// Deprecated: use request package instead -func (d *Daemon) SockRequestRaw(method, endpoint string, data io.Reader, ct string) (*http.Response, io.ReadCloser, error) { - return request.SockRequestRaw(method, endpoint, data, ct, d.Sock()) -} - // LogFileName returns the path the daemon's log file func (d *Daemon) LogFileName() string { return d.logFile.Name() @@ -746,12 +722,9 @@ func (d *Daemon) ReloadConfig() error { // NewClient creates new client based on daemon's socket path func (d *Daemon) NewClient() (*client.Client, error) { - httpClient, err := request.NewHTTPClient(d.Sock()) - if err != nil { - return nil, err - } - - return client.NewClient(d.Sock(), api.DefaultVersion, httpClient, nil) + return client.NewClientWithOpts( + client.FromEnv, + client.WithHost(d.Sock())) } // WaitInspectWithArgs waits for the specified expression to be equals to the specified expected string in the given time. diff --git a/components/engine/integration-cli/daemon/daemon_swarm.go b/components/engine/integration-cli/daemon/daemon_swarm.go index 4be0acc9e9..cb44f63f23 100644 --- a/components/engine/integration-cli/daemon/daemon_swarm.go +++ b/components/engine/integration-cli/daemon/daemon_swarm.go @@ -1,18 +1,18 @@ package daemon // import "github.com/docker/docker/integration-cli/daemon" import ( - "encoding/json" "fmt" - "net/http" "strings" "time" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/filters" "github.com/docker/docker/api/types/swarm" + "github.com/docker/docker/client" "github.com/docker/docker/integration-cli/checker" "github.com/go-check/check" "github.com/pkg/errors" + "github.com/stretchr/testify/require" "golang.org/x/net/context" ) @@ -234,31 +234,28 @@ func (d *Swarm) CheckServiceUpdateState(service string) func(*check.C) (interfac // CheckPluginRunning returns the runtime state of the plugin func (d *Swarm) CheckPluginRunning(plugin string) func(c *check.C) (interface{}, check.CommentInterface) { return func(c *check.C) (interface{}, check.CommentInterface) { - status, out, err := d.SockRequest("GET", "/plugins/"+plugin+"/json", nil) - c.Assert(err, checker.IsNil, check.Commentf(string(out))) - if status != http.StatusOK { - return false, nil + apiclient, err := d.NewClient() + require.NoError(c, err) + resp, _, err := apiclient.PluginInspectWithRaw(context.Background(), plugin) + if client.IsErrNotFound(err) { + return false, check.Commentf("%v", err) } - - var p types.Plugin - c.Assert(json.Unmarshal(out, &p), checker.IsNil, check.Commentf(string(out))) - - return p.Enabled, check.Commentf("%+v", p) + require.NoError(c, err) + return resp.Enabled, check.Commentf("%+v", resp) } } // CheckPluginImage returns the runtime state of the plugin func (d *Swarm) CheckPluginImage(plugin string) func(c *check.C) (interface{}, check.CommentInterface) { return func(c *check.C) (interface{}, check.CommentInterface) { - status, out, err := d.SockRequest("GET", "/plugins/"+plugin+"/json", nil) - c.Assert(err, checker.IsNil, check.Commentf(string(out))) - if status != http.StatusOK { - return false, nil + apiclient, err := d.NewClient() + require.NoError(c, err) + resp, _, err := apiclient.PluginInspectWithRaw(context.Background(), plugin) + if client.IsErrNotFound(err) { + return false, check.Commentf("%v", err) } - - var p types.Plugin - c.Assert(json.Unmarshal(out, &p), checker.IsNil, check.Commentf(string(out))) - return p.PluginReference, check.Commentf("%+v", p) + require.NoError(c, err) + return resp.PluginReference, check.Commentf("%+v", resp) } } diff --git a/components/engine/integration-cli/docker_api_attach_test.go b/components/engine/integration-cli/docker_api_attach_test.go index a3aa40f6a4..e191f278f7 100644 --- a/components/engine/integration-cli/docker_api_attach_test.go +++ b/components/engine/integration-cli/docker_api_attach_test.go @@ -4,9 +4,11 @@ import ( "bufio" "bytes" "context" + "fmt" "io" "net" "net/http" + "net/http/httputil" "strings" "time" @@ -73,10 +75,8 @@ func (s *DockerSuite) TestGetContainersAttachWebsocket(c *check.C) { // regression gh14320 func (s *DockerSuite) TestPostContainersAttachContainerNotFound(c *check.C) { - client, err := request.NewHTTPClient(daemonHost()) + resp, _, err := request.Post("/containers/doesnotexist/attach") c.Assert(err, checker.IsNil) - req, err := request.New(daemonHost(), "/containers/doesnotexist/attach", request.Method(http.MethodPost)) - resp, err := client.Do(req) // connection will shutdown, err should be "persistent connection closed" c.Assert(resp.StatusCode, checker.Equals, http.StatusNotFound) content, err := request.ReadBody(resp.Body) @@ -140,12 +140,12 @@ func (s *DockerSuite) TestPostContainersAttach(c *check.C) { cid, _ := dockerCmd(c, "run", "-di", "busybox", "cat") cid = strings.TrimSpace(cid) // Attach to the container's stdout stream. - conn, br, err := request.SockRequestHijack("POST", "/containers/"+cid+"/attach?stream=1&stdin=1&stdout=1", nil, "text/plain", daemonHost()) + conn, br, err := sockRequestHijack("POST", "/containers/"+cid+"/attach?stream=1&stdin=1&stdout=1", nil, "text/plain", daemonHost()) c.Assert(err, checker.IsNil) // Check if the data from stdout can be received. expectSuccess(conn, br, "stdout", false) // Attach to the container's stderr stream. - conn, br, err = request.SockRequestHijack("POST", "/containers/"+cid+"/attach?stream=1&stdin=1&stderr=1", nil, "text/plain", daemonHost()) + conn, br, err = sockRequestHijack("POST", "/containers/"+cid+"/attach?stream=1&stdin=1&stderr=1", nil, "text/plain", daemonHost()) c.Assert(err, checker.IsNil) // Since the container only emits stdout, attaching to stderr should return nothing. expectTimeout(conn, br, "stdout") @@ -153,10 +153,10 @@ func (s *DockerSuite) TestPostContainersAttach(c *check.C) { // Test the similar functions of the stderr stream. cid, _ = dockerCmd(c, "run", "-di", "busybox", "/bin/sh", "-c", "cat >&2") cid = strings.TrimSpace(cid) - conn, br, err = request.SockRequestHijack("POST", "/containers/"+cid+"/attach?stream=1&stdin=1&stderr=1", nil, "text/plain", daemonHost()) + conn, br, err = sockRequestHijack("POST", "/containers/"+cid+"/attach?stream=1&stdin=1&stderr=1", nil, "text/plain", daemonHost()) c.Assert(err, checker.IsNil) expectSuccess(conn, br, "stderr", false) - conn, br, err = request.SockRequestHijack("POST", "/containers/"+cid+"/attach?stream=1&stdin=1&stdout=1", nil, "text/plain", daemonHost()) + conn, br, err = sockRequestHijack("POST", "/containers/"+cid+"/attach?stream=1&stdin=1&stdout=1", nil, "text/plain", daemonHost()) c.Assert(err, checker.IsNil) expectTimeout(conn, br, "stderr") @@ -164,12 +164,12 @@ func (s *DockerSuite) TestPostContainersAttach(c *check.C) { cid, _ = dockerCmd(c, "run", "-dit", "busybox", "/bin/sh", "-c", "cat >&2") cid = strings.TrimSpace(cid) // Attach to stdout only. - conn, br, err = request.SockRequestHijack("POST", "/containers/"+cid+"/attach?stream=1&stdin=1&stdout=1", nil, "text/plain", daemonHost()) + conn, br, err = sockRequestHijack("POST", "/containers/"+cid+"/attach?stream=1&stdin=1&stdout=1", nil, "text/plain", daemonHost()) c.Assert(err, checker.IsNil) expectSuccess(conn, br, "stdout", true) // Attach without stdout stream. - conn, br, err = request.SockRequestHijack("POST", "/containers/"+cid+"/attach?stream=1&stdin=1&stderr=1", nil, "text/plain", daemonHost()) + conn, br, err = sockRequestHijack("POST", "/containers/"+cid+"/attach?stream=1&stdin=1&stderr=1", nil, "text/plain", daemonHost()) c.Assert(err, checker.IsNil) // Nothing should be received because both the stdout and stderr of the container will be // sent to the client as stdout when tty is enabled. @@ -210,3 +210,43 @@ func (s *DockerSuite) TestPostContainersAttach(c *check.C) { stdcopy.StdCopy(actualStdout, actualStderr, resp.Reader) c.Assert(actualStdout.Bytes(), checker.DeepEquals, []byte("hello\nsuccess"), check.Commentf("Attach didn't return the expected data from stdout")) } + +// SockRequestHijack creates a connection to specified host (with method, contenttype, …) and returns a hijacked connection +// and the output as a `bufio.Reader` +func sockRequestHijack(method, endpoint string, data io.Reader, ct string, daemon string, modifiers ...func(*http.Request)) (net.Conn, *bufio.Reader, error) { + req, client, err := newRequestClient(method, endpoint, data, ct, daemon, modifiers...) + if err != nil { + return nil, nil, err + } + + client.Do(req) + conn, br := client.Hijack() + return conn, br, nil +} + +// FIXME(vdemeester) httputil.ClientConn is deprecated, use http.Client instead (closer to actual client) +// Deprecated: Use New instead of NewRequestClient +// Deprecated: use request.Do (or Get, Delete, Post) instead +func newRequestClient(method, endpoint string, data io.Reader, ct, daemon string, modifiers ...func(*http.Request)) (*http.Request, *httputil.ClientConn, error) { + c, err := request.SockConn(time.Duration(10*time.Second), daemon) + if err != nil { + return nil, nil, fmt.Errorf("could not dial docker daemon: %v", err) + } + + client := httputil.NewClientConn(c, nil) + + req, err := http.NewRequest(method, endpoint, data) + if err != nil { + client.Close() + return nil, nil, fmt.Errorf("could not create new request: %v", err) + } + + for _, opt := range modifiers { + opt(req) + } + + if ct != "" { + req.Header.Set("Content-Type", ct) + } + return req, client, nil +} diff --git a/components/engine/integration-cli/docker_api_build_test.go b/components/engine/integration-cli/docker_api_build_test.go index 6da8e5bb8f..e5423a4741 100644 --- a/components/engine/integration-cli/docker_api_build_test.go +++ b/components/engine/integration-cli/docker_api_build_test.go @@ -319,8 +319,7 @@ func (s *DockerSuite) TestBuildOnBuildCache(c *check.C) { assert.Len(c, imageIDs, 2) parentID, childID := imageIDs[0], imageIDs[1] - client, err := request.NewClient() - require.NoError(c, err) + client := testEnv.APIClient() // check parentID is correct image, _, err := client.ImageInspectWithRaw(context.Background(), childID) @@ -329,12 +328,11 @@ func (s *DockerSuite) TestBuildOnBuildCache(c *check.C) { } func (s *DockerRegistrySuite) TestBuildCopyFromForcePull(c *check.C) { - client, err := request.NewClient() - require.NoError(c, err) + client := testEnv.APIClient() repoName := fmt.Sprintf("%v/dockercli/busybox", privateRegistryURL) // tag the image to upload it to the private registry - err = client.ImageTag(context.TODO(), "busybox", repoName) + err := client.ImageTag(context.TODO(), "busybox", repoName) assert.Nil(c, err) // push the image to the registry rc, err := client.ImagePush(context.TODO(), repoName, types.ImagePushOptions{RegistryAuth: "{}"}) @@ -545,9 +543,7 @@ func (s *DockerSuite) TestBuildWithSession(c *check.C) { assert.Equal(c, strings.Count(out, "Using cache"), 2) assert.Contains(c, out, "contentcontent") - client, err := request.NewClient() - require.NoError(c, err) - + client := testEnv.APIClient() du, err := client.DiskUsage(context.TODO()) assert.Nil(c, err) assert.True(c, du.BuilderSize > 10) @@ -582,9 +578,7 @@ func (s *DockerSuite) TestBuildWithSession(c *check.C) { } func testBuildWithSession(c *check.C, dir, dockerfile string) (outStr string) { - client, err := request.NewClient() - require.NoError(c, err) - + client := testEnv.APIClient() sess, err := session.NewSession("foo1", "foo") assert.Nil(c, err) diff --git a/components/engine/integration-cli/docker_api_containers_windows_test.go b/components/engine/integration-cli/docker_api_containers_windows_test.go index 25bc767e55..eb2892575c 100644 --- a/components/engine/integration-cli/docker_api_containers_windows_test.go +++ b/components/engine/integration-cli/docker_api_containers_windows_test.go @@ -6,13 +6,16 @@ import ( "fmt" "io/ioutil" "math/rand" - "net/http" "strings" winio "github.com/Microsoft/go-winio" - "github.com/docker/docker/integration-cli/checker" - "github.com/docker/docker/integration-cli/request" + "github.com/docker/docker/api/types" + "github.com/docker/docker/api/types/container" + "github.com/docker/docker/api/types/mount" "github.com/go-check/check" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/net/context" ) func (s *DockerSuite) TestContainersAPICreateMountsBindNamedPipe(c *check.C) { @@ -44,28 +47,30 @@ func (s *DockerSuite) TestContainersAPICreateMountsBindNamedPipe(c *check.C) { containerPipeName := `\\.\pipe\docker-cli-test-pipe` text := "hello from a pipe" cmd := fmt.Sprintf("echo %s > %s", text, containerPipeName) - name := "test-bind-npipe" - data := map[string]interface{}{ - "Image": testEnv.PlatformDefaults.BaseImage, - "Cmd": []string{"cmd", "/c", cmd}, - "HostConfig": map[string]interface{}{"Mounts": []map[string]interface{}{{"Type": "npipe", "Source": hostPipeName, "Target": containerPipeName}}}, - } - status, resp, err := request.SockRequest("POST", "/containers/create?name="+name, data, daemonHost()) - c.Assert(err, checker.IsNil, check.Commentf(string(resp))) - c.Assert(status, checker.Equals, http.StatusCreated, check.Commentf(string(resp))) + ctx := context.Background() + client := testEnv.APIClient() + _, err = client.ContainerCreate(ctx, + &container.Config{ + Image: testEnv.PlatformDefaults.BaseImage, + Cmd: []string{"cmd", "/c", cmd}, + }, &container.HostConfig{ + Mounts: []mount.Mount{ + { + Type: "npipe", + Source: hostPipeName, + Target: containerPipeName, + }, + }, + }, + nil, name) + require.NoError(c, err) - status, _, err = request.SockRequest("POST", "/containers/"+name+"/start", nil, daemonHost()) - c.Assert(err, checker.IsNil) - c.Assert(status, checker.Equals, http.StatusNoContent) + err = client.ContainerStart(ctx, name, types.ContainerStartOptions{}) + require.NoError(c, err) err = <-ch - if err != nil { - c.Fatal(err) - } - result := strings.TrimSpace(string(b)) - if result != text { - c.Errorf("expected pipe to contain %s, got %s", text, result) - } + require.NoError(c, err) + assert.Equal(c, text, strings.TrimSpace(string(b))) } diff --git a/components/engine/integration-cli/docker_api_exec_resize_test.go b/components/engine/integration-cli/docker_api_exec_resize_test.go index 961d911cbd..7289b2689c 100644 --- a/components/engine/integration-cli/docker_api_exec_resize_test.go +++ b/components/engine/integration-cli/docker_api_exec_resize_test.go @@ -59,7 +59,7 @@ func (s *DockerSuite) TestExecResizeImmediatelyAfterExecStart(c *check.C) { } payload := bytes.NewBufferString(`{"Tty":true}`) - conn, _, err := request.SockRequestHijack("POST", fmt.Sprintf("/exec/%s/start", execID), payload, "application/json", daemonHost()) + conn, _, err := sockRequestHijack("POST", fmt.Sprintf("/exec/%s/start", execID), payload, "application/json", daemonHost()) if err != nil { return fmt.Errorf("Failed to start the exec: %q", err.Error()) } diff --git a/components/engine/integration-cli/docker_api_images_test.go b/components/engine/integration-cli/docker_api_images_test.go index a5628e41b9..7c6c235354 100644 --- a/components/engine/integration-cli/docker_api_images_test.go +++ b/components/engine/integration-cli/docker_api_images_test.go @@ -157,33 +157,21 @@ func (s *DockerSuite) TestAPIImagesSearchJSONContentType(c *check.C) { // Test case for 30027: image size reported as -1 in v1.12 client against v1.13 daemon. // This test checks to make sure both v1.12 and v1.13 client against v1.13 daemon get correct `Size` after the fix. func (s *DockerSuite) TestAPIImagesSizeCompatibility(c *check.C) { - cli, err := client.NewEnvClient() - c.Assert(err, checker.IsNil) - defer cli.Close() + apiclient := testEnv.APIClient() + defer apiclient.Close() - images, err := cli.ImageList(context.Background(), types.ImageListOptions{}) + images, err := apiclient.ImageList(context.Background(), types.ImageListOptions{}) c.Assert(err, checker.IsNil) c.Assert(len(images), checker.Not(checker.Equals), 0) for _, image := range images { c.Assert(image.Size, checker.Not(checker.Equals), int64(-1)) } - type v124Image struct { - ID string `json:"Id"` - ParentID string `json:"ParentId"` - RepoTags []string - RepoDigests []string - Created int64 - Size int64 - VirtualSize int64 - Labels map[string]string - } - - cli, err = client.NewClientWithOpts(client.FromEnv, client.WithVersion("v1.24")) + apiclient, err = client.NewClientWithOpts(client.FromEnv, client.WithVersion("v1.24")) c.Assert(err, checker.IsNil) - defer cli.Close() + defer apiclient.Close() - v124Images, err := cli.ImageList(context.Background(), types.ImageListOptions{}) + v124Images, err := apiclient.ImageList(context.Background(), types.ImageListOptions{}) c.Assert(err, checker.IsNil) c.Assert(len(v124Images), checker.Not(checker.Equals), 0) for _, image := range v124Images { diff --git a/components/engine/integration-cli/docker_api_ipcmode_test.go b/components/engine/integration-cli/docker_api_ipcmode_test.go index 02548875a3..5bca6d0d78 100644 --- a/components/engine/integration-cli/docker_api_ipcmode_test.go +++ b/components/engine/integration-cli/docker_api_ipcmode_test.go @@ -12,7 +12,6 @@ import ( "github.com/docker/docker/api/types/container" "github.com/docker/docker/integration-cli/checker" "github.com/docker/docker/integration-cli/cli" - "github.com/docker/docker/integration-cli/request" "github.com/go-check/check" "golang.org/x/net/context" ) @@ -59,8 +58,7 @@ func testIpcNonePrivateShareable(c *check.C, mode string, mustBeMounted bool, mu } ctx := context.Background() - client, err := request.NewClient() - c.Assert(err, checker.IsNil) + client := testEnv.APIClient() resp, err := client.ContainerCreate(ctx, &cfg, &hostCfg, nil, "") c.Assert(err, checker.IsNil) @@ -125,8 +123,7 @@ func testIpcContainer(s *DockerSuite, c *check.C, donorMode string, mustWork boo } ctx := context.Background() - client, err := request.NewClient() - c.Assert(err, checker.IsNil) + client := testEnv.APIClient() // create and start the "donor" container resp, err := client.ContainerCreate(ctx, &cfg, &hostCfg, nil, "") @@ -195,9 +192,7 @@ func (s *DockerSuite) TestAPIIpcModeHost(c *check.C) { } ctx := context.Background() - client, err := request.NewClient() - c.Assert(err, checker.IsNil) - + client := testEnv.APIClient() resp, err := client.ContainerCreate(ctx, &cfg, &hostCfg, nil, "") c.Assert(err, checker.IsNil) c.Assert(len(resp.Warnings), checker.Equals, 0) diff --git a/components/engine/integration-cli/docker_api_swarm_test.go b/components/engine/integration-cli/docker_api_swarm_test.go index a9b0458038..fd0b4e6278 100644 --- a/components/engine/integration-cli/docker_api_swarm_test.go +++ b/components/engine/integration-cli/docker_api_swarm_test.go @@ -3,12 +3,10 @@ package main import ( - "encoding/json" "fmt" "io/ioutil" "net" "net/http" - "net/url" "os" "path/filepath" "strings" @@ -21,11 +19,14 @@ import ( "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/swarm" + "github.com/docker/docker/client" "github.com/docker/docker/integration-cli/checker" "github.com/docker/docker/integration-cli/daemon" "github.com/docker/docker/integration-cli/request" "github.com/docker/swarmkit/ca" "github.com/go-check/check" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "golang.org/x/net/context" ) @@ -1006,32 +1007,19 @@ func (s *DockerSwarmSuite) TestSwarmRepeatedRootRotation(c *check.C) { func (s *DockerSwarmSuite) TestAPINetworkInspectWithScope(c *check.C) { d := s.AddDaemon(c, true, true) - name := "foo" - networkCreateRequest := types.NetworkCreateRequest{ - Name: name, - } + name := "test-scoped-network" + ctx := context.Background() + apiclient, err := d.NewClient() + require.NoError(c, err) - var n types.NetworkCreateResponse - networkCreateRequest.NetworkCreate.Driver = "overlay" + resp, err := apiclient.NetworkCreate(ctx, name, types.NetworkCreate{Driver: "overlay"}) + require.NoError(c, err) - status, out, err := d.SockRequest("POST", "/networks/create", networkCreateRequest) - c.Assert(err, checker.IsNil, check.Commentf(string(out))) - c.Assert(status, checker.Equals, http.StatusCreated, check.Commentf(string(out))) - c.Assert(json.Unmarshal(out, &n), checker.IsNil) + network, err := apiclient.NetworkInspect(ctx, name, types.NetworkInspectOptions{}) + require.NoError(c, err) + assert.Equal(c, "swarm", network.Scope) + assert.Equal(c, resp.ID, network.ID) - var r types.NetworkResource - - status, body, err := d.SockRequest("GET", "/networks/"+name, nil) - c.Assert(err, checker.IsNil, check.Commentf(string(out))) - c.Assert(status, checker.Equals, http.StatusOK, check.Commentf(string(out))) - c.Assert(json.Unmarshal(body, &r), checker.IsNil) - c.Assert(r.Scope, checker.Equals, "swarm") - c.Assert(r.ID, checker.Equals, n.ID) - - v := url.Values{} - v.Set("scope", "local") - - status, _, err = d.SockRequest("GET", "/networks/"+name+"?"+v.Encode(), nil) - c.Assert(err, checker.IsNil, check.Commentf(string(out))) - c.Assert(status, checker.Equals, http.StatusNotFound, check.Commentf(string(out))) + _, err = apiclient.NetworkInspect(ctx, name, types.NetworkInspectOptions{Scope: "local"}) + assert.True(c, client.IsErrNotFound(err)) } diff --git a/components/engine/integration-cli/docker_cli_plugins_test.go b/components/engine/integration-cli/docker_cli_plugins_test.go index e49c428490..8ca7254440 100644 --- a/components/engine/integration-cli/docker_cli_plugins_test.go +++ b/components/engine/integration-cli/docker_cli_plugins_test.go @@ -15,7 +15,6 @@ import ( "github.com/docker/docker/integration-cli/cli" "github.com/docker/docker/integration-cli/daemon" "github.com/docker/docker/integration-cli/fixtures/plugin" - "github.com/docker/docker/integration-cli/request" "github.com/go-check/check" "github.com/gotestyourself/gotestyourself/icmd" "golang.org/x/net/context" @@ -159,9 +158,7 @@ func (s *DockerSuite) TestPluginInstallDisableVolumeLs(c *check.C) { } func (ps *DockerPluginSuite) TestPluginSet(c *check.C) { - // Create a new plugin with extra settings - client, err := request.NewClient() - c.Assert(err, checker.IsNil, check.Commentf("failed to create test client")) + client := testEnv.APIClient() name := "test" ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) @@ -171,7 +168,8 @@ func (ps *DockerPluginSuite) TestPluginSet(c *check.C) { mntSrc := "foo" devPath := "/dev/bar" - err = plugin.Create(ctx, client, name, func(cfg *plugin.Config) { + // Create a new plugin with extra settings + err := plugin.Create(ctx, client, name, func(cfg *plugin.Config) { cfg.Env = []types.PluginEnv{{Name: "DEBUG", Value: &initialValue, Settable: []string{"value"}}} cfg.Mounts = []types.PluginMount{ {Name: "pmount1", Settable: []string{"source"}, Type: "none", Source: &mntSrc}, @@ -401,12 +399,11 @@ func (s *DockerTrustSuite) TestPluginUntrustedInstall(c *check.C) { func (ps *DockerPluginSuite) TestPluginIDPrefix(c *check.C) { name := "test" - client, err := request.NewClient() - c.Assert(err, checker.IsNil, check.Commentf("error creating test client")) + client := testEnv.APIClient() ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) initialValue := "0" - err = plugin.Create(ctx, client, name, func(cfg *plugin.Config) { + err := plugin.Create(ctx, client, name, func(cfg *plugin.Config) { cfg.Env = []types.PluginEnv{{Name: "DEBUG", Value: &initialValue, Settable: []string{"value"}}} }) cancel() @@ -466,8 +463,7 @@ func (ps *DockerPluginSuite) TestPluginListDefaultFormat(c *check.C) { c.Assert(err, check.IsNil) name := "test:latest" - client, err := request.NewClient() - c.Assert(err, checker.IsNil, check.Commentf("error creating test client")) + client := testEnv.APIClient() ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) defer cancel() diff --git a/components/engine/integration-cli/request/request.go b/components/engine/integration-cli/request/request.go index e4a1e3e892..4e61e09359 100644 --- a/components/engine/integration-cli/request/request.go +++ b/components/engine/integration-cli/request/request.go @@ -1,16 +1,13 @@ package request // import "github.com/docker/docker/integration-cli/request" import ( - "bufio" "bytes" "crypto/tls" "encoding/json" - "fmt" "io" "io/ioutil" "net" "net/http" - "net/http/httputil" "net/url" "os" "path/filepath" @@ -95,11 +92,11 @@ func Do(endpoint string, modifiers ...func(*http.Request) error) (*http.Response // DoOnHost creates and execute a request on the specified host and endpoint, with the specified request modifiers func DoOnHost(host, endpoint string, modifiers ...func(*http.Request) error) (*http.Response, io.ReadCloser, error) { - req, err := New(host, endpoint, modifiers...) + req, err := newRequest(host, endpoint, modifiers...) if err != nil { return nil, nil, err } - client, err := NewHTTPClient(host) + client, err := newHTTPClient(host) if err != nil { return nil, nil, err } @@ -114,18 +111,15 @@ func DoOnHost(host, endpoint string, modifiers ...func(*http.Request) error) (*h return resp, body, err } -// New creates a new http Request to the specified host and endpoint, with the specified request modifiers -func New(host, endpoint string, modifiers ...func(*http.Request) error) (*http.Request, error) { - _, addr, _, err := dclient.ParseHost(host) +// newRequest creates a new http Request to the specified host and endpoint, with the specified request modifiers +func newRequest(host, endpoint string, modifiers ...func(*http.Request) error) (*http.Request, error) { + hostUrl, err := dclient.ParseHostURL(host) if err != nil { - return nil, err - } - if err != nil { - return nil, errors.Wrapf(err, "could not parse url %q", host) + return nil, errors.Wrapf(err, "failed parsing url %q", host) } req, err := http.NewRequest("GET", endpoint, nil) if err != nil { - return nil, fmt.Errorf("could not create new request: %v", err) + return nil, errors.Wrap(err, "failed to create request") } if os.Getenv("DOCKER_TLS_VERIFY") != "" { @@ -133,7 +127,7 @@ func New(host, endpoint string, modifiers ...func(*http.Request) error) (*http.R } else { req.URL.Scheme = "http" } - req.URL.Host = addr + req.URL.Host = hostUrl.Host for _, config := range modifiers { if err := config(req); err != nil { @@ -143,15 +137,16 @@ func New(host, endpoint string, modifiers ...func(*http.Request) error) (*http.R return req, nil } -// NewHTTPClient creates an http client for the specific host -func NewHTTPClient(host string) (*http.Client, error) { +// newHTTPClient creates an http client for the specific host +// TODO: Share more code with client.defaultHTTPClient +func newHTTPClient(host string) (*http.Client, error) { // FIXME(vdemeester) 10*time.Second timeout of SockRequest… ? - proto, addr, _, err := dclient.ParseHost(host) + hostUrl, err := dclient.ParseHostURL(host) if err != nil { return nil, err } transport := new(http.Transport) - if proto == "tcp" && os.Getenv("DOCKER_TLS_VERIFY") != "" { + if hostUrl.Scheme == "tcp" && os.Getenv("DOCKER_TLS_VERIFY") != "" { // Setup the socket TLS configuration. tlsConfig, err := getTLSConfig() if err != nil { @@ -160,102 +155,22 @@ func NewHTTPClient(host string) (*http.Client, error) { transport = &http.Transport{TLSClientConfig: tlsConfig} } transport.DisableKeepAlives = true - err = sockets.ConfigureTransport(transport, proto, addr) - return &http.Client{ - Transport: transport, - }, err + err = sockets.ConfigureTransport(transport, hostUrl.Scheme, hostUrl.Host) + return &http.Client{Transport: transport}, err } // NewClient returns a new Docker API client +// Deprecated: Use Execution.APIClient() func NewClient() (dclient.APIClient, error) { return dclient.NewClientWithOpts(dclient.WithHost(DaemonHost())) } -// FIXME(vdemeester) httputil.ClientConn is deprecated, use http.Client instead (closer to actual client) -// Deprecated: Use New instead of NewRequestClient -// Deprecated: use request.Do (or Get, Delete, Post) instead -func newRequestClient(method, endpoint string, data io.Reader, ct, daemon string, modifiers ...func(*http.Request)) (*http.Request, *httputil.ClientConn, error) { - c, err := SockConn(time.Duration(10*time.Second), daemon) - if err != nil { - return nil, nil, fmt.Errorf("could not dial docker daemon: %v", err) - } - - client := httputil.NewClientConn(c, nil) - - req, err := http.NewRequest(method, endpoint, data) - if err != nil { - client.Close() - return nil, nil, fmt.Errorf("could not create new request: %v", err) - } - - for _, opt := range modifiers { - opt(req) - } - - if ct != "" { - req.Header.Set("Content-Type", ct) - } - return req, client, nil -} - -// SockRequest create a request against the specified host (with method, endpoint and other request modifier) and -// returns the status code, and the content as an byte slice -// Deprecated: use request.Do instead -func SockRequest(method, endpoint string, data interface{}, daemon string, modifiers ...func(*http.Request)) (int, []byte, error) { - jsonData := bytes.NewBuffer(nil) - if err := json.NewEncoder(jsonData).Encode(data); err != nil { - return -1, nil, err - } - - res, body, err := SockRequestRaw(method, endpoint, jsonData, "application/json", daemon, modifiers...) - if err != nil { - return -1, nil, err - } - b, err := ReadBody(body) - return res.StatusCode, b, err -} - // ReadBody read the specified ReadCloser content and returns it func ReadBody(b io.ReadCloser) ([]byte, error) { defer b.Close() return ioutil.ReadAll(b) } -// SockRequestRaw create a request against the specified host (with method, endpoint and other request modifier) and -// returns the http response, the output as a io.ReadCloser -// Deprecated: use request.Do (or Get, Delete, Post) instead -func SockRequestRaw(method, endpoint string, data io.Reader, ct, daemon string, modifiers ...func(*http.Request)) (*http.Response, io.ReadCloser, error) { - req, client, err := newRequestClient(method, endpoint, data, ct, daemon, modifiers...) - if err != nil { - return nil, nil, err - } - - resp, err := client.Do(req) - if err != nil { - client.Close() - return resp, nil, err - } - body := ioutils.NewReadCloserWrapper(resp.Body, func() error { - defer resp.Body.Close() - return client.Close() - }) - - return resp, body, err -} - -// SockRequestHijack creates a connection to specified host (with method, contenttype, …) and returns a hijacked connection -// and the output as a `bufio.Reader` -func SockRequestHijack(method, endpoint string, data io.Reader, ct string, daemon string, modifiers ...func(*http.Request)) (net.Conn, *bufio.Reader, error) { - req, client, err := newRequestClient(method, endpoint, data, ct, daemon, modifiers...) - if err != nil { - return nil, nil, err - } - - client.Do(req) - conn, br := client.Hijack() - return conn, br, nil -} - // SockConn opens a connection on the specified socket func SockConn(timeout time.Duration, daemon string) (net.Conn, error) { daemonURL, err := url.Parse(daemon) diff --git a/components/engine/integration-cli/trust_server_test.go b/components/engine/integration-cli/trust_server_test.go index 19abe87196..f312083ee3 100644 --- a/components/engine/integration-cli/trust_server_test.go +++ b/components/engine/integration-cli/trust_server_test.go @@ -17,7 +17,6 @@ import ( "github.com/docker/docker/integration-cli/checker" "github.com/docker/docker/integration-cli/cli" "github.com/docker/docker/integration-cli/fixtures/plugin" - "github.com/docker/docker/integration-cli/request" "github.com/docker/go-connections/tlsconfig" "github.com/go-check/check" "github.com/gotestyourself/gotestyourself/icmd" @@ -230,11 +229,10 @@ func (s *DockerTrustSuite) setupTrustedImage(c *check.C, name string) string { func (s *DockerTrustSuite) setupTrustedplugin(c *check.C, source, name string) string { repoName := fmt.Sprintf("%v/dockercli/%s:latest", privateRegistryURL, name) - client, err := request.NewClient() - c.Assert(err, checker.IsNil, check.Commentf("could not create test client")) + client := testEnv.APIClient() ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) - err = plugin.Create(ctx, client, repoName) + err := plugin.Create(ctx, client, repoName) cancel() c.Assert(err, checker.IsNil, check.Commentf("could not create test plugin")) From 10bf273bff640955a23c9e8f33ddcb94cc3805ff Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 16 Feb 2018 17:29:19 -0500 Subject: [PATCH 22/22] Remove explicit DOCKER_API_VERSION from integration env setup Use the default version because it is used by the client package Signed-off-by: Daniel Nephin Upstream-commit: e73d742cd7deee396eac3c97664b40264ee358cb Component: engine --- components/engine/hack/make/.integration-daemon-start | 4 ---- 1 file changed, 4 deletions(-) diff --git a/components/engine/hack/make/.integration-daemon-start b/components/engine/hack/make/.integration-daemon-start index 7314f71c5c..20801fccee 100644 --- a/components/engine/hack/make/.integration-daemon-start +++ b/components/engine/hack/make/.integration-daemon-start @@ -7,10 +7,6 @@ export PATH="$base/binary-daemon:$base/dynbinary-daemon:$PATH" export TEST_CLIENT_BINARY=docker -# Do not bump this version! Integration tests should no longer rely on the docker cli, they should be -# API tests instead. For the existing tests the scripts will use a frozen version of the docker cli -# with a DOCKER_API_VERSION frozen to 1.30, which should ensure that the CI remains green at all times. -export DOCKER_API_VERSION=1.30 if [ -n "$DOCKER_CLI_PATH" ]; then export TEST_CLIENT_BINARY=/usr/local/cli/$(basename "$DOCKER_CLI_PATH") fi