From 22ccb37e889175d4771cfe4228ef42813c210165 Mon Sep 17 00:00:00 2001 From: pacoxu Date: Wed, 16 Jun 2021 15:29:04 +0800 Subject: [PATCH 1/2] add update-unwanted-dependencies.sh to track unwanted dependencies Signed-off-by: pacoxu Co-authored-by: Jordan Liggitt Signed-off-by: pacoxu --- cmd/dependencyverifier/OWNERS | 9 + cmd/dependencyverifier/dependencyverifier.go | 179 +++++++++++++++++++ hack/lint-dependencies.sh | 14 +- hack/unwanted-dependencies.json | 24 +++ 4 files changed, 214 insertions(+), 12 deletions(-) create mode 100644 cmd/dependencyverifier/OWNERS create mode 100644 cmd/dependencyverifier/dependencyverifier.go create mode 100644 hack/unwanted-dependencies.json diff --git a/cmd/dependencyverifier/OWNERS b/cmd/dependencyverifier/OWNERS new file mode 100644 index 00000000000..25ed74fe51f --- /dev/null +++ b/cmd/dependencyverifier/OWNERS @@ -0,0 +1,9 @@ +# See the OWNERS docs at https://go.k8s.io/owners + +reviewers: + - pacoxu + - gautierdelorme +approvers: + - bentheelder + - pacoxu + - liggitt \ No newline at end of file diff --git a/cmd/dependencyverifier/dependencyverifier.go b/cmd/dependencyverifier/dependencyverifier.go new file mode 100644 index 00000000000..ca3a20b7567 --- /dev/null +++ b/cmd/dependencyverifier/dependencyverifier.go @@ -0,0 +1,179 @@ +/* +Copyright 2021 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 main + +import ( + "bytes" + "encoding/json" + "fmt" + "io/ioutil" + "log" + "os" + "os/exec" + "reflect" + "strings" +) + +type Unwanted struct { + // things we want to stop referencing + Spec UnwantedSpec `json:"spec"` + // status of our unwanted dependencies + Status UnwantedStatus `json:"status"` +} + +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 + // 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 add DirectReferences + // unwanted modules we still directly reference from spec.roots, based on `go mod graph` content. + // eliminating things from this list is good. + DirectReferences []string `json:"directReferences"` + + // unwanted modules indirectly referenced from modules other than spec.roots, based on `go mod graph` content. + // eliminating things from this list is good, but usually requires working with upstreams to do so. + IndirectReferences []string `json:"indirectReferences"` +} + +// 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.IndirectReferences) { + config.Status.IndirectReferences = append(config.Status.IndirectReferences, unwanted) + } + } + } +} + +// runCommand runs the cmd and returns the combined stdout and stderr, or an +// error if the command failed. +func runCommand(cmd ...string) (string, error) { + output, err := exec.Command(cmd[0], cmd[1:]...).CombinedOutput() + if err != nil { + return "", fmt.Errorf("failed to run %q: %s (%s)", strings.Join(cmd, " "), err, output) + } + return string(output), nil +} + +func readFile(path string) (string, error) { + content, err := ioutil.ReadFile(path) + // Convert []byte to string and print to screen + return string(content), err +} + +func stringInSlice(a string, list []string) bool { + for _, b := range list { + if b == a { + return true + } + } + return false +} + +func convertToMap(modStr string) map[string][]string { + modMap := make(map[string][]string) + 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] + second := strings.Split(deps[1], "@")[0] + original, ok := modMap[first] + if !ok { + modMap[first] = []string{second} + } else if stringInSlice(second, original) { + continue + } else { + modMap[first] = append(original, second) + } + } else { + // skip invalid line + log.Printf("!!!invalid line in mod.graph: %s", line) + continue + } + } + return modMap +} + +// option1: dependencyverifier dependencies.json +// it will run `go mod graph` and check it. +// option2: dependencyverifier dependencies.json mod.graph +// it will check the specified mod graph result file. +func main() { + var modeGraphStr string + var err error + if len(os.Args) == 2 { + // run `go mod graph` + modeGraphStr, err = runCommand("go", "mod", "graph") + if err != nil { + log.Fatalf("Error running 'go mod graph': %s", err) + } + } else if len(os.Args) == 3 { + modGraphFile := string(os.Args[2]) + modeGraphStr, err = readFile(modGraphFile) + // read file, such as `mod.graph` + if err != nil { + log.Fatalf("Error reading mod file %s: %s", modGraphFile, err) + } + } else { + log.Fatalf("Usage: %s dependencies.json {mod.graph}", os.Args[0]) + } + + dependenciesJSONPath := string(os.Args[1]) + dependencies, err := readFile(dependenciesJSONPath) + if err != nil { + log.Fatalf("Error reading dependencies file %s: %s", dependencies, err) + } + + // load Unwanted from json + configFromFile := &Unwanted{} + decoder := json.NewDecoder(bytes.NewBuffer([]byte(dependencies))) + decoder.DisallowUnknownFields() + if err := decoder.Decode(configFromFile); err != nil { + log.Fatalf("Error reading dependencies file %s: %s", dependenciesJSONPath, err) + } + + // Check and update status of struct Unwanted + modeGraph := convertToMap(modeGraphStr) + config := &Unwanted{} + config.Spec.UnwantedModules = configFromFile.Spec.UnwantedModules + config.checkUpdateStatus(modeGraph) + + // DeepEqual check current status with it in json file + // TODO sort array + if !reflect.DeepEqual(configFromFile.Status, config.Status) { + fmt.Println("Status in ./hack/unwanted-dependencies.json:\n", configFromFile.Status) + fmt.Println("Status detected:\n", config.Status) + log.Println("!!! Please update ./hack/unwanted-dependencies.json") + os.Exit(1) + } +} diff --git a/hack/lint-dependencies.sh b/hack/lint-dependencies.sh index dc0d89e1d35..ab911620dfb 100755 --- a/hack/lint-dependencies.sh +++ b/hack/lint-dependencies.sh @@ -42,18 +42,8 @@ kube::util::require-jq rc=0 # List of dependencies we need to avoid dragging back into kubernetes/kubernetes -forbidden_repos=( - "k8s.io/klog" # we have switched to klog v2, so avoid klog v1 -) -for forbidden_repo in "${forbidden_repos[@]}"; do - deps_on_forbidden=$(go mod graph | grep " ${forbidden_repo}@" || echo "") - if [ -n "${deps_on_forbidden}" ]; then - kube::log::error "The following have transitive dependencies on ${forbidden_repo}, which is not allowed:" - echo "${deps_on_forbidden}" - echo "" - rc=1 - fi -done +# Check if unwanted dependencies are removed +go run k8s.io/kubernetes/cmd/dependencyverifier "${KUBE_ROOT}/hack/unwanted-dependencies.json" outdated=$(go list -m -json all | jq -r " select(.Replace.Version != null) | diff --git a/hack/unwanted-dependencies.json b/hack/unwanted-dependencies.json new file mode 100644 index 00000000000..4f13ca92268 --- /dev/null +++ b/hack/unwanted-dependencies.json @@ -0,0 +1,24 @@ +{ + "spec": { + "unwantedModules": { + "github.com/go-kit/kit": "lots of transitive dependencies, see https://github.com/prometheus/common/issues/255", + "github.com/go-openapi/analysis": "use k8s.io/kube-openapi/pkg/validation/spec", + "github.com/go-openapi/spec": "use k8s.io/kube-openapi/pkg/validation/spec instead", + "github.com/go-openapi/strfmt": "use k8s.io/kube-openapi/pkg/validation/strfmt instead", + "github.com/go-openapi/validate": "use k8s.io/kube-openapi/pkg/validation/validate instead", + "github.com/influxdata/influxdb1-client": "", + "go.mongodb.org/mongo-driver": "", + "k8s.io/klog": "we have switched to klog v2, so avoid klog v1", + "rsc.io/quote": "refer to #102833", + "rsc.io/sampler": "refer to #102833", + "github.com/hashicorp/hcl": "", + "github.com/spf13/viper": "refer to #102598" + } + }, + "status": { + "indirectReferences": [ + "github.com/hashicorp/hcl", + "github.com/spf13/viper" + ] + } +} \ No newline at end of file From b99e1e4aa9c8fdd2ef73cf91838655c452ed7399 Mon Sep 17 00:00:00 2001 From: pacoxu Date: Mon, 5 Jul 2021 13:54:27 +0800 Subject: [PATCH 2/2] use reference as we cannot distinguishing direct/indirect with `go mod graph` --- cmd/dependencyverifier/OWNERS | 10 +-- cmd/dependencyverifier/dependencyverifier.go | 69 ++++++++++++++------ hack/unwanted-dependencies.json | 12 +++- 3 files changed, 63 insertions(+), 28 deletions(-) diff --git a/cmd/dependencyverifier/OWNERS b/cmd/dependencyverifier/OWNERS index 25ed74fe51f..464ca43fbb7 100644 --- a/cmd/dependencyverifier/OWNERS +++ b/cmd/dependencyverifier/OWNERS @@ -1,9 +1,9 @@ # See the OWNERS docs at https://go.k8s.io/owners reviewers: - - pacoxu - - gautierdelorme +- dep-reviewers +- pacoxu +- gautierdelorme approvers: - - bentheelder - - pacoxu - - liggitt \ No newline at end of file +- dep-approvers +- pacoxu diff --git a/cmd/dependencyverifier/dependencyverifier.go b/cmd/dependencyverifier/dependencyverifier.go index ca3a20b7567..ecf367945a3 100644 --- a/cmd/dependencyverifier/dependencyverifier.go +++ b/cmd/dependencyverifier/dependencyverifier.go @@ -24,7 +24,7 @@ import ( "log" "os" "os/exec" - "reflect" + "sort" "strings" ) @@ -50,14 +50,10 @@ type UnwantedStatus struct { // eliminating things from this list is good. Vendored []string `json:"vendored"` - // TODO implement checks for add DirectReferences + // 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. - DirectReferences []string `json:"directReferences"` - - // unwanted modules indirectly referenced from modules other than spec.roots, based on `go mod graph` content. - // eliminating things from this list is good, but usually requires working with upstreams to do so. - IndirectReferences []string `json:"indirectReferences"` + // eliminating things from this list is good, and sometimes requires working with upstreams to do so. + References []string `json:"references"` } // Check all unwanted dependencies and update its status. @@ -65,11 +61,13 @@ 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.IndirectReferences) { - config.Status.IndirectReferences = append(config.Status.IndirectReferences, unwanted) + 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 @@ -107,13 +105,13 @@ func convertToMap(modStr string) map[string][]string { if len(deps) == 2 { first := strings.Split(deps[0], "@")[0] second := strings.Split(deps[1], "@")[0] - original, ok := modMap[first] + original, ok := modMap[second] if !ok { - modMap[first] = []string{second} - } else if stringInSlice(second, original) { + modMap[second] = []string{first} + } else if stringInSlice(first, original) { continue } else { - modMap[first] = append(original, second) + modMap[second] = append(original, first) } } else { // skip invalid line @@ -124,6 +122,23 @@ func convertToMap(modStr string) map[string][]string { return modMap } +// difference returns a-b and b-a as a simple map. +func difference(a, b []string) (map[string]bool, map[string]bool) { + aMinusB := map[string]bool{} + bMinusA := map[string]bool{} + for _, dependency := range a { + aMinusB[dependency] = true + } + for _, dependency := range b { + if _, found := aMinusB[dependency]; found { + delete(aMinusB, dependency) + } else { + bMinusA[dependency] = true + } + } + return aMinusB, bMinusA +} + // option1: dependencyverifier dependencies.json // it will run `go mod graph` and check it. // option2: dependencyverifier dependencies.json mod.graph @@ -168,12 +183,26 @@ func main() { config.Spec.UnwantedModules = configFromFile.Spec.UnwantedModules config.checkUpdateStatus(modeGraph) - // DeepEqual check current status with it in json file - // TODO sort array - if !reflect.DeepEqual(configFromFile.Status, config.Status) { - fmt.Println("Status in ./hack/unwanted-dependencies.json:\n", configFromFile.Status) - fmt.Println("Status detected:\n", config.Status) - log.Println("!!! Please update ./hack/unwanted-dependencies.json") + 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) + if len(removedReferences) > 0 { + log.Println("Good news! The following unwanted dependencies are no longer referenced:") + 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 { + log.Printf("The following unwanted dependencies marked in %s are referenced:", dependenciesJSONPath) + for reference := range unwantedReferences { + log.Printf(" %s (referenced by %s)", reference, strings.Join(modeGraph[reference], ", ")) + } + log.Printf("!!! Avoid updating referencing modules to versions that reintroduce use of unwanted dependencies\n") + needUpdate = true + } + if needUpdate { os.Exit(1) } } diff --git a/hack/unwanted-dependencies.json b/hack/unwanted-dependencies.json index 4f13ca92268..ae6d29976e4 100644 --- a/hack/unwanted-dependencies.json +++ b/hack/unwanted-dependencies.json @@ -11,14 +11,20 @@ "k8s.io/klog": "we have switched to klog v2, so avoid klog v1", "rsc.io/quote": "refer to #102833", "rsc.io/sampler": "refer to #102833", + "github.com/hashicorp/golang-lru": "", "github.com/hashicorp/hcl": "", - "github.com/spf13/viper": "refer to #102598" + "github.com/json-iterator/go": "refer to #105030", + "github.com/spf13/viper": "refer to #102598", + "github.com/go-bindata/go-bindata ": "refer to #99829" } }, "status": { - "indirectReferences": [ + "references": [ + "github.com/go-kit/kit", + "github.com/hashicorp/golang-lru", "github.com/hashicorp/hcl", + "github.com/json-iterator/go", "github.com/spf13/viper" ] } -} \ No newline at end of file +}