diff --git a/api/v1/helmrepository_types.go b/api/v1/helmrepository_types.go index e69f28001..689588fad 100644 --- a/api/v1/helmrepository_types.go +++ b/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. diff --git a/docs/spec/v1/helmrepositories.md b/docs/spec/v1/helmrepositories.md index 274d2cd66..9b811855d 100644 --- a/docs/spec/v1/helmrepositories.md +++ b/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 ### 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,6 +784,9 @@ 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`: @@ -789,6 +794,13 @@ attributes in the HelmRepository's `.status.conditions`: - `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 diff --git a/internal/controller/helmrepository_controller.go b/internal/controller/helmrepository_controller.go index 0caf5c9e3..20b046acb 100644 --- a/internal/controller/helmrepository_controller.go +++ b/internal/controller/helmrepository_controller.go @@ -696,6 +696,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. @@ -708,14 +710,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 diff --git a/internal/controller/helmrepository_controller_test.go b/internal/controller/helmrepository_controller_test.go index f76d4f221..7cda77194 100644 --- a/internal/controller/helmrepository_controller_test.go +++ b/internal/controller/helmrepository_controller_test.go @@ -1652,10 +1652,17 @@ func TestHelmRepositoryReconciler_ReconcileTypeUpdatePredicateFilter(t *testing. if err := testEnv.Get(ctx, key, obj); err != nil { return false } + ready := conditions.Get(obj, meta.ReadyCondition) return newGen == obj.Generation && + conditions.IsReady(obj) && + ready != nil && ready.Reason == sourcev1.NoIndexReason && !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 @@ -1861,8 +1868,12 @@ func TestHelmRepositoryReconciler_ociMigration(t *testing.T) { g.Eventually(func() bool { _ = testEnv.Get(ctx, hrKey, hr) - return !intpredicates.HelmRepositoryOCIRequireMigration(hr) + ready := conditions.Get(hr, meta.ReadyCondition) + return conditions.IsReady(hr) && + ready != nil && ready.Reason == sourcev1.NoIndexReason && + !intpredicates.HelmRepositoryOCIRequireMigration(hr) }, timeout, time.Second).Should(BeTrue()) + g.Expect(controllerutil.ContainsFinalizer(hr, sourcev1.SourceFinalizer)).To(BeFalse()) // Migrates updated object with finalizer. @@ -1874,7 +1885,10 @@ func TestHelmRepositoryReconciler_ociMigration(t *testing.T) { g.Eventually(func() bool { _ = testEnv.Get(ctx, hrKey, hr) - return !intpredicates.HelmRepositoryOCIRequireMigration(hr) + ready := conditions.Get(hr, meta.ReadyCondition) + return conditions.IsReady(hr) && + ready != nil && ready.Reason == sourcev1.NoIndexReason && + !intpredicates.HelmRepositoryOCIRequireMigration(hr) }, timeout, time.Second).Should(BeTrue()) // Migrates deleted object with finalizer. @@ -1917,3 +1931,218 @@ 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()) + + g.Eventually(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 && + !intpredicates.HelmRepositoryOCIRequireMigration(hr) + }, timeout).Should(BeTrue()) + + 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).Message).To(ContainSubstring("do not produce an index artifact")) + + // 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)) +} + +func TestHelmRepositoryReconciler_migrationToStatic_staleReady(t *testing.T) { + tests := []struct { + name string + markReady func(obj *sourcev1.HelmRepository) + }{ + { + name: "Succeeded reason", + markReady: func(obj *sourcev1.HelmRepository) { + conditions.MarkTrue(obj, meta.ReadyCondition, meta.SucceededReason, "fetched index") + }, + }, + { + name: "empty reason", + markReady: func(obj *sourcev1.HelmRepository) { + conditions.MarkTrue(obj, meta.ReadyCondition, "", "") + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + obj := &sourcev1.HelmRepository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "oci-stale-ready", + Namespace: "default", + Generation: 1, + }, + Spec: sourcev1.HelmRepositorySpec{ + Type: sourcev1.HelmRepositoryTypeOCI, + URL: "oci://example.com/charts", + Interval: metav1.Duration{Duration: interval}, + }, + Status: sourcev1.HelmRepositoryStatus{ + ObservedGeneration: 1, + }, + } + tt.markReady(obj) + g.Expect(intpredicates.HelmRepositoryOCIRequireMigration(obj)).To(BeTrue()) + + 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()) + }) + } +} diff --git a/internal/predicates/helmrepository_type_predicate.go b/internal/predicates/helmrepository_type_predicate.go index 714d77942..39df2df94 100644 --- a/internal/predicates/helmrepository_type_predicate.go +++ b/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,13 @@ 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 with reason +// NoIndex. Empty status and stale Ready (for example Succeeded) are not +// treated as migrated so a one-shot reconcile can establish that condition. func HelmRepositoryOCIRequireMigration(o client.Object) bool { if o == nil { return false @@ -65,22 +73,24 @@ 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 reason NoIndex 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) && + conditions.GetReason(obj, meta.ReadyCondition) == sourcev1.NoIndexReason } diff --git a/internal/predicates/helmrepository_type_predicate_test.go b/internal/predicates/helmrepository_type_predicate_test.go index e98728413..fbe26c84d 100644 --- a/internal/predicates/helmrepository_type_predicate_test.go +++ b/internal/predicates/helmrepository_type_predicate_test.go @@ -39,7 +39,7 @@ func TestHelmRepositoryOCIMigrationPredicate_Create(t *testing.T) { beforeFunc: func(o *sourcev1.HelmRepository) { o.Spec.Type = sourcev1.HelmRepositoryTypeOCI }, - want: false, + want: true, }, { name: "new oci helm repo with default observed gen status", @@ -58,13 +58,45 @@ func TestHelmRepositoryOCIMigrationPredicate_Create(t *testing.T) { want: true, }, { - name: "old oci helm repo with status only", + name: "static oci helm repo with Ready NoIndex", beforeFunc: func(o *sourcev1.HelmRepository) { o.Spec.Type = sourcev1.HelmRepositoryTypeOCI o.Status = sourcev1.HelmRepositoryStatus{ ObservedGeneration: 3, } - conditions.MarkTrue(o, meta.ReadyCondition, "foo", "bar") + conditions.MarkTrue(o, meta.ReadyCondition, sourcev1.NoIndexReason, "bar") + }, + want: false, + }, + { + name: "oci helm repo with stale Ready Succeeded", + beforeFunc: func(o *sourcev1.HelmRepository) { + o.Spec.Type = sourcev1.HelmRepositoryTypeOCI + o.Status = sourcev1.HelmRepositoryStatus{ + ObservedGeneration: 3, + } + conditions.MarkTrue(o, meta.ReadyCondition, meta.SucceededReason, "fetched index") + }, + want: true, + }, + { + name: "oci helm repo with Ready empty reason", + beforeFunc: func(o *sourcev1.HelmRepository) { + o.Spec.Type = sourcev1.HelmRepositoryTypeOCI + conditions.MarkTrue(o, meta.ReadyCondition, "", "") + }, + want: true, + }, + { + name: "oci helm repo with leftover artifact", + beforeFunc: func(o *sourcev1.HelmRepository) { + o.Spec.Type = sourcev1.HelmRepositoryTypeOCI + o.Status = sourcev1.HelmRepositoryStatus{ + ObservedGeneration: 3, + Artifact: &meta.Artifact{}, + URL: "http://some-address", + } + conditions.MarkTrue(o, meta.ReadyCondition, meta.SucceededReason, "fetched index") }, want: true, }, @@ -76,7 +108,7 @@ func TestHelmRepositoryOCIMigrationPredicate_Create(t *testing.T) { o.Status = sourcev1.HelmRepositoryStatus{ ObservedGeneration: 3, } - conditions.MarkTrue(o, meta.ReadyCondition, "foo", "bar") + conditions.MarkTrue(o, meta.ReadyCondition, sourcev1.NoIndexReason, "bar") }, want: true, }, @@ -111,19 +143,45 @@ func TestHelmRepositoryOCIMigrationPredicate_Update(t *testing.T) { want bool }{ { - name: "update oci repo", + name: "update empty oci repo", + beforeFunc: func(oldObj, newObj *sourcev1.HelmRepository) { + oldObj.Spec = sourcev1.HelmRepositorySpec{ + Type: sourcev1.HelmRepositoryTypeOCI, + URL: "oci://foo/bar", + } + *newObj = *oldObj.DeepCopy() + newObj.Spec.URL = "oci://foo/baz" + }, + want: true, + }, + { + name: "update static oci repo with Ready NoIndex", beforeFunc: func(oldObj, newObj *sourcev1.HelmRepository) { oldObj.Spec = sourcev1.HelmRepositorySpec{ Type: sourcev1.HelmRepositoryTypeOCI, URL: "oci://foo/bar", } + conditions.MarkTrue(oldObj, meta.ReadyCondition, sourcev1.NoIndexReason, "bar") *newObj = *oldObj.DeepCopy() newObj.Spec.URL = "oci://foo/baz" }, want: false, }, { - name: "migrate old oci repo with status only", + name: "update oci repo with stale Ready Succeeded", + beforeFunc: func(oldObj, newObj *sourcev1.HelmRepository) { + oldObj.Spec = sourcev1.HelmRepositorySpec{ + Type: sourcev1.HelmRepositoryTypeOCI, + URL: "oci://foo/bar", + } + conditions.MarkTrue(oldObj, meta.ReadyCondition, meta.SucceededReason, "fetched index") + *newObj = *oldObj.DeepCopy() + newObj.Spec.URL = "oci://foo/baz" + }, + want: true, + }, + { + name: "migrate old oci repo with leftover artifact", beforeFunc: func(oldObj, newObj *sourcev1.HelmRepository) { oldObj.Generation = 2 oldObj.Spec = sourcev1.HelmRepositorySpec{ @@ -131,8 +189,10 @@ func TestHelmRepositoryOCIMigrationPredicate_Update(t *testing.T) { } oldObj.Status = sourcev1.HelmRepositoryStatus{ ObservedGeneration: 2, + Artifact: &meta.Artifact{}, + URL: "http://some-address", } - conditions.MarkTrue(oldObj, meta.ReadyCondition, "foo", "bar") + conditions.MarkTrue(oldObj, meta.ReadyCondition, meta.SucceededReason, "fetched index") *newObj = *oldObj.DeepCopy() newObj.Generation = 3 @@ -227,12 +287,28 @@ func TestHelmRepositoryOCIMigrationPredicate_Delete(t *testing.T) { want: true, }, { - name: "oci without finalizer or status", + name: "static oci without finalizer", beforeFunc: func(obj *sourcev1.HelmRepository) { obj.Spec.Type = sourcev1.HelmRepositoryTypeOCI + conditions.MarkTrue(obj, meta.ReadyCondition, sourcev1.NoIndexReason, "bar") }, want: false, }, + { + name: "oci with stale Ready Succeeded", + beforeFunc: func(obj *sourcev1.HelmRepository) { + obj.Spec.Type = sourcev1.HelmRepositoryTypeOCI + conditions.MarkTrue(obj, meta.ReadyCondition, meta.SucceededReason, "fetched index") + }, + want: true, + }, + { + name: "oci without finalizer or status", + beforeFunc: func(obj *sourcev1.HelmRepository) { + obj.Spec.Type = sourcev1.HelmRepositoryTypeOCI + }, + want: true, + }, { name: "default helm repo", beforeFunc: func(obj *sourcev1.HelmRepository) {