From abd85284d2c45a1583b617065938a652dea7e9cf Mon Sep 17 00:00:00 2001 From: He Weiwei Date: Thu, 24 Feb 2022 10:27:45 +0800 Subject: [PATCH] feat: merge local and remote tags for repo of proxy cache project in list API (#16394) Signed-off-by: He Weiwei --- src/controller/proxy/remote.go | 6 + .../replication/flow/mock_adapter_test.go | 23 ++ .../transfer/image/transfer_test.go | 4 + src/pkg/reg/adapter/adapter.go | 1 + src/server/middleware/repoproxy/tag.go | 110 +++++++++ src/server/registry/route.go | 3 +- src/server/registry/tag.go | 122 ++-------- src/server/registry/util/util.go | 126 +++++++++++ src/server/registry/util/util_test.go | 212 +++++++++++++++++- .../controller/proxy/remote_interface.go | 23 ++ 10 files changed, 528 insertions(+), 102 deletions(-) create mode 100644 src/server/middleware/repoproxy/tag.go diff --git a/src/controller/proxy/remote.go b/src/controller/proxy/remote.go index 31b856935..dca749d7a 100644 --- a/src/controller/proxy/remote.go +++ b/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) +} diff --git a/src/controller/replication/flow/mock_adapter_test.go b/src/controller/replication/flow/mock_adapter_test.go index 939f191a8..810e4c8c6 100644 --- a/src/controller/replication/flow/mock_adapter_test.go +++ b/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) diff --git a/src/controller/replication/transfer/image/transfer_test.go b/src/controller/replication/transfer/image/transfer_test.go index 0859b83ef..527979785 100644 --- a/src/controller/replication/transfer/image/transfer_test.go +++ b/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) diff --git a/src/pkg/reg/adapter/adapter.go b/src/pkg/reg/adapter/adapter.go index dbac8bc6e..81d6cdfbf 100644 --- a/src/pkg/reg/adapter/adapter.go +++ b/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 diff --git a/src/server/middleware/repoproxy/tag.go b/src/server/middleware/repoproxy/tag.go new file mode 100644 index 000000000..071635e33 --- /dev/null +++ b/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 +} diff --git a/src/server/registry/route.go b/src/server/registry/route.go index 7feee5597..892b05327 100644 --- a/src/server/registry/route.go +++ b/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(). diff --git a/src/server/registry/tag.go b/src/server/registry/tag.go index e6168b6c0..c243f45b9 100644 --- a/src/server/registry/tag.go +++ b/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) } diff --git a/src/server/registry/util/util.go b/src/server/registry/util/util.go index 9972149b1..8a7b19497 100644 --- a/src/server/registry/util/util.go +++ b/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 +} diff --git a/src/server/registry/util/util_test.go b/src/server/registry/util/util_test.go index df0e73ed5..5975c363d 100644 --- a/src/server/registry/util/util_test.go +++ b/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) + } + }) + } +} diff --git a/src/testing/controller/proxy/remote_interface.go b/src/testing/controller/proxy/remote_interface.go index 7f62d6318..ba231b946 100644 --- a/src/testing/controller/proxy/remote_interface.go +++ b/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)