Repository: helm/chartmuseum

Issue:
Auto-purge, get rid of old chart versions
Add feature flags that enable auto-removal of old chart versions in storage based on various age / last used / version parameters

Discussion:
@cep21: I'm interested in this ticket.  It's something I could do out of band, but the missing piece is I don't think there's anything in chartmuseum that tracks last used for charts.  Does this data currently exist?  If we were to add it, where would it be stored?@scbizu: @cep21 Happy with your interests , maybe [this](https://github.com/chartmuseum/storage/blob/main/storage.go#L32) is what you find ? If you can help us with this feature , it will make a big sense :)@cep21: @scbizu I think "last read" makes more sense than "last modified", right?  You would want to remove charts people are no longer downloading, for example.@scbizu: @cep21 You're right ,  but the storage itself do not support the last read timestamp yet . It will be a huge PR if we add a new mechanism to the storage structure , we should handle every REQ from `helm pull`, and update the last read timestamp. 

I think this is why this issue still tag with `help wanted` XD@cep21: One idea is to store this information inside redis, for example.  Another is to use the backend storage itself to store this information and "sync" some kind of ledger every 60 seconds (for example).@scbizu: I prefer the second one , the AutoPurge should be provided as an interface function , and it can be differ from the real storage backends. The expiration duration should be configurable if users open the auto purge feature .@cep21: > and "sync" some kind of ledger every 60 seconds 

This could be tricky if people are running multiple chartmuseum instances for redundancy, since we'll have to merge ledgers@vadasambar: It'd be great if we could have more than one conditions to decide whether to delete the charts or not. E.g., Instead of "Delete all the charts older than 2 months", it would be better if we have "Delete all the charts older than 2 months matching a particular regex". This is because you might not want to delete release (e.g., `2.1.0`) charts but if you want to delete pre-release charts (e.g., `2.1.0-custom-fix` or `2.1.0-pr-3245`), you could use regex to match all the pre-release charts older than specified time period (Check #383 ). 

However, one thing that concerns me about regex is you'd want to test it out first to see which charts would be deleted with that particular regex to avoid deleting charts you didn't intend to delete. @scbizu: The thread is too long to track information now . And let me draw the conclusion till now , the key points list below:

* Purge according to the `last read`(or can be fallback to `last modified` ?)
* Should solve the multi-tenants problem (purge one tenant should not affect others) <- need further discussions 
* Purge supports some conditions (e.g.: regex rules)
* Optional: provide a flag such as `dry-run` , it logs the will-be-purged chart before purge the chart.

@cep21: Above looks right.  Bullet (3) is probably an enhancement off the core request: bullet (1).  Also the difference between `last read` and `last modified` is pretty huge in both use and ease-of-implementation.@vadasambar: >Optional: should provide a flag to determine whether users need to soft delete the chart. (soft delete here means not really purges the chart but logs the will-be-purged charts)

Nitpick: I think it'd be better to call it `dry-run` instead. `soft delete` makes me think that the chart would be archived or removed from the index but the data would still be there but what we want to show using `soft delete` (as far as I've understood) is what's going to be deleted if you run a delete operation. 

Everything else looks good to me @scbizu @scbizu: I will be self-assigned to draft one implementation this weekend since I think it will make a big sense for decreasing the pressure of index so that we can both decrease the latency of our APIs and save the disk size of chart storage .

Maybe it will be provides with `--per-chart-max-version`, it will keep the latest N charts as your configuration. However, since I do not know which chart is currently be used , we can add more stuffs (like stick some charts so that they will not be removed from storage) later .

(The dry-run option is already implemented inside our company maybe I can open source it later)

(off-topic: Our CI failed again because of too large index refreshing XD)@jasondamour: Where was this left off? I'm willing to try picking up the remaining work. We have 2.5k charts, and would like to purge as many as possible@scbizu: @jasondamour  This is already implemented , you can use the version in our HEAD and try the `-per-chart-max-version`  option to start chartmuseum .

Solution patch:
diff --git a/cmd/chartmuseum/main.go b/cmd/chartmuseum/main.go
index c890430..9716d5d 100644
--- a/cmd/chartmuseum/main.go
+++ b/cmd/chartmuseum/main.go
@@ -114,6 +114,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 92c7b2e..24f6b67 100644
--- a/pkg/chartmuseum/server.go
+++ b/pkg/chartmuseum/server.go
@@ -69,6 +69,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
 		// Deprecated: see https://github.com/helm/chartmuseum/issues/485 for more info
 		EnforceSemver2 bool
 		// Deprecated: Debug is no longer effective. ServerOptions now requires the Logger field to be set and configured with LoggerOptions accordingly.
@@ -135,6 +138,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 05af152..143fda9 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 8ee70f6..f093a4e 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
@@ -67,15 +66,21 @@ type (
 		TenantCacheKeyLock     *sync.Mutex
 		CacheInterval          time.Duration
 		EventChan              chan event
+		ChartLimits            *ObjectsPerChartLimit
 		// Deprecated: see https://github.com/helm/chartmuseum/issues/485 for more info
 		EnforceSemver2 bool
 	}
 
+	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 (
 		DisableDelete          bool
 		UseStatefiles          bool
 		CacheInterval          time.Duration
+		PerChartLimit          int
 		// Deprecated: see https://github.com/helm/chartmuseum/issues/485 for more info
 		EnforceSemver2 bool
 	}
@@ -119,6 +125,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,
@@ -143,6 +156,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/config/vars.go b/pkg/config/vars.go
index 6f71f9a..3af5b13 100644
--- a/pkg/config/vars.go
+++ b/pkg/config/vars.go
@@ -777,6 +777,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 0509f86..a09e425 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



Test patch:
diff --git a/pkg/chartmuseum/server/multitenant/server_test.go b/pkg/chartmuseum/server/multitenant/server_test.go
index 1865bfa..8cd2be3 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)