From bcde4812dbb6b051c673b5bf4717cabf8d85bb71 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 13 Jul 2018 09:53:36 +0200 Subject: [PATCH 1/8] Remove stray uses of "golang.org/x/net/context" Signed-off-by: Sebastiaan van Stijn Upstream-commit: 4c0a050ee24beb399b11c48829522e48f8d0b98a Component: engine --- components/engine/client/build_cancel.go | 3 +-- components/engine/client/hijack_test.go | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/components/engine/client/build_cancel.go b/components/engine/client/build_cancel.go index 4cf8c980a9..74df495089 100644 --- a/components/engine/client/build_cancel.go +++ b/components/engine/client/build_cancel.go @@ -1,9 +1,8 @@ package client // import "github.com/docker/docker/client" import ( + "context" "net/url" - - "golang.org/x/net/context" ) // BuildCancel requests the daemon to cancel ongoing build request diff --git a/components/engine/client/hijack_test.go b/components/engine/client/hijack_test.go index 823bf344f5..d71dc9ea38 100644 --- a/components/engine/client/hijack_test.go +++ b/components/engine/client/hijack_test.go @@ -1,6 +1,7 @@ package client import ( + "context" "fmt" "io/ioutil" "net" @@ -12,7 +13,6 @@ import ( "github.com/docker/docker/api/server/httputils" "github.com/docker/docker/api/types" "github.com/pkg/errors" - "golang.org/x/net/context" "gotest.tools/assert" ) From 8ce22ea6b6207683fb077592fd47f8e666b24148 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 13 Jul 2018 09:54:24 +0200 Subject: [PATCH 2/8] Fix API template to not use "golang.org/x/net/context" Signed-off-by: Sebastiaan van Stijn Upstream-commit: a8b4e04e2f6ae95a5146c4c4d794cda1a725c634 Component: engine --- components/engine/api/templates/server/operation.gotmpl | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/components/engine/api/templates/server/operation.gotmpl b/components/engine/api/templates/server/operation.gotmpl index 8bed59d920..cf24aacc5f 100644 --- a/components/engine/api/templates/server/operation.gotmpl +++ b/components/engine/api/templates/server/operation.gotmpl @@ -8,10 +8,8 @@ package {{ .Package }} // ---------------------------------------------------------------------------- import ( + "context" "net/http" - - context "golang.org/x/net/context" - {{ range .DefaultImports }}{{ printf "%q" . }} {{ end }} {{ range $key, $value := .Imports }}{{ $key }} {{ printf "%q" $value }} From 91b7b13fc24246d6320fe33200bdf93c1838ee5a Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 13 Jul 2018 12:55:59 +0200 Subject: [PATCH 3/8] Refactor daemon.info to reduce cyclomatic complexity Before this change; gocyclo daemon/info.go 17 daemon (*Daemon).SystemInfo daemon/info.go:27:1 2 daemon (*Daemon).SystemVersion daemon/info.go:150:1 1 daemon (*Daemon).showPluginsInfo daemon/info.go:195:1 After this change; gocyclo daemon/info.go 8 daemon (*Daemon).fillSecurityOptions daemon/info.go:150:1 5 daemon operatingSystem daemon/info.go:201:1 3 daemon (*Daemon).fillDriverInfo daemon/info.go:121:1 2 daemon hostName daemon/info.go:172:1 2 daemon memInfo daemon/info.go:192:1 2 daemon kernelVersion daemon/info.go:182:1 1 daemon (*Daemon).SystemVersion daemon/info.go:81:1 1 daemon (*Daemon).SystemInfo daemon/info.go:27:1 1 daemon (*Daemon).fillPluginsInfo daemon/info.go:138:1 Signed-off-by: Sebastiaan van Stijn Upstream-commit: 50eaed4d0c1492479ed4d83eb97424d21a3e458d Component: engine --- components/engine/daemon/info.go | 195 ++++++++++++++++--------------- 1 file changed, 103 insertions(+), 92 deletions(-) diff --git a/components/engine/daemon/info.go b/components/engine/daemon/info.go index 7b011fe324..641b2b17cb 100644 --- a/components/engine/daemon/info.go +++ b/components/engine/daemon/info.go @@ -25,70 +25,9 @@ import ( // SystemInfo returns information about the host server the daemon is running on. func (daemon *Daemon) SystemInfo() (*types.Info, error) { - kernelVersion := "" - if kv, err := kernel.GetKernelVersion(); err != nil { - logrus.Warnf("Could not get kernel version: %v", err) - } else { - kernelVersion = kv.String() - } - - operatingSystem := "" - if s, err := operatingsystem.GetOperatingSystem(); err != nil { - logrus.Warnf("Could not get operating system name: %v", err) - } else { - operatingSystem = s - } - - // Don't do containerized check on Windows - if runtime.GOOS != "windows" { - if inContainer, err := operatingsystem.IsContainerized(); err != nil { - logrus.Errorf("Could not determine if daemon is containerized: %v", err) - operatingSystem += " (error determining if containerized)" - } else if inContainer { - operatingSystem += " (containerized)" - } - } - - meminfo, err := system.ReadMemInfo() - if err != nil { - logrus.Errorf("Could not read system memory info: %v", err) - meminfo = &system.MemInfo{} - } - sysInfo := sysinfo.New(true) cRunning, cPaused, cStopped := stateCtr.get() - securityOptions := []string{} - if sysInfo.AppArmor { - securityOptions = append(securityOptions, "name=apparmor") - } - if sysInfo.Seccomp && supportsSeccomp { - profile := daemon.seccompProfilePath - if profile == "" { - profile = "default" - } - securityOptions = append(securityOptions, fmt.Sprintf("name=seccomp,profile=%s", profile)) - } - if selinuxEnabled() { - securityOptions = append(securityOptions, "name=selinux") - } - rootIDs := daemon.idMappings.RootPair() - if rootIDs.UID != 0 || rootIDs.GID != 0 { - securityOptions = append(securityOptions, "name=userns") - } - - var ds [][2]string - drivers := "" - statuses := daemon.imageService.LayerStoreStatus() - for os, gd := range daemon.graphDrivers { - ds = append(ds, statuses[os]...) - drivers += gd - if len(daemon.graphDrivers) > 1 { - drivers += fmt.Sprintf(" (%s) ", os) - } - } - drivers = strings.TrimSpace(drivers) - v := &types.Info{ ID: daemon.ID, Containers: cRunning + cPaused + cStopped, @@ -96,27 +35,25 @@ func (daemon *Daemon) SystemInfo() (*types.Info, error) { ContainersPaused: cPaused, ContainersStopped: cStopped, Images: daemon.imageService.CountImages(), - Driver: drivers, - DriverStatus: ds, - Plugins: daemon.showPluginsInfo(), IPv4Forwarding: !sysInfo.IPv4ForwardingDisabled, BridgeNfIptables: !sysInfo.BridgeNFCallIPTablesDisabled, BridgeNfIP6tables: !sysInfo.BridgeNFCallIP6TablesDisabled, Debug: debug.IsEnabled(), + Name: hostName(), NFd: fileutils.GetTotalUsedFds(), NGoroutines: runtime.NumGoroutine(), SystemTime: time.Now().Format(time.RFC3339Nano), LoggingDriver: daemon.defaultLogConfig.Type, CgroupDriver: daemon.getCgroupDriver(), NEventsListener: daemon.EventsService.SubscribersCount(), - KernelVersion: kernelVersion, - OperatingSystem: operatingSystem, + KernelVersion: kernelVersion(), + OperatingSystem: operatingSystem(), IndexServerAddress: registry.IndexServer, OSType: platform.OSType, Architecture: platform.Architecture, RegistryConfig: daemon.RegistryService.ServiceConfig(), NCPU: sysinfo.NumCPU(), - MemTotal: meminfo.MemTotal, + MemTotal: memInfo().MemTotal, GenericResources: daemon.genericResources, DockerRootDir: daemon.configStore.Root, Labels: daemon.configStore.Labels, @@ -128,32 +65,21 @@ func (daemon *Daemon) SystemInfo() (*types.Info, error) { HTTPSProxy: sockets.GetProxyEnv("https_proxy"), NoProxy: sockets.GetProxyEnv("no_proxy"), LiveRestoreEnabled: daemon.configStore.LiveRestoreEnabled, - SecurityOptions: securityOptions, Isolation: daemon.defaultIsolation, } // Retrieve platform specific info daemon.FillPlatformInfo(v, sysInfo) - - hostname := "" - if hn, err := os.Hostname(); err != nil { - logrus.Warnf("Could not get hostname: %v", err) - } else { - hostname = hn - } - v.Name = hostname + daemon.fillDriverInfo(v) + daemon.fillPluginsInfo(v) + daemon.fillSecurityOptions(v, sysInfo) return v, nil } // SystemVersion returns version information about the daemon. func (daemon *Daemon) SystemVersion() types.Version { - kernelVersion := "" - if kv, err := kernel.GetKernelVersion(); err != nil { - logrus.Warnf("Could not get kernel version: %v", err) - } else { - kernelVersion = kv.String() - } + kernelVersion := kernelVersion() v := types.Version{ Components: []types.ComponentVersion{ @@ -192,15 +118,100 @@ func (daemon *Daemon) SystemVersion() types.Version { return v } -func (daemon *Daemon) showPluginsInfo() types.PluginsInfo { - var pluginsInfo types.PluginsInfo +func (daemon *Daemon) fillDriverInfo(v *types.Info) { + var ds [][2]string + drivers := "" + statuses := daemon.imageService.LayerStoreStatus() + for os, gd := range daemon.graphDrivers { + ds = append(ds, statuses[os]...) + drivers += gd + if len(daemon.graphDrivers) > 1 { + drivers += fmt.Sprintf(" (%s) ", os) + } + } + drivers = strings.TrimSpace(drivers) - pluginsInfo.Volume = daemon.volumes.GetDriverList() - pluginsInfo.Network = daemon.GetNetworkDriverList() - // The authorization plugins are returned in the order they are - // used as they constitute a request/response modification chain. - pluginsInfo.Authorization = daemon.configStore.AuthorizationPlugins - pluginsInfo.Log = logger.ListDrivers() - - return pluginsInfo + v.Driver = drivers + v.DriverStatus = ds +} + +func (daemon *Daemon) fillPluginsInfo(v *types.Info) { + v.Plugins = types.PluginsInfo{ + Volume: daemon.volumes.GetDriverList(), + Network: daemon.GetNetworkDriverList(), + + // The authorization plugins are returned in the order they are + // used as they constitute a request/response modification chain. + Authorization: daemon.configStore.AuthorizationPlugins, + Log: logger.ListDrivers(), + } +} + +func (daemon *Daemon) fillSecurityOptions(v *types.Info, sysInfo *sysinfo.SysInfo) { + var securityOptions []string + if sysInfo.AppArmor { + securityOptions = append(securityOptions, "name=apparmor") + } + if sysInfo.Seccomp && supportsSeccomp { + profile := daemon.seccompProfilePath + if profile == "" { + profile = "default" + } + securityOptions = append(securityOptions, fmt.Sprintf("name=seccomp,profile=%s", profile)) + } + if selinuxEnabled() { + securityOptions = append(securityOptions, "name=selinux") + } + if rootIDs := daemon.idMappings.RootPair(); rootIDs.UID != 0 || rootIDs.GID != 0 { + securityOptions = append(securityOptions, "name=userns") + } + v.SecurityOptions = securityOptions +} + +func hostName() string { + hostname := "" + if hn, err := os.Hostname(); err != nil { + logrus.Warnf("Could not get hostname: %v", err) + } else { + hostname = hn + } + return hostname +} + +func kernelVersion() string { + kernelVersion := "" + if kv, err := kernel.GetKernelVersion(); err != nil { + logrus.Warnf("Could not get kernel version: %v", err) + } else { + kernelVersion = kv.String() + } + return kernelVersion +} + +func memInfo() *system.MemInfo { + memInfo, err := system.ReadMemInfo() + if err != nil { + logrus.Errorf("Could not read system memory info: %v", err) + memInfo = &system.MemInfo{} + } + return memInfo +} + +func operatingSystem() string { + operatingSystem := "" + if s, err := operatingsystem.GetOperatingSystem(); err != nil { + logrus.Warnf("Could not get operating system name: %v", err) + } else { + operatingSystem = s + } + // Don't do containerized check on Windows + if runtime.GOOS != "windows" { + if inContainer, err := operatingsystem.IsContainerized(); err != nil { + logrus.Errorf("Could not determine if daemon is containerized: %v", err) + operatingSystem += " (error determining if containerized)" + } else if inContainer { + operatingSystem += " (containerized)" + } + } + return operatingSystem } From c1dba76a948a3e919ef211e4e65377f95d9de477 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 13 Jul 2018 13:14:45 +0200 Subject: [PATCH 4/8] Unexport daemon.FillPlatformInfo Signed-off-by: Sebastiaan van Stijn Upstream-commit: c03d3a416b24c18dfbb938397052eebd8b67d1a3 Component: engine --- components/engine/daemon/info.go | 2 +- components/engine/daemon/info_unix.go | 4 ++-- components/engine/daemon/info_windows.go | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/components/engine/daemon/info.go b/components/engine/daemon/info.go index 641b2b17cb..0ed81c3b5e 100644 --- a/components/engine/daemon/info.go +++ b/components/engine/daemon/info.go @@ -69,7 +69,7 @@ func (daemon *Daemon) SystemInfo() (*types.Info, error) { } // Retrieve platform specific info - daemon.FillPlatformInfo(v, sysInfo) + daemon.fillPlatformInfo(v, sysInfo) daemon.fillDriverInfo(v) daemon.fillPluginsInfo(v) daemon.fillSecurityOptions(v, sysInfo) diff --git a/components/engine/daemon/info_unix.go b/components/engine/daemon/info_unix.go index 56be9c06fb..864e8816a9 100644 --- a/components/engine/daemon/info_unix.go +++ b/components/engine/daemon/info_unix.go @@ -14,8 +14,8 @@ import ( "github.com/sirupsen/logrus" ) -// FillPlatformInfo fills the platform related info. -func (daemon *Daemon) FillPlatformInfo(v *types.Info, sysInfo *sysinfo.SysInfo) { +// fillPlatformInfo fills the platform related info. +func (daemon *Daemon) fillPlatformInfo(v *types.Info, sysInfo *sysinfo.SysInfo) { v.MemoryLimit = sysInfo.MemoryLimit v.SwapLimit = sysInfo.SwapLimit v.KernelMemory = sysInfo.KernelMemory diff --git a/components/engine/daemon/info_windows.go b/components/engine/daemon/info_windows.go index e452369fc8..bf97971479 100644 --- a/components/engine/daemon/info_windows.go +++ b/components/engine/daemon/info_windows.go @@ -5,6 +5,6 @@ import ( "github.com/docker/docker/pkg/sysinfo" ) -// FillPlatformInfo fills the platform related info. -func (daemon *Daemon) FillPlatformInfo(v *types.Info, sysInfo *sysinfo.SysInfo) { +// fillPlatformInfo fills the platform related info. +func (daemon *Daemon) fillPlatformInfo(v *types.Info, sysInfo *sysinfo.SysInfo) { } From d1e4c2c76654ea9dfe263659b8235ba4a5c3985f Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 13 Jul 2018 16:57:33 +0200 Subject: [PATCH 5/8] Fix golint warning on generated "volume" types Should fix ``` api/types/volume/volume_create.go Line 10: warning: comment on exported type VolumeCreateBody should be of the form "VolumeCreateBody ..." (with optional leading article) (golint) api/types/volume/volume_list.go Line 12: warning: comment on exported type VolumeListOKBody should be of the form "VolumeListOKBody ..." (with optional leading article) (golint) ``` Signed-off-by: Sebastiaan van Stijn Upstream-commit: bd06a5ea4df919ff323510fba10801b5673a3af6 Component: engine --- components/engine/api/swagger.yaml | 5 ++++- components/engine/api/types/volume/volume_create.go | 2 +- components/engine/api/types/volume/volume_list.go | 2 +- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/components/engine/api/swagger.yaml b/components/engine/api/swagger.yaml index 86374415fd..0e7e69977e 100644 --- a/components/engine/api/swagger.yaml +++ b/components/engine/api/swagger.yaml @@ -5922,7 +5922,7 @@ paths: headers: X-Docker-Container-Path-Stat: type: "string" - description: "TODO" + description: "A base64 - encoded JSON object with some filesystem header information about the path" 400: description: "Bad parameter" schema: @@ -7589,6 +7589,7 @@ paths: schema: type: "object" title: "VolumeListResponse" + description: "Volume list response" required: [Volumes, Warnings] properties: Volumes: @@ -7665,6 +7666,8 @@ paths: description: "Volume configuration" schema: type: "object" + description: "Volume configuration" + title: "VolumeConfig" properties: Name: description: "The new volume's name. If not specified, Docker generates a name." diff --git a/components/engine/api/types/volume/volume_create.go b/components/engine/api/types/volume/volume_create.go index 539e9b97d9..f12e48612c 100644 --- a/components/engine/api/types/volume/volume_create.go +++ b/components/engine/api/types/volume/volume_create.go @@ -7,7 +7,7 @@ package volume // See hack/generate-swagger-api.sh // ---------------------------------------------------------------------------- -// VolumeCreateBody +// VolumeCreateBody Volume configuration // swagger:model VolumeCreateBody type VolumeCreateBody struct { diff --git a/components/engine/api/types/volume/volume_list.go b/components/engine/api/types/volume/volume_list.go index 1bb279dbb3..020198f737 100644 --- a/components/engine/api/types/volume/volume_list.go +++ b/components/engine/api/types/volume/volume_list.go @@ -9,7 +9,7 @@ package volume import "github.com/docker/docker/api/types" -// VolumeListOKBody +// VolumeListOKBody Volume list response // swagger:model VolumeListOKBody type VolumeListOKBody struct { From 5fea4ae591ed7f40304a1d450d77124648967a66 Mon Sep 17 00:00:00 2001 From: Flavio Crisciani Date: Fri, 13 Jul 2018 11:21:44 -0700 Subject: [PATCH 6/8] Fix flakyness in TestDockerNetworkInternalMode Instead of waiting for the DNS to fail, try to access a specific external IP and verify that 100% of the pakcets are being lost. Signed-off-by: Flavio Crisciani Upstream-commit: a2bb2144b3e0d49ac615976bfac462a307d85f0e Component: engine --- .../engine/integration-cli/docker_cli_network_unix_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/engine/integration-cli/docker_cli_network_unix_test.go b/components/engine/integration-cli/docker_cli_network_unix_test.go index d4167f6620..60f211cda8 100644 --- a/components/engine/integration-cli/docker_cli_network_unix_test.go +++ b/components/engine/integration-cli/docker_cli_network_unix_test.go @@ -1644,9 +1644,9 @@ func (s *DockerSuite) TestDockerNetworkInternalMode(c *check.C) { c.Assert(waitRun("first"), check.IsNil) dockerCmd(c, "run", "-d", "--net=internal", "--name=second", "busybox:glibc", "top") c.Assert(waitRun("second"), check.IsNil) - out, _, err := dockerCmdWithError("exec", "first", "ping", "-W", "4", "-c", "1", "www.google.com") + out, _, err := dockerCmdWithError("exec", "first", "ping", "-W", "4", "-c", "1", "8.8.8.8") c.Assert(err, check.NotNil) - c.Assert(out, checker.Contains, "ping: bad address") + c.Assert(out, checker.Contains, "100% packet loss") _, _, err = dockerCmdWithError("exec", "second", "ping", "-c", "1", "first") c.Assert(err, check.IsNil) } From 14ed98c1a49c38fba19c3a7e11b8ab8a0b678d92 Mon Sep 17 00:00:00 2001 From: Arash Deshmeh Date: Fri, 13 Jul 2018 17:29:02 -0400 Subject: [PATCH 7/8] moved integration tests from docker_cli_config_create_test.go to integration/config Signed-off-by: Arash Deshmeh Upstream-commit: 0e57ceae0dd42491ed81538bf4d9164218080b66 Component: engine --- .../docker_cli_config_create_test.go | 101 ------------------ .../engine/integration/config/config_test.go | 99 +++++++++++++++-- 2 files changed, 88 insertions(+), 112 deletions(-) diff --git a/components/engine/integration-cli/docker_cli_config_create_test.go b/components/engine/integration-cli/docker_cli_config_create_test.go index 9bc108a587..c499a2c9cf 100644 --- a/components/engine/integration-cli/docker_cli_config_create_test.go +++ b/components/engine/integration-cli/docker_cli_config_create_test.go @@ -7,111 +7,10 @@ import ( "os" "strings" - "github.com/docker/docker/api/types/swarm" "github.com/docker/docker/integration-cli/checker" "github.com/go-check/check" ) -func (s *DockerSwarmSuite) TestConfigCreate(c *check.C) { - d := s.AddDaemon(c, true, true) - - testName := "test_config" - id := d.CreateConfig(c, swarm.ConfigSpec{ - Annotations: swarm.Annotations{ - Name: testName, - }, - Data: []byte("TESTINGDATA"), - }) - c.Assert(id, checker.Not(checker.Equals), "", check.Commentf("configs: %s", id)) - - config := d.GetConfig(c, id) - c.Assert(config.Spec.Name, checker.Equals, testName) -} - -func (s *DockerSwarmSuite) TestConfigCreateWithLabels(c *check.C) { - d := s.AddDaemon(c, true, true) - - testName := "test_config" - id := d.CreateConfig(c, swarm.ConfigSpec{ - Annotations: swarm.Annotations{ - Name: testName, - Labels: map[string]string{ - "key1": "value1", - "key2": "value2", - }, - }, - Data: []byte("TESTINGDATA"), - }) - c.Assert(id, checker.Not(checker.Equals), "", check.Commentf("configs: %s", id)) - - config := d.GetConfig(c, id) - c.Assert(config.Spec.Name, checker.Equals, testName) - c.Assert(len(config.Spec.Labels), checker.Equals, 2) - c.Assert(config.Spec.Labels["key1"], checker.Equals, "value1") - c.Assert(config.Spec.Labels["key2"], checker.Equals, "value2") -} - -// Test case for 28884 -func (s *DockerSwarmSuite) TestConfigCreateResolve(c *check.C) { - d := s.AddDaemon(c, true, true) - - name := "test_config" - id := d.CreateConfig(c, swarm.ConfigSpec{ - Annotations: swarm.Annotations{ - Name: name, - }, - Data: []byte("foo"), - }) - c.Assert(id, checker.Not(checker.Equals), "", check.Commentf("configs: %s", id)) - - fake := d.CreateConfig(c, swarm.ConfigSpec{ - Annotations: swarm.Annotations{ - Name: id, - }, - Data: []byte("fake foo"), - }) - c.Assert(fake, checker.Not(checker.Equals), "", check.Commentf("configs: %s", fake)) - - out, err := d.Cmd("config", "ls") - c.Assert(err, checker.IsNil) - c.Assert(out, checker.Contains, name) - c.Assert(out, checker.Contains, fake) - - out, err = d.Cmd("config", "rm", id) - c.Assert(err, checker.IsNil) - c.Assert(out, checker.Contains, id) - - // Fake one will remain - out, err = d.Cmd("config", "ls") - c.Assert(err, checker.IsNil) - c.Assert(out, checker.Not(checker.Contains), name) - c.Assert(out, checker.Contains, fake) - - // Remove based on name prefix of the fake one - // (which is the same as the ID of foo one) should not work - // as search is only done based on: - // - Full ID - // - Full Name - // - Partial ID (prefix) - out, err = d.Cmd("config", "rm", id[:5]) - c.Assert(err, checker.Not(checker.IsNil)) - c.Assert(out, checker.Not(checker.Contains), id) - out, err = d.Cmd("config", "ls") - c.Assert(err, checker.IsNil) - c.Assert(out, checker.Not(checker.Contains), name) - c.Assert(out, checker.Contains, fake) - - // Remove based on ID prefix of the fake one should succeed - out, err = d.Cmd("config", "rm", fake[:5]) - c.Assert(err, checker.IsNil) - c.Assert(out, checker.Contains, fake[:5]) - out, err = d.Cmd("config", "ls") - c.Assert(err, checker.IsNil) - c.Assert(out, checker.Not(checker.Contains), name) - c.Assert(out, checker.Not(checker.Contains), id) - c.Assert(out, checker.Not(checker.Contains), fake) -} - func (s *DockerSwarmSuite) TestConfigCreateWithFile(c *check.C) { d := s.AddDaemon(c, true, true) diff --git a/components/engine/integration/config/config_test.go b/components/engine/integration/config/config_test.go index 3cbca23899..536a645b01 100644 --- a/components/engine/integration/config/config_test.go +++ b/components/engine/integration/config/config_test.go @@ -45,19 +45,10 @@ func TestConfigList(t *testing.T) { config1ID := createConfig(ctx, t, client, testName1, []byte("TESTINGDATA1"), map[string]string{"type": "production"}) - names := func(entries []swarmtypes.Config) []string { - var values []string - for _, entry := range entries { - values = append(values, entry.Spec.Name) - } - sort.Strings(values) - return values - } - // test by `config ls` entries, err := client.ConfigList(ctx, types.ConfigListOptions{}) assert.NilError(t, err) - assert.Check(t, is.DeepEqual(names(entries), testNames)) + assert.Check(t, is.DeepEqual(configNamesFromList(entries), testNames)) testCases := []struct { filters filters.Args @@ -92,7 +83,7 @@ func TestConfigList(t *testing.T) { Filters: tc.filters, }) assert.NilError(t, err) - assert.Check(t, is.DeepEqual(names(entries), tc.expected)) + assert.Check(t, is.DeepEqual(configNamesFromList(entries), tc.expected)) } } @@ -354,3 +345,89 @@ func TestConfigInspect(t *testing.T) { assert.NilError(t, err) assert.Check(t, is.DeepEqual(config, insp)) } + +func TestConfigCreateWithLabels(t *testing.T) { + skip.If(t, testEnv.DaemonInfo.OSType != "linux") + + defer setupTest(t)() + d := swarm.NewSwarm(t, testEnv) + defer d.Stop(t) + client := d.NewClientT(t) + defer client.Close() + + ctx := context.Background() + + labels := map[string]string{ + "key1": "value1", + "key2": "value2", + } + testName := t.Name() + configID := createConfig(ctx, t, client, testName, []byte("TESTINGDATA"), labels) + + insp, _, err := client.ConfigInspectWithRaw(ctx, configID) + assert.NilError(t, err) + assert.Check(t, is.Equal(insp.Spec.Name, testName)) + assert.Check(t, is.Equal(2, len(insp.Spec.Labels))) + assert.Check(t, is.Equal("value1", insp.Spec.Labels["key1"])) + assert.Check(t, is.Equal("value2", insp.Spec.Labels["key2"])) +} + +// Test case for 28884 +func TestConfigCreateResolve(t *testing.T) { + skip.If(t, testEnv.DaemonInfo.OSType != "linux") + + defer setupTest(t)() + d := swarm.NewSwarm(t, testEnv) + defer d.Stop(t) + client := d.NewClientT(t) + defer client.Close() + + ctx := context.Background() + + configName := "test_config_" + t.Name() + + configID := createConfig(ctx, t, client, configName, []byte("foo"), nil) + fakeName := configID + fakeID := createConfig(ctx, t, client, fakeName, []byte("fake foo"), nil) + + entries, err := client.ConfigList(ctx, types.ConfigListOptions{}) + assert.NilError(t, err) + assert.Assert(t, is.Contains(configNamesFromList(entries), configName)) + assert.Assert(t, is.Contains(configNamesFromList(entries), fakeName)) + + err = client.ConfigRemove(ctx, configID) + assert.NilError(t, err) + + // Fake one will remain + entries, err = client.ConfigList(ctx, types.ConfigListOptions{}) + assert.NilError(t, err) + assert.Assert(t, is.DeepEqual(configNamesFromList(entries), []string{fakeName})) + + // Remove based on name prefix of the fake one + // (which is the same as the ID of foo one) should not work + // as search is only done based on: + // - Full ID + // - Full Name + // - Partial ID (prefix) + err = client.ConfigRemove(ctx, configID[:5]) + assert.Assert(t, nil != err) + entries, err = client.ConfigList(ctx, types.ConfigListOptions{}) + assert.NilError(t, err) + assert.Assert(t, is.DeepEqual(configNamesFromList(entries), []string{fakeName})) + + // Remove based on ID prefix of the fake one should succeed + err = client.ConfigRemove(ctx, fakeID[:5]) + assert.NilError(t, err) + entries, err = client.ConfigList(ctx, types.ConfigListOptions{}) + assert.NilError(t, err) + assert.Assert(t, is.Equal(0, len(entries))) +} + +func configNamesFromList(entries []swarmtypes.Config) []string { + var values []string + for _, entry := range entries { + values = append(values, entry.Spec.Name) + } + sort.Strings(values) + return values +} From 900616b99575d90b129d5f66bdee8fa9aa814319 Mon Sep 17 00:00:00 2001 From: Yuichiro Kaneko Date: Thu, 5 Jul 2018 09:23:15 +0900 Subject: [PATCH 8/8] Return error if basename is expanded to blank Fix: https://github.com/moby/moby/issues/37325 Signed-off-by: Yuichiro Kaneko Upstream-commit: c9542d313e2a52807644742e5fd684bc2de9f507 Component: engine --- .../engine/builder/dockerfile/dispatchers.go | 10 ++++++++-- .../builder/dockerfile/dispatchers_test.go | 16 ++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/components/engine/builder/dockerfile/dispatchers.go b/components/engine/builder/dockerfile/dispatchers.go index 6ee2b17cda..2dd0177ad3 100644 --- a/components/engine/builder/dockerfile/dispatchers.go +++ b/components/engine/builder/dockerfile/dispatchers.go @@ -274,11 +274,17 @@ func (d *dispatchRequest) getImageOrStage(name string, platform *specs.Platform) } return imageMount.Image(), nil } -func (d *dispatchRequest) getFromImage(shlex *shell.Lex, name string, platform *specs.Platform) (builder.Image, error) { - name, err := d.getExpandedString(shlex, name) +func (d *dispatchRequest) getFromImage(shlex *shell.Lex, basename string, platform *specs.Platform) (builder.Image, error) { + name, err := d.getExpandedString(shlex, basename) if err != nil { return nil, err } + // Empty string is interpreted to FROM scratch by images.GetImageAndReleasableLayer, + // so validate expanded result is not empty. + if name == "" { + return nil, errors.Errorf("base name (%s) should not be blank", basename) + } + return d.getImageOrStage(name, platform) } diff --git a/components/engine/builder/dockerfile/dispatchers_test.go b/components/engine/builder/dockerfile/dispatchers_test.go index c61a45b03a..b623985efe 100644 --- a/components/engine/builder/dockerfile/dispatchers_test.go +++ b/components/engine/builder/dockerfile/dispatchers_test.go @@ -157,6 +157,22 @@ func TestFromWithArg(t *testing.T) { assert.Check(t, is.Len(sb.state.buildArgs.GetAllMeta(), 1)) } +func TestFromWithArgButBuildArgsNotGiven(t *testing.T) { + b := newBuilderWithMockBackend() + args := NewBuildArgs(make(map[string]*string)) + + metaArg := instructions.ArgCommand{} + cmd := &instructions.Stage{ + BaseName: "${THETAG}", + } + err := processMetaArg(metaArg, shell.NewLex('\\'), args) + + sb := newDispatchRequest(b, '\\', nil, args, newStagesBuildResults()) + assert.NilError(t, err) + err = initializeStage(sb, cmd) + assert.Error(t, err, "base name (${THETAG}) should not be blank") +} + func TestFromWithUndefinedArg(t *testing.T) { tag, expected := "sometag", "expectedthisid"