From 73e75ce517bbdff135d68dcfb682eb3b94736601 Mon Sep 17 00:00:00 2001 From: Nace Sc Date: Mon, 18 Sep 2023 20:43:22 -0500 Subject: [PATCH] pkg/repo,pkg/chartmuseum/server: per-limit-chart chart expiration removes prefix mathching for chartpath preparing (#721) Fix #714 Signed-off-by: scbizu --- pkg/chartmuseum/server/multitenant/api.go | 6 +- .../server/multitenant/server_test.go | 58 ++++++++++--------- pkg/repo/chart.go | 28 +++++---- scripts/setup-test-environment.sh | 2 + testdata/charts/mychart-service/.helmignore | 2 + testdata/charts/mychart-service/Chart.yaml | 2 + .../charts/mychart-service/templates/pod.yaml | 9 +++ testdata/charts/mychart-service/values.yaml | 0 8 files changed, 66 insertions(+), 41 deletions(-) create mode 100644 testdata/charts/mychart-service/.helmignore create mode 100644 testdata/charts/mychart-service/Chart.yaml create mode 100644 testdata/charts/mychart-service/templates/pod.yaml create mode 100644 testdata/charts/mychart-service/values.yaml diff --git a/pkg/chartmuseum/server/multitenant/api.go b/pkg/chartmuseum/server/multitenant/api.go index a33c1a3c..ac3f667a 100644 --- a/pkg/chartmuseum/server/multitenant/api.go +++ b/pkg/chartmuseum/server/multitenant/api.go @@ -230,7 +230,8 @@ func extractFromChart(content []byte) (name string, version string, err error) { } func (server *MultiTenantServer) PutWithLimit(ctx *gin.Context, log cm_logger.LoggingFn, repo string, - filename string, content []byte) error { + 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) @@ -251,7 +252,8 @@ func (server *MultiTenantServer) PutWithLimit(ctx *gin.Context, log cm_logger.Lo } var newObjs []storage.Object for _, obj := range objs { - if !strings.HasPrefix(obj.Path, name) || strings.HasSuffix(obj.Path, ".prov") { + n, _ := cm_repo.GetExactChartNameVersion(obj.Path) + if strings.Compare(n, name) != 0 || strings.HasSuffix(obj.Path, ".prov") { continue } log(cm_logger.DebugLevel, "PutWithLimit", "current object name", obj.Path) diff --git a/pkg/chartmuseum/server/multitenant/server_test.go b/pkg/chartmuseum/server/multitenant/server_test.go index ae632c40..df713f0c 100644 --- a/pkg/chartmuseum/server/multitenant/server_test.go +++ b/pkg/chartmuseum/server/multitenant/server_test.go @@ -42,14 +42,17 @@ import ( 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 ( + testTarballPath = "../../../../testdata/charts/mychart/mychart-0.1.0.tgz" + testTarballPathV2 = "../../../../testdata/charts/mychart/mychart-0.2.0.tgz" + testTarballPathV0 = "../../../../testdata/charts/mychart/mychart-0.0.1.tgz" + testServiceTarballPathV0 = "../../../../testdata/charts/mychart-service/mychart-service-0.0.1.tgz" + testProvfilePath = "../../../../testdata/charts/mychart/mychart-0.1.0.tgz.prov" + otherTestTarballPath = "../../../../testdata/charts/otherchart/otherchart-0.1.0.tgz" + otherTestProvfilePath = "../../../../testdata/charts/otherchart/otherchart-0.1.0.tgz.prov" + badTestTarballPath = "../../../../testdata/badcharts/mybadchart/mybadchart-1.0.0.tgz" + badTestProvfilePath = "../../../../testdata/badcharts/mybadchart/mybadchart-1.0.0.tgz.prov" +) type MultiTenantServerTestSuite struct { suite.Suite @@ -676,7 +679,7 @@ entries: - charts/acs-engine-autoscaler-2.1.2.tgz version: 2.1.2 generated: "2018-05-23T15:14:46-05:00"`) - err = os.WriteFile(indexCacheFilePath, content, 0644) + err = os.WriteFile(indexCacheFilePath, content, 0o644) suite.Nil(err, "no error creating test index-cache.yaml") defer os.Remove(indexCacheFilePath) @@ -699,7 +702,7 @@ generated: "2018-05-23T15:14:46-05:00"`) // invalid, unparsable index-cache.yaml indexCacheFilePath = pathutil.Join(suite.TempDirectory, repo.StatefileFilename) content = []byte(`is this valid yaml? maybe. but its definitely not a valid index.yaml!`) - err = os.WriteFile(indexCacheFilePath, content, 0644) + err = os.WriteFile(indexCacheFilePath, content, 0o644) suite.Nil(err, "no error creating test index-cache.yaml") NewMultiTenantServer(MultiTenantServerOptions{ @@ -917,27 +920,25 @@ func (suite *MultiTenantServerTestSuite) TestMaxObjectsServer() { func (suite *MultiTenantServerTestSuite) TestPerChartLimit() { ns := "per-chart-limit" - content, err := os.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 = os.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") + expectUploadFiles := []string{ + testServiceTarballPathV0, + testTarballPathV0, + testTarballPathV2, + testTarballPath, + } - content, err = os.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") + for _, f := range expectUploadFiles { + content, err := os.ReadFile(f) + 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, "") + 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, "") @@ -945,6 +946,9 @@ func (suite *MultiTenantServerTestSuite) TestPerChartLimit() { 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") + + res = suite.doRequest(ns, "GET", "/api/charts/mychart-service/0.0.1", nil, "") + suite.Equal(200, res.Status(), "200 GET /api/charts/mychart-service-0.0.1") } func (suite *MultiTenantServerTestSuite) TestMaxUploadSizeServer() { @@ -967,7 +971,6 @@ func (suite *MultiTenantServerTestSuite) TestMaxUploadSizeServer() { } func (suite *MultiTenantServerTestSuite) TestMetrics() { - apiPrefix := pathutil.Join("/api", "a") content, err := os.ReadFile(testTarballPath) @@ -1300,7 +1303,6 @@ func (suite *MultiTenantServerTestSuite) testAllRoutes(repo string, depth int) { buf, w = suite.getBodyWithMultipartFormFiles([]string{"prov"}, []string{testTarballPath}) res = suite.doRequest(stype, "POST", fmt.Sprintf("%s/charts", apiPrefix), buf, w.FormDataContentType()) suite.Equal(400, res.Status(), fmt.Sprintf("400 POST %s/charts", apiPrefix)) - } func (suite *MultiTenantServerTestSuite) getBodyWithMultipartFormFiles(fields []string, filenames []string) (io.Reader, *multipart.Writer) { diff --git a/pkg/repo/chart.go b/pkg/repo/chart.go index c4b351a2..5a14ad24 100644 --- a/pkg/repo/chart.go +++ b/pkg/repo/chart.go @@ -113,23 +113,29 @@ func chartFromContent(content []byte) (*helm_chart.Chart, error) { func emptyChartVersionFromPackageFilename(filename string) *helm_repo.ChartVersion { noExt := strings.TrimSuffix(pathutil.Base(filename), fmt.Sprintf(".%s", ChartPackageFileExtension)) - parts := strings.Split(noExt, "-") + + name, version := GetExactChartNameVersion(noExt) + + metadata := &helm_chart.Metadata{Name: name, Version: version} + return &helm_repo.ChartVersion{Metadata: metadata} +} + +func GetExactChartNameVersion(path string) (string, string) { + parts := strings.Split(path, "-") lastIndex := len(parts) - 1 - name := parts[0] - version := "" + chartName := parts[0] + chartVersion := "" for idx := lastIndex; idx >= 1; idx-- { if _, err := strconv.Atoi(string(parts[idx][0])); err == nil { // see if this part looks like a version (starts w int) - version = strings.Join(parts[idx:], "-") - name = strings.Join(parts[:idx], "-") + chartVersion = strings.Join(parts[idx:], "-") + chartName = strings.Join(parts[:idx], "-") break } } - if version == "" { // no parts looked like a real version, just take everything after last hyphen - name = strings.Join(parts[:lastIndex], "-") - version = parts[lastIndex] + if chartVersion == "" { // no parts looked like a real version, just take everything after last hyphen + chartName = strings.Join(parts[:lastIndex], "-") + chartVersion = parts[lastIndex] } - - metadata := &helm_chart.Metadata{Name: name, Version: version} - return &helm_repo.ChartVersion{Metadata: metadata} + return chartName, chartVersion } diff --git a/scripts/setup-test-environment.sh b/scripts/setup-test-environment.sh index 88d20b28..03e73039 100755 --- a/scripts/setup-test-environment.sh +++ b/scripts/setup-test-environment.sh @@ -51,6 +51,8 @@ package_test_charts() { 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/. + + helm package --sign --key helm-test --keyring ../pgp/helm-test-key.secret --version 0.0.1 -d mychart-service/ mychart-service/. popd pushd testdata/badcharts/ diff --git a/testdata/charts/mychart-service/.helmignore b/testdata/charts/mychart-service/.helmignore new file mode 100644 index 00000000..9b28b351 --- /dev/null +++ b/testdata/charts/mychart-service/.helmignore @@ -0,0 +1,2 @@ +*.tgz +*.prov \ No newline at end of file diff --git a/testdata/charts/mychart-service/Chart.yaml b/testdata/charts/mychart-service/Chart.yaml new file mode 100644 index 00000000..697e23ee --- /dev/null +++ b/testdata/charts/mychart-service/Chart.yaml @@ -0,0 +1,2 @@ +name: mychart-service +version: 0.1.0 diff --git a/testdata/charts/mychart-service/templates/pod.yaml b/testdata/charts/mychart-service/templates/pod.yaml new file mode 100644 index 00000000..ad4aaaf3 --- /dev/null +++ b/testdata/charts/mychart-service/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 diff --git a/testdata/charts/mychart-service/values.yaml b/testdata/charts/mychart-service/values.yaml new file mode 100644 index 00000000..e69de29b