chartmuseum: add semver2 validation (#322)

Signed-off-by: scnace <scbizu@gmail.com>

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)

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

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

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

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

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{},

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

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

testdata/badsemver2chart/mybadchart/.helmignore

@@ -0,0 +1,2 @@
+*.tgz
+*.prov

testdata/badsemver2chart/mybadchart/Chart.yaml

@@ -0,0 +1,4 @@
+apiVersion: v1
+name: semver_charts
+version: 0.x.x
+

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']