From 13ee03912b27b3672e9aaee6456b196f789c1420 Mon Sep 17 00:00:00 2001
From: Branden Rolston <branden@upbound.io>
Date: Tue, 13 Sep 2022 18:36:33 -0700
Subject: [PATCH 1/2] Assert finalizers in RemoveFinalizer() test

Signed-off-by: Branden Rolston <branden@upbound.io>
(cherry picked from commit dd2d9881030036d293c0d3dffc68ed84faaa6e21)
---
 internal/controller/object/object_test.go | 30 +++++++++++++++++------
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/internal/controller/object/object_test.go b/internal/controller/object/object_test.go
index 12b6eb86..58e3f18d 100644
--- a/internal/controller/object/object_test.go
+++ b/internal/controller/object/object_test.go
@@ -22,6 +22,7 @@ import (
 	"time"
 
 	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
@@ -1274,7 +1275,8 @@ func Test_objFinalizer_RemoveFinalizer(t *testing.T) {
 		mg     resource.Managed
 	}
 	type want struct {
-		err error
+		err        error
+		finalizers []string
 	}
 	cases := map[string]struct {
 		args
@@ -1285,7 +1287,8 @@ func Test_objFinalizer_RemoveFinalizer(t *testing.T) {
 				mg: notKubernetesObject{},
 			},
 			want: want{
-				err: errors.New(errNotKubernetesObject),
+				err:        errors.New(errNotKubernetesObject),
+				finalizers: []string{},
 			},
 		},
 		"FailedToRemoveObjectFinalizer": {
@@ -1300,7 +1303,8 @@ func Test_objFinalizer_RemoveFinalizer(t *testing.T) {
 				},
 			},
 			want: want{
-				err: errors.Wrap(errBoom, errRemoveFinalizer),
+				err:        errors.Wrap(errBoom, errRemoveFinalizer),
+				finalizers: []string{},
 			},
 		},
 		"NoObjectFinalizerExists": {
@@ -1308,7 +1312,8 @@ func Test_objFinalizer_RemoveFinalizer(t *testing.T) {
 				mg: kubernetesObject(),
 			},
 			want: want{
-				err: nil,
+				err:        nil,
+				finalizers: nil,
 			},
 		},
 		"NoReferenceFinalizerExists": {
@@ -1328,7 +1333,8 @@ func Test_objFinalizer_RemoveFinalizer(t *testing.T) {
 				},
 			},
 			want: want{
-				err: nil,
+				err:        nil,
+				finalizers: []string{},
 			},
 		},
 		"FailedToRemoveReferenceFinalizer": {
@@ -1358,6 +1364,7 @@ func Test_objFinalizer_RemoveFinalizer(t *testing.T) {
 				err: errors.Wrap(
 					errors.Wrap(errBoom,
 						errRemoveReferenceFinalizer), errRemoveFinalizer),
+				finalizers: []string{},
 			},
 		},
 		"Success": {
@@ -1378,7 +1385,8 @@ func Test_objFinalizer_RemoveFinalizer(t *testing.T) {
 				},
 			},
 			want: want{
-				err: nil,
+				err:        nil,
+				finalizers: []string{},
 			},
 		},
 	}
@@ -1387,9 +1395,17 @@ func Test_objFinalizer_RemoveFinalizer(t *testing.T) {
 			f := &objFinalizer{
 				client: tc.args.client,
 			}
+
 			gotErr := f.RemoveFinalizer(context.Background(), tc.args.mg)
 			if diff := cmp.Diff(tc.want.err, gotErr, test.EquateErrors()); diff != "" {
-				t.Fatalf("f.RemoveFinalizer(...): -want error, +got error: %s", diff)
+				t.Errorf("f.RemoveFinalizer(...): -want error, +got error: %s", diff)
+			}
+
+			if _, ok := tc.args.mg.(*v1alpha1.Object); ok {
+				sort := cmpopts.SortSlices(func(a, b string) bool { return a < b })
+				if diff := cmp.Diff(tc.want.finalizers, tc.args.mg.GetFinalizers(), sort); diff != "" {
+					t.Errorf("managed resource finalizers: -want, +got: %s", diff)
+				}
 			}
 		})
 	}

From e3c8573cdec5d83593507cdec0f4b4d425928291 Mon Sep 17 00:00:00 2001
From: Branden Rolston <branden@upbound.io>
Date: Fri, 9 Sep 2022 18:28:54 -0700
Subject: [PATCH 2/2] Remove finalizers from referenced resources first

Signed-off-by: Branden Rolston <branden@upbound.io>
(cherry picked from commit 220ca4dc1fcc8cc0102c9c69b1beb0b0d756cc57)
---
 internal/controller/object/object.go      | 22 +++++++++++-----------
 internal/controller/object/object_test.go |  2 +-
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/internal/controller/object/object.go b/internal/controller/object/object.go
index 0e57383f..a15e3e7e 100644
--- a/internal/controller/object/object.go
+++ b/internal/controller/object/object.go
@@ -533,18 +533,8 @@ func (f *objFinalizer) RemoveFinalizer(ctx context.Context, res resource.Object)
 		return errors.New(errNotKubernetesObject)
 	}
 
-	if !meta.FinalizerExists(obj, objFinalizerName) {
-		return nil
-	}
-	meta.RemoveFinalizer(obj, objFinalizerName)
-
-	err := f.client.Update(ctx, obj)
-	if err != nil {
-		return errors.Wrap(err, errRemoveFinalizer)
-	}
-
 	// Remove finalizer from referenced resources if exists
-	err = f.handleRefFinalizer(ctx, obj, func(
+	err := f.handleRefFinalizer(ctx, obj, func(
 		ctx context.Context, res *unstructured.Unstructured, finalizer string) error {
 		if meta.FinalizerExists(res, finalizer) {
 			meta.RemoveFinalizer(res, finalizer)
@@ -554,5 +544,15 @@ func (f *objFinalizer) RemoveFinalizer(ctx context.Context, res resource.Object)
 		}
 		return nil
 	})
+	if err != nil {
+		return errors.Wrap(err, errRemoveFinalizer)
+	}
+
+	if !meta.FinalizerExists(obj, objFinalizerName) {
+		return nil
+	}
+	meta.RemoveFinalizer(obj, objFinalizerName)
+
+	err = f.client.Update(ctx, obj)
 	return errors.Wrap(err, errRemoveFinalizer)
 }
diff --git a/internal/controller/object/object_test.go b/internal/controller/object/object_test.go
index 58e3f18d..b74950d1 100644
--- a/internal/controller/object/object_test.go
+++ b/internal/controller/object/object_test.go
@@ -1364,7 +1364,7 @@ func Test_objFinalizer_RemoveFinalizer(t *testing.T) {
 				err: errors.Wrap(
 					errors.Wrap(errBoom,
 						errRemoveReferenceFinalizer), errRemoveFinalizer),
-				finalizers: []string{},
+				finalizers: []string{objFinalizerName},
 			},
 		},
 		"Success": {