diff --git a/internal/controller/object/object.go b/internal/controller/object/object.go index a8192af..bcda0c6 100644 --- a/internal/controller/object/object.go +++ b/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(), diff --git a/internal/controller/object/object_test.go b/internal/controller/object/object_test.go index 14d4429..cccdea1 100644 --- a/internal/controller/object/object_test.go +++ b/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) {