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

chartmuseum: add semver2 validation #322

Merged
merged 1 commit into from
Apr 15, 2020
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 @@ -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)
Expand Down
1 change: 1 addition & 0 deletions cmd/chartmuseum/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions pkg/chartmuseum/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
21 changes: 19 additions & 2 deletions pkg/chartmuseum/server/multitenant/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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
Expand Down Expand Up @@ -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()}
Expand Down
3 changes: 3 additions & 0 deletions pkg/chartmuseum/server/multitenant/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ type (
APIEnabled bool
DisableDelete bool
UseStatefiles bool
EnforceSemver2 bool
ChartURL string
ChartPostFormFieldName string
ProvPostFormFieldName string
Expand All @@ -84,6 +85,7 @@ type (
EnableAPI bool
DisableDelete bool
UseStatefiles bool
EnforceSemver2 bool
}

tenantInternals struct {
Expand Down Expand Up @@ -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{},
Expand Down
34 changes: 34 additions & 0 deletions pkg/chartmuseum/server/multitenant/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -63,6 +64,7 @@ type MultiTenantServerTestSuite struct {
ChartURLServer *MultiTenantServer
MaxObjectsServer *MultiTenantServer
MaxUploadSizeServer *MultiTenantServer
Semver2Server *MultiTenantServer
TempDirectory string
TestTarballFilename string
TestProvfileFilename string
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -641,6 +659,14 @@ func (suite *MultiTenantServerTestSuite) TestBadChartUpload() {
suite.Equal(400, res.Status(), "400 POST /api/charts")
}

func (suite *MultiTenantServerTestSuite) TestSemver2Validation() {
scbizu marked this conversation as resolved.
Show resolved Hide resolved
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, "")
Expand Down Expand Up @@ -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")
Expand Down
9 changes: 9 additions & 0 deletions pkg/config/vars.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
2 changes: 2 additions & 0 deletions testdata/badsemver2chart/mybadchart/.helmignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
*.tgz
*.prov
4 changes: 4 additions & 0 deletions testdata/badsemver2chart/mybadchart/Chart.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
apiVersion: v1
name: semver_charts
version: 0.x.x

9 changes: 9 additions & 0 deletions testdata/badsemver2chart/mybadchart/templates/pod.yaml
Original file line number Diff line number Diff line change
@@ -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']