From 684624b1a0d8919443527ed1213d8528ec341b6c Mon Sep 17 00:00:00 2001 From: Aurel Canciu Date: Tue, 27 Oct 2020 11:10:46 +0200 Subject: [PATCH] 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 --- controllers/helmchart_controller.go | 89 +++++++------ controllers/helmchart_controller_test.go | 102 +++++++++++++++ internal/helm/chart.go | 65 ++++++--- internal/helm/chart_test.go | 160 +++++++++++++++++++++++ 4 files changed, 355 insertions(+), 61 deletions(-) create mode 100644 internal/helm/chart_test.go diff --git a/controllers/helmchart_controller.go b/controllers/helmchart_controller.go index 569589cc..58685059 100644 --- a/controllers/helmchart_controller.go +++ b/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() - pkg.Destination = tmpDir - pkgPath, err := pkg.Run(chartPath, nil) + 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 @@ -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 { - err = fmt.Errorf("chart path error: %w", err) + chartFileInfo, err := os.Stat(chartPath) + if err != nil { + err = fmt.Errorf("chart location read error: %w", err) return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err } - - // Read the chart metadata - chartMetadata, err := chartutil.LoadChartfile(path.Join(chartPath, chartutil.ChartfileName)) + helmChart, err := loader.Load(chartPath) 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, - fmt.Sprintf("%s-%s.tgz", chartMetadata.Name, chartMetadata.Version)) + newArtifact := r.Storage.NewArtifactFor(chart.Kind, chart.ObjectMeta.GetObjectMeta(), helmChart.Metadata.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 } - // Overwrite default values if configured - if err := helm.OverwriteChartDefaultValues(chartPath, chart.Spec.ValuesFile); err != nil { - return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPackageFailedReason, err.Error()), err + // 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 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 diff --git a/controllers/helmchart_controller_test.go b/controllers/helmchart_controller_test.go index 6c088ef7..b4ad2076 100644 --- a/controllers/helmchart_controller_test.go +++ b/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()) + }) }) }) diff --git a/internal/helm/chart.go b/internal/helm/chart.go index b7bbea89..6bc27b39 100644 --- a/internal/helm/chart.go +++ b/internal/helm/chart.go @@ -18,35 +18,58 @@ package helm import ( "fmt" - "io" - "os" - "path" + "reflect" + helmchart "helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/chartutil" ) -// OverwriteChartDefaultValues overwrites the chart default values file in the -// given chartPath with the contents of the given valuesFile. -func OverwriteChartDefaultValues(chartPath, valuesFile string) error { +// OverwriteChartDefaultValues overwrites the chart default values file with the +// contents of the given valuesFile. +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() { - return fmt.Errorf("invalid values file path: %s", valuesFile) + + // Find override file and retrieve contents + 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) - if err != nil { - return fmt.Errorf("failed to open values file '%s': %w", chartutil.ValuesfileName, err) + + // Replace current values file in Raw field + for _, f := range chart.Raw { + if f.Name == chartutil.ValuesfileName { + // Do nothing if contents are equal + if reflect.DeepEqual(f.Data, valuesData) { + return false, nil + } + + // Replace in Files field + for _, f := range chart.Files { + if f.Name == chartutil.ValuesfileName { + f.Data = valuesData + } + } + + f.Data = valuesData + chart.Values = values + return true, nil + } } - defer t.Close() - if _, err := io.Copy(t, src); err != nil { - return fmt.Errorf("failed to overwrite default values with '%s': %w", valuesFile, err) - } - return 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) } diff --git a/internal/helm/chart_test.go b/internal/helm/chart_test.go new file mode 100644 index 00000000..d27769c2 --- /dev/null +++ b/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 + } + }) +}