Merge pull request #11 from authzed/existence-check-adopt

Add existence checking to adoption handler

adopt/adopt.go

@@ -29,11 +29,12 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 
+	"github.com/go-logr/logr"
+
 	"github.com/authzed/controller-idioms/handler"
 	"github.com/authzed/controller-idioms/queue"
 	"github.com/authzed/controller-idioms/typed"
 	"github.com/authzed/controller-idioms/typedctx"
-	"github.com/go-logr/logr"
 )
 
 // TODO: a variant where there can only be one owner (label only, fail if labelled for someone else)
@@ -73,12 +74,25 @@ type Object interface {
 // be applied with a client-go Patch call, and return a Secret.
 type ApplyFunc[K Object, A Adoptable[A]] func(ctx context.Context, object A, opts metav1.ApplyOptions) (result K, err error)
 
+// ExistsFunc should return nil if the object exists in the cluster, and an
+// error otherwise.
+type ExistsFunc func(ctx context.Context, nn types.NamespacedName) error
+
 // IndexKeyFunc returns the name of an index to use and the value to query it for.
 type IndexKeyFunc func(ctx context.Context) (indexName string, indexValue string)
 
 // Owned is used in object annotations to indicate an object is managed.
 const Owned = "owned"
 
+var (
+	// AlwaysExistsFunc is an ExistsFunc that always returns nil
+	AlwaysExistsFunc ExistsFunc = func(ctx context.Context, nn types.NamespacedName) error {
+		return nil
+	}
+	// NoopObjectMissingFunc is an ObjectMissing func that does nothing
+	NoopObjectMissingFunc = func(ctx context.Context, err error) {}
+)
+
 // AdoptionHandler implements handler.Handler to "adopt" an existing resource
 // under the controller's management. See the package description for more info.
 type AdoptionHandler[K Object, A Adoptable[A]] struct {
@@ -107,6 +121,9 @@ type AdoptionHandler[K Object, A Adoptable[A]] struct {
 	// ObjectAdoptedFunc is called when an adoption was performed
 	ObjectAdoptedFunc func(ctx context.Context, obj K)
 
+	// ObjectMissingFunc is called when the object cannot be found
+	ObjectMissingFunc func(ctx context.Context, err error)
+
 	// TODO: GetFromCache and Indexer could be replaced with an informerfactory
 	//  that can be used to get both
 
@@ -142,6 +159,9 @@ type AdoptionHandler[K Object, A Adoptable[A]] struct {
 	// ApplyFunc applies adoption-related changes to the object to the cluster
 	ApplyFunc ApplyFunc[K, A]
 
+	// ExistsFunc checks if the object to be adopted exists in the cluster
+	ExistsFunc ExistsFunc
+
 	// Next is the next handler in the chain (use NoopHandler if not chaining)
 	Next handler.ContextHandler
 }
@@ -151,6 +171,14 @@ func (s *AdoptionHandler[K, A]) Handle(ctx context.Context) {
 	adoptee := s.AdopteeCtx.MustValue(ctx)
 	owner := s.OwnerCtx.MustValue(ctx)
 
+	if s.ExistsFunc == nil {
+		s.ExistsFunc = AlwaysExistsFunc
+	}
+
+	if s.ObjectMissingFunc == nil {
+		s.ObjectMissingFunc = NoopObjectMissingFunc
+	}
+
 	// adoptee may be empty, but the handler still runs so that it can clean up
 	// any old references that may remain.
 
@@ -164,6 +192,12 @@ func (s *AdoptionHandler[K, A]) Handle(ctx context.Context) {
 	// this apply uses the controller as field manager, since it will be the
 	// same for all owners.
 	if len(adoptee.Name) > 0 && errors.IsNotFound(err) {
+		// check if object exists at all before applying
+		logger.V(5).Info("checking if object exists", "object", adoptee)
+		if err := s.ExistsFunc(ctx, adoptee); err != nil {
+			s.ObjectMissingFunc(ctx, err)
+			return
+		}
 		logger.V(5).Info("labelling object to make it visible to the index",
 			"adoptee", adoptee.String(),
 			"manager", s.ControllerFieldManager,

adopt/adopt_test.go

@@ -65,19 +65,21 @@ func TestSecretAdopterHandler(t *testing.T) {
 		},
 	}
 	tests := []struct {
-		name                string
-		secretName          string
-		cluster             types.NamespacedName
-		secretInCache       *corev1.Secret
-		cacheErr            error
-		secretsInIndex      []*corev1.Secret
-		applyCalls          []*applyCall
-		expectEvents        []string
-		expectNext          bool
-		expectRequeueErr    error
-		expectRequeueAPIErr error
-		expectRequeue       bool
-		expectCtxSecret     *corev1.Secret
+		name                   string
+		secretName             string
+		cluster                types.NamespacedName
+		secretInCache          *corev1.Secret
+		cacheErr               error
+		secretsInIndex         []*corev1.Secret
+		secretExistsErr        error
+		applyCalls             []*applyCall
+		expectEvents           []string
+		expectNext             bool
+		expectRequeueErr       error
+		expectRequeueAPIErr    error
+		expectRequeue          bool
+		expectObjectMissingErr error
+		expectCtxSecret        *corev1.Secret
 	}{
 		{
 			name: "no secret",
@@ -89,6 +91,19 @@ func TestSecretAdopterHandler(t *testing.T) {
 			applyCalls: []*applyCall{},
 			expectNext: true,
 		},
+		{
+			name:       "secret does not exist",
+			secretName: "secret",
+			cluster: types.NamespacedName{
+				Namespace: "test",
+				Name:      "test",
+			},
+			cacheErr:               secretNotFound("test"),
+			secretExistsErr:        secretNotFound("test"),
+			secretsInIndex:         []*corev1.Secret{},
+			applyCalls:             []*applyCall{},
+			expectObjectMissingErr: secretNotFound("test"),
+		},
 		{
 			name:       "secret needs adopting",
 			secretName: "secret",
@@ -312,6 +327,9 @@ func TestSecretAdopterHandler(t *testing.T) {
 				func(ctx context.Context) (*corev1.Secret, error) {
 					return tt.secretInCache, tt.cacheErr
 				},
+				func(ctx context.Context, err error) {
+					require.Equal(t, tt.expectObjectMissingErr, err)
+				},
 				typed.NewIndexer[*corev1.Secret](indexer),
 				func(ctx context.Context, secret *applycorev1.SecretApplyConfiguration, opts metav1.ApplyOptions) (result *corev1.Secret, err error) {
 					defer func() { applyCallIndex++ }()
@@ -320,6 +338,9 @@ func TestSecretAdopterHandler(t *testing.T) {
 					require.Equal(t, call.input, secret, "error on call %d", applyCallIndex)
 					return call.result, call.err
 				},
+				func(ctx context.Context, nn types.NamespacedName) error {
+					return tt.secretExistsErr
+				},
 				handler.NewHandlerFromFunc(func(ctx context.Context) {
 					nextCalled = true
 					require.Equal(t, tt.expectCtxSecret, CtxSecret.Value(ctx))
@@ -347,7 +368,7 @@ func TestSecretAdopterHandler(t *testing.T) {
 	}
 }
 
-func NewSecretAdoptionHandler(recorder record.EventRecorder, getFromCache func(ctx context.Context) (*corev1.Secret, error), secretIndexer *typed.Indexer[*corev1.Secret], secretApplyFunc ApplyFunc[*corev1.Secret, *applycorev1.SecretApplyConfiguration], next handler.Handler) handler.Handler {
+func NewSecretAdoptionHandler(recorder record.EventRecorder, getFromCache func(ctx context.Context) (*corev1.Secret, error), missingFunc func(context.Context, error), secretIndexer *typed.Indexer[*corev1.Secret], secretApplyFunc ApplyFunc[*corev1.Secret, *applycorev1.SecretApplyConfiguration], secretExistsFunc ExistsFunc, next handler.Handler) handler.Handler {
 	return handler.NewHandler(&AdoptionHandler[*corev1.Secret, *applycorev1.SecretApplyConfiguration]{
 		OperationsContext:      QueueOps,
 		ControllerFieldManager: "test-controller",
@@ -357,10 +378,11 @@ func NewSecretAdoptionHandler(recorder record.EventRecorder, getFromCache func(c
 		ObjectAdoptedFunc: func(ctx context.Context, secret *corev1.Secret) {
 			recorder.Eventf(secret, corev1.EventTypeNormal, EventSecretAdopted, "Secret was referenced by %s; it has been labelled to mark it as part of the configuration for that controller.", CtxOwnerNN.MustValue(ctx).String())
 		},
-		GetFromCache: getFromCache,
-		Indexer:      secretIndexer,
-		IndexName:    IndexName,
-		Labels:       map[string]string{ManagedLabelKey: ManagedLabelValue},
+		ObjectMissingFunc: missingFunc,
+		GetFromCache:      getFromCache,
+		Indexer:           secretIndexer,
+		IndexName:         IndexName,
+		Labels:            map[string]string{ManagedLabelKey: ManagedLabelValue},
 		NewPatch: func(nn types.NamespacedName) *applycorev1.SecretApplyConfiguration {
 			return applycorev1.Secret(nn.Name, nn.Namespace)
 		},
@@ -371,14 +393,12 @@ func NewSecretAdoptionHandler(recorder record.EventRecorder, getFromCache func(c
 		OwnerFieldManagerFunc: func(owner types.NamespacedName) string {
 			return "my-owner-" + owner.Namespace + "-" + owner.Name
 		},
-		ApplyFunc: secretApplyFunc,
-		Next:      next,
+		ApplyFunc:  secretApplyFunc,
+		ExistsFunc: secretExistsFunc,
+		Next:       next,
 	}, "adoptSecret")
 }
 
-func ExampleAdoptionHandler_Handle() {
-}
-
 func ExpectEvents(t *testing.T, recorder *record.FakeRecorder, expected []string) {
 	close(recorder.Events)
 	events := make([]string, 0)