From 1266f13afadd01dfce3c57331dfb24ce815a025f Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Mon, 5 Jan 2015 15:14:11 -0800 Subject: [PATCH 1/5] Move StringSet to new collections package As part of the efforts to break up the common package before disaster strikes, a new collections package has been created. More may belong there but for now, it only includes an implementation of StringSet. Signed-off-by: Stephen J Day --- auth/token/accesscontroller.go | 6 +++--- auth/token/token.go | 6 +++--- auth/token/token_test.go | 6 +++--- auth/token/util.go | 6 +++--- {common => collections}/stringset.go | 2 +- 5 files changed, 13 insertions(+), 13 deletions(-) rename {common => collections}/stringset.go (97%) diff --git a/auth/token/accesscontroller.go b/auth/token/accesscontroller.go index ca92861ef..b1a262f40 100644 --- a/auth/token/accesscontroller.go +++ b/auth/token/accesscontroller.go @@ -14,7 +14,7 @@ import ( "github.com/docker/libtrust" "github.com/docker/distribution/auth" - "github.com/docker/distribution/common" + "github.com/docker/distribution/collections" ) // accessSet maps a typed, named resource to @@ -241,8 +241,8 @@ func (ac *accessController) Authorized(req *http.Request, accessItems ...auth.Ac } verifyOpts := VerifyOptions{ - TrustedIssuers: common.NewStringSet(ac.issuer), - AcceptedAudiences: common.NewStringSet(ac.service), + TrustedIssuers: collections.NewStringSet(ac.issuer), + AcceptedAudiences: collections.NewStringSet(ac.service), Roots: ac.rootCerts, TrustedKeys: ac.trustedKeys, } diff --git a/auth/token/token.go b/auth/token/token.go index cdbf71073..d63a45a14 100644 --- a/auth/token/token.go +++ b/auth/token/token.go @@ -14,7 +14,7 @@ import ( "github.com/docker/libtrust" "github.com/docker/distribution/auth" - "github.com/docker/distribution/common" + "github.com/docker/distribution/collections" ) const ( @@ -71,8 +71,8 @@ type Token struct { // VerifyOptions is used to specify // options when verifying a JSON Web Token. type VerifyOptions struct { - TrustedIssuers common.StringSet - AcceptedAudiences common.StringSet + TrustedIssuers collections.StringSet + AcceptedAudiences collections.StringSet Roots *x509.CertPool TrustedKeys map[string]libtrust.PublicKey } diff --git a/auth/token/token_test.go b/auth/token/token_test.go index 9c8ab6cb7..e8248217f 100644 --- a/auth/token/token_test.go +++ b/auth/token/token_test.go @@ -18,7 +18,7 @@ import ( "github.com/docker/libtrust" "github.com/docker/distribution/auth" - "github.com/docker/distribution/common" + "github.com/docker/distribution/collections" ) func makeRootKeys(numKeys int) ([]libtrust.PrivateKey, error) { @@ -196,8 +196,8 @@ func TestTokenVerify(t *testing.T) { } verifyOps := VerifyOptions{ - TrustedIssuers: common.NewStringSet(issuer), - AcceptedAudiences: common.NewStringSet(audience), + TrustedIssuers: collections.NewStringSet(issuer), + AcceptedAudiences: collections.NewStringSet(audience), Roots: rootPool, TrustedKeys: trustedKeys, } diff --git a/auth/token/util.go b/auth/token/util.go index 8aa46bac0..f90721710 100644 --- a/auth/token/util.go +++ b/auth/token/util.go @@ -5,7 +5,7 @@ import ( "errors" "strings" - "github.com/docker/distribution/common" + "github.com/docker/distribution/collections" ) // joseBase64UrlEncode encodes the given data using the standard base64 url @@ -35,11 +35,11 @@ func joseBase64UrlDecode(s string) ([]byte, error) { // actionSet is a special type of stringSet. type actionSet struct { - common.StringSet + collections.StringSet } func newActionSet(actions ...string) actionSet { - return actionSet{common.NewStringSet(actions...)} + return actionSet{collections.NewStringSet(actions...)} } // Contains calls StringSet.Contains() for diff --git a/common/stringset.go b/collections/stringset.go similarity index 97% rename from common/stringset.go rename to collections/stringset.go index 36f4ba5a6..8ce1becbc 100644 --- a/common/stringset.go +++ b/collections/stringset.go @@ -1,4 +1,4 @@ -package common +package collections // StringSet is a useful type for looking up strings. type StringSet map[string]struct{} From 8be20212f1a4b80b5ceeb1242b0e87a34056d460 Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Mon, 5 Jan 2015 16:04:30 -0800 Subject: [PATCH 2/5] Move tarsum utilities out of common package In preparation for removing the common package, the tarsum utilities are being moved to the more relevant digest package. This functionality will probably go away in the future, but it's maintained here for the time being. Signed-off-by: Stephen J Day --- digest/digest.go | 3 +-- {common => digest}/tarsum.go | 2 +- {common => digest}/tarsum_test.go | 2 +- storage/paths.go | 3 +-- 4 files changed, 4 insertions(+), 6 deletions(-) rename {common => digest}/tarsum.go (99%) rename {common => digest}/tarsum_test.go (99%) diff --git a/digest/digest.go b/digest/digest.go index dcf6bc910..a5d5b5a84 100644 --- a/digest/digest.go +++ b/digest/digest.go @@ -10,7 +10,6 @@ import ( "regexp" "strings" - "github.com/docker/distribution/common" "github.com/docker/docker/pkg/tarsum" ) @@ -105,7 +104,7 @@ func FromBytes(p []byte) (Digest, error) { func (d Digest) Validate() error { s := string(d) // Common case will be tarsum - _, err := common.ParseTarSum(s) + _, err := ParseTarSum(s) if err == nil { return nil } diff --git a/common/tarsum.go b/digest/tarsum.go similarity index 99% rename from common/tarsum.go rename to digest/tarsum.go index a1a56d6d8..6c32bc590 100644 --- a/common/tarsum.go +++ b/digest/tarsum.go @@ -1,4 +1,4 @@ -package common +package digest import ( "fmt" diff --git a/common/tarsum_test.go b/digest/tarsum_test.go similarity index 99% rename from common/tarsum_test.go rename to digest/tarsum_test.go index e860c9cdf..5a10b7468 100644 --- a/common/tarsum_test.go +++ b/digest/tarsum_test.go @@ -1,4 +1,4 @@ -package common +package digest import ( "reflect" diff --git a/storage/paths.go b/storage/paths.go index 9a61e0815..a19f2d702 100644 --- a/storage/paths.go +++ b/storage/paths.go @@ -5,7 +5,6 @@ import ( "path" "strings" - "github.com/docker/distribution/common" "github.com/docker/distribution/digest" ) @@ -189,7 +188,7 @@ func digestPathComoponents(dgst digest.Digest) ([]string, error) { hex, } - if tsi, err := common.ParseTarSum(dgst.String()); err == nil { + if tsi, err := digest.ParseTarSum(dgst.String()); err == nil { // We have a tarsum! version := tsi.Version if version == "" { From d88884c51c6427708617ce46cefc500ee29d943b Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Mon, 5 Jan 2015 16:44:03 -0800 Subject: [PATCH 3/5] Move names regular expressions to api/v2 packages Because the repository name definitions are part of the v2 specification, they have been moved out of the common package. This is part of the effort to break up the common package into more sensible components. Signed-off-by: Stephen J Day --- api/v2/descriptors.go | 15 +++++++-------- {common => api/v2}/names.go | 2 +- {common => api/v2}/names_test.go | 2 +- 3 files changed, 9 insertions(+), 10 deletions(-) rename {common => api/v2}/names.go (99%) rename {common => api/v2}/names_test.go (99%) diff --git a/api/v2/descriptors.go b/api/v2/descriptors.go index dfbbc6f90..e97dd37c2 100644 --- a/api/v2/descriptors.go +++ b/api/v2/descriptors.go @@ -4,7 +4,6 @@ import ( "net/http" "regexp" - "github.com/docker/distribution/common" "github.com/docker/distribution/digest" ) @@ -12,7 +11,7 @@ var ( nameParameterDescriptor = ParameterDescriptor{ Name: "name", Type: "string", - Format: common.RepositoryNameRegexp.String(), + Format: RepositoryNameRegexp.String(), Required: true, Description: `Name of the target repository.`, } @@ -20,7 +19,7 @@ var ( tagParameterDescriptor = ParameterDescriptor{ Name: "tag", Type: "string", - Format: common.TagNameRegexp.String(), + Format: TagNameRegexp.String(), Required: true, Description: `Tag of the target manifiest.`, } @@ -307,7 +306,7 @@ var routeDescriptors = []RouteDescriptor{ }, { Name: RouteNameTags, - Path: "/v2/{name:" + common.RepositoryNameRegexp.String() + "}/tags/list", + Path: "/v2/{name:" + RepositoryNameRegexp.String() + "}/tags/list", Entity: "Tags", Description: "Retrieve information about tags.", Methods: []MethodDescriptor{ @@ -352,7 +351,7 @@ var routeDescriptors = []RouteDescriptor{ }, { Name: RouteNameManifest, - Path: "/v2/{name:" + common.RepositoryNameRegexp.String() + "}/manifests/{tag:" + common.TagNameRegexp.String() + "}", + Path: "/v2/{name:" + RepositoryNameRegexp.String() + "}/manifests/{tag:" + TagNameRegexp.String() + "}", Entity: "Manifest", Description: "Create, update and retrieve manifests.", Methods: []MethodDescriptor{ @@ -514,7 +513,7 @@ var routeDescriptors = []RouteDescriptor{ { Name: RouteNameBlob, - Path: "/v2/{name:" + common.RepositoryNameRegexp.String() + "}/blobs/{digest:" + digest.DigestRegexp.String() + "}", + Path: "/v2/{name:" + RepositoryNameRegexp.String() + "}/blobs/{digest:" + digest.DigestRegexp.String() + "}", Entity: "Blob", Description: "Fetch the blob identified by `name` and `digest`. Used to fetch layers by tarsum digest.", Methods: []MethodDescriptor{ @@ -592,7 +591,7 @@ var routeDescriptors = []RouteDescriptor{ { Name: RouteNameBlobUpload, - Path: "/v2/{name:" + common.RepositoryNameRegexp.String() + "}/blobs/uploads/", + Path: "/v2/{name:" + RepositoryNameRegexp.String() + "}/blobs/uploads/", Entity: "Intiate Blob Upload", Description: "Initiate a blob upload. This endpoint can be used to create resumable uploads or monolithic uploads.", Methods: []MethodDescriptor{ @@ -700,7 +699,7 @@ var routeDescriptors = []RouteDescriptor{ { Name: RouteNameBlobUploadChunk, - Path: "/v2/{name:" + common.RepositoryNameRegexp.String() + "}/blobs/uploads/{uuid}", + Path: "/v2/{name:" + RepositoryNameRegexp.String() + "}/blobs/uploads/{uuid}", Entity: "Blob Upload", Description: "Interact with blob uploads. Clients should never assemble URLs for this endpoint and should only take it through the `Location` header on related API requests.", Methods: []MethodDescriptor{ diff --git a/common/names.go b/api/v2/names.go similarity index 99% rename from common/names.go rename to api/v2/names.go index 0172c4806..e0c05d484 100644 --- a/common/names.go +++ b/api/v2/names.go @@ -1,4 +1,4 @@ -package common +package v2 import ( "fmt" diff --git a/common/names_test.go b/api/v2/names_test.go similarity index 99% rename from common/names_test.go rename to api/v2/names_test.go index e88257bde..038386e4c 100644 --- a/common/names_test.go +++ b/api/v2/names_test.go @@ -1,4 +1,4 @@ -package common +package v2 import ( "testing" From adaa2246e7eab75c36c22be75de2a43dff5e7aeb Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Mon, 5 Jan 2015 16:53:13 -0800 Subject: [PATCH 4/5] Move testutil package to top-level Since the common package no longer exists, the testutil package is being moved up to the root. Ideally, we don't have large omnibus packages, like testutil, but we can fix that in another refactoring round. Signed-off-by: Stephen J Day --- api_test.go | 2 +- client/client_test.go | 2 +- digest/verifiers_test.go | 2 +- storage/layer_test.go | 2 +- {common/testutil => testutil}/handler.go | 0 {common/testutil => testutil}/tarfile.go | 0 6 files changed, 4 insertions(+), 4 deletions(-) rename {common/testutil => testutil}/handler.go (100%) rename {common/testutil => testutil}/tarfile.go (100%) diff --git a/api_test.go b/api_test.go index 6e8c403c7..5d05e77f4 100644 --- a/api_test.go +++ b/api_test.go @@ -14,11 +14,11 @@ import ( "testing" "github.com/docker/distribution/api/v2" - "github.com/docker/distribution/common/testutil" "github.com/docker/distribution/configuration" "github.com/docker/distribution/digest" "github.com/docker/distribution/manifest" _ "github.com/docker/distribution/storagedriver/inmemory" + "github.com/docker/distribution/testutil" "github.com/docker/libtrust" "github.com/gorilla/handlers" ) diff --git a/client/client_test.go b/client/client_test.go index 76867bcfd..65e73f34b 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -9,9 +9,9 @@ import ( "sync" "testing" - "github.com/docker/distribution/common/testutil" "github.com/docker/distribution/digest" "github.com/docker/distribution/manifest" + "github.com/docker/distribution/testutil" ) type testBlob struct { diff --git a/digest/verifiers_test.go b/digest/verifiers_test.go index 5d595ec14..939a8c816 100644 --- a/digest/verifiers_test.go +++ b/digest/verifiers_test.go @@ -7,7 +7,7 @@ import ( "os" "testing" - "github.com/docker/distribution/common/testutil" + "github.com/docker/distribution/testutil" ) func TestDigestVerifier(t *testing.T) { diff --git a/storage/layer_test.go b/storage/layer_test.go index 166d803a7..5d985d682 100644 --- a/storage/layer_test.go +++ b/storage/layer_test.go @@ -9,10 +9,10 @@ import ( "os" "testing" - "github.com/docker/distribution/common/testutil" "github.com/docker/distribution/digest" "github.com/docker/distribution/storagedriver" "github.com/docker/distribution/storagedriver/inmemory" + "github.com/docker/distribution/testutil" ) // TestSimpleLayerUpload covers the layer upload process, exercising common diff --git a/common/testutil/handler.go b/testutil/handler.go similarity index 100% rename from common/testutil/handler.go rename to testutil/handler.go diff --git a/common/testutil/tarfile.go b/testutil/tarfile.go similarity index 100% rename from common/testutil/tarfile.go rename to testutil/tarfile.go From aea52c7fb5d301131c0acef95b5a12c37532687c Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Mon, 5 Jan 2015 18:21:03 -0800 Subject: [PATCH 5/5] Remove exported StringSet type and collections package The exported StringSet type is not necessary for the current use case of validating issues and audiences. The exported fields on VerifyOptions have been changed to require string slices. The collections package has been removed and the StringSet has been moved to the token package, where it is used. Signed-off-by: Stephen J Day --- auth/token/accesscontroller.go | 11 +++++------ {collections => auth/token}/stringset.go | 16 ++++++++-------- auth/token/token.go | 11 +++++------ auth/token/token_test.go | 8 +++----- auth/token/util.go | 21 +++++++++++++++------ 5 files changed, 36 insertions(+), 31 deletions(-) rename {collections => auth/token}/stringset.go (64%) diff --git a/auth/token/accesscontroller.go b/auth/token/accesscontroller.go index b1a262f40..4bbbc3e01 100644 --- a/auth/token/accesscontroller.go +++ b/auth/token/accesscontroller.go @@ -14,7 +14,6 @@ import ( "github.com/docker/libtrust" "github.com/docker/distribution/auth" - "github.com/docker/distribution/collections" ) // accessSet maps a typed, named resource to @@ -38,7 +37,7 @@ func newAccessSet(accessItems ...auth.Access) accessSet { accessSet[resource] = set } - set.Add(access.Action) + set.add(access.Action) } return accessSet @@ -48,7 +47,7 @@ func newAccessSet(accessItems ...auth.Access) accessSet { func (s accessSet) contains(access auth.Access) bool { actionSet, ok := s[access.Resource] if ok { - return actionSet.Contains(access.Action) + return actionSet.contains(access.Action) } return false @@ -61,7 +60,7 @@ func (s accessSet) scopeParam() string { scopes := make([]string, 0, len(s)) for resource, actionSet := range s { - actions := strings.Join(actionSet.Keys(), ",") + actions := strings.Join(actionSet.keys(), ",") scopes = append(scopes, fmt.Sprintf("%s:%s:%s", resource.Type, resource.Name, actions)) } @@ -241,8 +240,8 @@ func (ac *accessController) Authorized(req *http.Request, accessItems ...auth.Ac } verifyOpts := VerifyOptions{ - TrustedIssuers: collections.NewStringSet(ac.issuer), - AcceptedAudiences: collections.NewStringSet(ac.service), + TrustedIssuers: []string{ac.issuer}, + AcceptedAudiences: []string{ac.service}, Roots: ac.rootCerts, TrustedKeys: ac.trustedKeys, } diff --git a/collections/stringset.go b/auth/token/stringset.go similarity index 64% rename from collections/stringset.go rename to auth/token/stringset.go index 8ce1becbc..1d04f104c 100644 --- a/collections/stringset.go +++ b/auth/token/stringset.go @@ -1,30 +1,30 @@ -package collections +package token // StringSet is a useful type for looking up strings. -type StringSet map[string]struct{} +type stringSet map[string]struct{} // NewStringSet creates a new StringSet with the given strings. -func NewStringSet(keys ...string) StringSet { - ss := make(StringSet, len(keys)) - ss.Add(keys...) +func newStringSet(keys ...string) stringSet { + ss := make(stringSet, len(keys)) + ss.add(keys...) return ss } // Add inserts the given keys into this StringSet. -func (ss StringSet) Add(keys ...string) { +func (ss stringSet) add(keys ...string) { for _, key := range keys { ss[key] = struct{}{} } } // Contains returns whether the given key is in this StringSet. -func (ss StringSet) Contains(key string) bool { +func (ss stringSet) contains(key string) bool { _, ok := ss[key] return ok } // Keys returns a slice of all keys in this StringSet. -func (ss StringSet) Keys() []string { +func (ss stringSet) keys() []string { keys := make([]string, 0, len(ss)) for key := range ss { diff --git a/auth/token/token.go b/auth/token/token.go index d63a45a14..49449f3ba 100644 --- a/auth/token/token.go +++ b/auth/token/token.go @@ -14,7 +14,6 @@ import ( "github.com/docker/libtrust" "github.com/docker/distribution/auth" - "github.com/docker/distribution/collections" ) const ( @@ -71,8 +70,8 @@ type Token struct { // VerifyOptions is used to specify // options when verifying a JSON Web Token. type VerifyOptions struct { - TrustedIssuers collections.StringSet - AcceptedAudiences collections.StringSet + TrustedIssuers []string + AcceptedAudiences []string Roots *x509.CertPool TrustedKeys map[string]libtrust.PublicKey } @@ -132,13 +131,13 @@ func NewToken(rawToken string) (*Token, error) { // Returns a nil error if the token is valid. func (t *Token) Verify(verifyOpts VerifyOptions) error { // Verify that the Issuer claim is a trusted authority. - if !verifyOpts.TrustedIssuers.Contains(t.Claims.Issuer) { + if !contains(verifyOpts.TrustedIssuers, t.Claims.Issuer) { log.Errorf("token from untrusted issuer: %q", t.Claims.Issuer) return ErrInvalidToken } // Verify that the Audience claim is allowed. - if !verifyOpts.AcceptedAudiences.Contains(t.Claims.Audience) { + if !contains(verifyOpts.AcceptedAudiences, t.Claims.Audience) { log.Errorf("token intended for another audience: %q", t.Claims.Audience) return ErrInvalidToken } @@ -332,7 +331,7 @@ func (t *Token) accessSet() accessSet { } for _, action := range resourceActions.Actions { - set.Add(action) + set.add(action) } } diff --git a/auth/token/token_test.go b/auth/token/token_test.go index e8248217f..ba81900b5 100644 --- a/auth/token/token_test.go +++ b/auth/token/token_test.go @@ -15,10 +15,8 @@ import ( "testing" "time" - "github.com/docker/libtrust" - "github.com/docker/distribution/auth" - "github.com/docker/distribution/collections" + "github.com/docker/libtrust" ) func makeRootKeys(numKeys int) ([]libtrust.PrivateKey, error) { @@ -196,8 +194,8 @@ func TestTokenVerify(t *testing.T) { } verifyOps := VerifyOptions{ - TrustedIssuers: collections.NewStringSet(issuer), - AcceptedAudiences: collections.NewStringSet(audience), + TrustedIssuers: []string{issuer}, + AcceptedAudiences: []string{audience}, Roots: rootPool, TrustedKeys: trustedKeys, } diff --git a/auth/token/util.go b/auth/token/util.go index f90721710..bf3e01e83 100644 --- a/auth/token/util.go +++ b/auth/token/util.go @@ -4,8 +4,6 @@ import ( "encoding/base64" "errors" "strings" - - "github.com/docker/distribution/collections" ) // joseBase64UrlEncode encodes the given data using the standard base64 url @@ -35,15 +33,26 @@ func joseBase64UrlDecode(s string) ([]byte, error) { // actionSet is a special type of stringSet. type actionSet struct { - collections.StringSet + stringSet } func newActionSet(actions ...string) actionSet { - return actionSet{collections.NewStringSet(actions...)} + return actionSet{newStringSet(actions...)} } // Contains calls StringSet.Contains() for // either "*" or the given action string. -func (s actionSet) Contains(action string) bool { - return s.StringSet.Contains("*") || s.StringSet.Contains(action) +func (s actionSet) contains(action string) bool { + return s.stringSet.contains("*") || s.stringSet.contains(action) +} + +// contains returns true if q is found in ss. +func contains(ss []string, q string) bool { + for _, s := range ss { + if s == q { + return true + } + } + + return false }