feat: merge local and remote tags for repo of proxy cache project in list API (#16394)

Signed-off-by: He Weiwei <hweiwei@vmware.com>

src/controller/proxy/remote.go

@@ -33,6 +33,8 @@ type RemoteInterface interface {
 	Manifest(repo string, ref string) (distribution.Manifest, string, error)
 	// ManifestExist checks manifest exist, if exist, return digest
 	ManifestExist(repo string, ref string) (bool, *distribution.Descriptor, error)
+	// ListTags returns all tags of the repo
+	ListTags(repo string) ([]string, error)
 }
 
 // remoteHelper defines operations related to remote repository under proxy
@@ -91,3 +93,7 @@ func (r *remoteHelper) Manifest(repo string, ref string) (distribution.Manifest,
 func (r *remoteHelper) ManifestExist(repo string, ref string) (bool, *distribution.Descriptor, error) {
 	return r.registry.ManifestExist(repo, ref)
 }
+
+func (r *remoteHelper) ListTags(repo string) ([]string, error) {
+	return r.registry.ListTags(repo)
+}

src/controller/replication/flow/mock_adapter_test.go

@@ -161,6 +161,29 @@ func (_m *mockAdapter) Info() (*model.RegistryInfo, error) {
 	return r0, r1
 }
 
+// ListTags provides a mock function with given fields: repository
+func (_m *mockAdapter) ListTags(repository string) ([]string, error) {
+	ret := _m.Called(repository)
+
+	var r0 []string
+	if rf, ok := ret.Get(0).(func(string) []string); ok {
+		r0 = rf(repository)
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).([]string)
+		}
+	}
+
+	var r1 error
+	if rf, ok := ret.Get(1).(func(string) error); ok {
+		r1 = rf(repository)
+	} else {
+		r1 = ret.Error(1)
+	}
+
+	return r0, r1
+}
+
 // ManifestExist provides a mock function with given fields: repository, reference
 func (_m *mockAdapter) ManifestExist(repository string, reference string) (bool, *distribution.Descriptor, error) {
 	ret := _m.Called(repository, reference)

src/controller/replication/transfer/image/transfer_test.go

@@ -103,6 +103,10 @@ func (f *fakeRegistry) MountBlob(srcRepository, digest, dstRepository string) er
 	return nil
 }
 
+func (f *fakeRegistry) ListTags(repository string) (tags []string, err error) {
+	return nil, nil
+}
+
 func TestFactory(t *testing.T) {
 	tr, err := factory(nil, nil)
 	require.Nil(t, err)

src/pkg/reg/adapter/adapter.go

@@ -63,6 +63,7 @@ type ArtifactRegistry interface {
 	MountBlob(srcRepository, digest, dstRepository string) (err error)
 	CanBeMount(digest string) (mount bool, repository string, err error) // check whether the blob can be mounted from the remote registry
 	DeleteTag(repository, tag string) error
+	ListTags(repository string) (tags []string, err error)
 }
 
 // ChartRegistry defines the capabilities that a chart registry should have

src/server/middleware/repoproxy/tag.go

@@ -0,0 +1,110 @@
+//  Copyright Project Harbor Authors
+//
+//  Licensed under the Apache License, Version 2.0 (the "License");
+//  you may not use this file except in compliance with the License.
+//  You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+//  Unless required by applicable law or agreed to in writing, software
+//  distributed under the License is distributed on an "AS IS" BASIS,
+//  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+//  See the License for the specific language governing permissions and
+//  limitations under the License.
+
+package repoproxy
+
+import (
+	"context"
+	"net/http"
+	"strings"
+
+	"github.com/goharbor/harbor/src/controller/proxy"
+	"github.com/goharbor/harbor/src/controller/repository"
+	"github.com/goharbor/harbor/src/controller/tag"
+	"github.com/goharbor/harbor/src/lib/errors"
+	libhttp "github.com/goharbor/harbor/src/lib/http"
+	"github.com/goharbor/harbor/src/lib/log"
+	"github.com/goharbor/harbor/src/lib/q"
+	"github.com/goharbor/harbor/src/server/middleware"
+	"github.com/goharbor/harbor/src/server/registry/util"
+)
+
+// TagsListMiddleware handle list tags request
+func TagsListMiddleware() func(http.Handler) http.Handler {
+	return middleware.New(func(w http.ResponseWriter, r *http.Request, next http.Handler) {
+		ctx := r.Context()
+
+		art, p, _, err := preCheck(ctx)
+		if err != nil {
+			libhttp.SendError(w, err)
+			return
+		}
+
+		if !canProxy(ctx, p) {
+			next.ServeHTTP(w, r)
+			return
+		}
+
+		n, _, err := util.ParseNAndLastParameters(r)
+		if err != nil {
+			libhttp.SendError(w, err)
+			return
+		}
+
+		if n != nil && *n == 0 {
+			util.SendListTagsResponse(w, r, nil)
+			return
+		}
+
+		logger := log.G(ctx).WithField("project", art.ProjectName).WithField("repository", art.Repository)
+
+		tags, err := getLocalTags(ctx, art.Repository)
+		if err != nil {
+			libhttp.SendError(w, err)
+			return
+		}
+
+		defer func() {
+			util.SendListTagsResponse(w, r, tags)
+		}()
+
+		remote, err := proxy.NewRemoteHelper(ctx, p.RegistryID)
+		if err != nil {
+			logger.Warningf("failed to get remote interface, error: %v, fallback to local tags", err)
+			return
+		}
+
+		remoteRepository := strings.TrimPrefix(art.Repository, art.ProjectName+"/")
+		remoteTags, err := remote.ListTags(remoteRepository)
+		if err != nil {
+			logger.Warningf("failed to get remote tags, error: %v, fallback to local tags", err)
+			return
+		}
+
+		tags = append(tags, remoteTags...)
+	})
+}
+
+var getLocalTags = func(ctx context.Context, repo string) ([]string, error) {
+	r, err := repository.Ctl.GetByName(ctx, repo)
+	if err != nil {
+		if errors.IsNotFoundErr(err) {
+			return nil, nil
+		}
+
+		return nil, err
+	}
+
+	items, err := tag.Ctl.List(ctx, q.New(q.KeyWords{"RepositoryID": r.RepositoryID}), nil)
+	if err != nil {
+		return nil, err
+	}
+
+	tags := make([]string, len(items))
+	for i := range items {
+		tags[i] = items[i].Name
+	}
+
+	return tags, nil
+}

src/server/registry/route.go

@@ -15,11 +15,11 @@
 package registry
 
 import (
-	"github.com/goharbor/harbor/src/server/middleware/cosign"
 	"net/http"
 
 	"github.com/goharbor/harbor/src/server/middleware/blob"
 	"github.com/goharbor/harbor/src/server/middleware/contenttrust"
+	"github.com/goharbor/harbor/src/server/middleware/cosign"
 	"github.com/goharbor/harbor/src/server/middleware/immutable"
 	"github.com/goharbor/harbor/src/server/middleware/metric"
 	"github.com/goharbor/harbor/src/server/middleware/quota"
@@ -45,6 +45,7 @@ func RegisterRoutes() {
 		Method(http.MethodGet).
 		Path("/*/tags/list").
 		Middleware(metric.InjectOpIDMiddleware(metric.ListTagOperationID)).
+		Middleware(repoproxy.TagsListMiddleware()).
 		Handler(newTagHandler())
 	// manifest
 	root.NewRoute().

src/server/registry/tag.go

@@ -15,18 +15,14 @@
 package registry
 
 import (
-	"encoding/json"
-	"fmt"
+	"net/http"
+
 	"github.com/goharbor/harbor/src/controller/repository"
 	"github.com/goharbor/harbor/src/controller/tag"
-	"github.com/goharbor/harbor/src/lib/errors"
 	lib_http "github.com/goharbor/harbor/src/lib/http"
 	"github.com/goharbor/harbor/src/lib/q"
 	"github.com/goharbor/harbor/src/server/registry/util"
 	"github.com/goharbor/harbor/src/server/router"
-	"net/http"
-	"sort"
-	"strconv"
 )
 
 func newTagHandler() http.Handler {
@@ -37,9 +33,8 @@ func newTagHandler() http.Handler {
 }
 
 type tagHandler struct {
-	repoCtl        repository.Controller
-	tagCtl         tag.Controller
-	repositoryName string
+	repoCtl repository.Controller
+	tagCtl  tag.Controller
 }
 
 // get return the list of tags
@@ -55,31 +50,24 @@ type tagHandler struct {
 //    ]
 // }
 func (t *tagHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
-	var maxEntries int
-	var err error
-
-	reqQ := req.URL.Query()
-	lastEntry := reqQ.Get("last")
-	withN := reqQ.Get("n") != ""
-	if withN {
-		maxEntries, err = strconv.Atoi(reqQ.Get("n"))
-		if err != nil || maxEntries < 0 {
-			err := errors.New(err).WithCode(errors.BadRequestCode).WithMessage("the N must be a positive int type")
-			lib_http.SendError(w, err)
-			return
-		}
-	}
-
-	tagNames := make([]string, 0)
-
-	t.repositoryName = router.Param(req.Context(), ":splat")
-	repository, err := t.repoCtl.GetByName(req.Context(), t.repositoryName)
+	n, _, err := util.ParseNAndLastParameters(req)
+	if err != nil {
+		lib_http.SendError(w, err)
+		return
+	}
+
+	if n != nil && *n == 0 {
+		util.SendListTagsResponse(w, req, nil)
+		return
+	}
+
+	repositoryName := router.Param(req.Context(), ":splat")
+	repository, err := t.repoCtl.GetByName(req.Context(), repositoryName)
 	if err != nil {
 		lib_http.SendError(w, err)
 		return
 	}
 
-	// get tags ...
 	tags, err := t.tagCtl.List(req.Context(), &q.Query{
 		Keywords: map[string]interface{}{
 			"RepositoryID": repository.RepositoryID,
@@ -88,77 +76,11 @@ func (t *tagHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 		lib_http.SendError(w, err)
 		return
 	}
-	if len(tags) == 0 {
-		t.sendResponse(w, req, tagNames)
-		return
+
+	tagNames := make([]string, len(tags))
+	for i := range tags {
+		tagNames[i] = tags[i].Name
 	}
 
-	for _, tag := range tags {
-		tagNames = append(tagNames, tag.Name)
-	}
-	sort.Strings(tagNames)
-	if !withN {
-		t.sendResponse(w, req, tagNames)
-		return
-	}
-
-	// handle the pagination
-	resTags := tagNames
-	tagNamesLen := len(tagNames)
-	// with "last", get items form lastEntryIndex+1 to lastEntryIndex+maxEntries
-	// without "last", get items from 0 to maxEntries'
-	if lastEntry != "" {
-		lastEntryIndex := util.IndexString(tagNames, lastEntry)
-		if lastEntryIndex == -1 {
-			err := errors.New(nil).WithCode(errors.BadRequestCode).WithMessage(fmt.Sprintf("the last: %s should be a valid tag name.", lastEntry))
-			lib_http.SendError(w, err)
-			return
-		}
-		if lastEntryIndex+1+maxEntries > tagNamesLen {
-			resTags = tagNames[lastEntryIndex+1 : tagNamesLen]
-		} else {
-			resTags = tagNames[lastEntryIndex+1 : lastEntryIndex+1+maxEntries]
-		}
-	} else {
-		if maxEntries > tagNamesLen {
-			maxEntries = tagNamesLen
-		}
-		resTags = tagNames[0:maxEntries]
-	}
-
-	if len(resTags) == 0 {
-		t.sendResponse(w, req, resTags)
-		return
-	}
-
-	// compare the last item to define whether return the link header.
-	// if equals, means that there is no more items in DB. Do not need to give the link header.
-	if tagNames[len(tagNames)-1] != resTags[len(resTags)-1] {
-		urlStr, err := util.SetLinkHeader(req.URL.String(), maxEntries, resTags[len(resTags)-1])
-		if err != nil {
-			lib_http.SendError(w, err)
-			return
-		}
-		w.Header().Set("Link", urlStr)
-	}
-	t.sendResponse(w, req, resTags)
-	return
-}
-
-// sendResponse ...
-func (t *tagHandler) sendResponse(w http.ResponseWriter, req *http.Request, tagNames []string) {
-	w.Header().Set("Content-Type", "application/json; charset=utf-8")
-	enc := json.NewEncoder(w)
-	if err := enc.Encode(tagsAPIResponse{
-		Name: t.repositoryName,
-		Tags: tagNames,
-	}); err != nil {
-		lib_http.SendError(w, err)
-		return
-	}
-}
-
-type tagsAPIResponse struct {
-	Name string   `json:"name"`
-	Tags []string `json:"tags"`
+	util.SendListTagsResponse(w, req, tagNames)
 }

src/server/registry/util/util.go

@@ -1,10 +1,31 @@
+// Copyright 2018 Project Harbor Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
 package util
 
 import (
+	"encoding/json"
 	"fmt"
+	"net/http"
 	"net/url"
 	"sort"
 	"strconv"
+	"strings"
+
+	"github.com/goharbor/harbor/src/lib/errors"
+	libhttp "github.com/goharbor/harbor/src/lib/http"
+	"github.com/goharbor/harbor/src/server/router"
 )
 
 // SetLinkHeader ...
@@ -36,3 +57,108 @@ func IndexString(strs []string, x string) int {
 	}
 	return -1
 }
+
+// ParseNAndLastParameters parse the n and last parameters from the query of the http request
+func ParseNAndLastParameters(r *http.Request) (*int, string, error) {
+	q := r.URL.Query()
+
+	var n *int
+
+	if q.Get("n") != "" {
+		value, err := strconv.Atoi(q.Get("n"))
+		if err != nil || value < 0 {
+			return nil, "", errors.New(err).WithCode(errors.BadRequestCode).WithMessage("the N must be a positive int type")
+		}
+
+		n = &value
+	}
+
+	return n, q.Get("last"), nil
+}
+
+// SendListTagsResponse sends the response for list tags API
+func SendListTagsResponse(w http.ResponseWriter, r *http.Request, tags []string) {
+	n, last, err := ParseNAndLastParameters(r)
+	if err != nil {
+		libhttp.SendError(w, err)
+		return
+	}
+
+	items, nextLast := pickItems(sortedAndUniqueItems(tags), n, last)
+
+	if nextLast != "" && n != nil { // NOTE: when the nextLast is not empty the n must not be nil
+		link, err := SetLinkHeader(r.URL.String(), *n, nextLast)
+		if err != nil {
+			libhttp.SendError(w, err)
+			return
+		}
+
+		w.Header().Set("Link", link)
+	}
+
+	body := struct {
+		Name string   `json:"name"`
+		Tags []string `json:"tags"`
+	}{
+		Name: router.Param(r.Context(), ":splat"),
+		Tags: items,
+	}
+
+	w.Header().Set("Content-Type", "application/json; charset=utf-8")
+	if err := json.NewEncoder(w).Encode(body); err != nil {
+		libhttp.SendError(w, err)
+	}
+}
+
+func sortedAndUniqueItems(items []string) []string {
+	n := len(items)
+	if n <= 1 {
+		return items[0:n]
+	}
+
+	sort.Strings(items)
+
+	j := 1
+	for i := 1; i < n; i++ {
+		if items[i] != items[i-1] {
+			items[j] = items[i]
+			j++
+		}
+	}
+
+	return items[0:j]
+}
+
+// pickItems returns the first n elements which is bigger than the last from items, if the n is 0, return the empty slice
+// NOTE: the items must be ordered and value of n is equal or great than 0 when n isn't nil
+func pickItems(items []string, n *int, last string) ([]string, string) {
+	if len(items) == 0 || (n != nil && *n == 0) {
+		// no items found or request n is 0
+		return []string{}, ""
+	}
+
+	if n == nil {
+		l := len(items)
+		n = &l
+	}
+
+	i := 0
+	if last != "" {
+		i = sort.Search(len(items), func(ix int) bool { return strings.Compare(items[ix], last) > 0 })
+	}
+
+	j := i + *n
+
+	if j >= len(items) {
+		j = len(items)
+	}
+
+	result := items[i:j]
+
+	nextLast := ""
+	if len(result) > 0 && items[len(items)-1] != result[len(result)-1] {
+		nextLast = result[len(result)-1]
+	}
+
+	return result, nextLast
+}

src/server/registry/util/util_test.go

@@ -1,8 +1,24 @@
+// Copyright 2018 Project Harbor Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
 package util
 
 import (
-	"github.com/stretchr/testify/assert"
+	"reflect"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
 )
 
 func TestCreateLinkEntry(t *testing.T) {
@@ -27,3 +43,197 @@ func TestIndexString(t *testing.T) {
 	assert.True(t, IndexString(a, "Z") == -1)
 	assert.True(t, IndexString(a, "") == -1)
 }
+
+func Test_pickItems(t *testing.T) {
+	p := func(n int) *int {
+		return &n
+	}
+
+	type args struct {
+		items []string
+		n     *int
+		last  string
+	}
+	tests := []struct {
+		name  string
+		args  args
+		want  []string
+		want1 string
+	}{
+		{
+			"no parameters",
+			args{[]string{"a", "b", "c", "d"}, nil, ""},
+			[]string{"a", "b", "c", "d"},
+			"",
+		},
+		{
+			"n=0",
+			args{[]string{"a", "b", "c", "d"}, p(0), ""},
+			[]string{},
+			"",
+		},
+		{
+			"n=1",
+			args{[]string{"a", "b", "c", "d"}, p(1), ""},
+			[]string{"a"},
+			"a",
+		},
+		{
+			"n=2",
+			args{[]string{"a", "b", "c", "d"}, p(2), ""},
+			[]string{"a", "b"},
+			"b",
+		},
+		{
+			"n=4", // n is the count of items
+			args{[]string{"a", "b", "c", "d"}, p(4), ""},
+			[]string{"a", "b", "c", "d"},
+			"",
+		},
+		{
+			"n=5", // n is bigger than the count of items
+			args{[]string{"a", "b", "c", "d"}, p(5), ""},
+			[]string{"a", "b", "c", "d"},
+			"",
+		},
+		{
+			"last=a",
+			args{[]string{"a", "b", "c", "d"}, nil, "a"},
+			[]string{"b", "c", "d"},
+			"",
+		},
+		{
+			"last=d",
+			args{[]string{"a", "b", "c", "d"}, nil, "d"},
+			[]string{},
+			"",
+		},
+		{
+			"n=1 last=a",
+			args{[]string{"a", "b", "c", "d"}, p(1), "a"},
+			[]string{"b"},
+			"b",
+		},
+		{
+			"n=2 last=a",
+			args{[]string{"a", "b", "c", "d"}, p(2), "a"},
+			[]string{"b", "c"},
+			"c",
+		},
+		{
+			"n=3 last=a", // just the left n items
+			args{[]string{"a", "b", "c", "d"}, p(3), "a"},
+			[]string{"b", "c", "d"},
+			"",
+		},
+		{
+			"n=4 last=a", // left items is less than n
+			args{[]string{"a", "b", "c", "d"}, p(4), "a"},
+			[]string{"b", "c", "d"},
+			"",
+		},
+		{
+			"n=1 last=d", // last is the last element of the items
+			args{[]string{"a", "b", "c", "d"}, p(1), "d"},
+			[]string{},
+			"",
+		},
+		{
+			"last=c", // last not found and there is an elem bigger than it
+			args{[]string{"a", "b", "d", "e"}, nil, "c"},
+			[]string{"d", "e"},
+			"",
+		},
+		{
+			"last=e", // last not found and there isn't an elem bigger than it
+			args{[]string{"a", "b", "c", "d"}, nil, "e"},
+			[]string{},
+			"",
+		},
+		{
+			"last=e n=10",
+			args{[]string{"a", "b", "c", "d"}, p(10), "e"},
+			[]string{},
+			"",
+		},
+		{
+			"one item",
+			args{[]string{"a"}, nil, ""},
+			[]string{"a"},
+			"",
+		},
+		{
+			"one item n=2",
+			args{[]string{"a"}, p(2), ""},
+			[]string{"a"},
+			"",
+		},
+		{
+			"two items",
+			args{[]string{"a", "b"}, nil, ""},
+			[]string{"a", "b"},
+			"",
+		},
+		{
+			"three items",
+			args{[]string{"a", "b", "c"}, nil, ""},
+			[]string{"a", "b", "c"},
+			"",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, got1 := pickItems(tt.args.items, tt.args.n, tt.args.last)
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("pickItems() got = %v, want %v", got, tt.want)
+			}
+			if got1 != tt.want1 {
+				t.Errorf("pickItems() got1 = %v, want %v", got1, tt.want1)
+			}
+		})
+	}
+}
+
+func Test_sortedAndUniqueItems(t *testing.T) {
+	type args struct {
+		items []string
+	}
+	tests := []struct {
+		name string
+		args args
+		want []string
+	}{
+		{
+			"nil",
+			args{nil},
+			nil,
+		},
+		{
+			"no item",
+			args{[]string{}},
+			[]string{},
+		},
+		{
+			"one item",
+			args{[]string{"a"}},
+			[]string{"a"},
+		},
+		{
+			"duplicate items",
+			args{[]string{"a", "a", "a"}},
+			[]string{"a"},
+		},
+		{
+			"unordered and duplicate items",
+			args{[]string{"a", "c", "a", "b", "a"}},
+			[]string{"a", "b", "c"},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := sortedAndUniqueItems(tt.args.items); !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("sortedAndUniqueItems() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

src/testing/controller/proxy/remote_interface.go

@@ -45,6 +45,29 @@ func (_m *RemoteInterface) BlobReader(repo string, dig string) (int64, io.ReadCl
 	return r0, r1, r2
 }
 
+// ListTags provides a mock function with given fields: repo
+func (_m *RemoteInterface) ListTags(repo string) ([]string, error) {
+	ret := _m.Called(repo)
+
+	var r0 []string
+	if rf, ok := ret.Get(0).(func(string) []string); ok {
+		r0 = rf(repo)
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).([]string)
+		}
+	}
+
+	var r1 error
+	if rf, ok := ret.Get(1).(func(string) error); ok {
+		r1 = rf(repo)
+	} else {
+		r1 = ret.Error(1)
+	}
+
+	return r0, r1
+}
+
 // Manifest provides a mock function with given fields: repo, ref
 func (_m *RemoteInterface) Manifest(repo string, ref string) (distribution.Manifest, string, error) {
 	ret := _m.Called(repo, ref)