Add support for loading packaged helm charts

The feature allows the source-controller to load packaged helm charts
for HelmChart resource artifacts from GitRepository and Bucket sources

Signed-off-by: Aurel Canciu <aurelcanciu@gmail.com>

controllers/helmchart_controller.go

@@ -28,7 +28,7 @@ import (
 
 	"github.com/fluxcd/pkg/apis/meta"
 	"github.com/go-logr/logr"
-	"helm.sh/helm/v3/pkg/action"
+	"helm.sh/helm/v3/pkg/chart/loader"
 	"helm.sh/helm/v3/pkg/chartutil"
 	"helm.sh/helm/v3/pkg/getter"
 	corev1 "k8s.io/api/core/v1"
@@ -323,30 +323,35 @@ func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context,
 	)
 	switch {
 	case chart.Spec.ValuesFile != "" && chart.Spec.ValuesFile != chartutil.ValuesfileName:
+		var (
+			tmpDir  string
+			pkgPath string
+		)
+		// Load the chart
+		helmChart, err := loader.LoadArchive(res)
+		if err != nil {
+			err = fmt.Errorf("load chart error: %w", err)
+			return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
+		}
+
+		// Overwrite values file
+		if changed, err := helm.OverwriteChartDefaultValues(helmChart, chart.Spec.ValuesFile); err != nil {
+			return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPackageFailedReason, err.Error()), err
+		} else if !changed {
+			// No changes, skip to write original package to storage
+			goto skipToDefault
+		}
+
 		// Create temporary working directory
-		tmpDir, err := ioutil.TempDir("", fmt.Sprintf("%s-%s-", chart.Namespace, chart.Name))
+		tmpDir, err = ioutil.TempDir("", fmt.Sprintf("%s-%s-", chart.Namespace, chart.Name))
 		if err != nil {
 			err = fmt.Errorf("tmp dir error: %w", err)
 			return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
 		}
 		defer os.RemoveAll(tmpDir)
 
-		// Untar chart into working directory
-		if _, err = untar.Untar(res, tmpDir); err != nil {
-			err = fmt.Errorf("chart untar error: %w", err)
-			return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
-		}
-
-		// Overwrite values file
-		chartPath := path.Join(tmpDir, chartVer.Name)
-		if err := helm.OverwriteChartDefaultValues(chartPath, chart.Spec.ValuesFile); err != nil {
-			return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPackageFailedReason, err.Error()), err
-		}
-
 		// Package the chart with the new default values
-		pkg := action.NewPackage()
+		pkgPath, err = chartutil.Save(helmChart, tmpDir)
-		pkg.Destination = tmpDir
-		pkgPath, err := pkg.Run(chartPath, nil)
 		if err != nil {
 			err = fmt.Errorf("chart package error: %w", err)
 			return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPackageFailedReason, err.Error()), err
@@ -360,6 +365,8 @@ func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context,
 
 		readyMessage = fmt.Sprintf("Fetched and packaged revision: %s", newArtifact.Revision)
 		readyReason = sourcev1.ChartPackageSucceededReason
+	skipToDefault:
+		fallthrough
 	default:
 		// Write artifact to storage
 		if err := r.Storage.AtomicWriteFile(&newArtifact, res, 0644); err != nil {
@@ -401,23 +408,22 @@ func (r *HelmChartReconciler) reconcileFromTarballArtifact(ctx context.Context,
 	}
 	f.Close()
 
-	// Ensure configured path is a chart directory
+	// Load the chart
 	chartPath := path.Join(tmpDir, chart.Spec.Chart)
-	if _, err := chartutil.IsChartDir(chartPath); err != nil {
+	chartFileInfo, err := os.Stat(chartPath)
-		err = fmt.Errorf("chart path error: %w", err)
+	if err != nil {
+		err = fmt.Errorf("chart location read error: %w", err)
 		return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
 	}
-
+	helmChart, err := loader.Load(chartPath)
-	// Read the chart metadata
-	chartMetadata, err := chartutil.LoadChartfile(path.Join(chartPath, chartutil.ChartfileName))
 	if err != nil {
-		err = fmt.Errorf("load chart metadata error: %w", err)
+		err = fmt.Errorf("load chart error: %w", err)
 		return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
 	}
 
 	// Return early if the revision is still the same as the current chart artifact
-	newArtifact := r.Storage.NewArtifactFor(chart.Kind, chart.ObjectMeta.GetObjectMeta(), chartMetadata.Version,
+	newArtifact := r.Storage.NewArtifactFor(chart.Kind, chart.ObjectMeta.GetObjectMeta(), helmChart.Metadata.Version,
-		fmt.Sprintf("%s-%s.tgz", chartMetadata.Name, chartMetadata.Version))
+		fmt.Sprintf("%s-%s.tgz", helmChart.Metadata.Name, helmChart.Metadata.Version))
 	if !force && meta.HasReadyCondition(chart.Status.Conditions) && chart.GetArtifact().HasRevision(newArtifact.Revision) {
 		if newArtifact.URL != artifact.URL {
 			r.Storage.SetArtifactURL(chart.GetArtifact())
@@ -426,9 +432,22 @@ func (r *HelmChartReconciler) reconcileFromTarballArtifact(ctx context.Context,
 		return chart, nil
 	}
 
+	// Either (re)package the chart with the declared default values file,
+	// or write the chart directly to storage.
+	pkgPath := chartPath
+	isDir := chartFileInfo.IsDir()
+	if isDir || (chart.Spec.ValuesFile != "" && chart.Spec.ValuesFile != chartutil.ValuesfileName) {
 		// Overwrite default values if configured
-	if err := helm.OverwriteChartDefaultValues(chartPath, chart.Spec.ValuesFile); err != nil {
+		if changed, err := helm.OverwriteChartDefaultValues(helmChart, chart.Spec.ValuesFile); err != nil {
 			return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPackageFailedReason, err.Error()), err
+		} else if isDir || changed {
+			// Package the chart
+			pkgPath, err = chartutil.Save(helmChart, tmpDir)
+			if err != nil {
+				err = fmt.Errorf("chart package error: %w", err)
+				return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPackageFailedReason, err.Error()), err
+			}
+		}
 	}
 
 	// Ensure artifact directory exists
@@ -446,16 +465,6 @@ func (r *HelmChartReconciler) reconcileFromTarballArtifact(ctx context.Context,
 	}
 	defer unlock()
 
-	// Package the chart, we use the action here instead of relying on the
-	// chartutil.Save method as the action performs a dependency check for us
-	pkg := action.NewPackage()
-	pkg.Destination = tmpDir
-	pkgPath, err := pkg.Run(chartPath, nil)
-	if err != nil {
-		err = fmt.Errorf("chart package error: %w", err)
-		return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPackageFailedReason, err.Error()), err
-	}
-
 	// Copy the packaged chart to the artifact path
 	if err := r.Storage.CopyFromPath(&newArtifact, pkgPath); err != nil {
 		err = fmt.Errorf("failed to write chart package to storage: %w", err)
@@ -463,7 +472,7 @@ func (r *HelmChartReconciler) reconcileFromTarballArtifact(ctx context.Context,
 	}
 
 	// Update symlink
-	cUrl, err := r.Storage.Symlink(newArtifact, fmt.Sprintf("%s-latest.tgz", chartMetadata.Name))
+	cUrl, err := r.Storage.Symlink(newArtifact, fmt.Sprintf("%s-latest.tgz", helmChart.Metadata.Name))
 	if err != nil {
 		err = fmt.Errorf("storage error: %w", err)
 		return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err

controllers/helmchart_controller_test.go

@@ -39,6 +39,7 @@ import (
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 	helmchart "helm.sh/helm/v3/pkg/chart"
+	"helm.sh/helm/v3/pkg/chart/loader"
 	"helm.sh/helm/v3/pkg/chartutil"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -590,5 +591,106 @@ var _ = Describe("HelmChartReconciler", func() {
 					!storage.ArtifactExist(*got.Status.Artifact)
 			}, timeout, interval).Should(BeTrue())
 		})
+
+		It("Creates artifacts with .tgz file", func() {
+			fs := memfs.New()
+			gitrepo, err := git.Init(memory.NewStorage(), fs)
+			Expect(err).NotTo(HaveOccurred())
+
+			wt, err := gitrepo.Worktree()
+			Expect(err).NotTo(HaveOccurred())
+
+			u, err := url.Parse(gitServer.HTTPAddress())
+			Expect(err).NotTo(HaveOccurred())
+			u.Path = path.Join(u.Path, fmt.Sprintf("repository-%s.git", randStringRunes(5)))
+
+			_, err = gitrepo.CreateRemote(&config.RemoteConfig{
+				Name: "origin",
+				URLs: []string{u.String()},
+			})
+			Expect(err).NotTo(HaveOccurred())
+
+			chartDir := "testdata/helmchart"
+			helmChart, err := loader.LoadDir(chartDir)
+			Expect(err).NotTo(HaveOccurred())
+
+			chartPackagePath, err := ioutil.TempDir("", fmt.Sprintf("chartpackage-%s-%s", helmChart.Name(), randStringRunes(5)))
+			Expect(err).NotTo(HaveOccurred())
+			defer os.RemoveAll(chartPackagePath)
+
+			pkg, err := chartutil.Save(helmChart, chartPackagePath)
+			Expect(err).NotTo(HaveOccurred())
+
+			b, err := ioutil.ReadFile(pkg)
+			Expect(err).NotTo(HaveOccurred())
+
+			tgz := filepath.Base(pkg)
+			ff, err := fs.Create(tgz)
+			Expect(err).NotTo(HaveOccurred())
+
+			_, err = ff.Write(b)
+			Expect(err).NotTo(HaveOccurred())
+
+			ff.Close()
+			_, err = wt.Add(tgz)
+			Expect(err).NotTo(HaveOccurred())
+
+			_, err = wt.Commit("Helm chart", &git.CommitOptions{Author: &object.Signature{
+				Name:  "John Doe",
+				Email: "john@example.com",
+				When:  time.Now(),
+			}})
+			Expect(err).NotTo(HaveOccurred())
+
+			err = gitrepo.Push(&git.PushOptions{})
+			Expect(err).NotTo(HaveOccurred())
+
+			repositoryKey := types.NamespacedName{
+				Name:      fmt.Sprintf("git-repository-sample-%s", randStringRunes(5)),
+				Namespace: namespace.Name,
+			}
+			repository := &sourcev1.GitRepository{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      repositoryKey.Name,
+					Namespace: repositoryKey.Namespace,
+				},
+				Spec: sourcev1.GitRepositorySpec{
+					URL:      u.String(),
+					Interval: metav1.Duration{Duration: indexInterval},
+				},
+			}
+			Expect(k8sClient.Create(context.Background(), repository)).Should(Succeed())
+			defer k8sClient.Delete(context.Background(), repository)
+
+			key := types.NamespacedName{
+				Name:      "helmchart-sample-" + randStringRunes(5),
+				Namespace: namespace.Name,
+			}
+			chart := &sourcev1.HelmChart{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      key.Name,
+					Namespace: key.Namespace,
+				},
+				Spec: sourcev1.HelmChartSpec{
+					Chart:   tgz,
+					Version: "*",
+					SourceRef: sourcev1.LocalHelmChartSourceReference{
+						Kind: sourcev1.GitRepositoryKind,
+						Name: repositoryKey.Name,
+					},
+					Interval: metav1.Duration{Duration: pullInterval},
+				},
+			}
+			Expect(k8sClient.Create(context.Background(), chart)).Should(Succeed())
+			defer k8sClient.Delete(context.Background(), chart)
+
+			By("Expecting artifact")
+			got := &sourcev1.HelmChart{}
+			Eventually(func() bool {
+				_ = k8sClient.Get(context.Background(), key, got)
+				return got.Status.Artifact != nil &&
+					storage.ArtifactExist(*got.Status.Artifact)
+			}, timeout, interval).Should(BeTrue())
+		})
 	})
 })

internal/helm/chart.go

@@ -18,35 +18,58 @@ package helm
 
 import (
 	"fmt"
-	"io"
+	"reflect"
-	"os"
-	"path"
 
+	helmchart "helm.sh/helm/v3/pkg/chart"
 	"helm.sh/helm/v3/pkg/chartutil"
 )
 
-// OverwriteChartDefaultValues overwrites the chart default values file in the
+// OverwriteChartDefaultValues overwrites the chart default values file with the
-// given chartPath with the contents of the given valuesFile.
+// contents of the given valuesFile.
-func OverwriteChartDefaultValues(chartPath, valuesFile string) error {
+func OverwriteChartDefaultValues(chart *helmchart.Chart, valuesFile string) (bool, error) {
 	if valuesFile == "" || valuesFile == chartutil.ValuesfileName {
-		return nil
+		return false, nil
 	}
-	srcPath := path.Join(chartPath, valuesFile)
+
-	if f, err := os.Stat(srcPath); os.IsNotExist(err) || !f.Mode().IsRegular() {
+	// Find override file and retrieve contents
-		return fmt.Errorf("invalid values file path: %s", valuesFile)
+	var valuesData []byte
+	for _, f := range chart.Files {
+		if f.Name == valuesFile {
+			valuesData = f.Data
+			break
 		}
-	src, err := os.Open(srcPath)
+	}
+	if valuesData == nil {
+		return false, fmt.Errorf("failed to locate override values file: %s", valuesFile)
+	}
+
+	// Read override values file data
+	values, err := chartutil.ReadValues(valuesData)
 	if err != nil {
-		return fmt.Errorf("failed to open values file '%s': %w", valuesFile, err)
+		return false, fmt.Errorf("failed to parse override values file: %s", valuesFile)
 	}
-	defer src.Close()
+
-	t, err := os.OpenFile(path.Join(chartPath, chartutil.ValuesfileName), os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0644)
+	// Replace current values file in Raw field
-	if err != nil {
+	for _, f := range chart.Raw {
-		return fmt.Errorf("failed to open values file '%s': %w", chartutil.ValuesfileName, err)
+		if f.Name == chartutil.ValuesfileName {
+			// Do nothing if contents are equal
+			if reflect.DeepEqual(f.Data, valuesData) {
+				return false, nil
 			}
-	defer t.Close()
+
-	if _, err := io.Copy(t, src); err != nil {
+			// Replace in Files field
-		return fmt.Errorf("failed to overwrite default values with '%s': %w", valuesFile, err)
+			for _, f := range chart.Files {
+				if f.Name == chartutil.ValuesfileName {
+					f.Data = valuesData
 				}
-	return nil
+			}
+
+			f.Data = valuesData
+			chart.Values = values
+			return true, nil
+		}
+	}
+
+	// This should never happen, helm charts must have a values.yaml file to be valid
+	return false, fmt.Errorf("failed to locate values file: %s", chartutil.ValuesfileName)
 }

internal/helm/chart_test.go

@@ -0,0 +1,160 @@
+/*
+Copyright 2020 The Flux CD contributors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package helm
+
+import (
+	"reflect"
+	"strings"
+	"testing"
+
+	helmchart "helm.sh/helm/v3/pkg/chart"
+	"helm.sh/helm/v3/pkg/chartutil"
+)
+
+var (
+	originalValuesFixture []byte            = []byte("override: original")
+	overrideValuesFixture []byte            = []byte("override: test")
+	chartFilesFixture     []*helmchart.File = []*helmchart.File{
+		{
+			Name: "values.yaml",
+			Data: originalValuesFixture,
+		},
+		{
+			Name: "values-identical.yaml",
+			Data: originalValuesFixture,
+		},
+		{
+			Name: "values-override.yaml",
+			Data: overrideValuesFixture,
+		},
+		{
+			Name: "values-invalid.yaml",
+			Data: []byte(":fail!"),
+		},
+	}
+	chartFixture helmchart.Chart = helmchart.Chart{
+		Metadata: &helmchart.Metadata{
+			Name:    "test",
+			Version: "0.1.0",
+		},
+		Raw:   chartFilesFixture,
+		Files: chartFilesFixture,
+	}
+)
+
+func TestOverwriteChartDefaultValues(t *testing.T) {
+	for _, tt := range []string{"", "values.yaml", "values-identical.yaml"} {
+		t.Run(tt, func(t *testing.T) {
+			fixture := chartFixture
+			ok, err := OverwriteChartDefaultValues(&fixture, tt)
+			if ok {
+				t.Error("OverwriteChartDefaultValues() should return false")
+				return
+			}
+			if err != nil {
+				t.Errorf("OverwriteChartDefaultValues() error = %v", err)
+				return
+			}
+			for _, f := range fixture.Raw {
+				if f.Name == chartutil.ValuesfileName && !reflect.DeepEqual(f.Data, originalValuesFixture) {
+					t.Error("OverwriteChartDefaultValues() should not override values.yaml in Raw field")
+					return
+				}
+			}
+			for _, f := range fixture.Files {
+				if f.Name == chartutil.ValuesfileName && !reflect.DeepEqual(f.Data, originalValuesFixture) {
+					t.Error("OverwriteChartDefaultValues() should not override values.yaml in Files field")
+					return
+				}
+			}
+		})
+	}
+
+	t.Run("values-error.yaml", func(t *testing.T) {
+		fixture := chartFixture
+		ok, err := OverwriteChartDefaultValues(&fixture, "values-error.yaml")
+		if ok {
+			t.Error("OverwriteChartDefaultValues() should return false")
+		}
+		if err == nil {
+			t.Error("OverwriteChartDefaultValues() expects an error")
+			return
+		} else if !strings.Contains(err.Error(), "failed to locate override values file") {
+			t.Error("OverwriteChartDefaultValues() returned invalid error")
+			return
+		}
+	})
+
+	t.Run("values-override.yaml", func(t *testing.T) {
+		fixture := chartFixture
+		ok, err := OverwriteChartDefaultValues(&fixture, "values-override.yaml")
+		if err != nil {
+			t.Errorf("OverwriteChartDefaultValues() error = %v", err)
+			return
+		}
+		if !ok {
+			t.Error("OverwriteChartDefaultValues() should return true")
+			return
+		}
+		for _, f := range fixture.Raw {
+			if f.Name == chartutil.ValuesfileName && string(f.Data) != string(overrideValuesFixture) {
+				t.Error("OverwriteChartDefaultValues() should override values.yaml in Raw field")
+				return
+			}
+		}
+		for _, f := range fixture.Files {
+			if f.Name == chartutil.ValuesfileName && string(f.Data) != string(overrideValuesFixture) {
+				t.Error("OverwriteChartDefaultValues() should override values.yaml in Files field")
+				return
+			}
+		}
+
+		// Context: the impossible chart, no values.yaml file defined!
+		fixture.Raw = fixture.Raw[1:]
+		fixture.Files = fixture.Files[1:]
+		ok, err = OverwriteChartDefaultValues(&fixture, "values-override.yaml")
+		if ok {
+			t.Error("OverwriteChartDefaultValues() should return false")
+			return
+		}
+		if err == nil {
+			t.Error("OverwriteChartDefaultValues() expects an error")
+			return
+		} else if !strings.Contains(err.Error(), "failed to locate values file") {
+			t.Error("OverwriteChartDefaultValues() returned invalid error")
+			return
+		}
+	})
+
+	t.Run("values-invalid.yaml", func(t *testing.T) {
+		fixture := chartFixture
+		fixture.Raw[0].Data = fixture.Raw[1].Data
+		fixture.Files[0].Data = fixture.Files[1].Data
+		ok, err := OverwriteChartDefaultValues(&fixture, "values-invalid.yaml")
+		if ok {
+			t.Error("OverwriteChartDefaultValues() should return false")
+			return
+		}
+		if err == nil {
+			t.Error("OverwriteChartDefaultValues() expects an error")
+			return
+		} else if !strings.Contains(err.Error(), "failed to parse override values file") {
+			t.Error("OverwriteChartDefaultValues() returned invalid error")
+			return
+		}
+	})
+}