From 062df46e47d0ea46d7b5e467796732c1aff85d8f Mon Sep 17 00:00:00 2001
From: roy wang <seiwy2010@gmail.com>
Date: Wed, 10 Feb 2021 15:44:15 +0900
Subject: [PATCH] fix ScopeDefinition not found blocking finalizer

Signed-off-by: roy wang <seiwy2010@gmail.com>
---
 .../applicationconfiguration/apply.go         |  7 +++
 .../applicationconfiguration/apply_test.go    | 56 +++++++++++++++++++
 2 files changed, 63 insertions(+)

diff --git a/pkg/controller/core.oam.dev/v1alpha2/applicationconfiguration/apply.go b/pkg/controller/core.oam.dev/v1alpha2/applicationconfiguration/apply.go
index b2fe29047..93a767a32 100644
--- a/pkg/controller/core.oam.dev/v1alpha2/applicationconfiguration/apply.go
+++ b/pkg/controller/core.oam.dev/v1alpha2/applicationconfiguration/apply.go
@@ -378,6 +378,8 @@ func (a *workloads) applyScope(ctx context.Context, wl Workload, s unstructured.
 	return nil
 }
 
+// applyScopeRemoval remove the workload reference from the scope's reference list.
+// If the scope or scope definition is not found(deleted), it's still regarded as remove successfully.
 func (a *workloads) applyScopeRemoval(ctx context.Context, namespace string, wr runtimev1alpha1.TypedReference, s v1alpha2.WorkloadScope) error {
 	scopeObject := unstructured.Unstructured{}
 	scopeObject.SetAPIVersion(s.Reference.APIVersion)
@@ -394,6 +396,11 @@ func (a *workloads) applyScopeRemoval(ctx context.Context, namespace string, wr
 
 	scopeDefinition, err := util.FetchScopeDefinition(ctx, a.rawClient, a.dm, &scopeObject)
 	if err != nil {
+		if apierrors.IsNotFound(err) {
+			// if the scope definition is deleted
+			// treat it as removal done to avoid blocking AppConfig finalizer
+			return nil
+		}
 		return errors.Wrapf(err, errFmtGetScopeDefinition, scopeObject.GetAPIVersion(), scopeObject.GetKind(), scopeObject.GetName())
 	}
 
diff --git a/pkg/controller/core.oam.dev/v1alpha2/applicationconfiguration/apply_test.go b/pkg/controller/core.oam.dev/v1alpha2/applicationconfiguration/apply_test.go
index f950e00b7..fedd9cbac 100644
--- a/pkg/controller/core.oam.dev/v1alpha2/applicationconfiguration/apply_test.go
+++ b/pkg/controller/core.oam.dev/v1alpha2/applicationconfiguration/apply_test.go
@@ -27,9 +27,11 @@ import (
 	"github.com/crossplane/crossplane-runtime/pkg/test"
 	"github.com/google/go-cmp/cmp"
 	"github.com/pkg/errors"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
@@ -325,6 +327,60 @@ func TestApplyWorkloads(t *testing.T) {
 				},
 			},
 		},
+		"SuccessRemovingWhenScopeDefinitionNotFound": {
+			reason:     "ScopeDefinition not found should not block dereference",
+			applicator: ApplyFn(func(_ context.Context, o runtime.Object, _ ...apply.ApplyOption) error { return nil }),
+			rawClient: &test.MockClient{
+				MockGet: func(_ context.Context, key client.ObjectKey, obj runtime.Object) error {
+					if key.Name == scope.GetName() {
+						scope := obj.(*unstructured.Unstructured)
+
+						refs := []interface{}{
+							map[string]interface{}{
+								"apiVersion": workload.GetAPIVersion(),
+								"kind":       workload.GetKind(),
+								"name":       workload.GetName(),
+							},
+						}
+
+						if err := fieldpath.Pave(scope.UnstructuredContent()).SetValue("spec.workloadRefs", refs); err == nil {
+							return err
+						}
+
+						return nil
+					}
+					if _, ok := obj.(*v1alpha2.ScopeDefinition); ok {
+						return apierrors.NewNotFound(schema.GroupResource{}, "test")
+					}
+					return nil
+				},
+			},
+			args: args{
+				w: []Workload{{
+					Workload: workload,
+					Traits:   []*Trait{{Object: *trait.DeepCopy()}},
+					Scopes:   []unstructured.Unstructured{},
+				}},
+				ws: []v1alpha2.WorkloadStatus{
+					{
+						Reference: v1alpha1.TypedReference{
+							APIVersion: workload.GetAPIVersion(),
+							Kind:       workload.GetKind(),
+							Name:       workload.GetName(),
+						},
+						Scopes: []v1alpha2.WorkloadScope{
+							{
+								Reference: v1alpha1.TypedReference{
+									APIVersion: scope.GetAPIVersion(),
+									Kind:       scope.GetKind(),
+									Name:       scope.GetName(),
+								},
+							},
+						},
+					},
+				},
+			},
+		},
 	}
 
 	for name, tc := range cases {