pkg/repo,pkg/chartmuseum: use object metadata to correctly get the ch…

…art version

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

acceptance_tests/helm.robot

@@ -53,11 +53,13 @@ Test Helm integration
     Upload test charts to ChartMuseum
     Upload provenance files to ChartMuseum
     Upload bad test charts to ChartMuseum
+    Upload test customized version charts to ChartMuseum
     Upload bad provenance files to ChartMuseum
     Able to update ChartMuseum repo
     Helm search returns test charts
     Able to fetch and verify test charts
     Delete test charts from ChartMuseum
+    Ensure delete customized version chart
     Able to update ChartMuseum repo
     Helm search does not return test charts
     Unable to fetch and verify test charts
@@ -76,12 +78,18 @@ Upload provenance files to ChartMuseum
 Upload bad test charts to ChartMuseum
     ChartMuseum.upload bad test charts
 
+Upload test customized version charts to ChartMuseum
+    ChartMuseum.upload test more version charts
+
 Upload bad provenance files to ChartMuseum
     ChartMuseum.upload bad provenance files
 
 Delete test charts from ChartMuseum
     ChartMuseum.delete test charts
 
+Ensure delete customized version chart
+    ChartMuseum.delete test version charts
+
 Able to add ChartMuseum as Helm chart repo
     Helm.add chart repo
     Helm.return code should be  0

acceptance_tests/lib/ChartMuseum.py

@@ -89,6 +89,25 @@ def upload_test_charts(self):
                     self.http_status_code_should_be(201, response.status_code)
             os.chdir('../')
 
+    def upload_test_more_version_charts(self):
+        charts_endpoint = '%s/api/charts' % common.HELM_REPO_URL
+        testcharts_dir = os.path.join(self.rootdir, common.TESTMOREVERSIONCHARTS_DIR)
+        os.chdir(testcharts_dir)
+        for d in os.listdir('.'):
+            if not os.path.isdir(d):
+                continue
+            os.chdir(d)
+            tgzs = glob.glob('*.tgz')
+            for tgz in tgzs:
+                print(('Uploading more version chart package "%s"' % tgz))
+                with open(tgz, 'rb') as f:
+                    response = requests.post(url=charts_endpoint, data=f.read())
+                    print(('POST %s' % charts_endpoint))
+                    print(('HTTP STATUS: %s' % response.status_code))
+                    print(('HTTP CONTENT: %s' % response.content))
+                    self.http_status_code_should_be(201, response.status_code)
+            os.chdir('../')
+
     def upload_bad_test_charts(self):
         charts_endpoint = '%s/api/charts' % common.HELM_REPO_URL
         testcharts_dir = os.path.join(self.rootdir, common.TESTBADCHARTS_DIR)
@@ -167,3 +186,32 @@ def delete_test_charts(self):
                     print(('HTTP CONTENT: %s' % response.content))
                     self.http_status_code_should_be(200, response.status_code)
             os.chdir('../')
+
+    def delete_test_version_charts(self):
+        endpoint = '%s/api/charts' % common.HELM_REPO_URL
+        testcharts_dir = os.path.join(self.rootdir, common.TESTMOREVERSIONCHARTS_DIR)
+        os.chdir(testcharts_dir)
+        for d in os.listdir('.'):
+            if not os.path.isdir(d):
+                continue
+            os.chdir(d)
+            f = open("Chart.yaml","r")
+            name = ''
+            version = ''
+            for lines in f:
+                v_prefix = 'version: '
+                name_prefix = 'name: '
+                if lines.startswith(v_prefix):
+                    version = lines[len(v_prefix):]
+                elif lines.startswith(name_prefix):
+                    name = lines[len(name_prefix):]
+            print(('Delete test chart "%s-%s"' % (name, version)))
+            epoint = '%s/%s/%s' % (endpoint, name, version)
+            response = requests.delete(url=epoint)
+            print(('HTTP STATUS: %s' % response.status_code))
+            print(('HTTP CONTENT: %s' % response.content))
+            self.http_status_code_should_be(200, response.status_code)
+            # checks if the chart still exists
+            response = requests.get(url=epoint)
+            self.http_status_code_should_be(404, response.status_code)
+            os.chdir('../')

acceptance_tests/lib/common.py

@@ -8,6 +8,7 @@
 HELM_REPO_URL = 'http://localhost:%d' % PORT
 TESTCHARTS_DIR = 'testdata/charts'
 TESTBADCHARTS_DIR = 'testdata/badcharts'
+TESTMOREVERSIONCHARTS_DIR = 'testdata/charts/mychart2'
 ACCEPTANCE_DIR = '.acceptance/'
 STORAGE_DIR = os.path.join(ACCEPTANCE_DIR, 'storage/')
 KEYRING = 'testdata/pgp/helm-test-key.pub'

go.mod

@@ -7,6 +7,8 @@ replace (
 	go.etcd.io/etcd => github.com/eddycjy/etcd v0.5.0-alpha.5.0.20200218102753-4258cdd2efdf
 )
 
+replace github.com/chartmuseum/storage => github.com/chartmuseum/storage v0.9.2-0.20201014231951-6dd385da21c3
+
 require (
 	github.com/Masterminds/semver/v3 v3.1.0
 	github.com/alicebob/gopher-json v0.0.0-20200520072559-a9ecdc9d1d3a // indirect

go.sum

@@ -150,8 +150,8 @@ github.com/cespare/xxhash/v2 v2.1.1/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XL
 github.com/chai2010/gettext-go v0.0.0-20160711120539-c6fed771bfd5/go.mod h1:/iP1qXHoty45bqomnu2LM+VVyAEdWN+vtSHGlQgyxbw=
 github.com/chartmuseum/auth v0.4.2 h1:Ch7sh2BpoYLN20zc6U6NQyB3JPEdBJ0quWH6+NQHcQ0=
 github.com/chartmuseum/auth v0.4.2/go.mod h1:nO+nNadtlcIQrnVdL2QxI+XYoe/4sd8kypYT956miQM=
-github.com/chartmuseum/storage v0.9.1 h1:RgnG4jC6Xhvw0sMS8A+tA0pO5xg+NU0rNz/X+iPEG/8=
-github.com/chartmuseum/storage v0.9.1/go.mod h1:rEOwJVPHU569BC22tUhl6g64sYPxFzZKrxxzjz+zvWQ=
+github.com/chartmuseum/storage v0.9.2-0.20201014231951-6dd385da21c3 h1:+CGbrnh/pOLmE50kU5KIjzwSMNTcXAAXvVVpYW7/1bk=
+github.com/chartmuseum/storage v0.9.2-0.20201014231951-6dd385da21c3/go.mod h1:rEOwJVPHU569BC22tUhl6g64sYPxFzZKrxxzjz+zvWQ=
 github.com/chzyer/logex v1.1.10/go.mod h1:+Ywpsq7O8HXn0nuIou7OrIPyXbp3wmkHB+jjWRnGsAI=
 github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e/go.mod h1:nSuG5e5PlCu98SY8svDHJxuZscDgtXS6KTTbou5AhLI=
 github.com/chzyer/test v0.0.0-20180213035817-a1ea475d72b1/go.mod h1:Q3SI9o4m/ZMnBNeIyt5eFwwo7qiLfzFZmjNmxjkiQlU=

pkg/chartmuseum/server/multitenant/cache.go

@@ -36,11 +36,13 @@ import (
 	"context"
 	"encoding/json"
 	"errors"
-	"go.uber.org/zap"
+	"fmt"
 	pathutil "path"
 	"sync"
 	"time"
 
+	"go.uber.org/zap"
+
 	cm_logger "helm.sh/chartmuseum/pkg/chartmuseum/logger"
 	cm_repo "helm.sh/chartmuseum/pkg/repo"
 
@@ -201,6 +203,11 @@ func (server *MultiTenantServer) fetchChartsInStorage(log cm_logger.LoggingFn, r
 	filteredObjects := []cm_storage.Object{}
 	for _, object := range allObjects {
 		if object.HasExtension(cm_repo.ChartPackageFileExtension) {
+			// Since ListObject cannot fetch the content from file list
+			object, err = server.StorageBackend.GetObject(object.Path)
+			if err != nil {
+				return nil, fmt.Errorf("backend storage: chart not found: %q", err)
+			}
 			filteredObjects = append(filteredObjects, object)
 		}
 	}

pkg/repo/chart.go

@@ -61,6 +61,11 @@ func ChartPackageFilenameFromContent(content []byte) (string, error) {
 // ChartVersionFromStorageObject returns a chart version from a storage object
 func ChartVersionFromStorageObject(object storage.Object) (*helm_repo.ChartVersion, error) {
 	if len(object.Content) == 0 {
+		if object.Meta.Version != "" && object.Meta.Name != "" {
+			return &helm_repo.ChartVersion{
+				Metadata: &helm_chart.Metadata{Name: object.Meta.Name, Version: object.Meta.Version},
+			}, nil
+		}
 		chartVersion := emptyChartVersionFromPackageFilename(object.Path)
 		if chartVersion.Name == "" || chartVersion.Version == "" {
 			return nil, ErrorInvalidChartPackage
@@ -86,7 +91,13 @@ func ChartVersionFromStorageObject(object storage.Object) (*helm_repo.ChartVersi
 
 // StorageObjectFromChartVersion returns a storage object from a chart version (empty content)
 func StorageObjectFromChartVersion(chartVersion *helm_repo.ChartVersion) storage.Object {
+	meta := storage.Metadata{}
+	if chartVersion.Metadata != nil {
+		meta.Name = chartVersion.Name
+		meta.Version = chartVersion.Version
+	}
 	object := storage.Object{
+		Meta:         meta,
 		Path:         pathutil.Base(chartVersion.URLs[0]),
 		Content:      []byte{},
 		LastModified: chartVersion.Created,

pkg/repo/chart_test.go

@@ -23,6 +23,7 @@ import (
 
 	"github.com/chartmuseum/storage"
 	"github.com/stretchr/testify/suite"
+	"helm.sh/helm/v3/pkg/chart"
 	helm_repo "helm.sh/helm/v3/pkg/repo"
 )
 
@@ -83,6 +84,39 @@ func (suite *ChartTestSuite) TestChartVersionFromStorageObject() {
 	suite.Equal("mychart", chartVersion.Name, "chart name as expected")
 	suite.Equal("1.0.4-SNAPSHOT", chartVersion.Version, "chart version as expected")
 
+	snapshotObject2 := storage.Object{
+		Meta:         storage.Metadata{Name: "mychart", Version: "1.0.4-SNAPSHOT-1"},
+		Path:         "mychart-1.0.4-SNAPSHOT-1.tgz",
+		Content:      []byte{},
+		LastModified: time.Now(),
+	}
+	chartVersion, err = ChartVersionFromStorageObject(snapshotObject2)
+	suite.Nil(err)
+	suite.Equal("mychart", chartVersion.Name, "chart name as expected")
+	suite.Equal("1.0.4-SNAPSHOT-1", chartVersion.Version, "chart version as expected")
+
+	snapshotObject3 := storage.Object{
+		Meta:         storage.Metadata{Name: "mychart", Version: "1.0-SNAPSHOT-1"},
+		Path:         "mychart-1.0-SNAPSHOT-1.tgz",
+		Content:      []byte{},
+		LastModified: time.Now(),
+	}
+	chartVersion, err = ChartVersionFromStorageObject(snapshotObject3)
+	suite.Nil(err)
+	suite.Equal("mychart", chartVersion.Name, "chart name as expected")
+	suite.Equal("1.0-SNAPSHOT-1", chartVersion.Version, "chart version as expected")
+
+	snapshotObject4 := storage.Object{
+		Meta:         storage.Metadata{Name: "mychart", Version: "1-SNAPSHOT-1"},
+		Path:         "mychart-1-SNAPSHOT-1.tgz",
+		Content:      []byte{},
+		LastModified: time.Now(),
+	}
+	chartVersion, err = ChartVersionFromStorageObject(snapshotObject4)
+	suite.Nil(err)
+	suite.Equal("mychart", chartVersion.Name, "chart name as expected")
+	suite.Equal("1-SNAPSHOT-1", chartVersion.Version, "chart version as expected")
+
 	multiHyphenObject := storage.Object{
 		Path:         "my-long-hyphenated-chart-name-1.0.4.tgz",
 		Content:      []byte{},
@@ -140,12 +174,18 @@ func (suite *ChartTestSuite) TestChartVersionFromStorageObject() {
 func (suite *ChartTestSuite) TestStorageObjectFromChartVersion() {
 	now := time.Now()
 	chartVersion := &helm_repo.ChartVersion{
+		Metadata: &chart.Metadata{
+			Name:    "mychart",
+			Version: "0.1.0",
+		},
 		URLs:    []string{"charts/mychart-0.1.0.tgz"},
 		Created: now,
 	}
 	object := StorageObjectFromChartVersion(chartVersion)
 	suite.Equal(now, object.LastModified, "object last modified as expected")
 	suite.Equal("mychart-0.1.0.tgz", object.Path, "object path as expected")
+	suite.Equal("mychart", object.Meta.Name, "object chart name as expected")
+	suite.Equal("0.1.0", object.Meta.Version, "object chart version as expected")
 	suite.Equal([]byte{}, object.Content, "object content as expected")
 }
 

testdata/charts/mychart2/.helmignore

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

testdata/charts/mychart2/Chart.yaml

@@ -0,0 +1,2 @@
+name: mychart2
+version: 0.1.0-SNAPSHOT-1

testdata/charts/mychart2/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']