From 84c982306fb38bf243f8a59ea50a560d8ea64dc6 Mon Sep 17 00:00:00 2001 From: scnace Date: Mon, 14 Jun 2021 15:37:25 +0800 Subject: [PATCH] cmd/chartmuseum,pkg/chartmuseum,pkg/config: add new per-chart-limit-option , impls #316 Signed-off-by: scnace --- cmd/chartmuseum/main.go | 1 + pkg/chartmuseum/server.go | 4 + pkg/chartmuseum/server/multitenant/api.go | 80 ++++++++++++++++++- pkg/chartmuseum/server/multitenant/server.go | 20 ++++- .../server/multitenant/server_test.go | 63 +++++++++++++++ pkg/config/vars.go | 9 +++ scripts/setup-test-environment.sh | 3 + 7 files changed, 175 insertions(+), 5 deletions(-) diff --git a/cmd/chartmuseum/main.go b/cmd/chartmuseum/main.go index eab910b7..97e12f0b 100644 --- a/cmd/chartmuseum/main.go +++ b/cmd/chartmuseum/main.go @@ -104,6 +104,7 @@ func cliHandler(c *cli.Context) { EnforceSemver2: conf.GetBool("enforce-semver2"), CacheInterval: conf.GetDuration("cacheinterval"), Host: conf.GetString("listen.host"), + PerChartLimit: conf.GetInt("per-chart-limit"), } server, err := newServer(options) diff --git a/pkg/chartmuseum/server.go b/pkg/chartmuseum/server.go index 6084838f..99088b70 100644 --- a/pkg/chartmuseum/server.go +++ b/pkg/chartmuseum/server.go @@ -72,6 +72,9 @@ type ( CacheInterval time.Duration Host string Version string + // PerChartLimit allow museum server to keep max N version Charts + // And avoid swelling too large(if so , the index genertion will become slow) + PerChartLimit int } // Server is a generic interface for web servers @@ -140,6 +143,7 @@ func NewServer(options ServerOptions) (Server, error) { AllowForceOverwrite: options.AllowForceOverwrite, Version: options.Version, CacheInterval: options.CacheInterval, + PerChartLimit: options.PerChartLimit, // Deprecated options // EnforceSemver2 - see https://github.com/helm/chartmuseum/issues/485 for more info EnforceSemver2: options.EnforceSemver2, diff --git a/pkg/chartmuseum/server/multitenant/api.go b/pkg/chartmuseum/server/multitenant/api.go index 05af1528..143fda98 100644 --- a/pkg/chartmuseum/server/multitenant/api.go +++ b/pkg/chartmuseum/server/multitenant/api.go @@ -21,10 +21,14 @@ import ( "net/http" pathutil "path/filepath" "sort" + "strings" + "github.com/chartmuseum/storage" + "github.com/gin-gonic/gin" cm_logger "helm.sh/chartmuseum/pkg/chartmuseum/logger" cm_repo "helm.sh/chartmuseum/pkg/repo" + "helm.sh/helm/v3/pkg/chart" helm_repo "helm.sh/helm/v3/pkg/repo" ) @@ -130,8 +134,7 @@ func (server *MultiTenantServer) uploadChartPackage(log cm_logger.LoggingFn, rep log(cm_logger.DebugLevel, "Adding package to storage", "package", filename, ) - err = server.StorageBackend.PutObject(pathutil.Join(repo, filename), content) - if err != nil { + if err := server.PutWithLimit(&gin.Context{}, log, repo, filename, content); err != nil { return filename, &HTTPError{http.StatusInternalServerError, err.Error()} } if found { @@ -199,3 +202,76 @@ func (server *MultiTenantServer) checkStorageLimit(repo string, filename string, } return false, nil } + +func extractFromChart(content []byte) (name string, version string, err error) { + cv, err := cm_repo.ChartVersionFromStorageObject(storage.Object{ + Content: content, + }) + if err != nil { + return "", "", err + } + return cv.Metadata.Name, cv.Metadata.Version, nil +} + +func (server *MultiTenantServer) PutWithLimit(ctx *gin.Context, log cm_logger.LoggingFn, repo string, + filename string, content []byte) error { + if server.ChartLimits == nil { + log(cm_logger.DebugLevel, "PutWithLimit: per-chart-limit not set") + return server.StorageBackend.PutObject(pathutil.Join(repo, filename), content) + } + limit := server.ChartLimits.Limit + name, _, err := extractFromChart(content) + if err != nil { + return err + } + // lock the backend storage resource to always get the correct one + server.ChartLimits.Lock() + defer server.ChartLimits.Unlock() + // clean the oldest chart(both index and storage) + // storage cache first + objs, err := server.StorageBackend.ListObjects(repo) + if err != nil { + return err + } + var newObjs []storage.Object + for _, obj := range objs { + if !strings.HasPrefix(obj.Path, name) || strings.HasSuffix(obj.Path, ".prov") { + continue + } + log(cm_logger.DebugLevel, "PutWithLimit", "current object name", obj.Path) + newObjs = append(newObjs, obj) + } + if len(newObjs) < limit { + log(cm_logger.DebugLevel, "PutWithLimit", "current objects", len(newObjs)) + return server.StorageBackend.PutObject(pathutil.Join(repo, filename), content) + } + sort.Slice(newObjs, func(i, j int) bool { + return newObjs[i].LastModified.Unix() < newObjs[j].LastModified.Unix() + }) + + log(cm_logger.DebugLevel, "PutWithLimit", "old chart", newObjs[0].Path) + // should we support delete N out-of-date charts ? + // and should we must ensure the delete operation is ok ? + o, err := server.StorageBackend.GetObject(pathutil.Join(repo, newObjs[0].Path)) + if err != nil { + return err + } + if err := server.StorageBackend.DeleteObject(pathutil.Join(repo, newObjs[0].Path)); err != nil { + return fmt.Errorf("PutWithLimit: clean the old chart: %w", err) + } + cv, err := cm_repo.ChartVersionFromStorageObject(o) + if err != nil { + return fmt.Errorf("PutWithLimit: extract chartversion from storage object: %w", err) + } + if err = server.StorageBackend.PutObject(pathutil.Join(repo, filename), content); err != nil { + return fmt.Errorf("PutWithLimit: put new chart: %w", err) + } + go server.emitEvent(ctx, repo, deleteChart, &helm_repo.ChartVersion{ + Metadata: &chart.Metadata{ + Name: cv.Name, + Version: cv.Version, + }, + Removed: true, + }) + return nil +} diff --git a/pkg/chartmuseum/server/multitenant/server.go b/pkg/chartmuseum/server/multitenant/server.go index fb084309..5b1114a3 100644 --- a/pkg/chartmuseum/server/multitenant/server.go +++ b/pkg/chartmuseum/server/multitenant/server.go @@ -27,7 +27,6 @@ import ( cm_router "helm.sh/chartmuseum/pkg/chartmuseum/router" cm_repo "helm.sh/chartmuseum/pkg/repo" - "github.com/chartmuseum/storage" cm_storage "github.com/chartmuseum/storage" "github.com/gin-gonic/gin" ) @@ -47,7 +46,7 @@ type ( MultiTenantServer struct { Logger *cm_logger.Logger Router *cm_router.Router - StorageBackend storage.Backend + StorageBackend cm_storage.Backend TimestampTolerance time.Duration ExternalCacheStore cache.Store InternalCacheStore map[string]*cacheEntry @@ -68,13 +67,19 @@ type ( TenantCacheKeyLock *sync.Mutex CacheInterval time.Duration EventChan chan event + ChartLimits *ObjectsPerChartLimit + } + + ObjectsPerChartLimit struct { + *sync.Mutex + Limit int } // MultiTenantServerOptions are options for constructing a MultiTenantServer MultiTenantServerOptions struct { Logger *cm_logger.Logger Router *cm_router.Router - StorageBackend storage.Backend + StorageBackend cm_storage.Backend ExternalCacheStore cache.Store TimestampTolerance time.Duration ChartURL string @@ -91,6 +96,7 @@ type ( UseStatefiles bool EnforceSemver2 bool CacheInterval time.Duration + PerChartLimit int } tenantInternals struct { @@ -117,6 +123,13 @@ func NewMultiTenantServer(options MultiTenantServerOptions) (*MultiTenantServer, if options.ChartURL != "" { chartURL = options.ChartURL + options.Router.ContextPath } + var l *ObjectsPerChartLimit + if options.PerChartLimit > 0 { + l = &ObjectsPerChartLimit{ + Mutex: &sync.Mutex{}, + Limit: options.PerChartLimit, + } + } server := &MultiTenantServer{ Logger: options.Logger, @@ -141,6 +154,7 @@ func NewMultiTenantServer(options MultiTenantServerOptions) (*MultiTenantServer, Tenants: map[string]*tenantInternals{}, TenantCacheKeyLock: &sync.Mutex{}, CacheInterval: options.CacheInterval, + ChartLimits: l, } server.Router.SetRoutes(server.Routes()) diff --git a/pkg/chartmuseum/server/multitenant/server_test.go b/pkg/chartmuseum/server/multitenant/server_test.go index 1865bfa6..8cd2be3c 100644 --- a/pkg/chartmuseum/server/multitenant/server_test.go +++ b/pkg/chartmuseum/server/multitenant/server_test.go @@ -44,6 +44,7 @@ var maxUploadSize = 1024 * 1024 * 20 // These are generated from scripts/setup-test-environment.sh var testTarballPath = "../../../../testdata/charts/mychart/mychart-0.1.0.tgz" var testTarballPathV2 = "../../../../testdata/charts/mychart/mychart-0.2.0.tgz" +var testTarballPathV0 = "../../../../testdata/charts/mychart/mychart-0.0.1.tgz" var testProvfilePath = "../../../../testdata/charts/mychart/mychart-0.1.0.tgz.prov" var otherTestTarballPath = "../../../../testdata/charts/otherchart/otherchart-0.1.0.tgz" var otherTestProvfilePath = "../../../../testdata/charts/otherchart/otherchart-0.1.0.tgz.prov" @@ -64,6 +65,7 @@ type MultiTenantServerTestSuite struct { MaxObjectsServer *MultiTenantServer MaxUploadSizeServer *MultiTenantServer Semver2Server *MultiTenantServer + PerChartLimitServer *MultiTenantServer TempDirectory string TestTarballFilename string TestProvfileFilename string @@ -109,6 +111,8 @@ func (suite *MultiTenantServerTestSuite) doRequest(stype string, method string, suite.MaxUploadSizeServer.Router.HandleContext(c) case "semver2": suite.Semver2Server.Router.HandleContext(c) + case "per-chart-limit": + suite.PerChartLimitServer.Router.HandleContext(c) } return c.Writer @@ -344,6 +348,12 @@ func (suite *MultiTenantServerTestSuite) SetupSuite() { suite.Nil(err, "no error creating new overwrite server") suite.OverwriteServer = server + router = cm_router.NewRouter(cm_router.RouterOptions{ + Logger: logger, + Depth: 0, + MaxUploadSize: maxUploadSize, + }) + server, err = NewMultiTenantServer(MultiTenantServerOptions{ Logger: logger, Router: router, @@ -357,6 +367,27 @@ func (suite *MultiTenantServerTestSuite) SetupSuite() { suite.Nil(err, "no error creating semantic version server") suite.Semver2Server = server + router = cm_router.NewRouter(cm_router.RouterOptions{ + Logger: logger, + Depth: 0, + MaxUploadSize: maxUploadSize, + }) + + server, err = NewMultiTenantServer(MultiTenantServerOptions{ + Logger: logger, + Router: router, + StorageBackend: backend, + TimestampTolerance: time.Duration(0), + EnableAPI: true, + AllowOverwrite: true, + ChartPostFormFieldName: "chart", + CacheInterval: time.Duration(time.Second), + PerChartLimit: 2, + }) + suite.NotNil(server) + suite.Nil(err, "no error creating per-chart-limit server") + suite.PerChartLimitServer = server + router = cm_router.NewRouter(cm_router.RouterOptions{ Logger: logger, Depth: 0, @@ -777,6 +808,38 @@ func (suite *MultiTenantServerTestSuite) TestMaxObjectsServer() { suite.Equal(507, res.Status(), "507 POST /api/prov") } +func (suite *MultiTenantServerTestSuite) TestPerChartLimit() { + ns := "per-chart-limit" + content, err := ioutil.ReadFile(testTarballPathV0) + suite.Nil(err, "no error opening test tarball") + body := bytes.NewBuffer(content) + res := suite.doRequest(ns, "POST", "/api/charts", body, "") + suite.Equal(201, res.Status(), "201 POST /api/charts") + + content, err = ioutil.ReadFile(testTarballPathV2) + suite.Nil(err, "no error opening test tarball") + body = bytes.NewBuffer(content) + res = suite.doRequest(ns, "POST", "/api/charts", body, "") + suite.Equal(201, res.Status(), "201 POST /api/charts") + + content, err = ioutil.ReadFile(testTarballPath) + suite.Nil(err, "no error opening test tarball") + body = bytes.NewBuffer(content) + res = suite.doRequest(ns, "POST", "/api/charts", body, "") + suite.Equal(201, res.Status(), "201 POST /api/charts") + + time.Sleep(time.Second) + + res = suite.doRequest(ns, "GET", "/api/charts/mychart/0.2.0", nil, "") + suite.Equal(200, res.Status(), "200 GET /api/charts/mychart-0.2.0") + + res = suite.doRequest(ns, "GET", "/api/charts/mychart/0.1.0", nil, "") + suite.Equal(200, res.Status(), "200 GET /api/charts/mychart-0.1.0") + + res = suite.doRequest(ns, "GET", "/api/charts/mychart/0.0.1", nil, "") + suite.Equal(404, res.Status(), "200 GET /api/charts/mychart-0.0.1") +} + func (suite *MultiTenantServerTestSuite) TestMaxUploadSizeServer() { // trigger 413s, "request too large" content, err := ioutil.ReadFile(testTarballPath) diff --git a/pkg/config/vars.go b/pkg/config/vars.go index 6485312e..f6c6a589 100644 --- a/pkg/config/vars.go +++ b/pkg/config/vars.go @@ -775,6 +775,15 @@ var configVars = map[string]configVar{ EnvVar: "LISTEN_HOST", }, }, + "per-chart-limit": { + Type: intType, + Default: 0, + CLIFlag: cli.IntFlag{ + Name: "per-chart-limit", + Usage: "limits the museum server stores the max N versions per chart", + EnvVar: "PER_CHART_LIMIT", + }, + }, } func populateCLIFlags() { diff --git a/scripts/setup-test-environment.sh b/scripts/setup-test-environment.sh index 0509f865..a09e4254 100755 --- a/scripts/setup-test-environment.sh +++ b/scripts/setup-test-environment.sh @@ -49,6 +49,8 @@ package_test_charts() { done # add another version to repo for metric tests helm package --sign --key helm-test --keyring ../pgp/helm-test-key.secret --version 0.2.0 -d mychart/ mychart/. + # add another version for per chart limit test + helm package --sign --key helm-test --keyring ../pgp/helm-test-key.secret --version 0.0.1 -d mychart/ mychart/. popd pushd testdata/badcharts/ @@ -57,6 +59,7 @@ package_test_charts() { helm package . popd done + popd } main