From ff2a7e61c9c1f11e119fc87be9bb42ef814c3050 Mon Sep 17 00:00:00 2001 From: Wang Yan Date: Mon, 20 Apr 2020 23:37:16 +0800 Subject: [PATCH] fix catalog api issue (#11666) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The v2 catalog API needs to filter out the empty repository and the repository which artifacts are all with no tags. 1，In v2.0.0, Harbor does not delete repository even there is no artifact, it's different with v1.10.0 2, Compares with docker distribution, it doesn't return the respository with untagged images. Signed-off-by: wang yan --- src/server/registry/catalog.go | 45 ++++++++- src/server/registry/catalog_test.go | 141 ++++++++++++++++++++++++++++ 2 files changed, 183 insertions(+), 3 deletions(-) diff --git a/src/server/registry/catalog.go b/src/server/registry/catalog.go index 82acee98d..299dfde29 100644 --- a/src/server/registry/catalog.go +++ b/src/server/registry/catalog.go @@ -15,10 +15,13 @@ package registry import ( + "context" "encoding/json" "fmt" + "github.com/goharbor/harbor/src/controller/artifact" "github.com/goharbor/harbor/src/controller/repository" "github.com/goharbor/harbor/src/lib/errors" + "github.com/goharbor/harbor/src/lib/q" serror "github.com/goharbor/harbor/src/server/error" "github.com/goharbor/harbor/src/server/registry/util" "net/http" @@ -29,11 +32,13 @@ import ( func newRepositoryHandler() http.Handler { return &repositoryHandler{ repoCtl: repository.Ctl, + artCtl: artifact.Ctl, } } type repositoryHandler struct { repoCtl repository.Controller + artCtl artifact.Controller } func (r *repositoryHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { @@ -54,7 +59,6 @@ func (r *repositoryHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) repoNames := make([]string, 0) // get all repositories - // ToDo filter out the untagged repos repoRecords, err := r.repoCtl.List(req.Context(), nil) if err != nil { serror.SendError(w, err) @@ -64,8 +68,15 @@ func (r *repositoryHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) r.sendResponse(w, req, repoNames) return } - for _, r := range repoRecords { - repoNames = append(repoNames, r.Name) + for _, repo := range repoRecords { + valid, err := r.validateRepo(req.Context(), repo.RepositoryID) + if err != nil { + serror.SendError(w, err) + return + } + if valid { + repoNames = append(repoNames, repo.Name) + } } sort.Strings(repoNames) if !withN { @@ -129,6 +140,34 @@ func (r *repositoryHandler) sendResponse(w http.ResponseWriter, req *http.Reques } } +// empty repo and all of artifacts are untagged should be filtered out. +func (r *repositoryHandler) validateRepo(ctx context.Context, repositoryID int64) (bool, error) { + arts, err := r.artCtl.List(ctx, &q.Query{ + Keywords: map[string]interface{}{ + "RepositoryID": repositoryID, + }, + }, &artifact.Option{ + WithTag: true, + }) + if err != nil { + return false, err + } + + // empty repo + if len(arts) == 0 { + return false, nil + } + + for _, art := range arts { + if len(art.Tags) != 0 { + return true, nil + } + } + + // if all of artifact are untagged, filter out + return false, nil +} + type catalogAPIResponse struct { Repositories []string `json:"repositories"` } diff --git a/src/server/registry/catalog_test.go b/src/server/registry/catalog_test.go index 26d3637f2..ff34c9280 100644 --- a/src/server/registry/catalog_test.go +++ b/src/server/registry/catalog_test.go @@ -17,8 +17,14 @@ package registry import ( "encoding/json" "github.com/goharbor/harbor/src/common/models" + "github.com/goharbor/harbor/src/controller/artifact" "github.com/goharbor/harbor/src/controller/repository" + "github.com/goharbor/harbor/src/controller/tag" + pkg_art "github.com/goharbor/harbor/src/pkg/artifact" + model_tag "github.com/goharbor/harbor/src/pkg/tag/model/tag" + artifacttesting "github.com/goharbor/harbor/src/testing/controller/artifact" repotesting "github.com/goharbor/harbor/src/testing/controller/repository" + "github.com/goharbor/harbor/src/testing/mock" "github.com/stretchr/testify/suite" "net/http" "net/http/httptest" @@ -28,16 +34,21 @@ import ( type catalogTestSuite struct { suite.Suite originalRepoCtl repository.Controller + originalArtCtl artifact.Controller repoCtl *repotesting.FakeController + artCtl *artifacttesting.Controller } func (c *catalogTestSuite) SetupSuite() { c.originalRepoCtl = repository.Ctl + c.originalArtCtl = artifact.Ctl } func (c *catalogTestSuite) SetupTest() { c.repoCtl = &repotesting.FakeController{} repository.Ctl = c.repoCtl + c.artCtl = &artifacttesting.Controller{} + artifact.Ctl = c.artCtl } func (c *catalogTestSuite) TearDownTest() { @@ -45,6 +56,7 @@ func (c *catalogTestSuite) TearDownTest() { func (c *catalogTestSuite) TearDownSuite() { repository.Ctl = c.originalRepoCtl + artifact.Ctl = c.originalArtCtl } func (c *catalogTestSuite) TestCatalog() { @@ -61,6 +73,22 @@ func (c *catalogTestSuite) TestCatalog() { Name: "busybox", }, }, nil) + mock.OnAnything(c.artCtl, "List").Return([]*artifact.Artifact{ + { + Artifact: pkg_art.Artifact{ + ProjectID: 1, + RepositoryID: 1, + }, + Tags: []*tag.Tag{ + { + Tag: model_tag.Tag{ + RepositoryID: 1, + ArtifactID: 1, + }, + }, + }, + }, + }, nil) w = httptest.NewRecorder() newRepositoryHandler().ServeHTTP(w, req) c.Equal(http.StatusOK, w.Code) @@ -87,6 +115,22 @@ func (c *catalogTestSuite) TestCatalogPaginationN1() { Name: "busybox", }, }, nil) + mock.OnAnything(c.artCtl, "List").Return([]*artifact.Artifact{ + { + Artifact: pkg_art.Artifact{ + ProjectID: 1, + RepositoryID: 1, + }, + Tags: []*tag.Tag{ + { + Tag: model_tag.Tag{ + RepositoryID: 1, + ArtifactID: 1, + }, + }, + }, + }, + }, nil) w = httptest.NewRecorder() newRepositoryHandler().ServeHTTP(w, req) c.Equal(http.StatusOK, w.Code) @@ -114,6 +158,22 @@ func (c *catalogTestSuite) TestCatalogPaginationN2() { Name: "busybox", }, }, nil) + mock.OnAnything(c.artCtl, "List").Return([]*artifact.Artifact{ + { + Artifact: pkg_art.Artifact{ + ProjectID: 1, + RepositoryID: 1, + }, + Tags: []*tag.Tag{ + { + Tag: model_tag.Tag{ + RepositoryID: 1, + ArtifactID: 1, + }, + }, + }, + }, + }, nil) w = httptest.NewRecorder() newRepositoryHandler().ServeHTTP(w, req) c.Equal(http.StatusOK, w.Code) @@ -141,6 +201,22 @@ func (c *catalogTestSuite) TestCatalogPaginationN3() { Name: "busybox", }, }, nil) + mock.OnAnything(c.artCtl, "List").Return([]*artifact.Artifact{ + { + Artifact: pkg_art.Artifact{ + ProjectID: 1, + RepositoryID: 1, + }, + Tags: []*tag.Tag{ + { + Tag: model_tag.Tag{ + RepositoryID: 1, + ArtifactID: 1, + }, + }, + }, + }, + }, nil) w = httptest.NewRecorder() newRepositoryHandler().ServeHTTP(w, req) c.Equal(http.StatusOK, w.Code) @@ -154,6 +230,71 @@ func (c *catalogTestSuite) TestCatalogPaginationN3() { c.Equal("hello-world", ctlg.Repositories[0]) } +func (c *catalogTestSuite) TestCatalogUntaggedArtifact() { + c.SetupTest() + req := httptest.NewRequest(http.MethodGet, "/v2/_catalog", nil) + var w *httptest.ResponseRecorder + c.repoCtl.On("List").Return([]*models.RepoRecord{ + { + RepositoryID: 1, + Name: "hello-world", + }, + { + RepositoryID: 2, + Name: "busybox", + }, + }, nil) + // untagged artifact + mock.OnAnything(c.artCtl, "List").Return([]*artifact.Artifact{ + { + Artifact: pkg_art.Artifact{ + ProjectID: 1, + RepositoryID: 1, + }, + }, + }, nil) + w = httptest.NewRecorder() + newRepositoryHandler().ServeHTTP(w, req) + c.Equal(http.StatusOK, w.Code) + var ctlg struct { + Repositories []string `json:"repositories"` + } + decoder := json.NewDecoder(w.Body) + err := decoder.Decode(&ctlg) + c.Nil(err) + c.Equal(0, len(ctlg.Repositories)) +} + +func (c *catalogTestSuite) TestCatalogEmptyRepo() { + c.SetupTest() + req := httptest.NewRequest(http.MethodGet, "/v2/_catalog", nil) + var w *httptest.ResponseRecorder + c.repoCtl.On("List").Return([]*models.RepoRecord{ + { + RepositoryID: 1, + Name: "hello-world", + }, + { + RepositoryID: 2, + Name: "busybox", + }, + }, nil) + // empty repository + mock.OnAnything(c.artCtl, "List").Return([]*artifact.Artifact{ + {}, + }, nil) + w = httptest.NewRecorder() + newRepositoryHandler().ServeHTTP(w, req) + c.Equal(http.StatusOK, w.Code) + var ctlg struct { + Repositories []string `json:"repositories"` + } + decoder := json.NewDecoder(w.Body) + err := decoder.Decode(&ctlg) + c.Nil(err) + c.Equal(0, len(ctlg.Repositories)) +} + func TestCatalogTestSuite(t *testing.T) { suite.Run(t, &catalogTestSuite{}) }