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

Use go list -m -versions to determine available versions of a go module #4434

Merged
merged 8 commits into from Nov 23, 2021

Conversation

mctofu
Copy link
Contributor

@mctofu mctofu commented Nov 19, 2021

This removes usage of the native helper to resolve available go module versions. Instead we'll use the existing go tooling which has added support for this. This has a few benefits:

  • No longer need to maintain https://github.com/dependabot/gomodules-extracted or make calls to internal go libs
  • No longer need to manually handle exclude directives
  • Retract directives are now supported so Dependabot won't propose updates to retracted versions

We previously attempted this change in #3630 but the approach was abandoned after we found the change was negatively impacting performance. It was further noted that using go list -m -versions was causing the go tooling to download all of the modules referenced in go.mod and this accounted for the bulk of the performance hit. This appears to only happen when GOPRIVATE=* is set which we currently need to do to support private modules.

As a workaround I found that providing a go.mod with no requirements was a way to sidestep the performance issue. We still end up getting the version information we are looking for and no extra modules are downloaded! The performance is close to the native version as well.

Example output / performance

Using native helper
[dependabot-core-dev] /tmp/go $ rm -rf /home/dependabot/go/pkg/mod/cache
[dependabot-core-dev] /tmp/go $ cat go.mod
module foo

require github.com/dependabot-fixtures/go-modules-lib v1.0.0

go 1.15
[dependabot-core-dev] /tmp/go $ time echo '{"function":"getVersions","args":{"dependency":{"name":"github.com/dependabot-fixtures/go-modules-lib","version":"v1.0.0"}}}'  | GOPRIVATE='*' /opt/go_modules/bin/helper
{"result":["v1.0.0","v1.0.1","v1.0.5","v1.0.6","v1.1.0","v1.2.0-pre1","v1.2.0-pre2"]}
real    0m1.017s
user    0m0.105s
sys     0m0.087s
[dependabot-core-dev] /tmp/go $ du -sk /home/dependabot/go/pkg/mod/cache
176     /home/dependabot/go/pkg/mod/cache
Using `go list -m -versions` with project `go.mod`
[dependabot-core-dev] /tmp/go $ rm -rf /home/dependabot/go/pkg/mod/cache
[dependabot-core-dev] /tmp/go $ cat go.mod
module foo

require github.com/dependabot-fixtures/go-modules-lib v1.0.0

go 1.15
[dependabot-core-dev] /tmp/go $ time GOPRIVATE='*' go list -m -versions -json github.com/dependabot-fixtures/go-modules-lib
{
        "Path": "github.com/dependabot-fixtures/go-modules-lib",
        "Version": "v1.0.0",
        "Versions": [
                "v1.0.0",
                "v1.0.1",
                "v1.0.5",
                "v1.0.6",
                "v1.1.0",
                "v1.2.0-pre1",
                "v1.2.0-pre2"
        ],
        "Time": "2018-10-18T21:48:48Z",
        "GoMod": "/home/dependabot/go/pkg/mod/cache/download/github.com/dependabot-fixtures/go-modules-lib/@v/v1.0.0.mod"
}

real    0m8.828s
user    0m1.882s
sys     0m1.465s
[dependabot-core-dev] /tmp/go $ du -sk /home/dependabot/go/pkg/mod/cache
24844   /home/dependabot/go/pkg/mod/cache
Using `go list -m -versions` with stub `go.mod` (this PR)
[dependabot-core-dev] /tmp/go $ rm -rf /home/dependabot/go/pkg/mod/cache
[dependabot-core-dev] /tmp/go $ cat go.mod
module foo
[dependabot-core-dev] /tmp/go $ time GOPRIVATE='*' go list -m -versions -json github.com/dependabot-fixtures/go-modules-lib
{
        "Path": "github.com/dependabot-fixtures/go-modules-lib",
        "Versions": [
                "v1.0.0",
                "v1.0.1",
                "v1.0.5",
                "v1.0.6",
                "v1.1.0",
                "v1.2.0-pre1",
                "v1.2.0-pre2"
        ]
}

real    0m1.705s
user    0m0.140s
sys     0m0.135s
[dependabot-core-dev] /tmp/go $ du -sk /home/dependabot/go/pkg/mod/cache
228     /home/dependabot/go/pkg/mod/cache

Other notes

  • This change revealed that the v2 release of our test fixture wasn't actually valid. I created a valid v3 release and added handling for invalid version errors.
  • I believe the usage of SharedHelpers.run_shell_command will escape the args being passed to go mod list -m -versions and go mod edit to prevent command injection. I'd appreciate a 2nd look at this though.

go list returns a different error message:
go list -m: malformed module path "pkg-errors": missing dot in first path element
go list -m properly handles retractions so this test is now passing
Prior error:
go list -m: loading module retractions for github.com/dependabot-fixtures/go-modules-lib/v2@v2.0.0: version "v2.0.0" invalid: go.mod has non-.../v2 module path "github.com/dependabot-fixtures/go-modules-lib" (and .../v2/go.mod does not exist) at revision v2.0.0
@mctofu mctofu changed the title Use go mod list -m -versions to determine available versions of a go module Use go list -m -versions to determine available versions of a go module Nov 20, 2021
@mctofu mctofu marked this pull request as ready for review November 20, 2021 00:37
@mctofu mctofu requested a review from a team as a code owner November 20, 2021 00:37
@jeffwidman
Copy link
Member

As a workaround I found that providing a go.mod with no requirements was a way to sidestep the performance issue. We still end up getting the version information we are looking for and no extra modules are downloaded!

Very clever!

Copy link
Member

@jurre jurre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great, so glad we're able to get rid of gomodules-extracted!

I believe the usage of SharedHelpers.run_shell_command will escape the args being passed to go mod list -m -versions and go mod edit to prevent command injection. I'd appreciate a 2nd look at this though.

That's right, unless we explicitly tell run_shell_command to allow_unsafe_shell_command it runs any command we pass through Shellwords which should protect against any command injection 👍

@weitzj
Copy link

weitzj commented Nov 22, 2021

Looking forward to this release.

Co-Authored-By: Mattt Zmuda <mattt@github.com>
Co-Authored-By: Landon Grindheim <landongrindheim@github.com>
File.write("go.mod", go_mod.content)
manifest = parse_manifest

# Set up an empty go.mod so 'go list -m' won't attempt to download dependencies. This
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One other observation from testing is that we can also run go list -m -versions without a go.mod and it works the same way. However, it won't know about the exclude directives and won't be able to apply them for us so creating a go.mod with the excludes is still preferred.

@mctofu mctofu merged commit 9060bc4 into main Nov 23, 2021
@mctofu mctofu deleted the mctofu/go-mod-list-versions branch November 23, 2021 18:41
@mctofu mctofu mentioned this pull request Nov 23, 2021
case "getVersions":
var args updatechecker.Args
parseArgs(helperParams.Args, &args)
funcOut, funcErr = updatechecker.GetVersions(&args)
case "getVcsRemoteForImport":
var args importresolver.Args
parseArgs(helperParams.Args, &args)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May be able to delete this as well, I filed #4448 for that

@jeffwidman
Copy link
Member

Nicely done!

I opened dependabot/gomodules-extracted#17 and dependabot/gomodules-extracted#18 as followups to this.

@mctofu
Copy link
Contributor Author

mctofu commented Nov 24, 2021

I noticed one unanticipated side effect where Dependabot now updates major versions for +incompatible versions: #4453

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
L: go:modules Golang modules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants