From b5011990331b7ca45e2d719685594680edab6172 Mon Sep 17 00:00:00 2001 From: Steven Zou Date: Fri, 3 Aug 2018 17:50:03 +0800 Subject: [PATCH] Enhance the project deletable checking logic to include helm charts checking add utility handler in chart controller to support getting charts by ns add ut case for utility handler update deletable checking logic in project controller refactor project deletable test case --- src/chartserver/controller.go | 13 ++++++++ src/chartserver/utility_handler.go | 37 +++++++++++++++++++++ src/chartserver/utility_handler_test.go | 44 +++++++++++++++++++++++++ src/ui/api/project.go | 25 +++++++++++--- src/ui/api/project_test.go | 44 +++++++++++++++++++++++++ 5 files changed, 158 insertions(+), 5 deletions(-) create mode 100644 src/chartserver/utility_handler.go create mode 100644 src/chartserver/utility_handler_test.go diff --git a/src/chartserver/controller.go b/src/chartserver/controller.go index 9f2824a16..bf148387b 100644 --- a/src/chartserver/controller.go +++ b/src/chartserver/controller.go @@ -36,6 +36,9 @@ type Controller struct { //To cover all the manipulation requests manipulationHandler *ManipulationHandler + + //To cover the other utility requests + utilityHandler *UtilityHandler } //NewController is constructor of the chartserver.Controller @@ -86,6 +89,11 @@ func NewController(backendServer *url.URL) (*Controller, error) { backendServerAddress: backendServer, chartCache: cache, }, + utilityHandler: &UtilityHandler{ + apiClient: client, + backendServerAddress: backendServer, + chartOperator: operator, + }, }, nil } @@ -104,6 +112,11 @@ func (c *Controller) GetManipulationHandler() *ManipulationHandler { return c.manipulationHandler } +//GetUtilityHandler returns the reference of UtilityHandler +func (c *Controller) GetUtilityHandler() *UtilityHandler { + return c.utilityHandler +} + //What's the cache driver if it is set func parseCacheDriver() (string, bool) { driver, ok := os.LookupEnv(cacheDriverENVKey) diff --git a/src/chartserver/utility_handler.go b/src/chartserver/utility_handler.go new file mode 100644 index 000000000..dc09f83fb --- /dev/null +++ b/src/chartserver/utility_handler.go @@ -0,0 +1,37 @@ +package chartserver + +import ( + "errors" + "fmt" + "net/url" + "strings" +) + +//UtilityHandler provides utility methods +type UtilityHandler struct { + //Parse and process the chart version to provide required info data + chartOperator *ChartOperator + + //HTTP client used to call the realted APIs of the backend chart repositories + apiClient *ChartClient + + //Point to the url of the backend server + backendServerAddress *url.URL +} + +//GetChartsByNs gets the chart list under the namespace +func (uh *UtilityHandler) GetChartsByNs(namespace string) ([]*ChartInfo, error) { + if len(strings.TrimSpace(namespace)) == 0 { + return nil, errors.New("empty namespace when getting chart list") + } + + path := fmt.Sprintf("/api/%s/charts", namespace) + url := fmt.Sprintf("%s%s", uh.backendServerAddress.String(), path) + + content, err := uh.apiClient.GetContent(url) + if err != nil { + return nil, err + } + + return uh.chartOperator.GetChartList(content) +} diff --git a/src/chartserver/utility_handler_test.go b/src/chartserver/utility_handler_test.go new file mode 100644 index 000000000..a88dae63c --- /dev/null +++ b/src/chartserver/utility_handler_test.go @@ -0,0 +1,44 @@ +package chartserver + +import ( + "net/http" + "net/http/httptest" + "net/url" + "testing" +) + +//TestGetChartsByNs tests GetChartsByNs method in UtilityHandler +func TestGetChartsByNs(t *testing.T) { + mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.RequestURI { + case "/api/repo1/charts": + if r.Method == http.MethodGet { + w.Write(chartListContent) + return + } + } + + w.WriteHeader(http.StatusNotImplemented) + w.Write([]byte("not supported")) + })) + defer mockServer.Close() + + serverURL, err := url.Parse(mockServer.URL) + if err != nil { + t.Fatal(err) + } + + theController, err := NewController(serverURL) + if err != nil { + t.Fatal(err) + } + + charts, err := theController.GetUtilityHandler().GetChartsByNs("repo1") + if err != nil { + t.Fatal(err) + } + + if len(charts) != 2 { + t.Fatalf("expect 2 items but got %d", len(charts)) + } +} diff --git a/src/ui/api/project.go b/src/ui/api/project.go index e75a6e72d..006f70a84 100644 --- a/src/ui/api/project.go +++ b/src/ui/api/project.go @@ -217,7 +217,7 @@ func (p *ProjectAPI) Delete() { return } - result, err := deletable(p.project.ProjectID) + result, err := p.deletable(p.project.ProjectID) if err != nil { p.HandleInternalServerError(fmt.Sprintf( "failed to check the deletable of project %d: %v", p.project.ProjectID, err)) @@ -258,7 +258,7 @@ func (p *ProjectAPI) Deletable() { return } - result, err := deletable(p.project.ProjectID) + result, err := p.deletable(p.project.ProjectID) if err != nil { p.HandleInternalServerError(fmt.Sprintf( "failed to check the deletable of project %d: %v", p.project.ProjectID, err)) @@ -269,7 +269,7 @@ func (p *ProjectAPI) Deletable() { p.ServeJSON() } -func deletable(projectID int64) (*deletableResp, error) { +func (p *ProjectAPI) deletable(projectID int64) (*deletableResp, error) { count, err := dao.GetTotalOfRepositories(&models.RepositoryQuery{ ProjectIDs: []int64{projectID}, }) @@ -280,7 +280,7 @@ func deletable(projectID int64) (*deletableResp, error) { if count > 0 { return &deletableResp{ Deletable: false, - Message: "the project contains repositories, can not be deleled", + Message: "the project contains repositories, can not be deleted", }, nil } @@ -292,10 +292,25 @@ func deletable(projectID int64) (*deletableResp, error) { if len(policies) > 0 { return &deletableResp{ Deletable: false, - Message: "the project contains replication rules, can not be deleled", + Message: "the project contains replication rules, can not be deleted", }, nil } + //Check helm charts number + if config.WithChartMuseum() { + charts, err := chartController.GetUtilityHandler().GetChartsByNs(p.project.Name) + if err != nil { + return nil, err + } + + if len(charts) > 0 { + return &deletableResp{ + Deletable: false, + Message: "the project contains helm charts, can not be deleted", + }, nil + } + } + return &deletableResp{ Deletable: true, }, nil diff --git a/src/ui/api/project_test.go b/src/ui/api/project_test.go index f2ef7a7d5..4a10e7394 100644 --- a/src/ui/api/project_test.go +++ b/src/ui/api/project_test.go @@ -16,10 +16,14 @@ package api import ( "fmt" "net/http" + "net/http/httptest" + "net/url" "strconv" "testing" "time" + "github.com/vmware/harbor/src/chartserver" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/vmware/harbor/src/common/dao" @@ -362,6 +366,13 @@ func TestProjectLogsFilter(t *testing.T) { func TestDeletable(t *testing.T) { apiTest := newHarborAPI() + chServer, oldController, err := mockChartController() + require.Nil(t, err) + require.NotNil(t, chServer) + defer chServer.Close() + defer func() { + chartController = oldController + }() project := models.Project{ Name: "project_for_test_deletable", @@ -398,3 +409,36 @@ func TestDeletable(t *testing.T) { assert.Equal(t, http.StatusOK, code) assert.False(t, del) } + +//Provides a mock chart controller for deletable test cases +func mockChartController() (*httptest.Server, *chartserver.Controller, error) { + mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.RequestURI { + case "/api/project_for_test_deletable/charts": + if r.Method == http.MethodGet { + w.Write([]byte("{}")) + return + } + } + + w.WriteHeader(http.StatusNotImplemented) + w.Write([]byte("not supported")) + })) + + var oldController, newController *chartserver.Controller + url, err := url.Parse(mockServer.URL) + if err == nil { + newController, err = chartserver.NewController(url) + } + + if err != nil { + mockServer.Close() + return nil, nil, err + } + + //Override current controller and keep the old one for restoring + oldController = chartController + chartController = newController + + return mockServer, oldController, nil +}