From 1bb3914de48f208ee67555ee726a9760a87b17a5 Mon Sep 17 00:00:00 2001 From: Wang Yan Date: Tue, 28 Mar 2023 13:51:37 +0800 Subject: [PATCH] fix referrers api response issue (#18430) 1, add fitler artifactType to header when the api is called with filter 2, give an empty json body on non aritfact scenario 3, give an empty array on non accessory scenario 4, fix the artifact type filter issue Signed-off-by: Wang Yan --- src/server/registry/referrers.go | 48 +++++++++++++++++++-------- src/server/registry/referrers_test.go | 48 ++++++++++++++++++++++++--- 2 files changed, 79 insertions(+), 17 deletions(-) diff --git a/src/server/registry/referrers.go b/src/server/registry/referrers.go index ac1762a4c..0fc8ed6be 100644 --- a/src/server/registry/referrers.go +++ b/src/server/registry/referrers.go @@ -38,6 +38,10 @@ func (r *referrersHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { repository := router.Param(ctx, ":splat") reference := router.Param(ctx, ":reference") at := req.URL.Query().Get("artifactType") + var filter string + if at != "" { + filter = "artifactType" + } // Check if the reference is a valid digest if _, err := digest.Parse(reference); err != nil { @@ -45,25 +49,20 @@ func (r *referrersHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { return } - result := &ocispec.Index{} - // Get the artifact by reference art, err := r.artifactManager.GetByDigest(ctx, repository, reference) if err != nil { if errors.IsNotFoundErr(err) { - // If artifact not found, return empty index - newListReferrersOK().WithPayload(result).WriteResponse(w) + // If artifact not found, return empty json + newListReferrersOK().WithPayload(nil).WriteResponse(w) return } lib_http.SendError(w, err) return } - // Query accessories with matching subject artifact digest and artifactType + // Query accessories with matching subject artifact digest query := q.New(q.KeyWords{"SubjectArtifactDigest": art.Digest, "SubjectArtifactRepo": art.RepositoryName}) - if at != "" { - query = q.New(q.KeyWords{"SubjectArtifactDigest": art.Digest, "SubjectArtifactRepo": art.RepositoryName, "Type": at}) - } total, err := r.accessoryManager.Count(ctx, query) if err != nil { lib_http.SendError(w, err) @@ -76,7 +75,7 @@ func (r *referrersHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { } // Build index manifest from accessories - var mfs []ocispec.Descriptor + mfs := make([]ocispec.Descriptor, 0) for _, acc := range accs { accArt, err := r.artifactManager.GetByDigest(ctx, repository, acc.GetData().Digest) if err != nil { @@ -90,10 +89,18 @@ func (r *referrersHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { Annotations: accArt.Annotations, ArtifactType: accArt.MediaType, } - mfs = append(mfs, mf) + // filter by the artifactType since the artifactType is actually the config media type of the artifact. + if at != "" { + if accArt.MediaType == at { + mfs = append(mfs, mf) + } + } else { + mfs = append(mfs, mf) + } } // Populate index manifest + result := &ocispec.Index{} result.SchemaVersion = ReferrersSchemaVersion result.MediaType = ReferrersMediaType result.Manifests = mfs @@ -102,6 +109,7 @@ func (r *referrersHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { baseAPI := &handler.BaseAPI{} newListReferrersOK(). WithXTotalCount(total). + WithFilter(filter). WithLink(baseAPI.Links(ctx, req.URL, total, query.PageNumber, query.PageSize).String()). WithPayload(result).WriteResponse(w) } @@ -111,6 +119,10 @@ type listReferrersOK struct { */ Link string `json:"Link"` + /*Filter refers to the filter used to fetch the referrers + + */ + Filter string `json:"Filter"` /*The total count of accessories */ @@ -119,7 +131,7 @@ type listReferrersOK struct { /* In: Body */ - Payload *ocispec.Index `json:"body,omitempty"` + Payload interface{} `json:"body,omitempty"` } // newListReferrersOK creates newlistReferrersOK with default headers values @@ -133,6 +145,12 @@ func (o *listReferrersOK) WithLink(link string) *listReferrersOK { return o } +// WithFilter adds the filter to the get referrers +func (o *listReferrersOK) WithFilter(filter string) *listReferrersOK { + o.Filter = filter + return o +} + // WithXTotalCount adds the xTotalCount to the list accessories o k response func (o *listReferrersOK) WithXTotalCount(xTotalCount int64) *listReferrersOK { o.XTotalCount = xTotalCount @@ -140,7 +158,7 @@ func (o *listReferrersOK) WithXTotalCount(xTotalCount int64) *listReferrersOK { } // WithPayload adds the payload to the list accessories o k response -func (o *listReferrersOK) WithPayload(payload *ocispec.Index) *listReferrersOK { +func (o *listReferrersOK) WithPayload(payload interface{}) *listReferrersOK { o.Payload = payload return o } @@ -153,6 +171,10 @@ func (o *listReferrersOK) WriteResponse(rw http.ResponseWriter) { if link != "" { rw.Header().Set("Link", link) } + filter := o.Filter + if filter != "" { + rw.Header().Set("OCI-Filters-Applied", filter) + } xTotalCount := swag.FormatInt64(o.XTotalCount) if xTotalCount != "" { rw.Header().Set("X-Total-Count", xTotalCount) @@ -162,7 +184,7 @@ func (o *listReferrersOK) WriteResponse(rw http.ResponseWriter) { payload := o.Payload if payload == nil { // return empty index - payload = &ocispec.Index{} + payload = struct{}{} } enc := json.NewEncoder(rw) diff --git a/src/server/registry/referrers_test.go b/src/server/registry/referrers_test.go index db574dea2..d9c960a01 100644 --- a/src/server/registry/referrers_test.go +++ b/src/server/registry/referrers_test.go @@ -3,7 +3,15 @@ package registry import ( "context" "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "testing" + beegocontext "github.com/beego/beego/v2/server/web/context" + ocispec "github.com/opencontainers/image-spec/specs-go/v1" + + "github.com/goharbor/harbor/src/lib/errors" accessorymodel "github.com/goharbor/harbor/src/pkg/accessory/model" basemodel "github.com/goharbor/harbor/src/pkg/accessory/model/base" "github.com/goharbor/harbor/src/pkg/artifact" @@ -11,10 +19,6 @@ import ( "github.com/goharbor/harbor/src/testing/mock" accessorytesting "github.com/goharbor/harbor/src/testing/pkg/accessory" arttesting "github.com/goharbor/harbor/src/testing/pkg/artifact" - ocispec "github.com/opencontainers/image-spec/specs-go/v1" - "net/http" - "net/http/httptest" - "testing" ) func TestReferrersHandlerOK(t *testing.T) { @@ -75,6 +79,42 @@ func TestReferrersHandlerOK(t *testing.T) { } } +func TestReferrersHandlerEmpty(t *testing.T) { + rec := httptest.NewRecorder() + digestVal := "sha256:6c3c624b58dbbcd3c0dd82b4c53f04194d1247c6eebdaab7c610cf7d66709b3b" + req, err := http.NewRequest("GET", "/v2/test/repository/referrers/sha256:6c3c624b58dbbcd3c0dd82b4c53f04194d1247c6eebdaab7c610cf7d66709b3b", nil) + if err != nil { + t.Fatal(err) + } + input := &beegocontext.BeegoInput{} + input.SetParam(":reference", digestVal) + *req = *(req.WithContext(context.WithValue(req.Context(), router.ContextKeyInput{}, input))) + + artifactMock := &arttesting.Manager{} + accessoryMock := &accessorytesting.Manager{} + + artifactMock.On("GetByDigest", mock.Anything, mock.Anything, mock.Anything). + Return(nil, errors.NotFoundError(nil)) + + handler := &referrersHandler{ + artifactManager: artifactMock, + accessoryManager: accessoryMock, + } + + handler.ServeHTTP(rec, req) + + // check that the response has the expected status code (200 OK) + if rec.Code != http.StatusOK { + t.Errorf("Expected status code %d, but got %d", http.StatusOK, rec.Code) + } + index := &ocispec.Index{} + json.Unmarshal([]byte(rec.Body.String()), index) + fmt.Println(index) + if index.SchemaVersion != 0 && len(index.Manifests) != -0 { + t.Errorf("Expected empty response body, but got %s", rec.Body.String()) + } +} + func TestReferrersHandler400(t *testing.T) { rec := httptest.NewRecorder() digestVal := "invalid"