search

dependabot / dependabot-core

🤖 Dependabot's core logic for creating update PRs.
https://docs.github.com/en/code-security/dependabot
MIT License
4.73k stars 1.02k forks source link

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

Open matthewhughes-uw opened 7 months ago

matthewhughes-uw commented 7 months ago

Is there an existing issue for this?

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: https://github.com/utilitywarehouse/manifest-checkers/pull/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

kwiesmueller commented 7 months ago

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. https://github.com/kubernetes/autoscaler/pull/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:

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

matthewhughes934 commented 7 months ago

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 commented 7 months ago

I see some more context/discussion around the go tool setting the toolchain directive here: https://github.com/golang/go/issues/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?)

phoenix2x commented 2 weeks ago

Hi everyone, Any updates on this? We hit the same issue where bot PRs fail with go: updates to go.mod needed; to update it:go mod tidy reading through docs seems like the bot needs to update the go directive

A module’s go line must declare a version greater than or equal to the go version declared by each of the modules listed in require statements.
....
Before Go 1.21, Go toolchains treated the go line as an advisory requirement: if builds succeeded the toolchain assumed everything worked, and if not it printed a note about the potential version mismatch. Go 1.21 changed the go line to be a mandatory requirement instead.

And looking into the bot code it seems to be reverting this change made by go get :https://github.com/dependabot/dependabot-core/blob/313fcff149b3126cb78b38d15f018907d729f8cc/go_modules/lib/dependabot/go_modules/file_updater/go_mod_updater.rb#L169

What would be the recommended way to fix the error above?