Static helmrepository OCI

Remove the HelmRepositoryOCI reconciler and make HelmRepository of type
OCI static. The existing HelmRepository OCI objects are migrated to
static object by removing their finalizers and status. New
HelmRepository OCI objects go through one time migration to remove the
status. These are not reconciled again, unless the type is changed to
default. On type switching from HelmRepository default to OCI, the
finalizer, status and artifact are removed to make the object static. On
switching from OCI to default, a complete reconciliation of
HelmRepository takes place to build artifact and add status and
finalizer.

The HelmRepository .spec.url has a new validation to check the URL
scheme. This is to add some validation to HelmRepository OCI since it's
not backed by a reconciler for full validation.

Add HelmRepositoryOCIMigrationPredicate predicate to detect and allow
reconciliation of HelmRepository OCI objects that need migration. The
other predicates that filtered the HelmRepository events based on the
type have been removed as all the HelmRepositories will now be
reconciled by a single reconciler. HelmRepositoryOCIMigrationPredicate
readily allows non-OCI objects and only checks if a migration is needed
for OCI type object.

Add controller tests for different migration scenarios.

Signed-off-by: Sunny <[email protected]>

Author: darkowlzz

api/v1beta2/helmrepository_types.go

@@ -44,6 +44,7 @@ const (
 type HelmRepositorySpec struct {
 	// URL of the Helm repository, a valid URL contains at least a protocol and
 	// host.
+	// +kubebuilder:validation:Pattern="^(http|https|oci)://.*$"
 	// +required
 	URL string `json:"url"`
 

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

@@ -373,6 +373,7 @@ spec:
               url:
                 description: URL of the Helm repository, a valid URL contains at least
                   a protocol and host.
+                pattern: ^(http|https|oci)://.*$
                 type: string
             required:
             - interval

hack/ci/e2e.sh

@@ -75,7 +75,6 @@ kubectl -n source-system rollout status deploy/source-controller --timeout=1m
 kubectl -n source-system wait gitrepository/gitrepository-sample --for=condition=ready --timeout=1m
 kubectl -n source-system wait ocirepository/ocirepository-sample --for=condition=ready --timeout=1m
 kubectl -n source-system wait helmrepository/helmrepository-sample --for=condition=ready --timeout=1m
-kubectl -n source-system wait helmrepository/helmrepository-sample-oci --for=condition=ready --timeout=1m
 kubectl -n source-system wait helmchart/helmchart-sample --for=condition=ready --timeout=1m
 kubectl -n source-system wait helmchart/helmchart-sample-oci --for=condition=ready --timeout=1m
 kubectl -n source-system delete -f "${ROOT_DIR}/config/samples"
@@ -145,7 +144,6 @@ kubectl -n source-system wait gitrepository/large-repo --for=condition=ready --t
 
 echo "Run HelmChart from OCI registry tests"
 kubectl -n source-system apply -f "${ROOT_DIR}/config/testdata/helmchart-from-oci/source.yaml"
-kubectl -n source-system wait helmrepository/podinfo --for=condition=ready --timeout=1m
 kubectl -n source-system wait helmchart/podinfo --for=condition=ready --timeout=1m
 kubectl -n source-system wait helmchart/podinfo-keyless --for=condition=ready --timeout=1m
 

internal/controller/helmchart_controller.go

@@ -18,6 +18,7 @@ package controller
 
 import (
 	"context"
+	"crypto/tls"
 	"errors"
 	"fmt"
 	"net/url"
@@ -139,6 +140,12 @@ type HelmChartReconciler struct {
 	patchOptions []patch.Option
 }
 
+// RegistryClientGeneratorFunc is a function that returns a registry client
+// and an optional file name.
+// The file is used to store the registry client credentials.
+// The caller is responsible for deleting the file.
+type RegistryClientGeneratorFunc func(tlsConfig *tls.Config, isLogin bool) (*helmreg.Client, string, error)
+
 func (r *HelmChartReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager) error {
 	return r.SetupWithManagerAndOptions(ctx, mgr, HelmChartReconcilerOptions{})
 }

internal/controller/helmchart_controller_test.go

@@ -197,7 +197,7 @@ func TestHelmChartReconciler_Reconcile(t *testing.T) {
 		{
 			name: "Stalling on invalid repository URL",
 			beforeFunc: func(repository *helmv1.HelmRepository) {
-				repository.Spec.URL = "://unsupported" // Invalid URL
+				repository.Spec.URL = "https://unsupported/foo://" // Invalid URL
 			},
 			assertFunc: func(g *WithT, obj *helmv1.HelmChart, _ *helmv1.HelmRepository) {
 				key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace}

internal/controller/helmrepository_controller.go

@@ -22,11 +22,13 @@ import (
 	"errors"
 	"fmt"
 	"net/url"
+	"strings"
 	"time"
 
 	"github.com/docker/go-units"
 	"github.com/opencontainers/go-digest"
 	helmgetter "helm.sh/helm/v3/pkg/getter"
+	helmreg "helm.sh/helm/v3/pkg/registry"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	kuberecorder "k8s.io/client-go/tools/record"
@@ -138,10 +140,7 @@ func (r *HelmRepositoryReconciler) SetupWithManagerAndOptions(mgr ctrl.Manager,
 		For(&helmv1.HelmRepository{}).
 		WithEventFilter(
 			predicate.And(
-				predicate.Or(
-					intpredicates.HelmRepositoryTypePredicate{RepositoryType: helmv1.HelmRepositoryTypeDefault},
-					intpredicates.HelmRepositoryTypePredicate{RepositoryType: ""},
-				),
+				intpredicates.HelmRepositoryOCIMigrationPredicate{},
 				predicate.Or(predicate.GenerationChangedPredicate{}, predicates.ReconcileRequestedPredicate{}),
 			),
 		).
@@ -164,6 +163,11 @@ func (r *HelmRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 	// Initialize the patch helper with the current version of the object.
 	serialPatcher := patch.NewSerialPatcher(obj, r.Client)
 
+	// If it's of type OCI, migrate the object to static.
+	if obj.Spec.Type == helmv1.HelmRepositoryTypeOCI {
+		return r.migrationToStatic(ctx, serialPatcher, obj)
+	}
+
 	// recResult stores the abstracted reconcile result.
 	var recResult sreconcile.Result
 
@@ -193,8 +197,8 @@ func (r *HelmRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 		r.Metrics.RecordDuration(ctx, obj, start)
 	}()
 
-	// Examine if the object is under deletion or if a type change has happened.
-	if !obj.ObjectMeta.DeletionTimestamp.IsZero() || (obj.Spec.Type != "" && obj.Spec.Type != helmv1.HelmRepositoryTypeDefault) {
+	// Examine if the object is under deletion.
+	if !obj.ObjectMeta.DeletionTimestamp.IsZero() {
 		recResult, retErr = r.reconcileDelete(ctx, obj)
 		return
 	}
@@ -391,6 +395,18 @@ func (r *HelmRepositoryReconciler) reconcileStorage(ctx context.Context, sp *pat
 // pointer is set to the newly fetched index.
 func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch.SerialPatcher,
 	obj *helmv1.HelmRepository, artifact *sourcev1.Artifact, chartRepo *repository.ChartRepository) (sreconcile.Result, error) {
+	// Ensure it's not an OCI URL. API validation ensures that only
+	// http/https/oci scheme are allowed.
+	if strings.HasPrefix(obj.Spec.URL, helmreg.OCIScheme) {
+		err := fmt.Errorf("'oci' URL scheme cannot be used with 'default' HelmRepository type")
+		e := serror.NewStalling(
+			fmt.Errorf("invalid Helm repository URL: %w", err),
+			sourcev1.URLInvalidReason,
+		)
+		conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
+		return sreconcile.ResultEmpty, e
+	}
+
 	normalizedURL, err := repository.NormalizeURL(obj.Spec.URL)
 	if err != nil {
 		e := serror.NewStalling(
@@ -685,3 +701,33 @@ func (r *HelmRepositoryReconciler) eventLogf(ctx context.Context, obj runtime.Ob
 	}
 	r.Eventf(obj, eventType, reason, msg)
 }
+
+// migrateToStatic is HelmRepository OCI migration to static object.
+func (r *HelmRepositoryReconciler) migrationToStatic(ctx context.Context, sp *patch.SerialPatcher, obj *helmv1.HelmRepository) (result ctrl.Result, err error) {
+	// Skip migration if suspended and not being deleted.
+	if obj.Spec.Suspend && obj.DeletionTimestamp.IsZero() {
+		return ctrl.Result{}, nil
+	}
+
+	if !intpredicates.HelmRepositoryOCIRequireMigration(obj) {
+		// Already migrated, nothing to do.
+		return ctrl.Result{}, nil
+	}
+
+	// Delete any artifact.
+	_, err = r.reconcileDelete(ctx, obj)
+	if err != nil {
+		return ctrl.Result{}, err
+	}
+	// Delete finalizer and reset the status.
+	controllerutil.RemoveFinalizer(obj, sourcev1.SourceFinalizer)
+	obj.Status = helmv1.HelmRepositoryStatus{}
+
+	if err := sp.Patch(ctx, obj); err != nil {
+		return ctrl.Result{}, err
+	}
+
+	r.eventLogf(ctx, obj, eventv1.EventTypeTrace, "Migration",
+		"removed artifact and finalizer to migrate to static HelmRepository of type OCI")
+	return ctrl.Result{}, nil
+}