Skip to content

Commit

Permalink
pkg/repo,pkg/chartmuseum/server: per-limit-chart chart expiration rem…
Browse files Browse the repository at this point in the history
…oves prefix mathching for chartpath preparing

Fix #714

Signed-off-by: scbizu <[email protected]>
  • Loading branch information
scbizu committed Sep 17, 2023
1 parent 26e2e63 commit 4b4168f
Show file tree
Hide file tree
Showing 8 changed files with 66 additions and 41 deletions.
6 changes: 4 additions & 2 deletions pkg/chartmuseum/server/multitenant/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
58 changes: 30 additions & 28 deletions pkg/chartmuseum/server/multitenant/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)

Expand All @@ -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{
Expand Down Expand Up @@ -917,34 +920,35 @@ 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, "")
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")

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() {
Expand All @@ -967,7 +971,6 @@ func (suite *MultiTenantServerTestSuite) TestMaxUploadSizeServer() {
}

func (suite *MultiTenantServerTestSuite) TestMetrics() {

apiPrefix := pathutil.Join("/api", "a")

content, err := os.ReadFile(testTarballPath)
Expand Down Expand Up @@ -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) {
Expand Down
28 changes: 17 additions & 11 deletions pkg/repo/chart.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
2 changes: 2 additions & 0 deletions scripts/setup-test-environment.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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/
Expand Down
2 changes: 2 additions & 0 deletions testdata/charts/mychart-service/.helmignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
*.tgz
*.prov
2 changes: 2 additions & 0 deletions testdata/charts/mychart-service/Chart.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
name: mychart-service
version: 0.1.0
9 changes: 9 additions & 0 deletions testdata/charts/mychart-service/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']
Empty file.

0 comments on commit 4b4168f

Please sign in to comment.