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

fix(go): Do not trim v prefix from versions in Go Mod Analyzer #7733

Merged
merged 13 commits into from
Oct 31, 2024

Conversation

Rutam21
Copy link
Contributor

@Rutam21 Rutam21 commented Oct 14, 2024

Description

This PR modifies the Go Mod Analyzer file so that the prefix "v" is not trimmed from the versions to maintain consistency with the Go Binary Analyzer.

Related issues

Checklist

@CLAassistant
Copy link

CLAassistant commented Oct 14, 2024

CLA assistant check
All committers have signed the CLA.

@Rutam21 Rutam21 changed the title IS-7711: Do not trim v prefix from versions in Go Mod Analyzer fix(go): Do not trim v prefix from versions in Go Mod Analyzer Oct 14, 2024
Copy link
Contributor

@DmitriyLewen DmitriyLewen left a comment

Choose a reason for hiding this comment

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

Hello @Rutam21
Thanks for your work!

can you update tests too?

Regards, Dmitriy

@Rutam21 Rutam21 requested a review from DmitriyLewen October 15, 2024 07:54
@Rutam21
Copy link
Contributor Author

Rutam21 commented Oct 15, 2024

Hello @DmitriyLewen, I have added the v prefix to the version numbers in the test and test-cases file for the Go Mod Analyzer so that it matches the required versioning format. Please review and suggest changes, if any. Thanks.

@DmitriyLewen
Copy link
Contributor

You also need to update integration and k8s tests.
mage test:updateGolden command should help

@Rutam21
Copy link
Contributor Author

Rutam21 commented Oct 24, 2024

@DmitriyLewen I tried running that command but it always errors out. Any suggestions on this?

go: downloading github.com/vmihailenco/msgpack/v5 v5.3.5
go: downloading github.com/dsnet/compress v0.0.1
go: downloading sigs.k8s.io/yaml v1.4.0
go: creating work dir: mkdir /tmp/go-build220524866: no space left on device
Error: running "go test -tags=integration ./integration/... ./pkg/fanal/test/integration/... -update" failed with exit code 1

@knqyf263
Copy link
Collaborator

@Rutam21 You don't have enough disk space.

go: creating work dir: mkdir /tmp/go-build220524866: no space left on device

If you have more space in another directory, you can specify it like TMPDIR=/path/to/another-dir.

@knqyf263
Copy link
Collaborator

@Rutam21 Do you have time to wrap it up? We want to include this fix in v0.57.0. If you're busy, we can take care of it.

@DmitriyLewen
Copy link
Contributor

DmitriyLewen commented Oct 29, 2024

@Rutam21 don't worry i updated golden files

@knqyf263 when I updated the gold files - I thought - do we need to update the docs/other tests?
e.g. :

Also i thought about stdlib - do we need to add v prefix?

  • go.mod go line uses x.y.z format (e.g. go 1.22.0)
  • go.mod toolchain and go version -m use gox.y.z format (e.g. toolchain go1.22.4 and trivy: go1.22.5)

@Rutam21
Copy link
Contributor Author

Rutam21 commented Oct 29, 2024

Thank you @DmitriyLewen. Due to storage constraints, the suggested command was failing for me always.

@knqyf263
Copy link
Collaborator

@knqyf263 when I updated the gold files - I thought - do we need to update the docs/other tests?

Yes, it would be great if the version formats are consistent in Trivy so that we will not get confused in the future.

Also, I thought about stdlib - do we need to add a v prefix?

goV seems like a canonical format, but our versioning library doesn't support it and needs special handling for that. So, I think we want to add the v prefix for consistency.

@DmitriyLewen
Copy link
Contributor

@knqyf263 I updated testcases and docs.
I didn't update rekor tests (it requires regenerating the digest, so I think we can skip the changes since it's not worth the time)

Also we need to update k8s scanner package to add v prefix, but i think that should be done in a separate PR.

So, I think we want to add the v prefix for consistency.

Done in d5a787b

@knqyf263
Copy link
Collaborator

Thanks. GitHub Actions is down now. We'll merge this PR after it's back.

@DmitriyLewen
Copy link
Contributor

@knqyf263 We don't add v prefix for stdlib of gobinaries.
I added it in - 5b5b7f8

Take a look, please.

I also created #7822 for k8s scanner

@knqyf263
Copy link
Collaborator

knqyf263 commented Oct 30, 2024

@DmitriyLewen
Copy link
Contributor

Rebased this PR.
Tests now pass.

@knqyf263
Copy link
Collaborator

@DmitriyLewen You requested changes before. Your review is blocking this PR.

@knqyf263 knqyf263 added this pull request to the merge queue Oct 31, 2024
@DmitriyLewen
Copy link
Contributor

Approved

Merged via the queue into aquasecurity:main with commit e872ec0 Oct 31, 2024
17 checks passed
@Rutam21 Rutam21 deleted the FIX-7711 branch October 31, 2024 06:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fix(golang): do not trim v prefix from versions
4 participants