From 27f91f80b9aeeabff4baeb0a08bf7e691799fd53 Mon Sep 17 00:00:00 2001 From: Nicolas De Loof Date: Fri, 8 Dec 2017 09:01:34 +0100 Subject: [PATCH 1/9] =?UTF-8?q?introduce=20=C2=AB=C2=A0exec=5Fdie=C2=A0?= =?UTF-8?q?=C2=BB=20event?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Nicolas De Loof Upstream-commit: aa6bb5cb698ebcf6662e6667bc58055b5b5bde23 Component: engine --- components/engine/api/swagger.yaml | 2 +- components/engine/daemon/exec.go | 15 +++- components/engine/daemon/health.go | 5 +- components/engine/daemon/monitor.go | 5 ++ components/engine/docs/api/version-history.md | 2 + .../engine/integration/system/event_test.go | 74 +++++++++++++++++++ 6 files changed, 98 insertions(+), 5 deletions(-) create mode 100644 components/engine/integration/system/event_test.go diff --git a/components/engine/api/swagger.yaml b/components/engine/api/swagger.yaml index 8c741182ca..32f75d735c 100644 --- a/components/engine/api/swagger.yaml +++ b/components/engine/api/swagger.yaml @@ -6925,7 +6925,7 @@ paths: Various objects within Docker report events when something happens to them. - Containers report these events: `attach`, `commit`, `copy`, `create`, `destroy`, `detach`, `die`, `exec_create`, `exec_detach`, `exec_start`, `export`, `health_status`, `kill`, `oom`, `pause`, `rename`, `resize`, `restart`, `start`, `stop`, `top`, `unpause`, and `update` + Containers report these events: `attach`, `commit`, `copy`, `create`, `destroy`, `detach`, `die`, `exec_create`, `exec_detach`, `exec_start`, `exec_die`, `export`, `health_status`, `kill`, `oom`, `pause`, `rename`, `resize`, `restart`, `start`, `stop`, `top`, `unpause`, and `update` Images report these events: `delete`, `import`, `load`, `pull`, `push`, `save`, `tag`, and `untag` diff --git a/components/engine/daemon/exec.go b/components/engine/daemon/exec.go index 83b7de2255..65e3204480 100644 --- a/components/engine/daemon/exec.go +++ b/components/engine/daemon/exec.go @@ -138,7 +138,10 @@ func (d *Daemon) ContainerExecCreate(name string, config *types.ExecConfig) (str d.registerExecCommand(cntr, execConfig) - d.LogContainerEvent(cntr, "exec_create: "+execConfig.Entrypoint+" "+strings.Join(execConfig.Args, " ")) + attributes := map[string]string{ + "execID": execConfig.ID, + } + d.LogContainerEventWithAttributes(cntr, "exec_create: "+execConfig.Entrypoint+" "+strings.Join(execConfig.Args, " "), attributes) return execConfig.ID, nil } @@ -173,7 +176,10 @@ func (d *Daemon) ContainerExecStart(ctx context.Context, name string, stdin io.R c := d.containers.Get(ec.ContainerID) logrus.Debugf("starting exec command %s in container %s", ec.ID, c.ID) - d.LogContainerEvent(c, "exec_start: "+ec.Entrypoint+" "+strings.Join(ec.Args, " ")) + attributes := map[string]string{ + "execID": ec.ID, + } + d.LogContainerEventWithAttributes(c, "exec_start: "+ec.Entrypoint+" "+strings.Join(ec.Args, " "), attributes) defer func() { if err != nil { @@ -269,7 +275,10 @@ func (d *Daemon) ContainerExecStart(ctx context.Context, name string, stdin io.R if _, ok := err.(term.EscapeError); !ok { return errors.Wrap(systemError{err}, "exec attach failed") } - d.LogContainerEvent(c, "exec_detach") + attributes := map[string]string{ + "execID": ec.ID, + } + d.LogContainerEventWithAttributes(c, "exec_detach", attributes) } } return nil diff --git a/components/engine/daemon/health.go b/components/engine/daemon/health.go index 9acf19043b..ff3d54d45d 100644 --- a/components/engine/daemon/health.go +++ b/components/engine/daemon/health.go @@ -89,7 +89,10 @@ func (p *cmdProbe) run(ctx context.Context, d *Daemon, cntr *container.Container execConfig.Env = container.ReplaceOrAppendEnvValues(cntr.CreateDaemonEnvironment(execConfig.Tty, linkedEnv), execConfig.Env) d.registerExecCommand(cntr, execConfig) - d.LogContainerEvent(cntr, "exec_create: "+execConfig.Entrypoint+" "+strings.Join(execConfig.Args, " ")) + attributes := map[string]string{ + "execID": execConfig.ID, + } + d.LogContainerEventWithAttributes(cntr, "exec_create: "+execConfig.Entrypoint+" "+strings.Join(execConfig.Args, " "), attributes) output := &limitedBuffer{} err = d.ContainerExecStart(ctx, execConfig.ID, nil, output, output) diff --git a/components/engine/daemon/monitor.go b/components/engine/daemon/monitor.go index dd60239105..6f6928a4d9 100644 --- a/components/engine/daemon/monitor.go +++ b/components/engine/daemon/monitor.go @@ -128,6 +128,11 @@ func (daemon *Daemon) ProcessEvent(id string, e libcontainerd.EventType, ei libc // remove the exec command from the container's store only and not the // daemon's store so that the exec command can be inspected. c.ExecCommands.Delete(execConfig.ID, execConfig.Pid) + attributes := map[string]string{ + "execID": execConfig.ID, + "exitCode": strconv.Itoa(ec), + } + daemon.LogContainerEventWithAttributes(c, "exec_die", attributes) } else { logrus.WithFields(logrus.Fields{ "container": c.ID, diff --git a/components/engine/docs/api/version-history.md b/components/engine/docs/api/version-history.md index 0fdf4648dc..4eca7fbb11 100644 --- a/components/engine/docs/api/version-history.md +++ b/components/engine/docs/api/version-history.md @@ -17,6 +17,8 @@ keywords: "API, Docker, rcli, REST, documentation" [Docker Engine API v1.36](https://docs.docker.com/engine/api/v1.36/) documentation +* `Get /events` now return `exec_die` event when an exec process terminates. + ## v1.35 API changes diff --git a/components/engine/integration/system/event_test.go b/components/engine/integration/system/event_test.go new file mode 100644 index 0000000000..ef7eb1ff5b --- /dev/null +++ b/components/engine/integration/system/event_test.go @@ -0,0 +1,74 @@ +package system + +import ( + "context" + "testing" + + "time" + + "github.com/docker/docker/api/types" + "github.com/docker/docker/api/types/container" + "github.com/docker/docker/api/types/filters" + "github.com/docker/docker/api/types/network" + "github.com/docker/docker/api/types/strslice" + "github.com/docker/docker/integration/util/request" + "github.com/stretchr/testify/require" +) + +func TestEvents(t *testing.T) { + defer setupTest(t)() + ctx := context.Background() + client := request.NewAPIClient(t) + + container, err := client.ContainerCreate(ctx, + &container.Config{ + Image: "busybox", + Tty: true, + WorkingDir: "/root", + Cmd: strslice.StrSlice([]string{"top"}), + }, + &container.HostConfig{}, + &network.NetworkingConfig{}, + "foo", + ) + require.NoError(t, err) + err = client.ContainerStart(ctx, container.ID, types.ContainerStartOptions{}) + require.NoError(t, err) + + id, err := client.ContainerExecCreate(ctx, container.ID, + types.ExecConfig{ + Cmd: strslice.StrSlice([]string{"echo", "hello"}), + }, + ) + require.NoError(t, err) + + filters := filters.NewArgs( + filters.Arg("container", container.ID), + filters.Arg("event", "exec_die"), + ) + msg, errors := client.Events(ctx, types.EventsOptions{ + Filters: filters, + }) + + err = client.ContainerExecStart(ctx, id.ID, + types.ExecStartCheck{ + Detach: true, + Tty: false, + }, + ) + require.NoError(t, err) + + select { + case m := <-msg: + require.Equal(t, m.Type, "container") + require.Equal(t, m.Actor.ID, container.ID) + require.Equal(t, m.Action, "exec_die") + require.Equal(t, m.Actor.Attributes["execID"], id.ID) + require.Equal(t, m.Actor.Attributes["exitCode"], "0") + case err = <-errors: + t.Fatal(err) + case <-time.After(time.Second * 3): + t.Fatal("timeout hit") + } + +} From a19065e95147f834204fc1492343e6c3a96a8c4b Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Mon, 18 Dec 2017 16:02:23 -0500 Subject: [PATCH 2/9] Make container resource mounts unbindable It's a common scenario for admins and/or monitoring applications to mount in the daemon root dir into a container. When doing so all mounts get coppied into the container, often with private references. This can prevent removal of a container due to the various mounts that must be configured before a container is started (for example, for shared /dev/shm, or secrets) being leaked into another namespace, usually with private references. This is particularly problematic on older kernels (e.g. RHEL < 7.4) where a mount may be active in another namespace and attempting to remove a mountpoint which is active in another namespace fails. This change moves all container resource mounts into a common directory so that the directory can be made unbindable. What this does is prevents sub-mounts of this new directory from leaking into other namespaces when mounted with `rbind`... which is how all binds are handled for containers. Signed-off-by: Brian Goff Upstream-commit: eaa5192856c1ad09614318e88030554b96bb6e81 Component: engine --- components/engine/container/container.go | 29 +++++-- components/engine/container/container_unix.go | 30 +++++-- .../engine/container/container_windows.go | 26 ++++-- .../daemon/container_operations_unix.go | 39 ++++++++- .../daemon/container_operations_windows.go | 20 ++++- components/engine/daemon/oci_linux.go | 16 +++- components/engine/daemon/oci_windows.go | 16 +++- components/engine/daemon/start.go | 5 ++ .../container/mounts_linux_test.go | 84 +++++++++++++++++++ 9 files changed, 226 insertions(+), 39 deletions(-) create mode 100644 components/engine/integration/container/mounts_linux_test.go diff --git a/components/engine/container/container.go b/components/engine/container/container.go index dd11a11543..f19f9893f4 100644 --- a/components/engine/container/container.go +++ b/components/engine/container/container.go @@ -1022,14 +1022,23 @@ func (container *Container) InitializeStdio(iop *cio.DirectIO) (cio.IO, error) { return &rio{IO: iop, sc: container.StreamConfig}, nil } +// MountsResourcePath returns the path where mounts are stored for the given mount +func (container *Container) MountsResourcePath(mount string) (string, error) { + return container.GetRootResourcePath(filepath.Join("mounts", mount)) +} + // SecretMountPath returns the path of the secret mount for the container -func (container *Container) SecretMountPath() string { - return filepath.Join(container.Root, "secrets") +func (container *Container) SecretMountPath() (string, error) { + return container.MountsResourcePath("secrets") } // SecretFilePath returns the path to the location of a secret on the host. -func (container *Container) SecretFilePath(secretRef swarmtypes.SecretReference) string { - return filepath.Join(container.SecretMountPath(), secretRef.SecretID) +func (container *Container) SecretFilePath(secretRef swarmtypes.SecretReference) (string, error) { + secrets, err := container.SecretMountPath() + if err != nil { + return "", err + } + return filepath.Join(secrets, secretRef.SecretID), nil } func getSecretTargetPath(r *swarmtypes.SecretReference) string { @@ -1042,13 +1051,17 @@ func getSecretTargetPath(r *swarmtypes.SecretReference) string { // ConfigsDirPath returns the path to the directory where configs are stored on // disk. -func (container *Container) ConfigsDirPath() string { - return filepath.Join(container.Root, "configs") +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 { - return filepath.Join(container.ConfigsDirPath(), configRef.ConfigID) +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 } // CreateDaemonEnvironment creates a new environment variable slice for this container. diff --git a/components/engine/container/container_unix.go b/components/engine/container/container_unix.go index f030232060..63ad2389c4 100644 --- a/components/engine/container/container_unix.go +++ b/components/engine/container/container_unix.go @@ -151,7 +151,7 @@ func (container *Container) CopyImagePathContent(v volume.Volume, destination st // ShmResourcePath returns path to shm func (container *Container) ShmResourcePath() (string, error) { - return container.GetRootResourcePath("shm") + return container.MountsResourcePath("shm") } // HasMountFor checks if path is a mountpoint @@ -218,49 +218,61 @@ func (container *Container) IpcMounts() []Mount { } // SecretMounts returns the mounts for the secret path. -func (container *Container) SecretMounts() []Mount { +func (container *Container) SecretMounts() ([]Mount, error) { var mounts []Mount for _, r := range container.SecretReferences { if r.File == nil { continue } + src, err := container.SecretFilePath(*r) + if err != nil { + return nil, err + } mounts = append(mounts, Mount{ - Source: container.SecretFilePath(*r), + Source: src, Destination: getSecretTargetPath(r), Writable: false, }) } - return mounts + return mounts, nil } // UnmountSecrets unmounts the local tmpfs for secrets func (container *Container) UnmountSecrets() error { - if _, err := os.Stat(container.SecretMountPath()); err != nil { + p, err := container.SecretMountPath() + if err != nil { + return err + } + if _, err := os.Stat(p); err != nil { if os.IsNotExist(err) { return nil } return err } - return detachMounted(container.SecretMountPath()) + return mount.RecursiveUnmount(p) } // ConfigMounts returns the mounts for configs. -func (container *Container) ConfigMounts() []Mount { +func (container *Container) ConfigMounts() ([]Mount, error) { var mounts []Mount for _, configRef := range container.ConfigReferences { if configRef.File == nil { continue } + src, err := container.ConfigFilePath(*configRef) + if err != nil { + return nil, err + } mounts = append(mounts, Mount{ - Source: container.ConfigFilePath(*configRef), + Source: src, Destination: configRef.File.Name, Writable: false, }) } - return mounts + return mounts, nil } type conflictingUpdateOptions string diff --git a/components/engine/container/container_windows.go b/components/engine/container/container_windows.go index 92b50a6eb6..94a4e1c0b0 100644 --- a/components/engine/container/container_windows.go +++ b/components/engine/container/container_windows.go @@ -54,22 +54,30 @@ func (container *Container) CreateSecretSymlinks() error { // SecretMounts returns the mount for the secret path. // All secrets are stored in a single mount on Windows. Target symlinks are // created for each secret, pointing to the files in this mount. -func (container *Container) SecretMounts() []Mount { +func (container *Container) SecretMounts() ([]Mount, error) { var mounts []Mount if len(container.SecretReferences) > 0 { + src, err := container.SecretMountPath() + if err != nil { + return nil, err + } mounts = append(mounts, Mount{ - Source: container.SecretMountPath(), + Source: src, Destination: containerInternalSecretMountPath, Writable: false, }) } - return mounts + return mounts, nil } // UnmountSecrets unmounts the fs for secrets func (container *Container) UnmountSecrets() error { - return os.RemoveAll(container.SecretMountPath()) + p, err := container.SecretMountPath() + if err != nil { + return err + } + return os.RemoveAll(p) } // CreateConfigSymlinks creates symlinks to files in the config mount. @@ -96,17 +104,21 @@ 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 { +func (container *Container) ConfigMounts() ([]Mount, error) { var mounts []Mount if len(container.ConfigReferences) > 0 { + src, err := container.ConfigsDirPath() + if err != nil { + return nil, err + } mounts = append(mounts, Mount{ - Source: container.ConfigsDirPath(), + Source: src, Destination: containerInternalConfigsDirPath, Writable: false, }) } - return mounts + return mounts, nil } // DetachAndUnmount unmounts all volumes. diff --git a/components/engine/daemon/container_operations_unix.go b/components/engine/daemon/container_operations_unix.go index 9fe899b623..a123df3a9a 100644 --- a/components/engine/daemon/container_operations_unix.go +++ b/components/engine/daemon/container_operations_unix.go @@ -165,7 +165,10 @@ func (daemon *Daemon) setupSecretDir(c *container.Container) (setupErr error) { return nil } - localMountPath := c.SecretMountPath() + 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 @@ -204,7 +207,10 @@ func (daemon *Daemon) setupSecretDir(c *container.Container) (setupErr error) { // secrets are created in the SecretMountPath on the host, at a // single level - fPath := c.SecretFilePath(*s) + fPath, err := c.SecretFilePath(*s) + if err != nil { + return errors.Wrap(err, "error getting secret file path") + } if err := idtools.MkdirAllAndChown(filepath.Dir(fPath), 0700, rootIDs); err != nil { return errors.Wrap(err, "error creating secret mount path") } @@ -250,7 +256,10 @@ func (daemon *Daemon) setupConfigDir(c *container.Container) (setupErr error) { return nil } - localPath := c.ConfigsDirPath() + localPath, err := c.ConfigsDirPath() + if err != nil { + return err + } logrus.Debugf("configs: setting up config dir: %s", localPath) // retrieve possible remapped range start for root UID, GID @@ -279,7 +288,10 @@ func (daemon *Daemon) setupConfigDir(c *container.Container) (setupErr error) { continue } - fPath := c.ConfigFilePath(*configRef) + fPath, err := c.ConfigFilePath(*configRef) + if err != nil { + return err + } log := logrus.WithFields(logrus.Fields{"name": configRef.File.Name, "path": fPath}) @@ -379,3 +391,22 @@ func (daemon *Daemon) initializeNetworkingPaths(container *container.Container, container.ResolvConfPath = nc.ResolvConfPath return nil } + +func (daemon *Daemon) setupContainerMountsRoot(c *container.Container) error { + // get the root mount path so we can make it unbindable + p, err := c.MountsResourcePath("") + if err != nil { + return err + } + + if err := idtools.MkdirAllAndChown(p, 0700, daemon.idMappings.RootPair()); err != nil { + return err + } + + if err := mount.MakeUnbindable(p); err != nil { + // Setting unbindable is a precaution and is not neccessary for correct operation. + // Do not error out if this fails. + logrus.WithError(err).WithField("resource", p).WithField("container", c.ID).Warn("Error setting container resource mounts to unbindable, this may cause mount leakages, preventing removal of this container.") + } + return nil +} diff --git a/components/engine/daemon/container_operations_windows.go b/components/engine/daemon/container_operations_windows.go index 51762a2441..c762fc99cb 100644 --- a/components/engine/daemon/container_operations_windows.go +++ b/components/engine/daemon/container_operations_windows.go @@ -21,7 +21,10 @@ func (daemon *Daemon) setupConfigDir(c *container.Container) (setupErr error) { return nil } - localPath := c.ConfigsDirPath() + localPath, err := c.ConfigsDirPath() + if err != nil { + return err + } logrus.Debugf("configs: setting up config dir: %s", localPath) // create local config root @@ -48,7 +51,10 @@ func (daemon *Daemon) setupConfigDir(c *container.Container) (setupErr error) { continue } - fPath := c.ConfigFilePath(*configRef) + fPath, err := c.ConfigFilePath(*configRef) + if err != nil { + return err + } log := logrus.WithFields(logrus.Fields{"name": configRef.File.Name, "path": fPath}) @@ -94,7 +100,10 @@ func (daemon *Daemon) setupSecretDir(c *container.Container) (setupErr error) { return nil } - localMountPath := c.SecretMountPath() + localMountPath, err := c.SecretMountPath() + if err != nil { + return err + } logrus.Debugf("secrets: setting up secret dir: %s", localMountPath) // create local secret root @@ -123,7 +132,10 @@ func (daemon *Daemon) setupSecretDir(c *container.Container) (setupErr error) { // secrets are created in the SecretMountPath on the host, at a // single level - fPath := c.SecretFilePath(*s) + fPath, err := c.SecretFilePath(*s) + if err != nil { + return err + } logrus.WithFields(logrus.Fields{ "name": s.File.Name, "path": fPath, diff --git a/components/engine/daemon/oci_linux.go b/components/engine/daemon/oci_linux.go index 757de85366..53dc276565 100644 --- a/components/engine/daemon/oci_linux.go +++ b/components/engine/daemon/oci_linux.go @@ -812,6 +812,10 @@ func (daemon *Daemon) createSpec(c *container.Container) (*specs.Spec, error) { return nil, fmt.Errorf("linux seccomp: %v", err) } + if err := daemon.setupContainerMountsRoot(c); err != nil { + return nil, err + } + if err := daemon.setupIpcDirs(c); err != nil { return nil, err } @@ -839,11 +843,17 @@ func (daemon *Daemon) createSpec(c *container.Container) (*specs.Spec, error) { } ms = append(ms, tmpfsMounts...) - if m := c.SecretMounts(); m != nil { - ms = append(ms, m...) + secretMounts, err := c.SecretMounts() + if err != nil { + return nil, err } + ms = append(ms, secretMounts...) - ms = append(ms, c.ConfigMounts()...) + 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 { diff --git a/components/engine/daemon/oci_windows.go b/components/engine/daemon/oci_windows.go index c7c94f327a..9f135b87af 100644 --- a/components/engine/daemon/oci_windows.go +++ b/components/engine/daemon/oci_windows.go @@ -93,12 +93,20 @@ func (daemon *Daemon) createSpec(c *container.Container) (*specs.Spec, error) { } } - if m := c.SecretMounts(); m != nil { - mounts = append(mounts, m...) + secretMounts, err := c.SecretMounts() + if err != nil { + return nil, err + } + if secretMounts != nil { + mounts = append(mounts, secretMounts...) } - if m := c.ConfigMounts(); m != nil { - mounts = append(mounts, m...) + configMounts, err := c.ConfigMounts() + if err != nil { + return nil, err + } + if configMounts != nil { + mounts = append(mounts, configMounts...) } for _, mount := range mounts { diff --git a/components/engine/daemon/start.go b/components/engine/daemon/start.go index 008fb050d4..af9bfda014 100644 --- a/components/engine/daemon/start.go +++ b/components/engine/daemon/start.go @@ -9,6 +9,7 @@ import ( containertypes "github.com/docker/docker/api/types/container" "github.com/docker/docker/container" "github.com/docker/docker/errdefs" + "github.com/docker/docker/pkg/mount" "github.com/pkg/errors" "github.com/sirupsen/logrus" ) @@ -231,6 +232,10 @@ func (daemon *Daemon) Cleanup(container *container.Container) { logrus.Warnf("%s cleanup: failed to unmount secrets: %s", container.ID, err) } + if err := mount.RecursiveUnmount(container.Root); err != nil { + logrus.WithError(err).WithField("container", container.ID).Warn("Error while cleaning up container resource mounts.") + } + for _, eConfig := range container.ExecCommands.Commands() { daemon.unregisterExecCommand(container, eConfig) } diff --git a/components/engine/integration/container/mounts_linux_test.go b/components/engine/integration/container/mounts_linux_test.go new file mode 100644 index 0000000000..8c13258c30 --- /dev/null +++ b/components/engine/integration/container/mounts_linux_test.go @@ -0,0 +1,84 @@ +package container + +import ( + "bytes" + "context" + "fmt" + "testing" + + "github.com/docker/docker/api/types" + "github.com/docker/docker/api/types/container" + "github.com/docker/docker/api/types/mount" + "github.com/docker/docker/integration-cli/daemon" + "github.com/docker/docker/pkg/stdcopy" +) + +func TestContainerShmNoLeak(t *testing.T) { + t.Parallel() + d := daemon.New(t, "docker", "dockerd", daemon.Config{}) + client, err := d.NewClient() + if err != nil { + t.Fatal(err) + } + d.StartWithBusybox(t) + defer d.Stop(t) + + ctx := context.Background() + cfg := container.Config{ + Image: "busybox", + Cmd: []string{"top"}, + } + + ctr, err := client.ContainerCreate(ctx, &cfg, nil, nil, "") + if err != nil { + t.Fatal(err) + } + defer client.ContainerRemove(ctx, ctr.ID, types.ContainerRemoveOptions{Force: true}) + + if err := client.ContainerStart(ctx, ctr.ID, types.ContainerStartOptions{}); err != nil { + t.Fatal(err) + } + + // this should recursively bind mount everything in the test daemons root + // except of course we are hoping that the previous containers /dev/shm mount did not leak into this new container + hc := container.HostConfig{ + Mounts: []mount.Mount{ + { + Type: mount.TypeBind, + Source: d.Root, + Target: "/testdaemonroot", + BindOptions: &mount.BindOptions{Propagation: mount.PropagationRPrivate}}, + }, + } + cfg.Cmd = []string{"/bin/sh", "-c", fmt.Sprintf("mount | grep testdaemonroot | grep containers | grep %s", ctr.ID)} + cfg.AttachStdout = true + cfg.AttachStderr = true + ctrLeak, err := client.ContainerCreate(ctx, &cfg, &hc, nil, "") + if err != nil { + t.Fatal(err) + } + + attach, err := client.ContainerAttach(ctx, ctrLeak.ID, types.ContainerAttachOptions{ + Stream: true, + Stdout: true, + Stderr: true, + }) + if err != nil { + t.Fatal(err) + } + + if err := client.ContainerStart(ctx, ctrLeak.ID, types.ContainerStartOptions{}); err != nil { + t.Fatal(err) + } + + buf := bytes.NewBuffer(nil) + + if _, err := stdcopy.StdCopy(buf, buf, attach.Reader); err != nil { + t.Fatal(err) + } + + out := bytes.TrimSpace(buf.Bytes()) + if !bytes.Equal(out, []byte{}) { + t.Fatalf("mount leaked: %s", string(out)) + } +} From 0986b8a32cc6b2bd58db02176d6037e055d23619 Mon Sep 17 00:00:00 2001 From: Pradip Dhara Date: Fri, 12 Jan 2018 09:27:30 +0000 Subject: [PATCH 3/9] Fixing ingress network when upgrading from 17.09 to 17.12. Signed-off-by: Pradip Dhara Signed-off-by: Pradip Dhara Upstream-commit: 2d7a50e5855ad0571e76d29cd1ab9f8f3a48433b Component: engine --- .../engine/daemon/cluster/executor/container/executor.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/components/engine/daemon/cluster/executor/container/executor.go b/components/engine/daemon/cluster/executor/container/executor.go index 14e41fa837..b032163f95 100644 --- a/components/engine/daemon/cluster/executor/container/executor.go +++ b/components/engine/daemon/cluster/executor/container/executor.go @@ -146,6 +146,11 @@ func (e *executor) Configure(ctx context.Context, node *api.Node) error { attachments[na.Network.ID] = na.Addresses[0] } + if (ingressNA == nil) && (node.Attachment != nil) { + ingressNA = node.Attachment + attachments[ingressNA.Network.ID] = ingressNA.Addresses[0] + } + if ingressNA == nil { e.backend.ReleaseIngress() return e.backend.GetAttachmentStore().ResetAttachments(attachments) From c6678dac698e3eed3f79af1522ec87802bc2414a Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 11 Jan 2018 22:00:41 +0100 Subject: [PATCH 4/9] Bump containerd to 1.0.1 (9b55aab90508bd389d7654c4baf173a981477d55) Signed-off-by: Sebastiaan van Stijn Upstream-commit: 9047f66b1edd4dffcafc34f9c7f3390ddd65d10b Component: engine --- components/engine/hack/dockerfile/binaries-commits | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/engine/hack/dockerfile/binaries-commits b/components/engine/hack/dockerfile/binaries-commits index eddec4034e..1037f55431 100644 --- a/components/engine/hack/dockerfile/binaries-commits +++ b/components/engine/hack/dockerfile/binaries-commits @@ -8,7 +8,7 @@ RUNC_COMMIT=b2567b37d7b75eb4cf325b77297b140ea686ce8f # containerd is also pinned in vendor.conf. When updating the binary # version you may also need to update the vendor version to pick up bug # fixes or new APIs. -CONTAINERD_COMMIT=89623f28b87a6004d4b785663257362d1658a729 # v1.0.0 +CONTAINERD_COMMIT=9b55aab90508bd389d7654c4baf173a981477d55 # v1.0.1 TINI_COMMIT=949e6facb77383876aeff8a6944dde66b3089574 LIBNETWORK_COMMIT=7b2b1feb1de4817d522cc372af149ff48d25028e VNDR_COMMIT=a6e196d8b4b0cbbdc29aebdb20c59ac6926bb384 From 8d6e00613bdf98ce02c90eeb495250bdb8929725 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 11 Jan 2018 22:02:00 +0100 Subject: [PATCH 5/9] Bump runc to 7f24b40cc5423969b4554ef04ba0b00e2b4ba010 matching the version that's used by containerd 1.0.1 Signed-off-by: Sebastiaan van Stijn Upstream-commit: f58aa31075bf74ab8d2369dafb591ae43ed36ee6 Component: engine --- .../engine/hack/dockerfile/binaries-commits | 2 +- components/engine/vendor.conf | 2 +- .../github.com/opencontainers/runc/README.md | 2 +- .../runc/libcontainer/apparmor/apparmor.go | 37 ++++++++++++------ .../configs/cgroup_unsupported.go | 6 --- .../libcontainer/configs/device_defaults.go | 2 +- .../devices/{devices_linux.go => devices.go} | 0 .../devices/devices_unsupported.go | 3 -- .../runc/libcontainer/system/sysconfig.go | 2 +- .../libcontainer/user/lookup_unsupported.go | 38 ------------------- 10 files changed, 31 insertions(+), 63 deletions(-) delete mode 100644 components/engine/vendor/github.com/opencontainers/runc/libcontainer/configs/cgroup_unsupported.go rename components/engine/vendor/github.com/opencontainers/runc/libcontainer/devices/{devices_linux.go => devices.go} (100%) delete mode 100644 components/engine/vendor/github.com/opencontainers/runc/libcontainer/devices/devices_unsupported.go delete mode 100644 components/engine/vendor/github.com/opencontainers/runc/libcontainer/user/lookup_unsupported.go diff --git a/components/engine/hack/dockerfile/binaries-commits b/components/engine/hack/dockerfile/binaries-commits index 1037f55431..b2f55b09c0 100644 --- a/components/engine/hack/dockerfile/binaries-commits +++ b/components/engine/hack/dockerfile/binaries-commits @@ -3,7 +3,7 @@ TOMLV_COMMIT=9baf8a8a9f2ed20a8e54160840c492f937eeaf9a # When updating RUNC_COMMIT, also update runc in vendor.conf accordingly -RUNC_COMMIT=b2567b37d7b75eb4cf325b77297b140ea686ce8f +RUNC_COMMIT=7f24b40cc5423969b4554ef04ba0b00e2b4ba010 # containerd is also pinned in vendor.conf. When updating the binary # version you may also need to update the vendor version to pick up bug diff --git a/components/engine/vendor.conf b/components/engine/vendor.conf index f52021dc99..f7c020f18f 100644 --- a/components/engine/vendor.conf +++ b/components/engine/vendor.conf @@ -66,7 +66,7 @@ github.com/pborman/uuid v1.0 google.golang.org/grpc v1.3.0 # When updating, also update RUNC_COMMIT in hack/dockerfile/binaries-commits accordingly -github.com/opencontainers/runc b2567b37d7b75eb4cf325b77297b140ea686ce8f +github.com/opencontainers/runc 7f24b40cc5423969b4554ef04ba0b00e2b4ba010 github.com/opencontainers/runtime-spec v1.0.1 github.com/opencontainers/image-spec v1.0.1 github.com/seccomp/libseccomp-golang 32f571b70023028bd57d9288c20efbcb237f3ce0 diff --git a/components/engine/vendor/github.com/opencontainers/runc/README.md b/components/engine/vendor/github.com/opencontainers/runc/README.md index eabfb982bf..3ca7a1a227 100644 --- a/components/engine/vendor/github.com/opencontainers/runc/README.md +++ b/components/engine/vendor/github.com/opencontainers/runc/README.md @@ -56,7 +56,7 @@ make BUILDTAGS='seccomp apparmor' |-----------|------------------------------------|-------------| | seccomp | Syscall filtering | libseccomp | | selinux | selinux process and mount labeling | | -| apparmor | apparmor profile support | libapparmor | +| apparmor | apparmor profile support | | | ambient | ambient capability support | kernel 4.3 | diff --git a/components/engine/vendor/github.com/opencontainers/runc/libcontainer/apparmor/apparmor.go b/components/engine/vendor/github.com/opencontainers/runc/libcontainer/apparmor/apparmor.go index 82ed1a68a6..7fff0627fa 100644 --- a/components/engine/vendor/github.com/opencontainers/runc/libcontainer/apparmor/apparmor.go +++ b/components/engine/vendor/github.com/opencontainers/runc/libcontainer/apparmor/apparmor.go @@ -2,15 +2,10 @@ package apparmor -// #cgo LDFLAGS: -lapparmor -// #include -// #include -import "C" import ( "fmt" "io/ioutil" "os" - "unsafe" ) // IsEnabled returns true if apparmor is enabled for the host. @@ -24,16 +19,36 @@ func IsEnabled() bool { return false } +func setprocattr(attr, value string) error { + // Under AppArmor you can only change your own attr, so use /proc/self/ + // instead of /proc// like libapparmor does + path := fmt.Sprintf("/proc/self/attr/%s", attr) + + f, err := os.OpenFile(path, os.O_WRONLY, 0) + if err != nil { + return err + } + defer f.Close() + + _, err = fmt.Fprintf(f, "%s", value) + return err +} + +// changeOnExec reimplements aa_change_onexec from libapparmor in Go +func changeOnExec(name string) error { + value := "exec " + name + if err := setprocattr("exec", value); err != nil { + return fmt.Errorf("apparmor failed to apply profile: %s", err) + } + return nil +} + // ApplyProfile will apply the profile with the specified name to the process after // the next exec. func ApplyProfile(name string) error { if name == "" { return nil } - cName := C.CString(name) - defer C.free(unsafe.Pointer(cName)) - if _, err := C.aa_change_onexec(cName); err != nil { - return fmt.Errorf("apparmor failed to apply profile: %s", err) - } - return nil + + return changeOnExec(name) } diff --git a/components/engine/vendor/github.com/opencontainers/runc/libcontainer/configs/cgroup_unsupported.go b/components/engine/vendor/github.com/opencontainers/runc/libcontainer/configs/cgroup_unsupported.go deleted file mode 100644 index 95e2830a43..0000000000 --- a/components/engine/vendor/github.com/opencontainers/runc/libcontainer/configs/cgroup_unsupported.go +++ /dev/null @@ -1,6 +0,0 @@ -// +build !windows,!linux,!freebsd - -package configs - -type Cgroup struct { -} diff --git a/components/engine/vendor/github.com/opencontainers/runc/libcontainer/configs/device_defaults.go b/components/engine/vendor/github.com/opencontainers/runc/libcontainer/configs/device_defaults.go index 4d348d217e..e4f423c523 100644 --- a/components/engine/vendor/github.com/opencontainers/runc/libcontainer/configs/device_defaults.go +++ b/components/engine/vendor/github.com/opencontainers/runc/libcontainer/configs/device_defaults.go @@ -1,4 +1,4 @@ -// +build linux freebsd +// +build linux package configs diff --git a/components/engine/vendor/github.com/opencontainers/runc/libcontainer/devices/devices_linux.go b/components/engine/vendor/github.com/opencontainers/runc/libcontainer/devices/devices.go similarity index 100% rename from components/engine/vendor/github.com/opencontainers/runc/libcontainer/devices/devices_linux.go rename to components/engine/vendor/github.com/opencontainers/runc/libcontainer/devices/devices.go diff --git a/components/engine/vendor/github.com/opencontainers/runc/libcontainer/devices/devices_unsupported.go b/components/engine/vendor/github.com/opencontainers/runc/libcontainer/devices/devices_unsupported.go deleted file mode 100644 index 6649b9f2dc..0000000000 --- a/components/engine/vendor/github.com/opencontainers/runc/libcontainer/devices/devices_unsupported.go +++ /dev/null @@ -1,3 +0,0 @@ -// +build !linux - -package devices diff --git a/components/engine/vendor/github.com/opencontainers/runc/libcontainer/system/sysconfig.go b/components/engine/vendor/github.com/opencontainers/runc/libcontainer/system/sysconfig.go index b3a07cba3e..b8434f1050 100644 --- a/components/engine/vendor/github.com/opencontainers/runc/libcontainer/system/sysconfig.go +++ b/components/engine/vendor/github.com/opencontainers/runc/libcontainer/system/sysconfig.go @@ -1,4 +1,4 @@ -// +build cgo,linux cgo,freebsd +// +build cgo,linux package system diff --git a/components/engine/vendor/github.com/opencontainers/runc/libcontainer/user/lookup_unsupported.go b/components/engine/vendor/github.com/opencontainers/runc/libcontainer/user/lookup_unsupported.go deleted file mode 100644 index 4a8d00acbd..0000000000 --- a/components/engine/vendor/github.com/opencontainers/runc/libcontainer/user/lookup_unsupported.go +++ /dev/null @@ -1,38 +0,0 @@ -// +build !darwin,!dragonfly,!freebsd,!linux,!netbsd,!openbsd,!solaris - -package user - -import ( - "io" - "syscall" -) - -func GetPasswdPath() (string, error) { - return "", ErrUnsupported -} - -func GetPasswd() (io.ReadCloser, error) { - return nil, ErrUnsupported -} - -func GetGroupPath() (string, error) { - return "", ErrUnsupported -} - -func GetGroup() (io.ReadCloser, error) { - return nil, ErrUnsupported -} - -// CurrentUser looks up the current user by their user id in /etc/passwd. If the -// user cannot be found (or there is no /etc/passwd file on the filesystem), -// then CurrentUser returns an error. -func CurrentUser() (User, error) { - return LookupUid(syscall.Getuid()) -} - -// CurrentGroup looks up the current user's group by their primary group id's -// entry in /etc/passwd. If the group cannot be found (or there is no -// /etc/group file on the filesystem), then CurrentGroup returns an error. -func CurrentGroup() (Group, error) { - return LookupGid(syscall.Getgid()) -} From 2096535fce82226266bb5ce289e00d95ad672b84 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 13 Jan 2018 02:57:50 +0100 Subject: [PATCH 6/9] Fixes for libcontainer changes Libcontainer no longer provides placeholders for unsupported platforms, which cause the Windows builds to fail. This patch moves features that are not supported to platform-specific files. Signed-off-by: Sebastiaan van Stijn Upstream-commit: d1c34831e930c1f6b3de28cab3f4a358845a79d5 Component: engine --- .../engine/builder/dockerfile/internals.go | 79 ---------- .../builder/dockerfile/internals_linux.go | 88 +++++++++++ .../dockerfile/internals_linux_test.go | 138 ++++++++++++++++++ .../builder/dockerfile/internals_test.go | 130 ----------------- .../builder/dockerfile/internals_windows.go | 7 + .../integration-cli/docker_cli_run_test.go | 5 +- 6 files changed, 235 insertions(+), 212 deletions(-) create mode 100644 components/engine/builder/dockerfile/internals_linux.go create mode 100644 components/engine/builder/dockerfile/internals_linux_test.go create mode 100644 components/engine/builder/dockerfile/internals_windows.go diff --git a/components/engine/builder/dockerfile/internals.go b/components/engine/builder/dockerfile/internals.go index adce7b9709..01d8055394 100644 --- a/components/engine/builder/dockerfile/internals.go +++ b/components/engine/builder/dockerfile/internals.go @@ -12,7 +12,6 @@ import ( "path" "path/filepath" "runtime" - "strconv" "strings" "github.com/docker/docker/api/types" @@ -24,10 +23,8 @@ import ( "github.com/docker/docker/pkg/containerfs" "github.com/docker/docker/pkg/idtools" "github.com/docker/docker/pkg/stringid" - "github.com/docker/docker/pkg/symlink" "github.com/docker/docker/pkg/system" "github.com/docker/go-connections/nat" - lcUser "github.com/opencontainers/runc/libcontainer/user" "github.com/pkg/errors" ) @@ -217,82 +214,6 @@ func (b *Builder) performCopy(state *dispatchState, inst copyInstruction) error return b.exportImage(state, imageMount, runConfigWithCommentCmd) } -func parseChownFlag(chown, ctrRootPath string, idMappings *idtools.IDMappings) (idtools.IDPair, error) { - var userStr, grpStr string - parts := strings.Split(chown, ":") - if len(parts) > 2 { - return idtools.IDPair{}, errors.New("invalid chown string format: " + chown) - } - if len(parts) == 1 { - // if no group specified, use the user spec as group as well - userStr, grpStr = parts[0], parts[0] - } else { - userStr, grpStr = parts[0], parts[1] - } - - passwdPath, err := symlink.FollowSymlinkInScope(filepath.Join(ctrRootPath, "etc", "passwd"), ctrRootPath) - if err != nil { - return idtools.IDPair{}, errors.Wrapf(err, "can't resolve /etc/passwd path in container rootfs") - } - groupPath, err := symlink.FollowSymlinkInScope(filepath.Join(ctrRootPath, "etc", "group"), ctrRootPath) - if err != nil { - return idtools.IDPair{}, errors.Wrapf(err, "can't resolve /etc/group path in container rootfs") - } - uid, err := lookupUser(userStr, passwdPath) - if err != nil { - return idtools.IDPair{}, errors.Wrapf(err, "can't find uid for user "+userStr) - } - gid, err := lookupGroup(grpStr, groupPath) - if err != nil { - return idtools.IDPair{}, errors.Wrapf(err, "can't find gid for group "+grpStr) - } - - // convert as necessary because of user namespaces - chownPair, err := idMappings.ToHost(idtools.IDPair{UID: uid, GID: gid}) - if err != nil { - return idtools.IDPair{}, errors.Wrapf(err, "unable to convert uid/gid to host mapping") - } - return chownPair, nil -} - -func lookupUser(userStr, filepath string) (int, error) { - // if the string is actually a uid integer, parse to int and return - // as we don't need to translate with the help of files - uid, err := strconv.Atoi(userStr) - if err == nil { - return uid, nil - } - users, err := lcUser.ParsePasswdFileFilter(filepath, func(u lcUser.User) bool { - return u.Name == userStr - }) - if err != nil { - return 0, err - } - if len(users) == 0 { - return 0, errors.New("no such user: " + userStr) - } - return users[0].Uid, nil -} - -func lookupGroup(groupStr, filepath string) (int, error) { - // if the string is actually a gid integer, parse to int and return - // as we don't need to translate with the help of files - gid, err := strconv.Atoi(groupStr) - if err == nil { - return gid, nil - } - groups, err := lcUser.ParseGroupFileFilter(filepath, func(g lcUser.Group) bool { - return g.Name == groupStr - }) - if err != nil { - return 0, err - } - if len(groups) == 0 { - return 0, errors.New("no such group: " + groupStr) - } - return groups[0].Gid, nil -} - func createDestInfo(workingDir string, inst copyInstruction, imageMount *imageMount, platform string) (copyInfo, error) { // Twiddle the destination when it's a relative path - meaning, make it // relative to the WORKINGDIR diff --git a/components/engine/builder/dockerfile/internals_linux.go b/components/engine/builder/dockerfile/internals_linux.go new file mode 100644 index 0000000000..1314779248 --- /dev/null +++ b/components/engine/builder/dockerfile/internals_linux.go @@ -0,0 +1,88 @@ +package dockerfile + +import ( + "path/filepath" + "strconv" + "strings" + + "github.com/docker/docker/pkg/idtools" + "github.com/docker/docker/pkg/symlink" + lcUser "github.com/opencontainers/runc/libcontainer/user" + "github.com/pkg/errors" +) + +func parseChownFlag(chown, ctrRootPath string, idMappings *idtools.IDMappings) (idtools.IDPair, error) { + var userStr, grpStr string + parts := strings.Split(chown, ":") + if len(parts) > 2 { + return idtools.IDPair{}, errors.New("invalid chown string format: " + chown) + } + if len(parts) == 1 { + // if no group specified, use the user spec as group as well + userStr, grpStr = parts[0], parts[0] + } else { + userStr, grpStr = parts[0], parts[1] + } + + passwdPath, err := symlink.FollowSymlinkInScope(filepath.Join(ctrRootPath, "etc", "passwd"), ctrRootPath) + if err != nil { + return idtools.IDPair{}, errors.Wrapf(err, "can't resolve /etc/passwd path in container rootfs") + } + groupPath, err := symlink.FollowSymlinkInScope(filepath.Join(ctrRootPath, "etc", "group"), ctrRootPath) + if err != nil { + return idtools.IDPair{}, errors.Wrapf(err, "can't resolve /etc/group path in container rootfs") + } + uid, err := lookupUser(userStr, passwdPath) + if err != nil { + return idtools.IDPair{}, errors.Wrapf(err, "can't find uid for user "+userStr) + } + gid, err := lookupGroup(grpStr, groupPath) + if err != nil { + return idtools.IDPair{}, errors.Wrapf(err, "can't find gid for group "+grpStr) + } + + // convert as necessary because of user namespaces + chownPair, err := idMappings.ToHost(idtools.IDPair{UID: uid, GID: gid}) + if err != nil { + return idtools.IDPair{}, errors.Wrapf(err, "unable to convert uid/gid to host mapping") + } + return chownPair, nil +} + +func lookupUser(userStr, filepath string) (int, error) { + // if the string is actually a uid integer, parse to int and return + // as we don't need to translate with the help of files + uid, err := strconv.Atoi(userStr) + if err == nil { + return uid, nil + } + users, err := lcUser.ParsePasswdFileFilter(filepath, func(u lcUser.User) bool { + return u.Name == userStr + }) + if err != nil { + return 0, err + } + if len(users) == 0 { + return 0, errors.New("no such user: " + userStr) + } + return users[0].Uid, nil +} + +func lookupGroup(groupStr, filepath string) (int, error) { + // if the string is actually a gid integer, parse to int and return + // as we don't need to translate with the help of files + gid, err := strconv.Atoi(groupStr) + if err == nil { + return gid, nil + } + groups, err := lcUser.ParseGroupFileFilter(filepath, func(g lcUser.Group) bool { + return g.Name == groupStr + }) + if err != nil { + return 0, err + } + if len(groups) == 0 { + return 0, errors.New("no such group: " + groupStr) + } + return groups[0].Gid, nil +} diff --git a/components/engine/builder/dockerfile/internals_linux_test.go b/components/engine/builder/dockerfile/internals_linux_test.go new file mode 100644 index 0000000000..dd23a33307 --- /dev/null +++ b/components/engine/builder/dockerfile/internals_linux_test.go @@ -0,0 +1,138 @@ +package dockerfile + +import ( + "os" + "path/filepath" + "testing" + + "github.com/docker/docker/pkg/idtools" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestChownFlagParsing(t *testing.T) { + testFiles := map[string]string{ + "passwd": `root:x:0:0::/bin:/bin/false +bin:x:1:1::/bin:/bin/false +wwwwww:x:21:33::/bin:/bin/false +unicorn:x:1001:1002::/bin:/bin/false + `, + "group": `root:x:0: +bin:x:1: +wwwwww:x:33: +unicorn:x:1002: +somegrp:x:5555: +othergrp:x:6666: + `, + } + // test mappings for validating use of maps + idMaps := []idtools.IDMap{ + { + ContainerID: 0, + HostID: 100000, + Size: 65536, + }, + } + remapped := idtools.NewIDMappingsFromMaps(idMaps, idMaps) + unmapped := &idtools.IDMappings{} + + contextDir, cleanup := createTestTempDir(t, "", "builder-chown-parse-test") + defer cleanup() + + if err := os.Mkdir(filepath.Join(contextDir, "etc"), 0755); err != nil { + t.Fatalf("error creating test directory: %v", err) + } + + for filename, content := range testFiles { + createTestTempFile(t, filepath.Join(contextDir, "etc"), filename, content, 0644) + } + + // positive tests + for _, testcase := range []struct { + name string + chownStr string + idMapping *idtools.IDMappings + expected idtools.IDPair + }{ + { + name: "UIDNoMap", + chownStr: "1", + idMapping: unmapped, + expected: idtools.IDPair{UID: 1, GID: 1}, + }, + { + name: "UIDGIDNoMap", + chownStr: "0:1", + idMapping: unmapped, + expected: idtools.IDPair{UID: 0, GID: 1}, + }, + { + name: "UIDWithMap", + chownStr: "0", + idMapping: remapped, + expected: idtools.IDPair{UID: 100000, GID: 100000}, + }, + { + name: "UIDGIDWithMap", + chownStr: "1:33", + idMapping: remapped, + expected: idtools.IDPair{UID: 100001, GID: 100033}, + }, + { + name: "UserNoMap", + chownStr: "bin:5555", + idMapping: unmapped, + expected: idtools.IDPair{UID: 1, GID: 5555}, + }, + { + name: "GroupWithMap", + chownStr: "0:unicorn", + idMapping: remapped, + expected: idtools.IDPair{UID: 100000, GID: 101002}, + }, + { + name: "UserOnlyWithMap", + chownStr: "unicorn", + idMapping: remapped, + expected: idtools.IDPair{UID: 101001, GID: 101002}, + }, + } { + t.Run(testcase.name, func(t *testing.T) { + idPair, err := parseChownFlag(testcase.chownStr, contextDir, testcase.idMapping) + require.NoError(t, err, "Failed to parse chown flag: %q", testcase.chownStr) + assert.Equal(t, testcase.expected, idPair, "chown flag mapping failure") + }) + } + + // error tests + for _, testcase := range []struct { + name string + chownStr string + idMapping *idtools.IDMappings + descr string + }{ + { + name: "BadChownFlagFormat", + chownStr: "bob:1:555", + idMapping: unmapped, + descr: "invalid chown string format: bob:1:555", + }, + { + name: "UserNoExist", + chownStr: "bob", + idMapping: unmapped, + descr: "can't find uid for user bob: no such user: bob", + }, + { + name: "GroupNoExist", + chownStr: "root:bob", + idMapping: unmapped, + descr: "can't find gid for group bob: no such group: bob", + }, + } { + t.Run(testcase.name, func(t *testing.T) { + _, err := parseChownFlag(testcase.chownStr, contextDir, testcase.idMapping) + assert.EqualError(t, err, testcase.descr, "Expected error string doesn't match") + }) + } +} diff --git a/components/engine/builder/dockerfile/internals_test.go b/components/engine/builder/dockerfile/internals_test.go index 83a207c455..04c8c4bac8 100644 --- a/components/engine/builder/dockerfile/internals_test.go +++ b/components/engine/builder/dockerfile/internals_test.go @@ -2,8 +2,6 @@ package dockerfile import ( "fmt" - "os" - "path/filepath" "runtime" "testing" @@ -13,7 +11,6 @@ import ( "github.com/docker/docker/builder" "github.com/docker/docker/builder/remotecontext" "github.com/docker/docker/pkg/archive" - "github.com/docker/docker/pkg/idtools" "github.com/docker/go-connections/nat" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -171,130 +168,3 @@ func TestDeepCopyRunConfig(t *testing.T) { copy.Shell[0] = "sh" assert.Equal(t, fullMutableRunConfig(), runConfig) } - -func TestChownFlagParsing(t *testing.T) { - testFiles := map[string]string{ - "passwd": `root:x:0:0::/bin:/bin/false -bin:x:1:1::/bin:/bin/false -wwwwww:x:21:33::/bin:/bin/false -unicorn:x:1001:1002::/bin:/bin/false - `, - "group": `root:x:0: -bin:x:1: -wwwwww:x:33: -unicorn:x:1002: -somegrp:x:5555: -othergrp:x:6666: - `, - } - // test mappings for validating use of maps - idMaps := []idtools.IDMap{ - { - ContainerID: 0, - HostID: 100000, - Size: 65536, - }, - } - remapped := idtools.NewIDMappingsFromMaps(idMaps, idMaps) - unmapped := &idtools.IDMappings{} - - contextDir, cleanup := createTestTempDir(t, "", "builder-chown-parse-test") - defer cleanup() - - if err := os.Mkdir(filepath.Join(contextDir, "etc"), 0755); err != nil { - t.Fatalf("error creating test directory: %v", err) - } - - for filename, content := range testFiles { - createTestTempFile(t, filepath.Join(contextDir, "etc"), filename, content, 0644) - } - - // positive tests - for _, testcase := range []struct { - name string - chownStr string - idMapping *idtools.IDMappings - expected idtools.IDPair - }{ - { - name: "UIDNoMap", - chownStr: "1", - idMapping: unmapped, - expected: idtools.IDPair{UID: 1, GID: 1}, - }, - { - name: "UIDGIDNoMap", - chownStr: "0:1", - idMapping: unmapped, - expected: idtools.IDPair{UID: 0, GID: 1}, - }, - { - name: "UIDWithMap", - chownStr: "0", - idMapping: remapped, - expected: idtools.IDPair{UID: 100000, GID: 100000}, - }, - { - name: "UIDGIDWithMap", - chownStr: "1:33", - idMapping: remapped, - expected: idtools.IDPair{UID: 100001, GID: 100033}, - }, - { - name: "UserNoMap", - chownStr: "bin:5555", - idMapping: unmapped, - expected: idtools.IDPair{UID: 1, GID: 5555}, - }, - { - name: "GroupWithMap", - chownStr: "0:unicorn", - idMapping: remapped, - expected: idtools.IDPair{UID: 100000, GID: 101002}, - }, - { - name: "UserOnlyWithMap", - chownStr: "unicorn", - idMapping: remapped, - expected: idtools.IDPair{UID: 101001, GID: 101002}, - }, - } { - t.Run(testcase.name, func(t *testing.T) { - idPair, err := parseChownFlag(testcase.chownStr, contextDir, testcase.idMapping) - require.NoError(t, err, "Failed to parse chown flag: %q", testcase.chownStr) - assert.Equal(t, testcase.expected, idPair, "chown flag mapping failure") - }) - } - - // error tests - for _, testcase := range []struct { - name string - chownStr string - idMapping *idtools.IDMappings - descr string - }{ - { - name: "BadChownFlagFormat", - chownStr: "bob:1:555", - idMapping: unmapped, - descr: "invalid chown string format: bob:1:555", - }, - { - name: "UserNoExist", - chownStr: "bob", - idMapping: unmapped, - descr: "can't find uid for user bob: no such user: bob", - }, - { - name: "GroupNoExist", - chownStr: "root:bob", - idMapping: unmapped, - descr: "can't find gid for group bob: no such group: bob", - }, - } { - t.Run(testcase.name, func(t *testing.T) { - _, err := parseChownFlag(testcase.chownStr, contextDir, testcase.idMapping) - assert.EqualError(t, err, testcase.descr, "Expected error string doesn't match") - }) - } -} diff --git a/components/engine/builder/dockerfile/internals_windows.go b/components/engine/builder/dockerfile/internals_windows.go new file mode 100644 index 0000000000..931df2f02a --- /dev/null +++ b/components/engine/builder/dockerfile/internals_windows.go @@ -0,0 +1,7 @@ +package dockerfile + +import "github.com/docker/docker/pkg/idtools" + +func parseChownFlag(chown, ctrRootPath string, idMappings *idtools.IDMappings) (idtools.IDPair, error) { + return idMappings.RootPair(), nil +} diff --git a/components/engine/integration-cli/docker_cli_run_test.go b/components/engine/integration-cli/docker_cli_run_test.go index 90d5b46536..bf0cb0ddf8 100644 --- a/components/engine/integration-cli/docker_cli_run_test.go +++ b/components/engine/integration-cli/docker_cli_run_test.go @@ -36,7 +36,6 @@ import ( "github.com/docker/libnetwork/types" "github.com/go-check/check" "github.com/gotestyourself/gotestyourself/icmd" - libcontainerUser "github.com/opencontainers/runc/libcontainer/user" "golang.org/x/net/context" ) @@ -751,7 +750,7 @@ func (s *DockerSuite) TestRunUserByIDBig(c *check.C) { if err == nil { c.Fatal("No error, but must be.", out) } - if !strings.Contains(strings.ToUpper(out), strings.ToUpper(libcontainerUser.ErrRange.Error())) { + if !strings.Contains(strings.ToLower(out), "uids and gids must be in range") { c.Fatalf("expected error about uids range, got %s", out) } } @@ -764,7 +763,7 @@ func (s *DockerSuite) TestRunUserByIDNegative(c *check.C) { if err == nil { c.Fatal("No error, but must be.", out) } - if !strings.Contains(strings.ToUpper(out), strings.ToUpper(libcontainerUser.ErrRange.Error())) { + if !strings.Contains(strings.ToLower(out), "uids and gids must be in range") { c.Fatalf("expected error about uids range, got %s", out) } } From ebd586c5613a542ca99adeb602aaea38c4f0dd59 Mon Sep 17 00:00:00 2001 From: John Howard Date: Thu, 18 Jan 2018 08:01:22 -0800 Subject: [PATCH 7/9] LCOW remotefs - return error in Read() implementation Signed-off-by: John Howard Upstream-commit: 6112ad6e7d5d7f5afc698447da80f91bdbf62720 Component: engine --- components/engine/daemon/graphdriver/lcow/remotefs_file.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/components/engine/daemon/graphdriver/lcow/remotefs_file.go b/components/engine/daemon/graphdriver/lcow/remotefs_file.go index 138cea3c78..2a561428d3 100644 --- a/components/engine/daemon/graphdriver/lcow/remotefs_file.go +++ b/components/engine/daemon/graphdriver/lcow/remotefs_file.go @@ -86,7 +86,7 @@ func (l *lcowfile) Read(b []byte) (int, error) { buf, err := l.getResponse() if err != nil { - return 0, nil + return 0, err } n := copy(b, buf) @@ -105,7 +105,7 @@ func (l *lcowfile) Write(b []byte) (int, error) { _, err := l.getResponse() if err != nil { - return 0, nil + return 0, err } return len(b), nil @@ -168,7 +168,7 @@ func (l *lcowfile) Readdir(n int) ([]os.FileInfo, error) { var info []remotefs.FileInfo if err := json.Unmarshal(buf.Bytes(), &info); err != nil { - return nil, nil + return nil, err } osInfo := make([]os.FileInfo, len(info)) From ed76e0e519b4e942168d5b4c3869be1bd85b41b7 Mon Sep 17 00:00:00 2001 From: Adam Pointer Date: Fri, 19 Jan 2018 09:00:24 +0000 Subject: [PATCH 8/9] Alias container and network packages to stop name clashes Signed-off-by: Adam Pointer Upstream-commit: 7732ca94fc67a28cbd37c292dc29216255685eba Component: engine --- components/engine/client/interface.go | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/components/engine/client/interface.go b/components/engine/client/interface.go index dd8b388cfa..9fb8194fe0 100644 --- a/components/engine/client/interface.go +++ b/components/engine/client/interface.go @@ -6,11 +6,11 @@ import ( "time" "github.com/docker/docker/api/types" - "github.com/docker/docker/api/types/container" + containertypes "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/events" "github.com/docker/docker/api/types/filters" "github.com/docker/docker/api/types/image" - "github.com/docker/docker/api/types/network" + networktypes "github.com/docker/docker/api/types/network" "github.com/docker/docker/api/types/registry" "github.com/docker/docker/api/types/swarm" volumetypes "github.com/docker/docker/api/types/volume" @@ -43,8 +43,8 @@ type CommonAPIClient interface { type ContainerAPIClient interface { ContainerAttach(ctx context.Context, container string, options types.ContainerAttachOptions) (types.HijackedResponse, error) ContainerCommit(ctx context.Context, container string, options types.ContainerCommitOptions) (types.IDResponse, error) - ContainerCreate(ctx context.Context, config *container.Config, hostConfig *container.HostConfig, networkingConfig *network.NetworkingConfig, containerName string) (container.ContainerCreateCreatedBody, error) - ContainerDiff(ctx context.Context, container string) ([]container.ContainerChangeResponseItem, error) + ContainerCreate(ctx context.Context, config *containertypes.Config, hostConfig *containertypes.HostConfig, networkingConfig *networktypes.NetworkingConfig, containerName string) (containertypes.ContainerCreateCreatedBody, error) + ContainerDiff(ctx context.Context, container string) ([]containertypes.ContainerChangeResponseItem, error) ContainerExecAttach(ctx context.Context, execID string, config types.ExecStartCheck) (types.HijackedResponse, error) ContainerExecCreate(ctx context.Context, container string, config types.ExecConfig) (types.IDResponse, error) ContainerExecInspect(ctx context.Context, execID string) (types.ContainerExecInspect, error) @@ -65,10 +65,10 @@ type ContainerAPIClient interface { ContainerStats(ctx context.Context, container string, stream bool) (types.ContainerStats, error) ContainerStart(ctx context.Context, container string, options types.ContainerStartOptions) error ContainerStop(ctx context.Context, container string, timeout *time.Duration) error - ContainerTop(ctx context.Context, container string, arguments []string) (container.ContainerTopOKBody, error) + ContainerTop(ctx context.Context, container string, arguments []string) (containertypes.ContainerTopOKBody, error) ContainerUnpause(ctx context.Context, container string) error - ContainerUpdate(ctx context.Context, container string, updateConfig container.UpdateConfig) (container.ContainerUpdateOKBody, error) - ContainerWait(ctx context.Context, container string, condition container.WaitCondition) (<-chan container.ContainerWaitOKBody, <-chan error) + ContainerUpdate(ctx context.Context, container string, updateConfig containertypes.UpdateConfig) (containertypes.ContainerUpdateOKBody, error) + ContainerWait(ctx context.Context, container string, condition containertypes.WaitCondition) (<-chan containertypes.ContainerWaitOKBody, <-chan error) CopyFromContainer(ctx context.Context, container, srcPath string) (io.ReadCloser, types.ContainerPathStat, error) CopyToContainer(ctx context.Context, container, path string, content io.Reader, options types.CopyToContainerOptions) error ContainersPrune(ctx context.Context, pruneFilters filters.Args) (types.ContainersPruneReport, error) @@ -100,13 +100,13 @@ type ImageAPIClient interface { // NetworkAPIClient defines API client methods for the networks type NetworkAPIClient interface { - NetworkConnect(ctx context.Context, networkID, container string, config *network.EndpointSettings) error + NetworkConnect(ctx context.Context, network, container string, config *networktypes.EndpointSettings) error NetworkCreate(ctx context.Context, name string, options types.NetworkCreate) (types.NetworkCreateResponse, error) - NetworkDisconnect(ctx context.Context, networkID, container string, force bool) error - NetworkInspect(ctx context.Context, networkID string, options types.NetworkInspectOptions) (types.NetworkResource, error) - NetworkInspectWithRaw(ctx context.Context, networkID string, options types.NetworkInspectOptions) (types.NetworkResource, []byte, error) + NetworkDisconnect(ctx context.Context, network, container string, force bool) error + NetworkInspect(ctx context.Context, network string, options types.NetworkInspectOptions) (types.NetworkResource, error) + NetworkInspectWithRaw(ctx context.Context, network string, options types.NetworkInspectOptions) (types.NetworkResource, []byte, error) NetworkList(ctx context.Context, options types.NetworkListOptions) ([]types.NetworkResource, error) - NetworkRemove(ctx context.Context, networkID string) error + NetworkRemove(ctx context.Context, network string) error NetworksPrune(ctx context.Context, pruneFilter filters.Args) (types.NetworksPruneReport, error) } From 5e6be7b1c207d4c41d2d89d06ffb4ac6865a804f Mon Sep 17 00:00:00 2001 From: John Howard Date: Fri, 19 Jan 2018 12:22:39 -0800 Subject: [PATCH 9/9] Bump RS3 final build, and remove LCOW_SUPPORTED Signed-off-by: John Howard Upstream-commit: 5b24976ad4046a9c071b75df31d9269ad2e84732 Component: engine --- components/engine/pkg/system/init_windows.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/components/engine/pkg/system/init_windows.go b/components/engine/pkg/system/init_windows.go index 825e4b587a..9487947368 100644 --- a/components/engine/pkg/system/init_windows.go +++ b/components/engine/pkg/system/init_windows.go @@ -1,17 +1,12 @@ package system -import "os" - // lcowSupported determines if Linux Containers on Windows are supported. var lcowSupported = false // InitLCOW sets whether LCOW is supported or not -// TODO @jhowardmsft. -// 1. Replace with RS3 RTM build number. -// 2. Remove the getenv check when image-store is coalesced as shouldn't be needed anymore. func InitLCOW(experimental bool) { v := GetOSVersion() - if experimental && v.Build > 16278 && os.Getenv("LCOW_SUPPORTED") != "" { + if experimental && v.Build >= 16299 { lcowSupported = true } }