From bb8417266ac046c2da2ab43e05d9a234acc84812 Mon Sep 17 00:00:00 2001 From: Alexandre Mahdhaoui Date: Sun, 14 Apr 2024 01:21:28 +0200 Subject: [PATCH 1/4] =?UTF-8?q?=F0=9F=8C=B1=20verify=20go=20modules=20are?= =?UTF-8?q?=20in=20sync=20with=20upstream=20k/k?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit addresses issues were go modules aren't in sync with upstream k/k by adding these changes: - add `tools/cmd/gomodcheck/main.go` to: - Parse and compares k/k dependencies to controller-runtime's ones. - If any version diffs is found, returns a payload describing the diffs and exit 1. - The user may exclude packages by passing them as arguments. - extend the `verify-modules` make target with `gomodcheck`. --- Makefile | 9 +- hack/.gomodcheck.yaml | 14 +++ hack/tools/cmd/gomodcheck/main.go | 203 ++++++++++++++++++++++++++++++ 3 files changed, 225 insertions(+), 1 deletion(-) create mode 100644 hack/.gomodcheck.yaml create mode 100644 hack/tools/cmd/gomodcheck/main.go diff --git a/Makefile b/Makefile index 438613b3eb..e1a6941433 100644 --- a/Makefile +++ b/Makefile @@ -92,6 +92,12 @@ GOLANGCI_LINT_PKG := github.com/golangci/golangci-lint/cmd/golangci-lint $(GOLANGCI_LINT): # Build golangci-lint from tools folder. GOBIN=$(TOOLS_BIN_DIR) $(GO_INSTALL) $(GOLANGCI_LINT_PKG) $(GOLANGCI_LINT_BIN) $(GOLANGCI_LINT_VER) +GO_MOD_CHECK_DIR := $(abspath ./hack/tools/cmd/gomodcheck) +GO_MOD_CHECK := $(abspath $(TOOLS_BIN_DIR)/gomodcheck) +GO_MOD_CHECK_IGNORE := $(abspath ./hack/.gomodcheck.yaml) +$(GO_MOD_CHECK): # Build gomodcheck + go build -C $(GO_MOD_CHECK_DIR) -o $(GO_MOD_CHECK) + ## -------------------------------------- ## Linting ## -------------------------------------- @@ -130,11 +136,12 @@ clean-bin: ## Remove all generated binaries. rm -rf hack/tools/bin .PHONY: verify-modules -verify-modules: modules ## Verify go modules are up to date +verify-modules: modules $(GO_MOD_CHECK) ## Verify go modules are up to date @if !(git diff --quiet HEAD -- go.sum go.mod $(TOOLS_DIR)/go.mod $(TOOLS_DIR)/go.sum $(ENVTEST_DIR)/go.mod $(ENVTEST_DIR)/go.sum $(SCRATCH_ENV_DIR)/go.sum); then \ git diff; \ echo "go module files are out of date, please run 'make modules'"; exit 1; \ fi + $(GO_MOD_CHECK) $(GO_MOD_CHECK_IGNORE) APIDIFF_OLD_COMMIT ?= $(shell git rev-parse origin/main) diff --git a/hack/.gomodcheck.yaml b/hack/.gomodcheck.yaml new file mode 100644 index 0000000000..8dbe45fac0 --- /dev/null +++ b/hack/.gomodcheck.yaml @@ -0,0 +1,14 @@ +upstreamRefs: + - k8s.io/api + - k8s.io/apiextensions-apiserver + - k8s.io/apimachinery + - k8s.io/apiserver + - k8s.io/client-go + - k8s.io/component-base + - k8s.io/klog/v2 + # k8s.io/utils -> conflicts with k/k deps + +excludedModules: + # --- test dependencies: + - github.com/onsi/ginkgo/v2 + - github.com/onsi/gomega diff --git a/hack/tools/cmd/gomodcheck/main.go b/hack/tools/cmd/gomodcheck/main.go new file mode 100644 index 0000000000..9c32243976 --- /dev/null +++ b/hack/tools/cmd/gomodcheck/main.go @@ -0,0 +1,203 @@ +package main + +import ( + "encoding/json" + "fmt" + "os" + "os/exec" + "regexp" + "strings" + + "go.uber.org/zap" + "sigs.k8s.io/yaml" +) + +const ( + modFile = "./go.mod" +) + +type config struct { + UpstreamRefs []string `yaml:"upstreamRefs"` + ExcludedModules []string `yaml:"excludedModules"` +} + +type upstream struct { + Ref string `json:"ref"` + Version string `json:"version"` +} + +// representation of an out of sync module +type oosMod struct { + Name string `json:"name"` + Version string `json:"version"` + Upstreams []upstream `json:"upstreams"` +} + +func main() { + l, _ := zap.NewProduction() + logger := l.Sugar() + + if len(os.Args) < 2 { + fmt.Printf("USAGE: %s [PATH_TO_CONFIG_FILE]\n", os.Args[0]) + os.Exit(1) + } + + // --- 1. parse config + b, err := os.ReadFile(os.Args[1]) + if err != nil { + logger.Fatal(err.Error()) + } + + cfg := new(config) + if err := yaml.Unmarshal(b, cfg); err != nil { + logger.Fatal(err.Error()) + } + + excludedMods := make(map[string]any) + for _, mod := range cfg.ExcludedModules { + excludedMods[mod] = nil + } + + // --- 2. project mods + deps, err := parseModFile() + if err != nil { + logger.Fatal(err.Error()) + } + + // --- 3. upstream mods (holding upstream refs) + upstreamModGraph, err := getUpstreamModGraph(cfg.UpstreamRefs) + if err != nil { + logger.Fatal(err.Error()) + } + + oosMods := make([]oosMod, 0) + + // --- 4. validate + // for each module in our project, + // if it matches an upstream module, + // then for each upstream module, + // if project module version doesn't match upstream version, + // then we add the version and the ref to the list of out of sync modules. + for mod, version := range deps { + if _, ok := excludedMods[mod]; ok { + logger.Infof("skipped excluded module: %s", mod) + continue + } + + if versionToRef, ok := upstreamModGraph[mod]; ok { + upstreams := make([]upstream, 0) + + for upstreamVersion, upstreamRef := range versionToRef { + if version != upstreamVersion { + upstreams = append(upstreams, upstream{ + Ref: upstreamRef, + Version: upstreamVersion, + }) + } + } + + if len(upstreams) > 0 { + oosMods = append(oosMods, oosMod{ + Name: mod, + Version: version, + Upstreams: upstreams, + }) + } + } + } + + if len(oosMods) == 0 { + fmt.Println("Success! ๐ŸŽ‰") + os.Exit(0) + } + + b, err = json.MarshalIndent(map[string]any{"outOfSyncModules": oosMods}, "", " ") + if err != nil { + panic(err) + } + + fmt.Println(string(b)) + os.Exit(1) +} + +var ( + cleanMods = regexp.MustCompile(`\t| *//.*`) + modDelimStart = regexp.MustCompile(`^require.*`) + modDelimEnd = ")" +) + +func parseModFile() (map[string]string, error) { + b, err := os.ReadFile(modFile) + if err != nil { + return nil, err + } + + in := string(cleanMods.ReplaceAll(b, []byte(""))) + out := make(map[string]string) + + start := false + for _, s := range strings.Split(in, "\n") { + switch { + case modDelimStart.MatchString(s) && !start: + start = true + case s == modDelimEnd: + return out, nil + case start: + kv := strings.SplitN(s, " ", 2) + if len(kv) < 2 { + return nil, fmt.Errorf("unexpected format for module: %q", s) + } + + out[kv[0]] = kv[1] + } + } + + return out, nil +} + +func getUpstreamModGraph(upstreamRefs []string) (map[string]map[string]string, error) { + b, err := exec.Command("go", "mod", "graph").Output() + if err != nil { + return nil, err + } + + graph := string(b) + o1Refs := make(map[string]bool) + for _, upstreamRef := range upstreamRefs { + o1Refs[upstreamRef] = false + } + + modToVersionToUpstreamRef := make(map[string]map[string]string) + + for _, line := range strings.Split(graph, "\n") { + upstreamRef := strings.SplitN(line, "@", 2)[0] + if _, ok := o1Refs[upstreamRef]; ok { + o1Refs[upstreamRef] = true + kv := strings.SplitN(strings.SplitN(line, " ", 2)[1], "@", 2) + name := kv[0] + version := kv[1] + + if m, ok := modToVersionToUpstreamRef[kv[0]]; ok { + m[version] = upstreamRef + } else { + versionToRef := map[string]string{version: upstreamRef} + modToVersionToUpstreamRef[name] = versionToRef + } + } + } + + notFound := "" + for ref, found := range o1Refs { + if !found { + notFound = fmt.Sprintf("%s%s, ", notFound, ref) + } + } + + if notFound != "" { + return nil, fmt.Errorf("cannot verify modules;"+ + "the following specified upstream module cannot be found in go.mod: [ %s ]", + strings.TrimSuffix(notFound, ", ")) + } + + return modToVersionToUpstreamRef, nil +} From d09c61ed397c1ebe947fcebde3121127a5360779 Mon Sep 17 00:00:00 2001 From: Alexandre Mahdhaoui <80970827+alexandremahdhaoui@users.noreply.github.com> Date: Fri, 26 Apr 2024 15:45:30 +0200 Subject: [PATCH 2/4] Update Makefile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Stefan Bรผringer <4662360+sbueringer@users.noreply.github.com> --- Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile b/Makefile index e1a6941433..bff67cf0c8 100644 --- a/Makefile +++ b/Makefile @@ -95,6 +95,7 @@ $(GOLANGCI_LINT): # Build golangci-lint from tools folder. GO_MOD_CHECK_DIR := $(abspath ./hack/tools/cmd/gomodcheck) GO_MOD_CHECK := $(abspath $(TOOLS_BIN_DIR)/gomodcheck) GO_MOD_CHECK_IGNORE := $(abspath ./hack/.gomodcheck.yaml) +.PHONY: $(GO_MOD_CHECK) $(GO_MOD_CHECK): # Build gomodcheck go build -C $(GO_MOD_CHECK_DIR) -o $(GO_MOD_CHECK) From 6449beb31d2106892e213dfe211a9598bf1754f8 Mon Sep 17 00:00:00 2001 From: Alexandre Mahdhaoui Date: Sat, 27 Apr 2024 11:58:57 +0200 Subject: [PATCH 3/4] =?UTF-8?q?=F0=9F=8C=B1=20parse=20go.mod=20with=20modf?= =?UTF-8?q?ile.Parse?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Alexandre Mahdhaoui --- hack/.gomodcheck.yaml => .gomodcheck.yaml | 0 Makefile | 4 +- go.mod | 2 + go.sum | 2 + hack/tools/.keep | 0 hack/tools/cmd/gomodcheck/main.go | 123 +++++++++++----------- 6 files changed, 65 insertions(+), 66 deletions(-) rename hack/.gomodcheck.yaml => .gomodcheck.yaml (100%) delete mode 100644 hack/tools/.keep diff --git a/hack/.gomodcheck.yaml b/.gomodcheck.yaml similarity index 100% rename from hack/.gomodcheck.yaml rename to .gomodcheck.yaml diff --git a/Makefile b/Makefile index bff67cf0c8..52aa05c2a6 100644 --- a/Makefile +++ b/Makefile @@ -94,7 +94,7 @@ $(GOLANGCI_LINT): # Build golangci-lint from tools folder. GO_MOD_CHECK_DIR := $(abspath ./hack/tools/cmd/gomodcheck) GO_MOD_CHECK := $(abspath $(TOOLS_BIN_DIR)/gomodcheck) -GO_MOD_CHECK_IGNORE := $(abspath ./hack/.gomodcheck.yaml) +GO_MOD_CHECK_IGNORE := $(abspath .gomodcheck.yaml) .PHONY: $(GO_MOD_CHECK) $(GO_MOD_CHECK): # Build gomodcheck go build -C $(GO_MOD_CHECK_DIR) -o $(GO_MOD_CHECK) @@ -149,5 +149,3 @@ APIDIFF_OLD_COMMIT ?= $(shell git rev-parse origin/main) .PHONY: apidiff verify-apidiff: $(GO_APIDIFF) ## Check for API differences $(GO_APIDIFF) $(APIDIFF_OLD_COMMIT) --print-compatible - - diff --git a/go.mod b/go.mod index fdb8ffa1a8..0d2ca57f0b 100644 --- a/go.mod +++ b/go.mod @@ -30,6 +30,8 @@ require ( sigs.k8s.io/yaml v1.3.0 ) +require golang.org/x/mod v0.15.0 + require ( github.com/antlr4-go/antlr/v4 v4.13.0 // indirect github.com/asaskevich/govalidator v0.0.0-20190424111038-f61b66f89f4a // indirect diff --git a/go.sum b/go.sum index 11eb5c0ba3..58b291e93a 100644 --- a/go.sum +++ b/go.sum @@ -156,6 +156,8 @@ golang.org/x/exp v0.0.0-20230515195305-f3d0a9c9a5cc h1:mCRnTeVUjcrhlRmO0VK8a6k6R golang.org/x/exp v0.0.0-20230515195305-f3d0a9c9a5cc/go.mod h1:V1LtkGg67GoY2N1AnLN78QLrzxkLyJw7RJb1gzOOz9w= golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= +golang.org/x/mod v0.15.0 h1:SernR4v+D55NyBH2QiEQrlBAnj1ECL6AGrA5+dPaMY8= +golang.org/x/mod v0.15.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20190603091049-60506f45cf65/go.mod h1:HSz+uSET+XFnRR8LxR5pz3Of3rY3CfYBVs4xY44aLks= golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= diff --git a/hack/tools/.keep b/hack/tools/.keep deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/hack/tools/cmd/gomodcheck/main.go b/hack/tools/cmd/gomodcheck/main.go index 9c32243976..45f0e6b96d 100644 --- a/hack/tools/cmd/gomodcheck/main.go +++ b/hack/tools/cmd/gomodcheck/main.go @@ -5,10 +5,10 @@ import ( "fmt" "os" "os/exec" - "regexp" "strings" "go.uber.org/zap" + "golang.org/x/mod/modfile" "sigs.k8s.io/yaml" ) @@ -17,8 +17,8 @@ const ( ) type config struct { - UpstreamRefs []string `yaml:"upstreamRefs"` - ExcludedModules []string `yaml:"excludedModules"` + UpstreamRefs []string `json:"upstreamRefs"` + ExcludedModules []string `json:"excludedModules"` } type upstream struct { @@ -45,12 +45,12 @@ func main() { // --- 1. parse config b, err := os.ReadFile(os.Args[1]) if err != nil { - logger.Fatal(err.Error()) + fatal(err) } cfg := new(config) if err := yaml.Unmarshal(b, cfg); err != nil { - logger.Fatal(err.Error()) + fatal(err) } excludedMods := make(map[string]any) @@ -59,15 +59,15 @@ func main() { } // --- 2. project mods - deps, err := parseModFile() + projectModules, err := modulesFromGoModFile() if err != nil { - logger.Fatal(err.Error()) + fatal(err) } - // --- 3. upstream mods (holding upstream refs) - upstreamModGraph, err := getUpstreamModGraph(cfg.UpstreamRefs) + // --- 3. upstream mods + upstreamModules, err := modulesFromUpstreamModGraph(cfg.UpstreamRefs) if err != nil { - logger.Fatal(err.Error()) + fatal(err) } oosMods := make([]oosMod, 0) @@ -78,13 +78,13 @@ func main() { // then for each upstream module, // if project module version doesn't match upstream version, // then we add the version and the ref to the list of out of sync modules. - for mod, version := range deps { + for mod, version := range projectModules { if _, ok := excludedMods[mod]; ok { logger.Infof("skipped excluded module: %s", mod) continue } - if versionToRef, ok := upstreamModGraph[mod]; ok { + if versionToRef, ok := upstreamModules[mod]; ok { upstreams := make([]upstream, 0) for upstreamVersion, upstreamRef := range versionToRef { @@ -107,97 +107,94 @@ func main() { } if len(oosMods) == 0 { - fmt.Println("Success! ๐ŸŽ‰") + fmt.Println("๐ŸŽ‰ Success!") os.Exit(0) } b, err = json.MarshalIndent(map[string]any{"outOfSyncModules": oosMods}, "", " ") if err != nil { - panic(err) + fatal(err) } fmt.Println(string(b)) os.Exit(1) } -var ( - cleanMods = regexp.MustCompile(`\t| *//.*`) - modDelimStart = regexp.MustCompile(`^require.*`) - modDelimEnd = ")" -) - -func parseModFile() (map[string]string, error) { +func modulesFromGoModFile() (map[string]string, error) { b, err := os.ReadFile(modFile) if err != nil { return nil, err } - in := string(cleanMods.ReplaceAll(b, []byte(""))) - out := make(map[string]string) - - start := false - for _, s := range strings.Split(in, "\n") { - switch { - case modDelimStart.MatchString(s) && !start: - start = true - case s == modDelimEnd: - return out, nil - case start: - kv := strings.SplitN(s, " ", 2) - if len(kv) < 2 { - return nil, fmt.Errorf("unexpected format for module: %q", s) - } + f, err := modfile.Parse(modFile, b, nil) + if err != nil { + return nil, err + } - out[kv[0]] = kv[1] - } + out := make(map[string]string) + for _, mod := range f.Require { + out[mod.Mod.Path] = mod.Mod.Version } return out, nil } -func getUpstreamModGraph(upstreamRefs []string) (map[string]map[string]string, error) { +func modulesFromUpstreamModGraph(upstreamRefList []string) (map[string]map[string]string, error) { b, err := exec.Command("go", "mod", "graph").Output() if err != nil { return nil, err } graph := string(b) - o1Refs := make(map[string]bool) - for _, upstreamRef := range upstreamRefs { - o1Refs[upstreamRef] = false + + // upstreamRefs is a set of user specified upstream modules. + // The set has 2 functions: + // 1. Check if `go mod graph` modules are one of the user specified upstream modules. + // 2. Mark if a user specified upstream module was found in the module graph. + // If a user specified upstream module is not found, gomodcheck will exit with an error. + upstreamRefs := make(map[string]bool) + for _, ref := range upstreamRefList { + upstreamRefs[ref] = false } modToVersionToUpstreamRef := make(map[string]map[string]string) - for _, line := range strings.Split(graph, "\n") { - upstreamRef := strings.SplitN(line, "@", 2)[0] - if _, ok := o1Refs[upstreamRef]; ok { - o1Refs[upstreamRef] = true - kv := strings.SplitN(strings.SplitN(line, " ", 2)[1], "@", 2) - name := kv[0] - version := kv[1] - - if m, ok := modToVersionToUpstreamRef[kv[0]]; ok { - m[version] = upstreamRef - } else { - versionToRef := map[string]string{version: upstreamRef} - modToVersionToUpstreamRef[name] = versionToRef - } + ref := strings.SplitN(line, "@", 2)[0] + + if _, ok := upstreamRefs[ref]; !ok { + continue } + + upstreamRefs[ref] = true // mark the ref as found + + kv := strings.SplitN(strings.SplitN(line, " ", 2)[1], "@", 2) + name := kv[0] + version := kv[1] + + if _, ok := modToVersionToUpstreamRef[name]; !ok { + modToVersionToUpstreamRef[name] = make(map[string]string) + } + + modToVersionToUpstreamRef[name][version] = ref } - notFound := "" - for ref, found := range o1Refs { + notFoundErr := "" + for ref, found := range upstreamRefs { if !found { - notFound = fmt.Sprintf("%s%s, ", notFound, ref) + notFoundErr = fmt.Sprintf("%s%s, ", notFoundErr, ref) } } - if notFound != "" { - return nil, fmt.Errorf("cannot verify modules;"+ - "the following specified upstream module cannot be found in go.mod: [ %s ]", - strings.TrimSuffix(notFound, ", ")) + if notFoundErr != "" { + return nil, fmt.Errorf("cannot verify modules: "+ + "the following specified upstream module(s) cannot be found in go.mod: [ %s ]", + strings.TrimSuffix(notFoundErr, ", ")) } return modToVersionToUpstreamRef, nil } + +func fatal(err error) { + fmt.Printf("โŒ %s\n", err.Error()) + os.Exit(1) +} From 105349a24db40d371d0b37ceab1a77ab3d7a3c2d Mon Sep 17 00:00:00 2001 From: Alexandre Mahdhaoui Date: Sat, 27 Apr 2024 12:07:08 +0200 Subject: [PATCH 4/4] =?UTF-8?q?=F0=9F=8C=B1=20simplify=20syntax?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Alexandre Mahdhaoui --- hack/tools/cmd/gomodcheck/main.go | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/hack/tools/cmd/gomodcheck/main.go b/hack/tools/cmd/gomodcheck/main.go index 45f0e6b96d..5cbaf377e2 100644 --- a/hack/tools/cmd/gomodcheck/main.go +++ b/hack/tools/cmd/gomodcheck/main.go @@ -80,29 +80,33 @@ func main() { // then we add the version and the ref to the list of out of sync modules. for mod, version := range projectModules { if _, ok := excludedMods[mod]; ok { - logger.Infof("skipped excluded module: %s", mod) + logger.Infof("skipped: %s", mod) continue } if versionToRef, ok := upstreamModules[mod]; ok { - upstreams := make([]upstream, 0) + outOfSyncUpstream := make([]upstream, 0) for upstreamVersion, upstreamRef := range versionToRef { - if version != upstreamVersion { - upstreams = append(upstreams, upstream{ - Ref: upstreamRef, - Version: upstreamVersion, - }) + if version == upstreamVersion { // pass if version in sync. + continue } - } - if len(upstreams) > 0 { - oosMods = append(oosMods, oosMod{ - Name: mod, - Version: version, - Upstreams: upstreams, + outOfSyncUpstream = append(outOfSyncUpstream, upstream{ + Ref: upstreamRef, + Version: upstreamVersion, }) } + + if len(outOfSyncUpstream) == 0 { // pass if no out of sync upstreams. + continue + } + + oosMods = append(oosMods, oosMod{ + Name: mod, + Version: version, + Upstreams: outOfSyncUpstream, + }) } }