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

refactor: move formats from internal into syft module #1172

Merged
merged 6 commits into from
Sep 13, 2022

Conversation

cpendery
Copy link
Contributor

@cpendery cpendery commented Aug 23, 2022

Description

This pr enables the spdx and cyclonedx formatters to be called directly with those external libraries objects. Currently, if you build an sbom using these structs, you have to marshal it into json and then pass the reader to be un-marshaled and parsed into Syft's intermediate representation, causing wasted computations, especially when working with large SBOMs.
The often comes into play when calling Syft to produces packages for Grype's api methods.

Considerations:
Don't use interface{}: I agree, but fully using generics got a bit out of hand considering the Format struct is quite widely used. Happy to refactor as desired, like just using an interface of a union of two known types

@cpendery cpendery marked this pull request as ready for review August 23, 2022 02:52
@spiffcs
Copy link
Contributor

spiffcs commented Aug 31, 2022

I think I'm ok with interface here for now considering the overhead of the generic work.

cc @wagoodman since he usually has the final say when it comes to updating the interface types.

Code LGTM - I think just the consideration is worth the second pair of eyes.

@kzantow
Copy link
Contributor

kzantow commented Sep 1, 2022

@cpendery thanks for attending the community meeting to add context to this change -- do you have a specific format you're dealing with and would it just be easier to export the right conversion functions by moving these formatters to the top-level syft package and exporting the ToSyftModel functions?

@cpendery
Copy link
Contributor Author

cpendery commented Sep 1, 2022

@cpendery thanks for attending the community meeting to add context to this change -- do you have a specific format you're dealing with and would it just be easier to export the right conversion functions by moving these formatters to the top-level syft package and exporting the ToSyftModel functions?

I'm just working with the cyclonedx and spdx models. Just exposing the ToSyftModel function outside internal would be great and is all I actually need

@kzantow
Copy link
Contributor

kzantow commented Sep 8, 2022

@cpendery would you like to take a stab at the refactoring? If not, I could probably do this in the next day or two.

@cpendery
Copy link
Contributor Author

cpendery commented Sep 9, 2022

@cpendery would you like to take a stab at the refactoring? If not, I could probably do this in the next day or two.

I can refactor the pr and just pull out the formats package 👍

@cpendery cpendery force-pushed the feat-parser branch 2 times, most recently from 6a77416 to c634d2c Compare September 11, 2022 03:26
@cpendery cpendery requested a review from spiffcs September 12, 2022 16:07
@cpendery cpendery changed the title feat: enable formatters to take in external library structs refactor: move formats from internal into syft module & expose ToSyftModel methods Sep 12, 2022
@cpendery cpendery changed the title refactor: move formats from internal into syft module & expose ToSyftModel methods refactor: move formats from internal into syft module Sep 12, 2022
@spiffcs spiffcs merged commit 9097614 into anchore:main Sep 13, 2022
patrikbeno added a commit to patrikbeno/syft that referenced this pull request Sep 14, 2022
spiffcs added a commit to luhring/syft that referenced this pull request Sep 19, 2022
* main:
  bug: remove chance for panic; provide default attestation path (anchore#1214)
  refactor: update Makefile organization; update DEVELOPING.md instructions (anchore#1212)
  refactor: replace ioutil=>io; update linter (anchore#1211)
  Update bootstrap tools to latest versions. (anchore#1204)
  Add gosimports (anchore#1205)
  refactor: move formats from internal into syft module (anchore#1172)
  warn on errors from RPM DB parsing (anchore#1200)
  docs: improve Singularity image source docs (anchore#1190)

Signed-off-by: Christopher Phillips <[email protected]>
spiffcs pushed a commit that referenced this pull request Sep 19, 2022
aiwantaozi pushed a commit to aiwantaozi/syft that referenced this pull request Oct 20, 2022
spiffcs pushed a commit that referenced this pull request Oct 21, 2022
wagoodman pushed a commit to patrikbeno/syft that referenced this pull request Nov 14, 2022
wagoodman pushed a commit to patrikbeno/syft that referenced this pull request Nov 16, 2022
wagoodman added a commit that referenced this pull request Nov 16, 2022
* SBOM cataloger

Signed-off-by: Patrik Beno <[email protected]>

* sbom-cataloger: turn off by default

and add integration test

Signed-off-by: Patrik Beno <[email protected]>

* SBOM cataloger

Signed-off-by: Patrik Beno <[email protected]>

* SBOM cataloger (optimize)

Signed-off-by: Patrik Beno <[email protected]>

* SBOM cataloger (fix)

Signed-off-by: Patrik Beno <[email protected]>

* SBOM cataloger (fix imports #1172)

Signed-off-by: Patrik Beno <[email protected]>

* SBOM cataloger (fix: support group attribute in CDX SBOMs)

Signed-off-by: Patrik Beno <[email protected]>

* port to generic cataloger and add relationship to original file

Signed-off-by: Alex Goodman <[email protected]>

* generalize parser for all format globs

Signed-off-by: Alex Goodman <[email protected]>

Signed-off-by: Patrik Beno <[email protected]>
Signed-off-by: Alex Goodman <[email protected]>
Co-authored-by: Tom Fay <[email protected]>
Co-authored-by: Alex Goodman <[email protected]>
GijsCalis pushed a commit to GijsCalis/syft that referenced this pull request Feb 19, 2024
GijsCalis pushed a commit to GijsCalis/syft that referenced this pull request Feb 19, 2024
* SBOM cataloger

Signed-off-by: Patrik Beno <[email protected]>

* sbom-cataloger: turn off by default

and add integration test

Signed-off-by: Patrik Beno <[email protected]>

* SBOM cataloger

Signed-off-by: Patrik Beno <[email protected]>

* SBOM cataloger (optimize)

Signed-off-by: Patrik Beno <[email protected]>

* SBOM cataloger (fix)

Signed-off-by: Patrik Beno <[email protected]>

* SBOM cataloger (fix imports anchore#1172)

Signed-off-by: Patrik Beno <[email protected]>

* SBOM cataloger (fix: support group attribute in CDX SBOMs)

Signed-off-by: Patrik Beno <[email protected]>

* port to generic cataloger and add relationship to original file

Signed-off-by: Alex Goodman <[email protected]>

* generalize parser for all format globs

Signed-off-by: Alex Goodman <[email protected]>

Signed-off-by: Patrik Beno <[email protected]>
Signed-off-by: Alex Goodman <[email protected]>
Co-authored-by: Tom Fay <[email protected]>
Co-authored-by: Alex Goodman <[email protected]>
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