From 13222160e84a01db2174c0dbf3000761a756fd72 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Thu, 22 Dec 2016 11:44:09 -0800 Subject: [PATCH 1/4] Define PushResult in api types Signed-off-by: Tonis Tiigi --- api/types/types.go | 9 +++++++++ cli/command/image/trust.go | 24 ++++++++++++------------ distribution/push_v2.go | 12 ++---------- 3 files changed, 23 insertions(+), 22 deletions(-) diff --git a/api/types/types.go b/api/types/types.go index 47ec86ceeb..e97df9bc9c 100644 --- a/api/types/types.go +++ b/api/types/types.go @@ -547,3 +547,12 @@ type SecretCreateResponse struct { type SecretListOptions struct { Filters filters.Args } + +// PushResult contains the tag, manifest digest, and manifest size from the +// push. It's used to signal this information to the trust code in the client +// so it can sign the manifest if necessary. +type PushResult struct { + Tag string + Digest string + Size int +} diff --git a/cli/command/image/trust.go b/cli/command/image/trust.go index f32c301959..192bc047c9 100644 --- a/cli/command/image/trust.go +++ b/cli/command/image/trust.go @@ -9,19 +9,17 @@ import ( "path" "sort" - "golang.org/x/net/context" - "github.com/Sirupsen/logrus" "github.com/docker/distribution/digest" "github.com/docker/docker/api/types" "github.com/docker/docker/cli/command" "github.com/docker/docker/cli/trust" - "github.com/docker/docker/distribution" "github.com/docker/docker/pkg/jsonmessage" "github.com/docker/docker/reference" "github.com/docker/docker/registry" "github.com/docker/notary/client" "github.com/docker/notary/tuf/data" + "golang.org/x/net/context" ) type target struct { @@ -52,17 +50,19 @@ func trustedPush(ctx context.Context, cli *command.DockerCli, repoInfo *registry return } - var pushResult distribution.PushResult + var pushResult types.PushResult err := json.Unmarshal(*aux, &pushResult) - if err == nil && pushResult.Tag != "" && pushResult.Digest.Validate() == nil { - h, err := hex.DecodeString(pushResult.Digest.Hex()) - if err != nil { - target = nil - return + if err == nil && pushResult.Tag != "" { + if dgst, err := digest.ParseDigest(pushResult.Digest); err == nil { + h, err := hex.DecodeString(dgst.Hex()) + if err != nil { + target = nil + return + } + target.Name = pushResult.Tag + target.Hashes = data.Hashes{string(dgst.Algorithm()): h} + target.Length = int64(pushResult.Size) } - target.Name = pushResult.Tag - target.Hashes = data.Hashes{string(pushResult.Digest.Algorithm()): h} - target.Length = int64(pushResult.Size) } } diff --git a/distribution/push_v2.go b/distribution/push_v2.go index 1f8c822fec..e56f80065b 100644 --- a/distribution/push_v2.go +++ b/distribution/push_v2.go @@ -18,6 +18,7 @@ import ( "github.com/docker/distribution/manifest/schema2" distreference "github.com/docker/distribution/reference" "github.com/docker/distribution/registry/client" + apitypes "github.com/docker/docker/api/types" "github.com/docker/docker/distribution/metadata" "github.com/docker/docker/distribution/xfer" "github.com/docker/docker/layer" @@ -33,15 +34,6 @@ const ( middleLayerMaximumSize = 10 * (1 << 20) // 10MB ) -// PushResult contains the tag, manifest digest, and manifest size from the -// push. It's used to signal this information to the trust code in the client -// so it can sign the manifest if necessary. -type PushResult struct { - Tag string - Digest digest.Digest - Size int -} - type v2Pusher struct { v2MetadataService metadata.V2MetadataService ref reference.Named @@ -225,7 +217,7 @@ func (p *v2Pusher) pushV2Tag(ctx context.Context, ref reference.NamedTagged, id // Signal digest to the trust client so it can sign the // push, if appropriate. - progress.Aux(p.config.ProgressOutput, PushResult{Tag: ref.Tag(), Digest: manifestDigest, Size: len(canonicalManifest)}) + progress.Aux(p.config.ProgressOutput, apitypes.PushResult{Tag: ref.Tag(), Digest: manifestDigest.String(), Size: len(canonicalManifest)}) return nil } From feaf5902f650f2326e1c41e82dfe28962f1ba46e Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Thu, 22 Dec 2016 13:25:02 -0800 Subject: [PATCH 2/4] Move builder cli helper functions to own pkg Signed-off-by: Tonis Tiigi --- builder/utils_test.go | 26 ------- cli/command/image/build.go | 14 ++-- .../command/image/build}/context.go | 7 +- .../command/image/build}/context_test.go | 78 ++++++++++++++++++- .../command/image/build}/context_unix.go | 2 +- .../command/image/build}/context_windows.go | 2 +- 6 files changed, 92 insertions(+), 37 deletions(-) rename {builder => cli/command/image/build}/context.go (98%) rename {builder => cli/command/image/build}/context_test.go (78%) rename {builder => cli/command/image/build}/context_unix.go (90%) rename {builder => cli/command/image/build}/context_windows.go (94%) diff --git a/builder/utils_test.go b/builder/utils_test.go index 1101ff1d1d..adc264539a 100644 --- a/builder/utils_test.go +++ b/builder/utils_test.go @@ -59,29 +59,3 @@ func createTestTempFile(t *testing.T, dir, filename, contents string, perm os.Fi return filePath } - -// chdir changes current working directory to dir. -// It returns a function which changes working directory back to the previous one. -// This function is meant to be executed as a deferred call. -// When an error occurs, it terminates the test. -func chdir(t *testing.T, dir string) func() { - workingDirectory, err := os.Getwd() - - if err != nil { - t.Fatalf("Error when retrieving working directory: %s", err) - } - - err = os.Chdir(dir) - - if err != nil { - t.Fatalf("Error when changing directory to %s: %s", dir, err) - } - - return func() { - err = os.Chdir(workingDirectory) - - if err != nil { - t.Fatalf("Error when changing back to working directory (%s): %s", workingDirectory, err) - } - } -} diff --git a/cli/command/image/build.go b/cli/command/image/build.go index e3e7ff2b02..f194659e08 100644 --- a/cli/command/image/build.go +++ b/cli/command/image/build.go @@ -16,10 +16,10 @@ import ( "github.com/docker/docker/api" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" - "github.com/docker/docker/builder" "github.com/docker/docker/builder/dockerignore" "github.com/docker/docker/cli" "github.com/docker/docker/cli/command" + "github.com/docker/docker/cli/command/image/build" "github.com/docker/docker/opts" "github.com/docker/docker/pkg/archive" "github.com/docker/docker/pkg/fileutils" @@ -29,7 +29,7 @@ import ( "github.com/docker/docker/pkg/urlutil" "github.com/docker/docker/reference" runconfigopts "github.com/docker/docker/runconfig/opts" - "github.com/docker/go-units" + units "github.com/docker/go-units" "github.com/spf13/cobra" ) @@ -156,13 +156,13 @@ func runBuild(dockerCli *command.DockerCli, options buildOptions) error { switch { case specifiedContext == "-": - buildCtx, relDockerfile, err = builder.GetContextFromReader(dockerCli.In(), options.dockerfileName) + buildCtx, relDockerfile, err = build.GetContextFromReader(dockerCli.In(), options.dockerfileName) case urlutil.IsGitURL(specifiedContext): - tempDir, relDockerfile, err = builder.GetContextFromGitURL(specifiedContext, options.dockerfileName) + tempDir, relDockerfile, err = build.GetContextFromGitURL(specifiedContext, options.dockerfileName) case urlutil.IsURL(specifiedContext): - buildCtx, relDockerfile, err = builder.GetContextFromURL(progBuff, specifiedContext, options.dockerfileName) + buildCtx, relDockerfile, err = build.GetContextFromURL(progBuff, specifiedContext, options.dockerfileName) default: - contextDir, relDockerfile, err = builder.GetContextFromLocalDir(specifiedContext, options.dockerfileName) + contextDir, relDockerfile, err = build.GetContextFromLocalDir(specifiedContext, options.dockerfileName) } if err != nil { @@ -198,7 +198,7 @@ func runBuild(dockerCli *command.DockerCli, options buildOptions) error { } } - if err := builder.ValidateContextDirectory(contextDir, excludes); err != nil { + if err := build.ValidateContextDirectory(contextDir, excludes); err != nil { return fmt.Errorf("Error checking context: '%s'.", err) } diff --git a/builder/context.go b/cli/command/image/build/context.go similarity index 98% rename from builder/context.go rename to cli/command/image/build/context.go index 600f42319b..86157c359d 100644 --- a/builder/context.go +++ b/cli/command/image/build/context.go @@ -1,4 +1,4 @@ -package builder +package build import ( "bufio" @@ -20,6 +20,11 @@ import ( "github.com/docker/docker/pkg/streamformatter" ) +const ( + // DefaultDockerfileName is the Default filename with Docker commands, read by docker build + DefaultDockerfileName string = "Dockerfile" +) + // ValidateContextDirectory checks if all the contents of the directory // can be read and returns an error if some files can't be read // symlinks which point to non-existing files don't trigger an error diff --git a/builder/context_test.go b/cli/command/image/build/context_test.go similarity index 78% rename from builder/context_test.go rename to cli/command/image/build/context_test.go index 27d29d79f4..afa04a4fcd 100644 --- a/builder/context_test.go +++ b/cli/command/image/build/context_test.go @@ -1,10 +1,11 @@ -package builder +package build import ( "archive/tar" "bytes" "io" "io/ioutil" + "os" "path/filepath" "runtime" "strings" @@ -13,6 +14,8 @@ import ( "github.com/docker/docker/pkg/archive" ) +const dockerfileContents = "FROM busybox" + var prepareEmpty = func(t *testing.T) (string, func()) { return "", func() {} } @@ -305,3 +308,76 @@ func TestValidateContextDirectoryWithOneFile(t *testing.T) { func TestValidateContextDirectoryWithOneFileExcludes(t *testing.T) { testValidateContextDirectory(t, prepareOneFile, []string{DefaultDockerfileName}) } + +// createTestTempDir creates a temporary directory for testing. +// It returns the created path and a cleanup function which is meant to be used as deferred call. +// When an error occurs, it terminates the test. +func createTestTempDir(t *testing.T, dir, prefix string) (string, func()) { + path, err := ioutil.TempDir(dir, prefix) + + if err != nil { + t.Fatalf("Error when creating directory %s with prefix %s: %s", dir, prefix, err) + } + + return path, func() { + err = os.RemoveAll(path) + + if err != nil { + t.Fatalf("Error when removing directory %s: %s", path, err) + } + } +} + +// createTestTempSubdir creates a temporary directory for testing. +// It returns the created path but doesn't provide a cleanup function, +// so createTestTempSubdir should be used only for creating temporary subdirectories +// whose parent directories are properly cleaned up. +// When an error occurs, it terminates the test. +func createTestTempSubdir(t *testing.T, dir, prefix string) string { + path, err := ioutil.TempDir(dir, prefix) + + if err != nil { + t.Fatalf("Error when creating directory %s with prefix %s: %s", dir, prefix, err) + } + + return path +} + +// createTestTempFile creates a temporary file within dir with specific contents and permissions. +// When an error occurs, it terminates the test +func createTestTempFile(t *testing.T, dir, filename, contents string, perm os.FileMode) string { + filePath := filepath.Join(dir, filename) + err := ioutil.WriteFile(filePath, []byte(contents), perm) + + if err != nil { + t.Fatalf("Error when creating %s file: %s", filename, err) + } + + return filePath +} + +// chdir changes current working directory to dir. +// It returns a function which changes working directory back to the previous one. +// This function is meant to be executed as a deferred call. +// When an error occurs, it terminates the test. +func chdir(t *testing.T, dir string) func() { + workingDirectory, err := os.Getwd() + + if err != nil { + t.Fatalf("Error when retrieving working directory: %s", err) + } + + err = os.Chdir(dir) + + if err != nil { + t.Fatalf("Error when changing directory to %s: %s", dir, err) + } + + return func() { + err = os.Chdir(workingDirectory) + + if err != nil { + t.Fatalf("Error when changing back to working directory (%s): %s", workingDirectory, err) + } + } +} diff --git a/builder/context_unix.go b/cli/command/image/build/context_unix.go similarity index 90% rename from builder/context_unix.go rename to cli/command/image/build/context_unix.go index d1f72e0573..cb2634f079 100644 --- a/builder/context_unix.go +++ b/cli/command/image/build/context_unix.go @@ -1,6 +1,6 @@ // +build !windows -package builder +package build import ( "path/filepath" diff --git a/builder/context_windows.go b/cli/command/image/build/context_windows.go similarity index 94% rename from builder/context_windows.go rename to cli/command/image/build/context_windows.go index b8ba2ba231..c577cfa7be 100644 --- a/builder/context_windows.go +++ b/cli/command/image/build/context_windows.go @@ -1,6 +1,6 @@ // +build windows -package builder +package build import ( "path/filepath" From 9f3046f9a03f1a710bdb7fcddd37ff50e71e31c3 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Thu, 22 Dec 2016 13:26:30 -0800 Subject: [PATCH 3/4] Move imageID validation to stringid pkg Signed-off-by: Tonis Tiigi --- image/v1/imagev1.go | 10 ++-------- pkg/stringid/stringid.go | 14 +++++++++++++- reference/reference.go | 6 +++--- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/image/v1/imagev1.go b/image/v1/imagev1.go index 9c167a2ce9..b52b447e82 100644 --- a/image/v1/imagev1.go +++ b/image/v1/imagev1.go @@ -2,9 +2,7 @@ package v1 import ( "encoding/json" - "fmt" "reflect" - "regexp" "strings" "github.com/Sirupsen/logrus" @@ -12,10 +10,9 @@ import ( "github.com/docker/docker/api/types/versions" "github.com/docker/docker/image" "github.com/docker/docker/layer" + "github.com/docker/docker/pkg/stringid" ) -var validHex = regexp.MustCompile(`^([a-f0-9]{64})$`) - // noFallbackMinVersion is the minimum version for which v1compatibility // information will not be marshaled through the Image struct to remove // blank fields. @@ -149,8 +146,5 @@ func rawJSON(value interface{}) *json.RawMessage { // ValidateID checks whether an ID string is a valid image ID. func ValidateID(id string) error { - if ok := validHex.MatchString(id); !ok { - return fmt.Errorf("image ID %q is invalid", id) - } - return nil + return stringid.ValidateID(id) } diff --git a/pkg/stringid/stringid.go b/pkg/stringid/stringid.go index fa35d8bad5..82f85596c7 100644 --- a/pkg/stringid/stringid.go +++ b/pkg/stringid/stringid.go @@ -4,6 +4,7 @@ package stringid import ( "crypto/rand" "encoding/hex" + "fmt" "io" "regexp" "strconv" @@ -14,7 +15,10 @@ import ( const shortLen = 12 -var validShortID = regexp.MustCompile("^[a-z0-9]{12}$") +var ( + validShortID = regexp.MustCompile("^[a-f0-9]{12}$") + validHex = regexp.MustCompile(`^[a-f0-9]{64}$`) +) // IsShortID determines if an arbitrary string *looks like* a short ID. func IsShortID(id string) bool { @@ -67,3 +71,11 @@ func GenerateRandomID() string { func GenerateNonCryptoID() string { return generateID(false) } + +// ValidateID checks whether an ID string is a valid image ID. +func ValidateID(id string) error { + if ok := validHex.MatchString(id); !ok { + return fmt.Errorf("image ID %q is invalid", id) + } + return nil +} diff --git a/reference/reference.go b/reference/reference.go index 996fc50704..15854e5a5b 100644 --- a/reference/reference.go +++ b/reference/reference.go @@ -7,7 +7,7 @@ import ( "github.com/docker/distribution/digest" distreference "github.com/docker/distribution/reference" - "github.com/docker/docker/image/v1" + "github.com/docker/docker/pkg/stringid" ) const ( @@ -163,7 +163,7 @@ func IsNameOnly(ref Named) bool { // ParseIDOrReference parses string for an image ID or a reference. ID can be // without a default prefix. func ParseIDOrReference(idOrRef string) (digest.Digest, Named, error) { - if err := v1.ValidateID(idOrRef); err == nil { + if err := stringid.ValidateID(idOrRef); err == nil { idOrRef = "sha256:" + idOrRef } if dgst, err := digest.ParseDigest(idOrRef); err == nil { @@ -209,7 +209,7 @@ func normalize(name string) (string, error) { } func validateName(name string) error { - if err := v1.ValidateID(name); err == nil { + if err := stringid.ValidateID(name); err == nil { return fmt.Errorf("Invalid repository name (%s), cannot specify 64-byte hexadecimal strings", name) } return nil From 64981b9f095459ae65954ca80a86c8f4a735ef24 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Thu, 22 Dec 2016 13:51:47 -0800 Subject: [PATCH 4/4] Move UAStringKey to dockerversion pkg Removes grpc dependency from client Signed-off-by: Tonis Tiigi --- api/server/httputils/httputils.go | 3 --- api/server/server.go | 3 ++- dockerversion/useragent.go | 8 +++++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/api/server/httputils/httputils.go b/api/server/httputils/httputils.go index d37ee5cafe..2c2e8452f9 100644 --- a/api/server/httputils/httputils.go +++ b/api/server/httputils/httputils.go @@ -14,9 +14,6 @@ import ( // APIVersionKey is the client's requested API version. const APIVersionKey = "api-version" -// UAStringKey is used as key type for user-agent string in net/context struct -const UAStringKey = "upstream-user-agent" - // APIFunc is an adapter to allow the use of ordinary functions as Docker API endpoints. // Any function that has the appropriate signature can be registered as an API endpoint (e.g. getVersion). type APIFunc func(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error diff --git a/api/server/server.go b/api/server/server.go index d988b8e17f..d8b716e100 100644 --- a/api/server/server.go +++ b/api/server/server.go @@ -12,6 +12,7 @@ import ( "github.com/docker/docker/api/server/httputils" "github.com/docker/docker/api/server/middleware" "github.com/docker/docker/api/server/router" + "github.com/docker/docker/dockerversion" "github.com/gorilla/mux" "golang.org/x/net/context" ) @@ -128,7 +129,7 @@ func (s *Server) makeHTTPHandler(handler httputils.APIFunc) http.HandlerFunc { // apply to all requests. Data that is specific to the // immediate function being called should still be passed // as 'args' on the function call. - ctx := context.WithValue(context.Background(), httputils.UAStringKey, r.Header.Get("User-Agent")) + ctx := context.WithValue(context.Background(), dockerversion.UAStringKey, r.Header.Get("User-Agent")) handlerFunc := s.handlerWithGlobalMiddlewares(handler) vars := mux.Vars(r) diff --git a/dockerversion/useragent.go b/dockerversion/useragent.go index d2a891c4d6..53632cbc35 100644 --- a/dockerversion/useragent.go +++ b/dockerversion/useragent.go @@ -4,12 +4,14 @@ import ( "fmt" "runtime" - "github.com/docker/docker/api/server/httputils" "github.com/docker/docker/pkg/parsers/kernel" "github.com/docker/docker/pkg/useragent" "golang.org/x/net/context" ) +// UAStringKey is used as key type for user-agent string in net/context struct +const UAStringKey = "upstream-user-agent" + // DockerUserAgent is the User-Agent the Docker client uses to identify itself. // In accordance with RFC 7231 (5.5.3) is of the form: // [docker client's UA] UpstreamClient([upstream client's UA]) @@ -37,9 +39,9 @@ func DockerUserAgent(ctx context.Context) string { func getUserAgentFromContext(ctx context.Context) string { var upstreamUA string if ctx != nil { - var ki interface{} = ctx.Value(httputils.UAStringKey) + var ki interface{} = ctx.Value(UAStringKey) if ki != nil { - upstreamUA = ctx.Value(httputils.UAStringKey).(string) + upstreamUA = ctx.Value(UAStringKey).(string) } } return upstreamUA