From 0bf5998f963b2b205a79defe6020e85fabece1ce Mon Sep 17 00:00:00 2001 From: Shengwen YU Date: Wed, 9 Feb 2022 12:13:05 +0800 Subject: [PATCH] upgrade Chartmuseum to v0.14.0 (#16334) Signed-off-by: Shengwen Yu Co-authored-by: Shengwen Yu --- Makefile | 2 +- make/photon/chartserver/454.patch | 179 ---------------------- make/photon/chartserver/492.patch | 234 ----------------------------- make/photon/chartserver/builder | 2 +- make/photon/chartserver/compile.sh | 2 +- 5 files changed, 3 insertions(+), 416 deletions(-) delete mode 100644 make/photon/chartserver/454.patch delete mode 100644 make/photon/chartserver/492.patch diff --git a/Makefile b/Makefile index 3b0536e58..599e7d8a3 100644 --- a/Makefile +++ b/Makefile @@ -115,7 +115,7 @@ TRIVYVERSION=v0.22.0 TRIVYADAPTERVERSION=v0.25.0 # version of chartmuseum for pulling the source code -CHARTMUSEUM_SRC_TAG=v0.13.1 +CHARTMUSEUM_SRC_TAG=v0.14.0 # version of chartmuseum CHARTMUSEUMVERSION=$(CHARTMUSEUM_SRC_TAG)-redis diff --git a/make/photon/chartserver/454.patch b/make/photon/chartserver/454.patch deleted file mode 100644 index 9f244c808..000000000 --- a/make/photon/chartserver/454.patch +++ /dev/null @@ -1,179 +0,0 @@ -From 66cc2635880193ffb1226e3c790b36eef24cfd8b Mon Sep 17 00:00:00 2001 -From: scnace -Date: Mon, 3 May 2021 15:09:44 +0800 -Subject: [PATCH 1/2] pkg/chartmuseum/server: add tests for cover duplicate - index entry cases - -Signed-off-by: scnace ---- - pkg/chartmuseum/server/multitenant/api.go | 9 +++++---- - .../server/multitenant/server_test.go | 19 +++++++++++++++++++ - 2 files changed, 24 insertions(+), 4 deletions(-) - -diff --git a/pkg/chartmuseum/server/multitenant/api.go b/pkg/chartmuseum/server/multitenant/api.go -index afc2ab5c..2b03d5e3 100644 ---- a/pkg/chartmuseum/server/multitenant/api.go -+++ b/pkg/chartmuseum/server/multitenant/api.go -@@ -95,15 +95,15 @@ func (server *MultiTenantServer) deleteChartVersion(log cm_logger.LoggingFn, rep - return nil - } - --func (server *MultiTenantServer) uploadChartPackage(log cm_logger.LoggingFn, repo string, content []byte, force bool) (string, *HTTPError ){ -+func (server *MultiTenantServer) uploadChartPackage(log cm_logger.LoggingFn, repo string, content []byte, force bool) (string, *HTTPError) { - filename, err := cm_repo.ChartPackageFilenameFromContent(content) - if err != nil { -- return filename,&HTTPError{http.StatusInternalServerError, err.Error()} -+ return filename, &HTTPError{http.StatusInternalServerError, err.Error()} - } - - if pathutil.Base(filename) != filename { - // Name wants to break out of current directory -- return filename,&HTTPError{http.StatusBadRequest, fmt.Sprintf("%s is improperly formatted", filename)} -+ return filename, &HTTPError{http.StatusBadRequest, fmt.Sprintf("%s is improperly formatted", filename)} - } - - if !server.AllowOverwrite && (!server.AllowForceOverwrite || !force) { -@@ -139,7 +139,8 @@ func (server *MultiTenantServer) uploadChartPackage(log cm_logger.LoggingFn, rep - ) - err = server.StorageBackend.PutObject(pathutil.Join(repo, filename), content) - if err != nil { -- return filename, &HTTPError{http.StatusInternalServerError, err.Error()} } -+ return filename, &HTTPError{http.StatusInternalServerError, err.Error()} -+ } - return filename, nil - } - -diff --git a/pkg/chartmuseum/server/multitenant/server_test.go b/pkg/chartmuseum/server/multitenant/server_test.go -index 13050e25..138364f8 100644 ---- a/pkg/chartmuseum/server/multitenant/server_test.go -+++ b/pkg/chartmuseum/server/multitenant/server_test.go -@@ -339,6 +339,7 @@ func (suite *MultiTenantServerTestSuite) SetupSuite() { - AllowOverwrite: true, - ChartPostFormFieldName: "chart", - ProvPostFormFieldName: "prov", -+ CacheInterval: time.Duration(time.Second), - }) - suite.NotNil(server) - suite.Nil(err, "no error creating new overwrite server") -@@ -627,6 +628,14 @@ func (suite *MultiTenantServerTestSuite) TestDisabledDeleteServer() { - suite.Equal(404, res.Status(), "404 DELETE /api/charts/mychart/0.1.0") - } - -+func (suite *MultiTenantServerTestSuite) extractRepoEntryFromInternalCache(repo string) *cacheEntry { -+ local, ok := suite.OverwriteServer.InternalCacheStore[repo] -+ if ok { -+ return local -+ } -+ return nil -+} -+ - func (suite *MultiTenantServerTestSuite) TestOverwriteServer() { - // Check if files can be overwritten - content, err := ioutil.ReadFile(testTarballPath) -@@ -638,6 +647,16 @@ func (suite *MultiTenantServerTestSuite) TestOverwriteServer() { - res = suite.doRequest("overwrite", "POST", "/api/charts", body, "") - suite.Equal(201, res.Status(), "201 POST /api/charts") - -+ { -+ // waiting for the emit event -+ // the event is transferred via a channel , here do a simple wait for not changing the original structure -+ // only for testing purpose -+ time.Sleep(time.Second) -+ // depth: 0 -+ e := suite.extractRepoEntryFromInternalCache("") -+ suite.Equal(1, len(e.RepoIndex.Entries), "overwrite entries validation") -+ } -+ - content, err = ioutil.ReadFile(testProvfilePath) - suite.Nil(err, "no error opening test provenance file") - body = bytes.NewBuffer(content) - -From cd2e286da8148a7c114cb45867bf5c7b09e29467 Mon Sep 17 00:00:00 2001 -From: scnace -Date: Mon, 3 May 2021 15:42:00 +0800 -Subject: [PATCH 2/2] pkg/chartmuseum/server/multitenant: fix the bad action - type when upload package when overwrite option is set ,index entry addChart - should be updateChart under the overwrite cases. - -Signed-off-by: scnace ---- - pkg/chartmuseum/server/multitenant/api.go | 18 +++++++++++++++--- - pkg/chartmuseum/server/multitenant/handlers.go | 18 +++++++++++++++--- - 2 files changed, 30 insertions(+), 6 deletions(-) - -diff --git a/pkg/chartmuseum/server/multitenant/api.go b/pkg/chartmuseum/server/multitenant/api.go -index 2b03d5e3..902ab7c6 100644 ---- a/pkg/chartmuseum/server/multitenant/api.go -+++ b/pkg/chartmuseum/server/multitenant/api.go -@@ -106,11 +106,18 @@ func (server *MultiTenantServer) uploadChartPackage(log cm_logger.LoggingFn, rep - return filename, &HTTPError{http.StatusBadRequest, fmt.Sprintf("%s is improperly formatted", filename)} - } - -- if !server.AllowOverwrite && (!server.AllowForceOverwrite || !force) { -- _, err = server.StorageBackend.GetObject(pathutil.Join(repo, filename)) -- if err == nil { -+ // we should ensure that whether chart is existed even if the `overwrite` option is set -+ // For `overwrite` option , here will increase one `storage.GetObject` than before ; others should be equalvarant with the previous version. -+ var found bool -+ _, err = server.StorageBackend.GetObject(pathutil.Join(repo, filename)) -+ // found -+ if err == nil { -+ found = true -+ // For those no-overwrite servers, return the Conflict error. -+ if !server.AllowOverwrite && (!server.AllowForceOverwrite || !force) { - return filename, &HTTPError{http.StatusConflict, "file already exists"} - } -+ // continue with the `overwrite` servers - } - - if server.EnforceSemver2 { -@@ -141,6 +148,11 @@ func (server *MultiTenantServer) uploadChartPackage(log cm_logger.LoggingFn, rep - if err != nil { - return filename, &HTTPError{http.StatusInternalServerError, err.Error()} - } -+ if found { -+ // here is a fake conflict error for outside call -+ // In order to not add another return `bool` check (API Compatibility) -+ return filename, &HTTPError{http.StatusConflict, ""} -+ } - return filename, nil - } - -diff --git a/pkg/chartmuseum/server/multitenant/handlers.go b/pkg/chartmuseum/server/multitenant/handlers.go -index 3e1f0602..c6c31b01 100644 ---- a/pkg/chartmuseum/server/multitenant/handlers.go -+++ b/pkg/chartmuseum/server/multitenant/handlers.go -@@ -242,10 +242,22 @@ func (server *MultiTenantServer) postPackageRequestHandler(c *gin.Context) { - } - log := server.Logger.ContextLoggingFn(c) - _, force := c.GetQuery("force") -+ action := addChart - filename, err := server.uploadChartPackage(log, repo, content, force) - if err != nil { -- c.JSON(err.Status, gin.H{"error": err.Message}) -- return -+ // here should check both err.Status and err.Message -+ // The http.StatusConflict status means the chart is existed but overwrite is not sed OR chart is existed and overwrite is set -+ // err.Status == http.StatusConflict only denotes for chart is existed now. -+ if err.Status == http.StatusConflict { -+ if err.Message != "" { -+ c.JSON(err.Status, gin.H{"error": err.Message}) -+ return -+ } -+ action = updateChart -+ } else { -+ c.JSON(err.Status, gin.H{"error": err.Message}) -+ return -+ } - } - - chart, chartErr := cm_repo.ChartVersionFromStorageObject(cm_storage.Object{ -@@ -255,7 +267,7 @@ func (server *MultiTenantServer) postPackageRequestHandler(c *gin.Context) { - if chartErr != nil { - log(cm_logger.ErrorLevel, "cannot get chart from content", zap.Error(chartErr), zap.Binary("content", content)) - } -- server.emitEvent(c, repo, addChart, chart) -+ server.emitEvent(c, repo, action, chart) - - c.JSON(201, objectSavedResponse) - } diff --git a/make/photon/chartserver/492.patch b/make/photon/chartserver/492.patch deleted file mode 100644 index 1067b4d23..000000000 --- a/make/photon/chartserver/492.patch +++ /dev/null @@ -1,234 +0,0 @@ -From 5dd7f0370f73cdffa76707e4f1f715ee4e209f3e Mon Sep 17 00:00:00 2001 -From: DQ -Date: Fri, 24 Sep 2021 17:56:00 +0000 -Subject: [PATCH 1/2] Fix duplicate versions for same chart - -* The detailed issue is described in #450 -* And there is a PR #454 fixed one scenario of this issue -* But there is another ocassion in which users upload chart with prov -* in this PR is to handle this situation with the way similar with #454 - -Signed-off-by: DQ ---- - .../server/multitenant/handlers.go | 55 +++++++++++++------ - .../server/multitenant/server_test.go | 7 +++ - 2 files changed, 46 insertions(+), 16 deletions(-) - -diff --git a/pkg/chartmuseum/server/multitenant/handlers.go b/pkg/chartmuseum/server/multitenant/handlers.go -index c6c31b0..a39a00d 100644 ---- a/pkg/chartmuseum/server/multitenant/handlers.go -+++ b/pkg/chartmuseum/server/multitenant/handlers.go -@@ -299,8 +299,24 @@ func (server *MultiTenantServer) postPackageAndProvenanceRequestHandler(c *gin.C - _, force := c.GetQuery("force") - var chartContent []byte - var path string -+ // action used to determine what operation to emit -+ action := addChart - cpFiles, status, err := server.getChartAndProvFiles(c.Request, repo, force) -- if status != 200 { -+ if err != nil { -+ c.JSON(status, gin.H{"error": fmt.Sprintf("%s", err)}) -+ return -+ } -+ switch status { -+ case http.StatusOK: -+ case http.StatusConflict: -+ if !server.AllowOverwrite && (!server.AllowForceOverwrite || !force) { -+ c.JSON(status, gin.H{"error": fmt.Sprintf("%s", fmt.Errorf("chart already exists"))}) // conflict -+ return -+ } -+ log(cm_logger.DebugLevel, "chart already exists, but overwrite is allowed", zap.String("repo", repo)) -+ // update chart if chart already exists and overwrite is allowed -+ action = updateChart -+ default: - c.JSON(status, gin.H{"error": fmt.Sprintf("%s", err)}) - return - } -@@ -309,7 +325,7 @@ func (server *MultiTenantServer) postPackageAndProvenanceRequestHandler(c *gin.C - if len(c.Errors) > 0 { - return // this is a "request too large" - } -- c.JSON(400, gin.H{"error": fmt.Sprintf( -+ c.JSON(http.StatusBadRequest, gin.H{"error": fmt.Sprintf( - "no package or provenance file found in form fields %s and %s", - server.ChartPostFormFieldName, server.ProvPostFormFieldName), - }) -@@ -332,7 +348,7 @@ func (server *MultiTenantServer) postPackageAndProvenanceRequestHandler(c *gin.C - for _, ppf := range storedFiles { - server.StorageBackend.DeleteObject(ppf.filename) - } -- c.JSON(500, gin.H{"error": fmt.Sprintf("%s", err)}) -+ c.JSON(http.StatusInternalServerError, gin.H{"error": fmt.Sprintf("%s", err)}) - return - } - if ppf.field == defaultFormField { -@@ -350,9 +366,9 @@ func (server *MultiTenantServer) postPackageAndProvenanceRequestHandler(c *gin.C - log(cm_logger.ErrorLevel, "cannot get chart from content", zap.Error(err), zap.Binary("content", chartContent)) - } - -- server.emitEvent(c, repo, addChart, chart) -+ server.emitEvent(c, repo, action, chart) - -- c.JSON(201, objectSavedResponse) -+ c.JSON(http.StatusCreated, objectSavedResponse) - } - - func (server *MultiTenantServer) getChartAndProvFiles(req *http.Request, repo string, force bool) (map[string]*chartOrProvenanceFile, int, error) { -@@ -368,29 +384,36 @@ func (server *MultiTenantServer) getChartAndProvFiles(req *http.Request, repo st - {server.ProvPostFormFieldName, cm_repo.ProvenanceFilenameFromContent}, - } - -+ validStatusCode := http.StatusOK - cpFiles := make(map[string]*chartOrProvenanceFile) - for _, ff := range ffp { - content, err := extractContentFromRequest(req, ff.field) - if err != nil { -- return nil, 500, err -+ return nil, http.StatusInternalServerError, err - } - if content == nil { - continue - } - filename, err := ff.fn(content) - if err != nil { -- return nil, 400, err -+ return nil, http.StatusBadRequest, err - } - if _, ok := cpFiles[filename]; ok { - continue - } -- if status, err := server.validateChartOrProv(repo, filename, force); err != nil { -+ status, err := server.validateChartOrProv(repo, filename, force) -+ if err != nil { - return nil, status, err - } -+ // return conflict status code if the file already exists -+ if status == http.StatusConflict && validStatusCode != http.StatusConflict { -+ validStatusCode = status -+ } - cpFiles[filename] = &chartOrProvenanceFile{filename, content, ff.field} - } - -- return cpFiles, 200, nil -+ // validState code can be 200 or 409. Returning 409 means that the chart already exists -+ return cpFiles, validStatusCode, nil - } - - func extractContentFromRequest(req *http.Request, field string) ([]byte, error) { -@@ -408,7 +431,7 @@ func extractContentFromRequest(req *http.Request, field string) ([]byte, error) - - func (server *MultiTenantServer) validateChartOrProv(repo, filename string, force bool) (int, error) { - if pathutil.Base(filename) != filename { -- return 400, fmt.Errorf("%s is improperly formatted", filename) // Name wants to break out of current directory -+ return http.StatusBadRequest, fmt.Errorf("%s is improperly formatted", filename) // Name wants to break out of current directory - } - - var f string -@@ -417,11 +440,11 @@ func (server *MultiTenantServer) validateChartOrProv(repo, filename string, forc - } else { - f = repo + "/" + filename - } -- if !server.AllowOverwrite && (!server.AllowForceOverwrite || !force) { -- _, err := server.StorageBackend.GetObject(f) -- if err == nil { -- return 409, fmt.Errorf("%s already exists", f) // conflict -- } -+ // conflict does not mean the file is invalid. -+ // for example, when overwite is allowed, it's valid -+ // so that the client can decide what to do and here we just return conflict with no error -+ if _, err := server.StorageBackend.GetObject(f); err == nil { -+ return http.StatusConflict, nil - } -- return 200, nil -+ return http.StatusOK, nil - } -diff --git a/pkg/chartmuseum/server/multitenant/server_test.go b/pkg/chartmuseum/server/multitenant/server_test.go -index 138364f..477f349 100644 ---- a/pkg/chartmuseum/server/multitenant/server_test.go -+++ b/pkg/chartmuseum/server/multitenant/server_test.go -@@ -672,6 +672,13 @@ func (suite *MultiTenantServerTestSuite) TestOverwriteServer() { - buf, w = suite.getBodyWithMultipartFormFiles([]string{"chart", "prov"}, []string{testTarballPath, testProvfilePath}) - res = suite.doRequest("overwrite", "POST", "/api/charts", buf, w.FormDataContentType()) - suite.Equal(201, res.Status(), "201 POST /api/charts") -+ { -+ // the same as chart only case above -+ time.Sleep(time.Second) -+ // depth: 0 -+ e := suite.extractRepoEntryFromInternalCache("") -+ suite.Equal(1, len(e.RepoIndex.Entries), "overwrite entries validation") -+ } - } - - func (suite *MultiTenantServerTestSuite) TestBadChartUpload() { - -From 1ecdaa5811178f4d4d6d1cc8077c354cc8f5859f Mon Sep 17 00:00:00 2001 -From: DQ -Date: Thu, 30 Sep 2021 13:54:30 +0000 -Subject: [PATCH 2/2] Enhance: optimize loop in `getChartAndProvFiles` - -* If conflict, it didn't need to do the left logic, just return the file -* move out file format check logic out of `validateChartOrProv` -* these changes are discussed in https://github.com/helm/chartmuseum/pull/492#discussion_r716032288 - -Signed-off-by: DQ ---- - .../server/multitenant/handlers.go | 22 ++++++++++++------- - 1 file changed, 14 insertions(+), 8 deletions(-) - -diff --git a/pkg/chartmuseum/server/multitenant/handlers.go b/pkg/chartmuseum/server/multitenant/handlers.go -index a39a00d..fbaf450 100644 ---- a/pkg/chartmuseum/server/multitenant/handlers.go -+++ b/pkg/chartmuseum/server/multitenant/handlers.go -@@ -384,7 +384,7 @@ func (server *MultiTenantServer) getChartAndProvFiles(req *http.Request, repo st - {server.ProvPostFormFieldName, cm_repo.ProvenanceFilenameFromContent}, - } - -- validStatusCode := http.StatusOK -+ validReturnStatusCode := http.StatusOK - cpFiles := make(map[string]*chartOrProvenanceFile) - for _, ff := range ffp { - content, err := extractContentFromRequest(req, ff.field) -@@ -401,19 +401,29 @@ func (server *MultiTenantServer) getChartAndProvFiles(req *http.Request, repo st - if _, ok := cpFiles[filename]; ok { - continue - } -+ // if the file already exists, we don't need to validate it again -+ if validReturnStatusCode == http.StatusConflict { -+ cpFiles[filename] = &chartOrProvenanceFile{filename, content, ff.field} -+ continue -+ } -+ // check filename -+ if pathutil.Base(filename) != filename { -+ return nil, http.StatusBadRequest, fmt.Errorf("%s is improperly formatted", filename) // Name wants to break out of current directory -+ } -+ // check existence - status, err := server.validateChartOrProv(repo, filename, force) - if err != nil { - return nil, status, err - } - // return conflict status code if the file already exists -- if status == http.StatusConflict && validStatusCode != http.StatusConflict { -- validStatusCode = status -+ if status == http.StatusConflict { -+ validReturnStatusCode = status - } - cpFiles[filename] = &chartOrProvenanceFile{filename, content, ff.field} - } - - // validState code can be 200 or 409. Returning 409 means that the chart already exists -- return cpFiles, validStatusCode, nil -+ return cpFiles, validReturnStatusCode, nil - } - - func extractContentFromRequest(req *http.Request, field string) ([]byte, error) { -@@ -430,10 +440,6 @@ func extractContentFromRequest(req *http.Request, field string) ([]byte, error) - } - - func (server *MultiTenantServer) validateChartOrProv(repo, filename string, force bool) (int, error) { -- if pathutil.Base(filename) != filename { -- return http.StatusBadRequest, fmt.Errorf("%s is improperly formatted", filename) // Name wants to break out of current directory -- } -- - var f string - if repo == "" { - f = filename diff --git a/make/photon/chartserver/builder b/make/photon/chartserver/builder index 23bacee0e..4ef40cd80 100755 --- a/make/photon/chartserver/builder +++ b/make/photon/chartserver/builder @@ -4,7 +4,7 @@ set +e usage(){ echo "Usage: builder " - echo "e.g: builder golang:1.17.6 github.com/helm/chartmuseum v0.12.0 cmd/chartmuseum chartm" + echo "e.g: builder golang:1.17.6 github.com/helm/chartmuseum v0.14.0 cmd/chartmuseum chartm" exit 1 } diff --git a/make/photon/chartserver/compile.sh b/make/photon/chartserver/compile.sh index 6eb2bd5ef..07a18d388 100644 --- a/make/photon/chartserver/compile.sh +++ b/make/photon/chartserver/compile.sh @@ -3,7 +3,7 @@ set +e usage(){ echo "Usage: compile.sh " - echo "e.g: compile.sh github.com/helm/chartmuseum v0.5.1 cmd/chartmuseum chartm" + echo "e.g: compile.sh github.com/helm/chartmuseum v0.14.0 cmd/chartmuseum chartm" exit 1 }