cmd/chartmuseum,pkg/chartmuseum,pkg/config: add new per-chart-limit-o…

…ption , impls #316

Signed-off-by: scnace <[email protected]>

cmd/chartmuseum/main.go

@@ -18,13 +18,14 @@ package main
 
 import (
 	"fmt"
+	"log"
+	"os"
+	"strings"
+
 	"github.com/chartmuseum/storage"
 	"helm.sh/chartmuseum/pkg/cache"
 	"helm.sh/chartmuseum/pkg/chartmuseum"
 	"helm.sh/chartmuseum/pkg/config"
-	"log"
-	"os"
-	"strings"
 
 	"github.com/urfave/cli"
 )
@@ -102,6 +103,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)

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) {
 		EnforceSemver2:         options.EnforceSemver2,
 		Version:                options.Version,
 		CacheInterval:          options.CacheInterval,
+		PerChartLimit:          options.PerChartLimit,
 	})
 
 	return server, err

pkg/chartmuseum/server/multitenant/api.go

@@ -21,12 +21,15 @@ import (
 	"net/http"
 	pathutil "path/filepath"
 	"sort"
+	"strings"
 
 	"github.com/Masterminds/semver/v3"
 	"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"
 )
 
@@ -129,7 +132,7 @@ func (server *MultiTenantServer) uploadChartPackage(log cm_logger.LoggingFn, rep
 		if err != nil {
 			return filename, &HTTPError{http.StatusBadRequest, err.Error()}
 		}
-		if _, err := semver.StrictNewVersion(version.Metadata.Version); err != nil {
+		if _, err := semver.StrictNewVersion(version.Version); err != nil {
 			return filename, &HTTPError{http.StatusBadRequest, fmt.Errorf("semver2 validation: %w", err).Error()}
 		}
 	}
@@ -144,8 +147,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 {
@@ -213,3 +215,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
+}

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())

pkg/chartmuseum/server/multitenant/server_test.go

@@ -44,12 +44,13 @@ 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"
 var badTestTarballPath = "../../../../testdata/badcharts/mybadchart/mybadchart-1.0.0.tgz"
 var badTestProvfilePath = "../../../../testdata/badcharts/mybadchart/mybadchart-1.0.0.tgz.prov"
-var badTestSemver2Path = "../../../../testdata/badcharts/mybadsemver2chart/mybadsemver2chart-0.x.x.tgz"
+var badTestSemver2Path = "../../../../testdata/badcharts/mybadsemver2chart/mybadsemver2chart-a-bad-semver-version.tgz"
 
 type MultiTenantServerTestSuite struct {
 	suite.Suite
@@ -65,6 +66,7 @@ type MultiTenantServerTestSuite struct {
 	MaxObjectsServer     *MultiTenantServer
 	MaxUploadSizeServer  *MultiTenantServer
 	Semver2Server        *MultiTenantServer
+	PerChartLimitServer  *MultiTenantServer
 	TempDirectory        string
 	TestTarballFilename  string
 	TestProvfileFilename string
@@ -110,6 +112,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
@@ -345,6 +349,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,
@@ -359,6 +369,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,
@@ -780,6 +811,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)

pkg/config/vars.go

@@ -766,6 +766,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() {

scripts/setup-test-environment.sh

@@ -44,6 +44,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/
@@ -56,6 +58,7 @@ package_test_charts() {
         fi
         popd
     done
+    popd
 }
 
 main

testdata/badcharts/mybadsemver2chart/Chart.yaml

@@ -1,4 +1,4 @@
 apiVersion: v1
 name: mybadsemver2chart
-version: 0.x.x
+version: a-bad-semver-version