Fix unwanted dependencies scanner

cmd/dependencyverifier/dependencyverifier.go

@@ -35,38 +35,14 @@ type Unwanted struct {
 }
 
 type UnwantedSpec struct {
-	// TODO implement checks for RootModules
-	// module names/patterns of root modules whose dependencies should be considered direct references
-	RootModules []string `json:"rootModules"`
-
 	// module names we don't want to depend on, mapped to an optional message about why
 	UnwantedModules map[string]string `json:"unwantedModules"`
 }
 
 type UnwantedStatus struct {
-	// TODO implement checks for Vendored
+	// references to modules in the spec.unwantedModules list, based on `go mod graph` content.
-	// unwanted modules we still vendor, based on vendor/modules.txt content.
-	// eliminating things from this list is good.
-	Vendored []string `json:"vendored"`
-
-	// TODO implement checks for distinguishing direct/indirect
-	// unwanted modules we still directly reference from spec.roots, based on `go mod graph` content.
 	// eliminating things from this list is good, and sometimes requires working with upstreams to do so.
-	References []string `json:"references"`
+	UnwantedReferences map[string][]string `json:"unwantedReferences"`
-}
-
-// Check all unwanted dependencies and update its status.
-func (config *Unwanted) checkUpdateStatus(modeGraph map[string][]string) {
-	fmt.Println("Check all unwanted dependencies and update its status.")
-	for unwanted := range config.Spec.UnwantedModules {
-		if _, found := modeGraph[unwanted]; found {
-			if !stringInSlice(unwanted, config.Status.References) {
-				config.Status.References = append(config.Status.References, unwanted)
-			}
-		}
-	}
-	// sort deps
-	sort.Strings(config.Status.References)
 }
 
 // runCommand runs the cmd and returns the combined stdout and stderr, or an
@@ -85,44 +61,51 @@ func readFile(path string) (string, error) {
 	return string(content), err
 }
 
-func stringInSlice(a string, list []string) bool {
+func moduleInSlice(a module, list []module, matchVersion bool) bool {
 	for _, b := range list {
 		if b == a {
 			return true
 		}
+		if !matchVersion && b.name == a.name {
+			return true
+		}
 	}
 	return false
 }
 
-func convertToMap(modStr string) map[string][]string {
+// converts `go mod graph` output modStr into a map of from->[]to references and the main module
-	modMap := make(map[string][]string)
+func convertToMap(modStr string) ([]module, map[module][]module) {
+	var (
+		mainModulesList = []module{}
+		mainModules     = map[module]bool{}
+	)
+	modMap := make(map[module][]module)
 	for _, line := range strings.Split(modStr, "\n") {
 		if len(line) == 0 {
 			continue
 		}
 		deps := strings.Split(line, " ")
 		if len(deps) == 2 {
-			first := strings.Split(deps[0], "@")[0]
+			first := parseModule(deps[0])
-			second := strings.Split(deps[1], "@")[0]
+			second := parseModule(deps[1])
-			original, ok := modMap[second]
+			if first.version == "" || first.version == "v0.0.0" {
-			if !ok {
+				if !mainModules[first] {
-				modMap[second] = []string{first}
+					mainModules[first] = true
-			} else if stringInSlice(first, original) {
+					mainModulesList = append(mainModulesList, first)
-				continue
+				}
-			} else {
-				modMap[second] = append(original, first)
 			}
+			modMap[first] = append(modMap[first], second)
 		} else {
 			// skip invalid line
 			log.Printf("!!!invalid line in mod.graph: %s", line)
 			continue
 		}
 	}
-	return modMap
+	return mainModulesList, modMap
 }
 
-// difference returns a-b and b-a as a simple map.
+// difference returns a-b and b-a as sorted lists
-func difference(a, b []string) (map[string]bool, map[string]bool) {
+func difference(a, b []string) ([]string, []string) {
 	aMinusB := map[string]bool{}
 	bMinusA := map[string]bool{}
 	for _, dependency := range a {
@@ -135,7 +118,37 @@ func difference(a, b []string) (map[string]bool, map[string]bool) {
 			bMinusA[dependency] = true
 		}
 	}
-	return aMinusB, bMinusA
+	aMinusBList := []string{}
+	bMinusAList := []string{}
+	for dependency := range aMinusB {
+		aMinusBList = append(aMinusBList, dependency)
+	}
+	for dependency := range bMinusA {
+		bMinusAList = append(bMinusAList, dependency)
+	}
+	sort.Strings(aMinusBList)
+	sort.Strings(bMinusAList)
+	return aMinusBList, bMinusAList
+}
+
+type module struct {
+	name    string
+	version string
+}
+
+func (m module) String() string {
+	if len(m.version) == 0 {
+		return m.name
+	}
+	return m.name + "@" + m.version
+}
+
+func parseModule(s string) module {
+	if !strings.Contains(s, "@") {
+		return module{name: s}
+	}
+	parts := strings.SplitN(s, "@", 2)
+	return module{name: parts[0], version: parts[1]}
 }
 
 // option1: dependencyverifier dependencies.json
@@ -176,32 +189,150 @@ func main() {
 		log.Fatalf("Error reading dependencies file %s: %s", dependenciesJSONPath, err)
 	}
 
-	// Check and update status of struct Unwanted
+	// convert from `go mod graph` to main module and map of from->[]to references
-	modeGraph := convertToMap(modeGraphStr)
+	mainModules, moduleGraph := convertToMap(modeGraphStr)
+
+	// gather the effective versions by looking at the versions required by the main modules
+	effectiveVersions := map[string]module{}
+	for _, mainModule := range mainModules {
+		for _, override := range moduleGraph[mainModule] {
+			if _, ok := effectiveVersions[override.name]; !ok {
+				effectiveVersions[override.name] = override
+			}
+		}
+	}
+
+	unwantedToReferencers := map[string][]module{}
+	for _, mainModule := range mainModules {
+		// visit to find unwanted modules still referenced from the main module
+		visit(func(m module, via []module) {
+			if _, unwanted := configFromFile.Spec.UnwantedModules[m.name]; unwanted {
+				// this is unwanted, store what is referencing it
+				referencer := via[len(via)-1]
+				if !moduleInSlice(referencer, unwantedToReferencers[m.name], false) {
+					// // uncomment to get a detailed tree of the path that referenced the unwanted dependency
+					//
+					// i := 0
+					// for _, v := range via {
+					// 	if v.version != "" && v.version != "v0.0.0" {
+					// 		fmt.Println(strings.Repeat("  ", i), v)
+					// 		i++
+					// 	}
+					// }
+					// if i > 0 {
+					// 	fmt.Println(strings.Repeat("  ", i+1), m)
+					// 	fmt.Println()
+					// }
+					unwantedToReferencers[m.name] = append(unwantedToReferencers[m.name], referencer)
+				}
+			}
+		}, mainModule, moduleGraph, effectiveVersions)
+	}
+
 	config := &Unwanted{}
 	config.Spec.UnwantedModules = configFromFile.Spec.UnwantedModules
-	config.checkUpdateStatus(modeGraph)
+	for unwanted := range unwantedToReferencers {
+		if config.Status.UnwantedReferences == nil {
+			config.Status.UnwantedReferences = map[string][]string{}
+		}
+		sort.Slice(unwantedToReferencers[unwanted], func(i, j int) bool {
+			ri := unwantedToReferencers[unwanted][i]
+			rj := unwantedToReferencers[unwanted][j]
+			if ri.name != rj.name {
+				return ri.name < rj.name
+			}
+			return ri.version < rj.version
+		})
+		for _, referencer := range unwantedToReferencers[unwanted] {
+			// make sure any reference at all shows up as a non-nil status
+			if config.Status.UnwantedReferences == nil {
+				config.Status.UnwantedReferences[unwanted] = []string{}
+			}
+			// record specific names of versioned referents
+			if referencer.version != "" && referencer.version != "v0.0.0" {
+				config.Status.UnwantedReferences[unwanted] = append(config.Status.UnwantedReferences[unwanted], referencer.name)
+			}
+		}
+	}
 
 	needUpdate := false
+
 	// Compare unwanted list from unwanted-dependencies.json with current status from `go mod graph`
-	removedReferences, unwantedReferences := difference(configFromFile.Status.References, config.Status.References)
+	expected, err := json.MarshalIndent(configFromFile.Status, "", "  ")
-	if len(removedReferences) > 0 {
+	if err != nil {
-		log.Println("Good news! The following unwanted dependencies are no longer referenced:")
+		log.Fatal(err)
-		for reference := range removedReferences {
-			log.Printf("   %s", reference)
-		}
-		log.Printf("!!! Remove the unwanted dependencies from status in %s to ensure they don't get reintroduced", dependenciesJSONPath)
-		needUpdate = true
 	}
-	if len(unwantedReferences) > 0 {
+	actual, err := json.MarshalIndent(config.Status, "", "  ")
-		log.Printf("The following unwanted dependencies marked in %s are referenced:", dependenciesJSONPath)
+	if err != nil {
-		for reference := range unwantedReferences {
+		log.Fatal(err)
-			log.Printf("   %s (referenced by %s)", reference, strings.Join(modeGraph[reference], ", "))
+	}
+	if !bytes.Equal(expected, actual) {
+		log.Printf("Expected status of\n%s", string(expected))
+		log.Printf("Got status of\n%s", string(actual))
+	}
+	for expectedRef, expectedFrom := range configFromFile.Status.UnwantedReferences {
+		actualFrom, ok := config.Status.UnwantedReferences[expectedRef]
+		if !ok {
+			// disappeared entirely
+			log.Printf("Good news! Unwanted dependency %q is no longer referenced. Remove status.unwantedReferences[%q] in %s to ensure it doesn't get reintroduced.", expectedRef, expectedRef, dependenciesJSONPath)
+			needUpdate = true
+			continue
+		}
+		removedReferences, unwantedReferences := difference(expectedFrom, actualFrom)
+		if len(removedReferences) > 0 {
+			log.Printf("Good news! Unwanted module %q dropped the following dependants:", expectedRef)
+			for _, reference := range removedReferences {
+				log.Printf("   %s", reference)
+			}
+			log.Printf("!!! Remove those from status.unwantedReferences[%q] in %s to ensure they don't get reintroduced.", expectedRef, dependenciesJSONPath)
+			needUpdate = true
+		}
+		if len(unwantedReferences) > 0 {
+			log.Printf("Unwanted module %q marked in %s is referenced by new dependants:", expectedRef, dependenciesJSONPath)
+			for _, reference := range unwantedReferences {
+				log.Printf("   %s", reference)
+			}
+			log.Printf("!!! Avoid updating referencing modules to versions that reintroduce use of unwanted dependencies\n")
+			needUpdate = true
+		}
+	}
+	for actualRef, actualFrom := range config.Status.UnwantedReferences {
+		if _, expected := configFromFile.Status.UnwantedReferences[actualRef]; expected {
+			// expected, already ensured referencers were equal in the first loop
+			continue
+		}
+		log.Printf("Unwanted module %q marked in %s is referenced", actualRef, dependenciesJSONPath)
+		for _, reference := range actualFrom {
+			log.Printf("   %s", reference)
 		}
 		log.Printf("!!! Avoid updating referencing modules to versions that reintroduce use of unwanted dependencies\n")
 		needUpdate = true
 	}
+
 	if needUpdate {
 		os.Exit(1)
 	}
 }
+
+func visit(visitor func(m module, via []module), main module, references map[module][]module, effectiveVersions map[string]module) {
+	doVisit(visitor, main, nil, map[module]bool{}, references, effectiveVersions)
+}
+
+func doVisit(visitor func(m module, via []module), from module, via []module, visited map[module]bool, references map[module][]module, effectiveVersions map[string]module) {
+	visitor(from, via)
+	via = append(via, from)
+	if visited[from] {
+		return
+	}
+	for _, to := range references[from] {
+		// switch to the effective version of this dependency
+		if override, ok := effectiveVersions[to.name]; ok {
+			to = override
+		}
+		// recurse unless we've already visited this module in this traversal
+		if !moduleInSlice(to, via, false) {
+			doVisit(visitor, to, via, visited, references, effectiveVersions)
+		}
+	}
+	visited[from] = true
+}

hack/unwanted-dependencies.json

@@ -23,9 +23,17 @@
     }
   },
   "status": {
-    "references": [
+    "unwantedReferences": {
-      "github.com/go-kit/kit",
+      "github.com/go-kit/kit": [
-      "github.com/json-iterator/go"
+        "github.com/grpc-ecosystem/go-grpc-middleware"
-    ]
+      ],
+      "github.com/json-iterator/go": [
+        "github.com/prometheus/client_golang",
+        "go.etcd.io/etcd/client/v2",
+        "go.opentelemetry.io/contrib/instrumentation/github.com/emicklei/go-restful/otelrestful",
+        "k8s.io/kube-openapi",
+        "sigs.k8s.io/structured-merge-diff/v4"
+      ]
+    }
   }
 }