Set Ready on OCI HelmRepository objects

OCI HelmRepositories finished migration with an empty status, so
kubectl Ready/Status columns stayed blank. Set Ready=True with
reason NoIndex after migrating to a static object, and treat that
state as fully migrated so a later reconcile does not wipe it.

Assisted-by: Cursor Grok 4.6/cursor-grok-4.6-high-fast
Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: Rosh Ramadass <roshpr@gmail.com>

Author: roshpr

api/v1/helmrepository_types.go

@@ -156,6 +156,10 @@ const (
 	// IndexationFailedReason signals that the HelmRepository index fetch
 	// failed.
 	IndexationFailedReason string = "IndexationFailed"
+
+	// NoIndexReason signals that the HelmRepository is of type OCI and does
+	// not produce an index Artifact. Charts are resolved on demand.
+	NoIndexReason string = "NoIndex"
 )
 
 // GetConditions returns the status conditions of the object.

docs/spec/v1/helmrepositories.md

@@ -131,12 +131,12 @@ You can run this example by saving the manifest into `helmrepository.yaml`.
 
    ```console
    NAME      URL                                 AGE     READY   STATUS
-   podinfo   oci://ghcr.io/stefanprodan/charts   3m22s
+   podinfo   oci://ghcr.io/stefanprodan/charts   3m22s   True    OCI HelmRepositories do not produce an index artifact; charts are resolved on demand
    ```
 
-Because the OCI Helm repository is a data container, there's nothing to report
-for `READY` and `STATUS` columns above. The existence of the object can be
-considered to be ready for use.
+The controller sets `Ready=True` with reason `NoIndex`. OCI Helm repositories
+do not produce an index Artifact; [HelmCharts](helmcharts.md) resolve charts
+from the registry on demand.
 
 ## Writing a HelmRepository spec
 
@@ -522,10 +522,11 @@ For practical information, see
 
 ## Working with HelmRepositories
 
-**Note:** This section does not apply to [OCI Helm
-Repositories](#helm-oci-repository), being a data container, once created, they
-are ready to used by [HelmCharts](helmcharts.md).
- 
+**Note:** Triggering a reconcile and fetching an index Artifact do not apply to
+[OCI Helm Repositories](#helm-oci-repository). They are static data containers.
+Waiting for `Ready` does apply: the controller sets `Ready=True` with reason
+`NoIndex` after the object is recorded as a static source.
+
 ### Triggering a reconcile
 
 To manually tell the source-controller to reconcile a HelmRepository outside the
@@ -626,9 +627,10 @@ flux resume source helm <repository-name>
 
 ### Debugging a HelmRepository
 
-**Note:** This section does not apply to [OCI Helm
-Repositories](#helm-oci-repository), being a data container, they are static
-objects that don't require debugging if valid.
+**Note:** [OCI Helm Repositories](#helm-oci-repository) do not fetch an index.
+Chart pull failures are reported on the [HelmChart](helmcharts.md). The
+HelmRepository itself reports `Ready=True` with reason `NoIndex` when it has
+been recorded as a static source.
 
 There are several ways to gather information about a HelmRepository for debugging
 purposes.
@@ -695,9 +697,9 @@ specific HelmRepository, e.g. `flux logs --level=error --kind=HelmRepository --n
 
 ## HelmRepository Status
 
-**Note:** This section does not apply to [OCI Helm
-Repositories](#helm-oci-repository), they do not contain any information in the
-status.
+**Note:** [OCI Helm Repositories](#helm-oci-repository) do not report an
+Artifact. They set a `Ready=True` condition with reason `NoIndex`. The rest of
+this section describes HTTP/S Helm repositories.
 
 ### Artifact
 
@@ -782,13 +784,23 @@ characteristics:
 - The revision of the reported Artifact is up-to-date with the latest
   revision of the Helm repository.
 
+[OCI Helm Repositories](#helm-oci-repository) are marked ready without an
+Artifact. Charts are resolved on demand by [HelmChart](helmcharts.md).
+
 When the HelmRepository is "ready", the controller sets a Condition with the following
 attributes in the HelmRepository's `.status.conditions`:
 
 - `type: Ready`
 - `status: "True"`
 - `reason: Succeeded`
 
+For [OCI Helm Repositories](#helm-oci-repository), there is no Artifact. After
+the object is recorded as a static source, the controller sets:
+
+- `type: Ready`
+- `status: "True"`
+- `reason: NoIndex`
+
 This `Ready` Condition will retain a status value of `"True"` until the
 HelmRepository is marked as [reconciling](#reconciling-helmrepository), or e.g.
 a [transient error](#failed-helmrepository) occurs due to a temporary network

internal/controller/helmrepository_controller.go

@@ -712,6 +712,8 @@ func (r *HelmRepositoryReconciler) eventLogf(ctx context.Context, obj runtime.Ob
 	r.Eventf(obj, eventType, reason, "%s", msg)
 }
 
+const ociHelmRepositoryNoIndexMessage = "OCI HelmRepositories do not produce an index artifact; charts are resolved on demand"
+
 // migrateToStatic is HelmRepository OCI migration to static object.
 func (r *HelmRepositoryReconciler) migrationToStatic(ctx context.Context, sp *patch.SerialPatcher, obj *sourcev1.HelmRepository) (result ctrl.Result, err error) {
 	// Skip migration if suspended and not being deleted.
@@ -724,14 +726,22 @@ func (r *HelmRepositoryReconciler) migrationToStatic(ctx context.Context, sp *pa
 		return ctrl.Result{}, nil
 	}
 
-	// Delete any artifact.
+	// Delete any leftover HTTP-style artifact.
 	_, err = r.reconcileDelete(ctx, obj)
 	if err != nil {
 		return ctrl.Result{}, err
 	}
-	// Delete finalizer and reset the status.
+	// Remove the source finalizer; OCI objects are static data containers.
 	controllerutil.RemoveFinalizer(obj, sourcev1.SourceFinalizer)
-	obj.Status = sourcev1.HelmRepositoryStatus{}
+
+	if obj.DeletionTimestamp.IsZero() {
+		obj.Status = sourcev1.HelmRepositoryStatus{
+			ObservedGeneration: obj.GetGeneration(),
+		}
+		conditions.MarkTrue(obj, meta.ReadyCondition, sourcev1.NoIndexReason, "%s", ociHelmRepositoryNoIndexMessage)
+	} else {
+		obj.Status = sourcev1.HelmRepositoryStatus{}
+	}
 
 	if err := sp.Patch(ctx, obj); err != nil {
 		return ctrl.Result{}, err

internal/controller/helmrepository_controller_test.go

@@ -1756,6 +1756,10 @@ func TestHelmRepositoryReconciler_ReconcileTypeUpdatePredicateFilter(t *testing.
 			!intpredicates.HelmRepositoryOCIRequireMigration(obj)
 	}, timeout).Should(BeTrue())
 
+	g.Expect(obj.GetArtifact()).To(BeNil())
+	g.Expect(conditions.IsReady(obj)).To(BeTrue())
+	g.Expect(conditions.Get(obj, meta.ReadyCondition).Reason).To(Equal(sourcev1.NoIndexReason))
+
 	g.Expect(testEnv.Delete(ctx, obj)).To(Succeed())
 
 	// Wait for HelmRepository to be deleted
@@ -1963,6 +1967,9 @@ func TestHelmRepositoryReconciler_ociMigration(t *testing.T) {
 		_ = testEnv.Get(ctx, hrKey, hr)
 		return !intpredicates.HelmRepositoryOCIRequireMigration(hr)
 	}, timeout, time.Second).Should(BeTrue())
+	g.Expect(conditions.IsReady(hr)).To(BeTrue())
+	g.Expect(conditions.Get(hr, meta.ReadyCondition).Reason).To(Equal(sourcev1.NoIndexReason))
+	g.Expect(controllerutil.ContainsFinalizer(hr, sourcev1.SourceFinalizer)).To(BeFalse())
 
 	// Migrates updated object with finalizer.
 
@@ -1976,6 +1983,8 @@ func TestHelmRepositoryReconciler_ociMigration(t *testing.T) {
 		_ = testEnv.Get(ctx, hrKey, hr)
 		return !intpredicates.HelmRepositoryOCIRequireMigration(hr)
 	}, timeout, time.Second).Should(BeTrue())
+	g.Expect(conditions.IsReady(hr)).To(BeTrue())
+	g.Expect(conditions.Get(hr, meta.ReadyCondition).Reason).To(Equal(sourcev1.NoIndexReason))
 
 	// Migrates deleted object with finalizer.
 
@@ -2017,3 +2026,148 @@ func TestHelmRepositoryReconciler_ociMigration(t *testing.T) {
 		return false
 	}, timeout).Should(BeTrue())
 }
+
+func TestHelmRepositoryReconciler_ociReadyCondition(t *testing.T) {
+	g := NewWithT(t)
+
+	ns, err := testEnv.CreateNamespace(ctx, "hr-oci-ready-test")
+	g.Expect(err).ToNot(HaveOccurred())
+	t.Cleanup(func() {
+		g.Expect(testEnv.Cleanup(ctx, ns)).ToNot(HaveOccurred())
+	})
+
+	hr := &sourcev1.HelmRepository{
+		ObjectMeta: metav1.ObjectMeta{
+			GenerateName: "hr-oci-",
+			Namespace:    ns.Name,
+		},
+		Spec: sourcev1.HelmRepositorySpec{
+			Type:     sourcev1.HelmRepositoryTypeOCI,
+			URL:      "oci://ghcr.io/stefanprodan/charts",
+			Interval: metav1.Duration{Duration: interval},
+		},
+	}
+	g.Expect(testEnv.Create(ctx, hr)).ToNot(HaveOccurred())
+
+	waitForSourceReady(ctx, g, hr, false)
+
+	g.Expect(hr.GetArtifact()).To(BeNil())
+	g.Expect(hr.Status.URL).To(BeEmpty())
+	g.Expect(controllerutil.ContainsFinalizer(hr, sourcev1.SourceFinalizer)).To(BeFalse())
+	g.Expect(conditions.Get(hr, meta.ReadyCondition).Reason).To(Equal(sourcev1.NoIndexReason))
+	g.Expect(conditions.Get(hr, meta.ReadyCondition).Message).To(ContainSubstring("do not produce an index artifact"))
+	g.Expect(intpredicates.HelmRepositoryOCIRequireMigration(hr)).To(BeFalse())
+
+	// Spec updates on a static Ready object must not wipe the condition.
+	patchHelper, err := patch.NewHelper(hr, testEnv.Client)
+	g.Expect(err).ToNot(HaveOccurred())
+	hr.Spec.URL = "oci://ghcr.io/stefanprodan/charts/podinfo"
+	g.Expect(patchHelper.Patch(ctx, hr)).ToNot(HaveOccurred())
+
+	g.Consistently(func() bool {
+		if err := testEnv.Get(ctx, client.ObjectKeyFromObject(hr), hr); err != nil {
+			return false
+		}
+		ready := conditions.Get(hr, meta.ReadyCondition)
+		return conditions.IsReady(hr) && ready != nil && ready.Reason == sourcev1.NoIndexReason && hr.GetArtifact() == nil
+	}, 2*time.Second, 200*time.Millisecond).Should(BeTrue())
+}
+
+func TestHelmRepositoryReconciler_migrationToStatic(t *testing.T) {
+	g := NewWithT(t)
+
+	obj := &sourcev1.HelmRepository{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:       "oci-repo",
+			Namespace:  "default",
+			Generation: 1,
+			Finalizers: []string{sourcev1.SourceFinalizer},
+		},
+		Spec: sourcev1.HelmRepositorySpec{
+			Type:     sourcev1.HelmRepositoryTypeOCI,
+			URL:      "oci://example.com/charts",
+			Interval: metav1.Duration{Duration: interval},
+		},
+		Status: sourcev1.HelmRepositoryStatus{
+			ObservedGeneration: 1,
+			URL:                "http://source-controller/index.yaml",
+			Artifact:           &meta.Artifact{Path: "old-index.yaml"},
+		},
+	}
+	conditions.MarkTrue(obj, meta.ReadyCondition, meta.SucceededReason, "fetched index")
+
+	r := &HelmRepositoryReconciler{
+		Client: fakeclient.NewClientBuilder().
+			WithScheme(testEnv.GetScheme()).
+			WithObjects(obj).
+			WithStatusSubresource(&sourcev1.HelmRepository{}).
+			Build(),
+		EventRecorder: record.NewFakeRecorder(32),
+		Storage:       testStorage,
+	}
+
+	_, err := r.Reconcile(context.TODO(), ctrl.Request{NamespacedName: client.ObjectKeyFromObject(obj)})
+	g.Expect(err).ToNot(HaveOccurred())
+
+	got := &sourcev1.HelmRepository{}
+	g.Expect(r.Client.Get(context.TODO(), client.ObjectKeyFromObject(obj), got)).To(Succeed())
+	g.Expect(controllerutil.ContainsFinalizer(got, sourcev1.SourceFinalizer)).To(BeFalse())
+	g.Expect(got.GetArtifact()).To(BeNil())
+	g.Expect(got.Status.URL).To(BeEmpty())
+	g.Expect(conditions.IsReady(got)).To(BeTrue())
+	g.Expect(conditions.Get(got, meta.ReadyCondition).Reason).To(Equal(sourcev1.NoIndexReason))
+	g.Expect(got.Status.ObservedGeneration).To(Equal(got.Generation))
+	g.Expect(intpredicates.HelmRepositoryOCIRequireMigration(got)).To(BeFalse())
+
+	// A second reconcile must not wipe Ready.
+	_, err = r.Reconcile(context.TODO(), ctrl.Request{NamespacedName: client.ObjectKeyFromObject(obj)})
+	g.Expect(err).ToNot(HaveOccurred())
+	got2 := &sourcev1.HelmRepository{}
+	g.Expect(r.Client.Get(context.TODO(), client.ObjectKeyFromObject(obj), got2)).To(Succeed())
+	g.Expect(conditions.IsReady(got2)).To(BeTrue())
+	g.Expect(conditions.Get(got2, meta.ReadyCondition).Reason).To(Equal(sourcev1.NoIndexReason))
+	g.Expect(got2.GetArtifact()).To(BeNil())
+}
+
+func TestHelmRepositoryReconciler_migrationToStatic_emptyStatus(t *testing.T) {
+	g := NewWithT(t)
+
+	obj := &sourcev1.HelmRepository{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:       "oci-empty",
+			Namespace:  "default",
+			Generation: 1,
+		},
+		Spec: sourcev1.HelmRepositorySpec{
+			Type:     sourcev1.HelmRepositoryTypeOCI,
+			URL:      "oci://example.com/charts",
+			Interval: metav1.Duration{Duration: interval},
+		},
+	}
+
+	r := &HelmRepositoryReconciler{
+		Client: fakeclient.NewClientBuilder().
+			WithScheme(testEnv.GetScheme()).
+			WithObjects(obj).
+			WithStatusSubresource(&sourcev1.HelmRepository{}).
+			Build(),
+		EventRecorder: record.NewFakeRecorder(32),
+		Storage:       testStorage,
+	}
+
+	_, err := r.Reconcile(context.TODO(), ctrl.Request{NamespacedName: client.ObjectKeyFromObject(obj)})
+	g.Expect(err).ToNot(HaveOccurred())
+
+	got := &sourcev1.HelmRepository{}
+	g.Expect(r.Client.Get(context.TODO(), client.ObjectKeyFromObject(obj), got)).To(Succeed())
+	g.Expect(conditions.IsReady(got)).To(BeTrue())
+	g.Expect(conditions.Get(got, meta.ReadyCondition).Reason).To(Equal(sourcev1.NoIndexReason))
+	g.Expect(intpredicates.HelmRepositoryOCIRequireMigration(got)).To(BeFalse())
+
+	_, err = r.Reconcile(context.TODO(), ctrl.Request{NamespacedName: client.ObjectKeyFromObject(obj)})
+	g.Expect(err).ToNot(HaveOccurred())
+	got2 := &sourcev1.HelmRepository{}
+	g.Expect(r.Client.Get(context.TODO(), client.ObjectKeyFromObject(obj), got2)).To(Succeed())
+	g.Expect(conditions.IsReady(got2)).To(BeTrue())
+	g.Expect(conditions.Get(got2, meta.ReadyCondition).Reason).To(Equal(sourcev1.NoIndexReason))
+}

internal/predicates/helmrepository_type_predicate.go

@@ -22,6 +22,9 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 
+	"github.com/fluxcd/pkg/apis/meta"
+	"github.com/fluxcd/pkg/runtime/conditions"
+
 	sourcev1 "github.com/fluxcd/source-controller/api/v1"
 )
 
@@ -48,8 +51,12 @@ func (HelmRepositoryOCIMigrationPredicate) Delete(e event.DeleteEvent) bool {
 }
 
 // HelmRepositoryOCIRequireMigration returns if a given HelmRepository of type
-// OCI requires migration to static object. For non-OCI HelmRepository, it
+// OCI requires migration to a static object. For non-OCI HelmRepository, it
 // returns true.
+//
+// An OCI object is fully migrated when the source finalizer is gone, no
+// leftover index Artifact remains, and Ready=True is set. Empty status is
+// not treated as migrated so a one-shot reconcile can set that condition.
 func HelmRepositoryOCIRequireMigration(o client.Object) bool {
 	if o == nil {
 		return false
@@ -65,22 +72,22 @@ func HelmRepositoryOCIRequireMigration(o client.Object) bool {
 		return true
 	}
 
-	if controllerutil.ContainsFinalizer(hr, sourcev1.SourceFinalizer) || !hasEmptyHelmRepositoryStatus(hr) {
+	if controllerutil.ContainsFinalizer(hr, sourcev1.SourceFinalizer) {
 		return true
 	}
 
-	return false
+	if isOCIHelmRepositoryStatic(hr) {
+		return false
+	}
+
+	return true
 }
 
-// hasEmptyHelmRepositoryStatus checks if the status of a HelmRepository is
-// empty.
-func hasEmptyHelmRepositoryStatus(obj *sourcev1.HelmRepository) bool {
-	if obj.Status.ObservedGeneration == 0 &&
-		obj.Status.Conditions == nil &&
-		obj.Status.URL == "" &&
-		obj.Status.Artifact == nil &&
-		obj.Status.ReconcileRequestStatus.LastHandledReconcileAt == "" {
-		return true
+// isOCIHelmRepositoryStatic reports whether an OCI HelmRepository has completed
+// migration to a static object with Ready=True and no leftover index Artifact.
+func isOCIHelmRepositoryStatic(obj *sourcev1.HelmRepository) bool {
+	if obj.Status.Artifact != nil || obj.Status.URL != "" {
+		return false
 	}
-	return false
+	return conditions.IsTrue(obj, meta.ReadyCondition)
 }