From c6249de16a3bd6053011138a9c3702251b160840 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Thu, 15 Jun 2017 12:12:31 -0400 Subject: [PATCH 01/21] Enable dm deferred_* with version check This enables deferred device deletion/removal by default if the driver version in the kernel is new enough to support the feature. Signed-off-by: Brian Goff Upstream-commit: 0dc1a80565d522fc2cc7c65c3ad2d8ed83aeaf0f Component: engine --- .../daemon/graphdriver/devmapper/deviceset.go | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/components/engine/daemon/graphdriver/devmapper/deviceset.go b/components/engine/daemon/graphdriver/devmapper/deviceset.go index 49ea386f4a..c0116c7993 100644 --- a/components/engine/daemon/graphdriver/devmapper/deviceset.go +++ b/components/engine/daemon/graphdriver/devmapper/deviceset.go @@ -1693,16 +1693,6 @@ func (devices *DeviceSet) initDevmapper(doInit bool) (retErr error) { // give ourselves to libdm as a log handler devicemapper.LogInit(devices) - version, err := devicemapper.GetDriverVersion() - if err != nil { - // Can't even get driver version, assume not supported - return graphdriver.ErrNotSupported - } - - if err := determineDriverCapabilities(version); err != nil { - return graphdriver.ErrNotSupported - } - if err := devices.enableDeferredRemovalDeletion(); err != nil { return err } @@ -2643,6 +2633,22 @@ func NewDeviceSet(root string, doInit bool, options []string, uidMaps, gidMaps [ minFreeSpacePercent: defaultMinFreeSpacePercent, } + version, err := devicemapper.GetDriverVersion() + if err != nil { + // Can't even get driver version, assume not supported + return nil, graphdriver.ErrNotSupported + } + + if err := determineDriverCapabilities(version); err != nil { + return nil, graphdriver.ErrNotSupported + } + + if driverDeferredRemovalSupport && devicemapper.LibraryDeferredRemovalSupport { + // enable deferred stuff by default + enableDeferredDeletion = true + enableDeferredRemoval = true + } + foundBlkDiscard := false var lvmSetupConfig directLVMConfig for _, option := range options { From 125062e5cfb03b7b8d515808279c72d04ce11c01 Mon Sep 17 00:00:00 2001 From: Brian Schwind Date: Thu, 1 Jun 2017 12:59:59 +0900 Subject: [PATCH 02/21] Add note to CopyToContainer documentation Signed-off-by: Brian Schwind Upstream-commit: 1c3071e487db96ba8274ac2388eca0002e9cee77 Component: engine --- components/engine/client/container_copy.go | 1 + 1 file changed, 1 insertion(+) diff --git a/components/engine/client/container_copy.go b/components/engine/client/container_copy.go index 545aa54383..b7f4872be7 100644 --- a/components/engine/client/container_copy.go +++ b/components/engine/client/container_copy.go @@ -30,6 +30,7 @@ func (cli *Client) ContainerStatPath(ctx context.Context, containerID, path stri } // CopyToContainer copies content into the container filesystem. +// Note that `content` must be a Reader for a TAR func (cli *Client) CopyToContainer(ctx context.Context, container, path string, content io.Reader, options types.CopyToContainerOptions) error { query := url.Values{} query.Set("path", filepath.ToSlash(path)) // Normalize the paths used in the API. From 862be7247cf7a0827a77c2ed4634c46646b5a59f Mon Sep 17 00:00:00 2001 From: Alexander Midlash Date: Thu, 15 Jun 2017 20:35:22 +0000 Subject: [PATCH 03/21] Update downloader script, to ensure that Authorization header is not passed when downloading blobs. Signed-off-by: Alexander Midlash Upstream-commit: 5c38e4c16a0a51a0cffb4802ecc694e5f5bfbeed Component: engine --- .../contrib/download-frozen-image-v2.sh | 24 ++++++++++++------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/components/engine/contrib/download-frozen-image-v2.sh b/components/engine/contrib/download-frozen-image-v2.sh index 43bbf7e3cf..ee0b8b41f7 100755 --- a/components/engine/contrib/download-frozen-image-v2.sh +++ b/components/engine/contrib/download-frozen-image-v2.sh @@ -44,6 +44,17 @@ if [ "$(go env GOHOSTOS)" = 'windows' ]; then fi fi +fetch_blob() { + url=$1 + token=$2 + dest=$3 + echo "Attempting to download blob $url" + target=$(curl -sS -v -H "Authorization: Bearer $token" "$url" 2>&1 | grep "Location:" | sed 's/< Location: \(.*\)\r/\1/') + # curl blob (exclude auth token) + curl -fsS --progress "${target}" -o "$dest" +} + + while [ $# -gt 0 ]; do imageTag="$1" shift @@ -87,10 +98,7 @@ while [ $# -gt 0 ]; do imageId="${configDigest#*:}" # strip off "sha256:" configFile="$imageId.json" - curl -fsSL \ - -H "Authorization: Bearer $token" \ - "https://registry-1.docker.io/v2/$image/blobs/$configDigest" \ - -o "$dir/$configFile" + fetch_blob "https://registry-1.docker.io/v2/$image/blobs/$configDigest" $token "$dir/$configFile" layersFs="$(echo "$manifestJson" | jq --raw-output --compact-output '.layers[]')" IFS="$newlineIFS" @@ -158,10 +166,7 @@ while [ $# -gt 0 ]; do continue fi token="$(curl -fsSL "https://auth.docker.io/token?service=registry.docker.io&scope=repository:$image:pull" | jq --raw-output '.token')" - curl -fSL --progress \ - -H "Authorization: Bearer $token" \ - "https://registry-1.docker.io/v2/$image/blobs/$layerDigest" \ - -o "$dir/$layerTar" + fetch_blob "https://registry-1.docker.io/v2/$image/blobs/$layerDigest" $token "$dir/$layerTar" ;; *) @@ -231,7 +236,8 @@ while [ $# -gt 0 ]; do continue fi token="$(curl -fsSL "https://auth.docker.io/token?service=registry.docker.io&scope=repository:$image:pull" | jq --raw-output '.token')" - curl -fSL --progress -H "Authorization: Bearer $token" "https://registry-1.docker.io/v2/$image/blobs/$imageLayer" -o "$dir/$layerId/layer.tar" # -C - + # find redirect using token: + fetch_blob "https://registry-1.docker.io/v2/$image/blobs/$imageLayer" $token "$dir/$layerId/layer.tar" done ;; From ae62a09f9b95197eef6a1c3d0999bca6324694b9 Mon Sep 17 00:00:00 2001 From: Tianon Gravi Date: Tue, 20 Jun 2017 10:56:13 -0700 Subject: [PATCH 04/21] Update "download-frozen-image-v2.sh" such that redirects are optional If the registry responds directly with blob contents, use them, otherwise follow the redirect without Authorization headers (which likely aren't valid for the server being redirected to). This preserves the basic structure of the previous output with up to one additional progress bar per-layer (for the redirect request and then the following blob request). Signed-off-by: Tianon Gravi Upstream-commit: 4bbdc0b8f7b2faa3354738c300cfedb2b77c7316 Component: engine --- .../contrib/download-frozen-image-v2.sh | 58 +++++++++++++------ 1 file changed, 41 insertions(+), 17 deletions(-) diff --git a/components/engine/contrib/download-frozen-image-v2.sh b/components/engine/contrib/download-frozen-image-v2.sh index ee0b8b41f7..232679019d 100755 --- a/components/engine/contrib/download-frozen-image-v2.sh +++ b/components/engine/contrib/download-frozen-image-v2.sh @@ -44,16 +44,41 @@ if [ "$(go env GOHOSTOS)" = 'windows' ]; then fi fi -fetch_blob() { - url=$1 - token=$2 - dest=$3 - echo "Attempting to download blob $url" - target=$(curl -sS -v -H "Authorization: Bearer $token" "$url" 2>&1 | grep "Location:" | sed 's/< Location: \(.*\)\r/\1/') - # curl blob (exclude auth token) - curl -fsS --progress "${target}" -o "$dest" -} +registryBase='https://registry-1.docker.io' +authBase='https://auth.docker.io' +authService='registry.docker.io' +# https://github.com/moby/moby/issues/33700 +fetch_blob() { + local token="$1"; shift + local image="$1"; shift + local digest="$1"; shift + local targetFile="$1"; shift + local curlArgs=( "$@" ) + + local curlHeaders="$( + curl -S "${curlArgs[@]}" \ + -H "Authorization: Bearer $token" \ + "$registryBase/v2/$image/blobs/$digest" \ + -o "$targetFile" \ + -D- + )" + curlHeaders="$(echo "$curlHeaders" | tr -d '\r')" + if [ "$(echo "$curlHeaders" | awk 'NR == 1 { print $2; exit }')" != '200' ]; then + rm -f "$targetFile" + + local blobRedirect="$(echo "$curlHeaders" | awk -F ': ' 'tolower($1) == "location" { print $2; exit }')" + if [ -z "$blobRedirect" ]; then + echo >&2 "error: failed fetching '$image' blob '$digest'" + echo "$curlHeaders" | head -1 >&2 + return 1 + fi + + curl -fSL "${curlArgs[@]}" \ + "$blobRedirect" \ + -o "$targetFile" + fi +} while [ $# -gt 0 ]; do imageTag="$1" @@ -70,14 +95,14 @@ while [ $# -gt 0 ]; do imageFile="${image//\//_}" # "/" can't be in filenames :) - token="$(curl -fsSL "https://auth.docker.io/token?service=registry.docker.io&scope=repository:$image:pull" | jq --raw-output '.token')" + token="$(curl -fsSL "$authBase/token?service=$authService&scope=repository:$image:pull" | jq --raw-output '.token')" manifestJson="$( curl -fsSL \ -H "Authorization: Bearer $token" \ -H 'Accept: application/vnd.docker.distribution.manifest.v2+json' \ -H 'Accept: application/vnd.docker.distribution.manifest.v1+json' \ - "https://registry-1.docker.io/v2/$image/manifests/$digest" + "$registryBase/v2/$image/manifests/$digest" )" if [ "${manifestJson:0:1}" != '{' ]; then echo >&2 "error: /v2/$image/manifests/$digest returned something unexpected:" @@ -98,7 +123,7 @@ while [ $# -gt 0 ]; do imageId="${configDigest#*:}" # strip off "sha256:" configFile="$imageId.json" - fetch_blob "https://registry-1.docker.io/v2/$image/blobs/$configDigest" $token "$dir/$configFile" + fetch_blob "$token" "$image" "$configDigest" "$dir/$configFile" -s layersFs="$(echo "$manifestJson" | jq --raw-output --compact-output '.layers[]')" IFS="$newlineIFS" @@ -165,8 +190,8 @@ while [ $# -gt 0 ]; do echo "skipping existing ${layerId:0:12}" continue fi - token="$(curl -fsSL "https://auth.docker.io/token?service=registry.docker.io&scope=repository:$image:pull" | jq --raw-output '.token')" - fetch_blob "https://registry-1.docker.io/v2/$image/blobs/$layerDigest" $token "$dir/$layerTar" + token="$(curl -fsSL "$authBase/token?service=$authService&scope=repository:$image:pull" | jq --raw-output '.token')" + fetch_blob "$token" "$image" "$layerDigest" "$dir/$layerTar" --progress ;; *) @@ -235,9 +260,8 @@ while [ $# -gt 0 ]; do echo "skipping existing ${layerId:0:12}" continue fi - token="$(curl -fsSL "https://auth.docker.io/token?service=registry.docker.io&scope=repository:$image:pull" | jq --raw-output '.token')" - # find redirect using token: - fetch_blob "https://registry-1.docker.io/v2/$image/blobs/$imageLayer" $token "$dir/$layerId/layer.tar" + token="$(curl -fsSL "$authBase/token?service=$authService&scope=repository:$image:pull" | jq --raw-output '.token')" + fetch_blob "$token" "$image" "$imageLayer" "$dir/$layerId/layer.tar" --progress done ;; From a79192f925388e9edd7371342b123c3d384844a1 Mon Sep 17 00:00:00 2001 From: Raja Sami Date: Thu, 15 Jun 2017 13:05:40 +0500 Subject: [PATCH 05/21] Increase test Coverage of pkg/authorization Signed-off-by: Raja Sami Upstream-commit: f1eb0c0ebb85af2ae5373f16fd529588c07815cc Component: engine --- .../engine/pkg/authorization/api_test.go | 75 +++++++++++++++++++ .../pkg/authorization/middleware_test.go | 53 +++++++++++++ .../pkg/authorization/middleware_unix_test.go | 65 ++++++++++++++++ 3 files changed, 193 insertions(+) create mode 100644 components/engine/pkg/authorization/api_test.go create mode 100644 components/engine/pkg/authorization/middleware_test.go create mode 100644 components/engine/pkg/authorization/middleware_unix_test.go diff --git a/components/engine/pkg/authorization/api_test.go b/components/engine/pkg/authorization/api_test.go new file mode 100644 index 0000000000..1031949069 --- /dev/null +++ b/components/engine/pkg/authorization/api_test.go @@ -0,0 +1,75 @@ +package authorization + +import ( + "crypto/rand" + "crypto/rsa" + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" + "math/big" + "net/http" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +func TestPeerCertificateMarshalJSON(t *testing.T) { + template := &x509.Certificate{ + IsCA: true, + BasicConstraintsValid: true, + SubjectKeyId: []byte{1, 2, 3}, + SerialNumber: big.NewInt(1234), + Subject: pkix.Name{ + Country: []string{"Earth"}, + Organization: []string{"Mother Nature"}, + }, + NotBefore: time.Now(), + NotAfter: time.Now().AddDate(5, 5, 5), + + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth, x509.ExtKeyUsageServerAuth}, + KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, + } + // generate private key + privatekey, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + publickey := &privatekey.PublicKey + + // create a self-signed certificate. template = parent + var parent = template + raw, err := x509.CreateCertificate(rand.Reader, template, parent, publickey, privatekey) + require.NoError(t, err) + + cert, err := x509.ParseCertificate(raw) + require.NoError(t, err) + + var certs = []*x509.Certificate{cert} + addr := "www.authz.com/auth" + req, err := http.NewRequest("GET", addr, nil) + require.NoError(t, err) + + req.RequestURI = addr + req.TLS = &tls.ConnectionState{} + req.TLS.PeerCertificates = certs + req.Header.Add("header", "value") + + for _, c := range req.TLS.PeerCertificates { + pcObj := PeerCertificate(*c) + + t.Run("Marshalling :", func(t *testing.T) { + raw, err = pcObj.MarshalJSON() + require.NotNil(t, raw) + require.Nil(t, err) + }) + + t.Run("UnMarshalling :", func(t *testing.T) { + err := pcObj.UnmarshalJSON(raw) + require.Nil(t, err) + require.Equal(t, "Earth", pcObj.Subject.Country[0]) + require.Equal(t, true, pcObj.IsCA) + + }) + + } + +} diff --git a/components/engine/pkg/authorization/middleware_test.go b/components/engine/pkg/authorization/middleware_test.go new file mode 100644 index 0000000000..fc7401135c --- /dev/null +++ b/components/engine/pkg/authorization/middleware_test.go @@ -0,0 +1,53 @@ +package authorization + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/docker/docker/pkg/plugingetter" + "github.com/stretchr/testify/require" +) + +func TestMiddleware(t *testing.T) { + pluginNames := []string{"testPlugin1", "testPlugin2"} + var pluginGetter plugingetter.PluginGetter + m := NewMiddleware(pluginNames, pluginGetter) + authPlugins := m.getAuthzPlugins() + require.Equal(t, 2, len(authPlugins)) + require.EqualValues(t, pluginNames[0], authPlugins[0].Name()) + require.EqualValues(t, pluginNames[1], authPlugins[1].Name()) +} + +func TestNewResponseModifier(t *testing.T) { + recorder := httptest.NewRecorder() + modifier := NewResponseModifier(recorder) + modifier.Header().Set("H1", "V1") + modifier.Write([]byte("body")) + require.False(t, modifier.Hijacked()) + modifier.WriteHeader(http.StatusInternalServerError) + require.NotNil(t, modifier.RawBody()) + + raw, err := modifier.RawHeaders() + require.NotNil(t, raw) + require.Nil(t, err) + + headerData := strings.Split(strings.TrimSpace(string(raw)), ":") + require.EqualValues(t, "H1", strings.TrimSpace(headerData[0])) + require.EqualValues(t, "V1", strings.TrimSpace(headerData[1])) + + modifier.Flush() + modifier.FlushAll() + + if recorder.Header().Get("H1") != "V1" { + t.Fatalf("Header value must exists %s", recorder.Header().Get("H1")) + } + +} + +func setAuthzPlugins(m *Middleware, plugins []Plugin) { + m.mu.Lock() + m.plugins = plugins + m.mu.Unlock() +} diff --git a/components/engine/pkg/authorization/middleware_unix_test.go b/components/engine/pkg/authorization/middleware_unix_test.go new file mode 100644 index 0000000000..fd684f1208 --- /dev/null +++ b/components/engine/pkg/authorization/middleware_unix_test.go @@ -0,0 +1,65 @@ +// +build !windows + +package authorization + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/docker/docker/pkg/plugingetter" + "github.com/stretchr/testify/require" + "golang.org/x/net/context" +) + +func TestMiddlewareWrapHandler(t *testing.T) { + server := authZPluginTestServer{t: t} + server.start() + defer server.stop() + + authZPlugin := createTestPlugin(t) + pluginNames := []string{authZPlugin.name} + + var pluginGetter plugingetter.PluginGetter + middleWare := NewMiddleware(pluginNames, pluginGetter) + handler := func(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { + return nil + } + + authList := []Plugin{authZPlugin} + middleWare.SetPlugins([]string{"My Test Plugin"}) + setAuthzPlugins(middleWare, authList) + mdHandler := middleWare.WrapHandler(handler) + require.NotNil(t, mdHandler) + + addr := "www.example.com/auth" + req, _ := http.NewRequest("GET", addr, nil) + req.RequestURI = addr + req.Header.Add("header", "value") + + resp := httptest.NewRecorder() + ctx := context.Background() + + t.Run("Error Test Case :", func(t *testing.T) { + server.replayResponse = Response{ + Allow: false, + Msg: "Server Auth Not Allowed", + } + if err := mdHandler(ctx, resp, req, map[string]string{}); err == nil { + require.Error(t, err) + } + + }) + + t.Run("Positive Test Case :", func(t *testing.T) { + server.replayResponse = Response{ + Allow: true, + Msg: "Server Auth Allowed", + } + if err := mdHandler(ctx, resp, req, map[string]string{}); err != nil { + require.NoError(t, err) + } + + }) + +} From 9a5b3d1ecfd9e44a8efe36624665afd597f655c7 Mon Sep 17 00:00:00 2001 From: Raja Sami Date: Wed, 14 Jun 2017 16:49:44 +0500 Subject: [PATCH 06/21] Increase test Coverage of pkg/templates Signed-off-by: Raja Sami Upstream-commit: 27e771b714b599c449c8fb8f01547454912aeb28 Component: engine --- .../engine/pkg/templates/templates_test.go | 24 ++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/components/engine/pkg/templates/templates_test.go b/components/engine/pkg/templates/templates_test.go index 7f14802454..296bcb7107 100644 --- a/components/engine/pkg/templates/templates_test.go +++ b/components/engine/pkg/templates/templates_test.go @@ -57,14 +57,32 @@ func TestParseTruncateFunction(t *testing.T) { template: `{{truncate . 30}}`, expected: "tupx5xzf6hvsrhnruz5cr8gwp", }, + { + template: `{{pad . 3 3}}`, + expected: " tupx5xzf6hvsrhnruz5cr8gwp ", + }, } for _, testCase := range testCases { tm, err := Parse(testCase.template) assert.NoError(t, err) - var b bytes.Buffer - assert.NoError(t, tm.Execute(&b, source)) - assert.Equal(t, testCase.expected, b.String()) + t.Run("Non Empty Source Test with template: "+testCase.template, func(t *testing.T) { + var b bytes.Buffer + assert.NoError(t, tm.Execute(&b, source)) + assert.Equal(t, testCase.expected, b.String()) + }) + + t.Run("Empty Source Test with template: "+testCase.template, func(t *testing.T) { + var c bytes.Buffer + assert.NoError(t, tm.Execute(&c, "")) + assert.Equal(t, "", c.String()) + }) + + t.Run("Nil Source Test with template: "+testCase.template, func(t *testing.T) { + var c bytes.Buffer + assert.Error(t, tm.Execute(&c, nil)) + assert.Equal(t, "", c.String()) + }) } } From 2c8425881b86acf060258d40176dc0d200e4ad19 Mon Sep 17 00:00:00 2001 From: Kenfe-Mickael Laventure Date: Wed, 21 Jun 2017 17:36:19 -0700 Subject: [PATCH 07/21] Prevent a goroutine leak when healthcheck gets stopped Signed-off-by: Kenfe-Mickael Laventure Upstream-commit: 67297ba0051d39be544009ba76abea14bc0be8a4 Component: engine --- components/engine/daemon/health.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/components/engine/daemon/health.go b/components/engine/daemon/health.go index 48cf4c4255..b95b72a3c2 100644 --- a/components/engine/daemon/health.go +++ b/components/engine/daemon/health.go @@ -186,7 +186,7 @@ func monitor(d *Daemon, c *container.Container, stop chan struct{}, probe probe) logrus.Debugf("Running health check for container %s ...", c.ID) startTime := time.Now() ctx, cancelProbe := context.WithTimeout(context.Background(), probeTimeout) - results := make(chan *types.HealthcheckResult) + results := make(chan *types.HealthcheckResult, 1) go func() { healthChecksCounter.Inc() result, err := probe.run(ctx, d, c) @@ -209,8 +209,10 @@ func monitor(d *Daemon, c *container.Container, stop chan struct{}, probe probe) select { case <-stop: logrus.Debugf("Stop healthcheck monitoring for container %s (received while probing)", c.ID) - // Stop timeout and kill probe, but don't wait for probe to exit. cancelProbe() + // Wait for probe to exit (it might take a while to respond to the TERM + // signal and we don't want dying probes to pile up). + <-results return case result := <-results: handleProbeResult(d, c, result, stop) From d2f6c7daeefbb55fb34a5db04388c9ebd98d9080 Mon Sep 17 00:00:00 2001 From: Harald Albers Date: Fri, 23 Jun 2017 18:05:38 +0200 Subject: [PATCH 08/21] Add bash completion to the development container Adds an environment variable `DOCKER_BASH_COMPLETION_PATH` that allows to include a bash completion script into the development container. This is needid for development of bash completion. Signed-off-by: Harald Albers Upstream-commit: 0e2c424a7f32945a47e271cb804b091c83429912 Component: engine --- components/engine/Dockerfile | 3 +++ components/engine/Makefile | 4 +++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/components/engine/Dockerfile b/components/engine/Dockerfile index a1e7a2a691..afc7f5f639 100644 --- a/components/engine/Dockerfile +++ b/components/engine/Dockerfile @@ -217,6 +217,9 @@ COPY hack/dockerfile/install-binaries.sh /tmp/install-binaries.sh RUN /tmp/install-binaries.sh tomlv vndr runc containerd tini proxy dockercli ENV PATH=/usr/local/cli:$PATH +# Activate bash completion if mounted with DOCKER_BASH_COMPLETION_PATH +RUN ln -s /usr/local/completion/bash/docker /etc/bash_completion.d/docker + # Wrap all commands in the "docker-in-docker" script to allow nested containers ENTRYPOINT ["hack/dind"] diff --git a/components/engine/Makefile b/components/engine/Makefile index c2078fd579..526d6405ea 100644 --- a/components/engine/Makefile +++ b/components/engine/Makefile @@ -24,6 +24,7 @@ DOCKER_ENVS := \ -e DOCKER_BUILD_ARGS \ -e DOCKER_BUILD_GOGC \ -e DOCKER_BUILD_PKGS \ + -e DOCKER_BASH_COMPLETION_PATH \ -e DOCKER_CLI_PATH \ -e DOCKER_DEBUG \ -e DOCKER_EXPERIMENTAL \ @@ -65,7 +66,8 @@ PKGCACHE_VOLROOT := dockerdev-go-pkg-cache PKGCACHE_VOL := $(if $(PKGCACHE_DIR),$(CURDIR)/$(PKGCACHE_DIR)/,$(PKGCACHE_VOLROOT)-) DOCKER_MOUNT_PKGCACHE := $(if $(DOCKER_INCREMENTAL_BINARY),$(shell echo $(PKGCACHE_MAP) | sed -E 's@([^ ]*)@-v "$(PKGCACHE_VOL)\1"@g'),) DOCKER_MOUNT_CLI := $(if $(DOCKER_CLI_PATH),-v $(shell dirname $(DOCKER_CLI_PATH)):/usr/local/cli,) -DOCKER_MOUNT := $(DOCKER_MOUNT) $(DOCKER_MOUNT_PKGCACHE) $(DOCKER_MOUNT_CLI) +DOCKER_MOUNT_BASH_COMPLETION := $(if $(DOCKER_BASH_COMPLETION_PATH),-v $(shell dirname $(DOCKER_BASH_COMPLETION_PATH)):/usr/local/completion/bash,) +DOCKER_MOUNT := $(DOCKER_MOUNT) $(DOCKER_MOUNT_PKGCACHE) $(DOCKER_MOUNT_CLI) $(DOCKER_MOUNT_BASH_COMPLETION) GIT_BRANCH := $(shell git rev-parse --abbrev-ref HEAD 2>/dev/null) GIT_BRANCH_CLEAN := $(shell echo $(GIT_BRANCH) | sed -e "s/[^[:alnum:]]/-/g") From 78a82a7623d891f96077b60be7b9dd238a104ec5 Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Mon, 5 Dec 2016 18:55:07 -0800 Subject: [PATCH 09/21] Warn/deprecate continuing on empty lines in `Dockerfile` This fix is related to 29005 and 24693. Currently in `Dockerfile` empty lines will continue as long as there is line escape before. This may cause some issues. The issue in 24693 is an example. A non-empty line after an empty line might be considered to be a separate instruction by many users. However, it is actually part of the last instruction under the current `Dockerfile` parsing rule. This fix is an effort to reduce the confusion around the parsing of `Dockerfile`. Even though this fix does not change the behavior of the `Dockerfile` parsing, it tries to deprecate the empty line continuation and present a warning for the user. In this case, at least it prompt users to check for the Dockerfile and avoid the confusion if possible. Signed-off-by: Yong Tang Upstream-commit: 7815c8f8754d5473eda7cd80277a4ea3c59e3c29 Component: engine --- .../builder/dockerfile/parser/parser.go | 24 +++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/components/engine/builder/dockerfile/parser/parser.go b/components/engine/builder/dockerfile/parser/parser.go index a1f7a74570..6f6df9893d 100644 --- a/components/engine/builder/dockerfile/parser/parser.go +++ b/components/engine/builder/dockerfile/parser/parser.go @@ -243,6 +243,7 @@ type Result struct { AST *Node EscapeToken rune Platform string + Warnings []string } // Parse reads lines from a Reader, parses the lines into an AST and returns @@ -252,6 +253,7 @@ func Parse(rwc io.Reader) (*Result, error) { currentLine := 0 root := &Node{StartLine: -1} scanner := bufio.NewScanner(rwc) + warnings := []string{} var err error for scanner.Scan() { @@ -272,6 +274,7 @@ func Parse(rwc io.Reader) (*Result, error) { continue } + var hasEmptyContinuationLine bool for !isEndOfLine && scanner.Scan() { bytesRead, err := processLine(d, scanner.Bytes(), false) if err != nil { @@ -279,8 +282,8 @@ func Parse(rwc io.Reader) (*Result, error) { } currentLine++ - // TODO: warn this is being deprecated/removed if isEmptyContinuationLine(bytesRead) { + hasEmptyContinuationLine = true continue } @@ -289,13 +292,27 @@ func Parse(rwc io.Reader) (*Result, error) { line += continuationLine } + if hasEmptyContinuationLine { + warning := "[WARNING]: Empty continuation line found in:\n " + line + warnings = append(warnings, warning) + } + child, err := newNodeFromLine(line, d) if err != nil { return nil, err } root.AddChild(child, startLine, currentLine) } - return &Result{AST: root, EscapeToken: d.escapeToken, Platform: d.platformToken}, nil + + if len(warnings) > 0 { + warnings = append(warnings, "[WARNING]: Empty continuation lines will become errors in a future release.") + } + return &Result{ + AST: root, + Warnings: warnings, + EscapeToken: d.escapeToken, + Platform: d.platformToken, + }, nil } func trimComments(src []byte) []byte { @@ -326,6 +343,5 @@ func processLine(d *Directive, token []byte, stripLeftWhitespace bool) ([]byte, if stripLeftWhitespace { token = trimWhitespace(token) } - err := d.possibleParserDirective(string(token)) - return trimComments(token), err + return trimComments(token), d.possibleParserDirective(string(token)) } From 0a2d82735e511cf3a647e3bc106c8fd2dbf1e13d Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 16 Jun 2017 15:05:30 -0700 Subject: [PATCH 10/21] Add a test for warning on empty continuation lines. Signed-off-by: Daniel Nephin Upstream-commit: b47b375cb8bb0dca7ee0ebfa093bfc163ad867fb Component: engine --- .../engine/builder/dockerfile/builder.go | 1 + .../builder/dockerfile/parser/parser.go | 10 +++- .../builder/dockerfile/parser/parser_test.go | 57 ++++++++++++------- .../engine/builder/remotecontext/detect.go | 14 ++--- 4 files changed, 53 insertions(+), 29 deletions(-) diff --git a/components/engine/builder/dockerfile/builder.go b/components/engine/builder/dockerfile/builder.go index 80cfaf0bc2..cee1436f92 100644 --- a/components/engine/builder/dockerfile/builder.go +++ b/components/engine/builder/dockerfile/builder.go @@ -255,6 +255,7 @@ func (b *Builder) build(source builder.Source, dockerfile *parser.Result) (*buil return nil, errors.Errorf("failed to reach build target %s in Dockerfile", b.options.Target) } + dockerfile.PrintWarnings(b.Stderr) b.buildArgs.WarnOnUnusedBuildArgs(b.Stderr) if dispatchState.imageID == "" { diff --git a/components/engine/builder/dockerfile/parser/parser.go b/components/engine/builder/dockerfile/parser/parser.go index 6f6df9893d..9766fbfdfe 100644 --- a/components/engine/builder/dockerfile/parser/parser.go +++ b/components/engine/builder/dockerfile/parser/parser.go @@ -246,6 +246,14 @@ type Result struct { Warnings []string } +// PrintWarnings to the writer +func (r *Result) PrintWarnings(out io.Writer) { + if len(r.Warnings) == 0 { + return + } + fmt.Fprintf(out, strings.Join(r.Warnings, "\n")+"\n") +} + // Parse reads lines from a Reader, parses the lines into an AST and returns // the AST and escape token func Parse(rwc io.Reader) (*Result, error) { @@ -311,7 +319,7 @@ func Parse(rwc io.Reader) (*Result, error) { AST: root, Warnings: warnings, EscapeToken: d.escapeToken, - Platform: d.platformToken, + Platform: d.platformToken, }, nil } diff --git a/components/engine/builder/dockerfile/parser/parser_test.go b/components/engine/builder/dockerfile/parser/parser_test.go index 99b614ba9f..bb057ecab3 100644 --- a/components/engine/builder/dockerfile/parser/parser_test.go +++ b/components/engine/builder/dockerfile/parser/parser_test.go @@ -27,40 +27,39 @@ func getDirs(t *testing.T, dir string) []string { return dirs } -func TestTestNegative(t *testing.T) { +func TestParseErrorCases(t *testing.T) { for _, dir := range getDirs(t, negativeTestDir) { dockerfile := filepath.Join(negativeTestDir, dir, "Dockerfile") df, err := os.Open(dockerfile) - require.NoError(t, err) + require.NoError(t, err, dockerfile) defer df.Close() _, err = Parse(df) - assert.Error(t, err) + assert.Error(t, err, dockerfile) } } -func TestTestData(t *testing.T) { +func TestParseCases(t *testing.T) { for _, dir := range getDirs(t, testDir) { dockerfile := filepath.Join(testDir, dir, "Dockerfile") resultfile := filepath.Join(testDir, dir, "result") df, err := os.Open(dockerfile) - require.NoError(t, err) + require.NoError(t, err, dockerfile) defer df.Close() result, err := Parse(df) - require.NoError(t, err) + require.NoError(t, err, dockerfile) content, err := ioutil.ReadFile(resultfile) - require.NoError(t, err) + require.NoError(t, err, resultfile) if runtime.GOOS == "windows" { // CRLF --> CR to match Unix behavior content = bytes.Replace(content, []byte{'\x0d', '\x0a'}, []byte{'\x0a'}, -1) } - - assert.Contains(t, result.AST.Dump()+"\n", string(content), "In "+dockerfile) + assert.Equal(t, result.AST.Dump()+"\n", string(content), "In "+dockerfile) } } @@ -106,7 +105,7 @@ func TestParseWords(t *testing.T) { } } -func TestLineInformation(t *testing.T) { +func TestParseIncludesLineNumbers(t *testing.T) { df, err := os.Open(testFileLineInfo) require.NoError(t, err) defer df.Close() @@ -115,10 +114,8 @@ func TestLineInformation(t *testing.T) { require.NoError(t, err) ast := result.AST - if ast.StartLine != 5 || ast.endLine != 31 { - fmt.Fprintf(os.Stderr, "Wrong root line information: expected(%d-%d), actual(%d-%d)\n", 5, 31, ast.StartLine, ast.endLine) - t.Fatal("Root line information doesn't match result.") - } + assert.Equal(t, 5, ast.StartLine) + assert.Equal(t, 31, ast.endLine) assert.Len(t, ast.Children, 3) expected := [][]int{ {5, 5}, @@ -126,10 +123,32 @@ func TestLineInformation(t *testing.T) { {17, 31}, } for i, child := range ast.Children { - if child.StartLine != expected[i][0] || child.endLine != expected[i][1] { - t.Logf("Wrong line information for child %d: expected(%d-%d), actual(%d-%d)\n", - i, expected[i][0], expected[i][1], child.StartLine, child.endLine) - t.Fatal("Root line information doesn't match result.") - } + msg := fmt.Sprintf("Child %d", i) + assert.Equal(t, expected[i], []int{child.StartLine, child.endLine}, msg) } } + +func TestParseWarnsOnEmptyContinutationLine(t *testing.T) { + dockerfile := bytes.NewBufferString(` +FROM alpine:3.6 + +RUN something \ + + following \ + + more + +RUN another \ + + thing + `) + + result, err := Parse(dockerfile) + require.NoError(t, err) + warnings := result.Warnings + assert.Len(t, warnings, 3) + assert.Contains(t, warnings[0], "Empty continuation line found in") + assert.Contains(t, warnings[0], "RUN something following more") + assert.Contains(t, warnings[1], "RUN another thing") + assert.Contains(t, warnings[2], "will become errors in a future release") +} diff --git a/components/engine/builder/remotecontext/detect.go b/components/engine/builder/remotecontext/detect.go index 889f0fce34..22ad5b7c9f 100644 --- a/components/engine/builder/remotecontext/detect.go +++ b/components/engine/builder/remotecontext/detect.go @@ -110,17 +110,13 @@ func newURLRemote(url string, dockerfilePath string, progressReader func(in io.R return progressReader(rc), nil }, }) - if err != nil { - if err == dockerfileFoundErr { - res, err := parser.Parse(dockerfile) - if err != nil { - return nil, nil, err - } - return nil, res, nil - } + switch { + case err == dockerfileFoundErr: + res, err := parser.Parse(dockerfile) + return nil, res, err + case err != nil: return nil, nil, err } - return withDockerfileFromContext(c.(modifiableContext), dockerfilePath) } From cbfdedc6bd15b8d75ffb87a7e1614f81020fd4ed Mon Sep 17 00:00:00 2001 From: Vincent Demeester Date: Fri, 23 Jun 2017 16:26:17 +0200 Subject: [PATCH 11/21] Move some `api` package functions away - DisplayablePorts is a `cli` function, moving to `docker/cli` - Move MatchesContentType to the only package using it, `api/server/httputils` (and remove the deps on logrus for `api` package) Signed-off-by: Vincent Demeester Upstream-commit: 565aa41392b01251dfc9398eb69c23bdd8ea64e6 Component: engine --- components/engine/api/common.go | 101 ------- components/engine/api/common_test.go | 264 ------------------ .../engine/api/server/httputils/httputils.go | 15 +- .../api/server/httputils/httputils_test.go | 18 ++ 4 files changed, 30 insertions(+), 368 deletions(-) create mode 100644 components/engine/api/server/httputils/httputils_test.go diff --git a/components/engine/api/common.go b/components/engine/api/common.go index 142b76966c..10de8eafc5 100644 --- a/components/engine/api/common.go +++ b/components/engine/api/common.go @@ -4,15 +4,9 @@ import ( "encoding/json" "encoding/pem" "fmt" - "mime" "os" "path/filepath" - "sort" - "strconv" - "strings" - "github.com/Sirupsen/logrus" - "github.com/docker/docker/api/types" "github.com/docker/docker/pkg/ioutils" "github.com/docker/docker/pkg/system" "github.com/docker/libtrust" @@ -28,101 +22,6 @@ const ( NoBaseImageSpecifier string = "scratch" ) -// byPortInfo is a temporary type used to sort types.Port by its fields -type byPortInfo []types.Port - -func (r byPortInfo) Len() int { return len(r) } -func (r byPortInfo) Swap(i, j int) { r[i], r[j] = r[j], r[i] } -func (r byPortInfo) Less(i, j int) bool { - if r[i].PrivatePort != r[j].PrivatePort { - return r[i].PrivatePort < r[j].PrivatePort - } - - if r[i].IP != r[j].IP { - return r[i].IP < r[j].IP - } - - if r[i].PublicPort != r[j].PublicPort { - return r[i].PublicPort < r[j].PublicPort - } - - return r[i].Type < r[j].Type -} - -// DisplayablePorts returns formatted string representing open ports of container -// e.g. "0.0.0.0:80->9090/tcp, 9988/tcp" -// it's used by command 'docker ps' -func DisplayablePorts(ports []types.Port) string { - type portGroup struct { - first uint16 - last uint16 - } - groupMap := make(map[string]*portGroup) - var result []string - var hostMappings []string - var groupMapKeys []string - sort.Sort(byPortInfo(ports)) - for _, port := range ports { - current := port.PrivatePort - portKey := port.Type - if port.IP != "" { - if port.PublicPort != current { - hostMappings = append(hostMappings, fmt.Sprintf("%s:%d->%d/%s", port.IP, port.PublicPort, port.PrivatePort, port.Type)) - continue - } - portKey = fmt.Sprintf("%s/%s", port.IP, port.Type) - } - group := groupMap[portKey] - - if group == nil { - groupMap[portKey] = &portGroup{first: current, last: current} - // record order that groupMap keys are created - groupMapKeys = append(groupMapKeys, portKey) - continue - } - if current == (group.last + 1) { - group.last = current - continue - } - - result = append(result, formGroup(portKey, group.first, group.last)) - groupMap[portKey] = &portGroup{first: current, last: current} - } - for _, portKey := range groupMapKeys { - g := groupMap[portKey] - result = append(result, formGroup(portKey, g.first, g.last)) - } - result = append(result, hostMappings...) - return strings.Join(result, ", ") -} - -func formGroup(key string, start, last uint16) string { - parts := strings.Split(key, "/") - groupType := parts[0] - var ip string - if len(parts) > 1 { - ip = parts[0] - groupType = parts[1] - } - group := strconv.Itoa(int(start)) - if start != last { - group = fmt.Sprintf("%s-%d", group, last) - } - if ip != "" { - group = fmt.Sprintf("%s:%s->%s", ip, group, group) - } - return fmt.Sprintf("%s/%s", group, groupType) -} - -// MatchesContentType validates the content type against the expected one -func MatchesContentType(contentType, expectedType string) bool { - mimetype, _, err := mime.ParseMediaType(contentType) - if err != nil { - logrus.Errorf("Error parsing media type: %s error: %v", contentType, err) - } - return err == nil && mimetype == expectedType -} - // LoadOrCreateTrustKey attempts to load the libtrust key at the given path, // otherwise generates a new one func LoadOrCreateTrustKey(trustKeyPath string) (libtrust.PrivateKey, error) { diff --git a/components/engine/api/common_test.go b/components/engine/api/common_test.go index 4d67206ffa..f466616b0f 100644 --- a/components/engine/api/common_test.go +++ b/components/engine/api/common_test.go @@ -6,272 +6,8 @@ import ( "testing" "os" - - "github.com/docker/docker/api/types" ) -type ports struct { - ports []types.Port - expected string -} - -// DisplayablePorts -func TestDisplayablePorts(t *testing.T) { - cases := []ports{ - { - []types.Port{ - { - PrivatePort: 9988, - Type: "tcp", - }, - }, - "9988/tcp"}, - { - []types.Port{ - { - PrivatePort: 9988, - Type: "udp", - }, - }, - "9988/udp", - }, - { - []types.Port{ - { - IP: "0.0.0.0", - PrivatePort: 9988, - Type: "tcp", - }, - }, - "0.0.0.0:0->9988/tcp", - }, - { - []types.Port{ - { - PrivatePort: 9988, - PublicPort: 8899, - Type: "tcp", - }, - }, - "9988/tcp", - }, - { - []types.Port{ - { - IP: "4.3.2.1", - PrivatePort: 9988, - PublicPort: 8899, - Type: "tcp", - }, - }, - "4.3.2.1:8899->9988/tcp", - }, - { - []types.Port{ - { - IP: "4.3.2.1", - PrivatePort: 9988, - PublicPort: 9988, - Type: "tcp", - }, - }, - "4.3.2.1:9988->9988/tcp", - }, - { - []types.Port{ - { - PrivatePort: 9988, - Type: "udp", - }, { - PrivatePort: 9988, - Type: "udp", - }, - }, - "9988/udp, 9988/udp", - }, - { - []types.Port{ - { - IP: "1.2.3.4", - PublicPort: 9998, - PrivatePort: 9998, - Type: "udp", - }, { - IP: "1.2.3.4", - PublicPort: 9999, - PrivatePort: 9999, - Type: "udp", - }, - }, - "1.2.3.4:9998-9999->9998-9999/udp", - }, - { - []types.Port{ - { - IP: "1.2.3.4", - PublicPort: 8887, - PrivatePort: 9998, - Type: "udp", - }, { - IP: "1.2.3.4", - PublicPort: 8888, - PrivatePort: 9999, - Type: "udp", - }, - }, - "1.2.3.4:8887->9998/udp, 1.2.3.4:8888->9999/udp", - }, - { - []types.Port{ - { - PrivatePort: 9998, - Type: "udp", - }, { - PrivatePort: 9999, - Type: "udp", - }, - }, - "9998-9999/udp", - }, - { - []types.Port{ - { - IP: "1.2.3.4", - PrivatePort: 6677, - PublicPort: 7766, - Type: "tcp", - }, { - PrivatePort: 9988, - PublicPort: 8899, - Type: "udp", - }, - }, - "9988/udp, 1.2.3.4:7766->6677/tcp", - }, - { - []types.Port{ - { - IP: "1.2.3.4", - PrivatePort: 9988, - PublicPort: 8899, - Type: "udp", - }, { - IP: "1.2.3.4", - PrivatePort: 9988, - PublicPort: 8899, - Type: "tcp", - }, { - IP: "4.3.2.1", - PrivatePort: 2233, - PublicPort: 3322, - Type: "tcp", - }, - }, - "4.3.2.1:3322->2233/tcp, 1.2.3.4:8899->9988/tcp, 1.2.3.4:8899->9988/udp", - }, - { - []types.Port{ - { - PrivatePort: 9988, - PublicPort: 8899, - Type: "udp", - }, { - IP: "1.2.3.4", - PrivatePort: 6677, - PublicPort: 7766, - Type: "tcp", - }, { - IP: "4.3.2.1", - PrivatePort: 2233, - PublicPort: 3322, - Type: "tcp", - }, - }, - "9988/udp, 4.3.2.1:3322->2233/tcp, 1.2.3.4:7766->6677/tcp", - }, - { - []types.Port{ - { - PrivatePort: 80, - Type: "tcp", - }, { - PrivatePort: 1024, - Type: "tcp", - }, { - PrivatePort: 80, - Type: "udp", - }, { - PrivatePort: 1024, - Type: "udp", - }, { - IP: "1.1.1.1", - PublicPort: 80, - PrivatePort: 1024, - Type: "tcp", - }, { - IP: "1.1.1.1", - PublicPort: 80, - PrivatePort: 1024, - Type: "udp", - }, { - IP: "1.1.1.1", - PublicPort: 1024, - PrivatePort: 80, - Type: "tcp", - }, { - IP: "1.1.1.1", - PublicPort: 1024, - PrivatePort: 80, - Type: "udp", - }, { - IP: "2.1.1.1", - PublicPort: 80, - PrivatePort: 1024, - Type: "tcp", - }, { - IP: "2.1.1.1", - PublicPort: 80, - PrivatePort: 1024, - Type: "udp", - }, { - IP: "2.1.1.1", - PublicPort: 1024, - PrivatePort: 80, - Type: "tcp", - }, { - IP: "2.1.1.1", - PublicPort: 1024, - PrivatePort: 80, - Type: "udp", - }, - }, - "80/tcp, 80/udp, 1024/tcp, 1024/udp, 1.1.1.1:1024->80/tcp, 1.1.1.1:1024->80/udp, 2.1.1.1:1024->80/tcp, 2.1.1.1:1024->80/udp, 1.1.1.1:80->1024/tcp, 1.1.1.1:80->1024/udp, 2.1.1.1:80->1024/tcp, 2.1.1.1:80->1024/udp", - }, - } - - for _, port := range cases { - actual := DisplayablePorts(port.ports) - if port.expected != actual { - t.Fatalf("Expected %s, got %s.", port.expected, actual) - } - } -} - -// MatchesContentType -func TestJsonContentType(t *testing.T) { - if !MatchesContentType("application/json", "application/json") { - t.Fail() - } - - if !MatchesContentType("application/json; charset=utf-8", "application/json") { - t.Fail() - } - - if MatchesContentType("dockerapplication/json", "application/json") { - t.Fail() - } -} - // LoadOrCreateTrustKey func TestLoadOrCreateTrustKeyInvalidKeyFile(t *testing.T) { tmpKeyFolderPath, err := ioutil.TempDir("", "api-trustkey-test") diff --git a/components/engine/api/server/httputils/httputils.go b/components/engine/api/server/httputils/httputils.go index 2c2e8452f9..92cb67c56c 100644 --- a/components/engine/api/server/httputils/httputils.go +++ b/components/engine/api/server/httputils/httputils.go @@ -3,12 +3,12 @@ package httputils import ( "fmt" "io" + "mime" "net/http" "strings" + "github.com/Sirupsen/logrus" "golang.org/x/net/context" - - "github.com/docker/docker/api" ) // APIVersionKey is the client's requested API version. @@ -55,7 +55,7 @@ func CheckForJSON(r *http.Request) error { } // Otherwise it better be json - if api.MatchesContentType(ct, "application/json") { + if matchesContentType(ct, "application/json") { return nil } return fmt.Errorf("Content-Type specified (%s) must be 'application/json'", ct) @@ -86,3 +86,12 @@ func VersionFromContext(ctx context.Context) string { return "" } + +// matchesContentType validates the content type against the expected one +func matchesContentType(contentType, expectedType string) bool { + mimetype, _, err := mime.ParseMediaType(contentType) + if err != nil { + logrus.Errorf("Error parsing media type: %s error: %v", contentType, err) + } + return err == nil && mimetype == expectedType +} diff --git a/components/engine/api/server/httputils/httputils_test.go b/components/engine/api/server/httputils/httputils_test.go new file mode 100644 index 0000000000..d551b9d98a --- /dev/null +++ b/components/engine/api/server/httputils/httputils_test.go @@ -0,0 +1,18 @@ +package httputils + +import "testing" + +// matchesContentType +func TestJsonContentType(t *testing.T) { + if !matchesContentType("application/json", "application/json") { + t.Fail() + } + + if !matchesContentType("application/json; charset=utf-8", "application/json") { + t.Fail() + } + + if matchesContentType("dockerapplication/json", "application/json") { + t.Fail() + } +} From ea9175efe5042c6b2639da4d68eaa533c4d43b5f Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 23 Jun 2017 23:04:39 -0700 Subject: [PATCH 12/21] Cleanup gitignore Signed-off-by: Sebastiaan van Stijn Upstream-commit: a7b24296c8ad7928ba56a4c2ecd675a34934177d Component: engine --- components/engine/.gitignore | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/components/engine/.gitignore b/components/engine/.gitignore index 3e5ed1f82b..943e7f3f68 100644 --- a/components/engine/.gitignore +++ b/components/engine/.gitignore @@ -15,21 +15,8 @@ autogen/ bundles/ cmd/dockerd/dockerd -cmd/docker/docker contrib/builder/rpm/*/changelog dockerversion/version_autogen.go dockerversion/version_autogen_unix.go -docs/AWS_S3_BUCKET -docs/GITCOMMIT -docs/GIT_BRANCH -docs/VERSION -docs/_build -docs/_static -docs/_templates -docs/changed-files -# generated by man/md2man-all.sh -man/man1 -man/man5 -man/man8 vendor/pkg/ hack/integration-cli-on-swarm/integration-cli-on-swarm From 3172848991296f555558575a064a5f0d8f19eb30 Mon Sep 17 00:00:00 2001 From: Vincent Demeester Date: Sat, 24 Jun 2017 12:40:49 +0200 Subject: [PATCH 13/21] Update go-connections dependency Signed-off-by: Vincent Demeester Upstream-commit: 497e6e0caaf805f8693c35cb45edbc8f734f09f2 Component: engine --- components/engine/vendor.conf | 2 +- .../docker/go-connections/tlsconfig/certpool_go17.go | 3 --- .../docker/go-connections/tlsconfig/certpool_other.go | 2 -- .../github.com/docker/go-connections/tlsconfig/config.go | 2 -- 4 files changed, 1 insertion(+), 8 deletions(-) diff --git a/components/engine/vendor.conf b/components/engine/vendor.conf index fc7f643b27..387df4aad2 100644 --- a/components/engine/vendor.conf +++ b/components/engine/vendor.conf @@ -17,7 +17,7 @@ github.com/vdemeester/shakers 24d7f1d6a71aa5d9cbe7390e4afb66b7eef9e1b3 golang.org/x/net 7dcfb8076726a3fdd9353b6b8a1f1b6be6811bd6 golang.org/x/sys 8f0908ab3b2457e2e15403d3697c9ef5cb4b57a9 github.com/docker/go-units 9e638d38cf6977a37a8ea0078f3ee75a7cdb2dd1 -github.com/docker/go-connections e15c02316c12de00874640cd76311849de2aeed5 +github.com/docker/go-connections 3ede32e2033de7505e6500d6c868c2b9ed9f169d golang.org/x/text f72d8390a633d5dfb0cc84043294db9f6c935756 github.com/stretchr/testify 4d4bfba8f1d1027c4fdbe371823030df51419987 github.com/pmezard/go-difflib v1.0.0 diff --git a/components/engine/vendor/github.com/docker/go-connections/tlsconfig/certpool_go17.go b/components/engine/vendor/github.com/docker/go-connections/tlsconfig/certpool_go17.go index 1d5fa4c76d..1ca0965e06 100644 --- a/components/engine/vendor/github.com/docker/go-connections/tlsconfig/certpool_go17.go +++ b/components/engine/vendor/github.com/docker/go-connections/tlsconfig/certpool_go17.go @@ -5,8 +5,6 @@ package tlsconfig import ( "crypto/x509" "runtime" - - "github.com/Sirupsen/logrus" ) // SystemCertPool returns a copy of the system cert pool, @@ -14,7 +12,6 @@ import ( func SystemCertPool() (*x509.CertPool, error) { certpool, err := x509.SystemCertPool() if err != nil && runtime.GOOS == "windows" { - logrus.Infof("Unable to use system certificate pool: %v", err) return x509.NewCertPool(), nil } return certpool, err diff --git a/components/engine/vendor/github.com/docker/go-connections/tlsconfig/certpool_other.go b/components/engine/vendor/github.com/docker/go-connections/tlsconfig/certpool_other.go index 262c95e8cd..9ca974539a 100644 --- a/components/engine/vendor/github.com/docker/go-connections/tlsconfig/certpool_other.go +++ b/components/engine/vendor/github.com/docker/go-connections/tlsconfig/certpool_other.go @@ -5,12 +5,10 @@ package tlsconfig import ( "crypto/x509" - "github.com/Sirupsen/logrus" ) // SystemCertPool returns an new empty cert pool, // accessing system cert pool is supported in go 1.7 func SystemCertPool() (*x509.CertPool, error) { - logrus.Warn("Unable to use system certificate pool: requires building with go 1.7 or later") return x509.NewCertPool(), nil } diff --git a/components/engine/vendor/github.com/docker/go-connections/tlsconfig/config.go b/components/engine/vendor/github.com/docker/go-connections/tlsconfig/config.go index ad4b112ab3..1b31bbb8b1 100644 --- a/components/engine/vendor/github.com/docker/go-connections/tlsconfig/config.go +++ b/components/engine/vendor/github.com/docker/go-connections/tlsconfig/config.go @@ -13,7 +13,6 @@ import ( "io/ioutil" "os" - "github.com/Sirupsen/logrus" "github.com/pkg/errors" ) @@ -106,7 +105,6 @@ func certPool(caFile string, exclusivePool bool) (*x509.CertPool, error) { if !certPool.AppendCertsFromPEM(pem) { return nil, fmt.Errorf("failed to append certificates from PEM file: %q", caFile) } - logrus.Debugf("Trusting %d certs", len(certPool.Subjects())) return certPool, nil } From a08baae7265f539a6c0b54766b9247c86b5c45cf Mon Sep 17 00:00:00 2001 From: John Starks Date: Thu, 22 Jun 2017 17:15:10 -0700 Subject: [PATCH 14/21] LCOW: pass command arguments without extra quoting Signed-off-by: John Starks Upstream-commit: 19645521a92869be7210fb663ed5e0e82e329ca5 Component: engine --- components/engine/daemon/exec_windows.go | 6 ++++-- components/engine/daemon/oci_windows.go | 2 +- components/engine/libcontainerd/client_windows.go | 6 +++++- components/engine/libcontainerd/container_windows.go | 6 +++++- 4 files changed, 15 insertions(+), 5 deletions(-) diff --git a/components/engine/daemon/exec_windows.go b/components/engine/daemon/exec_windows.go index 1d6974cda9..b7b45149cf 100644 --- a/components/engine/daemon/exec_windows.go +++ b/components/engine/daemon/exec_windows.go @@ -8,7 +8,9 @@ import ( func execSetPlatformOpt(c *container.Container, ec *exec.Config, p *libcontainerd.Process) error { // Process arguments need to be escaped before sending to OCI. - p.Args = escapeArgs(p.Args) - p.User.Username = ec.User + if c.Platform == "windows" { + p.Args = escapeArgs(p.Args) + p.User.Username = ec.User + } return nil } diff --git a/components/engine/daemon/oci_windows.go b/components/engine/daemon/oci_windows.go index 67e3473537..9f44c66523 100644 --- a/components/engine/daemon/oci_windows.go +++ b/components/engine/daemon/oci_windows.go @@ -98,7 +98,7 @@ func (daemon *Daemon) createSpec(c *container.Container) (*specs.Spec, error) { // In s.Process s.Process.Args = append([]string{c.Path}, c.Args...) - if !c.Config.ArgsEscaped { + if !c.Config.ArgsEscaped && img.OS == "windows" { s.Process.Args = escapeArgs(s.Process.Args) } diff --git a/components/engine/libcontainerd/client_windows.go b/components/engine/libcontainerd/client_windows.go index d7f312e6e4..be77ea6e55 100644 --- a/components/engine/libcontainerd/client_windows.go +++ b/components/engine/libcontainerd/client_windows.go @@ -420,7 +420,11 @@ func (clnt *client) AddProcess(ctx context.Context, containerID, processFriendly // Configure the environment for the process createProcessParms.Environment = setupEnvironmentVariables(procToAdd.Env) - createProcessParms.CommandLine = strings.Join(procToAdd.Args, " ") + if container.ociSpec.Platform.OS == "windows" { + createProcessParms.CommandLine = strings.Join(procToAdd.Args, " ") + } else { + createProcessParms.CommandArgs = procToAdd.Args + } createProcessParms.User = procToAdd.User.Username logrus.Debugf("libcontainerd: commandLine: %s", createProcessParms.CommandLine) diff --git a/components/engine/libcontainerd/container_windows.go b/components/engine/libcontainerd/container_windows.go index 3fd8b6793d..af3e0ef579 100644 --- a/components/engine/libcontainerd/container_windows.go +++ b/components/engine/libcontainerd/container_windows.go @@ -82,7 +82,11 @@ func (ctr *container) start(attachStdio StdioCallback) error { // Configure the environment for the process createProcessParms.Environment = setupEnvironmentVariables(ctr.ociSpec.Process.Env) - createProcessParms.CommandLine = strings.Join(ctr.ociSpec.Process.Args, " ") + if ctr.ociSpec.Platform.OS == "windows" { + createProcessParms.CommandLine = strings.Join(ctr.ociSpec.Process.Args, " ") + } else { + createProcessParms.CommandArgs = ctr.ociSpec.Process.Args + } createProcessParms.User = ctr.ociSpec.Process.User.Username // LCOW requires the raw OCI spec passed through HCS and onwards to GCS for the utility VM. From 8170be09ea33714185f7e7b065a9f80b59d3ca9b Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 15 Jun 2017 16:29:18 +0200 Subject: [PATCH 15/21] Fix handling of remote "git@" notation `docker build` accepts remote repositories using either the `git://` notation, or `git@`. Docker attempted to parse both as an URL, however, `git@` is not an URL, but an argument to `git clone`. Go 1.7 silently ignored this, and managed to extract the needed information from these remotes, however, Go 1.8 does a more strict validation, and invalidated these. This patch adds a different path for `git@` remotes, to prevent them from being handled as URL (and invalidated). A test is also added, because there were no tests for handling of `git@` remotes. Signed-off-by: Sebastiaan van Stijn Upstream-commit: 913eb99fdcd26a4106250bd40dfe8b9c18564b23 Component: engine --- .../builder/remotecontext/git/gitutils.go | 61 ++++++++++++++----- .../remotecontext/git/gitutils_test.go | 39 ++++++++++-- 2 files changed, 81 insertions(+), 19 deletions(-) diff --git a/components/engine/builder/remotecontext/git/gitutils.go b/components/engine/builder/remotecontext/git/gitutils.go index fd8250dbc3..91a0f5baea 100644 --- a/components/engine/builder/remotecontext/git/gitutils.go +++ b/components/engine/builder/remotecontext/git/gitutils.go @@ -15,18 +15,24 @@ import ( "github.com/pkg/errors" ) +type gitRepo struct { + remote string + ref string + subdir string +} + // Clone clones a repository into a newly created directory which // will be under "docker-build-git" func Clone(remoteURL string) (string, error) { - if !urlutil.IsGitTransport(remoteURL) { - remoteURL = "https://" + remoteURL - } - root, err := ioutil.TempDir("", "docker-build-git") + repo, err := parseRemoteURL(remoteURL) + if err != nil { return "", err } - u, err := url.Parse(remoteURL) + fetch := fetchArgs(repo.remote, repo.ref) + + root, err := ioutil.TempDir("", "docker-build-git") if err != nil { return "", err } @@ -35,22 +41,47 @@ func Clone(remoteURL string) (string, error) { return "", errors.Wrapf(err, "failed to init repo at %s: %s", root, out) } - ref, subdir := getRefAndSubdir(u.Fragment) - fetch := fetchArgs(u, ref) - - u.Fragment = "" - // Add origin remote for compatibility with previous implementation that // used "git clone" and also to make sure local refs are created for branches - if out, err := gitWithinDir(root, "remote", "add", "origin", u.String()); err != nil { - return "", errors.Wrapf(err, "failed add origin repo at %s: %s", u.String(), out) + if out, err := gitWithinDir(root, "remote", "add", "origin", repo.remote); err != nil { + return "", errors.Wrapf(err, "failed add origin repo at %s: %s", repo.remote, out) } if output, err := gitWithinDir(root, fetch...); err != nil { return "", errors.Wrapf(err, "error fetching: %s", output) } - return checkoutGit(root, ref, subdir) + return checkoutGit(root, repo.ref, repo.subdir) +} + +func parseRemoteURL(remoteURL string) (gitRepo, error) { + repo := gitRepo{} + + if !urlutil.IsGitTransport(remoteURL) { + remoteURL = "https://" + remoteURL + } + + var fragment string + if strings.HasPrefix(remoteURL, "git@") { + // git@.. is not an URL, so cannot be parsed as URL + parts := strings.SplitN(remoteURL, "#", 2) + + repo.remote = parts[0] + if len(parts) == 2 { + fragment = parts[1] + } + repo.ref, repo.subdir = getRefAndSubdir(fragment) + } else { + u, err := url.Parse(remoteURL) + if err != nil { + return repo, err + } + + repo.ref, repo.subdir = getRefAndSubdir(u.Fragment) + u.Fragment = "" + repo.remote = u.String() + } + return repo, nil } func getRefAndSubdir(fragment string) (ref string, subdir string) { @@ -65,11 +96,11 @@ func getRefAndSubdir(fragment string) (ref string, subdir string) { return } -func fetchArgs(remoteURL *url.URL, ref string) []string { +func fetchArgs(remoteURL string, ref string) []string { args := []string{"fetch", "--recurse-submodules=yes"} shallow := true - if strings.HasPrefix(remoteURL.Scheme, "http") { + if urlutil.IsURL(remoteURL) { res, err := http.Head(fmt.Sprintf("%s/info/refs?service=git-upload-pack", remoteURL)) if err != nil || res.Header.Get("Content-Type") != "application/x-git-upload-pack-advertisement" { shallow = false diff --git a/components/engine/builder/remotecontext/git/gitutils_test.go b/components/engine/builder/remotecontext/git/gitutils_test.go index eb771f8c98..b6523a223e 100644 --- a/components/engine/builder/remotecontext/git/gitutils_test.go +++ b/components/engine/builder/remotecontext/git/gitutils_test.go @@ -16,6 +16,38 @@ import ( "github.com/stretchr/testify/require" ) +func TestParseRemoteURL(t *testing.T) { + dir, err := parseRemoteURL("git://github.com/user/repo.git") + require.NoError(t, err) + assert.NotEmpty(t, dir) + assert.Equal(t, gitRepo{"git://github.com/user/repo.git", "master", ""}, dir) + + dir, err = parseRemoteURL("git://github.com/user/repo.git#mybranch:mydir/mysubdir/") + require.NoError(t, err) + assert.NotEmpty(t, dir) + assert.Equal(t, gitRepo{"git://github.com/user/repo.git", "mybranch", "mydir/mysubdir/"}, dir) + + dir, err = parseRemoteURL("https://github.com/user/repo.git") + require.NoError(t, err) + assert.NotEmpty(t, dir) + assert.Equal(t, gitRepo{"https://github.com/user/repo.git", "master", ""}, dir) + + dir, err = parseRemoteURL("https://github.com/user/repo.git#mybranch:mydir/mysubdir/") + require.NoError(t, err) + assert.NotEmpty(t, dir) + assert.Equal(t, gitRepo{"https://github.com/user/repo.git", "mybranch", "mydir/mysubdir/"}, dir) + + dir, err = parseRemoteURL("git@github.com:user/repo.git") + require.NoError(t, err) + assert.NotEmpty(t, dir) + assert.Equal(t, gitRepo{"git@github.com:user/repo.git", "master", ""}, dir) + + dir, err = parseRemoteURL("git@github.com:user/repo.git#mybranch:mydir/mysubdir/") + require.NoError(t, err) + assert.NotEmpty(t, dir) + assert.Equal(t, gitRepo{"git@github.com:user/repo.git", "mybranch", "mydir/mysubdir/"}, dir) +} + func TestCloneArgsSmartHttp(t *testing.T) { mux := http.NewServeMux() server := httptest.NewServer(mux) @@ -28,7 +60,7 @@ func TestCloneArgsSmartHttp(t *testing.T) { w.Header().Set("Content-Type", fmt.Sprintf("application/x-%s-advertisement", q)) }) - args := fetchArgs(serverURL, "master") + args := fetchArgs(serverURL.String(), "master") exp := []string{"fetch", "--recurse-submodules=yes", "--depth", "1", "origin", "master"} assert.Equal(t, exp, args) } @@ -44,14 +76,13 @@ func TestCloneArgsDumbHttp(t *testing.T) { w.Header().Set("Content-Type", "text/plain") }) - args := fetchArgs(serverURL, "master") + args := fetchArgs(serverURL.String(), "master") exp := []string{"fetch", "--recurse-submodules=yes", "origin", "master"} assert.Equal(t, exp, args) } func TestCloneArgsGit(t *testing.T) { - u, _ := url.Parse("git://github.com/docker/docker") - args := fetchArgs(u, "master") + args := fetchArgs("git://github.com/docker/docker", "master") exp := []string{"fetch", "--recurse-submodules=yes", "--depth", "1", "origin", "master"} assert.Equal(t, exp, args) } From a8f8b4ccd8bcac6a07795fe78594026f0a99c625 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 26 Jun 2017 10:07:04 -0700 Subject: [PATCH 16/21] Move IsGitTransport() to gitutils This function was only used inside gitutils, and is written specifically for the requirements there. Signed-off-by: Sebastiaan van Stijn Upstream-commit: d3d1aabcc68f65d40acbf1b3adc02d13997bb8e2 Component: engine --- .../builder/remotecontext/git/gitutils.go | 8 +++++- .../remotecontext/git/gitutils_test.go | 27 +++++++++++++++++++ components/engine/pkg/urlutil/urlutil.go | 6 ----- components/engine/pkg/urlutil/urlutil_test.go | 14 ---------- 4 files changed, 34 insertions(+), 21 deletions(-) diff --git a/components/engine/builder/remotecontext/git/gitutils.go b/components/engine/builder/remotecontext/git/gitutils.go index 91a0f5baea..b94d462cd5 100644 --- a/components/engine/builder/remotecontext/git/gitutils.go +++ b/components/engine/builder/remotecontext/git/gitutils.go @@ -57,7 +57,7 @@ func Clone(remoteURL string) (string, error) { func parseRemoteURL(remoteURL string) (gitRepo, error) { repo := gitRepo{} - if !urlutil.IsGitTransport(remoteURL) { + if !isGitTransport(remoteURL) { remoteURL = "https://" + remoteURL } @@ -151,3 +151,9 @@ func gitWithinDir(dir string, args ...string) ([]byte, error) { func git(args ...string) ([]byte, error) { return exec.Command("git", args...).CombinedOutput() } + +// isGitTransport returns true if the provided str is a git transport by inspecting +// the prefix of the string for known protocols used in git. +func isGitTransport(str string) bool { + return urlutil.IsURL(str) || strings.HasPrefix(str, "git://") || strings.HasPrefix(str, "git@") +} diff --git a/components/engine/builder/remotecontext/git/gitutils_test.go b/components/engine/builder/remotecontext/git/gitutils_test.go index b6523a223e..c638a498f2 100644 --- a/components/engine/builder/remotecontext/git/gitutils_test.go +++ b/components/engine/builder/remotecontext/git/gitutils_test.go @@ -209,3 +209,30 @@ func TestCheckoutGit(t *testing.T) { assert.Equal(t, c.exp, string(b)) } } + +func TestValidGitTransport(t *testing.T) { + gitUrls := []string{ + "git://github.com/docker/docker", + "git@github.com:docker/docker.git", + "git@bitbucket.org:atlassianlabs/atlassian-docker.git", + "https://github.com/docker/docker.git", + "http://github.com/docker/docker.git", + "http://github.com/docker/docker.git#branch", + "http://github.com/docker/docker.git#:dir", + } + incompleteGitUrls := []string{ + "github.com/docker/docker", + } + + for _, url := range gitUrls { + if !isGitTransport(url) { + t.Fatalf("%q should be detected as valid Git prefix", url) + } + } + + for _, url := range incompleteGitUrls { + if isGitTransport(url) { + t.Fatalf("%q should not be detected as valid Git prefix", url) + } + } +} diff --git a/components/engine/pkg/urlutil/urlutil.go b/components/engine/pkg/urlutil/urlutil.go index 44152873b1..cfcd582036 100644 --- a/components/engine/pkg/urlutil/urlutil.go +++ b/components/engine/pkg/urlutil/urlutil.go @@ -29,12 +29,6 @@ func IsGitURL(str string) bool { return checkURL(str, "git") } -// IsGitTransport returns true if the provided str is a git transport by inspecting -// the prefix of the string for known protocols used in git. -func IsGitTransport(str string) bool { - return IsURL(str) || strings.HasPrefix(str, "git://") || strings.HasPrefix(str, "git@") -} - // IsTransportURL returns true if the provided str is a transport (tcp, tcp+tls, udp, unix) URL. func IsTransportURL(str string) bool { return checkURL(str, "transport") diff --git a/components/engine/pkg/urlutil/urlutil_test.go b/components/engine/pkg/urlutil/urlutil_test.go index d84145a160..e7579f554f 100644 --- a/components/engine/pkg/urlutil/urlutil_test.go +++ b/components/engine/pkg/urlutil/urlutil_test.go @@ -27,20 +27,6 @@ var ( } ) -func TestValidGitTransport(t *testing.T) { - for _, url := range gitUrls { - if !IsGitTransport(url) { - t.Fatalf("%q should be detected as valid Git prefix", url) - } - } - - for _, url := range incompleteGitUrls { - if IsGitTransport(url) { - t.Fatalf("%q should not be detected as valid Git prefix", url) - } - } -} - func TestIsGIT(t *testing.T) { for _, url := range gitUrls { if !IsGitURL(url) { From f5f8240661f2e708967de1f62db2a448ed8239cd Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Thu, 2 Mar 2017 15:47:02 -0500 Subject: [PATCH 17/21] Set a LastUpdated time in image metadata when an image tag is updated. Signed-off-by: Daniel Nephin Signed-off-by: Sebastiaan van Stijn Upstream-commit: 016eea004ba0109d7cd965eab242ed17e4f0f36c Component: engine --- components/engine/api/swagger.yaml | 6 ++++++ components/engine/api/types/types.go | 6 ++++++ components/engine/daemon/image_inspect.go | 8 ++++++++ components/engine/daemon/image_tag.go | 3 +++ components/engine/docs/api/version-history.md | 1 + components/engine/image/store.go | 19 +++++++++++++++++++ components/engine/image/store_test.go | 18 ++++++++++++++++++ 7 files changed, 61 insertions(+) diff --git a/components/engine/api/swagger.yaml b/components/engine/api/swagger.yaml index b4c1bd8b15..fd0c4c1715 100644 --- a/components/engine/api/swagger.yaml +++ b/components/engine/api/swagger.yaml @@ -949,6 +949,12 @@ definitions: type: "string" BaseLayer: type: "string" + Metadata: + type: "object" + properties: + LastTagTime: + type: "string" + format: "dateTime" ImageSummary: type: "object" diff --git a/components/engine/api/types/types.go b/components/engine/api/types/types.go index 2473497c89..c96df27332 100644 --- a/components/engine/api/types/types.go +++ b/components/engine/api/types/types.go @@ -45,6 +45,12 @@ type ImageInspect struct { VirtualSize int64 GraphDriver GraphDriverData RootFS RootFS + Metadata ImageMetadata +} + +// ImageMetadata contains engine-local data about the image +type ImageMetadata struct { + LastTagTime time.Time `json:",omitempty"` } // Container contains response of Engine API: diff --git a/components/engine/daemon/image_inspect.go b/components/engine/daemon/image_inspect.go index 672a4cc012..3baf265dab 100644 --- a/components/engine/daemon/image_inspect.go +++ b/components/engine/daemon/image_inspect.go @@ -61,6 +61,11 @@ func (daemon *Daemon) LookupImage(name string) (*types.ImageInspect, error) { comment = img.History[len(img.History)-1].Comment } + lastUpdated, err := daemon.stores[platform].imageStore.GetLastUpdated(img.ID()) + if err != nil { + return nil, err + } + imageInspect := &types.ImageInspect{ ID: img.ID().String(), RepoTags: repoTags, @@ -79,6 +84,9 @@ func (daemon *Daemon) LookupImage(name string) (*types.ImageInspect, error) { Size: size, VirtualSize: size, // TODO: field unused, deprecate RootFS: rootFSToAPIType(img.RootFS), + Metadata: types.ImageMetadata{ + LastTagTime: lastUpdated, + }, } imageInspect.GraphDriver.Name = daemon.GraphDriverName(platform) diff --git a/components/engine/daemon/image_tag.go b/components/engine/daemon/image_tag.go index 199ca9ad84..5f28daed0a 100644 --- a/components/engine/daemon/image_tag.go +++ b/components/engine/daemon/image_tag.go @@ -32,6 +32,9 @@ func (daemon *Daemon) TagImageWithReference(imageID image.ID, platform string, n return err } + if err := daemon.stores[platform].imageStore.SetLastUpdated(imageID); err != nil { + return err + } daemon.LogImageEvent(imageID.String(), reference.FamiliarString(newTag), "tag") return nil } diff --git a/components/engine/docs/api/version-history.md b/components/engine/docs/api/version-history.md index 1d19d56dce..aa089ed0a1 100644 --- a/components/engine/docs/api/version-history.md +++ b/components/engine/docs/api/version-history.md @@ -25,6 +25,7 @@ keywords: "API, Docker, rcli, REST, documentation" * `POST /session` is a new endpoint that can be used for running interactive long-running protocols between client and the daemon. This endpoint is experimental and only available if the daemon is started with experimental features enabled. +* `GET /images/(name)/get` now includes an `ImageMetadata` field which contains image metadata that is local to the engine and not part of the image config. ## v1.30 API changes diff --git a/components/engine/image/store.go b/components/engine/image/store.go index 541b07bba1..b215d977f8 100644 --- a/components/engine/image/store.go +++ b/components/engine/image/store.go @@ -6,6 +6,7 @@ import ( "runtime" "strings" "sync" + "time" "github.com/Sirupsen/logrus" "github.com/docker/distribution/digestset" @@ -23,6 +24,8 @@ type Store interface { Search(partialID string) (ID, error) SetParent(id ID, parent ID) error GetParent(id ID) (ID, error) + SetLastUpdated(id ID) error + GetLastUpdated(id ID) (time.Time, error) Children(id ID) []ID Map() map[ID]*Image Heads() map[ID]*Image @@ -259,6 +262,22 @@ func (is *store) GetParent(id ID) (ID, error) { return ID(d), nil // todo: validate? } +// SetLastUpdated time for the image ID to the current time +func (is *store) SetLastUpdated(id ID) error { + lastUpdated := []byte(time.Now().Format(time.RFC3339Nano)) + return is.fs.SetMetadata(id.Digest(), "lastUpdated", lastUpdated) +} + +// GetLastUpdated time for the image ID +func (is *store) GetLastUpdated(id ID) (time.Time, error) { + bytes, err := is.fs.GetMetadata(id.Digest(), "lastUpdated") + if err != nil || len(bytes) == 0 { + // No lastUpdated time + return time.Time{}, nil + } + return time.Parse(time.RFC3339Nano, string(bytes)) +} + func (is *store) Children(id ID) []ID { is.RLock() defer is.RUnlock() diff --git a/components/engine/image/store_test.go b/components/engine/image/store_test.go index 13318cf221..fc6d461d99 100644 --- a/components/engine/image/store_test.go +++ b/components/engine/image/store_test.go @@ -149,6 +149,24 @@ func defaultImageStore(t *testing.T) (Store, func()) { return store, cleanup } +func TestGetAndSetLastUpdated(t *testing.T) { + store, cleanup := defaultImageStore(t) + defer cleanup() + + id, err := store.Create([]byte(`{"comment": "abc1", "rootfs": {"type": "layers"}}`)) + assert.NoError(t, err) + + updated, err := store.GetLastUpdated(id) + assert.NoError(t, err) + assert.Equal(t, updated.IsZero(), true) + + assert.NoError(t, store.SetLastUpdated(id)) + + updated, err = store.GetLastUpdated(id) + assert.NoError(t, err) + assert.Equal(t, updated.IsZero(), false) +} + type mockLayerGetReleaser struct{} func (ls *mockLayerGetReleaser) Get(layer.ChainID) (layer.Layer, error) { From 29123f2ea7ad2257959195c0d19f39af1791826e Mon Sep 17 00:00:00 2001 From: David Glasser Date: Mon, 26 Jun 2017 14:34:51 -0700 Subject: [PATCH 18/21] Fix stderr logging for journald and syslog logger.PutMessage, added in #28762 (v17.04.0-ce), clears msg.Source. So journald and syslog were treating stderr messages as if they were stdout. Signed-off-by: David Glasser Upstream-commit: 917050c5728f2fb9958ccb3ab66a23766f741adc Component: engine --- components/engine/daemon/logger/journald/journald.go | 3 ++- components/engine/daemon/logger/syslog/syslog.go | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/components/engine/daemon/logger/journald/journald.go b/components/engine/daemon/logger/journald/journald.go index 9865a273c5..86d7378b5b 100644 --- a/components/engine/daemon/logger/journald/journald.go +++ b/components/engine/daemon/logger/journald/journald.go @@ -112,9 +112,10 @@ func (s *journald) Log(msg *logger.Message) error { } line := string(msg.Line) + source := msg.Source logger.PutMessage(msg) - if msg.Source == "stderr" { + if source == "stderr" { return journal.Send(line, journal.PriErr, vars) } return journal.Send(line, journal.PriInfo, vars) diff --git a/components/engine/daemon/logger/syslog/syslog.go b/components/engine/daemon/logger/syslog/syslog.go index 925473aeef..42855e1174 100644 --- a/components/engine/daemon/logger/syslog/syslog.go +++ b/components/engine/daemon/logger/syslog/syslog.go @@ -133,8 +133,9 @@ func New(info logger.Info) (logger.Logger, error) { func (s *syslogger) Log(msg *logger.Message) error { line := string(msg.Line) + source := msg.Source logger.PutMessage(msg) - if msg.Source == "stderr" { + if source == "stderr" { return s.writer.Err(line) } return s.writer.Info(line) From 1c03f9a2499ce32309398735ba4800101cd7ee0f Mon Sep 17 00:00:00 2001 From: John Howard Date: Mon, 26 Jun 2017 21:22:50 -0700 Subject: [PATCH 19/21] Vendor github.com/jhowardmsft/opengcs v0.0.7 Signed-off-by: John Howard Upstream-commit: 3f14e25a7f67e0236dfdcde792ac571ca7874c47 Component: engine --- components/engine/vendor.conf | 2 +- .../opengcs/gogcs/client/config.go | 42 ++++++++++++------- .../opengcs/gogcs/client/createsandbox.go | 4 +- .../opengcs/gogcs/client/tartovhd.go | 4 +- .../opengcs/gogcs/client/utilities.go | 15 +------ 5 files changed, 34 insertions(+), 33 deletions(-) diff --git a/components/engine/vendor.conf b/components/engine/vendor.conf index 387df4aad2..bad511c4d4 100644 --- a/components/engine/vendor.conf +++ b/components/engine/vendor.conf @@ -8,7 +8,7 @@ github.com/docker/libtrust 9cbd2a1374f46905c68a4eb3694a130610adc62a github.com/go-check/check 4ed411733c5785b40214c70bce814c3a3a689609 https://github.com/cpuguy83/check.git github.com/gorilla/context v1.1 github.com/gorilla/mux v1.1 -github.com/jhowardmsft/opengcs v0.0.4 +github.com/jhowardmsft/opengcs v0.0.7 github.com/kr/pty 5cf931ef8f github.com/mattn/go-shellwords v1.0.3 github.com/tchap/go-patricia v2.2.6 diff --git a/components/engine/vendor/github.com/jhowardmsft/opengcs/gogcs/client/config.go b/components/engine/vendor/github.com/jhowardmsft/opengcs/gogcs/client/config.go index 830d37bef0..49f3e0b1c5 100644 --- a/components/engine/vendor/github.com/jhowardmsft/opengcs/gogcs/client/config.go +++ b/components/engine/vendor/github.com/jhowardmsft/opengcs/gogcs/client/config.go @@ -52,19 +52,20 @@ const ( // // VHD is the priority. type Config struct { - KirdPath string // Path to where kernel/initrd are found (defaults to c:\program files\lcow) - KernelFile string // Kernel for Utility VM (embedded in a UEFI bootloader) - does NOT include full path, just filename - InitrdFile string // Initrd image for Utility VM - does NOT include full path, just filename - Vhdx string // VHD for booting the utility VM - is a full path - Name string // Name of the utility VM - RequestedMode Mode // What mode is preferred when validating - ActualMode Mode // What mode was obtained during validation - UvmTimeoutSeconds int // How long to wait for the utility VM to respond in seconds - Uvm hcsshim.Container // The actual container + KirdPath string // Path to where kernel/initrd are found (defaults to c:\program files\Linux Containers) + KernelFile string // Kernel for Utility VM (embedded in a UEFI bootloader) - does NOT include full path, just filename + InitrdFile string // Initrd image for Utility VM - does NOT include full path, just filename + Vhdx string // VHD for booting the utility VM - is a full path + Name string // Name of the utility VM + RequestedMode Mode // What mode is preferred when validating + ActualMode Mode // What mode was obtained during validation + UvmTimeoutSeconds int // How long to wait for the utility VM to respond in seconds + Uvm hcsshim.Container // The actual container + MappedVirtualDisks []hcsshim.MappedVirtualDisk // Data-disks to be attached } // GenerateDefault generates a default config from a set of options -// If baseDir is not supplied, defaults to $env:ProgramFiles\lcow +// If baseDir is not supplied, defaults to $env:ProgramFiles\Linux Containers func (config *Config) GenerateDefault(options []string) error { if config.UvmTimeoutSeconds < 0 { return fmt.Errorf("opengcs: cannot generate a config when supplied a negative utility VM timeout") @@ -111,7 +112,7 @@ func (config *Config) GenerateDefault(options []string) error { } if config.KirdPath == "" { - config.KirdPath = filepath.Join(os.Getenv("ProgramFiles"), "lcow") + config.KirdPath = filepath.Join(os.Getenv("ProgramFiles"), "Linux Containers") } if config.Vhdx == "" { @@ -138,6 +139,8 @@ func (config *Config) GenerateDefault(options []string) error { } } + config.MappedVirtualDisks = nil + return nil } @@ -172,11 +175,6 @@ func (config *Config) validate() error { return fmt.Errorf("opengcs: configuration is invalid") } - // Move to validation - //if _, err := os.Stat(baseDir); os.IsNotExist(err) { - // return fmt.Errorf("opengcs: cannot create default utility VM configuration as directory '%s' was not found", baseDir) - //} - if _, err := os.Stat(filepath.Join(config.KirdPath, config.KernelFile)); os.IsNotExist(err) { return fmt.Errorf("opengcs: kernel '%s' was not found", filepath.Join(config.KirdPath, config.KernelFile)) } @@ -185,6 +183,17 @@ func (config *Config) validate() error { } config.ActualMode = ModeActualKernelInitrd + + // Ensure all the MappedVirtualDisks exist on the host + for _, mvd := range config.MappedVirtualDisks { + if _, err := os.Stat(mvd.HostPath); err != nil { + return fmt.Errorf("opengcs: MappedVirtualDisk '%s' was not found", mvd.HostPath) + } + if mvd.ContainerPath == "" { + return fmt.Errorf("opengcs: MappedVirtualDisk '%s' has no container path", mvd.HostPath) + } + } + return nil } @@ -202,6 +211,7 @@ func (config *Config) Create() error { SystemType: "container", ContainerType: "linux", TerminateOnLastHandleClosed: true, + MappedVirtualDisks: config.MappedVirtualDisks, } if config.ActualMode == ModeActualVhdx { diff --git a/components/engine/vendor/github.com/jhowardmsft/opengcs/gogcs/client/createsandbox.go b/components/engine/vendor/github.com/jhowardmsft/opengcs/gogcs/client/createsandbox.go index 66ab703735..4fd9cc907a 100644 --- a/components/engine/vendor/github.com/jhowardmsft/opengcs/gogcs/client/createsandbox.go +++ b/components/engine/vendor/github.com/jhowardmsft/opengcs/gogcs/client/createsandbox.go @@ -25,7 +25,7 @@ func (config *Config) CreateSandbox(destFile string, maxSizeInMB uint32, cacheFi logrus.Debugf("opengcs: CreateSandbox: %s size:%dMB cache:%s", destFile, maxSizeInMB, cacheFile) // Retrieve from cache if the default size and already on disk - if maxSizeInMB == DefaultSandboxSizeMB { + if cacheFile != "" && maxSizeInMB == DefaultSandboxSizeMB { sandboxCacheLock.Lock() if _, err := os.Stat(cacheFile); err == nil { if err := copyFile(cacheFile, destFile); err != nil { @@ -61,7 +61,7 @@ func (config *Config) CreateSandbox(destFile string, maxSizeInMB uint32, cacheFi } // Populate the cache - if maxSizeInMB == DefaultSandboxSizeMB { + if cacheFile != "" && maxSizeInMB == DefaultSandboxSizeMB { sandboxCacheLock.Lock() // It may already exist due to being created on another thread, in which case no copy back needed. if _, err := os.Stat(cacheFile); os.IsNotExist(err) { diff --git a/components/engine/vendor/github.com/jhowardmsft/opengcs/gogcs/client/tartovhd.go b/components/engine/vendor/github.com/jhowardmsft/opengcs/gogcs/client/tartovhd.go index 8832e096e6..f6ce79ecb6 100644 --- a/components/engine/vendor/github.com/jhowardmsft/opengcs/gogcs/client/tartovhd.go +++ b/components/engine/vendor/github.com/jhowardmsft/opengcs/gogcs/client/tartovhd.go @@ -31,7 +31,9 @@ func (config *Config) TarToVhd(targetVHDFile string, reader io.Reader) (int64, e } // Don't need stdin now we've sent everything. This signals GCS that we are finished sending data. - process.Process.CloseStdin() + if err := process.Process.CloseStdin(); err != nil { + return 0, fmt.Errorf("opengcs: TarToVhd: %s: failed closing stdin handle: %s", targetVHDFile, err) + } // Write stdout contents of `tar2vhd` to the VHD file payloadSize, err := writeFileFromReader(targetVHDFile, process.Stdout, config.UvmTimeoutSeconds, fmt.Sprintf("output of tar2vhd to %s", targetVHDFile)) diff --git a/components/engine/vendor/github.com/jhowardmsft/opengcs/gogcs/client/utilities.go b/components/engine/vendor/github.com/jhowardmsft/opengcs/gogcs/client/utilities.go index 427cde7795..4ebd9bb156 100644 --- a/components/engine/vendor/github.com/jhowardmsft/opengcs/gogcs/client/utilities.go +++ b/components/engine/vendor/github.com/jhowardmsft/opengcs/gogcs/client/utilities.go @@ -41,20 +41,9 @@ func copyWithTimeout(dst io.Writer, src io.Reader, size int64, timeoutSeconds in done := make(chan resultType, 1) go func() { - // TODO @jhowardmsft. Needs platform fix. Improve reliability by - // chunking the data. Ultimately can just use io.Copy instead with no loop result := resultType{} - var copied int64 - for { - copied, result.err = io.CopyN(dst, src, 1024) - result.bytes += copied - if copied == 0 { - done <- result - break - } - // TODO @jhowardmsft - next line is debugging only. Remove - //logrus.Debugf("%s: copied so far %d\n", context, result.bytes) - } + result.bytes, result.err = io.Copy(dst, src) + done <- result }() var result resultType From 02efb014146c53efee1ddd6cf4b39a5622491351 Mon Sep 17 00:00:00 2001 From: John Howard Date: Mon, 26 Jun 2017 21:26:18 -0700 Subject: [PATCH 20/21] LCOW: Change to c:\Program Files\Linux Containers Signed-off-by: John Howard Upstream-commit: 2c43cbe0d5132618c74de4631c89c541bbafb8c2 Component: engine --- components/engine/libcontainerd/client_windows.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/engine/libcontainerd/client_windows.go b/components/engine/libcontainerd/client_windows.go index d7f312e6e4..ee310f6865 100644 --- a/components/engine/libcontainerd/client_windows.go +++ b/components/engine/libcontainerd/client_windows.go @@ -299,7 +299,7 @@ func (clnt *client) createLinux(containerID string, checkpoint string, checkpoin Owner: defaultOwner, TerminateOnLastHandleClosed: true, HvRuntime: &hcsshim.HvRuntime{ - ImagePath: `c:\program files\lcow`, + ImagePath: `c:\Program Files\Linux Containers`, LinuxKernelFile: `bootx64.efi`, LinuxInitrdFile: `initrd.img`, }, From 41493b0fade0fd0c1651596d6e577e79b3a15a61 Mon Sep 17 00:00:00 2001 From: lixiaobing10051267 Date: Tue, 27 Jun 2017 14:47:15 +0800 Subject: [PATCH 21/21] optimize for loop with rootfs.DiffIDs Signed-off-by: lixiaobing10051267 Upstream-commit: 80b2c326de29552c6ee4072fca75ae00eb3be41a Component: engine --- components/engine/distribution/push_v2.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/engine/distribution/push_v2.go b/components/engine/distribution/push_v2.go index 504083367e..ffc7d68107 100644 --- a/components/engine/distribution/push_v2.go +++ b/components/engine/distribution/push_v2.go @@ -147,7 +147,7 @@ func (p *v2Pusher) pushV2Tag(ctx context.Context, ref reference.NamedTagged, id } // Loop bounds condition is to avoid pushing the base layer on Windows. - for i := 0; i < len(rootfs.DiffIDs); i++ { + for range rootfs.DiffIDs { descriptor := descriptorTemplate descriptor.layer = l descriptor.checkedDigests = make(map[digest.Digest]struct{})