Repository: knative/pkg
Pull request #1859 (2020-10-27 22:48:01)

Add AssertOnlyDeps variant

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

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/1859#" title="Author self-approved">mattmoor</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:

- ~~[OWNERS](https://github.com/knative/pkg/blob/master/OWNERS)~~ [mattmoor]

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":[]} -->
mattmoor: We can add `Check` variants that return an `error`, but I think a `bool` is far too lossy.

Probably worth doing separately.

Patch:
diff --git a/depcheck/depcheck.go b/depcheck/depcheck.go
index 18a091ad3..08d0bff48 100644
--- a/depcheck/depcheck.go
+++ b/depcheck/depcheck.go
@@ -109,3 +109,32 @@ func AssertNoDependency(t *testing.T, banned map[string][]string) {
 		})
 	}
 }
+
+// AssertOnlyDependencies checks that the given import paths (the keys) only
+// depend (transitively) on certain allowed imports (the values).
+// Note: while perhaps counterintuitive we allow the value to be a superset
+// of the actual imports to that folks can use a constant that holds blessed
+// import paths.
+func AssertOnlyDependencies(t *testing.T, allowed map[string][]string) {
+	t.Helper()
+
+	for ip, allow := range allowed {
+		// Always include our own package in the set of allowed dependencies.
+		allowed := make(map[string]struct{}, len(allow)+1)
+		for _, x := range append(allow, ip) {
+			allowed[x] = struct{}{}
+		}
+		t.Run(ip, func(t *testing.T) {
+			g, err := buildGraph(ip)
+			if err != nil {
+				t.Fatal("buildGraph(queue) =", err)
+			}
+			for name := range g {
+				if _, ok := allowed[name]; !ok {
+					t.Errorf("Dependency %s of %s is not explicitly allowed\n%s", name, ip,
+						strings.Join(g.path(name), "\n"))
+				}
+			}
+		})
+	}
+}
diff --git a/depcheck/heavy.go b/depcheck/heavy.go
new file mode 100644
index 000000000..de5e81178
--- /dev/null
+++ b/depcheck/heavy.go
@@ -0,0 +1,28 @@
+/*
+Copyright 2020 The Knative 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 depcheck
+
+var (
+	KnownHeavyDependencies = []string{
+		"k8s.io/apimachinery/pkg/api/apitesting/fuzzer",
+
+		// As of 2020/10/27 this adds about 13MB to overall binary size.
+		"k8s.io/client-go/kubernetes",
+		// As of 2020/10/27 this adds about 7MB to overall binary size.
+		"contrib.go.opencensus.io/exporter/stackdriver",
+	}
+)


Test patch:
diff --git a/apis/depcheck_test.go b/apis/depcheck_test.go
new file mode 100644
index 000000000..d2430f89c
--- /dev/null
+++ b/apis/depcheck_test.go
@@ -0,0 +1,34 @@
+/*
+Copyright 2020 The Knative 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 apis_test
+
+import (
+	"testing"
+
+	"knative.dev/pkg/depcheck"
+)
+
+func TestNoDeps(t *testing.T) {
+	depcheck.AssertNoDependency(t, map[string][]string{
+		"knative.dev/pkg/apis":                depcheck.KnownHeavyDependencies,
+		"knative.dev/pkg/apis/duck":           depcheck.KnownHeavyDependencies,
+		"knative.dev/pkg/apis/duck/ducktypes": depcheck.KnownHeavyDependencies,
+		"knative.dev/pkg/apis/duck/v1alpha1":  depcheck.KnownHeavyDependencies,
+		"knative.dev/pkg/apis/duck/v1beta1":   depcheck.KnownHeavyDependencies,
+		"knative.dev/pkg/apis/duck/v1":        depcheck.KnownHeavyDependencies,
+	})
+}
diff --git a/depcheck/depcheck_test.go b/depcheck/depcheck_test.go
index 7f11bf67c..cfc0bdbbf 100644
--- a/depcheck/depcheck_test.go
+++ b/depcheck/depcheck_test.go
@@ -28,6 +28,9 @@ import (
 // TestExample doesn't follow the Go Example style because it isn't well
 // suited for libraries needing *testing.T
 func TestExample(t *testing.T) {
+
+	// For larger packages, it can make the most sense to simply avoid
+	// known "heavy" packages, which pull in large amount of code or data.
 	depcheck.AssertNoDependency(t, map[string][]string{
 		// Our duck types shouldn't depend on fuzzers.
 		"knative.dev/pkg/apis/duck/v1": {
@@ -47,4 +50,18 @@ func TestExample(t *testing.T) {
 		// 	"k8s.io/api/core/v1",
 		// },
 	})
+
+	// For small packages, it can make the most sense to curate
+	// the external dependencies very carefully.
+	depcheck.AssertOnlyDependencies(t, map[string][]string{
+		// Example libraries with very limited dependencies.
+		"knative.dev/pkg/pool": {
+			"context",
+			"sync",
+			"golang.org/x/sync/errgroup",
+		},
+		"knative.dev/pkg/ptr": {
+			"time",
+		},
+	})
 }
diff --git a/logging/depcheck_test.go b/logging/depcheck_test.go
new file mode 100644
index 000000000..e0d3a8dd4
--- /dev/null
+++ b/logging/depcheck_test.go
@@ -0,0 +1,29 @@
+/*
+Copyright 2020 The Knative 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 logging_test
+
+import (
+	"testing"
+
+	"knative.dev/pkg/depcheck"
+)
+
+func TestNoDeps(t *testing.T) {
+	depcheck.AssertNoDependency(t, map[string][]string{
+		"knative.dev/pkg/logging": depcheck.KnownHeavyDependencies,
+	})
+}
diff --git a/network/depcheck_test.go b/network/depcheck_test.go
new file mode 100644
index 000000000..285de24b3
--- /dev/null
+++ b/network/depcheck_test.go
@@ -0,0 +1,31 @@
+/*
+Copyright 2020 The Knative 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 network_test
+
+import (
+	"testing"
+
+	"knative.dev/pkg/depcheck"
+)
+
+func TestNoDeps(t *testing.T) {
+	depcheck.AssertNoDependency(t, map[string][]string{
+		"knative.dev/pkg/network":          depcheck.KnownHeavyDependencies,
+		"knative.dev/pkg/network/handlers": depcheck.KnownHeavyDependencies,
+		"knative.dev/pkg/network/prober":   depcheck.KnownHeavyDependencies,
+	})
+}