Add ExternalArtifact feature gate

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

Author: stefanprodan

docs/spec/v1/kustomizations.md

@@ -117,6 +117,7 @@ Artifact containing the YAML manifests. It has two required fields:
   + [GitRepository](https://github.com/fluxcd/source-controller/blob/main/docs/spec/v1/gitrepositories.md)
   + [OCIRepository](https://github.com/fluxcd/source-controller/blob/main/docs/spec/v1/ocirepositories.md)
   + [Bucket](https://github.com/fluxcd/source-controller/blob/main/docs/spec/v1/buckets.md)
+  + [ExternalArtifact](https://github.com/fluxcd/source-controller/blob/main/docs/spec/v1/externalartifacts.md) (requires `--feature-gates=ExternalArtifact=true` flag)
 - `name`: The Name of the referred Source object.
 
 #### Cross-namespace references

internal/controller/kustomization_acl_test.go

@@ -120,6 +120,7 @@ stringData:
 
 	t.Run("fails to reconcile from cross-namespace source", func(t *testing.T) {
 		reconciler.NoCrossNamespaceRefs = true
+		defer func() { reconciler.NoCrossNamespaceRefs = false }()
 
 		revision = "v2.0.0"
 		err = applyGitRepository(repositoryName, artifact, revision)
@@ -132,5 +133,6 @@ stringData:
 		}, timeout, time.Second).Should(BeTrue())
 
 		g.Expect(readyCondition.Reason).To(Equal(apiacl.AccessDeniedReason))
+		g.Expect(apimeta.IsStatusConditionTrue(resultK.Status.Conditions, meta.StalledCondition)).Should(BeTrue())
 	})
 }

internal/controller/kustomization_controller.go

@@ -89,26 +89,37 @@ type KustomizationReconciler struct {
 	kuberecorder.EventRecorder
 	runtimeCtrl.Metrics
 
-	artifactFetchRetries int
-	requeueDependency    time.Duration
-
-	Mapper                     apimeta.RESTMapper
-	APIReader                  client.Reader
-	ClusterReader              engine.ClusterReaderFactory
-	ControllerName             string
-	statusManager              string
-	NoCrossNamespaceRefs       bool
-	NoRemoteBases              bool
+	// Kubernetes options
+
+	APIReader      client.Reader
+	ClusterReader  engine.ClusterReaderFactory
+	ConcurrentSSA  int
+	ControllerName string
+	KubeConfigOpts runtimeClient.KubeConfigOptions
+	Mapper         apimeta.RESTMapper
+	StatusManager  string
+
+	// Multi-tenancy and security options
+
+	DefaultServiceAccount   string
+	DisallowedFieldManagers []string
+	NoCrossNamespaceRefs    bool
+	NoRemoteBases           bool
+	SOPSAgeSecret           string
+	TokenCache              *cache.TokenCache
+
+	// Retry and requeue options
+
+	ArtifactFetchRetries      int
+	DependencyRequeueInterval time.Duration
+
+	// Feature gates
+
+	AdditiveCELDependencyCheck bool
+	AllowExternalArtifact      bool
 	FailFast                   bool
-	DefaultServiceAccount      string
-	SOPSAgeSecret              string
-	KubeConfigOpts             runtimeClient.KubeConfigOptions
-	ConcurrentSSA              int
-	DisallowedFieldManagers    []string
-	StrictSubstitutions        bool
 	GroupChangeLog             bool
-	AdditiveCELDependencyCheck bool
-	TokenCache                 *cache.TokenCache
+	StrictSubstitutions        bool
 }
 
 func (r *KustomizationReconciler) Reconcile(ctx context.Context, req ctrl.Request) (result ctrl.Result, retErr error) {
@@ -207,9 +218,9 @@ func (r *KustomizationReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 
 		if acl.IsAccessDenied(err) {
 			conditions.MarkFalse(obj, meta.ReadyCondition, apiacl.AccessDeniedReason, "%s", err)
-			log.Error(err, "Access denied to cross-namespace source")
+			conditions.MarkStalled(obj, apiacl.AccessDeniedReason, "%s", err)
 			r.event(obj, "", "", eventv1.EventSeverityError, err.Error(), nil)
-			return ctrl.Result{RequeueAfter: obj.GetRetryInterval()}, nil
+			return ctrl.Result{}, reconcile.TerminalError(err)
 		}
 
 		// Retry with backoff on transient errors.
@@ -218,10 +229,10 @@ func (r *KustomizationReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 
 	// Requeue the reconciliation if the source artifact is not found.
 	if artifactSource.GetArtifact() == nil {
-		msg := fmt.Sprintf("Source artifact not found, retrying in %s", r.requeueDependency.String())
+		msg := fmt.Sprintf("Source artifact not found, retrying in %s", r.DependencyRequeueInterval.String())
 		conditions.MarkFalse(obj, meta.ReadyCondition, meta.ArtifactFailedReason, "%s", msg)
 		log.Info(msg)
-		return ctrl.Result{RequeueAfter: r.requeueDependency}, nil
+		return ctrl.Result{RequeueAfter: r.DependencyRequeueInterval}, nil
 	}
 	revision := artifactSource.GetArtifact().Revision
 	originRevision := getOriginRevision(artifactSource)
@@ -241,10 +252,10 @@ func (r *KustomizationReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 
 			// Retry on transient errors.
 			conditions.MarkFalse(obj, meta.ReadyCondition, meta.DependencyNotReadyReason, "%s", err)
-			msg := fmt.Sprintf("Dependencies do not meet ready condition, retrying in %s", r.requeueDependency.String())
+			msg := fmt.Sprintf("Dependencies do not meet ready condition, retrying in %s", r.DependencyRequeueInterval.String())
 			log.Info(msg)
 			r.event(obj, revision, originRevision, eventv1.EventSeverityInfo, msg, nil)
-			return ctrl.Result{RequeueAfter: r.requeueDependency}, nil
+			return ctrl.Result{RequeueAfter: r.DependencyRequeueInterval}, nil
 		}
 		log.Info("All dependencies are ready, proceeding with reconciliation")
 	}
@@ -254,10 +265,10 @@ func (r *KustomizationReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 
 	// Requeue at the specified retry interval if the artifact tarball is not found.
 	if errors.Is(reconcileErr, fetch.ErrFileNotFound) {
-		msg := fmt.Sprintf("Source is not ready, artifact not found, retrying in %s", r.requeueDependency.String())
+		msg := fmt.Sprintf("Source is not ready, artifact not found, retrying in %s", r.DependencyRequeueInterval.String())
 		conditions.MarkFalse(obj, meta.ReadyCondition, meta.ArtifactFailedReason, "%s", msg)
 		log.Info(msg)
-		return ctrl.Result{RequeueAfter: r.requeueDependency}, nil
+		return ctrl.Result{RequeueAfter: r.DependencyRequeueInterval}, nil
 	}
 
 	// Broadcast the reconciliation failure and requeue at the specified retry interval.
@@ -318,7 +329,7 @@ func (r *KustomizationReconciler) reconcile(
 	// Download artifact and extract files to the tmp dir.
 	fetcher := fetch.New(
 		fetch.WithLogger(ctrl.LoggerFrom(ctx)),
-		fetch.WithRetries(r.artifactFetchRetries),
+		fetch.WithRetries(r.ArtifactFetchRetries),
 		fetch.WithMaxDownloadSize(tar.UnlimitedUntarSize),
 		fetch.WithUntar(tar.WithMaxUntarSize(tar.UnlimitedUntarSize)),
 		fetch.WithHostnameOverwrite(os.Getenv("SOURCE_CONTROLLER_LOCALHOST")),
@@ -625,12 +636,20 @@ func (r *KustomizationReconciler) getSource(ctx context.Context,
 		Name:      obj.Spec.SourceRef.Name,
 	}
 
+	// Check if cross-namespace references are allowed.
 	if r.NoCrossNamespaceRefs && sourceNamespace != obj.GetNamespace() {
 		return src, acl.AccessDeniedError(
 			fmt.Sprintf("can't access '%s/%s', cross-namespace references have been blocked",
 				obj.Spec.SourceRef.Kind, namespacedName))
 	}
 
+	// Check if ExternalArtifact kind is allowed.
+	if obj.Spec.SourceRef.Kind == sourcev1.ExternalArtifactKind && !r.AllowExternalArtifact {
+		return src, acl.AccessDeniedError(
+			fmt.Sprintf("can't access '%s/%s', %s feature gate is disabled",
+				obj.Spec.SourceRef.Kind, namespacedName, sourcev1.ExternalArtifactKind))
+	}
+
 	switch obj.Spec.SourceRef.Kind {
 	case sourcev1.OCIRepositoryKind:
 		var repository sourcev1.OCIRepository
@@ -1204,7 +1223,7 @@ func (r *KustomizationReconciler) patch(ctx context.Context,
 	patchOpts = append(patchOpts,
 		patch.WithOwnedConditions{Conditions: ownedConditions},
 		patch.WithForceOverwriteConditions{},
-		patch.WithFieldOwner(r.statusManager),
+		patch.WithFieldOwner(r.StatusManager),
 	)
 
 	// Patch the object status, conditions and finalizers.

internal/controller/kustomization_externalartifact_test.go

@@ -24,6 +24,7 @@ import (
 	"testing"
 	"time"
 
+	apiacl "github.com/fluxcd/pkg/apis/acl"
 	"github.com/fluxcd/pkg/apis/meta"
 	"github.com/fluxcd/pkg/testserver"
 	sourcev1 "github.com/fluxcd/source-controller/api/v1"
@@ -41,6 +42,7 @@ func TestKustomizationReconciler_ExternalArtifact(t *testing.T) {
 	g := NewWithT(t)
 	id := "ea-" + randStringRunes(5)
 	revision := "v1.0.0"
+	reconciler.AllowExternalArtifact = true
 
 	err := createNamespace(id)
 	g.Expect(err).NotTo(HaveOccurred(), "failed to create test namespace")
@@ -129,6 +131,7 @@ stringData:
 
 	t.Run("watches for external artifact revision change", func(t *testing.T) {
 		newRev := "v2.0.0"
+
 		err = applyExternalArtifact(eaName, artifact, newRev)
 		g.Expect(err).NotTo(HaveOccurred())
 
@@ -142,6 +145,27 @@ stringData:
 		g.Expect(resultK.Status.History[0].LastReconciledStatus).To(Equal(meta.ReconciliationSucceededReason))
 		g.Expect(resultK.Status.History[0].Metadata).To(ContainElements(newRev))
 	})
+
+	t.Run("fails when external artifact feature gate is disable", func(t *testing.T) {
+		newRev := "v3.0.0"
+		reconciler.AllowExternalArtifact = false
+
+		err = applyExternalArtifact(eaName, artifact, newRev)
+		g.Expect(err).NotTo(HaveOccurred())
+
+		g.Eventually(func() bool {
+			_ = k8sClient.Get(context.Background(), client.ObjectKeyFromObject(kustomization), resultK)
+			readyCondition = apimeta.FindStatusCondition(resultK.Status.Conditions, meta.ReadyCondition)
+			return apimeta.IsStatusConditionFalse(resultK.Status.Conditions, meta.ReadyCondition)
+		}, timeout, time.Second).Should(BeTrue())
+
+		g.Expect(readyCondition.Reason).To(Equal(apiacl.AccessDeniedReason))
+		g.Expect(apimeta.IsStatusConditionTrue(resultK.Status.Conditions, meta.StalledCondition)).Should(BeTrue())
+
+		events := getEvents(resultK.GetName(), nil)
+		g.Expect(events[len(events)-1].Reason).To(BeIdenticalTo(apiacl.AccessDeniedReason))
+		g.Expect(events[len(events)-1].Message).To(ContainSubstring("feature gate is disabled"))
+	})
 }
 
 func applyExternalArtifact(objKey client.ObjectKey, artifactName string, revision string) error {

internal/controller/kustomization_indexers.go

@@ -20,14 +20,15 @@ import (
 	"context"
 	"fmt"
 
-	"github.com/fluxcd/pkg/apis/meta"
-	"github.com/fluxcd/pkg/runtime/conditions"
-	"github.com/fluxcd/pkg/runtime/dependency"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
+	"github.com/fluxcd/pkg/apis/meta"
+	"github.com/fluxcd/pkg/runtime/conditions"
+	"github.com/fluxcd/pkg/runtime/dependency"
+
 	kustomizev1 "github.com/fluxcd/kustomize-controller/api/v1"
 )
 

internal/controller/kustomization_manager.go

@@ -19,10 +19,7 @@ package controller
 import (
 	"context"
 	"fmt"
-	"time"
 
-	"github.com/fluxcd/pkg/runtime/predicates"
-	sourcev1 "github.com/fluxcd/source-controller/api/v1"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/client-go/util/workqueue"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -33,15 +30,17 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
+	"github.com/fluxcd/pkg/runtime/predicates"
+	sourcev1 "github.com/fluxcd/source-controller/api/v1"
+
 	kustomizev1 "github.com/fluxcd/kustomize-controller/api/v1"
 )
 
 // KustomizationReconcilerOptions contains options for the KustomizationReconciler.
 type KustomizationReconcilerOptions struct {
-	HTTPRetry                 int
-	DependencyRequeueInterval time.Duration
-	RateLimiter               workqueue.TypedRateLimiter[reconcile.Request]
-	WatchConfigsPredicate     predicate.Predicate
+	RateLimiter            workqueue.TypedRateLimiter[reconcile.Request]
+	WatchConfigsPredicate  predicate.Predicate
+	WatchExternalArtifacts bool
 }
 
 // SetupWithManager sets up the controller with the Manager.
@@ -57,12 +56,6 @@ func (r *KustomizationReconciler) SetupWithManager(ctx context.Context, mgr ctrl
 		indexSecret           = ".metadata.secret"
 	)
 
-	// Index the Kustomizations by the ExternalArtifact references they (may) point at.
-	if err := mgr.GetCache().IndexField(ctx, &kustomizev1.Kustomization{}, indexExternalArtifact,
-		r.indexBy(sourcev1.ExternalArtifactKind)); err != nil {
-		return fmt.Errorf("failed creating index %s: %w", indexExternalArtifact, err)
-	}
-
 	// Index the Kustomizations by the OCIRepository references they (may) point at.
 	if err := mgr.GetCache().IndexField(ctx, &kustomizev1.Kustomization{}, indexOCIRepository,
 		r.indexBy(sourcev1.OCIRepositoryKind)); err != nil {
@@ -81,6 +74,14 @@ func (r *KustomizationReconciler) SetupWithManager(ctx context.Context, mgr ctrl
 		return fmt.Errorf("failed creating index %s: %w", indexBucket, err)
 	}
 
+	// Index the Kustomizations by the ExternalArtifact references they (may) point at (if enabled).
+	if opts.WatchExternalArtifacts {
+		if err := mgr.GetCache().IndexField(ctx, &kustomizev1.Kustomization{}, indexExternalArtifact,
+			r.indexBy(sourcev1.ExternalArtifactKind)); err != nil {
+			return fmt.Errorf("failed creating index %s: %w", indexExternalArtifact, err)
+		}
+	}
+
 	// Index the Kustomization by the ConfigMap references they point to.
 	if err := mgr.GetFieldIndexer().IndexField(ctx, &kustomizev1.Kustomization{}, indexConfigMap,
 		func(o client.Object) []string {
@@ -128,19 +129,10 @@ func (r *KustomizationReconciler) SetupWithManager(ctx context.Context, mgr ctrl
 		return fmt.Errorf("failed creating index %s: %w", indexSecret, err)
 	}
 
-	r.requeueDependency = opts.DependencyRequeueInterval
-	r.statusManager = fmt.Sprintf("gotk-%s", r.ControllerName)
-	r.artifactFetchRetries = opts.HTTPRetry
-
-	return ctrl.NewControllerManagedBy(mgr).
+	ctrlBuilder := ctrl.NewControllerManagedBy(mgr).
 		For(&kustomizev1.Kustomization{}, builder.WithPredicates(
 			predicate.Or(predicate.GenerationChangedPredicate{}, predicates.ReconcileRequestedPredicate{}),
 		)).
-		Watches(
-			&sourcev1.ExternalArtifact{},
-			handler.EnqueueRequestsFromMapFunc(r.requestsForRevisionChangeOf(indexExternalArtifact)),
-			builder.WithPredicates(SourceRevisionChangePredicate{}),
-		).
 		Watches(
 			&sourcev1.OCIRepository{},
 			handler.EnqueueRequestsFromMapFunc(r.requestsForRevisionChangeOf(indexOCIRepository)),
@@ -165,9 +157,15 @@ func (r *KustomizationReconciler) SetupWithManager(ctx context.Context, mgr ctrl
 			&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.requestsForRevisionChangeOf(indexExternalArtifact)),
+			builder.WithPredicates(SourceRevisionChangePredicate{}),
+		)
+	}
+
+	return ctrlBuilder.WithOptions(controller.Options{RateLimiter: opts.RateLimiter}).Complete(r)
 }

internal/controller/suite_test.go

@@ -174,19 +174,21 @@ func TestMain(m *testing.M) {
 		kstatusInProgressCheck = kcheck.NewInProgressChecker(testEnv.Client)
 		kstatusInProgressCheck.DisableFetch = true
 		reconciler = &KustomizationReconciler{
-			ControllerName:          controllerName,
-			Client:                  testEnv,
-			Mapper:                  testEnv.GetRESTMapper(),
-			APIReader:               testEnv,
-			EventRecorder:           testEnv.GetEventRecorderFor(controllerName),
-			Metrics:                 testMetricsH,
-			ConcurrentSSA:           4,
-			DisallowedFieldManagers: []string{overrideManagerName},
-			SOPSAgeSecret:           sopsAgeSecret,
+			ControllerName:            controllerName,
+			StatusManager:             fmt.Sprintf("gotk-%s", controllerName),
+			Client:                    testEnv,
+			Mapper:                    testEnv.GetRESTMapper(),
+			APIReader:                 testEnv,
+			EventRecorder:             testEnv.GetEventRecorderFor(controllerName),
+			Metrics:                   testMetricsH,
+			DependencyRequeueInterval: 2 * time.Second,
+			ConcurrentSSA:             4,
+			DisallowedFieldManagers:   []string{overrideManagerName},
+			SOPSAgeSecret:             sopsAgeSecret,
 		}
 		if err := (reconciler).SetupWithManager(ctx, testEnv, KustomizationReconcilerOptions{
-			DependencyRequeueInterval: 2 * time.Second,
-			WatchConfigsPredicate:     predicate.Not(predicate.Funcs{}),
+			WatchConfigsPredicate:  predicate.Not(predicate.Funcs{}),
+			WatchExternalArtifacts: true,
 		}); err != nil {
 			panic(fmt.Sprintf("Failed to start KustomizationReconciler: %v", err))
 		}

internal/features/features.go

@@ -56,6 +56,9 @@ const (
 	// should be additive, meaning that the built-in readiness check will
 	// be added to the user-defined CEL expressions.
 	AdditiveCELDependencyCheck = "AdditiveCELDependencyCheck"
+
+	// ExternalArtifact controls whether the ExternalArtifact source type is enabled.
+	ExternalArtifact = "ExternalArtifact"
 )
 
 var features = map[string]bool{
@@ -77,6 +80,9 @@ var features = map[string]bool{
 	// AdditiveCELDependencyCheck
 	// opt-in from v1.7
 	AdditiveCELDependencyCheck: false,
+	// ExternalArtifact
+	// opt-in from v1.7
+	ExternalArtifact: false,
 }
 
 func init() {