From 2ac8b5df65a11a4175d83eec0d85967216f8e648 Mon Sep 17 00:00:00 2001 From: scnace Date: Thu, 16 Apr 2020 06:26:39 +0800 Subject: [PATCH] chartmuseum: add semver2 validation (#322) Signed-off-by: scnace --- cmd/chartmuseum/main.go | 1 + cmd/chartmuseum/main_test.go | 1 + go.mod | 1 + pkg/chartmuseum/server.go | 4 +++ pkg/chartmuseum/server/multitenant/api.go | 21 ++++++++++-- pkg/chartmuseum/server/multitenant/server.go | 3 ++ .../server/multitenant/server_test.go | 34 +++++++++++++++++++ pkg/config/vars.go | 9 +++++ .../badsemver2chart/mybadchart/.helmignore | 2 ++ .../badsemver2chart/mybadchart/Chart.yaml | 4 +++ .../mybadchart/templates/pod.yaml | 9 +++++ 11 files changed, 87 insertions(+), 2 deletions(-) create mode 100644 testdata/badsemver2chart/mybadchart/.helmignore create mode 100644 testdata/badsemver2chart/mybadchart/Chart.yaml create mode 100644 testdata/badsemver2chart/mybadchart/templates/pod.yaml diff --git a/cmd/chartmuseum/main.go b/cmd/chartmuseum/main.go index e2d8ec04..f7d44034 100644 --- a/cmd/chartmuseum/main.go +++ b/cmd/chartmuseum/main.go @@ -98,6 +98,7 @@ func cliHandler(c *cli.Context) { CORSAllowOrigin: conf.GetString("cors.alloworigin"), WriteTimeout: conf.GetInt("writetimeout"), ReadTimeout: conf.GetInt("readtimeout"), + EnforceSemver2: conf.GetBool("enforce-semver2"), } server, err := newServer(options) diff --git a/cmd/chartmuseum/main_test.go b/cmd/chartmuseum/main_test.go index 6b738022..1e351098 100644 --- a/cmd/chartmuseum/main_test.go +++ b/cmd/chartmuseum/main_test.go @@ -114,6 +114,7 @@ func (suite *MainTestSuite) TestMain() { os.Args = []string{"chartmuseum", "--storage", "local", "--storage-local-rootdir", "../../.chartstorage", "--cache", "wallet"} suite.Panics(main, "bad cache") suite.Equal("Unsupported cache store: wallet", suite.LastCrashMessage, "crashes with bad cache") + } func TestMainTestSuite(t *testing.T) { diff --git a/go.mod b/go.mod index f5462810..69ada480 100644 --- a/go.mod +++ b/go.mod @@ -9,6 +9,7 @@ replace ( ) require ( + github.com/Masterminds/semver/v3 v3.0.3 github.com/alicebob/gopher-json v0.0.0-20180125190556-5a6b3ba71ee6 // indirect github.com/alicebob/miniredis v2.5.0+incompatible github.com/chartmuseum/auth v0.4.1 diff --git a/pkg/chartmuseum/server.go b/pkg/chartmuseum/server.go index 0a1bb2b8..5cc0c218 100644 --- a/pkg/chartmuseum/server.go +++ b/pkg/chartmuseum/server.go @@ -65,6 +65,9 @@ type ( CORSAllowOrigin string ReadTimeout int WriteTimeout int + // EnforceSemver2 represents if the museum server always accept the Chart with [valid semantic version 2](https://semver.org/) + // More refers to : https://github.com/helm/chartmuseum/issues/320 + EnforceSemver2 bool } // Server is a generic interface for web servers @@ -128,6 +131,7 @@ func NewServer(options ServerOptions) (Server, error) { UseStatefiles: options.UseStatefiles, AllowOverwrite: options.AllowOverwrite, AllowForceOverwrite: options.AllowForceOverwrite, + EnforceSemver2: options.EnforceSemver2, }) return server, err diff --git a/pkg/chartmuseum/server/multitenant/api.go b/pkg/chartmuseum/server/multitenant/api.go index d99042a9..61aa4188 100644 --- a/pkg/chartmuseum/server/multitenant/api.go +++ b/pkg/chartmuseum/server/multitenant/api.go @@ -21,6 +21,8 @@ import ( pathutil "path/filepath" "sort" + "github.com/Masterminds/semver/v3" + "github.com/chartmuseum/storage" cm_logger "helm.sh/chartmuseum/pkg/chartmuseum/logger" cm_repo "helm.sh/chartmuseum/pkg/repo" @@ -41,11 +43,11 @@ func (server *MultiTenantServer) getAllCharts(log cm_logger.LoggingFn, repo stri keys = append(keys, k) } sort.Strings(keys) - end := offset+limit + end := offset + limit if len(keys) < end { end = len(keys) } - for i:=offset; i < end ; i++ { + for i := offset; i < end; i++ { result[keys[i]] = indexFile.Entries[keys[i]] } return result, nil @@ -109,6 +111,21 @@ func (server *MultiTenantServer) uploadChartPackage(log cm_logger.LoggingFn, rep return &HTTPError{409, "file already exists"} } } + + if server.EnforceSemver2 { + version, err := cm_repo.ChartVersionFromStorageObject(storage.Object{ + Content: content, + // Since we only need content to check for the chart version + // left the others fields to be default + }) + if err != nil { + return &HTTPError{400, err.Error()} + } + if _, err := semver.StrictNewVersion(version.Metadata.Version); err != nil { + return &HTTPError{400, fmt.Errorf("semver2 validation: %w", err).Error()} + } + } + limitReached, err := server.checkStorageLimit(repo, filename, force) if err != nil { return &HTTPError{500, err.Error()} diff --git a/pkg/chartmuseum/server/multitenant/server.go b/pkg/chartmuseum/server/multitenant/server.go index c22fa064..741ba959 100644 --- a/pkg/chartmuseum/server/multitenant/server.go +++ b/pkg/chartmuseum/server/multitenant/server.go @@ -58,6 +58,7 @@ type ( APIEnabled bool DisableDelete bool UseStatefiles bool + EnforceSemver2 bool ChartURL string ChartPostFormFieldName string ProvPostFormFieldName string @@ -84,6 +85,7 @@ type ( EnableAPI bool DisableDelete bool UseStatefiles bool + EnforceSemver2 bool } tenantInternals struct { @@ -128,6 +130,7 @@ func NewMultiTenantServer(options MultiTenantServerOptions) (*MultiTenantServer, APIEnabled: options.EnableAPI, DisableDelete: options.DisableDelete, UseStatefiles: options.UseStatefiles, + EnforceSemver2: options.EnforceSemver2, Limiter: make(chan struct{}, options.IndexLimit), Tenants: map[string]*tenantInternals{}, TenantCacheKeyLock: &sync.Mutex{}, diff --git a/pkg/chartmuseum/server/multitenant/server_test.go b/pkg/chartmuseum/server/multitenant/server_test.go index 5599fa96..75952b6f 100644 --- a/pkg/chartmuseum/server/multitenant/server_test.go +++ b/pkg/chartmuseum/server/multitenant/server_test.go @@ -49,6 +49,7 @@ var otherTestTarballPath = "../../../../testdata/charts/otherchart/otherchart-0. 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/badsemver2chart/semver-charts-0.x.x.tgz" type MultiTenantServerTestSuite struct { suite.Suite @@ -63,6 +64,7 @@ type MultiTenantServerTestSuite struct { ChartURLServer *MultiTenantServer MaxObjectsServer *MultiTenantServer MaxUploadSizeServer *MultiTenantServer + Semver2Server *MultiTenantServer TempDirectory string TestTarballFilename string TestProvfileFilename string @@ -106,6 +108,8 @@ func (suite *MultiTenantServerTestSuite) doRequest(stype string, method string, suite.MaxObjectsServer.Router.HandleContext(c) case "maxuploadsize": suite.MaxUploadSizeServer.Router.HandleContext(c) + case "semver2": + suite.Semver2Server.Router.HandleContext(c) } return c.Writer @@ -416,6 +420,20 @@ func (suite *MultiTenantServerTestSuite) SetupSuite() { suite.NotNil(server) suite.Nil(err, "no error creating new max upload size server") suite.MaxUploadSizeServer = server + + server, err = NewMultiTenantServer(MultiTenantServerOptions{ + Logger: logger, + Router: router, + StorageBackend: backend, + TimestampTolerance: time.Duration(0), + EnableAPI: true, + AllowOverwrite: true, + ChartPostFormFieldName: "chart", + EnforceSemver2: true, + }) + suite.NotNil(server) + suite.Nil(err, "no error validating semantic version server") + suite.Semver2Server = server } func (suite *MultiTenantServerTestSuite) TearDownSuite() { @@ -641,6 +659,14 @@ func (suite *MultiTenantServerTestSuite) TestBadChartUpload() { suite.Equal(400, res.Status(), "400 POST /api/charts") } +func (suite *MultiTenantServerTestSuite) TestSemver2Validation() { + content, err := ioutil.ReadFile(badTestSemver2Path) + suite.Nil(err, "no error opening test path") + body := bytes.NewBuffer(content) + res := suite.doRequest("semver2", "POST", "/api/charts", body, "") + suite.Equal(400, res.Status(), "400 POST /api/charts bad semver validation") +} + func (suite *MultiTenantServerTestSuite) TestForceOverwriteServer() { // Clear test repo to allow uploading again res := suite.doRequest("forceoverwrite", "DELETE", "/api/charts/mychart/0.1.0", nil, "") @@ -936,6 +962,14 @@ func (suite *MultiTenantServerTestSuite) testAllRoutes(repo string, depth int) { res = suite.doRequest(stype, "POST", fmt.Sprintf("%s/charts?force", apiPrefix), body, "") suite.Equal(409, res.Status(), fmt.Sprintf("409 POST %s/charts?force", apiPrefix)) + // with bad semver but without enforce semver2 + content, err = ioutil.ReadFile(badTestSemver2Path) + suite.Nil(err, "no error opening bad semver2 path") + + body = bytes.NewBuffer(content) + res = suite.doRequest(stype, "POST", fmt.Sprintf("%s/charts", apiPrefix), body, "") + suite.Equal(201, res.Status(), fmt.Sprintf("201 POST %s/charts", apiPrefix)) + // POST /api/:repo/prov content, err = ioutil.ReadFile(testProvfilePath) suite.Nil(err, "no error opening test provenance file") diff --git a/pkg/config/vars.go b/pkg/config/vars.go index 2b30ec49..0e82b9fb 100644 --- a/pkg/config/vars.go +++ b/pkg/config/vars.go @@ -721,6 +721,15 @@ var configVars = map[string]configVar{ EnvVar: "CORS_ALLOW_ORIGIN", }, }, + "enforce-semver2": { + Type: boolType, + Default: false, + CLIFlag: cli.BoolFlag{ + Name: "enforce-semver2", + Usage: "enforce the chart museum server only accepts the valid chart version as Helm does", + EnvVar: "ENFORCE_SEMVER2", + }, + }, } func populateCLIFlags() { diff --git a/testdata/badsemver2chart/mybadchart/.helmignore b/testdata/badsemver2chart/mybadchart/.helmignore new file mode 100644 index 00000000..efe6e35c --- /dev/null +++ b/testdata/badsemver2chart/mybadchart/.helmignore @@ -0,0 +1,2 @@ +*.tgz +*.prov diff --git a/testdata/badsemver2chart/mybadchart/Chart.yaml b/testdata/badsemver2chart/mybadchart/Chart.yaml new file mode 100644 index 00000000..0e23457b --- /dev/null +++ b/testdata/badsemver2chart/mybadchart/Chart.yaml @@ -0,0 +1,4 @@ +apiVersion: v1 +name: semver_charts +version: 0.x.x + diff --git a/testdata/badsemver2chart/mybadchart/templates/pod.yaml b/testdata/badsemver2chart/mybadchart/templates/pod.yaml new file mode 100644 index 00000000..ad4aaaf3 --- /dev/null +++ b/testdata/badsemver2chart/mybadchart/templates/pod.yaml @@ -0,0 +1,9 @@ +apiVersion: v1 +kind: Pod +metadata: + name: '{{- printf "%s-%s" .Release.Name .Chart.Name | trunc 63 | trimSuffix "-" -}}' +spec: + containers: + - image: busybox + name: '{{ .Chart.Name }}' + command: ['/bin/sh', '-c', 'while true; do echo {{ .Release.Name }}; sleep 5; done'] \ No newline at end of file