-
Notifications
You must be signed in to change notification settings - Fork 598
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 distro information to package URLs for OS packages #754
Conversation
Benchmark Test ResultsBenchmark results from the latest changes vs base branch
|
7cec450
to
5503fbd
Compare
3b8b6fe
to
e9bcf24
Compare
5503fbd
to
ff7557f
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.
If you think this is ready, 👍
}, | ||
metadata: DpkgMetadata{ | ||
Package: "p", | ||
Source: "s", | ||
Version: "v", | ||
Architecture: "a", | ||
}, | ||
expected: "pkg:deb/ubuntu/p@v?arch=a", | ||
expected: "pkg:deb/ubuntu/p@v?arch=a&distro=ubuntu-16.04", |
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.
One question, though: should these be more like pkg:deb/ubuntu/p@v?arch=a&distro=16.04
without the ubuntu-
? (After looking again at: https://github.com/package-url/purl-spec/blob/master/PURL-TYPES.rst#deb )
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.
Honestly, I'm not certain. The spec only has one "good" example with this here: https://github.com/package-url/purl-spec/blob/1b1e9b2afa6de1c855225fb08cb46878ad653925/PURL-SPECIFICATION.rst#some-purl-examples
pkg:deb/debian/[email protected]?arch=i386&distro=jessie
pkg:rpm/fedora/[email protected]?arch=i386&distro=fedora-25
The debian distro code-name is a bad idea for us, so I didn't approach it that way... so the fedora example was used as a basis. That being said, there is essentially 0 verbiage on correct usage of distro
even though it is a suggested qualifier (that we need).
References:
- https://github.com/package-url/purl-spec/tree/a21a9ecbab171fe06d1ac88843bc3721a42dbed0#known-qualifiers-keyvalue-pairs
- https://github.com/package-url/purl-spec/blob/master/PURL-TYPES.rst#rpm
Since there is little direction from the spec I'm open to other options 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.
From an offline conversation: we'll leave this as is for now since it seems more semantically correct to the term "distro" but are open to changing in the future.
Signed-off-by: Alex Goodman <[email protected]>
Signed-off-by: Alex Goodman <[email protected]>
Signed-off-by: Alex Goodman <[email protected]>
Signed-off-by: Alex Goodman <[email protected]>
ff7557f
to
0210a16
Compare
(force pushed to rebase from main now that #753 is merged) |
looks like the codeql workflow hasn't kicked in after a long while (hours) This could be due to the fact that the base branch was originally not Changing the PR from draft state and back didn't do the trick. Neither did switching the base branch to something else and back to main. Also it seems like all pull request events are triggering for this PR at all... maybe a github actions bug? |
I'm going to close and reopen this PR to hopefully trigger the pull_request events for this PR. |
* rename npm metadata struct Signed-off-by: Alex Goodman <[email protected]> * improve os package URLs Signed-off-by: Alex Goodman <[email protected]> * improve language package URLs Signed-off-by: Alex Goodman <[email protected]> * wire up composer pURL method Signed-off-by: Alex Goodman <[email protected]> Signed-off-by: fsl <[email protected]>
* rename npm metadata struct Signed-off-by: Alex Goodman <[email protected]> * improve os package URLs Signed-off-by: Alex Goodman <[email protected]> * improve language package URLs Signed-off-by: Alex Goodman <[email protected]> * wire up composer pURL method Signed-off-by: Alex Goodman <[email protected]>
* rename npm metadata struct Signed-off-by: Alex Goodman <[email protected]> * improve os package URLs Signed-off-by: Alex Goodman <[email protected]> * improve language package URLs Signed-off-by: Alex Goodman <[email protected]> * wire up composer pURL method Signed-off-by: Alex Goodman <[email protected]>
* rename npm metadata struct Signed-off-by: Alex Goodman <[email protected]> * improve os package URLs Signed-off-by: Alex Goodman <[email protected]> * improve language package URLs Signed-off-by: Alex Goodman <[email protected]> * wire up composer pURL method Signed-off-by: Alex Goodman <[email protected]>
This PR does primarily enhances up support for OS-related packages to include distro version information in any pURL generated:
The
ID
andVersionID
are encoded into the pURL as adistro
get-param; see the pURL spec for more info.Additionally this PR:
pkg
package (since this semantically represents packages)pkg
was inconsistent --the filename was updatedRelated to anchore/grype#395
Note: this PR cannot be merged until #753 is merged and this PR is rebased.