Fix for missing files in Helm 2 charts (#53)

* add failing test for requirements.yaml Signed-off-by: Josh Dolitsky <[email protected]> * fix for missing requirements.yaml in helm 2 Signed-off-by: Josh Dolitsky <[email protected]> * check based on chart apiversion vs helm exe Signed-off-by: Josh Dolitsky <[email protected]> * add vendored test chart dependency Signed-off-by: Josh Dolitsky <[email protected]>
chartmuseum · Dec 17, 2019 · 17794c3 · 17794c3
1 parent 2cbc00f
commit 17794c3
Show file tree

Hide file tree

Showing 12 changed files with 284 additions and 13 deletions.
diff --git a/.gitignore b/.gitignore
@@ -12,4 +12,5 @@ bin/
 dist/
 releases/
 testbin/
+testdata/**/tmp
 vendor/
diff --git a/acceptance_tests/02-chartmuseum.robot b/acceptance_tests/02-chartmuseum.robot
@@ -20,6 +20,16 @@ Test ChartMuseum integration
     set helm version    ${version}
     install helm plugin
     helm major version detected by plugin is  ${version}
+
+    use test chart built by same helm version
+    clear chartmuseum storage
+    Chart directory can be pushed to ChartMuseum
+    Chart directory can be pushed to ChartMuseum with custom version
+    Chart package can be pushed to ChartMuseum
+    Chart package can be pushed to ChartMuseum with custom version
+
+    use test chart built by opposite helm version
+    clear chartmuseum storage
     Chart directory can be pushed to ChartMuseum
     Chart directory can be pushed to ChartMuseum with custom version
     Chart package can be pushed to ChartMuseum
@@ -30,13 +40,17 @@ Chart directory can be pushed to ChartMuseum
     push chart directory
     HelmPush.return code should be   0
     package exists in chartmuseum storage
+    package contains expected files
+    HelmPush.return code should be   0
     ChartMuseum.return code should be   0
     clear chartmuseum storage
 
     # Repo URL
     push chart directory to url
     HelmPush.return code should be   0
     package exists in chartmuseum storage
+    package contains expected files
+    HelmPush.return code should be   0
     ChartMuseum.return code should be   0
     clear chartmuseum storage
 
@@ -45,13 +59,17 @@ Chart directory can be pushed to ChartMuseum with custom version
     push chart directory    latest
     HelmPush.return code should be   0
     package exists in chartmuseum storage   latest
+    package contains expected files
+    HelmPush.return code should be   0
     ChartMuseum.return code should be   0
     clear chartmuseum storage
 
     # Repo URL
     push chart directory to url    latest
     HelmPush.return code should be   0
     package exists in chartmuseum storage   latest
+    package contains expected files
+    HelmPush.return code should be   0
     ChartMuseum.return code should be   0
     clear chartmuseum storage
 
@@ -60,13 +78,17 @@ Chart package can be pushed to ChartMuseum
     push chart package
     HelmPush.return code should be   0
     package exists in chartmuseum storage
+    package contains expected files
+    HelmPush.return code should be   0
     ChartMuseum.return code should be   0
     clear chartmuseum storage
 
     # Repo URL
     push chart package to url
     HelmPush.return code should be   0
     package exists in chartmuseum storage
+    package contains expected files
+    HelmPush.return code should be   0
     ChartMuseum.return code should be   0
     clear chartmuseum storage
 
@@ -75,13 +97,17 @@ Chart package can be pushed to ChartMuseum with custom version
     push chart package  latest
     HelmPush.return code should be   0
     package exists in chartmuseum storage   latest
+    package contains expected files
+    HelmPush.return code should be   0
     ChartMuseum.return code should be   0
     clear chartmuseum storage
 
     # Repo URL
     push chart package to url  latest
     HelmPush.return code should be   0
     package exists in chartmuseum storage   latest
+    package contains expected files
+    HelmPush.return code should be   0
     ChartMuseum.return code should be   0
     clear chartmuseum storage
 

diff --git a/acceptance_tests/lib/ChartMuseum.py b/acceptance_tests/lib/ChartMuseum.py
@@ -29,5 +29,23 @@ def print_chartmuseum_logs(self):
     def package_exists_in_chartmuseum_storage(self, find=''):
         self.run_command('find %s -maxdepth 1 -name "*%s.tgz" | grep tgz' % (common.STORAGE_DIR, find))
 
+    def package_contains_expected_files(self):
+        # Check for requirements.yaml in Helm 2 (a Helm 2-specific file)
+        checkRequirementsYamlCmd = '(cd %s && mkdir -p tmp && tar -xf *.tgz --directory tmp && find tmp -name requirements.yaml | grep requirements.yaml)' % common.STORAGE_DIR
+
+        # Check for values.schema.json in Helm 3 (a Helm 3-specific file)
+        checkValuesSchemaJsonCmd = '(cd %s && mkdir -p tmp && tar -xf *.tgz --directory tmp && find tmp -name values.schema.json | grep values.schema.json)' % common.STORAGE_DIR
+
+        if common.USE_OPPOSITE_VERSION:
+            if 'helm3' in common.HELM_EXE:
+                self.run_command(checkRequirementsYamlCmd)
+            else:
+                self.run_command(checkValuesSchemaJsonCmd)
+        else:
+            if 'helm3' in common.HELM_EXE:
+                self.run_command(checkValuesSchemaJsonCmd)
+            else:
+                self.run_command(checkRequirementsYamlCmd)
+
     def clear_chartmuseum_storage(self):
         self.run_command('rm %s*.tgz' % common.STORAGE_DIR)
diff --git a/acceptance_tests/lib/Helm.py b/acceptance_tests/lib/Helm.py
@@ -14,6 +14,12 @@ def set_helm_version(self, version):
         else:
             raise Exception('invalid Helm version provided: %s' % version)
 
+    def use_test_chart_built_by_same_helm_version(self):
+        common.USE_OPPOSITE_VERSION = False
+
+    def use_test_chart_built_by_opposite_helm_version(self):
+        common.USE_OPPOSITE_VERSION = True
+
     def add_chart_repo(self):
         self.remove_chart_repo()
         self.run_command('%s repo add %s %s' % (common.HELM_EXE, common.HELM_REPO_NAME, common.HELM_REPO_URL))

diff --git a/acceptance_tests/lib/HelmPush.py b/acceptance_tests/lib/HelmPush.py
@@ -3,9 +3,14 @@
 
 class HelmPush(common.CommandRunner):
     def _testchart_path(self):
-        if 'helm3' in common.HELM_EXE:
+        if common.USE_OPPOSITE_VERSION:
+            if 'helm3' in common.HELM_EXE:
+                return '%s/helm2/mychart' % common.TESTCHARTS_DIR
             return '%s/helm3/my-v3-chart' % common.TESTCHARTS_DIR
-        return '%s/helm2/mychart' % common.TESTCHARTS_DIR
+        else:
+            if 'helm3' in common.HELM_EXE:
+                return '%s/helm3/my-v3-chart' % common.TESTCHARTS_DIR
+            return '%s/helm2/mychart' % common.TESTCHARTS_DIR
 
     def helm_major_version_detected_by_plugin_is(self, version):
         cmd = '%s push --check-helm-version' % common.HELM_EXE

diff --git a/acceptance_tests/lib/common.py b/acceptance_tests/lib/common.py
@@ -11,6 +11,7 @@
 STORAGE_DIR = os.path.join(ACCEPTANCE_DIR, 'storage/')
 LOGFILE = '.chartmuseum.log'
 HELM_EXE = 'HELM_HOME=%s helm2' % os.getenv('TEST_HELM_HOME', '')
+USE_OPPOSITE_VERSION = False
 
 
 class CommandRunner(object):

diff --git a/pkg/helm/chart.go b/pkg/helm/chart.go
@@ -4,31 +4,56 @@ import (
 	"helm.sh/helm/v3/pkg/chart"
 	"helm.sh/helm/v3/pkg/chart/loader"
 	"helm.sh/helm/v3/pkg/chartutil"
+	v2chartutil "k8s.io/helm/pkg/chartutil"
+	v2chart "k8s.io/helm/pkg/proto/hapi/chart"
 )
 
 type (
 	// Chart is a helm package that contains metadata
 	Chart struct {
-		*chart.Chart
+		V3 *chart.Chart
+		V2 *v2chart.Chart
 	}
 )
 
 // SetVersion overrides the chart version
 func (c *Chart) SetVersion(version string) {
-	c.Metadata.Version = version
+	if c.V2 != nil {
+		c.V2.Metadata.Version = version
+	} else {
+		c.V3.Metadata.Version = version
+	}
 }
 
 // GetChartByName returns a chart by "name", which can be
 // either a directory or .tgz package
 func GetChartByName(name string) (*Chart, error) {
-	cc, err := loader.Load(name)
+	c := &Chart{}
+	v3c, err := loader.Load(name)
 	if err != nil {
 		return nil, err
 	}
-	return &Chart{cc}, nil
+
+	// If the Helm 2 API version (v1) is detected, use the old
+	// method to load the chart
+	if v3c.Metadata.APIVersion == chart.APIVersionV1 {
+		v2c, err := v2chartutil.Load(name)
+		if err != nil {
+			return nil, err
+		}
+		c.V2 = v2c
+	} else {
+		c.V3 = v3c
+	}
+
+	return c, nil
 }
 
 // CreateChartPackage creates a new .tgz package in directory
 func CreateChartPackage(c *Chart, outDir string) (string, error) {
-	return chartutil.Save(c.Chart, outDir)
+	if c.V2 != nil {
+		return v2chartutil.Save(c.V2, outDir)
+	} else {
+		return chartutil.Save(c.V3, outDir)
+	}
 }
diff --git a/pkg/helm/chart_test.go b/pkg/helm/chart_test.go
@@ -15,8 +15,8 @@ func TestSetVersion(t *testing.T) {
 		t.Error("unexpected error getting test tarball chart", err)
 	}
 	c.SetVersion("latest")
-	if c.Metadata.Version != "latest" {
-		t.Errorf("expected chart version to be latest, instead got %s", c.Metadata.Version)
+	if c.V2.Metadata.Version != "latest" {
+		t.Errorf("expected chart version to be latest, instead got %s", c.V2.Metadata.Version)
 	}
 }
 
@@ -32,11 +32,11 @@ func TestGetChartByName(t *testing.T) {
 	if err != nil {
 		t.Error("unexpected error getting test tarball chart", err)
 	}
-	if c.Metadata.Name != "mychart" {
-		t.Errorf("expexted chart name to be mychart, instead got %s", c.Metadata.Name)
+	if c.V2.Metadata.Name != "mychart" {
+		t.Errorf("expexted chart name to be mychart, instead got %s", c.V2.Metadata.Name)
 	}
-	if c.Metadata.Version != "0.1.0" {
-		t.Errorf("expexted chart version to be 0.1.0, instead got %s", c.Metadata.Version)
+	if c.V2.Metadata.Version != "0.1.0" {
+		t.Errorf("expexted chart version to be 0.1.0, instead got %s", c.V2.Metadata.Version)
 	}
 }
 

diff --git a/testdata/charts/helm2/mychart/charts/mariadb-5.11.3.tgz b/testdata/charts/helm2/mychart/charts/mariadb-5.11.3.tgz
diff --git a/testdata/charts/helm2/mychart/requirements.lock b/testdata/charts/helm2/mychart/requirements.lock
@@ -0,0 +1,6 @@
+dependencies:
+- name: mariadb
+  repository: https://kubernetes-charts.storage.googleapis.com/
+  version: 5.11.3
+digest: sha256:e09c8ca7126923a30e39f442c3863b44684d4eb3f7b6dc869f0206da4463f416
+generated: "2019-12-16T18:06:12.535235-06:00"
diff --git a/testdata/charts/helm2/mychart/requirements.yaml b/testdata/charts/helm2/mychart/requirements.yaml
@@ -0,0 +1,4 @@
+dependencies:
+  - name: mariadb
+    version: 5.x.x
+    repository: https://kubernetes-charts.storage.googleapis.com/