Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Go directive in go.mod not updated when a new dependency requires it #9527

Open
1 task done
matthewhughes-uw opened this issue Apr 18, 2024 · 3 comments
Open
1 task done
Labels
L: go:modules Golang modules T: bug 🐞 Something isn't working

Comments

@matthewhughes-uw
Copy link

matthewhughes-uw commented Apr 18, 2024

Is there an existing issue for this?

  • I have searched the existing issues

Package ecosystem

go_module

Package manager version

No response

Language version

Go 1.22.0

Manifest location and content before the Dependabot update

No response

dependabot.yml content

Source: https://github.com/utilitywarehouse/manifest-checkers/blob/main/.github/dependabot.yml

version: 2
updates:
  - package-ecosystem: "gomod"
    directory: "/"
    schedule:
      interval: "daily"
  - package-ecosystem: "github-actions"
    directory: "/"
    schedule:
      interval: "daily"

Updated dependency

k8s.io/apimachinery from v0.29.3 to v0.30.0

What you expected to see, versus what you actually saw

What I expected to see: dependency is updated, the update includes a change that means the dependency requires go 1.22.0+ so I expect dependabot to update the go directive in go.mod to reflect this, like go get does:

$ go get k8s.io/apimachinery@v0.30.0
$ git diff go.mod
diff --git a/go.mod b/go.mod
index 306a5dd..810faaf 100644
--- a/go.mod
+++ b/go.mod
@@ -1,6 +1,8 @@
 module github.com/utilitywarehouse/manifest-checkers
 
-go 1.21.0
+go 1.22.0
+
+toolchain go1.22.2
 
 require (
 	github.com/golangci/golangci-lint v1.57.2
@@ -8,7 +10,7 @@ require (
 	github.com/urfave/cli/v2 v2.27.1
 	gitlab.com/matthewhughes/go-cov v0.3.0
 	golang.org/x/sync v0.7.0
-	k8s.io/apimachinery v0.29.3
+	k8s.io/apimachinery v0.30.0
 	k8s.io/client-go v0.29.3
 	sigs.k8s.io/controller-runtime v0.17.3
 )
@@ -197,7 +199,7 @@ require (
 	golang.org/x/exp v0.0.0-20240103183307-be819d1f06fc // indirect
 	golang.org/x/exp/typeparams v0.0.0-20240314144324-c7f7c6466f7f // indirect
 	golang.org/x/mod v0.16.0 // indirect
-	golang.org/x/net v0.22.0 // indirect
+	golang.org/x/net v0.23.0 // indirect
 	golang.org/x/oauth2 v0.15.0 // indirect
 	golang.org/x/sys v0.18.0 // indirect
 	golang.org/x/term v0.18.0 // indirect
@@ -212,8 +214,8 @@ require (
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 	honnef.co/go/tools v0.4.7 // indirect
 	k8s.io/api v0.29.3 // indirect
-	k8s.io/klog/v2 v2.110.1 // indirect
-	k8s.io/kube-openapi v0.0.0-20231010175941-2dd684a91f00 // indirect
+	k8s.io/klog/v2 v2.120.1 // indirect
+	k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340 // indirect
 	k8s.io/utils v0.0.0-20230726121419-3b25d923346b // indirect
 	mvdan.cc/gofumpt v0.6.0 // indirect
 	mvdan.cc/unparam v0.0.0-20240104100049-c549a3470d14 // indirect

The diff produced by dependabot (see: utilitywarehouse/manifest-checkers#20):

diff --git a/go.mod b/go.mod
index 306a5dd..6f421b3 100644
--- a/go.mod
+++ b/go.mod
@@ -1,6 +1,7 @@
 module github.com/utilitywarehouse/manifest-checkers
 
 go 1.21.0
+toolchain go1.22.2
 
 require (
        github.com/golangci/golangci-lint v1.57.2
@@ -8,7 +9,7 @@ require (
        github.com/urfave/cli/v2 v2.27.1
        gitlab.com/matthewhughes/go-cov v0.3.0
        golang.org/x/sync v0.7.0
-       k8s.io/apimachinery v0.29.3
+       k8s.io/apimachinery v0.30.0
        k8s.io/client-go v0.29.3
        sigs.k8s.io/controller-runtime v0.17.3
 )
@@ -197,7 +198,7 @@ require (
        golang.org/x/exp v0.0.0-20240103183307-be819d1f06fc // indirect
        golang.org/x/exp/typeparams v0.0.0-20240314144324-c7f7c6466f7f // indirect
        golang.org/x/mod v0.16.0 // indirect
-       golang.org/x/net v0.22.0 // indirect
+       golang.org/x/net v0.23.0 // indirect
        golang.org/x/oauth2 v0.15.0 // indirect
        golang.org/x/sys v0.18.0 // indirect
        golang.org/x/term v0.18.0 // indirect
@@ -212,8 +213,8 @@ require (
        gopkg.in/yaml.v3 v3.0.1 // indirect
        honnef.co/go/tools v0.4.7 // indirect
        k8s.io/api v0.29.3 // indirect
-       k8s.io/klog/v2 v2.110.1 // indirect
-       k8s.io/kube-openapi v0.0.0-20231010175941-2dd684a91f00 // indirect
+       k8s.io/klog/v2 v2.120.1 // indirect
+       k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340 // indirect
        k8s.io/utils v0.0.0-20230726121419-3b25d923346b // indirect
        mvdan.cc/gofumpt v0.6.0 // indirect
        mvdan.cc/unparam v0.0.0-20240104100049-c549a3470d14 // indirect

This causes issues, e.g. running go test ./... errors with go: updates to go.mod needed; to update it:

Native package manager behavior

See above

Images of the diff or a link to the PR, issue, or logs

Links in expected behaviour above

Smallest manifest that reproduces the issue

No response

@matthewhughes-uw matthewhughes-uw added the T: bug 🐞 Something isn't working label Apr 18, 2024
@github-actions github-actions bot added L: git:submodules Git submodules L: github:actions GitHub Actions L: go:modules Golang modules labels Apr 18, 2024
@jakecoffman jakecoffman removed L: github:actions GitHub Actions L: git:submodules Git submodules labels Apr 18, 2024
@kwiesmueller
Copy link

kwiesmueller commented Apr 22, 2024

I just noticed for the first time in a dependabot PR that it's setting the toolchain field and am not sure if that's the right behavior.
kubernetes/autoscaler#6735

I think which toolchain version is recommended should be a per project decision as there may be subtle differences in behavior. From what I read on https://go.dev/ref/mod#go-mod-file-toolchain the toolchain is set by the go command now automatically based on it's own version. If people agree that the toolchain choice depends on the project this could be an issue for projects like dependabot.

Options I see right now are:

  • Let projects specify the go tool version dependabot uses
  • Remove any toolchain changes from the patches (potentially causing inconsistencies?)

LMK if this is unrelated to this issue, I can move it to a new one.

@matthewhughes934
Copy link

Piecing together the bits that were seen in the original post: per the Dockerfile

By specifying go1.20.10, we use 1.20.10 for any go.mod with go directive <= 1.20.
In the file_parser, GOTOOLCHAIN=local+auto is set otherwise, which uses the latest version above

running GOTOOLCHAIN=local+auto go get k8s.io/apimachinery@v0.30.0 I see the toolchain directive is added at go1.22.2 (i.e. this is wholly the doing of the go command). This looks to be the documented behaviour: https://go.dev/ref/mod#go-mod-file-toolchain

For reproducibility, the go command writes its own toolchain name in a toolchain line any time it is updating the go version in the go.mod file (usually during go get).

Next it looks like dependabot will edit the go directive if it was updated (here) I don't see any explicit docs on when to expect this to be updated by the go tool (from reading https://go.dev/ref/mod#go-mod-file-go) but I'm wondering if it's reasonable to persist a change here if one was made (under the assumption that change to go directive = update a dependency that requires a newer version of Go)

@matthewhughes934
Copy link

matthewhughes934 commented Apr 25, 2024

I see some more context/discussion around the go tool setting the toolchain directive here: golang/go#65847

From that thread: dependabot could control updates to the toolchain directive by simply stopping when there's also an update to the go directive (maybe dependabot a should flag such updates as incompatible anyway?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
L: go:modules Golang modules T: bug 🐞 Something isn't working
Projects
Status: No status
Development

No branches or pull requests

4 participants