diff --git a/manifest/schema1/config_builder.go b/manifest/schema1/config_builder.go index 5cdd76796..be0123731 100644 --- a/manifest/schema1/config_builder.go +++ b/manifest/schema1/config_builder.go @@ -9,11 +9,10 @@ import ( "github.com/docker/distribution" "github.com/docker/distribution/context" - "github.com/docker/distribution/reference" - "github.com/docker/libtrust" - "github.com/docker/distribution/digest" "github.com/docker/distribution/manifest" + "github.com/docker/distribution/reference" + "github.com/docker/libtrust" ) type diffID digest.Digest @@ -95,7 +94,7 @@ func (mb *configManifestBuilder) Build(ctx context.Context) (m distribution.Mani } if len(img.RootFS.DiffIDs) != len(mb.descriptors) { - return nil, errors.New("number of descriptors and number of layers in rootfs must match") + return nil, fmt.Errorf("number of descriptors and number of layers in rootfs must match: len(%v) != len(%v)", img.RootFS.DiffIDs, mb.descriptors) } // Generate IDs for each layer diff --git a/manifest/schema2/builder_test.go b/manifest/schema2/builder_test.go index 02ed401bf..851f917cb 100644 --- a/manifest/schema2/builder_test.go +++ b/manifest/schema2/builder_test.go @@ -203,8 +203,8 @@ func TestBuilder(t *testing.T) { } references := manifest.References() - - if !reflect.DeepEqual(references, descriptors) { + expected := append([]distribution.Descriptor{manifest.Target()}, descriptors...) + if !reflect.DeepEqual(references, expected) { t.Fatal("References() does not match the descriptors added") } } diff --git a/manifest/schema2/manifest.go b/manifest/schema2/manifest.go index dd2ed114c..1a1282709 100644 --- a/manifest/schema2/manifest.go +++ b/manifest/schema2/manifest.go @@ -69,7 +69,10 @@ type Manifest struct { // References returnes the descriptors of this manifests references. func (m Manifest) References() []distribution.Descriptor { - return m.Layers + references := make([]distribution.Descriptor, 0, 1+len(m.Layers)) + references = append(references, m.Config) + references = append(references, m.Layers...) + return references } // Target returns the target of this signed manifest. diff --git a/manifest/schema2/manifest_test.go b/manifest/schema2/manifest_test.go index 459d614cd..f0003d18b 100644 --- a/manifest/schema2/manifest_test.go +++ b/manifest/schema2/manifest_test.go @@ -90,16 +90,22 @@ func TestManifest(t *testing.T) { } references := deserialized.References() - if len(references) != 1 { + if len(references) != 2 { t.Fatalf("unexpected number of references: %d", len(references)) } - if references[0].Digest != "sha256:62d8908bee94c202b2d35224a221aaa2058318bfa9879fa541efaecba272331b" { + + if !reflect.DeepEqual(references[0], target) { + t.Fatalf("first reference should be target: %v != %v", references[0], target) + } + + // Test the second reference + if references[1].Digest != "sha256:62d8908bee94c202b2d35224a221aaa2058318bfa9879fa541efaecba272331b" { t.Fatalf("unexpected digest in reference: %s", references[0].Digest.String()) } - if references[0].MediaType != MediaTypeLayer { + if references[1].MediaType != MediaTypeLayer { t.Fatalf("unexpected media type in reference: %s", references[0].MediaType) } - if references[0].Size != 153263 { + if references[1].Size != 153263 { t.Fatalf("unexpected size in reference: %d", references[0].Size) } } diff --git a/manifests.go b/manifests.go index 2ac7c8f21..c4fb63450 100644 --- a/manifests.go +++ b/manifests.go @@ -12,8 +12,13 @@ import ( // references and an optional target type Manifest interface { // References returns a list of objects which make up this manifest. - // The references are strictly ordered from base to head. A reference - // is anything which can be represented by a distribution.Descriptor + // A reference is anything which can be represented by a + // distribution.Descriptor. These can consist of layers, resources or other + // manifests. + // + // While no particular order is required, implementations should return + // them from highest to lowest priority. For example, one might want to + // return the base layer before the top layer. References() []Descriptor // Payload provides the serialized format of the manifest, in addition to @@ -36,6 +41,9 @@ type ManifestBuilder interface { // AppendReference includes the given object in the manifest after any // existing dependencies. If the add fails, such as when adding an // unsupported dependency, an error may be returned. + // + // The destination of the reference is dependent on the manifest type and + // the dependency type. AppendReference(dependency Describable) error } diff --git a/registry/handlers/images.go b/registry/handlers/images.go index df7f869be..96316348c 100644 --- a/registry/handlers/images.go +++ b/registry/handlers/images.go @@ -205,7 +205,7 @@ func (imh *imageManifestHandler) convertSchema2Manifest(schema2Manifest *schema2 } builder := schema1.NewConfigManifestBuilder(imh.Repository.Blobs(imh), imh.Context.App.trustKey, ref, configJSON) - for _, d := range schema2Manifest.References() { + for _, d := range schema2Manifest.Layers { if err := builder.AppendReference(d); err != nil { imh.Errors = append(imh.Errors, v2.ErrorCodeManifestInvalid.WithDetail(err)) return nil, err diff --git a/registry/storage/garbagecollect.go b/registry/storage/garbagecollect.go index fe3bd9b92..e982c994c 100644 --- a/registry/storage/garbagecollect.go +++ b/registry/storage/garbagecollect.go @@ -6,7 +6,6 @@ import ( "github.com/docker/distribution" "github.com/docker/distribution/context" "github.com/docker/distribution/digest" - "github.com/docker/distribution/manifest/schema2" "github.com/docker/distribution/reference" "github.com/docker/distribution/registry/storage/driver" ) @@ -63,14 +62,6 @@ func MarkAndSweep(ctx context.Context, storageDriver driver.StorageDriver, regis emit("%s: marking blob %s", repoName, descriptor.Digest) } - switch manifest.(type) { - case *schema2.DeserializedManifest: - config := manifest.(*schema2.DeserializedManifest).Config - emit("%s: marking configuration %s", repoName, config.Digest) - markSet[config.Digest] = struct{}{} - break - } - return nil }) diff --git a/registry/storage/schema2manifesthandler.go b/registry/storage/schema2manifesthandler.go index 1d221410e..9fe71bb4d 100644 --- a/registry/storage/schema2manifesthandler.go +++ b/registry/storage/schema2manifesthandler.go @@ -9,6 +9,7 @@ import ( "github.com/docker/distribution" "github.com/docker/distribution/context" "github.com/docker/distribution/digest" + "github.com/docker/distribution/manifest/schema1" "github.com/docker/distribution/manifest/schema2" ) @@ -71,53 +72,62 @@ func (ms *schema2ManifestHandler) Put(ctx context.Context, manifest distribution func (ms *schema2ManifestHandler) verifyManifest(ctx context.Context, mnfst schema2.DeserializedManifest, skipDependencyVerification bool) error { var errs distribution.ErrManifestVerification - if !skipDependencyVerification { - target := mnfst.Target() - _, err := ms.repository.Blobs(ctx).Stat(ctx, target.Digest) + if skipDependencyVerification { + return nil + } + + manifestService, err := ms.repository.Manifests(ctx) + if err != nil { + return err + } + + blobsService := ms.repository.Blobs(ctx) + + for _, descriptor := range mnfst.References() { + var err error + + switch descriptor.MediaType { + case schema2.MediaTypeForeignLayer: + // Clients download this layer from an external URL, so do not check for + // its presense. + if len(descriptor.URLs) == 0 { + err = errMissingURL + } + allow := ms.manifestURLs.allow + deny := ms.manifestURLs.deny + for _, u := range descriptor.URLs { + var pu *url.URL + pu, err = url.Parse(u) + if err != nil || (pu.Scheme != "http" && pu.Scheme != "https") || pu.Fragment != "" || (allow != nil && !allow.MatchString(u)) || (deny != nil && deny.MatchString(u)) { + err = errInvalidURL + break + } + } + case schema2.MediaTypeManifest, schema1.MediaTypeManifest: + var exists bool + exists, err = manifestService.Exists(ctx, descriptor.Digest) + if err != nil || !exists { + err = distribution.ErrBlobUnknown // just coerce to unknown. + } + + fallthrough // double check the blob store. + default: + // forward all else to blob storage + if len(descriptor.URLs) == 0 { + _, err = blobsService.Stat(ctx, descriptor.Digest) + } + } + if err != nil { if err != distribution.ErrBlobUnknown { errs = append(errs, err) } // On error here, we always append unknown blob errors. - errs = append(errs, distribution.ErrManifestBlobUnknown{Digest: target.Digest}) - } - - for _, fsLayer := range mnfst.References() { - var err error - if fsLayer.MediaType != schema2.MediaTypeForeignLayer { - if len(fsLayer.URLs) == 0 { - _, err = ms.repository.Blobs(ctx).Stat(ctx, fsLayer.Digest) - } else { - err = errUnexpectedURL - } - } else { - // Clients download this layer from an external URL, so do not check for - // its presense. - if len(fsLayer.URLs) == 0 { - err = errMissingURL - } - allow := ms.manifestURLs.allow - deny := ms.manifestURLs.deny - for _, u := range fsLayer.URLs { - var pu *url.URL - pu, err = url.Parse(u) - if err != nil || (pu.Scheme != "http" && pu.Scheme != "https") || pu.Fragment != "" || (allow != nil && !allow.MatchString(u)) || (deny != nil && deny.MatchString(u)) { - err = errInvalidURL - break - } - } - } - if err != nil { - if err != distribution.ErrBlobUnknown { - errs = append(errs, err) - } - - // On error here, we always append unknown blob errors. - errs = append(errs, distribution.ErrManifestBlobUnknown{Digest: fsLayer.Digest}) - } + errs = append(errs, distribution.ErrManifestBlobUnknown{Digest: descriptor.Digest}) } } + if len(errs) != 0 { return errs } diff --git a/registry/storage/schema2manifesthandler_test.go b/registry/storage/schema2manifesthandler_test.go index 73a7e336a..5051fa3de 100644 --- a/registry/storage/schema2manifesthandler_test.go +++ b/registry/storage/schema2manifesthandler_test.go @@ -57,9 +57,10 @@ func TestVerifyManifestForeignLayer(t *testing.T) { errMissingURL, }, { + // regular layers may have foreign urls layer, []string{"http://foo/bar"}, - errUnexpectedURL, + nil, }, { foreignLayer,