pkg/repo: inject chart metadata(name/version) into storage object (#362)

* pkg/repo,pkg/chartmuseum: use object metadata to correctly get the chart version

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

* pkg/chartmuseum/server: log error rather than panic if mutitant reponame not found

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

* mod: bump storage to v0.10.0

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

acceptance_tests/helm.robot

@@ -59,6 +59,7 @@ Test Helm integration
     Able to fetch and verify test charts
     Delete test charts from ChartMuseum
     Able to update ChartMuseum repo
+    Ensure test charts deleted
     Helm search does not return test charts
     Unable to fetch and verify test charts
 
@@ -82,6 +83,9 @@ Upload bad provenance files to ChartMuseum
 Delete test charts from ChartMuseum
     ChartMuseum.delete test charts
 
+Ensure test charts deleted
+    ChartMuseum.ensure charts deleted
+
 Able to add ChartMuseum as Helm chart repo
     Helm.add chart repo
     Helm.return code should be  0
@@ -98,6 +102,8 @@ Helm search returns test charts
 Helm search does not return test charts
     Helm.search for chart  mychart
     Helm.output does not contain  mychart
+    Helm.search for chart  mychart2
+    Helm.output does not contain  mychart2
 
 Able to fetch and verify test charts
     Helm.fetch and verify chart  mychart

acceptance_tests/lib/ChartMuseum.py

@@ -151,6 +151,7 @@ def delete_test_charts(self):
         testcharts_dir = os.path.join(self.rootdir, common.TESTCHARTS_DIR)
         os.chdir(testcharts_dir)
         for d in os.listdir('.'):
+            # delete all charts inside /mychart (also includes mychart2)
             if not os.path.isdir(d):
                 continue
             os.chdir(d)
@@ -167,3 +168,22 @@ def delete_test_charts(self):
                     print(('HTTP CONTENT: %s' % response.content))
                     self.http_status_code_should_be(200, response.status_code)
             os.chdir('../')
+
+    def ensure_charts_deleted(self):
+        endpoint = '%s/api/charts' % common.HELM_REPO_URL
+        testcharts_dir = os.path.join(self.rootdir, common.TESTCHARTS_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:
+                tmp = tgz[:-4].rsplit('-', 1)
+                name = tmp[0]
+                version = tmp[1]
+                with open(tgz):
+                    epoint = '%s/%s/%s' % (endpoint, name, version)
+                    response = requests.get(url=epoint)
+                    self.http_status_code_should_be(404, response.status_code)
+            os.chdir('../')

go.mod

@@ -5,14 +5,15 @@ go 1.15
 replace (
 	github.com/NetEase-Object-Storage/nos-golang-sdk => github.com/karuppiah7890/nos-golang-sdk v0.0.0-20191116042345-0792ba35abcc
 	go.etcd.io/etcd => github.com/eddycjy/etcd v0.5.0-alpha.5.0.20200218102753-4258cdd2efdf
+	google.golang.org/grpc => google.golang.org/grpc v1.29.1
 )
 
 require (
 	github.com/Masterminds/semver/v3 v3.1.0
 	github.com/alicebob/gopher-json v0.0.0-20200520072559-a9ecdc9d1d3a // indirect
 	github.com/alicebob/miniredis v2.5.0+incompatible
 	github.com/chartmuseum/auth v0.4.2
-	github.com/chartmuseum/storage v0.9.1
+	github.com/chartmuseum/storage v0.10.0
 	github.com/ghodss/yaml v1.0.0
 	github.com/gin-contrib/size v0.0.0-20200815104238-dc717522c4e2
 	github.com/gin-gonic/gin v1.6.3

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)
 		}
 	}
@@ -506,7 +513,11 @@ func (server *MultiTenantServer) startEventListener() {
 		}
 		index := entry.RepoIndex
 
-		tenant := server.Tenants[e.RepoName]
+		tenant, ok := server.Tenants[e.RepoName]
+		if !ok {
+			log(cm_logger.ErrorLevel, "Error find tenants repo name", zap.Error(err), zap.String("repo", repo))
+			continue
+		}
 		tenant.RegenerationLock.Lock()
 
 		if e.ChartVersion == nil {

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