Implicitly track resources loaded in the resolver (#141)

We can implicitly track all resources loaded via .Get() in the resolver without exposing the reconciler-runtime config as part of the contract. This is done by wrapping the config object, intercepting the calls to .Get() and then delegating those calls to .TrackAndGet(). This wrapper is only applied within the controller as the tracks are not useful outside of that context. Signed-off-by: Scott Andrews <[email protected]>
servicebinding · Aug 2, 2022 · f2b3695 · f2b3695
1 parent 2906cba
commit f2b3695
Show file tree

Hide file tree

Showing 5 changed files with 75 additions and 32 deletions.
diff --git a/controllers/client.go b/controllers/client.go
@@ -0,0 +1,36 @@
+/*
+Copyright 2022 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package controllers
+
+import (
+	"context"
+
+	"github.com/vmware-labs/reconciler-runtime/reconcilers"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+func TrackingClient(config reconcilers.Config) client.Client {
+	return &trackingClient{config}
+}
+
+type trackingClient struct {
+	reconcilers.Config
+}
+
+func (c *trackingClient) Get(ctx context.Context, key client.ObjectKey, obj client.Object) error {
+	return c.Config.TrackAndGet(ctx, key, obj)
+}
diff --git a/controllers/servicebinding_controller.go b/controllers/servicebinding_controller.go
@@ -74,7 +74,7 @@ func ResolveBindingSecret() reconcilers.SubReconciler {
 				Namespace:  resource.Namespace,
 				Name:       resource.Spec.Service.Name,
 			}
-			secretName, err := resolver.New(c).LookupBindingSecret(ctx, ref)
+			secretName, err := resolver.New(TrackingClient(c)).LookupBindingSecret(ctx, ref)
 			if err != nil {
 				if apierrs.IsNotFound(err) {
 					// leave Unknown, the provisioned service may be created shortly
@@ -120,7 +120,7 @@ func ResolveWorkloads() reconcilers.SubReconciler {
 				Namespace:  resource.Namespace,
 				Name:       resource.Spec.Workload.Name,
 			}
-			workloads, err := resolver.New(c).LookupWorkloads(ctx, ref, resource.Spec.Workload.Selector)
+			workloads, err := resolver.New(TrackingClient(c)).LookupWorkloads(ctx, ref, resource.Spec.Workload.Selector)
 			if err != nil {
 				if apierrs.IsNotFound(err) {
 					// leave Unknown, the workload may be created shortly
@@ -158,7 +158,7 @@ func ProjectBinding() reconcilers.SubReconciler {
 		SyncDuringFinalization: true,
 		Sync: func(ctx context.Context, resource *servicebindingv1beta1.ServiceBinding) error {
 			c := reconcilers.RetrieveConfigOrDie(ctx)
-			projector := projector.New(resolver.New(c))
+			projector := projector.New(resolver.New(TrackingClient(c)))
 
 			workloads := RetrieveWorkloads(ctx)
 			projectedWorkloads := make([]runtime.Object, len(workloads))

diff --git a/controllers/servicebinding_controller_test.go b/controllers/servicebinding_controller_test.go
@@ -76,6 +76,11 @@ func TestServiceBindingReconciler(t *testing.T) {
 			})
 		})
 
+	workloadMapping := dieservicebindingv1beta1.ClusterWorkloadResourceMappingBlank.
+		MetadataDie(func(d *diemetav1.ObjectMetaDie) {
+			d.Name("deployments.apps")
+		})
+
 	workload := dieappsv1.DeploymentBlank.
 		DieStamp(func(r *appsv1.Deployment) {
 			r.APIVersion = "apps/v1"
@@ -164,6 +169,7 @@ func TestServiceBindingReconciler(t *testing.T) {
 		},
 		ExpectTracks: []rtesting.TrackRequest{
 			rtesting.NewTrackRequest(projectedWorkload, serviceBinding, scheme),
+			rtesting.NewTrackRequest(workloadMapping, serviceBinding, scheme),
 		},
 	}, {
 		Name: "newly created",
@@ -174,6 +180,7 @@ func TestServiceBindingReconciler(t *testing.T) {
 		},
 		ExpectTracks: []rtesting.TrackRequest{
 			rtesting.NewTrackRequest(projectedWorkload, serviceBinding, scheme),
+			rtesting.NewTrackRequest(workloadMapping, serviceBinding, scheme),
 		},
 		ExpectEvents: []rtesting.Event{
 			rtesting.NewEvent(serviceBinding, scheme, corev1.EventTypeNormal, "FinalizerPatched", "Patched finalizer %q", "servicebinding.io/finalizer"),
@@ -219,6 +226,7 @@ func TestServiceBindingReconciler(t *testing.T) {
 		},
 		ExpectTracks: []rtesting.TrackRequest{
 			rtesting.NewTrackRequest(projectedWorkload, serviceBinding, scheme),
+			rtesting.NewTrackRequest(workloadMapping, serviceBinding, scheme),
 		},
 		ExpectEvents: []rtesting.Event{
 			rtesting.NewEvent(serviceBinding, scheme, corev1.EventTypeNormal, "Updated", "Updated Deployment %q", "my-workload"),
@@ -686,6 +694,11 @@ func TestProjectBinding(t *testing.T) {
 			})
 		})
 
+	workloadMapping := dieservicebindingv1beta1.ClusterWorkloadResourceMappingBlank.
+		MetadataDie(func(d *diemetav1.ObjectMetaDie) {
+			d.Name("deployments.apps")
+		})
+
 	workload := dieappsv1.DeploymentBlank.
 		DieStamp(func(r *appsv1.Deployment) {
 			r.APIVersion = "apps/v1"
@@ -772,6 +785,9 @@ func TestProjectBinding(t *testing.T) {
 				projectedWorkload.DieReleaseUnstructured(),
 			},
 		},
+		ExpectTracks: []rtesting.TrackRequest{
+			rtesting.NewTrackRequest(workloadMapping, serviceBinding, scheme),
+		},
 	}, {
 		Name: "unproject terminating workload",
 		Resource: serviceBinding.
@@ -795,6 +811,9 @@ func TestProjectBinding(t *testing.T) {
 				unprojectedWorkload,
 			},
 		},
+		ExpectTracks: []rtesting.TrackRequest{
+			rtesting.NewTrackRequest(workloadMapping, serviceBinding, scheme),
+		},
 	}}
 
 	rts.Run(t, scheme, func(t *testing.T, rtc *rtesting.SubReconcilerTestCase, c reconcilers.Config) reconcilers.SubReconciler {

diff --git a/resolver/cluster.go b/resolver/cluster.go
@@ -20,7 +20,6 @@ import (
 	"context"
 	"fmt"
 
-	"github.com/vmware-labs/reconciler-runtime/reconcilers"
 	corev1 "k8s.io/api/core/v1"
 	apierrs "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -33,28 +32,28 @@ import (
 	servicebindingv1beta1 "github.com/servicebinding/runtime/apis/v1beta1"
 )
 
-// New creates a new resolver backed by a reconciler-runtime config
-func New(config reconcilers.Config) Resolver {
+// New creates a new resolver backed by a controller-runtime client
+func New(client client.Client) Resolver {
 	return &clusterResolver{
-		config: config,
+		client: client,
 	}
 }
 
 type clusterResolver struct {
-	config reconcilers.Config
+	client client.Client
 }
 
 func (m *clusterResolver) LookupMapping(ctx context.Context, workload runtime.Object) (*servicebindingv1beta1.ClusterWorkloadResourceMappingTemplate, error) {
-	gvk, err := apiutil.GVKForObject(workload, m.config.Scheme())
+	gvk, err := apiutil.GVKForObject(workload, m.client.Scheme())
 	if err != nil {
 		return nil, err
 	}
-	rm, err := m.config.RESTMapper().RESTMapping(gvk.GroupKind(), gvk.Version)
+	rm, err := m.client.RESTMapper().RESTMapping(gvk.GroupKind(), gvk.Version)
 	if err != nil {
 		return nil, err
 	}
 	wrm := &servicebindingv1beta1.ClusterWorkloadResourceMapping{}
-	err = m.config.Get(ctx, types.NamespacedName{Name: fmt.Sprintf("%s.%s", rm.Resource.Resource, rm.Resource.Group)}, wrm)
+	err = m.client.Get(ctx, types.NamespacedName{Name: fmt.Sprintf("%s.%s", rm.Resource.Resource, rm.Resource.Group)}, wrm)
 	if err != nil {
 		if !apierrs.IsNotFound(err) {
 			return nil, err
@@ -91,7 +90,7 @@ func (r *clusterResolver) LookupBindingSecret(ctx context.Context, serviceRef co
 	service := &unstructured.Unstructured{}
 	service.SetAPIVersion(serviceRef.APIVersion)
 	service.SetKind(serviceRef.Kind)
-	if err := r.config.TrackAndGet(ctx, client.ObjectKey{Namespace: serviceRef.Namespace, Name: serviceRef.Name}, service); err != nil {
+	if err := r.client.Get(ctx, client.ObjectKey{Namespace: serviceRef.Namespace, Name: serviceRef.Name}, service); err != nil {
 		return "", err
 	}
 	secretName, exists, err := unstructured.NestedString(service.UnstructuredContent(), "status", "binding", "name")
@@ -115,7 +114,7 @@ func (r *clusterResolver) lookupWorkload(ctx context.Context, workloadRef corev1
 	workload := &unstructured.Unstructured{}
 	workload.SetAPIVersion(workloadRef.APIVersion)
 	workload.SetKind(workloadRef.Kind)
-	if err := r.config.TrackAndGet(ctx, client.ObjectKey{Namespace: workloadRef.Namespace, Name: workloadRef.Name}, workload); err != nil {
+	if err := r.client.Get(ctx, client.ObjectKey{Namespace: workloadRef.Namespace, Name: workloadRef.Name}, workload); err != nil {
 		return nil, err
 	}
 	return workload, nil
@@ -129,7 +128,7 @@ func (r *clusterResolver) lookupWorkloads(ctx context.Context, workloadRef corev
 	if err != nil {
 		return nil, err
 	}
-	if err := r.config.List(ctx, workloads, client.InNamespace(workloadRef.Namespace), client.MatchingLabelsSelector{Selector: ls}); err != nil {
+	if err := r.client.List(ctx, workloads, client.InNamespace(workloadRef.Namespace), client.MatchingLabelsSelector{Selector: ls}); err != nil {
 		return nil, err
 	}
 

diff --git a/resolver/cluster_test.go b/resolver/cluster_test.go
@@ -21,9 +21,7 @@ import (
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
-	"github.com/vmware-labs/reconciler-runtime/reconcilers"
 	rtesting "github.com/vmware-labs/reconciler-runtime/testing"
-	"github.com/vmware-labs/reconciler-runtime/tracker"
 	appsv1 "k8s.io/api/apps/v1"
 	batchv1 "k8s.io/api/batch/v1"
 	corev1 "k8s.io/api/core/v1"
@@ -277,14 +275,11 @@ func TestClusterResolver_LookupMapping(t *testing.T) {
 		t.Run(c.name, func(t *testing.T) {
 			ctx := context.TODO()
 
-			config := reconcilers.Config{
-				Client:  rtesting.NewFakeClient(scheme, c.givenObjects...),
-				Tracker: tracker.New(0),
-			}
-			restMapper := config.RESTMapper().(*meta.DefaultRESTMapper)
+			client := rtesting.NewFakeClient(scheme, c.givenObjects...)
+			restMapper := client.RESTMapper().(*meta.DefaultRESTMapper)
 			restMapper.Add(schema.GroupVersionKind{Group: "apps", Version: "v1", Kind: "Deployment"}, meta.RESTScopeNamespace)
 			restMapper.Add(schema.GroupVersionKind{Group: "batch", Version: "v1", Kind: "CronJob"}, meta.RESTScopeNamespace)
-			resolver := resolver.New(config)
+			resolver := resolver.New(client)
 
 			actual, err := resolver.LookupMapping(ctx, c.workload)
 
@@ -390,11 +385,8 @@ func TestClusterResolver_LookupBindingSecret(t *testing.T) {
 		t.Run(c.name, func(t *testing.T) {
 			ctx := context.TODO()
 
-			config := reconcilers.Config{
-				Client:  rtesting.NewFakeClient(scheme, c.givenObjects...),
-				Tracker: tracker.New(0),
-			}
-			resolver := resolver.New(config)
+			client := rtesting.NewFakeClient(scheme, c.givenObjects...)
+			resolver := resolver.New(client)
 
 			actual, err := resolver.LookupBindingSecret(ctx, c.serviceRef)
 
@@ -693,11 +685,8 @@ func TestClusterResolver_LookupWorkloads(t *testing.T) {
 		t.Run(c.name, func(t *testing.T) {
 			ctx := context.TODO()
 
-			config := reconcilers.Config{
-				Client:  rtesting.NewFakeClient(scheme, c.givenObjects...),
-				Tracker: tracker.New(0),
-			}
-			resolver := resolver.New(config)
+			client := rtesting.NewFakeClient(scheme, c.givenObjects...)
+			resolver := resolver.New(client)
 
 			actual, err := resolver.LookupWorkloads(ctx, c.serviceRef, c.selector)