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.

Sign up for GitHub

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

Jump to bottom

Add pURL generation for java packages + fix NPM pURL generation #812

Merged
merged 3 commits into from Feb 10, 2022
Merged

Add pURL generation for java packages + fix NPM pURL generation #812

merged 3 commits into from Feb 10, 2022

Conversation

wagoodman
Copy link
Contributor

@wagoodman wagoodman commented Feb 10, 2022 •
edited

This PR enhances pURL generation in the following ways:

  • Java pURLs now cover more cases. By using the Group ID extraction logic that drives the CPE generation, and name/version extraction logic provided by the java cataloger today, we can assemble a pURL even if we are missing a pom.properties file. Note: enough parser-specific logic was needed that this is synthesized in the java cataloger package and persisted in the java.Metadata.
  • NPM pURLs never accounted for scoped packages, which the purl spec does account for: the 'namespace' is used for the scope of a scoped NPM package. So the package name @angular/animation should be split such that the pURL namespace is @angular instead of encoding the full name into the pURL package name.

Related to anchore/grype#395

@wagoodman
enhance pURL generation for java packages
db970ac 
Signed-off-by: Alex Goodman <alex.goodman@anchore.com>
@wagoodman
optionally split out npm namespaces for pURL generation
8904dae 
Signed-off-by: Alex Goodman <alex.goodman@anchore.com>
@wagoodman wagoodman added the enhancement New feature or request label Feb 10, 2022
@wagoodman wagoodman requested a review from a team February 10, 2022 18:27
@github-actions
Copy link

github-actions bot commented Feb 10, 2022 •
edited

Benchmark Test Results

Benchmark results from the latest changes vs base branch 
name                                                       old time/op    new time/op    delta
ImagePackageCatalogers/ruby-gemspec-cataloger-2              1.28ms ± 0%    1.26ms ± 1%  -1.32%  (p=0.016 n=4+5)
ImagePackageCatalogers/python-package-cataloger-2            2.93ms ± 2%    2.89ms ± 0%  -1.63%  (p=0.016 n=5+4)
ImagePackageCatalogers/php-composer-installed-cataloger-2    1.01ms ± 1%    0.99ms ± 0%  -1.42%  (p=0.008 n=5+5)
ImagePackageCatalogers/javascript-package-cataloger-2         786µs ± 1%     768µs ± 0%  -2.19%  (p=0.008 n=5+5)
ImagePackageCatalogers/dpkgdb-cataloger-2                     911µs ± 0%     896µs ± 0%  -1.58%  (p=0.008 n=5+5)
ImagePackageCatalogers/rpmdb-cataloger-2                      822µs ± 1%     803µs ± 0%  -2.21%  (p=0.008 n=5+5)
ImagePackageCatalogers/java-cataloger-2                      22.8ms ± 1%    22.2ms ± 1%  -2.66%  (p=0.008 n=5+5)
ImagePackageCatalogers/apkdb-cataloger-2                     1.22ms ± 1%    1.20ms ± 0%  -1.61%  (p=0.008 n=5+5)
ImagePackageCatalogers/go-module-binary-cataloger-2          1.94µs ± 0%    1.91µs ± 1%  -1.56%  (p=0.008 n=5+5)

name                                                       old alloc/op   new alloc/op   delta
ImagePackageCatalogers/ruby-gemspec-cataloger-2               252kB ± 0%     252kB ± 0%    ~     (p=0.841 n=5+5)
ImagePackageCatalogers/python-package-cataloger-2            1.06MB ± 0%    1.06MB ± 0%    ~     (p=0.151 n=5+5)
ImagePackageCatalogers/php-composer-installed-cataloger-2     253kB ± 0%     253kB ± 0%  -0.11%  (p=0.008 n=5+5)
ImagePackageCatalogers/javascript-package-cataloger-2         207kB ± 0%     207kB ± 0%    ~     (p=0.690 n=5+5)
ImagePackageCatalogers/dpkgdb-cataloger-2                     254kB ± 0%     254kB ± 0%  -0.09%  (p=0.032 n=5+5)
ImagePackageCatalogers/rpmdb-cataloger-2                      236kB ± 0%     235kB ± 0%  -0.12%  (p=0.008 n=5+5)
ImagePackageCatalogers/java-cataloger-2                      4.19MB ± 0%    4.18MB ± 0%    ~     (p=0.421 n=5+5)
ImagePackageCatalogers/apkdb-cataloger-2                     1.30MB ± 0%    1.30MB ± 0%  +0.04%  (p=0.032 n=5+5)
ImagePackageCatalogers/go-module-binary-cataloger-2            608B ± 0%      608B ± 0%    ~     (all equal)

name                                                       old allocs/op  new allocs/op  delta
ImagePackageCatalogers/ruby-gemspec-cataloger-2               6.33k ± 0%     6.33k ± 0%    ~     (all equal)
ImagePackageCatalogers/python-package-cataloger-2             21.4k ± 0%     21.4k ± 0%    ~     (p=0.246 n=5+5)
ImagePackageCatalogers/php-composer-installed-cataloger-2     7.25k ± 0%     7.25k ± 0%    ~     (p=0.881 n=5+5)
ImagePackageCatalogers/javascript-package-cataloger-2         5.36k ± 0%     5.36k ± 0%  +0.02%  (p=0.008 n=5+5)
ImagePackageCatalogers/dpkgdb-cataloger-2                     7.10k ± 0%     7.10k ± 0%    ~     (all equal)
ImagePackageCatalogers/rpmdb-cataloger-2                      6.82k ± 0%     6.82k ± 0%    ~     (all equal)
ImagePackageCatalogers/java-cataloger-2                       86.8k ± 0%     86.8k ± 0%  +0.03%  (p=0.016 n=5+5)
ImagePackageCatalogers/apkdb-cataloger-2                      7.37k ± 0%     7.37k ± 0%    ~     (p=0.238 n=4+5)
ImagePackageCatalogers/go-module-binary-cataloger-2            14.0 ± 0%      14.0 ± 0%    ~     (all equal)

@wagoodman
nit updates
271c706 
Signed-off-by: Alex Goodman <alex.goodman@anchore.com>
kzantow
kzantow approved these changes Feb 10, 2022
View reviewed changes
Copy link
Contributor

@kzantow kzantow left a comment

Choose a reason for hiding this comment

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

LGTM -- just not quite sure about setting maven for all java types

syft/pkg/cataloger/java/package_url.go
}

pURL := packageurl.NewPackageURL(
packageurl.TypeMaven, // TODO: should we filter down by package types here?
Copy link
Contributor

Choose a reason for hiding this comment

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

is this always maven? ... I see the purl spec doesn't seem to have any other java types, which is probably ok

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm also mixed on this.

I see the purl spec doesn't seem to have any other java types

That's what made me pause on excluding these. If we don't have a pURL for these packages, then package type extraction will be impossible. So it seems right to at least include a purl that matches the java ecosystem. (I suspect in the future this will change)

syft/pkg/cataloger/java/archive_filename.go Show resolved Hide resolved
syft/pkg/npm_package_json_metadata.go Show resolved Hide resolved
@wagoodman wagoodman merged commit ca03243 into main Feb 10, 2022
@wagoodman wagoodman deleted the add-java-purls branch February 10, 2022 18:46
spiffcs added a commit that referenced this pull request Feb 17, 2022
@spiffcs
Merge branch '510-attach-sbomb-attestation' of https://github.com/anc…
6c2fe7d 
…hore/syft into 510-attach-sbomb-attestation

* '510-attach-sbomb-attestation' of https://github.com/anchore/syft:
  Upgrade install.sh to support installations for previous versions (#830)
  remove duplicate manifest lines (#828)
  bump stereoscope to include functional options (#823)
  update golang crypto library dependency (#815)
  deduplicate SPDX tag-value package IDs (#813)
  Add pURL generation for java packages + fix NPM pURL generation (#812)
spiffcs added a commit that referenced this pull request Feb 17, 2022
@spiffcs
Merge branch 'main' into 510-attach-sbomb-attestation
4f46f92 
* main:
  Upgrade install.sh to support installations for previous versions (#830)
  remove duplicate manifest lines (#828)
  bump stereoscope to include functional options (#823)
  update golang crypto library dependency (#815)
  deduplicate SPDX tag-value package IDs (#813)
  Add pURL generation for java packages + fix NPM pURL generation (#812)

Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
spiffcs pushed a commit that referenced this pull request Feb 18, 2022
@wagoodman @spiffcs
Add pURL generation for java packages + fix NPM pURL generation (#812)
f677d5b 
* enhance pURL generation for java packages

Signed-off-by: Alex Goodman <alex.goodman@anchore.com>

* optionally split out npm namespaces for pURL generation

Signed-off-by: Alex Goodman <alex.goodman@anchore.com>

* nit updates

Signed-off-by: Alex Goodman <alex.goodman@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
GijsCalis pushed a commit to GijsCalis/syft that referenced this pull request Feb 19, 2024
@wagoodman
Add pURL generation for java packages + fix NPM pURL generation (anch…
c5bfcfb 
…ore#812)

* enhance pURL generation for java packages

Signed-off-by: Alex Goodman <alex.goodman@anchore.com>

* optionally split out npm namespaces for pURL generation

Signed-off-by: Alex Goodman <alex.goodman@anchore.com>

* nit updates

Signed-off-by: Alex Goodman <alex.goodman@anchore.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kzantow kzantow kzantow approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@wagoodman @kzantow