-
Notifications
You must be signed in to change notification settings - Fork 45
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
Add support for Go workspaces #999
Add support for Go workspaces #999
Conversation
7ac5ce1
to
4fe7c95
Compare
3acda43
to
63d252b
Compare
e89852c
to
391a749
Compare
391a749
to
53906bc
Compare
) | ||
|
||
for module in workspace_modules: | ||
module.version = main_module_version |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I could tell, Cachito is ignoring version tags applied to submodules. So we can just use the main module version for all workspaces, since we're pretty much keeping the current behavior.
We should probably fix this in a follow up PR, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested it with:
{
"repo": "https://github.com/cachito-testing/gomod-pandemonium",
"ref": "0c6890c3280a00271891f4bd04705a56151428f0",
"pkg_managers": ["gomod"]
}
And I got:
{
"name": "github.com/cachito-testing/gomod-pandemonium/terminaltor",
"purl": "pkg:golang/github.com%2Fcachito-testing%2Fgomod-pandemonium%[email protected]",
"type": "library",
"version": "v0.1.0"
}
Although the version for terminaltor should've been v1.0.0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(The SBOM also had an entry in which the version for terminaltor was ./terminaltor
, so the behavior is a little odd)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think Cachito does support submodule versioning. But only if you tell it to process the submodule 🥲
i.e.
{
"repo": "https://github.com/cachito-testing/gomod-pandemonium",
"ref": "0c6890c3280a00271891f4bd04705a56151428f0",
"pkg_managers": ["gomod"],
"packages": {
"gomod": [{"path": "."}, {"path": "terminaltor"}, {"path": "weird"}]
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, I tried specifying all modules using the input you provided, and it still is correctly picking up for terminaltor:
{
"name": "github.com/cachito-testing/gomod-pandemonium/terminaltor",
"purl": "pkg:golang/github.com%2Fcachito-testing%2Fgomod-pandemonium%[email protected]",
"type": "library",
"version": "v1.0.0"
}
Which means we're still preserving the same behavior: submodule versions will only be picked up if the submodule is specified as a Cachito input package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't see any duplication in the resulting SBOM that happens as a side effect of specifying all the submodules/workspaces present in the repo. The terminaltor module has a duplication regardless of what input packages you specify, though:
{
"name": "github.com/cachito-testing/gomod-pandemonium/terminaltor",
"purl": "pkg:golang/github.com%2Fcachito-testing%2Fgomod-pandemonium%[email protected]",
"type": "library",
"version": "./terminaltor"
},
{
"name": "github.com/cachito-testing/gomod-pandemonium/terminaltor",
"purl": "pkg:golang/github.com%2Fcachito-testing%2Fgomod-pandemonium%[email protected]",
"type": "library",
"version": "v1.0.0"
}
This does not happen with weird
... I'm not sure what is causing it yet. 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On the one hand, with workspaces it doesn't make any sense to require that the user specify all submodules
On the other hand, ¯\_(ツ)_/¯
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, so I found out that the previous solution was indeed creating duplicate results in the SBOM. I rewrote it so:
- it picks the right version for each workspace
- the user does not need to specify multiple input packages anymore
0d6898d
to
8274915
Compare
New push: rewrites the logic that parses the workspace data so that the versions applied to submodules are properly picked up. |
"module_deps": main_module_deps, | ||
"packages": main_packages, | ||
} | ||
|
||
|
||
def _set_local_modules_versions( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like the idea of updating the object attributes in place, but I also didn't want to do a deep dive into refactoring get_golang_version
. This function takes a lot of attributes, and anything I wrote in the LocalModules class would bring all this crap together 😕
a442a58
to
a5c4200
Compare
Go 1.18 introduced support for workspaces¹, which were until now explicitly forbidden by Cachito when processing a request. The main change this feature brings is the return value for the "go list -m" command, which was used to retrieve the name of each of the modules specified as input packages in a Cachito request. This command now returns a list of all workspaces defined in the "go.work" file, which we will use to properly list them as part of the request's output. ¹ https://go.dev/ref/mod#workspaces Signed-off-by: Bruno Pimentel <[email protected]>
Since the support for workspaces was not properly implemented, we had a strict check on the existence of a "go.work" file in a repository, which would then trigger a failure in a request. Now that proper support is implemented, let's remove this check. Signed-off-by: Bruno Pimentel <[email protected]>
Signed-off-by: Bruno Pimentel <[email protected]>
a5c4200
to
88b8e75
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Just to make sure I understand the limitations
- all the workspaces will be reported with correct versions, doesn't matter if the user tells cachito to process just one workspace or all of them
- packages from a workspace will only be reported if the user tells cachito to process that workspace
More or less?
Yes, because
Yes, because of the limitation of |
c36a7da
This commit changes the resolve_gomod function so that it can handle a json stream coming from
go list -m
, which happens when workspaces are used in a Go project.It also removes the previous restrictions on using workspaces.
Maintainers will complete the following section