Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add per chart limit option #466

Merged
merged 2 commits into from
Jan 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cmd/chartmuseum/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions pkg/chartmuseum/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand Down
80 changes: 78 additions & 2 deletions pkg/chartmuseum/server/multitenant/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
20 changes: 17 additions & 3 deletions pkg/chartmuseum/server/multitenant/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
}
Expand Down Expand Up @@ -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,
Expand All @@ -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())
Expand Down
63 changes: 63 additions & 0 deletions pkg/chartmuseum/server/multitenant/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -64,6 +65,7 @@ type MultiTenantServerTestSuite struct {
MaxObjectsServer *MultiTenantServer
MaxUploadSizeServer *MultiTenantServer
Semver2Server *MultiTenantServer
PerChartLimitServer *MultiTenantServer
TempDirectory string
TestTarballFilename string
TestProvfileFilename string
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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)
Expand Down
9 changes: 9 additions & 0 deletions pkg/config/vars.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
3 changes: 3 additions & 0 deletions scripts/setup-test-environment.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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/
Expand All @@ -57,6 +59,7 @@ package_test_charts() {
helm package .
popd
done
popd
}

main