Enable deleting objects when identity of manifest changes

internal/controller/object/object.go

@@ -89,6 +89,7 @@ const (
 	errGetObservedState        = "cannot get observed state"
 	errGetDesiredState         = "cannot get desired state"
 	errUnmarshalTemplate       = "cannot unmarshal template"
+	errUnmarshalAtProvider     = "cannot unmarshal atProvider"
 	errFailedToMarshalExisting = "cannot marshal existing resource"
 
 	errGetReferencedResource       = "cannot get referenced resource"
@@ -365,6 +366,30 @@ func (c *external) Observe(ctx context.Context, mg resource.Managed) (managed.Ex
 		c.kindObserver.WatchResources(c.rest, obj.Spec.ProviderConfigReference.Name, manifest.GroupVersionKind())
 	}
 
+	if len(obj.Status.AtProvider.Manifest.Raw) > 0 && obj.GetDeletionPolicy() == xpv1.DeletionDelete {
+		atProviderManifest := &unstructured.Unstructured{}
+		if err := json.Unmarshal(obj.Status.AtProvider.Manifest.Raw, atProviderManifest); err != nil {
+			return managed.ExternalObservation{}, errors.Wrap(err, errUnmarshalAtProvider)
+		}
+
+		// The name may be empty in the manifest, in which case we should use the name of the object
+		expectedName := manifest.GetName()
+		if expectedName == "" {
+			expectedName = obj.GetName()
+		}
+
+		// If the identity (i.e GVK, name or namespace) of the resource has changed, we need to delete the old resource
+		// before we can proceed with the observing the new resource.
+		if atProviderManifest.GroupVersionKind() != manifest.GroupVersionKind() ||
+			atProviderManifest.GetNamespace() != manifest.GetNamespace() ||
+			atProviderManifest.GetName() != expectedName {
+
+			if err := c.client.Delete(ctx, atProviderManifest); err != nil && !kerrors.IsNotFound(err) {
+				return managed.ExternalObservation{}, errors.Wrap(err, errDeleteObject)
+			}
+		}
+	}
+
 	current := manifest.DeepCopy()
 	err = c.client.Get(ctx, types.NamespacedName{
 		Namespace: current.GetNamespace(),

internal/controller/object/object_test.go

@@ -639,6 +639,123 @@ func TestObserve(t *testing.T) {
 				err: nil,
 			},
 		},
+		"Deleting object with different identity fails": {
+			args: args{
+				mg: kubernetesObject(func(obj *v1alpha2.Object) {
+					obj.Spec.ForProvider.Manifest = runtime.RawExtension{Raw: []byte(`{
+						"apiVersion": "v1",
+						"kind": "ConfigMap",
+						"metadata": {
+							"name": "name",
+							"namespace": "namespace"
+						}
+					}`)}
+
+					obj.Status.AtProvider.Manifest = runtime.RawExtension{Raw: []byte(`{
+						"apiVersion": "v1",
+						"kind": "ConfigMap",
+						"metadata": {
+							"name": "old-name",
+							"namespace": "namespace"
+						}
+					}`)}
+
+					obj.Spec.DeletionPolicy = xpv1.DeletionDelete
+				}),
+				client: resource.ClientApplicator{
+					Client: &test.MockClient{
+						MockDelete: test.NewMockDeleteFn(nil, func(obj client.Object) error {
+							if obj.GetName() != "old-name" {
+								t.Errorf("expected object to be deleted with name old-name, got %s", obj.GetName())
+							}
+							return errBoom
+						}),
+					},
+				},
+			},
+			want: want{
+				out: managed.ExternalObservation{ResourceExists: false},
+				err: errors.Wrap(errBoom, errDeleteObject),
+			},
+		},
+		"Deleting object with different identity succeeds": {
+			args: args{
+				mg: kubernetesObject(func(obj *v1alpha2.Object) {
+					obj.Spec.ForProvider.Manifest = runtime.RawExtension{Raw: []byte(`{
+						"apiVersion": "v1",
+						"kind": "ConfigMap",
+						"metadata": {
+							"name": "name",
+							"namespace": "namespace"
+						}
+					}`)}
+
+					obj.Status.AtProvider.Manifest = runtime.RawExtension{Raw: []byte(`{
+						"apiVersion": "v1",
+						"kind": "ConfigMap",
+						"metadata": {
+							"name": "old-name",
+							"namespace": "namespace"
+						}
+					}`)}
+
+					obj.Spec.DeletionPolicy = xpv1.DeletionDelete
+				}),
+				client: resource.ClientApplicator{
+					Client: &test.MockClient{
+						MockGet: test.NewMockGetFn(nil, func(obj client.Object) error {
+							return kerrors.NewNotFound(schema.GroupResource{}, "")
+						}),
+						MockDelete: test.NewMockDeleteFn(nil, func(obj client.Object) error {
+							if obj.GetName() != "old-name" {
+								t.Errorf("expected object to be deleted with name old-name, got %s", obj.GetName())
+							}
+							return nil
+						}),
+					},
+				},
+			},
+			want: want{
+				out: managed.ExternalObservation{ResourceExists: false},
+				err: nil,
+			},
+		},
+		"Deletion skipped if deletion policy is orphan": {
+			args: args{
+				mg: kubernetesObject(func(obj *v1alpha2.Object) {
+					obj.Spec.ForProvider.Manifest = runtime.RawExtension{Raw: []byte(`{
+						"apiVersion": "v1",
+						"kind": "ConfigMap",
+						"metadata": {
+							"name": "name",
+							"namespace": "namespace"
+						}
+					}`)}
+
+					obj.Status.AtProvider.Manifest = runtime.RawExtension{Raw: []byte(`{
+						"apiVersion": "v1",
+						"kind": "ConfigMap",
+						"metadata": {
+							"name": "old-name",
+							"namespace": "namespace"
+						}
+					}`)}
+
+					obj.Spec.DeletionPolicy = xpv1.DeletionOrphan
+				}),
+				client: resource.ClientApplicator{
+					Client: &test.MockClient{
+						MockGet: test.NewMockGetFn(nil, func(obj client.Object) error {
+							return kerrors.NewNotFound(schema.GroupResource{}, "")
+						}),
+					},
+				},
+			},
+			want: want{
+				out: managed.ExternalObservation{ResourceExists: false},
+				err: nil,
+			},
+		},
 	}
 	for name, tc := range cases {
 		t.Run(name, func(t *testing.T) {