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

Add CycloneDX decoder #811

Merged
merged 16 commits into from
Feb 18, 2022
Merged

Conversation

kzantow
Copy link
Contributor

@kzantow kzantow commented Feb 10, 2022

This PR adds support for decoding CycloneDX XML and JSON formats 🎉

This is a prerequisite for anchore/grype#481

@kzantow kzantow added the WIP work in progress / do not merge label Feb 10, 2022
Signed-off-by: Keith Zantow <[email protected]>
@kzantow kzantow force-pushed the add-cyclonedx-decoder branch from 275e7c4 to 22d6d43 Compare February 10, 2022 18:07
Signed-off-by: Keith Zantow <[email protected]>
Signed-off-by: Keith Zantow <[email protected]>
Signed-off-by: Keith Zantow <[email protected]>
Signed-off-by: Keith Zantow <[email protected]>
Signed-off-by: Keith Zantow <[email protected]>
Signed-off-by: Keith Zantow <[email protected]>
Signed-off-by: Keith Zantow <[email protected]>
Signed-off-by: Keith Zantow <[email protected]>
Signed-off-by: Keith Zantow <[email protected]>
Signed-off-by: Keith Zantow <[email protected]>
@kzantow kzantow removed the WIP work in progress / do not merge label Feb 12, 2022
@kzantow kzantow requested a review from a team February 14, 2022 14:21
Copy link
Contributor

@wagoodman wagoodman left a comment

Choose a reason for hiding this comment

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

The original intent of the formats package was to have the following structure:

├── common
│   ├── spdxhelpers/     # common spdx functions that to not reference model packages (only uses strings, ints, etc. ... primitives) 
│   └── ...
└── spdx22json
    ├── decoder.go
    ├── encoder.go
    ├── format.go         # wire up all functions to return a format object
    ├── model/
    │   └──    ...         # the "format model" contains only struct definitions, no behavior
    ├── to_format_model.go    # behavior to translate from syft sbom.SBOM to the format model  
    ├── to_syft_model.go      # behavior to translate from the format model to syft sbom.SBOM 
    └── validator.go

This pattern has quickly eroded with the introduction of 3rd party libs that are shared across multiple formats. This isn't a bad thing since we're getting a lot of functionality and models from these libs.

From the SPDX and CycloneDX work it seems like we're moving towards (using spdx randomly as an example):

├── common
│   └── spdxhelpers/     # do all of the model translation work for all spdx formats
│         ├── (all of the helpers)
│         ├── to_syft_model.go
│         └── to_format_model.go 
└── spdx22json
    ├── decoder.go    
    ├── encoder.go     
    ├── format.go         # wire up all functions to return a format object
    └── validator.go

(I realize this PR isn't the first to start this, but I still wanted to point out the direction change.)

This PR is drifting in a slightly different direction (again, spdx choice here is arbitrary):

├── common
│   └── spdxhelpers/     # do all of the model translation work for all spdx formats
│         ├── (all of the helpers)
│         ├── decoder.go    
│         ├── encoder.go     
│         ├── to_syft_model.go
│         ├── to_format_model.go 
│         └── validator.go
└── spdx22json
    └── format.go         # wire up all functions to return a format object

Here the spdx helpers are also making format choices by putting the encoder/decoders into the helpers package.

My ask: we should be consistent across formats when possible regarding these kinds of conventions. I think we should keep the encoder/decoders/validators in the respective format package and not in the helper packages.

Signed-off-by: Keith Zantow <[email protected]>
@wagoodman
Copy link
Contributor

From an earlier conversation about #811 (review) , let chat about this further in a followup PR (not blocking for this one).

Copy link
Contributor

@wagoodman wagoodman left a comment

Choose a reason for hiding this comment

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

🎉

@kzantow kzantow merged commit 20c1d14 into anchore:main Feb 18, 2022
@kzantow kzantow deleted the add-cyclonedx-decoder branch February 18, 2022 16:19
GijsCalis pushed a commit to GijsCalis/syft that referenced this pull request Feb 19, 2024
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