Merge pull request #485 from fluxcd/helmchart-reconciler-dev

controllers/helmchart_controller_test.go

@@ -25,7 +25,6 @@ import (
  "path"
  "path/filepath"
  "strings"
- "testing"
  "time"
 
  "github.com/fluxcd/pkg/apis/meta"
@@ -732,6 +731,7 @@ var _ = Describe("HelmChartReconciler", func() {
  }, timeout, interval).Should(BeTrue())
  helmChart, err := loader.Load(storage.LocalPath(*now.Status.Artifact))
  Expect(err).NotTo(HaveOccurred())
+ Expect(helmChart.Values).ToNot(BeNil())
  Expect(helmChart.Values["testDefault"]).To(BeTrue())
  Expect(helmChart.Values["testOverride"]).To(BeFalse())
 
@@ -1326,26 +1326,3 @@ var _ = Describe("HelmChartReconciler", func() {
  })
  })
 })
-
-func Test_validHelmChartName(t *testing.T) {
- tests := []struct {
- name string
- chart string
- expectErr bool
- }{
- {"valid", "drupal", false},
- {"valid dash", "nginx-lego", false},
- {"valid dashes", "aws-cluster-autoscaler", false},
- {"valid alphanum", "ng1nx-leg0", false},
- {"invalid slash", "artifactory/invalid", true},
- {"invalid dot", "in.valid", true},
- {"invalid uppercase", "inValid", true},
- }
- for _, tt := range tests {
- t.Run(tt.name, func(t *testing.T) {
- if err := validHelmChartName(tt.chart); (err != nil) != tt.expectErr {
- t.Errorf("validHelmChartName() error = %v, expectErr %v", err, tt.expectErr)
- }
- })
- }
-}

controllers/helmrepository_controller.go

@@ -17,14 +17,14 @@ limitations under the License.
 package controllers
 
 import (
- "bytes"
  "context"
  "fmt"
  "net/url"
+ "os"
  "time"
 
  "github.com/go-logr/logr"
- "helm.sh/helm/v3/pkg/getter"
+ helmgetter "helm.sh/helm/v3/pkg/getter"
  corev1 "k8s.io/api/core/v1"
  apimeta "k8s.io/apimachinery/pkg/api/meta"
  metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -37,15 +37,15 @@ import (
  "sigs.k8s.io/controller-runtime/pkg/controller"
  "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
  "sigs.k8s.io/controller-runtime/pkg/predicate"
- "sigs.k8s.io/yaml"
 
  "github.com/fluxcd/pkg/apis/meta"
  "github.com/fluxcd/pkg/runtime/events"
  "github.com/fluxcd/pkg/runtime/metrics"
  "github.com/fluxcd/pkg/runtime/predicates"
 
  sourcev1 "github.com/fluxcd/source-controller/api/v1beta1"
- "github.com/fluxcd/source-controller/internal/helm"
+ "github.com/fluxcd/source-controller/internal/helm/getter"
+ "github.com/fluxcd/source-controller/internal/helm/repository"
 )
 
 // +kubebuilder:rbac:groups=source.toolkit.fluxcd.io,resources=helmrepositories,verbs=get;list;watch;create;update;patch;delete
@@ -58,7 +58,7 @@ type HelmRepositoryReconciler struct {
  client.Client
  Scheme *runtime.Scheme
  Storage *Storage
- Getters getter.Providers
+ Getters helmgetter.Providers
  EventRecorder kuberecorder.EventRecorder
  ExternalEventRecorder *events.Recorder
  MetricsRecorder *metrics.Recorder
@@ -170,96 +170,108 @@ func (r *HelmRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reque
  return ctrl.Result{RequeueAfter: repository.GetInterval().Duration}, nil
 }
 
-func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, repository sourcev1.HelmRepository) (sourcev1.HelmRepository, error) {
- clientOpts := []getter.Option{
- getter.WithURL(repository.Spec.URL),
- getter.WithTimeout(repository.Spec.Timeout.Duration),
- getter.WithPassCredentialsAll(repository.Spec.PassCredentials),
+func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, repo sourcev1.HelmRepository) (sourcev1.HelmRepository, error) {
+ clientOpts := []helmgetter.Option{
+ helmgetter.WithURL(repo.Spec.URL),
+ helmgetter.WithTimeout(repo.Spec.Timeout.Duration),
+ helmgetter.WithPassCredentialsAll(repo.Spec.PassCredentials),
  }
- if repository.Spec.SecretRef != nil {
+ if repo.Spec.SecretRef != nil {
  name := types.NamespacedName{
- Namespace: repository.GetNamespace(),
- Name: repository.Spec.SecretRef.Name,
+ Namespace: repo.GetNamespace(),
+ Name: repo.Spec.SecretRef.Name,
  }
 
  var secret corev1.Secret
  err := r.Client.Get(ctx, name, &secret)
  if err != nil {
  err = fmt.Errorf("auth secret error: %w", err)
- return sourcev1.HelmRepositoryNotReady(repository, sourcev1.AuthenticationFailedReason, err.Error()), err
+ return sourcev1.HelmRepositoryNotReady(repo, sourcev1.AuthenticationFailedReason, err.Error()), err
  }
 
- opts, cleanup, err := helm.ClientOptionsFromSecret(secret)
+ authDir, err := os.MkdirTemp("", repo.Kind+"-"+repo.Namespace+"-"+repo.Name+"-")
+ if err != nil {
+ err = fmt.Errorf("failed to create temporary working directory for credentials: %w", err)
+ return sourcev1.HelmRepositoryNotReady(repo, sourcev1.AuthenticationFailedReason, err.Error()), err
+ }
+ defer os.RemoveAll(authDir)
+
+ opts, err := getter.ClientOptionsFromSecret(authDir, secret)
  if err != nil {
  err = fmt.Errorf("auth options error: %w", err)
- return sourcev1.HelmRepositoryNotReady(repository, sourcev1.AuthenticationFailedReason, err.Error()), err
+ return sourcev1.HelmRepositoryNotReady(repo, sourcev1.AuthenticationFailedReason, err.Error()), err
  }
- defer cleanup()
  clientOpts = append(clientOpts, opts...)
  }
 
- chartRepo, err := helm.NewChartRepository(repository.Spec.URL, r.Getters, clientOpts)
+ chartRepo, err := repository.NewChartRepository(repo.Spec.URL, "", r.Getters, clientOpts)
  if err != nil {
  switch err.(type) {
  case *url.Error:
- return sourcev1.HelmRepositoryNotReady(repository, sourcev1.URLInvalidReason, err.Error()), err
+ return sourcev1.HelmRepositoryNotReady(repo, sourcev1.URLInvalidReason, err.Error()), err
  default:
- return sourcev1.HelmRepositoryNotReady(repository, sourcev1.IndexationFailedReason, err.Error()), err
+ return sourcev1.HelmRepositoryNotReady(repo, sourcev1.IndexationFailedReason, err.Error()), err
  }
  }
- if err := chartRepo.DownloadIndex(); err != nil {
+ checksum, err := chartRepo.CacheIndex()
+ if err != nil {
  err = fmt.Errorf("failed to download repository index: %w", err)
- return sourcev1.HelmRepositoryNotReady(repository, sourcev1.IndexationFailedReason, err.Error()), err
+ return sourcev1.HelmRepositoryNotReady(repo, sourcev1.IndexationFailedReason, err.Error()), err
  }
+ defer chartRepo.RemoveCache()
 
- indexBytes, err := yaml.Marshal(&chartRepo.Index)
- if err != nil {
- return sourcev1.HelmRepositoryNotReady(repository, sourcev1.StorageOperationFailedReason, err.Error()), err
- }
- hash := r.Storage.Checksum(bytes.NewReader(indexBytes))
- artifact := r.Storage.NewArtifactFor(repository.Kind,
- repository.ObjectMeta.GetObjectMeta(),
- hash,
- fmt.Sprintf("index-%s.yaml", hash))
- // return early on unchanged index
- if apimeta.IsStatusConditionTrue(repository.Status.Conditions, meta.ReadyCondition) && repository.GetArtifact().HasRevision(artifact.Revision) {
- if artifact.URL != repository.GetArtifact().URL {
- r.Storage.SetArtifactURL(repository.GetArtifact())
- repository.Status.URL = r.Storage.SetHostname(repository.Status.URL)
+ artifact := r.Storage.NewArtifactFor(repo.Kind,
+ repo.ObjectMeta.GetObjectMeta(),
+ "",
+ fmt.Sprintf("index-%s.yaml", checksum))
+
+ // Return early on unchanged index
+ if apimeta.IsStatusConditionTrue(repo.Status.Conditions, meta.ReadyCondition) &&
+ (repo.GetArtifact() != nil && repo.GetArtifact().Checksum == checksum) {
+ if artifact.URL != repo.GetArtifact().URL {
+ r.Storage.SetArtifactURL(repo.GetArtifact())
+ repo.Status.URL = r.Storage.SetHostname(repo.Status.URL)
  }
- return repository, nil
+ return repo, nil
+ }
+
+ // Load the cached repository index to ensure it passes validation
+ if err := chartRepo.LoadFromCache(); err != nil {
+ return sourcev1.HelmRepositoryNotReady(repo, sourcev1.IndexationFailedReason, err.Error()), err
  }
+ // The repository checksum is the SHA256 of the loaded bytes, after sorting
+ artifact.Revision = chartRepo.Checksum
+ chartRepo.Unload()
 
- // create artifact dir
+ // Create artifact dir
  err = r.Storage.MkdirAll(artifact)
  if err != nil {
  err = fmt.Errorf("unable to create repository index directory: %w", err)
- return sourcev1.HelmRepositoryNotReady(repository, sourcev1.StorageOperationFailedReason, err.Error()), err
+ return sourcev1.HelmRepositoryNotReady(repo, sourcev1.StorageOperationFailedReason, err.Error()), err
  }
 
- // acquire lock
+ // Acquire lock
  unlock, err := r.Storage.Lock(artifact)
  if err != nil {
  err = fmt.Errorf("unable to acquire lock: %w", err)
- return sourcev1.HelmRepositoryNotReady(repository, sourcev1.StorageOperationFailedReason, err.Error()), err
+ return sourcev1.HelmRepositoryNotReady(repo, sourcev1.StorageOperationFailedReason, err.Error()), err
  }
  defer unlock()
 
- // save artifact to storage
- if err := r.Storage.AtomicWriteFile(&artifact, bytes.NewReader(indexBytes), 0644); err != nil {
- err = fmt.Errorf("unable to write repository index file: %w", err)
- return sourcev1.HelmRepositoryNotReady(repository, sourcev1.StorageOperationFailedReason, err.Error()), err
+ // Save artifact to storage
+ if err = r.Storage.CopyFromPath(&artifact, chartRepo.CachePath); err != nil {
+ return sourcev1.HelmRepositoryNotReady(repo, sourcev1.StorageOperationFailedReason, err.Error()), err
  }
 
- // update index symlink
+ // Update index symlink
  indexURL, err := r.Storage.Symlink(artifact, "index.yaml")
  if err != nil {
  err = fmt.Errorf("storage error: %w", err)
- return sourcev1.HelmRepositoryNotReady(repository, sourcev1.StorageOperationFailedReason, err.Error()), err
+ return sourcev1.HelmRepositoryNotReady(repo, sourcev1.StorageOperationFailedReason, err.Error()), err
  }
 
  message := fmt.Sprintf("Fetched revision: %s", artifact.Revision)
- return sourcev1.HelmRepositoryReady(repository, artifact, indexURL, sourcev1.IndexationSucceededReason, message), nil
+ return sourcev1.HelmRepositoryReady(repo, artifact, indexURL, sourcev1.IndexationSucceededReason, message), nil
 }
 
 func (r *HelmRepositoryReconciler) reconcileDelete(ctx context.Context, repository sourcev1.HelmRepository) (ctrl.Result, error) {

go.mod

@@ -38,6 +38,7 @@ require (
  github.com/minio/minio-go/v7 v7.0.10
  github.com/onsi/ginkgo v1.16.4
  github.com/onsi/gomega v1.14.0
+ github.com/otiai10/copy v1.7.0
  github.com/spf13/pflag v1.0.5
  github.com/yvasiyarov/go-metrics v0.0.0-20150112132944-c25f46c4b940 // indirect
  github.com/yvasiyarov/gorelic v0.0.7 // indirect

go.sum

@@ -738,6 +738,13 @@ github.com/openzipkin-contrib/zipkin-go-opentracing v0.4.5/go.mod h1:/wsWhb9smxS
 github.com/openzipkin/zipkin-go v0.1.6/go.mod h1:QgAqvLzwWbR/WpD4A3cGpPtJrZXNIiJc5AZX7/PBEpw=
 github.com/openzipkin/zipkin-go v0.2.1/go.mod h1:NaW6tEwdmWMaCDZzg8sh+IBNOxHMPnhQw8ySjnjRyN4=
 github.com/openzipkin/zipkin-go v0.2.2/go.mod h1:NaW6tEwdmWMaCDZzg8sh+IBNOxHMPnhQw8ySjnjRyN4=
+github.com/otiai10/copy v1.7.0 h1:hVoPiN+t+7d2nzzwMiDHPSOogsWAStewq3TwU05+clE=
+github.com/otiai10/copy v1.7.0/go.mod h1:rmRl6QPdJj6EiUqXQ/4Nn2lLXoNQjFCQbbNrxgc/t3U=
+github.com/otiai10/curr v0.0.0-20150429015615-9b4961190c95/go.mod h1:9qAhocn7zKJG+0mI8eUu6xqkFDYS2kb2saOteoSB3cE=
+github.com/otiai10/curr v1.0.0/go.mod h1:LskTG5wDwr8Rs+nNQ+1LlxRjAtTZZjtJW4rMXl6j4vs=
+github.com/otiai10/mint v1.3.0/go.mod h1:F5AjcsTsWUqX+Na9fpHb52P8pcRX2CI6A3ctIT91xUo=
+github.com/otiai10/mint v1.3.3 h1:7JgpsBaN0uMkyju4tbYHu0mnM55hNKVYLsXmwr15NQI=
+github.com/otiai10/mint v1.3.3/go.mod h1:/yxELlJQ0ufhjUwhshSj+wFjZ78CnZ48/1wtmBH1OTc=
 github.com/pact-foundation/pact-go v1.0.4/go.mod h1:uExwJY4kCzNPcHRj+hCR/HBbOOIwwtUjcrb0b5/5kLM=
 github.com/pascaldekloe/goe v0.0.0-20180627143212-57f6aae5913c/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc=
 github.com/pborman/uuid v1.2.0/go.mod h1:X/NO0urCmaxf9VXbdlT7C2Yzkj2IKimNn4k+gtPdI/k=