Add test cases to get helmcharts to use chartRefs

Signed-off-by: Soule BA <bah.soule@gmail.com>

api/v2beta2/helmrelease_types.go

@@ -84,7 +84,7 @@ type HelmReleaseSpec struct {
 	// ChartRef holds a reference to a source controller resource containing the
 	// Helm chart artifact.
 	// +optional
-	ChartRef *CrosssNameSpaceSourceReference `json:"chartRef,omitempty"`
+	ChartRef *CrossNamespaceSourceReference `json:"chartRef,omitempty"`
 
 	// Interval at which to reconcile the Helm release.
 	// +kubebuilder:validation:Type=string
@@ -1247,6 +1247,16 @@ func (in *HelmRelease) GetStatusConditions() *[]metav1.Condition {
 	return &in.Status.Conditions
 }
 
+// IsChartRefPresent returns true if the HelmRelease has a ChartRef.
+func (in *HelmRelease) IsChartRefPresent() bool {
+	return in.Spec.ChartRef != nil
+}
+
+// IsChartTemplatePresent returns true if the HelmRelease has a ChartTemplate.
+func (in *HelmRelease) IsChartTemplatePresent() bool {
+	return in.Spec.Chart.Spec.Chart != ""
+}
+
 // +kubebuilder:object:root=true
 
 // HelmReleaseList contains a list of HelmRelease objects.

api/v2beta2/reference_types.go

@@ -42,9 +42,9 @@ type CrossNamespaceObjectReference struct {
 	Namespace string `json:"namespace,omitempty"`
 }
 
-// CrosssNameSpaceSourceReference contains enough information to let you locate
+// CrossNamespaceSourceReference contains enough information to let you locate
 // the typed referenced object at cluster level.
-type CrosssNameSpaceSourceReference struct {
+type CrossNamespaceSourceReference struct {
 	// APIVersion of the referent.
 	// +optional
 	APIVersion string `json:"apiVersion,omitempty"`

config/crd/bases/helm.toolkit.fluxcd.io_helmreleases.yaml

@@ -1420,7 +1420,8 @@ spec:
                 - spec
                 type: object
               chartRef:
-                description: ChartRef holds a reference to a v1beta2.HelmChart resource
+                description: ChartRef holds a reference to a source controller resource
+                  containing the Helm chart artifact.
                 properties:
                   apiVersion:
                     description: APIVersion of the referent.

docs/api/v2beta2/helm.md

@@ -87,14 +87,15 @@ for this HelmRelease.</p>
 <td>
 <code>chartRef</code><br>
 <em>
-<a href="#helm.toolkit.fluxcd.io/v2beta2.CrosssNameSpaceSourceReference">
-CrosssNameSpaceSourceReference
+<a href="#helm.toolkit.fluxcd.io/v2beta2.CrossNamespaceSourceReference">
+CrossNamespaceSourceReference
 </a>
 </em>
 </td>
 <td>
 <em>(Optional)</em>
-<p>ChartRef holds a reference to a v1beta2.HelmChart resource</p>
+<p>ChartRef holds a reference to a source controller resource containing the
+Helm chart artifact.</p>
 </td>
 </tr>
 <tr>
@@ -484,13 +485,13 @@ string
 </table>
 </div>
 </div>
-<h3 id="helm.toolkit.fluxcd.io/v2beta2.CrosssNameSpaceSourceReference">CrosssNameSpaceSourceReference
+<h3 id="helm.toolkit.fluxcd.io/v2beta2.CrossNamespaceSourceReference">CrossNamespaceSourceReference
 </h3>
 <p>
 (<em>Appears on:</em>
 <a href="#helm.toolkit.fluxcd.io/v2beta2.HelmReleaseSpec">HelmReleaseSpec</a>)
 </p>
-<p>CrosssNameSpaceSourceReference contains enough information to let you locate
+<p>CrossNamespaceSourceReference contains enough information to let you locate
 the typed referenced object at cluster level.</p>
 <div class="md-typeset__scrollwrap">
 <div class="md-typeset__table">
@@ -1097,14 +1098,15 @@ for this HelmRelease.</p>
 <td>
 <code>chartRef</code><br>
 <em>
-<a href="#helm.toolkit.fluxcd.io/v2beta2.CrosssNameSpaceSourceReference">
-CrosssNameSpaceSourceReference
+<a href="#helm.toolkit.fluxcd.io/v2beta2.CrossNamespaceSourceReference">
+CrossNamespaceSourceReference
 </a>
 </em>
 </td>
 <td>
 <em>(Optional)</em>
-<p>ChartRef holds a reference to a v1beta2.HelmChart resource</p>
+<p>ChartRef holds a reference to a source controller resource containing the
+Helm chart artifact.</p>
 </td>
 </tr>
 <tr>

internal/controller/helmrelease_controller.go

@@ -673,7 +673,7 @@ func (r *HelmReleaseReconciler) buildRESTClientGetter(ctx context.Context, obj *
 // It returns the source object or an error.
 func (r *HelmReleaseReconciler) getSource(ctx context.Context, obj *v2.HelmRelease) (source.Source, error) {
 	var name, namespace string
-	if isChartRefPresent(obj) {
+	if obj.IsChartRefPresent() {
 		if obj.Spec.ChartRef.Kind == sourcev1.OCIRepositoryKind {
 			return r.getHelmChartFromOCIRef(ctx, obj)
 		}
@@ -855,26 +855,18 @@ func isOCIRepositoryReady(obj *sourcev1.OCIRepository) (bool, string) {
 	}
 }
 
-func isChartRefPresent(obj *v2.HelmRelease) bool {
-	return obj.Spec.ChartRef != nil
-}
-
-func isChartTemplatePresent(obj *v2.HelmRelease) bool {
-	return obj.Spec.Chart.Spec.Chart != ""
-}
-
 func isValidChartRef(obj *v2.HelmRelease) bool {
-	return (isChartRefPresent(obj) && !isChartTemplatePresent(obj)) ||
-		(!isChartRefPresent(obj) && isChartTemplatePresent(obj))
+	return (obj.IsChartRefPresent() && !obj.IsChartTemplatePresent()) ||
+		(!obj.IsChartRefPresent() && obj.IsChartTemplatePresent())
 }
 
 func getNamespacedName(obj *v2.HelmRelease) (types.NamespacedName, error) {
 	namespacedName := types.NamespacedName{}
 	switch {
-	case isChartRefPresent(obj) && !isChartTemplatePresent(obj):
+	case obj.IsChartRefPresent() && !obj.IsChartTemplatePresent():
 		namespacedName.Namespace = obj.Spec.ChartRef.Namespace
 		namespacedName.Name = obj.Spec.ChartRef.Name
-	case !isChartRefPresent(obj) && isChartTemplatePresent(obj):
+	case !obj.IsChartRefPresent() && obj.IsChartTemplatePresent():
 		namespacedName.Namespace = obj.Spec.Chart.GetNamespace(obj.GetNamespace())
 		namespacedName.Name = obj.GetHelmChartName()
 	default:

internal/controller/helmrelease_controller_test.go

@@ -2016,6 +2016,25 @@ func TestHelmReleaseReconciler_getHelmChart(t *testing.T) {
 			wantErr:         true,
 			disallowCrossNS: true,
 		},
+		{
+			"get chart from reference",
+			&v2.HelmRelease{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "default",
+				},
+				Spec: v2.HelmReleaseSpec{
+					ChartRef: &v2.CrossNamespaceSourceReference{
+						Kind:      "HelmChart",
+						Name:      "some-chart-name",
+						Namespace: "some-namespace",
+					},
+				},
+			},
+			chart,
+			true,
+			false,
+			false,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -2414,3 +2433,89 @@ func Test_isHelmChartReady(t *testing.T) {
 		})
 	}
 }
+
+func Test_isOCIRepositoryReady(t *testing.T) {
+	mock := &sourcev1b2.OCIRepository{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:       "mock",
+			Namespace:  "default",
+			Generation: 2,
+		},
+		Status: sourcev1b2.OCIRepositoryStatus{
+			ObservedGeneration: 2,
+			Conditions: []metav1.Condition{
+				{
+					Type:   meta.ReadyCondition,
+					Status: metav1.ConditionTrue,
+				},
+			},
+			Artifact: &sourcev1.Artifact{},
+		},
+	}
+
+	tests := []struct {
+		name       string
+		obj        *sourcev1b2.OCIRepository
+		want       bool
+		wantReason string
+	}{
+		{
+			name: "OCIRepository is ready",
+			obj:  mock.DeepCopy(),
+			want: true,
+		},
+		{
+			name: "OCIRepository generation differs from observed generation while Ready=True",
+			obj: func() *sourcev1b2.OCIRepository {
+				m := mock.DeepCopy()
+				m.Generation = 3
+				return m
+			}(),
+			want:       false,
+			wantReason: "OCIRepository 'default/mock' is not ready: latest generation of object has not been reconciled",
+		},
+		{
+			name: "OCIRepository generation differs from observed generation while Ready=False",
+			obj: func() *sourcev1b2.OCIRepository {
+				m := mock.DeepCopy()
+				m.Generation = 3
+				conditions.MarkFalse(m, meta.ReadyCondition, "Reason", "some reason")
+				return m
+			}(),
+			want:       false,
+			wantReason: "OCIRepository 'default/mock' is not ready: some reason",
+		},
+		{
+			name: "OCIRepository has Stalled=True",
+			obj: func() *sourcev1b2.OCIRepository {
+				m := mock.DeepCopy()
+				conditions.MarkFalse(m, meta.ReadyCondition, "Reason", "some reason")
+				conditions.MarkStalled(m, "Reason", "some stalled reason")
+				return m
+			}(),
+			want:       false,
+			wantReason: "OCIRepository 'default/mock' is not ready: some stalled reason",
+		},
+		{
+			name: "OCIRepository does not have an Artifact",
+			obj: func() *sourcev1b2.OCIRepository {
+				m := mock.DeepCopy()
+				m.Status.Artifact = nil
+				return m
+			}(),
+			want:       false,
+			wantReason: "OCIRepository 'default/mock' is not ready: does not have an artifact",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, gotReason := isOCIRepositoryReady(tt.obj)
+			if got != tt.want {
+				t.Errorf("isOCIRepositoryReady() got = %v, want %v", got, tt.want)
+			}
+			if gotReason != tt.wantReason {
+				t.Errorf("isOCIRepositoryReady() reason = %v, want %v", gotReason, tt.wantReason)
+			}
+		})
+	}
+}

internal/reconcile/helmchart_template.go

@@ -76,24 +76,6 @@ func NewHelmChartTemplate(client client.Client, recorder record.EventRecorder, f
 	}
 }
 
-func isChartRefPresent(obj *v2.HelmRelease) bool {
-	return obj.Spec.ChartRef != nil
-}
-
-func isChartTemplatePresent(obj *v2.HelmRelease) bool {
-	return obj.Spec.Chart.Spec.Chart != ""
-}
-
-func mustCleanDeployedChart(obj *v2.HelmRelease) bool {
-	if isChartRefPresent(obj) && !isChartTemplatePresent(obj) {
-		if obj.Status.HelmChart != "" {
-			return true
-		}
-	}
-
-	return false
-}
-
 func (r *HelmChartTemplate) Reconcile(ctx context.Context, req *Request) error {
 	var (
 		obj      = req.Object
@@ -122,7 +104,7 @@ func (r *HelmChartTemplate) Reconcile(ctx context.Context, req *Request) error {
 		return nil
 	}
 
-	if isChartRefPresent(obj) {
+	if obj.IsChartRefPresent() {
 		// if a chartRef is present, we do not need to reconcile the HelmChart from the template.
 		return nil
 	}
@@ -262,3 +244,13 @@ func buildHelmChartFromTemplate(obj *v2.HelmRelease) *sourcev1.HelmChart {
 	}
 	return result
 }
+
+func mustCleanDeployedChart(obj *v2.HelmRelease) bool {
+	if obj.IsChartRefPresent() && !obj.IsChartTemplatePresent() {
+		if obj.Status.HelmChart != "" {
+			return true
+		}
+	}
+
+	return false
+}