From 6bae7ca5976ac55d8890a12c19e286ddc58a7719 Mon Sep 17 00:00:00 2001 From: Mike Brown Date: Thu, 20 Jul 2017 12:48:46 -0500 Subject: [PATCH] refactor adding enum for storage types Signed-off-by: Mike Brown --- manifest/manifestlist/manifestlist_test.go | 1 + manifest/ocischema/builder_test.go | 1 + manifest/ocischema/manifest.go | 1 - manifest/ocischema/manifest_test.go | 1 + registry/handlers/manifests.go | 67 ++++++++++++---------- registry/storage/driver/storagedriver.go | 2 +- 6 files changed, 41 insertions(+), 32 deletions(-) diff --git a/manifest/manifestlist/manifestlist_test.go b/manifest/manifestlist/manifestlist_test.go index 5ab328dd..6909895b 100644 --- a/manifest/manifestlist/manifestlist_test.go +++ b/manifest/manifestlist/manifestlist_test.go @@ -111,6 +111,7 @@ func TestManifestList(t *testing.T) { } } +// TODO (mikebrow): add annotations on the index and individual manifests var expectedOCIImageIndexSerialization = []byte(`{ "schemaVersion": 2, "mediaType": "application/vnd.oci.image.index.v1+json", diff --git a/manifest/ocischema/builder_test.go b/manifest/ocischema/builder_test.go index 33e9164f..ca785495 100644 --- a/manifest/ocischema/builder_test.go +++ b/manifest/ocischema/builder_test.go @@ -10,6 +10,7 @@ import ( "github.com/opencontainers/image-spec/specs-go/v1" ) +// TODO (not assigned): consider making mockBlobService common for ocischema, schema2, and schema1 type mockBlobService struct { descriptors map[digest.Digest]distribution.Descriptor } diff --git a/manifest/ocischema/manifest.go b/manifest/ocischema/manifest.go index a5b199d4..c3d9046a 100644 --- a/manifest/ocischema/manifest.go +++ b/manifest/ocischema/manifest.go @@ -57,7 +57,6 @@ func (m Manifest) References() []distribution.Descriptor { references := make([]distribution.Descriptor, 0, 1+len(m.Layers)) references = append(references, m.Config) references = append(references, m.Layers...) - // TODO: (mikebrow/stevvooe) should we return annotations as references return references } diff --git a/manifest/ocischema/manifest_test.go b/manifest/ocischema/manifest_test.go index 55dc0306..714c50a5 100644 --- a/manifest/ocischema/manifest_test.go +++ b/manifest/ocischema/manifest_test.go @@ -10,6 +10,7 @@ import ( "github.com/opencontainers/image-spec/specs-go/v1" ) +// TODO (mikebrow): add annotations to the test var expectedManifestSerialization = []byte(`{ "schemaVersion": 2, "mediaType": "application/vnd.oci.image.manifest.v1+json", diff --git a/registry/handlers/manifests.go b/registry/handlers/manifests.go index 2c6c7856..ad39ccb5 100644 --- a/registry/handlers/manifests.go +++ b/registry/handlers/manifests.go @@ -30,6 +30,17 @@ const ( imageClass = "image" ) +type storageType int + +const ( + manifestSchema1 storageType = iota // 0 + manifestSchema2 // 1 + manifestlistSchema // 2 + ociSchema // 3 + ociImageIndexSchema // 4 + numStorageTypes // 5 +) + // manifestDispatcher takes the request context and builds the // appropriate handler for handling manifest requests. func manifestDispatcher(ctx *Context, r *http.Request) http.Handler { @@ -75,10 +86,8 @@ func (imh *manifestHandler) GetManifest(w http.ResponseWriter, r *http.Request) imh.Errors = append(imh.Errors, err) return } - supportsSchema2 := false - supportsManifestList := false - supportsOCISchema := false - supportsOCIImageIndex := false + var supports [numStorageTypes]bool + // this parsing of Accept headers is not quite as full-featured as godoc.org's parser, but we don't care about "q=" values // https://github.com/golang/gddo/blob/e91d4165076d7474d20abda83f92d15c7ebc3e81/httputil/header/header.go#L165-L202 for _, acceptHeader := range r.Header["Accept"] { @@ -97,16 +106,16 @@ func (imh *manifestHandler) GetManifest(w http.ResponseWriter, r *http.Request) mediaType = strings.TrimSpace(mediaType) if mediaType == schema2.MediaTypeManifest { - supportsSchema2 = true + supports[manifestSchema2] = true } if mediaType == manifestlist.MediaTypeManifestList { - supportsManifestList = true + supports[manifestlistSchema] = true } if mediaType == v1.MediaTypeImageManifest { - supportsOCISchema = true + supports[ociSchema] = true } if mediaType == v1.MediaTypeImageIndex { - supportsOCIImageIndex = true + supports[ociImageIndexSchema] = true } } } @@ -145,36 +154,34 @@ func (imh *manifestHandler) GetManifest(w http.ResponseWriter, r *http.Request) } return } - + // determine the type of the returned manifest + manifestType := manifestSchema1 schema2Manifest, isSchema2 := manifest.(*schema2.DeserializedManifest) manifestList, isManifestList := manifest.(*manifestlist.DeserializedManifestList) - isAnOCIManifest := isSchema2 && (schema2Manifest.MediaType == v1.MediaTypeImageManifest) - isAnOCIImageIndex := isManifestList && (manifestList.MediaType == v1.MediaTypeImageIndex) + if isSchema2 { + manifestType = manifestSchema2 + } else if _, isOCImanifest := manifest.(*ocischema.DeserializedManifest); isOCImanifest { + manifestType = ociSchema + } else if isManifestList { + if manifestList.MediaType == manifestlist.MediaTypeManifestList { + manifestType = manifestlistSchema + } else if manifestList.MediaType == v1.MediaTypeImageIndex { + manifestType = ociImageIndexSchema + } + } - if (isSchema2 && !isAnOCIManifest) && (supportsOCISchema && !supportsSchema2) { - ctxu.GetLogger(imh).Debug("manifest is schema2, but accept header only supports OCISchema") - w.WriteHeader(http.StatusNotFound) + if manifestType == ociSchema && !supports[ociSchema] { + imh.Errors = append(imh.Errors, v2.ErrorCodeManifestUnknown.WithMessage("OCI manifest found, but accept header does not support OCI manifests")) return } - if (isManifestList && !isAnOCIImageIndex) && (supportsOCIImageIndex && !supportsManifestList) { - ctxu.GetLogger(imh).Debug("manifestlist is not OCI, but accept header only supports an OCI manifestlist") - w.WriteHeader(http.StatusNotFound) - return - } - if isAnOCIManifest && (!supportsOCISchema && supportsSchema2) { - ctxu.GetLogger(imh).Debug("manifest is OCI, but accept header only supports schema2") - w.WriteHeader(http.StatusNotFound) - return - } - if isAnOCIImageIndex && (!supportsOCIImageIndex && supportsManifestList) { - ctxu.GetLogger(imh).Debug("manifestlist is OCI, but accept header only supports non-OCI manifestlists") - w.WriteHeader(http.StatusNotFound) + if manifestType == ociImageIndexSchema && !supports[ociImageIndexSchema] { + imh.Errors = append(imh.Errors, v2.ErrorCodeManifestUnknown.WithMessage("OCI index found, but accept header does not support OCI indexes")) return } // Only rewrite schema2 manifests when they are being fetched by tag. // If they are being fetched by digest, we can't return something not // matching the digest. - if imh.Tag != "" && isSchema2 && !(supportsSchema2 || supportsOCISchema) { + if imh.Tag != "" && manifestType == manifestSchema2 && !supports[manifestSchema2] { // Rewrite manifest in schema1 format ctxu.GetLogger(imh).Infof("rewriting manifest %s in schema1 format to support old client", imh.Digest.String()) @@ -182,7 +189,7 @@ func (imh *manifestHandler) GetManifest(w http.ResponseWriter, r *http.Request) if err != nil { return } - } else if imh.Tag != "" && isManifestList && !(supportsManifestList || supportsOCIImageIndex) { + } else if imh.Tag != "" && manifestType == manifestlistSchema && !supports[manifestlistSchema] { // Rewrite manifest in schema1 format ctxu.GetLogger(imh).Infof("rewriting manifest list %s in schema1 format to support old client", imh.Digest.String()) @@ -212,7 +219,7 @@ func (imh *manifestHandler) GetManifest(w http.ResponseWriter, r *http.Request) } // If necessary, convert the image manifest - if schema2Manifest, isSchema2 := manifest.(*schema2.DeserializedManifest); isSchema2 && !(supportsSchema2 || supportsOCISchema) { + if schema2Manifest, isSchema2 := manifest.(*schema2.DeserializedManifest); isSchema2 && !supports[manifestSchema2] { manifest, err = imh.convertSchema2Manifest(schema2Manifest) if err != nil { return diff --git a/registry/storage/driver/storagedriver.go b/registry/storage/driver/storagedriver.go index 46c1b9e0..b220713f 100644 --- a/registry/storage/driver/storagedriver.go +++ b/registry/storage/driver/storagedriver.go @@ -116,7 +116,7 @@ type FileWriter interface { // number of path components separated by slashes, where each component is // restricted to alphanumeric characters or a period, underscore, or // hyphen. -var PathRegexp = regexp.MustCompile(`^(/[A-Za-z0-9._:-]+)+$`) +var PathRegexp = regexp.MustCompile(`^(/[A-Za-z0-9._-]+)+$`) // ErrUnsupportedMethod may be returned in the case where a StorageDriver implementation does not support an optional method. type ErrUnsupportedMethod struct {