Add ExternalArtifact feature gate

Signed-off-by: Stefan Prodan <[email protected]>

Author: stefanprodan

config/default/kustomization.yaml

@@ -2,8 +2,8 @@ apiVersion: kustomize.config.k8s.io/v1beta1
 kind: Kustomization
 namespace: helm-system
 resources:
-- https://github.com/fluxcd/source-controller/releases/download/v1.7.0-rc.2/source-controller.crds.yaml
-- https://github.com/fluxcd/source-controller/releases/download/v1.7.0-rc.2/source-controller.deployment.yaml
+- https://github.com/fluxcd/source-controller/releases/download/v1.7.0-rc.3/source-controller.crds.yaml
+- https://github.com/fluxcd/source-controller/releases/download/v1.7.0-rc.3/source-controller.deployment.yaml
 - ../crd
 - ../rbac
 - ../manager

docs/spec/v2/helmreleases.md

@@ -205,11 +205,16 @@ HelmRelease object.
 
 ### Chart reference
 
-`.spec.chartRef` is an optional field used to refer to an [OCIRepository resource](https://fluxcd.io/flux/components/source/ocirepositories/) or a [HelmChart resource](https://fluxcd.io/flux/components/source/helmcharts/)
-from which to fetch the Helm chart. The chart is fetched by the controller with the
-information provided by `.status.artifact` of the referenced resource.
+`.spec.chartRef` is an optional field used to refer to the Source object which has an
+Artifact containing the Helm chart. It has two required fields:
 
-For a referenced resource of `kind OCIRepository`, the chart version of the last
+- `kind`: The Kind of the referred Source object. Supported Source types:
+  + [OCIRepository](https://fluxcd.io/flux/components/source/ocirepositories/)
+  + [HelmChart](https://fluxcd.io/flux/components/source/helmcharts/)
+  + [ExternalArtifact](https://fluxcd.io/flux/components/source/externalartifacts/) (requires `--feature-gates=ExternalArtifact=true` flag)
+- `name`: The Name of the referred Source object.
+
+For a referenced resource of kind `OCIRepository`, the chart version of the last
 release attempt is reported in `.status.lastAttemptedRevision`. The version is in
 the format `<version>+<digest[0:12]>`. The digest of the OCI artifact is appended
 to the version to ensure that a change in the artifact content triggers a new release.

go.mod

@@ -28,7 +28,7 @@ require (
 	github.com/fluxcd/pkg/runtime v0.83.0
 	github.com/fluxcd/pkg/ssa v0.53.0
 	github.com/fluxcd/pkg/testserver v0.13.0
-	github.com/fluxcd/source-controller/api v1.7.0-rc.2
+	github.com/fluxcd/source-controller/api v1.7.0-rc.3
 	github.com/go-logr/logr v1.4.3
 	github.com/google/cel-go v0.26.1
 	github.com/google/go-cmp v0.7.0

go.sum

@@ -166,8 +166,8 @@ github.com/fluxcd/pkg/ssa v0.53.0 h1:EtKFAYWXohIGkzPhIrv8NbV5zYr4iZHY6DaNxMR9+bU
 github.com/fluxcd/pkg/ssa v0.53.0/go.mod h1:0IZxnnH8XkDkFzWjoMJsFEwPIWPOk3Gc/WR5+gT/KgI=
 github.com/fluxcd/pkg/testserver v0.13.0 h1:xEpBcEYtD7bwvZ+i0ZmChxKkDo/wfQEV3xmnzVybSSg=
 github.com/fluxcd/pkg/testserver v0.13.0/go.mod h1:akRYv3FLQUsme15na9ihECRG6hBuqni4XEY9W8kzs8E=
-github.com/fluxcd/source-controller/api v1.7.0-rc.2 h1:ny21QMsZ1Gs5t5Rx7Pd1s0xc5UT7B4hGySzX+mhWHnw=
-github.com/fluxcd/source-controller/api v1.7.0-rc.2/go.mod h1:sbJibK4Ik+2AuTRRLXPA+n2u6nLUIGaxC07ava+RqeM=
+github.com/fluxcd/source-controller/api v1.7.0-rc.3 h1:+9cd//77LAgp0XRe97CXUaPnu78jsRNWTXq95GHGhgc=
+github.com/fluxcd/source-controller/api v1.7.0-rc.3/go.mod h1:sbJibK4Ik+2AuTRRLXPA+n2u6nLUIGaxC07ava+RqeM=
 github.com/foxcpp/go-mockdns v1.1.0 h1:jI0rD8M0wuYAxL7r/ynTrCQQq0BVqfB99Vgk7DlmewI=
 github.com/foxcpp/go-mockdns v1.1.0/go.mod h1:IhLeSFGed3mJIAXPH2aiRQB+kqz7oqu8ld2qVbOu7Wk=
 github.com/frankban/quicktest v1.14.6 h1:7Xjx+VpznH+oBnejlPUj8oUpdxnVs4f8XU8WnHkI4W8=

internal/controller/helmrelease_controller.go

@@ -87,19 +87,26 @@ type HelmReleaseReconciler struct {
 	kuberecorder.EventRecorder
 	helper.Metrics
 
-	GetClusterConfig func() (*rest.Config, error)
-	ClientOpts       runtimeClient.Options
-	KubeConfigOpts   runtimeClient.KubeConfigOptions
-	APIReader        client.Reader
-	TokenCache       *cache.TokenCache
-
-	FieldManager               string
-	DefaultServiceAccount      string
-	DisableChartDigestTracking bool
-	AdditiveCELDependencyCheck bool
+	// Kubernetes configuration
+
+	FieldManager          string
+	DefaultServiceAccount string
+	GetClusterConfig      func() (*rest.Config, error)
+	ClientOpts            runtimeClient.Options
+	KubeConfigOpts        runtimeClient.KubeConfigOptions
+	APIReader             client.Reader
+	TokenCache            *cache.TokenCache
+
+	// Retry and requeue configuration
+
+	DependencyRequeueInterval time.Duration
+	ArtifactFetchRetries      int
 
-	requeueDependency    time.Duration
-	artifactFetchRetries int
+	// Feature gates
+
+	AdditiveCELDependencyCheck bool
+	AllowExternalArtifact      bool
+	DisableChartDigestTracking bool
 }
 
 var (
@@ -221,14 +228,14 @@ func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context, patchHelpe
 
 			// Retry on transient errors.
 			msg := fmt.Sprintf("dependencies do not meet ready condition (%s): retrying in %s",
-				err.Error(), r.requeueDependency.String())
+				err.Error(), r.DependencyRequeueInterval.String())
 			conditions.MarkFalse(obj, meta.ReadyCondition, v2.DependencyNotReadyReason, "%s", err)
 			r.Eventf(obj, corev1.EventTypeNormal, v2.DependencyNotReadyReason, err.Error())
 			log.Info(msg)
 
 			// Exponential backoff would cause execution to be prolonged too much,
 			// instead we requeue on a fixed interval.
-			return ctrl.Result{RequeueAfter: r.requeueDependency}, errWaitForDependency
+			return ctrl.Result{RequeueAfter: r.DependencyRequeueInterval}, errWaitForDependency
 		}
 
 		log.Info("all dependencies are ready")
@@ -268,7 +275,7 @@ func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context, patchHelpe
 		conditions.MarkFalse(obj, meta.ReadyCondition, "SourceNotReady", "%s", msg)
 		// Do not requeue immediately, when the artifact is created
 		// the watcher should trigger a reconciliation.
-		return jitter.JitteredRequeueInterval(ctrl.Result{RequeueAfter: r.requeueDependency}), errWaitForChart
+		return jitter.JitteredRequeueInterval(ctrl.Result{RequeueAfter: r.DependencyRequeueInterval}), errWaitForChart
 	}
 	// Remove any stale corresponding Ready=False condition with Unknown.
 	if conditions.HasAnyReason(obj, meta.ReadyCondition, "SourceNotReady") {
@@ -293,13 +300,13 @@ func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context, patchHelpe
 	}
 
 	// Load chart from artifact.
-	loadedChart, err := loader.SecureLoadChartFromURL(loader.NewRetryableHTTPClient(ctx, r.artifactFetchRetries), source.GetArtifact().URL, source.GetArtifact().Digest)
+	loadedChart, err := loader.SecureLoadChartFromURL(loader.NewRetryableHTTPClient(ctx, r.ArtifactFetchRetries), source.GetArtifact().URL, source.GetArtifact().Digest)
 	if err != nil {
 		if errors.Is(err, loader.ErrFileNotFound) {
-			msg := fmt.Sprintf("Source not ready: artifact not found. Retrying in %s", r.requeueDependency.String())
+			msg := fmt.Sprintf("Source not ready: artifact not found. Retrying in %s", r.DependencyRequeueInterval.String())
 			conditions.MarkFalse(obj, meta.ReadyCondition, v2.ArtifactFailedReason, "%s", msg)
 			log.Info(msg)
-			return ctrl.Result{RequeueAfter: r.requeueDependency}, errWaitForDependency
+			return ctrl.Result{RequeueAfter: r.DependencyRequeueInterval}, errWaitForDependency
 		}
 
 		conditions.MarkFalse(obj, meta.ReadyCondition, v2.ArtifactFailedReason, "Could not load chart: %s", err)
@@ -827,6 +834,13 @@ func (r *HelmReleaseReconciler) getSourceFromExternalArtifact(ctx context.Contex
 		return nil, err
 	}
 
+	// Check if ExternalArtifact kind is allowed.
+	if obj.Spec.ChartRef.Kind == sourcev1.ExternalArtifactKind && !r.AllowExternalArtifact {
+		return nil, acl.AccessDeniedError(
+			fmt.Sprintf("can't access '%s/%s/%s', %s feature gate is disabled",
+				obj.Spec.ChartRef.Kind, namespace, name, features.ExternalArtifact))
+	}
+
 	or := sourcev1.ExternalArtifact{}
 	if err := r.Client.Get(ctx, sourceRef, &or); err != nil {
 		return nil, err

internal/controller/helmrelease_controller_test.go

@@ -107,14 +107,14 @@ func TestHelmReleaseReconciler_reconcileRelease(t *testing.T) {
 				WithStatusSubresource(&v2.HelmRelease{}).
 				WithObjects(dependency, obj).
 				Build(),
-			EventRecorder:     record.NewFakeRecorder(32),
-			requeueDependency: 5 * time.Second,
+			EventRecorder:             record.NewFakeRecorder(32),
+			DependencyRequeueInterval: 5 * time.Second,
 		}
 		r.APIReader = r.Client
 
 		res, err := r.reconcileRelease(context.TODO(), patch.NewSerialPatcher(obj, r.Client), obj)
 		g.Expect(err).To(Equal(errWaitForDependency))
-		g.Expect(res.RequeueAfter).To(Equal(r.requeueDependency))
+		g.Expect(res.RequeueAfter).To(Equal(r.DependencyRequeueInterval))
 
 		g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{
 			*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, ""),
@@ -235,7 +235,7 @@ func TestHelmReleaseReconciler_reconcileRelease(t *testing.T) {
 
 		res, err := r.reconcileRelease(context.TODO(), patch.NewSerialPatcher(obj, r.Client), obj)
 		g.Expect(err).To(Equal(errWaitForChart))
-		g.Expect(res.RequeueAfter).To(Equal(r.requeueDependency))
+		g.Expect(res.RequeueAfter).To(Equal(r.DependencyRequeueInterval))
 
 		g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{
 			*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, ""),
@@ -292,7 +292,7 @@ func TestHelmReleaseReconciler_reconcileRelease(t *testing.T) {
 
 		res, err := r.reconcileRelease(context.TODO(), patch.NewSerialPatcher(obj, r.Client), obj)
 		g.Expect(err).To(Equal(errWaitForChart))
-		g.Expect(res.RequeueAfter).To(Equal(r.requeueDependency))
+		g.Expect(res.RequeueAfter).To(Equal(r.DependencyRequeueInterval))
 
 		g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{
 			*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, ""),
@@ -400,13 +400,13 @@ func TestHelmReleaseReconciler_reconcileRelease(t *testing.T) {
 				WithStatusSubresource(&v2.HelmRelease{}).
 				WithObjects(chart, obj).
 				Build(),
-			requeueDependency: 10 * time.Second,
+			DependencyRequeueInterval: 10 * time.Second,
 		}
 		r.APIReader = r.Client
 
 		res, err := r.reconcileRelease(context.TODO(), patch.NewSerialPatcher(obj, r.Client), obj)
 		g.Expect(err).To(Equal(errWaitForDependency))
-		g.Expect(res.RequeueAfter).To(Equal(r.requeueDependency))
+		g.Expect(res.RequeueAfter).To(Equal(r.DependencyRequeueInterval))
 
 		g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{
 			*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, ""),
@@ -1112,7 +1112,7 @@ func TestHelmReleaseReconciler_reconcileReleaseFromHelmChartSource(t *testing.T)
 
 		res, err := r.reconcileRelease(context.TODO(), patch.NewSerialPatcher(obj, r.Client), obj)
 		g.Expect(err).To(Equal(errWaitForChart))
-		g.Expect(res.RequeueAfter).To(Equal(r.requeueDependency))
+		g.Expect(res.RequeueAfter).To(Equal(r.DependencyRequeueInterval))
 
 		g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{
 			*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, ""),
@@ -1166,13 +1166,13 @@ func TestHelmReleaseReconciler_reconcileReleaseFromHelmChartSource(t *testing.T)
 				WithStatusSubresource(&v2.HelmRelease{}).
 				WithObjects(chart, obj).
 				Build(),
-			requeueDependency: 10 * time.Second,
-			EventRecorder:     record.NewFakeRecorder(32),
+			DependencyRequeueInterval: 10 * time.Second,
+			EventRecorder:             record.NewFakeRecorder(32),
 		}
 
 		res, err := r.reconcileRelease(context.TODO(), patch.NewSerialPatcher(obj, r.Client), obj)
 		g.Expect(err).To(Equal(errWaitForDependency))
-		g.Expect(res.RequeueAfter).To(Equal(r.requeueDependency))
+		g.Expect(res.RequeueAfter).To(Equal(r.DependencyRequeueInterval))
 
 		g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{
 			*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, ""),
@@ -1246,13 +1246,13 @@ func TestHelmReleaseReconciler_reconcileReleaseFromHelmChartSource(t *testing.T)
 				WithStatusSubresource(&v2.HelmRelease{}).
 				WithObjects(chart, sharedChart, obj).
 				Build(),
-			requeueDependency: 10 * time.Second,
-			EventRecorder:     record.NewFakeRecorder(32),
+			DependencyRequeueInterval: 10 * time.Second,
+			EventRecorder:             record.NewFakeRecorder(32),
 		}
 
 		res, err := r.reconcileRelease(context.TODO(), patch.NewSerialPatcher(obj, r.Client), obj)
 		g.Expect(err).To(Equal(errWaitForDependency))
-		g.Expect(res.RequeueAfter).To(Equal(r.requeueDependency))
+		g.Expect(res.RequeueAfter).To(Equal(r.DependencyRequeueInterval))
 
 		g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{
 			*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, ""),
@@ -1579,7 +1579,7 @@ func TestHelmReleaseReconciler_reconcileReleaseFromOCIRepositorySource(t *testin
 
 		res, err := r.reconcileRelease(context.TODO(), patch.NewSerialPatcher(obj, r.Client), obj)
 		g.Expect(err).To(Equal(errWaitForChart))
-		g.Expect(res.RequeueAfter).To(Equal(r.requeueDependency))
+		g.Expect(res.RequeueAfter).To(Equal(r.DependencyRequeueInterval))
 
 		g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{
 			*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, ""),
@@ -1695,13 +1695,13 @@ func TestHelmReleaseReconciler_reconcileReleaseFromOCIRepositorySource(t *testin
 				WithStatusSubresource(&v2.HelmRelease{}).
 				WithObjects(ocirepo, obj).
 				Build(),
-			requeueDependency: 10 * time.Second,
-			EventRecorder:     record.NewFakeRecorder(32),
+			DependencyRequeueInterval: 10 * time.Second,
+			EventRecorder:             record.NewFakeRecorder(32),
 		}
 
 		res, err := r.reconcileRelease(context.TODO(), patch.NewSerialPatcher(obj, r.Client), obj)
 		g.Expect(err).To(Equal(errWaitForDependency))
-		g.Expect(res.RequeueAfter).To(Equal(r.requeueDependency))
+		g.Expect(res.RequeueAfter).To(Equal(r.DependencyRequeueInterval))
 
 		g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{
 			*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, ""),
@@ -1775,13 +1775,13 @@ func TestHelmReleaseReconciler_reconcileReleaseFromOCIRepositorySource(t *testin
 				WithStatusSubresource(&v2.HelmRelease{}).
 				WithObjects(chart, ocirepo, obj).
 				Build(),
-			requeueDependency: 10 * time.Second,
-			EventRecorder:     record.NewFakeRecorder(32),
+			DependencyRequeueInterval: 10 * time.Second,
+			EventRecorder:             record.NewFakeRecorder(32),
 		}
 
 		res, err := r.reconcileRelease(context.TODO(), patch.NewSerialPatcher(obj, r.Client), obj)
 		g.Expect(err).To(Equal(errWaitForDependency))
-		g.Expect(res.RequeueAfter).To(Equal(r.requeueDependency))
+		g.Expect(res.RequeueAfter).To(Equal(r.DependencyRequeueInterval))
 
 		g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{
 			*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, ""),

internal/controller/helmrelease_manager.go

@@ -19,7 +19,6 @@ package controller
 import (
 	"context"
 	"fmt"
-	"time"
 
 	"github.com/fluxcd/pkg/runtime/predicates"
 	sourcev1 "github.com/fluxcd/source-controller/api/v1"
@@ -38,10 +37,9 @@ import (
 )
 
 type HelmReleaseReconcilerOptions struct {
-	HTTPRetry                 int
-	DependencyRequeueInterval time.Duration
-	RateLimiter               workqueue.TypedRateLimiter[reconcile.Request]
-	WatchConfigsPredicate     predicate.Predicate
+	RateLimiter            workqueue.TypedRateLimiter[reconcile.Request]
+	WatchExternalArtifacts bool
+	WatchConfigsPredicate  predicate.Predicate
 }
 
 func (r *HelmReleaseReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opts HelmReleaseReconcilerOptions) error {
@@ -116,10 +114,7 @@ func (r *HelmReleaseReconciler) SetupWithManager(ctx context.Context, mgr ctrl.M
 		return err
 	}
 
-	r.requeueDependency = opts.DependencyRequeueInterval
-	r.artifactFetchRetries = opts.HTTPRetry
-
-	return ctrl.NewControllerManagedBy(mgr).
+	ctrlBuilder := ctrl.NewControllerManagedBy(mgr).
 		For(&v2.HelmRelease{}, builder.WithPredicates(
 			predicate.Or(predicate.GenerationChangedPredicate{}, predicates.ReconcileRequestedPredicate{}),
 		)).
@@ -133,11 +128,6 @@ func (r *HelmReleaseReconciler) SetupWithManager(ctx context.Context, mgr ctrl.M
 			handler.EnqueueRequestsFromMapFunc(r.requestsForOCIRepositoryChange),
 			builder.WithPredicates(intpredicates.SourceRevisionChangePredicate{}),
 		).
-		Watches(
-			&sourcev1.ExternalArtifact{},
-			handler.EnqueueRequestsFromMapFunc(r.requestsForExternalArtifactChange),
-			builder.WithPredicates(intpredicates.SourceRevisionChangePredicate{}),
-		).
 		WatchesMetadata(
 			&corev1.ConfigMap{},
 			handler.EnqueueRequestsFromMapFunc(r.requestsForConfigDependency(indexConfigMap)),
@@ -147,9 +137,16 @@ func (r *HelmReleaseReconciler) SetupWithManager(ctx context.Context, mgr ctrl.M
 			&corev1.Secret{},
 			handler.EnqueueRequestsFromMapFunc(r.requestsForConfigDependency(indexSecret)),
 			builder.WithPredicates(predicate.ResourceVersionChangedPredicate{}, opts.WatchConfigsPredicate),
-		).
-		WithOptions(controller.Options{
-			RateLimiter: opts.RateLimiter,
-		}).
-		Complete(r)
+		)
+
+	if opts.WatchExternalArtifacts {
+		ctrlBuilder = ctrlBuilder.Watches(
+			&sourcev1.ExternalArtifact{},
+			handler.EnqueueRequestsFromMapFunc(r.requestsForExternalArtifactChange),
+			builder.WithPredicates(intpredicates.SourceRevisionChangePredicate{}),
+		)
+	}
+
+	return ctrlBuilder.WithOptions(controller.Options{RateLimiter: opts.RateLimiter}).Complete(r)
+
 }