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

feat: migrate node-modules to a osv-scalibr extractor #1345

Merged
merged 3 commits into from
Oct 27, 2024

Conversation

another-rex
Copy link
Collaborator

Ideally, node package extracts can be done with packagejson extractor directly (As every dir inside a node_modules directory has a package.json file), but that changes the image scanning snapshots too much to be part of this initial PR, and introduces new OS findings that are difficult to filter out.

This keeps the existing solution of extracting directly with node_modules by implementing an extractor that wraps the packagejsonlock extractor to minimise snapshot changes.

@codecov-commenter
Copy link

codecov-commenter commented Oct 25, 2024

Codecov Report

Attention: Patch coverage is 0% with 14 lines in your changes missing coverage. Please review.

Project coverage is 68.44%. Comparing base (4069367) to head (59fd1b6).
Report is 2 commits behind head on v2.

Files with missing lines Patch % Lines
...alibr/language/javascript/nodemodules/extractor.go 0.00% 14 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##               v2    #1345      +/-   ##
==========================================
+ Coverage   68.36%   68.44%   +0.08%     
==========================================
  Files         185      187       +2     
  Lines       17817    17872      +55     
==========================================
+ Hits        12180    12233      +53     
- Misses       4961     4966       +5     
+ Partials      676      673       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@another-rex another-rex merged commit 1cd58a7 into google:v2 Oct 27, 2024
13 checks passed
another-rex added a commit that referenced this pull request Nov 1, 2024
This PR contains all the code required to move to osv-scalibr while
making the existing code compile and pass all tests (container tests not
passing because of a bug in the scalibr alpine extractor).

Changes not mentioned in the following list will be split off in
separate PRs which should land before this PR.

Those are:
- [x] #1337 
- [x] #1331 
- [x] #1338 
- [x] #1341
- [x] #1345


Changes in this PR:
- Fixture changes:
- Scalibr Python requirements.txt extractor currently doesn't support
packages without versions, so added some version strings to the test
files
- Image package required quite a bit of reworking to successfully
update.
- Add the ability to iterate through a directory via the pathtree
library
  - Support scalibr FS interface for Layers
- Add conversion code to convert inventories from osv-scalibr back to
v1's lockfile and Inventory
- This is done to minimize snapshot changes. Followup PRs should remove
this conversion
- Add `internal/lockfilescalibr` package:
  - `errors.go` adds common extraction errors we want to translate.
- `translation.go` adds helper functions and translation logic between
osv-scanner v1 extractor names, and osv-scalibr extractor names.



Changes in followup PRs:
- Delete lockfiles package and migrate everything to use osv-scalibr
extractors
- Remove conversion code in image

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Gareth Jones <[email protected]>
Co-authored-by: Xueqin Cui <[email protected]>
Co-authored-by: Michael Kedar <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
another-rex added a commit that referenced this pull request Nov 29, 2024
Ideally, node package extracts can be done with packagejson extractor
directly (As every dir inside a node_modules directory has a
package.json file), but that changes the image scanning snapshots too
much to be part of this initial PR, and introduces new OS findings that
are difficult to filter out.

This keeps the existing solution of extracting directly with
node_modules by implementing an extractor that wraps the
`packagejsonlock` extractor to minimise snapshot changes.
another-rex added a commit that referenced this pull request Nov 29, 2024
This PR contains all the code required to move to osv-scalibr while
making the existing code compile and pass all tests (container tests not
passing because of a bug in the scalibr alpine extractor).

Changes not mentioned in the following list will be split off in
separate PRs which should land before this PR.

Those are:
- [x] #1337
- [x] #1331
- [x] #1338
- [x] #1341
- [x] #1345

Changes in this PR:
- Fixture changes:
- Scalibr Python requirements.txt extractor currently doesn't support
packages without versions, so added some version strings to the test
files
- Image package required quite a bit of reworking to successfully
update.
- Add the ability to iterate through a directory via the pathtree
library
  - Support scalibr FS interface for Layers
- Add conversion code to convert inventories from osv-scalibr back to
v1's lockfile and Inventory
- This is done to minimize snapshot changes. Followup PRs should remove
this conversion
- Add `internal/lockfilescalibr` package:
  - `errors.go` adds common extraction errors we want to translate.
- `translation.go` adds helper functions and translation logic between
osv-scanner v1 extractor names, and osv-scalibr extractor names.

Changes in followup PRs:
- Delete lockfiles package and migrate everything to use osv-scalibr
extractors
- Remove conversion code in image

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Gareth Jones <[email protected]>
Co-authored-by: Xueqin Cui <[email protected]>
Co-authored-by: Michael Kedar <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
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.

4 participants