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 pomxml extractor which also performs transitive dependency resolution #1331

Merged
merged 5 commits into from
Oct 22, 2024

Conversation

another-rex
Copy link
Collaborator

@another-rex another-rex commented Oct 18, 2024

Part of #1330

No functional change is made compared to the version in internal/manifest, just switched to use the osv-scalibr interface.

Extractors moved to lockfilescalibr as a temporary staging ground before moving to osv-scalibr.

@another-rex another-rex requested a review from cuixq October 18, 2024 03:43
@codecov-commenter
Copy link

codecov-commenter commented Oct 18, 2024

Codecov Report

Attention: Patch coverage is 69.74790% with 36 lines in your changes missing coverage. Please review.

Project coverage is 68.42%. Comparing base (e054385) to head (c92bc3e).

Files with missing lines Patch % Lines
...ckfilescalibr/language/java/pomxmlnet/extractor.go 69.74% 31 Missing and 5 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1331      +/-   ##
==========================================
- Coverage   68.43%   68.42%   -0.01%     
==========================================
  Files         183      184       +1     
  Lines       17606    17725     +119     
==========================================
+ Hits        12049    12129      +80     
- Misses       4895     4928      +33     
- Partials      662      668       +6     

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

@cuixq
Copy link
Contributor

cuixq commented Oct 18, 2024

There is a PR about Maven registry support #1286, I would recommend to merge this PR afterwards (refactoring is needed I think).

@another-rex
Copy link
Collaborator Author

@cuixq This should be ready to review now!

@another-rex another-rex changed the base branch from main to v2 October 22, 2024 23:25
@another-rex another-rex merged commit e67449e into google:v2 Oct 22, 2024
15 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] <support@github.com>
Co-authored-by: Gareth Jones <Jones258@Gmail.com>
Co-authored-by: Xueqin Cui <72771658+cuixq@users.noreply.github.com>
Co-authored-by: Michael Kedar <michaelkedar@google.com>
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
…ncy resolution (#1331)

Part of #1330

No functional change is made compared to the version in
`internal/manifest`, just switched to use the osv-scalibr interface.

Extractors moved to lockfilescalibr as a temporary staging ground before
moving to osv-scalibr.
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] <support@github.com>
Co-authored-by: Gareth Jones <Jones258@Gmail.com>
Co-authored-by: Xueqin Cui <72771658+cuixq@users.noreply.github.com>
Co-authored-by: Michael Kedar <michaelkedar@google.com>
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.

3 participants