feat(variables): early return on deletion skips secret retrieval (#238)

Signed-off-by: Markus Siebert <[email protected]>

Author: markussiebert

pkg/cluster/controller/groups/variables/zz_controller.go

@@ -131,6 +131,11 @@ func (e *external) Observe(ctx context.Context, mg resource.Managed) (managed.Ex
 		return managed.ExternalObservation{}, errors.Wrap(err, errGetFailed)
 	}
 
+	// Deleting: only need to determine external resource still exists.
+	if !cr.ObjectMeta.DeletionTimestamp.IsZero() {
+		return managed.ExternalObservation{ResourceExists: true}, nil
+	}
+
 	if cr.Spec.ForProvider.ValueSecretRef != nil {
 		if err = e.updateVariableFromSecret(mg, ctx, cr.Spec.ForProvider.ValueSecretRef, &cr.Spec.ForProvider); err != nil {
 			return managed.ExternalObservation{}, errors.Wrap(err, errUpdateFailed)

pkg/cluster/controller/groups/variables/zz_controller_test.go

@@ -28,6 +28,7 @@ import (
 	"github.com/google/go-cmp/cmp"
 	gitlab "gitlab.com/gitlab-org/api/client-go"
 	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	"github.com/crossplane-contrib/provider-gitlab/apis/namespaced/groups/v1alpha1"
@@ -133,6 +134,14 @@ func withEnvironmentScope(scope string) variableModifier {
 	}
 }
 
+var deletionTime = metav1.Now()
+
+func withDeletionTimestamp() variableModifier {
+	return func(r *v1alpha1.Variable) {
+		r.ObjectMeta.DeletionTimestamp = &deletionTime
+	}
+}
+
 func variable(m ...variableModifier) *v1alpha1.Variable {
 	cr := &v1alpha1.Variable{}
 	for _, f := range m {
@@ -360,6 +369,36 @@ func TestObserve(t *testing.T) {
 				err: errors.Wrap(errors.New(common.ErrSecretKeyNotFound), errGetFailed),
 			},
 		},
+
+		"DeletingEarlyReturnSkipsSecret": {
+			args: args{
+				kube: &test.MockClient{
+					MockGet: func(_ context.Context, key client.ObjectKey, obj client.Object) error { return errBoom },
+				},
+				variable: &fake.MockClient{
+					MockGetGroupVariable: func(gid interface{}, key string, opt *gitlab.GetGroupVariableOptions, options ...gitlab.RequestOptionFunc) (*gitlab.GroupVariable, *gitlab.Response, error) {
+						return &pv, &gitlab.Response{}, nil
+					},
+				},
+				cr: variable(
+					withGroupID(groupID),
+					withKey(variableKey),
+					withValueSecretRef(common.TestCreateLocalSecretKeySelector("something", "blah")),
+					withDeletionTimestamp(),
+					withConditions(xpv1.Deleting()),
+				),
+			},
+			want: want{
+				cr: variable(
+					withGroupID(groupID),
+					withKey(variableKey),
+					withValueSecretRef(common.TestCreateLocalSecretKeySelector("something", "blah")),
+					withDeletionTimestamp(),
+					withConditions(xpv1.Deleting()),
+				),
+				result: managed.ExternalObservation{ResourceExists: true},
+			},
+		},
 	}
 
 	for name, tc := range cases {

pkg/cluster/controller/projects/variables/zz_controller.go

@@ -131,6 +131,11 @@ func (e *external) Observe(ctx context.Context, mg resource.Managed) (managed.Ex
 		return managed.ExternalObservation{}, errors.Wrap(err, errGetFailed)
 	}
 
+	// Deleting: only need to determine external resource still exists.
+	if !cr.ObjectMeta.DeletionTimestamp.IsZero() {
+		return managed.ExternalObservation{ResourceExists: true}, nil
+	}
+
 	if cr.Spec.ForProvider.ValueSecretRef != nil {
 		if err = e.updateVariableFromSecret(mg, ctx, cr.Spec.ForProvider.ValueSecretRef, &cr.Spec.ForProvider); err != nil {
 			return managed.ExternalObservation{}, errors.Wrap(err, errUpdateFailed)

pkg/cluster/controller/projects/variables/zz_controller_test.go

@@ -28,6 +28,7 @@ import (
 	"github.com/google/go-cmp/cmp"
 	gitlab "gitlab.com/gitlab-org/api/client-go"
 	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	"github.com/crossplane-contrib/provider-gitlab/apis/namespaced/projects/v1alpha1"
@@ -133,6 +134,14 @@ func withEnvironmentScope(scope string) variableModifier {
 	}
 }
 
+var deletionTime = metav1.Now()
+
+func withDeletionTimestamp() variableModifier {
+	return func(r *v1alpha1.Variable) {
+		r.ObjectMeta.DeletionTimestamp = &deletionTime
+	}
+}
+
 func variable(m ...variableModifier) *v1alpha1.Variable {
 	cr := &v1alpha1.Variable{}
 	for _, f := range m {
@@ -360,6 +369,35 @@ func TestObserve(t *testing.T) {
 				err: errors.Wrap(errors.New(common.ErrSecretKeyNotFound), errGetFailed),
 			},
 		},
+		"DeletingEarlyReturnSkipsSecret": {
+			args: args{
+				kube: &test.MockClient{
+					MockGet: func(_ context.Context, key client.ObjectKey, obj client.Object) error { return errBoom },
+				},
+				variable: &fake.MockClient{
+					MockGetVariable: func(pid interface{}, key string, opt *gitlab.GetProjectVariableOptions, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectVariable, *gitlab.Response, error) {
+						return &pv, &gitlab.Response{}, nil
+					},
+				},
+				cr: variable(
+					withProjectID(projectID),
+					withKey(variableKey),
+					withValueSecretRef(common.TestCreateLocalSecretKeySelector("", "blah")),
+					withDeletionTimestamp(),
+					withConditions(xpv1.Deleting()),
+				),
+			},
+			want: want{
+				cr: variable(
+					withProjectID(projectID),
+					withKey(variableKey),
+					withValueSecretRef(common.TestCreateLocalSecretKeySelector("", "blah")),
+					withDeletionTimestamp(),
+					withConditions(xpv1.Deleting()),
+				),
+				result: managed.ExternalObservation{ResourceExists: true},
+			},
+		},
 	}
 
 	for name, tc := range cases {