ociregistry/{ociclient,ociserver}: better Referrers support

- support artifact type filtering
- include (and recognize) `OCI-Filters-Applied` header according to the spec
- implement paging when the server only returns a limited set of referrers results
- implement referrers-tag fallback behavior when the Referrers API is
not supported by a server.
- add a server option to disable artifactType filtering

Updates #6
Fixes #35
Fixes #40
Fixes #41
Fixes #42

Signed-off-by: Roger Peppe <[email protected]>
Change-Id: Icd040930924b99f916e07d7848e3a5cae7b63b1f
Reviewed-on: https://review.gerrithub.io/c/cue-labs/oci/+/1218339
TryBot-Result: CUE porcuepine <[email protected]>
Reviewed-by: Daniel Martí <[email protected]>

Author: rogpeppe

ociregistry/internal/conformance/conformance_test.go

@@ -60,6 +60,20 @@ func TestMem(t *testing.T) {
 	})
 }
 
+func TestServerWithReferrersFilteringDisabled(t *testing.T) {
+	// Just run the distribution tests because the extra tests expect
+	// the referrers API to be enabled and fully working.
+	t.Run("distribution", func(t *testing.T) {
+		testDistribution(t, func(t *testing.T) string {
+			srv := httptest.NewServer(ociserver.New(ocidebug.New(ocimem.New(), t.Logf), &ociserver.Options{
+				DisableReferrersFiltering: true,
+			}))
+			t.Cleanup(srv.Close)
+			return srv.URL
+		})
+	})
+}
+
 func TestClientAsProxy(t *testing.T) {
 	runTests(t, func(t *testing.T) string {
 		direct := httptest.NewServer(ociserver.New(ocimem.New(), &ociserver.Options{

ociregistry/internal/ocirequest/create.go

@@ -40,7 +40,7 @@ func (req *Request) MustConstruct() (method string, ustr string) {
 	return method, ustr
 }
 
-func (req *Request) construct() (method string, url string) {
+func (req *Request) construct() (method string, urlStr string) {
 	switch req.Kind {
 	case ReqPing:
 		return "GET", "/v2/"
@@ -77,7 +77,11 @@ func (req *Request) construct() (method string, url string) {
 	case ReqTagsList:
 		return "GET", "/v2/" + req.Repo + "/tags/list" + req.listParams()
 	case ReqReferrersList:
-		return "GET", "/v2/" + req.Repo + "/referrers/" + req.Digest
+		p := "/v2/" + req.Repo + "/referrers/" + req.Digest
+		if req.ArtifactType != "" {
+			p += "?" + url.Values{"artifactType": {req.ArtifactType}}.Encode()
+		}
+		return "GET", p
 	case ReqCatalogList:
 		return "GET", "/v2/_catalog" + req.listParams()
 	default:

ociregistry/internal/ocirequest/request.go

@@ -91,17 +91,22 @@ type Request struct {
 	// Valid for:
 	//	ReqTagsList
 	//	ReqCatalog
-	//	ReqReferrers
 	ListN int
 
-	// listLast holds the item to start just after
+	// ListLast holds the item to start just after
 	// when listing.
 	//
 	// Valid for:
 	//	ReqTagsList
 	//	ReqCatalog
-	//	ReqReferrers
 	ListLast string
+
+	// ArtifactType holds the artifact type to filter by when
+	// listing.
+	//
+	// Valid for:
+	//	ReqReferrersList
+	ArtifactType string
 }
 
 type Kind int
@@ -373,10 +378,11 @@ func Parse(method string, u *url.URL) (*Request, error) {
 		if !ociref.IsValidRepository(rreq.Repo) {
 			return nil, ociregistry.ErrNameInvalid
 		}
-		// TODO is there any kind of pagination for referrers?
-		// We'll set ListN to be future-proof.
+		// Unlike other list-oriented endpoints, there appears to be no defined way for the client
+		// to indicate the desired number of results, but set ListN anyway to be future-proof.
 		rreq.ListN = -1
 		rreq.Digest = last
+		rreq.ArtifactType = urlq.Get("artifactType")
 		rreq.Kind = ReqReferrersList
 		return &rreq, nil
 	}

ociregistry/ociclient/lister.go

@@ -17,9 +17,11 @@ package ociclient
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
+	"slices"
 	"strings"
 
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
@@ -29,11 +31,11 @@ import (
 )
 
 func (c *client) Repositories(ctx context.Context, startAfter string) ociregistry.Seq[string] {
-	return c.pager(ctx, &ocirequest.Request{
+	return pager(ctx, c, &ocirequest.Request{
 		Kind:     ocirequest.ReqCatalogList,
 		ListN:    c.listPageSize,
 		ListLast: startAfter,
-	}, func(resp *http.Response) ([]string, error) {
+	}, true, func(resp *http.Response) ([]string, error) {
 		data, err := io.ReadAll(resp.Body)
 		if err != nil {
 			return nil, err
@@ -49,12 +51,12 @@ func (c *client) Repositories(ctx context.Context, startAfter string) ociregistr
 }
 
 func (c *client) Tags(ctx context.Context, repoName, startAfter string) ociregistry.Seq[string] {
-	return c.pager(ctx, &ocirequest.Request{
+	return pager(ctx, c, &ocirequest.Request{
 		Kind:     ocirequest.ReqTagsList,
 		Repo:     repoName,
 		ListN:    c.listPageSize,
 		ListLast: startAfter,
-	}, func(resp *http.Response) ([]string, error) {
+	}, true, func(resp *http.Response) ([]string, error) {
 		data, err := io.ReadAll(resp.Body)
 		if err != nil {
 			return nil, err
@@ -71,54 +73,80 @@ func (c *client) Tags(ctx context.Context, repoName, startAfter string) ociregis
 }
 
 func (c *client) Referrers(ctx context.Context, repoName string, digest ociregistry.Digest, artifactType string) ociregistry.Seq[ociregistry.Descriptor] {
-	// TODO paging
-	resp, err := c.doRequest(ctx, &ocirequest.Request{
-		Kind:   ocirequest.ReqReferrersList,
-		Repo:   repoName,
-		Digest: string(digest),
-		ListN:  c.listPageSize,
-	})
-	if err != nil {
-		return ociregistry.ErrorSeq[ociregistry.Descriptor](err)
-	}
-
-	data, err := io.ReadAll(resp.Body)
-	resp.Body.Close()
-	if err != nil {
-		return ociregistry.ErrorSeq[ociregistry.Descriptor](err)
-	}
-	var referrersResponse ocispec.Index
-	if err := json.Unmarshal(data, &referrersResponse); err != nil {
-		return ociregistry.ErrorSeq[ociregistry.Descriptor](fmt.Errorf("cannot unmarshal referrers response: %v", err))
-	}
-	return ociregistry.SliceSeq(referrersResponse.Manifests)
+	return pager(ctx, c, &ocirequest.Request{
+		Kind:         ocirequest.ReqReferrersList,
+		Repo:         repoName,
+		Digest:       string(digest),
+		ListN:        c.listPageSize,
+		ArtifactType: artifactType,
+	}, false, func(resp *http.Response) ([]ociregistry.Descriptor, error) {
+		body := resp.Body
+		if resp.StatusCode == http.StatusNotFound {
+			body.Close()
+			body = nil
+			// Fall back to the referrers tag API.
+			// From https://github.com/opencontainers/distribution-spec/blob/main/spec.md#unavailable-referrers-api :
+			//	A client querying the referrers API and receiving a
+			//	404 Not Found MUST fallback to using an image index
+			//	pushed to a tag described by the referrers tag
+			//	schema.
+			r, err := c.GetTag(ctx, repoName, referrersTag(digest))
+			if err != nil {
+				if errors.Is(err, ociregistry.ErrManifestUnknown) {
+					return nil, nil
+				}
+				return nil, err
+			}
+			body = r
+		}
+		data, err := io.ReadAll(body)
+		body.Close()
+		if err != nil {
+			return nil, err
+		}
+		var referrersResponse ocispec.Index
+		if err := json.Unmarshal(data, &referrersResponse); err != nil {
+			return nil, fmt.Errorf("cannot unmarshal referrers response: %v", err)
+		}
+		if artifactType == "" || resp.Header.Get("OCI-Filters-Applied") == "artifactType" {
+			return referrersResponse.Manifests, nil
+		}
+		// The server hasn't filtered the responses, so we must.
+		// TODO is it OK to assume that the index contains correctly populated
+		// artifact type and attributes fields when we've fallen back to the referrer tags API?
+		// If not, we might have to retrieve all the individual manifests to check that info.
+		manifests := slices.DeleteFunc(referrersResponse.Manifests, func(desc ociregistry.Descriptor) bool {
+			return desc.ArtifactType != artifactType
+		})
+		return manifests, nil
+	}, http.StatusOK, http.StatusNotFound)
 }
 
 // pager returns an iterator for a list entry point. It starts by sending the given
 // initial request and parses each response into its component items using
 // parseResponse. It tries to use the Link header in each response to continue
-// the iteration, falling back to using the "last" query parameter.
-func (c *client) pager(ctx context.Context, initialReq *ocirequest.Request, parseResponse func(*http.Response) ([]string, error)) ociregistry.Seq[string] {
-	return func(yield func(string, error) bool) {
-		// We assume that the same scope is applicable to all page requests.
+// the iteration, falling back to using the "last" query parameter if
+// canUseLast is true.
+func pager[T any](ctx context.Context, c *client, initialReq *ocirequest.Request, canUseLast bool, parseResponse func(*http.Response) ([]T, error), okStatuses ...int) ociregistry.Seq[T] {
+	return func(yield func(T, error) bool) {
+		// We assume that the same auth scope is applicable to all page requests.
 		req, err := newRequest(ctx, initialReq, nil)
 		if err != nil {
-			yield("", err)
+			yield(*new(T), err)
 			return
 		}
 		for {
-			resp, err := c.do(req)
+			resp, err := c.do(req, okStatuses...)
 			if err != nil {
-				yield("", err)
+				yield(*new(T), err)
 				return
 			}
 			items, err := parseResponse(resp)
 			resp.Body.Close()
 			if err != nil {
-				yield("", err)
+				yield(*new(T), err)
 				return
 			}
-			// TODO sanity check that items are in lexical order?
 			for _, item := range items {
 				if !yield(item, nil) {
 					return
@@ -131,28 +159,35 @@ func (c *client) pager(ctx context.Context, initialReq *ocirequest.Request, pars
 				//     is less than <int>.
 				return
 			}
-			req, err = nextLink(ctx, resp, initialReq, items[len(items)-1])
+			req, err = nextLink(ctx, resp, initialReq, canUseLast, items[len(items)-1])
 			if err != nil {
-				yield("", fmt.Errorf("invalid Link header in response: %v", err))
+				yield(*new(T), fmt.Errorf("invalid Link header in response: %v", err))
+				return
+			}
+			if req == nil {
+				// No link found; assume there are no more items.
 				return
 			}
 		}
 	}
 }
 
-// nextLink tries to form a request that can be sent to obtain the next page
+// nextLink ttries to form a request that can be sent to obtain the next page
 // in a set of list results.
 // The given response holds the response received from the previous
 // list request; initialReq holds the request that initiated the listing,
 // and last holds the final item returned in the previous response.
-func nextLink(ctx context.Context, resp *http.Response, initialReq *ocirequest.Request, last string) (*http.Request, error) {
+func nextLink[T any](ctx context.Context, resp *http.Response, initialReq *ocirequest.Request, canUseLast bool, last T) (*http.Request, error) {
 	link0 := resp.Header.Get("Link")
 	if link0 == "" {
+		if !canUseLast {
+			return nil, nil
+		}
 		// This is beyond the first page and there was no Link
 		// in the previous response (the standard doesn't mandate
 		// one), so add a "last" parameter to the initial request.
 		rreq := *initialReq
-		rreq.ListLast = last
+		rreq.ListLast = fmt.Sprint(last)
 		req, err := newRequest(ctx, &rreq, nil)
 		if err != nil {
 			// Given that we could form the initial request, this should
@@ -178,3 +213,38 @@ func nextLink(ctx context.Context, resp *http.Response, initialReq *ocirequest.R
 	}
 	return http.NewRequestWithContext(ctx, "GET", linkURL.String(), nil)
 }
+
+// referrersTag returns the referrers tag for the given digest, as described
+// in https://github.com/opencontainers/distribution-spec/blob/main/spec.md#referrers-tag-schema
+func referrersTag(digest ociregistry.Digest) string {
+	// It's hard to know what the spec means by "with any characters not allowed by <reference> tags replaced with -",
+	// because different characters are allowed in different contexts (for example, a dot character
+	// is allowed except when it's at the start.
+	// In practice, however, the set of characters is very limited, and the only
+	// disallowed character in common use is :, so just use a naive algorithm.
+	return truncateAndMap(digest.Algorithm().String(), 32) + "-" + truncateAndMap(digest.Encoded(), 64)
+}
+
+func truncateAndMap(s string, n int) string {
+	// regexp: [a-zA-Z0-9_][a-zA-Z0-9._-]{0,127}
+
+	s = strings.Map(func(r rune) rune {
+		switch {
+		case 'a' <= r && r <= 'z':
+			return r
+		case 'A' <= r && r <= 'Z':
+			return r
+		case '0' <= r && r <= '9':
+			return r
+		case r == '.' || r == '_' || r == '-':
+			return r
+		}
+		return '-'
+	}, s)
+	// Note: it's OK to use n as a byte index because the
+	// above Map has eliminated all non-ASCII characters.
+	if len(s) <= n {
+		return s
+	}
+	return s[:n]
+}