[Backport release-1.14] Ensure ownerRef on objects for inactive package revisions

go.mod

@@ -107,7 +107,7 @@ require (
 	github.com/aws/aws-sdk-go-v2/service/sso v1.12.10 // indirect
 	github.com/aws/aws-sdk-go-v2/service/ssooidc v1.14.10 // indirect
 	github.com/aws/aws-sdk-go-v2/service/sts v1.19.0 // indirect
-	github.com/aws/smithy-go v1.13.5 // indirect
+	github.com/aws/smithy-go v1.13.5
 	github.com/awslabs/amazon-ecr-credential-helper/ecr-login v0.0.0-20230510185313-f5e39e5f34c7 // indirect
 	github.com/beorn7/perks v1.0.1 // indirect
 	github.com/bufbuild/protocompile v0.6.0 // indirect

internal/controller/pkg/revision/establisher.go

@@ -166,9 +166,11 @@ func (e *APIEstablisher) ReleaseObjects(ctx context.Context, parent v1.PackageRe
 				return errors.Wrapf(err, errFmtGetOwnedObject, u.GetKind(), u.GetName())
 			}
 			ors := u.GetOwnerReferences()
+			found := false
 			changed := false
 			for i := range ors {
 				if ors[i].UID == parent.GetUID() {
+					found = true
 					if ors[i].Controller != nil && *ors[i].Controller {
 						ors[i].Controller = ptr.To(false)
 						changed = true
@@ -182,6 +184,11 @@ func (e *APIEstablisher) ReleaseObjects(ctx context.Context, parent v1.PackageRe
 				// happens active revision or the package itself will still take
 				// over the ownership of such resources.
 			}
+			if !found {
+				// Make sure the package revision exists as an owner.
+				ors = append(ors, meta.AsOwner(meta.TypedReferenceTo(parent, parent.GetObjectKind().GroupVersionKind())))
+				changed = true
+			}
 			if changed {
 				u.SetOwnerReferences(ors)
 				if err := e.client.Update(ctx, &u); err != nil {

internal/controller/pkg/revision/establisher_test.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"testing"
 
+	"github.com/aws/smithy-go/ptr"
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
 	admv1 "k8s.io/api/admissionregistration/v1"
@@ -669,6 +670,67 @@ func TestAPIEstablisherReleaseObjects(t *testing.T) {
 				err: nil,
 			},
 		},
+		"OwnedIfNotAlready": {
+			reason: "ReleaseObjects should put owner reference back if we are not already the owner.",
+			args: args{
+				est: &APIEstablisher{
+					client: &test.MockClient{
+						MockGet: func(ctx context.Context, key client.ObjectKey, obj client.Object) error {
+							o := obj.(*unstructured.Unstructured)
+							o.SetOwnerReferences([]metav1.OwnerReference{
+								{
+									APIVersion: "pkg.crossplane.io/v1",
+									Kind:       "Provider",
+									Name:       "provider-helm",
+									UID:        "some-other-uid-1234",
+									Controller: &noControl,
+								},
+							})
+							return nil
+						},
+						MockUpdate: func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error {
+							o := obj.(*unstructured.Unstructured)
+							if len(o.GetOwnerReferences()) != 2 {
+								t.Errorf("expected 2 owner references, got %d", len(o.GetOwnerReferences()))
+							}
+							found := false
+							for _, ref := range o.GetOwnerReferences() {
+								if ref.Kind == "ProviderRevision" && ref.UID == "some-unique-uid-2312" {
+									found = true
+									if ptr.ToBool(ref.Controller) {
+										t.Errorf("expected controller to be false, got %t", *ref.Controller)
+									}
+								}
+							}
+							if !found {
+								t.Errorf("expected to find owner reference for revision with uid some-unique-uid-2312")
+							}
+							return nil
+						},
+					},
+				},
+				parent: &v1.ProviderRevision{
+					TypeMeta: metav1.TypeMeta{
+						Kind: "ProviderRevision",
+					},
+					ObjectMeta: metav1.ObjectMeta{
+						UID: "some-unique-uid-2312",
+					},
+					Status: v1.PackageRevisionStatus{
+						ObjectRefs: []xpv1.TypedReference{
+							{
+								APIVersion: "apiextensions.k8s.io/v1",
+								Kind:       "CustomResourceDefinition",
+								Name:       "releases.helm.crossplane.io",
+							},
+						},
+					},
+				},
+			},
+			want: want{
+				err: nil,
+			},
+		},
 		"SuccessfulRelease": {
 			reason: "ReleaseObjects should be successful if we can release control of existing objects",
 			args: args{