Repository: knative/pkg
Pull request #1719 (2020-09-18 20:19:27)

Various nits around the code
/assign mattmoor @tcnghia 

knative-prow-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/1719#" title="Author self-approved">vagababov</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative%2Fpkg).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[apis/OWNERS](https://github.com/knative/pkg/blob/master/apis/OWNERS)~~ [vagababov]
- ~~[metrics/OWNERS](https://github.com/knative/pkg/blob/master/metrics/OWNERS)~~ [vagababov]
- ~~[network/OWNERS](https://github.com/knative/pkg/blob/master/network/OWNERS)~~ [vagababov]
- ~~[reconciler/OWNERS](https://github.com/knative/pkg/blob/master/reconciler/OWNERS)~~ [vagababov]
- ~~[test/OWNERS](https://github.com/knative/pkg/blob/master/test/OWNERS)~~ [vagababov]
- ~~[testutils/OWNERS](https://github.com/knative/pkg/blob/master/testutils/OWNERS)~~ [vagababov]
- ~~[webhook/OWNERS](https://github.com/knative/pkg/blob/master/webhook/OWNERS)~~ [vagababov]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
codecov[bot]: # [Codecov](https://codecov.io/gh/knative/pkg/pull/1719?src=pr&el=h1) Report
> Merging [#1719](https://codecov.io/gh/knative/pkg/pull/1719?src=pr&el=desc) into [master](https://codecov.io/gh/knative/pkg/commit/6ce80a7c271ad5dc321ed6b48bf400d36f63dffb?el=desc) will **increase** coverage by `0.02%`.
> The diff coverage is `0.00%`.

[![Impacted file tree graph](https://codecov.io/gh/knative/pkg/pull/1719/graphs/tree.svg?width=650&height=150&src=pr&token=rlqpDnfxbO)](https://codecov.io/gh/knative/pkg/pull/1719?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #1719      +/-   ##
==========================================
+ Coverage   65.99%   66.01%   +0.02%     
==========================================
  Files         240      240              
  Lines        9944     9944              
==========================================
+ Hits         6563     6565       +2     
+ Misses       3004     3003       -1     
+ Partials      377      376       -1     
```


| [Impacted Files](https://codecov.io/gh/knative/pkg/pull/1719?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [apis/field\_error.go](https://codecov.io/gh/knative/pkg/pull/1719/diff?src=pr&el=tree#diff-YXBpcy9maWVsZF9lcnJvci5nbw==) | `98.29% <ø> (ø)` | |
| [metrics/utils.go](https://codecov.io/gh/knative/pkg/pull/1719/diff?src=pr&el=tree#diff-bWV0cmljcy91dGlscy5nbw==) | `0.00% <ø> (ø)` | |
| [network/transports.go](https://codecov.io/gh/knative/pkg/pull/1719/diff?src=pr&el=tree#diff-bmV0d29yay90cmFuc3BvcnRzLmdv) | `92.50% <ø> (ø)` | |
| [reconciler/filter.go](https://codecov.io/gh/knative/pkg/pull/1719/diff?src=pr&el=tree#diff-cmVjb25jaWxlci9maWx0ZXIuZ28=) | `100.00% <ø> (ø)` | |
| [test/vegeta/pacers/steady\_up\_pacer.go](https://codecov.io/gh/knative/pkg/pull/1719/diff?src=pr&el=tree#diff-dGVzdC92ZWdldGEvcGFjZXJzL3N0ZWFkeV91cF9wYWNlci5nbw==) | `83.78% <ø> (ø)` | |
| [...webhook-apicoverage/resourcetree/structkindnode.go](https://codecov.io/gh/knative/pkg/pull/1719/diff?src=pr&el=tree#diff-dGVzdC93ZWJob29rLWFwaWNvdmVyYWdlL3Jlc291cmNldHJlZS9zdHJ1Y3RraW5kbm9kZS5nbw==) | `48.48% <0.00%> (ø)` | |
| [...estutils/clustermanager/e2e-tests/boskos/boskos.go](https://codecov.io/gh/knative/pkg/pull/1719/diff?src=pr&el=tree#diff-dGVzdHV0aWxzL2NsdXN0ZXJtYW5hZ2VyL2UyZS10ZXN0cy9ib3Nrb3MvYm9za29zLmdv) | `76.47% <ø> (ø)` | |
| [webhook/certificates/resources/certs.go](https://codecov.io/gh/knative/pkg/pull/1719/diff?src=pr&el=tree#diff-d2ViaG9vay9jZXJ0aWZpY2F0ZXMvcmVzb3VyY2VzL2NlcnRzLmdv) | `62.02% <ø> (ø)` | |
| [test/gcs/mock/mock.go](https://codecov.io/gh/knative/pkg/pull/1719/diff?src=pr&el=tree#diff-dGVzdC9nY3MvbW9jay9tb2NrLmdv) | `91.39% <0.00%> (+1.07%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/knative/pkg/pull/1719?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/knative/pkg/pull/1719?src=pr&el=footer). Last update [6ce80a7...600275a](https://codecov.io/gh/knative/pkg/pull/1719?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/.codecov.yaml b/.codecov.yaml
index e63994c53..a0a3ae073 100644
--- a/.codecov.yaml
+++ b/.codecov.yaml
@@ -2,5 +2,7 @@ ignore:
   - "**/zz_generated*.go" # Ignore generated files.
   - "client"
   - "hack"
+  - "reconciler/testing"
+  - "testing"
   - "third_party"
   - "vendor"
diff --git a/apis/field_error.go b/apis/field_error.go
index cc353b58b..3997b5944 100644
--- a/apis/field_error.go
+++ b/apis/field_error.go
@@ -103,7 +103,7 @@ func (fe *FieldError) ViaKey(key string) *FieldError {
 }
 
 // ViaFieldKey is the short way to chain: err.ViaKey(bar).ViaField(foo)
-func (fe *FieldError) ViaFieldKey(field string, key string) *FieldError {
+func (fe *FieldError) ViaFieldKey(field, key string) *FieldError {
 	return fe.ViaKey(key).ViaField(field)
 }
 
diff --git a/metrics/utils.go b/metrics/utils.go
index e2e56a415..04773c575 100644
--- a/metrics/utils.go
+++ b/metrics/utils.go
@@ -38,7 +38,7 @@ func MaybeInsertIntTag(key tag.Key, value int, cond bool) tag.Mutator {
 }
 
 // MaybeInsertBoolTag conditionally insert the tag when cond is true.
-func MaybeInsertBoolTag(key tag.Key, value bool, cond bool) tag.Mutator {
+func MaybeInsertBoolTag(key tag.Key, value, cond bool) tag.Mutator {
 	if cond {
 		return tag.Insert(key, strconv.FormatBool(value))
 	}
diff --git a/network/transports.go b/network/transports.go
index a692c5f5b..84f7a1e9c 100644
--- a/network/transports.go
+++ b/network/transports.go
@@ -34,7 +34,7 @@ func (rt RoundTripperFunc) RoundTrip(r *http.Request) (*http.Response, error) {
 	return rt(r)
 }
 
-func newAutoTransport(v1 http.RoundTripper, v2 http.RoundTripper) http.RoundTripper {
+func newAutoTransport(v1, v2 http.RoundTripper) http.RoundTripper {
 	return RoundTripperFunc(func(r *http.Request) (*http.Response, error) {
 		t := v1
 		if r.ProtoMajor == 2 {
diff --git a/reconciler/filter.go b/reconciler/filter.go
index f9042cb90..36c86c4d3 100644
--- a/reconciler/filter.go
+++ b/reconciler/filter.go
@@ -19,7 +19,7 @@ package reconciler
 import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 // AnnotationFilterFunc creates a FilterFunc only accepting objects with given annotation key and value
-func AnnotationFilterFunc(key string, value string, allowUnset bool) func(interface{}) bool {
+func AnnotationFilterFunc(key, value string, allowUnset bool) func(interface{}) bool {
 	return func(obj interface{}) bool {
 		if mo, ok := obj.(metav1.Object); ok {
 			return mapHasOrDefault(mo.GetAnnotations(), key, value, allowUnset)
@@ -41,7 +41,7 @@ func LabelExistsFilterFunc(label string) func(obj interface{}) bool {
 }
 
 // LabelFilterFunc creates a FilterFunc only accepting objects where a label is set to a specific value.
-func LabelFilterFunc(label string, value string, allowUnset bool) func(interface{}) bool {
+func LabelFilterFunc(label, value string, allowUnset bool) func(interface{}) bool {
 	return func(obj interface{}) bool {
 		if mo, ok := obj.(metav1.Object); ok {
 			return mapHasOrDefault(mo.GetLabels(), label, value, allowUnset)
diff --git a/webhook/certificates/resources/certs.go b/webhook/certificates/resources/certs.go
index 26d9d2d2b..9d1bab16a 100644
--- a/webhook/certificates/resources/certs.go
+++ b/webhook/certificates/resources/certs.go
@@ -94,7 +94,7 @@ func createServerCertTemplate(name, namespace string, notAfter time.Time) (*x509
 }
 
 // Actually sign the cert and return things in a form that we can use later on
-func createCert(template, parent *x509.Certificate, pub interface{}, parentPriv interface{}) (
+func createCert(template, parent *x509.Certificate, pub, parentPriv interface{}) (
 	cert *x509.Certificate, certPEM []byte, err error) {
 
 	certDER, err := x509.CreateCertificate(rand.Reader, template, parent, pub, parentPriv)
diff --git a/webhook/psbinding/README.md b/webhook/psbinding/README.md
index f7294997c..4083093b3 100644
--- a/webhook/psbinding/README.md
+++ b/webhook/psbinding/README.md
@@ -231,3 +231,44 @@ With the above defined, then in our webhook's `main.go` we invoke
 		githubbinding.NewController, githubbinding.NewWebhook,
 	)
 ```
+
+### Subresource reconciler
+
+Sometimes we might find the need for controlling not only `psbinding.Bindable`
+and `duckv1.WithPod`, but also other resources. We can achieve this by
+implementing `psbinding.SubResourcesReconcilerInterface` and injecting it in the
+`psbinding.BaseReconciler`.
+
+For example we can implement a SubResourcesReconciler to create/delete k8s
+resources:
+
+```go
+type FooBindingSubResourcesReconciler struct {
+    Client kubernetes.Interface
+}
+
+func (fr *FooBindingSubresourcesReconciler) Reconcile(ctx context.Context, fb psbinding.Bindable) error {
+    // Logic to create k8s resources here
+    return err
+}
+
+func (fr *FooBindingSubresourcesReconciler) ReconcileDeletion(ctx context.Context, fb psbinding.Bindable) error {
+    // Logic to delete k8s resources related to our Bindable
+    return err
+}
+
+```
+
+The SubResourcesReconciler can be then injected in the
+`psbinding.BaseReconciler` as follows:
+
+```go
+kclient := kubeclient.Get(ctx)
+srr := FooBindingSubResourcesReconciler{
+    Client: kclient,
+}
+c := &psbinding.BaseReconciler{
+		...
+        SubresourcesReconciler: srr
+	}
+```
diff --git a/webhook/psbinding/reconciler.go b/webhook/psbinding/reconciler.go
index ba9f35c5f..1bea867c9 100644
--- a/webhook/psbinding/reconciler.go
+++ b/webhook/psbinding/reconciler.go
@@ -45,6 +45,14 @@ import (
 	"knative.dev/pkg/tracker"
 )
 
+// SubResourcesReconcilerInterface is used to reconcile binding related
+// sub-resources. Reconcile is executed after Binding's ReconcileSubject
+// and ReconcileDeletion will be executed before Binding's ReconcileDeletion
+type SubResourcesReconcilerInterface interface {
+	Reconcile(ctx context.Context, fb Bindable) error
+	ReconcileDeletion(ctx context.Context, fb Bindable) error
+}
+
 var jsonLabelPatch = map[string]interface{}{
 	"metadata": map[string]interface{}{
 		"labels": map[string]string{duck.BindingIncludeLabel: "true"},
@@ -88,6 +96,9 @@ type BaseReconciler struct {
 
 	// Namespace Lister
 	NamespaceLister corev1listers.NamespaceLister
+
+	// Sub-resources reconciler. Used to reconcile Binding related resources
+	SubResourcesReconciler SubResourcesReconcilerInterface
 }
 
 // Check that our Reconciler implements controller.Reconciler
@@ -151,6 +162,12 @@ func (r *BaseReconciler) reconcile(ctx context.Context, fb Bindable) error {
 	if fb.GetDeletionTimestamp() != nil {
 		// Check for a DeletionTimestamp.  If present, elide the normal
 		// reconcile logic and do our finalizer handling.
+		if r.SubResourcesReconciler != nil {
+			// If a SubResourceReconciler is defined, finalize related resources
+			if err := r.SubResourcesReconciler.ReconcileDeletion(ctx, fb); err != nil {
+				return err
+			}
+		}
 		return r.ReconcileDeletion(ctx, fb)
 	}
 	// Make sure that our conditions have been initialized.
@@ -166,6 +183,12 @@ func (r *BaseReconciler) reconcile(ctx context.Context, fb Bindable) error {
 	if err := r.ReconcileSubject(ctx, fb, fb.Do); err != nil {
 		return err
 	}
+	if r.SubResourcesReconciler != nil {
+		// If a SubResourceReconciler is defined, reconcile related resources
+		if err := r.SubResourcesReconciler.Reconcile(ctx, fb); err != nil {
+			return err
+		}
+	}
 
 	// Update the observed generation once we have successfully reconciled
 	// our spec.


Test patch:
diff --git a/test/crd.go b/test/crd.go
index ac74d09a7..eb7c0e0b3 100644
--- a/test/crd.go
+++ b/test/crd.go
@@ -31,7 +31,7 @@ const (
 )
 
 // ServiceAccount returns ServiceAccount object in given namespace
-func ServiceAccount(name string, namespace string) *corev1.ServiceAccount {
+func ServiceAccount(name, namespace string) *corev1.ServiceAccount {
 	return &corev1.ServiceAccount{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      name,
diff --git a/test/monitoring/monitoring.go b/test/monitoring/monitoring.go
index 031a8834b..1a6bbc19e 100644
--- a/test/monitoring/monitoring.go
+++ b/test/monitoring/monitoring.go
@@ -33,24 +33,21 @@ import (
 
 // CheckPortAvailability checks to see if the port is available on the machine.
 func CheckPortAvailability(port int) error {
-	server, err := net.Listen("tcp", fmt.Sprintf(":%d", port))
+	server, err := net.Listen("tcp", fmt.Sprint(":", port))
 	if err != nil {
 		// Port is likely taken
 		return err
 	}
-	server.Close()
-
-	return nil
+	return server.Close()
 }
 
 // GetPods retrieves the current existing podlist for the app in monitoring namespace
 // This uses app=<app> as labelselector for selecting pods
 func GetPods(ctx context.Context, kubeClientset *kubernetes.Clientset, app, namespace string) (*v1.PodList, error) {
-	pods, err := kubeClientset.CoreV1().Pods(namespace).List(ctx, metav1.ListOptions{LabelSelector: fmt.Sprintf("app=%s", app)})
+	pods, err := kubeClientset.CoreV1().Pods(namespace).List(ctx, metav1.ListOptions{LabelSelector: "app=" + app})
 	if err == nil && len(pods.Items) == 0 {
-		err = fmt.Errorf("no %s Pod found on the cluster. Ensure monitoring is switched on for your Knative Setup", app)
+		err = fmt.Errorf("pod %s not found on the cluster. Ensure monitoring is switched on for your Knative Setup", app)
 	}
-
 	return pods, err
 }
 
@@ -85,7 +82,7 @@ func PortForward(logf logging.FormatLogger, podList *v1.PodList, localPort, remo
 		return 0, fmt.Errorf("failed to port forward: %w", err)
 	}
 
-	logf("running %s port-forward in background, pid = %d", podName, portFwdProcess.Pid)
+	logf("Running %s port-forward in background, pid = %d", podName, portFwdProcess.Pid)
 	return portFwdProcess.Pid, nil
 }
 
diff --git a/test/prometheus/prometheus_test.go b/test/prometheus/prometheus_test.go
index b248b570a..f316016d7 100644
--- a/test/prometheus/prometheus_test.go
+++ b/test/prometheus/prometheus_test.go
@@ -84,18 +84,18 @@ func (tpa *TestPromAPI) QueryRange(ctx context.Context, query string, r v1.Range
 }
 
 // Series finds series by label matchers.
-func (tpa *TestPromAPI) Series(ctx context.Context, matches []string, startTime time.Time, endTime time.Time) ([]model.LabelSet, error) {
+func (tpa *TestPromAPI) Series(context.Context, []string, time.Time, time.Time) ([]model.LabelSet, error) {
 	return nil, nil
 }
 
 // Snapshot creates a snapshot of all current data into snapshots/<datetime>-<rand>
 // under the TSDB's data directory and returns the directory as response.
-func (tpa *TestPromAPI) Snapshot(ctx context.Context, skipHead bool) (v1.SnapshotResult, error) {
+func (tpa *TestPromAPI) Snapshot(context.Context, bool) (v1.SnapshotResult, error) {
 	return v1.SnapshotResult{}, nil
 }
 
 // Targets returns an overview of the current state of the Prometheus target discovery.
-func (t *TestPromAPI) Targets(ctx context.Context) (v1.TargetsResult, error) {
+func (t *TestPromAPI) Targets(context.Context) (v1.TargetsResult, error) {
 	return v1.TargetsResult{}, nil
 }
 
diff --git a/test/vegeta/pacers/steady_up_pacer.go b/test/vegeta/pacers/steady_up_pacer.go
index 11beb518b..18584cc2f 100644
--- a/test/vegeta/pacers/steady_up_pacer.go
+++ b/test/vegeta/pacers/steady_up_pacer.go
@@ -50,7 +50,7 @@ type steadyUpPacer struct {
 }
 
 // NewSteadyUp returns a new SteadyUpPacer with the given config.
-func NewSteadyUp(min vegeta.Rate, max vegeta.Rate, upDuration time.Duration) (vegeta.Pacer, error) {
+func NewSteadyUp(min, max vegeta.Rate, upDuration time.Duration) (vegeta.Pacer, error) {
 	if upDuration <= 0 || min.Freq <= 0 || min.Per <= 0 || max.Freq <= 0 || max.Per <= 0 {
 		return nil, errors.New("configuration for this SteadyUpPacer is invalid")
 	}
diff --git a/test/webhook-apicoverage/coveragecalculator/ignorefields.go b/test/webhook-apicoverage/coveragecalculator/ignorefields.go
index 96302a8d7..cf867012c 100644
--- a/test/webhook-apicoverage/coveragecalculator/ignorefields.go
+++ b/test/webhook-apicoverage/coveragecalculator/ignorefields.go
@@ -67,7 +67,7 @@ func (ig *IgnoredFields) ReadFromFile(filePath string) error {
 }
 
 // FieldIgnored method given a package, type and field returns true if the field is marked ignored.
-func (ig *IgnoredFields) FieldIgnored(packageName string, typeName string, fieldName string) bool {
+func (ig *IgnoredFields) FieldIgnored(packageName, typeName, fieldName string) bool {
 	if ig.ignoredFields != nil {
 		for key, value := range ig.ignoredFields {
 			if strings.HasSuffix(packageName, key) {
diff --git a/test/webhook-apicoverage/resourcetree/structkindnode.go b/test/webhook-apicoverage/resourcetree/structkindnode.go
index b96640fc1..7eaa7e517 100644
--- a/test/webhook-apicoverage/resourcetree/structkindnode.go
+++ b/test/webhook-apicoverage/resourcetree/structkindnode.go
@@ -44,7 +44,7 @@ func (s *StructKindNode) initialize(field string, parent NodeInterface, t reflec
 func (s *StructKindNode) buildChildNodes(t reflect.Type) {
 	// For types that are part of the standard package, we treat them as leaf nodes and don't expand further.
 	// https://golang.org/pkg/reflect/#StructField.
-	if len(s.FieldType.PkgPath()) == 0 {
+	if s.FieldType.PkgPath() == "" {
 		s.LeafNode = true
 		return
 	}
diff --git a/test/webhook-apicoverage/tools/tools.go b/test/webhook-apicoverage/tools/tools.go
index d869be987..aa813d761 100644
--- a/test/webhook-apicoverage/tools/tools.go
+++ b/test/webhook-apicoverage/tools/tools.go
@@ -244,7 +244,7 @@ func WriteResourcePercentages(outputFile string,
 func CleanupJunitFiles(artifactsDir string) {
 	filepath.Walk(artifactsDir, func(path string, info os.FileInfo, err error) error {
 		if !info.IsDir() && jUnitFileRegexExpr.MatchString(info.Name()) {
-			os.Remove(path)
+			return os.Remove(path)
 		}
 		return nil
 	})
diff --git a/testutils/clustermanager/e2e-tests/boskos/boskos.go b/testutils/clustermanager/e2e-tests/boskos/boskos.go
index c90d71f84..7d99ba23b 100644
--- a/testutils/clustermanager/e2e-tests/boskos/boskos.go
+++ b/testutils/clustermanager/e2e-tests/boskos/boskos.go
@@ -58,7 +58,7 @@ type Client struct {
 // are passed directly to k8s boskos client. Refer to
 // [k8s boskos](https://github.com/kubernetes/test-infra/tree/master/boskos) for more details.
 // If host is "", it looks up JOB_NAME environment variable and set it to be the host name.
-func NewClient(host string, user string, pass string) (*Client, error) {
+func NewClient(host, user, pass string) (*Client, error) {
 	if host == "" {
 		host = common.GetOSEnv("JOB_NAME")
 	}
diff --git a/testutils/clustermanager/perf-tests/pkg/cluster_test.go b/testutils/clustermanager/perf-tests/pkg/cluster_test.go
index e6580f10f..af30d127a 100644
--- a/testutils/clustermanager/perf-tests/pkg/cluster_test.go
+++ b/testutils/clustermanager/perf-tests/pkg/cluster_test.go
@@ -402,7 +402,7 @@ func getAddonsForCluster(cluster *container.Cluster) string {
 	return strings.Join(addons, ",")
 }
 
-func combineClusterMaps(m1 map[string]ClusterConfig, m2 map[string]ClusterConfig) map[string]ClusterConfig {
+func combineClusterMaps(m1, m2 map[string]ClusterConfig) map[string]ClusterConfig {
 	for name, config := range m2 {
 		m1[name] = config
 	}
diff --git a/webhook/psbinding/table_test.go b/webhook/psbinding/table_test.go
index 1ae11be65..34429fe12 100644
--- a/webhook/psbinding/table_test.go
+++ b/webhook/psbinding/table_test.go
@@ -26,6 +26,7 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	jsonpatch "gomodules.xyz/jsonpatch/v2"
+	fakek8s "k8s.io/client-go/kubernetes/fake"
 	"knative.dev/pkg/apis"
 	"knative.dev/pkg/client/injection/ducks/duck/v1/podspecable"
 	kubeclient "knative.dev/pkg/client/injection/kube/client/fake"
@@ -2008,6 +2009,188 @@ func TestBaseReconcile(t *testing.T) {
 	}))
 }
 
+func TestBaseReconcileWithSubResourcesReconciler(t *testing.T) {
+	table := TableTest{{
+		Name: "create new subresource",
+		Key:  "foo/bar",
+		Objects: []runtime.Object{
+			&TestBindable{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace:  "foo",
+					Name:       "bar",
+					Finalizers: []string{"testbindables.duck.knative.dev"},
+				},
+				Spec: TestBindableSpec{
+					BindingSpec: duckv1alpha1.BindingSpec{
+						Subject: tracker.Reference{
+							APIVersion: "apps/v1",
+							Kind:       "Deployment",
+							Namespace:  "foo",
+							Name:       "on-it",
+						},
+					},
+					Foo: "asdfasdfasdfasdf",
+				},
+				Status: TestBindableStatus{
+					Status: duckv1.Status{
+						Conditions: []apis.Condition{{
+							Type:   "Ready",
+							Status: "True",
+						}},
+					},
+				},
+			},
+			&appsv1.Deployment{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "foo",
+					Name:      "on-it",
+				},
+				Spec: appsv1.DeploymentSpec{
+					Template: corev1.PodTemplateSpec{
+						Spec: corev1.PodSpec{
+							Containers: []corev1.Container{{
+								Name:  "foo",
+								Image: "busybox",
+								Env: []corev1.EnvVar{{
+									Name:  "FOO",
+									Value: "asdfasdfasdfasdf",
+								}},
+							}},
+						},
+					},
+				},
+			},
+			&corev1.Namespace{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "foo",
+				},
+			},
+		},
+		WantPatches: []clientgotesting.PatchActionImpl{
+			patchAddLabel("foo"),
+		},
+		WantCreates: []runtime.Object{
+			&corev1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "bar",
+				},
+			},
+		},
+	},
+		{
+			Name: "delete resource and subresource",
+			Key:  "foo/bar",
+			Objects: []runtime.Object{
+				&TestBindable{
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace:         "foo",
+						DeletionTimestamp: &metav1.Time{Time: time.Now()},
+						Name:              "bar",
+						Finalizers:        []string{"testbindables.duck.knative.dev"},
+					},
+					Spec: TestBindableSpec{
+						BindingSpec: duckv1alpha1.BindingSpec{
+							Subject: tracker.Reference{
+								APIVersion: "apps/v1",
+								Kind:       "Deployment",
+								Namespace:  "foo",
+								Name:       "on-it",
+							},
+						},
+						Foo: "asdfasdfasdfasdf",
+					},
+					Status: TestBindableStatus{
+						Status: duckv1.Status{
+							Conditions: []apis.Condition{{
+								Type:   "Ready",
+								Status: "True",
+							}},
+						},
+					},
+				},
+				&appsv1.Deployment{
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: "foo",
+						Name:      "on-it",
+					},
+					Spec: appsv1.DeploymentSpec{
+						Template: corev1.PodTemplateSpec{
+							Spec: corev1.PodSpec{
+								Containers: []corev1.Container{{
+									Name:  "foo",
+									Image: "busybox",
+									Env: []corev1.EnvVar{{
+										Name:  "FOO",
+										Value: "asdfasdfasdfasdf",
+									}},
+								}},
+							},
+						},
+					},
+				},
+				&corev1.Namespace{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "foo",
+					},
+				},
+				&corev1.ConfigMap{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "bar",
+						Namespace: "foo",
+					},
+				},
+			},
+			WantPatches: []clientgotesting.PatchActionImpl{
+				patchAddLabel("foo"),
+				patchRemoveEnv("foo", "on-it"),
+				patchRemoveFinalizer("foo", "bar", "" /* resource version */),
+			},
+			WantDeletes: []clientgotesting.DeleteActionImpl{
+				deletedConfigmap("foo", "bar"),
+			},
+		},
+	}
+
+	table.Test(t, MakeFactory(func(ctx context.Context, listers *Listers, cmw configmap.Watcher) controller.Reconciler {
+		gvr := SchemeGroupVersion.WithResource("testbindables")
+		ctx = podspecable.WithDuck(ctx)
+
+		dc := dynamicclient.Get(ctx)
+
+		kc := kubeclient.Get(ctx)
+		srr := &dummySubResourcesReconciler{
+			client: kc,
+		}
+		return &BaseReconciler{
+			GVR: gvr,
+
+			DynamicClient: dc,
+			Factory:       podspecable.Get(ctx),
+
+			Tracker: &FakeTracker{},
+
+			Recorder: record.NewFakeRecorder(20),
+
+			Get: func(namespace, name string) (Bindable, error) {
+				for _, elt := range listers.GetDuckObjects() {
+					b, ok := elt.(*TestBindable)
+					if !ok {
+						continue
+					}
+					if b.Namespace != namespace || b.Name != name {
+						continue
+					}
+					return b, nil
+				}
+				return nil, apierrs.NewNotFound(gvr.GroupResource(), name)
+			},
+			NamespaceLister: listers.GetNamespaceLister(),
+
+			SubResourcesReconciler: srr,
+		}
+	}))
+}
+
 func mustTU(t *testing.T, ro duck.OneOfOurs) *unstructured.Unstructured {
 	u, err := duck.ToUnstructured(ro)
 	if err != nil {
@@ -2080,3 +2263,40 @@ func patchRemoveEnv(namespace, name string) clientgotesting.PatchActionImpl {
 	action.Patch = []byte(patch)
 	return action
 }
+
+func deletedConfigmap(namespace string, name string) clientgotesting.DeleteActionImpl {
+	action := clientgotesting.DeleteActionImpl{}
+	resource := schema.GroupVersionResource{
+		Group:    "core",
+		Version:  "v1",
+		Resource: "configmaps",
+	}
+	actionImpl := clientgotesting.ActionImpl{
+		Namespace:   namespace,
+		Verb:        "delete",
+		Resource:    resource,
+		Subresource: "",
+	}
+	action.ActionImpl = actionImpl
+	action.Name = name
+	return action
+}
+
+type dummySubResourcesReconciler struct {
+	client *fakek8s.Clientset
+}
+
+func (d *dummySubResourcesReconciler) Reconcile(ctx context.Context, fb Bindable) error {
+	cfg := corev1.ConfigMap{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: fb.GetName(),
+		},
+	}
+
+	_, err := d.client.CoreV1().ConfigMaps(fb.GetNamespace()).Create(ctx, &cfg, metav1.CreateOptions{})
+	return err
+}
+
+func (d *dummySubResourcesReconciler) ReconcileDeletion(ctx context.Context, fb Bindable) error {
+	return d.client.CoreV1().ConfigMaps(fb.GetNamespace()).Delete(ctx, fb.GetName(), metav1.DeleteOptions{})
+}