-
Notifications
You must be signed in to change notification settings - Fork 23
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 golang purl service #316
Conversation
Signed-off-by: Tushar Goel <[email protected]>
Signed-off-by: Tushar Goel <[email protected]>
Signed-off-by: Tushar Goel <[email protected]>
Signed-off-by: Tushar Goel <[email protected]>
Signed-off-by: Tushar Goel <[email protected]>
Signed-off-by: Tushar Goel <[email protected]>
Signed-off-by: Tushar Goel <[email protected]>
Signed-off-by: Tushar Goel <[email protected]>
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.
Thanks @TG1999, few nits for your consideration.
], | ||
responses={200: GoLangPurlResponseSerializer()}, | ||
) | ||
class GolangPurlViewSet(viewsets.ViewSet): |
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.
Need some docsting. Being more explicit about the fact that the endpoint accepts a standard Go import string and returns the corresponding PackageURL will be helpful.
packagedb/tests/test_api.py
Outdated
class ToGolangPurlTestCase(TestCase): | ||
|
||
def test_to_golang_purl(self): | ||
response = self.client.get("/api/to_purl/go", data={"go_package": "github.com/gorilla/[email protected]"}, follow=True) |
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.
AFAIK, github.com/gorilla/[email protected]
is not a valid import statement.
Does Go allow specifying a particular package version using an @
symbol in an import statement?
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.
Good catch, not a go import https://github.com/moby/moby/blob/6c10086976d07d4746e03dcfd188972a2f07e1c9/vendor.mod#L51 but *.mod
files does have version representation like this? should I change the code in packageurl-python to accept this kind of inputs as well ?
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.
should I change the code in packageurl-python to accept this kind of inputs as well ?
Update the code to also accept this would make sense. With proper documentation.
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.
BTW, see also https://stackoverflow.com/questions/24855081/how-do-i-import-a-specific-version-of-a-package-using-go-get and in particular the reference to http://labix.org/gopkg.in
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.
Good catch, not a go import https://github.com/moby/moby/blob/6c10086976d07d4746e03dcfd188972a2f07e1c9/vendor.mod#L51 but
*.mod
files does have version representation like this? should I change the code in packageurl-python to accept this kind of inputs as well ?
IMO it makes sense to support getting PURLs from both Go import and Go Manifest.
However, I'm not sure if we want to support obtaining PURLs from Go dependency management commands like...
go get github.com/gorilla/[email protected]
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.
@keshav-space so should we throw an error when we encounter any package like this ? when there is "@" in package, we should error out ?
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 have added a PR here https://github.com/package-url/packageurl-python/pull/148/files, please have a look
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.
@keshav-space so should we throw an error when we encounter any package like this ? when there is "@" in package, we should error out ?
If we don't support them in packageurl-python, then we should raise an error here.
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 this is all OK and handled in the packageurl library. And we do not support the @ notation from a "go get"
Signed-off-by: Tushar Goel <[email protected]>
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.
We just need a bit of documentation and changelog and how to test this to complete the thing.
Signed-off-by: Tushar Goel <[email protected]>
Signed-off-by: Tushar Goel <[email protected]>
Signed-off-by: Tushar Goel <[email protected]>
Signed-off-by: Tushar Goel <[email protected]>
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.
@TG1999 LGTM, few nits for your consideration.
Co-authored-by: Keshav Priyadarshi <[email protected]>
Co-authored-by: Keshav Priyadarshi <[email protected]>
Fixes: #259
Added a
/api/to_purl/go
endpoint to get a golang purl from a go import string or a package string in go.mod file