From 2c23ef615e7da9cd9221167a655c6156ce10b2b9 Mon Sep 17 00:00:00 2001 From: "Lixia (Sylvia) Lei" Date: Tue, 25 Jul 2023 10:26:10 +0800 Subject: [PATCH] feat: update Referrers API for distribution-spec v1.1.0-rc3 (#553) Resolves: #443 Resolves: #533 Signed-off-by: Lixia (Sylvia) Lei --- registry/remote/referrers.go | 7 +- registry/remote/referrers_test.go | 72 ++++++------- registry/remote/repository.go | 60 ++++++++--- registry/remote/repository_test.go | 167 ++++++++++++++++++++++++++++- 4 files changed, 244 insertions(+), 62 deletions(-) diff --git a/registry/remote/referrers.go b/registry/remote/referrers.go index a3ed08ca..8c304953 100644 --- a/registry/remote/referrers.go +++ b/registry/remote/referrers.go @@ -22,7 +22,6 @@ import ( ocispec "github.com/opencontainers/image-spec/specs-go/v1" "oras.land/oras-go/v2/content" "oras.land/oras-go/v2/internal/descriptor" - "oras.land/oras-go/v2/internal/spec" ) // zeroDigest represents a digest that consists of zeros. zeroDigest is used @@ -110,10 +109,8 @@ func buildReferrersTag(desc ocispec.Descriptor) string { return alg + "-" + encoded } -// isReferrersFilterApplied checks annotations to see if requested is in the -// applied filter list. -func isReferrersFilterApplied(annotations map[string]string, requested string) bool { - applied := annotations[spec.AnnotationReferrersFiltersApplied] +// isReferrersFilterApplied checks if requsted is in the applied filter list. +func isReferrersFilterApplied(applied, requested string) bool { if applied == "" || requested == "" { return false } diff --git a/registry/remote/referrers_test.go b/registry/remote/referrers_test.go index 1b8b98f8..8a1bdda3 100644 --- a/registry/remote/referrers_test.go +++ b/registry/remote/referrers_test.go @@ -63,63 +63,57 @@ func Test_buildReferrersTag(t *testing.T) { func Test_isReferrersFilterApplied(t *testing.T) { tests := []struct { - name string - annotations map[string]string - requested string - want bool + name string + applied string + requested string + want bool }{ { - name: "single filter applied, specified filter matches", - annotations: map[string]string{spec.AnnotationReferrersFiltersApplied: "artifactType"}, - requested: "artifactType", - want: true, + name: "single filter applied, specified filter matches", + applied: "artifactType", + requested: "artifactType", + want: true, }, { - name: "single filter applied, specified filter does not match", - annotations: map[string]string{spec.AnnotationReferrersFiltersApplied: "foo"}, - requested: "artifactType", - want: false, + name: "single filter applied, specified filter does not match", + applied: "foo", + requested: "artifactType", + want: false, }, { - name: "multiple filters applied, specified filter matches", - annotations: map[string]string{spec.AnnotationReferrersFiltersApplied: "foo,artifactType"}, - requested: "artifactType", - want: true, + name: "multiple filters applied, specified filter matches", + applied: "foo,artifactType", + requested: "artifactType", + want: true, }, { - name: "multiple filters applied, specified filter does not match", - annotations: map[string]string{spec.AnnotationReferrersFiltersApplied: "foo,bar"}, - requested: "artifactType", - want: false, + name: "multiple filters applied, specified filter does not match", + applied: "foo,bar", + requested: "artifactType", + want: false, }, { - name: "single filter applied, specified filter empty", - annotations: map[string]string{spec.AnnotationReferrersFiltersApplied: "foo"}, - requested: "", - want: false, + name: "single filter applied, no specified filter", + applied: "foo", + requested: "", + want: false, }, { - name: "no filter applied", - annotations: map[string]string{}, - requested: "artifactType", - want: false, + name: "no filter applied, specified filter does not match", + applied: "", + requested: "artifactType", + want: false, }, { - name: "empty filter applied", - annotations: map[string]string{spec.AnnotationReferrersFiltersApplied: ""}, - requested: "artifactType", - want: false, - }, - { - name: "no filter applied, specified filter empty", - annotations: map[string]string{}, - requested: "", - want: false, + name: "no filter applied, no specified filter", + applied: "", + requested: "", + want: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := isReferrersFilterApplied(tt.annotations, tt.requested); got != tt.want { + if got := isReferrersFilterApplied(tt.applied, tt.requested); got != tt.want { t.Errorf("isReferrersFilterApplied() = %v, want %v", got, tt.want) } }) diff --git a/registry/remote/repository.go b/registry/remote/repository.go index c8c45baa..59abb909 100644 --- a/registry/remote/repository.go +++ b/registry/remote/repository.go @@ -47,11 +47,32 @@ import ( "oras.land/oras-go/v2/registry/remote/internal/errutil" ) -// dockerContentDigestHeader - The Docker-Content-Digest header, if present -// on the response, returns the canonical digest of the uploaded blob. -// See https://docs.docker.com/registry/spec/api/#digest-header -// See https://github.com/opencontainers/distribution-spec/blob/v1.1.0-rc1/spec.md#pull -const dockerContentDigestHeader = "Docker-Content-Digest" +const ( + // headerDockerContentDigest is the "Docker-Content-Digest" header. + // If present on the response, it contains the canonical digest of the + // uploaded blob. + // + // References: + // - https://docs.docker.com/registry/spec/api/#digest-header + // - https://github.com/opencontainers/distribution-spec/blob/v1.1.0-rc1/spec.md#pull + headerDockerContentDigest = "Docker-Content-Digest" + + // headerOCIFiltersApplied is the "OCI-Filters-Applied" header. + // If present on the response, it contains a comma-separated list of the + // applied filters. + // + // Reference: + // - https://github.com/opencontainers/distribution-spec/blob/v1.1.0-rc3/spec.md#listing-referrers + headerOCIFiltersApplied = "OCI-Filters-Applied" +) + +// filterTypeArtifactType is the "artifactType" filter applied on the list of +// referrers. +// +// References: +// - https://github.com/opencontainers/distribution-spec/blob/v1.1.0-rc3/spec.md#listing-referrers +// - https://github.com/opencontainers/distribution-spec/blob/v1.1.0-rc1/spec.md#listing-referrers +const filterTypeArtifactType = "artifactType" // Client is an interface for a HTTP client. type Client interface { @@ -497,10 +518,19 @@ func (r *Repository) referrersPageByAPI(ctx context.Context, artifactType string if err := json.NewDecoder(lr).Decode(&index); err != nil { return "", fmt.Errorf("%s %q: failed to decode response: %w", resp.Request.Method, resp.Request.URL, err) } + referrers := index.Manifests - if artifactType != "" && !isReferrersFilterApplied(index.Annotations, "artifactType") { - // perform client side filtering if the filter is not applied on the server side - referrers = filterReferrers(referrers, artifactType) + if artifactType != "" { + // check both filters header and filters annotations for compatibility + // reference for filters header: https://github.com/opencontainers/distribution-spec/blob/v1.1.0-rc3/spec.md#listing-referrers + // reference for filters annotations: https://github.com/opencontainers/distribution-spec/blob/v1.1.0-rc1/spec.md#listing-referrers + filtersHeader := resp.Header.Get(headerOCIFiltersApplied) + filtersAnnotation := index.Annotations[spec.AnnotationReferrersFiltersApplied] + if !isReferrersFilterApplied(filtersHeader, filterTypeArtifactType) && + !isReferrersFilterApplied(filtersAnnotation, filterTypeArtifactType) { + // perform client side filtering if the filter is not applied on the server side + referrers = filterReferrers(referrers, artifactType) + } } if len(referrers) > 0 { if err := fn(referrers); err != nil { @@ -1420,13 +1450,13 @@ func (s *manifestStore) generateDescriptor(resp *http.Response, ref registry.Ref // 4. Validate Server Digest (if present) var serverHeaderDigest digest.Digest - if serverHeaderDigestStr := resp.Header.Get(dockerContentDigestHeader); serverHeaderDigestStr != "" { + if serverHeaderDigestStr := resp.Header.Get(headerDockerContentDigest); serverHeaderDigestStr != "" { if serverHeaderDigest, err = digest.Parse(serverHeaderDigestStr); err != nil { return ocispec.Descriptor{}, fmt.Errorf( "%s %q: invalid response header value: `%s: %s`; %w", resp.Request.Method, resp.Request.URL, - dockerContentDigestHeader, + headerDockerContentDigest, serverHeaderDigestStr, err, ) @@ -1443,7 +1473,7 @@ func (s *manifestStore) generateDescriptor(resp *http.Response, ref registry.Ref // immediate fail return ocispec.Descriptor{}, fmt.Errorf( "HTTP %s request missing required header %q", - httpMethod, dockerContentDigestHeader, + httpMethod, headerDockerContentDigest, ) } // Otherwise, just trust the client-supplied digest @@ -1465,7 +1495,7 @@ func (s *manifestStore) generateDescriptor(resp *http.Response, ref registry.Ref return ocispec.Descriptor{}, fmt.Errorf( "%s %q: invalid response; digest mismatch in %s: received %q when expecting %q", resp.Request.Method, resp.Request.URL, - dockerContentDigestHeader, contentDigest, + headerDockerContentDigest, contentDigest, refDigest, ) } @@ -1497,7 +1527,7 @@ func calculateDigestFromResponse(resp *http.Response, maxMetadataBytes int64) (d // OCI distribution-spec states the Docker-Content-Digest header is optional. // Reference: https://github.com/opencontainers/distribution-spec/blob/v1.0.1/spec.md#legacy-docker-support-http-headers func verifyContentDigest(resp *http.Response, expected digest.Digest) error { - digestStr := resp.Header.Get(dockerContentDigestHeader) + digestStr := resp.Header.Get(headerDockerContentDigest) if len(digestStr) == 0 { return nil @@ -1508,7 +1538,7 @@ func verifyContentDigest(resp *http.Response, expected digest.Digest) error { return fmt.Errorf( "%s %q: invalid response header: `%s: %s`", resp.Request.Method, resp.Request.URL, - dockerContentDigestHeader, digestStr, + headerDockerContentDigest, digestStr, ) } @@ -1516,7 +1546,7 @@ func verifyContentDigest(resp *http.Response, expected digest.Digest) error { return fmt.Errorf( "%s %q: invalid response; digest mismatch in %s: received %q when expecting %q", resp.Request.Method, resp.Request.URL, - dockerContentDigestHeader, contentDigest, + headerDockerContentDigest, contentDigest, expected, ) } diff --git a/registry/remote/repository_test.go b/registry/remote/repository_test.go index deb894cb..a18d4166 100644 --- a/registry/remote/repository_test.go +++ b/registry/remote/repository_test.go @@ -321,7 +321,7 @@ func TestRepository_Mount(t *testing.T) { t.Errorf("unexpected value for 'from' parameter; got %q want %q", got, want) } gotMount++ - w.Header().Set(dockerContentDigestHeader, blobDesc.Digest.String()) + w.Header().Set(headerDockerContentDigest, blobDesc.Digest.String()) w.WriteHeader(201) return default: @@ -1892,6 +1892,8 @@ func TestRepository_Referrers_ServerFiltering(t *testing.T) { }, }, } + + // Test with filter annotations only var ts *httptest.Server ts = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { path := "/v2/test/referrers/" + manifestDesc.Digest.String() @@ -1930,6 +1932,7 @@ func TestRepository_Referrers_ServerFiltering(t *testing.T) { }, MediaType: ocispec.MediaTypeImageIndex, Manifests: referrers, + // set filter annotations Annotations: map[string]string{ spec.AnnotationReferrersFiltersApplied: "artifactType", }, @@ -1969,6 +1972,164 @@ func TestRepository_Referrers_ServerFiltering(t *testing.T) { if index != len(referrerSet) { t.Errorf("fn invoked %d time(s), want %d", index, len(referrerSet)) } + + // Test with filter header only + ts = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + path := "/v2/test/referrers/" + manifestDesc.Digest.String() + queryParams, err := url.ParseQuery(r.URL.RawQuery) + if err != nil { + t.Fatal("failed to parse url query") + } + if r.Method != http.MethodGet || + r.URL.Path != path || + reflect.DeepEqual(queryParams["artifactType"], []string{"application%2Fvnd.test"}) { + t.Errorf("unexpected access: %s %q", r.Method, r.URL) + w.WriteHeader(http.StatusNotFound) + return + } + q := r.URL.Query() + n, err := strconv.Atoi(q.Get("n")) + if err != nil || n != 2 { + t.Errorf("bad page size: %s", q.Get("n")) + w.WriteHeader(http.StatusBadRequest) + return + } + var referrers []ocispec.Descriptor + switch q.Get("test") { + case "foo": + referrers = referrerSet[1] + w.Header().Set("Link", fmt.Sprintf(`<%s%s?n=2&test=bar>; rel="next"`, ts.URL, path)) + case "bar": + referrers = referrerSet[2] + default: + referrers = referrerSet[0] + w.Header().Set("Link", fmt.Sprintf(`<%s?n=2&test=foo>; rel="next"`, path)) + } + result := ocispec.Index{ + Versioned: specs.Versioned{ + SchemaVersion: 2, // historical value. does not pertain to OCI or docker version + }, + MediaType: ocispec.MediaTypeImageIndex, + Manifests: referrers, + } + // set filter header + w.Header().Set("OCI-Filters-Applied", "artifactType") + if err := json.NewEncoder(w).Encode(result); err != nil { + t.Errorf("failed to write response: %v", err) + } + })) + defer ts.Close() + uri, err = url.Parse(ts.URL) + if err != nil { + t.Fatalf("invalid test http server: %v", err) + } + + repo, err = NewRepository(uri.Host + "/test") + if err != nil { + t.Fatalf("NewRepository() error = %v", err) + } + repo.PlainHTTP = true + repo.ReferrerListPageSize = 2 + + ctx = context.Background() + index = 0 + if err := repo.Referrers(ctx, manifestDesc, "application/vnd.test", func(got []ocispec.Descriptor) error { + if index >= len(referrerSet) { + t.Fatalf("out of index bound: %d", index) + } + referrers := referrerSet[index] + index++ + if !reflect.DeepEqual(got, referrers) { + t.Errorf("Repository.Referrers() = %v, want %v", got, referrers) + } + return nil + }); err != nil { + t.Errorf("Repository.Referrers() error = %v", err) + } + if index != len(referrerSet) { + t.Errorf("fn invoked %d time(s), want %d", index, len(referrerSet)) + } + + // Test with both filter annotation and filter header + ts = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + path := "/v2/test/referrers/" + manifestDesc.Digest.String() + queryParams, err := url.ParseQuery(r.URL.RawQuery) + if err != nil { + t.Fatal("failed to parse url query") + } + if r.Method != http.MethodGet || + r.URL.Path != path || + reflect.DeepEqual(queryParams["artifactType"], []string{"application%2Fvnd.test"}) { + t.Errorf("unexpected access: %s %q", r.Method, r.URL) + w.WriteHeader(http.StatusNotFound) + return + } + q := r.URL.Query() + n, err := strconv.Atoi(q.Get("n")) + if err != nil || n != 2 { + t.Errorf("bad page size: %s", q.Get("n")) + w.WriteHeader(http.StatusBadRequest) + return + } + var referrers []ocispec.Descriptor + switch q.Get("test") { + case "foo": + referrers = referrerSet[1] + w.Header().Set("Link", fmt.Sprintf(`<%s%s?n=2&test=bar>; rel="next"`, ts.URL, path)) + case "bar": + referrers = referrerSet[2] + default: + referrers = referrerSet[0] + w.Header().Set("Link", fmt.Sprintf(`<%s?n=2&test=foo>; rel="next"`, path)) + } + result := ocispec.Index{ + Versioned: specs.Versioned{ + SchemaVersion: 2, // historical value. does not pertain to OCI or docker version + }, + MediaType: ocispec.MediaTypeImageIndex, + Manifests: referrers, + // set filter annotation + Annotations: map[string]string{ + spec.AnnotationReferrersFiltersApplied: "artifactType", + }, + } + // set filter header + w.Header().Set("OCI-Filters-Applied", "artifactType") + if err := json.NewEncoder(w).Encode(result); err != nil { + t.Errorf("failed to write response: %v", err) + } + })) + defer ts.Close() + uri, err = url.Parse(ts.URL) + if err != nil { + t.Fatalf("invalid test http server: %v", err) + } + + repo, err = NewRepository(uri.Host + "/test") + if err != nil { + t.Fatalf("NewRepository() error = %v", err) + } + repo.PlainHTTP = true + repo.ReferrerListPageSize = 2 + + ctx = context.Background() + index = 0 + if err := repo.Referrers(ctx, manifestDesc, "application/vnd.test", func(got []ocispec.Descriptor) error { + if index >= len(referrerSet) { + t.Fatalf("out of index bound: %d", index) + } + referrers := referrerSet[index] + index++ + if !reflect.DeepEqual(got, referrers) { + t.Errorf("Repository.Referrers() = %v, want %v", got, referrers) + } + return nil + }); err != nil { + t.Errorf("Repository.Referrers() error = %v", err) + } + if index != len(referrerSet) { + t.Errorf("fn invoked %d time(s), want %d", index, len(referrerSet)) + } } func TestRepository_Referrers_ClientFiltering(t *testing.T) { @@ -2940,7 +3101,7 @@ func Test_generateBlobDescriptorWithVariousDockerContentDigestHeaders(t *testing resp := http.Response{ Header: http.Header{ "Content-Type": []string{"application/vnd.docker.distribution.manifest.v2+json"}, - dockerContentDigestHeader: []string{dcdIOStruct.serverCalculatedDigest.String()}, + headerDockerContentDigest: []string{dcdIOStruct.serverCalculatedDigest.String()}, }, } if method == http.MethodGet { @@ -4953,7 +5114,7 @@ func Test_ManifestStore_generateDescriptorWithVariousDockerContentDigestHeaders( resp := http.Response{ Header: http.Header{ "Content-Type": []string{"application/vnd.docker.distribution.manifest.v2+json"}, - dockerContentDigestHeader: []string{dcdIOStruct.serverCalculatedDigest.String()}, + headerDockerContentDigest: []string{dcdIOStruct.serverCalculatedDigest.String()}, }, } if method == http.MethodGet {