Merge pull request #151 from fluxcd/helmchart-bucket-source

Author: hiddeco

.github/workflows/e2e.yaml

@@ -81,12 +81,22 @@ jobs:
           wget -q https://dl.min.io/client/mc/release/linux-amd64/mc
           chmod +x mc
           ./mc alias set minio http://localhost:9000 myaccesskey mysecretkey --api S3v4
-          ./mc mb minio/podinfo
-          ./mc cp --recursive ./config/testdata/minio/manifests minio/podinfo
-      - name: Run S3 tests
+          kubectl -n source-system apply -f ./config/testdata/minio/secret.yaml
+      - name: Run Bucket tests
         run: |
-          kubectl -n source-system apply -f ./config/testdata/minio/source.yaml
+          ./mc mb minio/podinfo
+          ./mc mirror ./config/testdata/minio/manifests/ minio/podinfo
+
+          kubectl -n source-system apply -f ./config/testdata/bucket/source.yaml
           kubectl -n source-system wait bucket/podinfo --for=condition=ready --timeout=1m
+      - name: Run HelmChart from Bucket tests
+        run: |
+          ./mc mb minio/charts
+          ./mc mirror ./controllers/testdata/helmchart/ minio/charts/helmchart
+
+          kubectl -n source-system apply -f ./config/testdata/helmchart-from-bucket/source.yaml
+          kubectl -n source-system wait bucket/charts --for=condition=ready --timeout=1m
+          kubectl -n source-system wait helmchart/helmchart-bucket --for=condition=ready --timeout=1m
       - name: Debug failure
         if: failure()
         run: |

api/v1alpha1/helmchart_types.go

@@ -30,7 +30,7 @@ type HelmChartSpec struct {
 	Chart string `json:"chart"`
 
 	// The chart version semver expression, ignored for charts from GitRepository
-	// sources. Defaults to latest when omitted.
+	// and Bucket sources. Defaults to latest when omitted.
 	// +optional
 	Version string `json:"version,omitempty"`
 
@@ -55,8 +55,8 @@ type LocalHelmChartSourceReference struct {
 	// +optional
 	APIVersion string `json:"apiVersion,omitempty"`
 
-	// Kind of the referent, valid values are ('HelmRepository', 'GitRepository').
-	// +kubebuilder:validation:Enum=HelmRepository;GitRepository
+	// Kind of the referent, valid values are ('HelmRepository', 'GitRepository', 'Bucket').
+	// +kubebuilder:validation:Enum=HelmRepository;GitRepository;Bucket
 	// +required
 	Kind string `json:"kind"`
 

config/crd/bases/source.toolkit.fluxcd.io_helmcharts.yaml

@@ -73,10 +73,11 @@ spec:
                     type: string
                   kind:
                     description: Kind of the referent, valid values are ('HelmRepository',
-                      'GitRepository').
+                      'GitRepository', 'Bucket').
                     enum:
                     - HelmRepository
                     - GitRepository
+                    - Bucket
                     type: string
                   name:
                     description: Name of the referent.
@@ -91,7 +92,7 @@ spec:
                 type: string
               version:
                 description: The chart version semver expression, ignored for charts
-                  from GitRepository sources. Defaults to latest when omitted.
+                  from GitRepository and Bucket sources. Defaults to latest when omitted.
                 type: string
             required:
             - chart

config/testdata/minio/source.yaml config/testdata/bucket/source.yamlconfig/testdata/minio/source.yaml renamed to config/testdata/bucket/source.yaml

@@ -1,3 +1,4 @@
+---
 apiVersion: source.toolkit.fluxcd.io/v1alpha1
 kind: Bucket
 metadata:
@@ -11,11 +12,3 @@ spec:
   insecure: true
   secretRef:
     name: minio-credentials
----
-apiVersion: v1
-kind: Secret
-metadata:
-  name: minio-credentials
-data:
-  accesskey: bXlhY2Nlc3NrZXk=
-  secretkey: bXlzZWNyZXRrZXk=

config/testdata/helmchart-from-bucket/source.yaml

@@ -0,0 +1,25 @@
+---
+apiVersion: source.toolkit.fluxcd.io/v1alpha1
+kind: Bucket
+metadata:
+  name: charts
+spec:
+  interval: 1m
+  provider: generic
+  bucketName: charts
+  endpoint: minio.minio.svc.cluster.local:9000
+  region: us-east-1
+  insecure: true
+  secretRef:
+    name: minio-credentials
+---
+apiVersion: source.toolkit.fluxcd.io/v1alpha1
+kind: HelmChart
+metadata:
+  name: helmchart-bucket
+spec:
+  chart: ./helmchart
+  sourceRef:
+    kind: Bucket
+    name: charts
+  interval: 1m

config/testdata/minio/secret.yaml

@@ -0,0 +1,8 @@
+---
+apiVersion: v1
+kind: Secret
+metadata:
+  name: minio-credentials
+data:
+  accesskey: bXlhY2Nlc3NrZXk=
+  secretkey: bXlzZWNyZXRrZXk=

controllers/helmchart_controller.go

@@ -119,33 +119,39 @@ func (r *HelmChartReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 		log.Error(err, "unable to purge old artifacts")
 	}
 
+	// Retrieve the source
+	source, err := r.getSource(ctx, chart)
+	if err != nil {
+		chart = sourcev1.HelmChartNotReady(*chart.DeepCopy(), sourcev1.ChartPullFailedReason, err.Error())
+		if err := r.Status().Update(ctx, &chart); err != nil {
+			log.Error(err, "unable to update status")
+		}
+		return ctrl.Result{Requeue: true}, err
+	}
+
+	// Assert source is ready
+	if source.GetArtifact() == nil {
+		err = fmt.Errorf("no artifact found for source `%s` kind '%s'",
+			chart.Spec.SourceRef.Name, chart.Spec.SourceRef.Kind)
+		chart = sourcev1.HelmChartNotReady(*chart.DeepCopy(), sourcev1.ChartPullFailedReason, err.Error())
+		if err := r.Status().Update(ctx, &chart); err != nil {
+			log.Error(err, "unable to update status")
+		}
+		return ctrl.Result{Requeue: true}, err
+	}
+
 	// Perform the reconciliation for the chart source type
 	var reconciledChart sourcev1.HelmChart
 	var reconcileErr error
-	switch chart.Spec.SourceRef.Kind {
-	case sourcev1.HelmRepositoryKind:
-		repository, err := r.getChartRepositoryWithArtifact(ctx, chart)
-		if err != nil {
-			chart = sourcev1.HelmChartNotReady(*chart.DeepCopy(), sourcev1.ChartPullFailedReason, err.Error())
-			if err := r.Status().Update(ctx, &chart); err != nil {
-				log.Error(err, "unable to update status")
-			}
-			return ctrl.Result{Requeue: true}, err
-		}
-		reconciledChart, reconcileErr = r.reconcileFromHelmRepository(ctx, repository, *chart.DeepCopy(), changed)
-	case sourcev1.GitRepositoryKind:
-		repository, err := r.getGitRepositoryWithArtifact(ctx, chart)
-		if err != nil {
-			chart = sourcev1.HelmChartNotReady(*chart.DeepCopy(), sourcev1.ChartPullFailedReason, err.Error())
-			if err := r.Status().Update(ctx, &chart); err != nil {
-				log.Error(err, "unable to update status")
-			}
-			return ctrl.Result{Requeue: true}, err
-		}
-		reconciledChart, reconcileErr = r.reconcileFromGitRepository(ctx, repository, *chart.DeepCopy(), changed)
+	switch typedSource := source.(type) {
+	case *sourcev1.HelmRepository:
+		reconciledChart, reconcileErr = r.reconcileFromHelmRepository(ctx, *typedSource, *chart.DeepCopy(), changed)
+	case *sourcev1.GitRepository, *sourcev1.Bucket:
+		reconciledChart, reconcileErr = r.reconcileFromTarballArtifact(ctx, *typedSource.GetArtifact(),
+			*chart.DeepCopy(), changed)
 	default:
 		err := fmt.Errorf("unable to reconcile unsupported source reference kind '%s'", chart.Spec.SourceRef.Kind)
-		return ctrl.Result{}, err
+		return ctrl.Result{Requeue: false}, err
 	}
 
 	// Update status with the reconciliation result
@@ -188,6 +194,41 @@ func (r *HelmChartReconciler) SetupWithManagerAndOptions(mgr ctrl.Manager, opts
 		Complete(r)
 }
 
+func (r *HelmChartReconciler) getSource(ctx context.Context, chart sourcev1.HelmChart) (sourcev1.Source, error) {
+	var source sourcev1.Source
+	namespacedName := types.NamespacedName{
+		Namespace: chart.GetNamespace(),
+		Name:      chart.Spec.SourceRef.Name,
+	}
+	switch chart.Spec.SourceRef.Kind {
+	case sourcev1.HelmRepositoryKind:
+		var repository sourcev1.HelmRepository
+		err := r.Client.Get(ctx, namespacedName, &repository)
+		if err != nil {
+			return source, fmt.Errorf("failed to retrieve source: %w", err)
+		}
+		source = &repository
+	case sourcev1.GitRepositoryKind:
+		var repository sourcev1.GitRepository
+		err := r.Client.Get(ctx, namespacedName, &repository)
+		if err != nil {
+			return source, fmt.Errorf("failed to retrieve source: %w", err)
+		}
+		source = &repository
+	case sourcev1.BucketKind:
+		var bucket sourcev1.Bucket
+		err := r.Client.Get(ctx, namespacedName, &bucket)
+		if err != nil {
+			return source, fmt.Errorf("failed to retrieve source: %w", err)
+		}
+		source = &bucket
+	default:
+		return source, fmt.Errorf("source `%s` kind '%s' not supported",
+			chart.Spec.SourceRef.Name, chart.Spec.SourceRef.Kind)
+	}
+	return source, nil
+}
+
 func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context,
 	repository sourcev1.HelmRepository, chart sourcev1.HelmChart, force bool) (sourcev1.HelmChart, error) {
 	cv, err := helm.GetDownloadableChartVersionFromIndex(r.Storage.LocalPath(*repository.GetArtifact()),
@@ -201,8 +242,8 @@ func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context,
 		fmt.Sprintf("%s-%s.tgz", cv.Name, cv.Version))
 	if !force && repository.GetArtifact() != nil && repository.GetArtifact().Revision == cv.Version {
 		if artifact.URL != repository.GetArtifact().URL {
-			r.Storage.SetArtifactURL(repository.GetArtifact())
-			repository.Status.URL = r.Storage.SetHostname(repository.Status.URL)
+			r.Storage.SetArtifactURL(chart.GetArtifact())
+			repository.Status.URL = r.Storage.SetHostname(chart.Status.URL)
 		}
 		return chart, nil
 	}
@@ -353,35 +394,8 @@ func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context,
 	return sourcev1.HelmChartReady(chart, artifact, chartUrl, readyReason, readyMessage), nil
 }
 
-// getChartRepositoryWithArtifact attempts to get the v1alpha1.HelmRepository
-// for the given chart. It returns an error if the HelmRepository could
-// not be retrieved or if does not have an artifact.
-func (r *HelmChartReconciler) getChartRepositoryWithArtifact(ctx context.Context, chart sourcev1.HelmChart) (sourcev1.HelmRepository, error) {
-	if chart.Spec.SourceRef.Name == "" {
-		return sourcev1.HelmRepository{}, fmt.Errorf("no HelmRepository reference given")
-	}
-
-	name := types.NamespacedName{
-		Namespace: chart.GetNamespace(),
-		Name:      chart.Spec.SourceRef.Name,
-	}
-
-	var repository sourcev1.HelmRepository
-	err := r.Client.Get(ctx, name, &repository)
-	if err != nil {
-		err = fmt.Errorf("failed to get HelmRepository '%s': %w", name, err)
-		return repository, err
-	}
-
-	if repository.GetArtifact() == nil {
-		err = fmt.Errorf("no repository index artifact found for HelmRepository '%s'", name)
-	}
-
-	return repository, err
-}
-
-func (r *HelmChartReconciler) reconcileFromGitRepository(ctx context.Context,
-	repository sourcev1.GitRepository, chart sourcev1.HelmChart, force bool) (sourcev1.HelmChart, error) {
+func (r *HelmChartReconciler) reconcileFromTarballArtifact(ctx context.Context,
+	artifact sourcev1.Artifact, chart sourcev1.HelmChart, force bool) (sourcev1.HelmChart, error) {
 	// Create temporary working directory
 	tmpDir, err := ioutil.TempDir("", fmt.Sprintf("%s-%s-", chart.Namespace, chart.Name))
 	if err != nil {
@@ -390,8 +404,8 @@ func (r *HelmChartReconciler) reconcileFromGitRepository(ctx context.Context,
 	}
 	defer os.RemoveAll(tmpDir)
 
-	// Open GitRepository artifact file and untar files into working directory
-	f, err := os.Open(r.Storage.LocalPath(*repository.GetArtifact()))
+	// Open the tarball artifact file and untar files into working directory
+	f, err := os.Open(r.Storage.LocalPath(artifact))
 	if err != nil {
 		err = fmt.Errorf("artifact open error: %w", err)
 		return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
@@ -418,12 +432,12 @@ func (r *HelmChartReconciler) reconcileFromGitRepository(ctx context.Context,
 	}
 
 	// Return early if the revision is still the same as the current chart artifact
-	artifact := r.Storage.NewArtifactFor(chart.Kind, chart.ObjectMeta.GetObjectMeta(), chartMetadata.Version,
+	chartArtifact := r.Storage.NewArtifactFor(chart.Kind, chart.ObjectMeta.GetObjectMeta(), chartMetadata.Version,
 		fmt.Sprintf("%s-%s.tgz", chartMetadata.Name, chartMetadata.Version))
 	if !force && chart.GetArtifact() != nil && chart.GetArtifact().Revision == chartMetadata.Version {
-		if artifact.URL != repository.GetArtifact().URL {
-			r.Storage.SetArtifactURL(repository.GetArtifact())
-			repository.Status.URL = r.Storage.SetHostname(repository.Status.URL)
+		if chartArtifact.URL != artifact.URL {
+			r.Storage.SetArtifactURL(&chartArtifact)
+			chart.Status.URL = r.Storage.SetHostname(chart.Status.URL)
 		}
 		return chart, nil
 	}
@@ -436,14 +450,14 @@ func (r *HelmChartReconciler) reconcileFromGitRepository(ctx context.Context,
 	}
 
 	// Ensure artifact directory exists
-	err = r.Storage.MkdirAll(artifact)
+	err = r.Storage.MkdirAll(chartArtifact)
 	if err != nil {
 		err = fmt.Errorf("unable to create artifact directory: %w", err)
 		return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
 	}
 
 	// Acquire a lock for the artifact
-	unlock, err := r.Storage.Lock(artifact)
+	unlock, err := r.Storage.Lock(chartArtifact)
 	if err != nil {
 		err = fmt.Errorf("unable to acquire lock: %w", err)
 		return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
@@ -466,49 +480,22 @@ func (r *HelmChartReconciler) reconcileFromGitRepository(ctx context.Context,
 		err = fmt.Errorf("failed to open chart package: %w", err)
 		return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
 	}
-	if err := r.Storage.Copy(&artifact, cf); err != nil {
+	if err := r.Storage.Copy(&chartArtifact, cf); err != nil {
 		cf.Close()
 		err = fmt.Errorf("failed to copy chart package to storage: %w", err)
 		return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
 	}
 	cf.Close()
 
 	// Update symlink
-	cUrl, err := r.Storage.Symlink(artifact, fmt.Sprintf("%s-latest.tgz", chartMetadata.Name))
+	cUrl, err := r.Storage.Symlink(chartArtifact, fmt.Sprintf("%s-latest.tgz", chartMetadata.Name))
 	if err != nil {
 		err = fmt.Errorf("storage error: %w", err)
 		return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
 	}
 
-	message := fmt.Sprintf("Fetched and packaged revision: %s", artifact.Revision)
-	return sourcev1.HelmChartReady(chart, artifact, cUrl, sourcev1.ChartPackageSucceededReason, message), nil
-}
-
-// getGitRepositoryWithArtifact attempts to get the GitRepository for the given
-// chart. It returns an error if the v1alpha1.GitRepository could not be retrieved
-// or does not have an artifact.
-func (r *HelmChartReconciler) getGitRepositoryWithArtifact(ctx context.Context, chart sourcev1.HelmChart) (sourcev1.GitRepository, error) {
-	if chart.Spec.SourceRef.Name == "" {
-		return sourcev1.GitRepository{}, fmt.Errorf("no GitRepository reference given")
-	}
-
-	name := types.NamespacedName{
-		Namespace: chart.GetNamespace(),
-		Name:      chart.Spec.SourceRef.Name,
-	}
-
-	var repository sourcev1.GitRepository
-	err := r.Client.Get(ctx, name, &repository)
-	if err != nil {
-		err = fmt.Errorf("failed to get GitRepository '%s': %w", name, err)
-		return repository, err
-	}
-
-	if repository.GetArtifact() == nil {
-		err = fmt.Errorf("no artifact found for GitRepository '%s'", repository.Name)
-	}
-
-	return repository, err
+	message := fmt.Sprintf("Fetched and packaged revision: %s", chartArtifact.Revision)
+	return sourcev1.HelmChartReady(chart, chartArtifact, cUrl, sourcev1.ChartPackageSucceededReason, message), nil
 }
 
 // resetStatus returns a modified v1alpha1.HelmChart and a boolean indicating

controllers/helmchart_controller_test.go

@@ -152,7 +152,7 @@ var _ = Describe("HelmChartReconciler", func() {
 				_ = k8sClient.Get(context.Background(), key, updated)
 				for _, c := range updated.Status.Conditions {
 					if c.Reason == sourcev1.ChartPullFailedReason &&
-						strings.Contains(c.Message, "failed to get HelmRepository") {
+						strings.Contains(c.Message, "failed to retrieve source") {
 						return true
 					}
 				}