fix: empty repository url while reconciling (#493)

## Description

Fix open-component-model/ocm-project#226


## What type of PR is this? (check all applicable)

- [ ] 🍕 Feature
- [ ] 🐛 Bug Fix
- [ ] 📝 Documentation Update
- [ ] 🎨 Style
- [ ] 🧑‍💻 Code Refactor
- [ ] 🔥 Performance Improvements
- [ ] ✅ Test
- [ ] 🤖 Build
- [ ] 🔁 CI
- [ ] 📦 Chore (Release)
- [ ] ⏩ Revert

## Related Tickets & Documents

<!-- 
Please use this format link issue numbers: Fixes #123

https://docs.github.com/en/free-pro-team@latest/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword
-->
- Related Issue # (issue)
- Closes # (issue)
- Fixes # (issue)
> Remove if not applicable

## Screenshots

<!-- Visual changes require screenshots -->


## Added tests?

- [ ] 👍 yes
- [ ] 🙅 no, because they aren't needed
- [ ] 🙋 no, because I need help
- [ ] Separate ticket for tests # (issue/pr)

Please describe the tests that you ran to verify your changes. Provide
instructions so we can reproduce. Please also list any relevant details
for your test configuration


## Added to documentation?

- [ ] 📜 README.md
- [ ] 🙅 no documentation needed

## Checklist:

- [ ] My code follows the style guidelines of this project
- [ ] I have performed a self-review of my code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] New and existing unit tests pass locally with my changes
- [ ] Any dependent changes have been merged and published in downstream
modules

api/v1alpha1/componentversion_types.go

@@ -179,6 +179,11 @@ func (in *ComponentVersion) GetComponentName() string {
 	return in.Spec.Component
 }
 
+// GetRepositoryURL returns the repository URL that the component version has been reconciled to.
+func (in *ComponentVersion) GetRepositoryURL() string {
+	return in.Status.ReplicatedRepositoryURL
+}
+
 // GetVersion returns the reconciled version for the component.
 func (in *ComponentVersion) GetVersion() string {
 	return in.Status.ReconciledVersion

controllers/componentversion_contoller_test.go

@@ -260,7 +260,7 @@ func TestComponentVersionWithTransferReconcile(t *testing.T) {
 
 	assert.Len(t, cv.Status.ComponentDescriptor.References, 1)
 	assert.Equal(t, "test-ref-1", cv.Status.ComponentDescriptor.References[0].Name)
-	assert.Equal(t, "github.com/open-component-model/internal-test", cv.Status.ReplicatedRepositoryURL)
+	assert.Equal(t, "github.com/open-component-model/internal-test", cv.GetRepositoryURL())
 	assert.True(t, conditions.IsTrue(cv, meta.ReadyCondition))
 
 	t.Log("checking label values")

controllers/componentversion_controller.go

@@ -242,7 +242,8 @@ func (r *ComponentVersionReconciler) reconcile(
 
 	obj.Status.ReplicatedRepositoryURL = obj.Spec.Repository.URL
 
-	cv, err := r.OCMClient.GetComponentVersion(ctx, octx, obj.Status.ReplicatedRepositoryURL, obj.Spec.Component, version)
+	// Get the component version from the original repository URL.
+	cv, err := r.OCMClient.GetComponentVersion(ctx, octx, obj.GetRepositoryURL(), obj.Spec.Component, version)
 	if err != nil {
 		err = fmt.Errorf("failed to get component version: %w", err)
 		status.MarkNotReady(
@@ -257,6 +258,7 @@ func (r *ComponentVersionReconciler) reconcile(
 
 	defer cv.Close()
 
+	// If there is a transfer requested, transfer the cv to that location.
 	if obj.Spec.Destination != nil {
 		rreconcile.ProgressiveStatus(false, obj, meta.ProgressingReason, "transferring component to target repository: %s", obj.Spec.Destination.URL)
 
@@ -267,10 +269,11 @@ func (r *ComponentVersionReconciler) reconcile(
 			return ctrl.Result{}, err
 		}
 
+		// set the new URL to the destination URL
 		obj.Status.ReplicatedRepositoryURL = obj.Spec.Destination.URL
 
-		// overwrite the component version
-		cv, err = r.OCMClient.GetComponentVersion(ctx, octx, obj.Status.ReplicatedRepositoryURL, obj.Spec.Component, version)
+		// update the ocm component version to be the new version from the replicated destination
+		cv, err = r.OCMClient.GetComponentVersion(ctx, octx, obj.GetRepositoryURL(), obj.Spec.Component, version)
 		if err != nil {
 			err = fmt.Errorf("failed to get transferred component version: %w", err)
 			status.MarkNotReady(
@@ -444,7 +447,7 @@ func (r *ComponentVersionReconciler) constructComponentDescriptorsForReference(
 	ref ocmdesc.ComponentReference,
 ) (*v1alpha1.Reference, error) {
 	// get component version
-	rcv, err := r.OCMClient.GetComponentVersion(ctx, octx, parent.Status.ReplicatedRepositoryURL, ref.ComponentName, ref.Version)
+	rcv, err := r.OCMClient.GetComponentVersion(ctx, octx, parent.GetRepositoryURL(), ref.ComponentName, ref.Version)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get component version: %w", err)
 	}

controllers/mutation_reconcile_looper.go

@@ -199,6 +199,10 @@ func (m *MutationReconcileLooper) localize(
 		return "", fmt.Errorf("failed to get component version: %w", err)
 	}
 
+	if !conditions.IsReady(cv) || cv.GetRepositoryURL() == "" {
+		return "", fmt.Errorf("component version is not ready yet")
+	}
+
 	var refPath []ocmmetav1.Identity
 	if mutationSpec.ConfigRef.ResourceRef != nil {
 		refPath = mutationSpec.ConfigRef.ResourceRef.ReferencePath
@@ -374,7 +378,7 @@ func (m *MutationReconcileLooper) createSubstitutionRulesForLocalization(
 		return nil, fmt.Errorf("failed to create authenticated client: %w", err)
 	}
 
-	compvers, err := m.OCMClient.GetComponentVersion(ctx, octx, cv.Status.ReplicatedRepositoryURL, cv.Spec.Component, cv.Status.ReconciledVersion)
+	compvers, err := m.OCMClient.GetComponentVersion(ctx, octx, cv.GetRepositoryURL(), cv.Spec.Component, cv.Status.ReconciledVersion)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get component version: %w", err)
 	}

controllers/resource_controller.go

@@ -171,8 +171,8 @@ func (r *ResourceReconciler) reconcile(
 		return ctrl.Result{}, err
 	}
 
-	var componentVersion v1alpha1.ComponentVersion
-	if err := r.Get(ctx, obj.Spec.SourceRef.GetObjectKey(), &componentVersion); err != nil {
+	componentVersion := &v1alpha1.ComponentVersion{}
+	if err := r.Get(ctx, obj.Spec.SourceRef.GetObjectKey(), componentVersion); err != nil {
 		if apierrors.IsNotFound(err) {
 			return ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, nil
 		}
@@ -183,23 +183,23 @@ func (r *ResourceReconciler) reconcile(
 		return ctrl.Result{}, err
 	}
 
-	if !conditions.IsReady(&componentVersion) {
+	if !conditions.IsReady(componentVersion) || componentVersion.GetRepositoryURL() == "" {
 		status.MarkNotReady(r.EventRecorder, obj, v1alpha1.ComponentVersionNotReadyReason, "component version not ready yet")
 
 		return ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, nil
 	}
 
 	rreconcile.ProgressiveStatus(false, obj, meta.ProgressingReason, "component version %s ready, processing ocm resource", componentVersion.Name)
 
-	octx, err := r.OCMClient.CreateAuthenticatedOCMContext(ctx, &componentVersion)
+	octx, err := r.OCMClient.CreateAuthenticatedOCMContext(ctx, componentVersion)
 	if err != nil {
 		err = fmt.Errorf("failed to create authenticated client: %w", err)
 		status.MarkAsStalled(r.EventRecorder, obj, v1alpha1.AuthenticatedContextCreationFailedReason, err.Error())
 
 		return ctrl.Result{}, nil
 	}
 
-	reader, digest, size, err := r.OCMClient.GetResource(ctx, octx, &componentVersion, obj.Spec.SourceRef.ResourceRef)
+	reader, digest, size, err := r.OCMClient.GetResource(ctx, octx, componentVersion, obj.Spec.SourceRef.ResourceRef)
 	if err != nil {
 		err = fmt.Errorf("failed to get resource: %w", err)
 		status.MarkNotReady(r.EventRecorder, obj, v1alpha1.GetResourceFailedReason, err.Error())

controllers/resourcepipeline_controller.go

@@ -263,28 +263,28 @@ func (r *ResourcePipelineReconciler) getComponentVersionAccess(
 	ctx context.Context,
 	obj *v1alpha1.ResourcePipeline,
 ) (ocmcore.ComponentVersionAccess, error) {
-	var componentVersion v1alpha1.ComponentVersion
+	componentVersion := &v1alpha1.ComponentVersion{}
 	key := types.NamespacedName{
 		Name:      obj.Spec.SourceRef.Name,
 		Namespace: obj.Spec.SourceRef.Namespace,
 	}
-	if err := r.Get(ctx, key, &componentVersion); err != nil {
+	if err := r.Get(ctx, key, componentVersion); err != nil {
 		return nil, err
 	}
 
-	if !conditions.IsReady(&componentVersion) {
+	if !conditions.IsReady(componentVersion) {
 		return nil, errCVNotReady
 	}
 
-	octx, err := r.OCMClient.CreateAuthenticatedOCMContext(ctx, &componentVersion)
+	octx, err := r.OCMClient.CreateAuthenticatedOCMContext(ctx, componentVersion)
 	if err != nil {
 		return nil, fmt.Errorf("could not create auth context: %w", err)
 	}
 
 	return r.OCMClient.GetComponentVersion(
 		ctx,
 		octx,
-		componentVersion.Status.ReplicatedRepositoryURL,
+		componentVersion.GetRepositoryURL(),
 		componentVersion.GetComponentName(),
 		componentVersion.GetVersion(),
 	)

controllers/suite_test.go

@@ -99,6 +99,9 @@ var (
 			},
 			Verify: []v1alpha1.Signature{},
 		},
+		Status: v1alpha1.ComponentVersionStatus{
+			ReplicatedRepositoryURL: "github.com/open-component-model/test",
+		},
 	}
 	DefaultResource = &v1alpha1.Resource{
 		TypeMeta: metav1.TypeMeta{

pkg/ocm/ocm.go

@@ -225,7 +225,7 @@ func (c *Client) GetResource(
 		return c.cache.FetchDataByIdentity(ctx, name, version)
 	}
 
-	cva, err := c.GetComponentVersion(ctx, octx, cv.Status.ReplicatedRepositoryURL, cv.Spec.Component, cv.Status.ReconciledVersion)
+	cva, err := c.GetComponentVersion(ctx, octx, cv.GetRepositoryURL(), cv.Spec.Component, cv.Status.ReconciledVersion)
 	if err != nil {
 		return nil, "", -1, fmt.Errorf("failed to get component Version: %w", err)
 	}

pkg/ocm/ocm_test.go

@@ -430,7 +430,7 @@ func TestClient_GetComponentVersion(t *testing.T) {
 		},
 	}
 
-	cva, err := ocmClient.GetComponentVersion(context.Background(), octx, cv.Status.ReplicatedRepositoryURL, component, "v0.0.1")
+	cva, err := ocmClient.GetComponentVersion(context.Background(), octx, cv.GetRepositoryURL(), component, "v0.0.1")
 	assert.NoError(t, err)
 	assert.Equal(t, cv.Spec.Component, cva.GetName())
 }