fix: adjust the resource controller to use the descriptor version for defaulting instead of the component version (#690)

<!-- markdownlint-disable MD041 -->
#### What this PR does / why we need it

#### Which issue(s) this PR fixes
<!--
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
-->

Signed-off-by: Gergely Brautigam <[email protected]>

Author: Skarlso

controllers/resource_controller.go

@@ -205,12 +205,6 @@ func (r *ResourceReconciler) reconcile(
 	// The reader is unused here, but we should still close it, so it's not left over.
 	defer reader.Close()
 
-	version := componentVersion.Status.ReconciledVersion
-	// GetVersion returns resourceRef.Version
-	if obj.Spec.SourceRef.GetVersion() != "" {
-		version = obj.Spec.SourceRef.GetVersion()
-	}
-
 	// This is important because THIS is the actual component for our resource. If we used ComponentVersion in the
 	// below identity, that would be the top-level component instead of the component that this resource belongs to.
 	componentDescriptor, err := component.GetComponentDescriptor(ctx, r.Client, obj.GetReferencePath(), componentVersion.Status.ComponentDescriptor)
@@ -231,6 +225,12 @@ func (r *ResourceReconciler) reconcile(
 		return ctrl.Result{}, err
 	}
 
+	version := componentDescriptor.Spec.Version
+	// GetVersion returns resourceRef.Version
+	if obj.Spec.SourceRef.GetVersion() != "" {
+		version = obj.Spec.SourceRef.GetVersion()
+	}
+
 	rreconcile.ProgressiveStatus(false, obj, meta.ProgressingReason, "resource retrieve, constructing snapshot with name %s", obj.GetSnapshotName())
 
 	identity := r.constructIdentity(componentDescriptor, obj, version)
@@ -265,7 +265,7 @@ func (r *ResourceReconciler) reconcile(
 		return ctrl.Result{}, err
 	}
 
-	obj.Status.LastAppliedResourceVersion = obj.Spec.SourceRef.GetVersion()
+	obj.Status.LastAppliedResourceVersion = version
 	obj.Status.LastAppliedComponentVersion = componentVersion.Status.ReconciledVersion
 
 	metrics.SnapshotNumberOfBytesReconciled.WithLabelValues(obj.GetSnapshotName(), digest, componentVersion.Name).Set(float64(size))

controllers/resource_controller_test.go

@@ -261,6 +261,83 @@ func XTestResourceReconcilerFailed(t *testing.T) {
 	assert.True(t, conditions.IsFalse(resource, meta.ReadyCondition))
 }
 
+func TestResourceReconcilerVersionDefaulting(t *testing.T) {
+	t.Log("setting up resource object without explicit version")
+	resource := DefaultResource.DeepCopy()
+	resource.Spec.SourceRef.ResourceRef.ReferencePath = nil
+	resource.Spec.SourceRef.ResourceRef.Version = ""
+	resource.Status.SnapshotName = "test-resource-version-default"
+
+	t.Log("setting up component version with different reconciled version")
+	cv := DefaultComponent.DeepCopy()
+	cv.Status.ReconciledVersion = "v0.9.9"
+	
+	t.Log("setting up component descriptor with different version")
+	cd := DefaultComponentDescriptor.DeepCopy()
+	cd.Spec.Version = "v1.2.3"
+	
+	cv.Status.ComponentDescriptor = v1alpha1.Reference{
+		Name:    resource.Spec.SourceRef.Name,
+		Version: resource.Spec.SourceRef.GetVersion(),
+		ComponentDescriptorRef: meta.NamespacedObjectReference{
+			Name:      cd.Name,
+			Namespace: cd.Namespace,
+		},
+	}
+	conditions.MarkTrue(cv,
+		meta.ReadyCondition,
+		meta.SucceededReason,
+		"Applied version: v0.9.9")
+
+	client := env.FakeKubeClient(WithObjects(cv, resource, cd))
+	t.Log("priming fake cache")
+	cache := &cachefakes.FakeCache{}
+	cache.PushDataReturns("digest", nil)
+
+	t.Log("priming fake ocm client")
+	ocmClient := &fakes.MockFetcher{}
+	ocmClient.GetResourceReturns(io.NopCloser(bytes.NewBuffer([]byte("content"))), "digest", nil)
+	recorder := record.NewFakeRecorder(32)
+
+	rr := ResourceReconciler{
+		Scheme:        env.scheme,
+		Client:        client,
+		OCMClient:     ocmClient,
+		EventRecorder: recorder,
+		Cache:         cache,
+	}
+
+	t.Log("calling reconcile on resource controller")
+	_, err := rr.Reconcile(context.Background(), ctrl.Request{
+		NamespacedName: types.NamespacedName{
+			Namespace: resource.Namespace,
+			Name:      resource.Name,
+		},
+	})
+	require.NoError(t, err)
+
+	t.Log("verifying generated snapshot uses component descriptor version")
+	snapshot := &v1alpha1.Snapshot{}
+	err = client.Get(context.Background(), types.NamespacedName{
+		Name:      resource.Status.SnapshotName,
+		Namespace: resource.Namespace,
+	}, snapshot)
+
+	require.NoError(t, err)
+	assert.Equal(t, "digest", snapshot.Spec.Digest)
+	assert.Equal(t, "v1.2.3", snapshot.Spec.Tag)
+
+	t.Log("verifying resource status uses component descriptor version")
+	err = client.Get(context.Background(), types.NamespacedName{
+		Name:      resource.Name,
+		Namespace: resource.Namespace,
+	}, resource)
+
+	require.NoError(t, err)
+	assert.Equal(t, "v1.2.3", resource.Status.LastAppliedResourceVersion)
+	assert.True(t, conditions.IsTrue(resource, meta.ReadyCondition))
+}
+
 // TODO: rewrite these so that they test the predicate functions.
 func XTestResourceShouldReconcile(t *testing.T) {
 	testcase := []struct {