feat: add cyclonedx schema version selection #2123
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| package cyclonedxjson | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/CycloneDX/cyclonedx-go" | ||
| ) | ||
|
|
||
| func TestFormatVersions(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| expectedVersion string | ||
| }{ | ||
| { | ||
|
|
||
| "cyclonedx-json should default to v1.4", | ||
| cyclonedx.SpecVersion1_4.String(), | ||
| }, | ||
| } | ||
|
|
||
| for _, c := range tests { | ||
| c := c | ||
| t.Run(c.name, func(t *testing.T) { | ||
| sbomFormat := Format() | ||
| if sbomFormat.ID() != ID { | ||
| t.Errorf("expected ID %q, got %q", ID, sbomFormat.ID()) | ||
| } | ||
|
|
||
| if sbomFormat.Version() != c.expectedVersion { | ||
| t.Errorf("expected version %q, got %q", c.expectedVersion, sbomFormat.Version()) | ||
| } | ||
| }) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,14 +7,66 @@ import ( | |
| "github.com/anchore/syft/syft/sbom" | ||
| ) | ||
|
|
||
| const ID sbom.FormatID = "cyclonedx" | ||
|
There was a problem hiding this comment. I think this is a consequential change, can you expound on this a bit? (I think there is an alias for cyclonedx already, which I want to say defaults to json, but I haven't verified this) There was a problem hiding this comment. actually, this doesn't seem right: this is changing the ID for this format to something more generic, but the ID should be as specific as possible. SPDX does this by having a list of IDs, where the first is the ID and the remaining are unique aliases Then that list is used in the format object syft/syft/formats/spdxtagvalue/format.go Line 18 in 9de4129 This PR seems to be reversing this, where the aliases are more specific Since all of these instances are the same can you make a list and share that similar to SPDX? There was a problem hiding this comment. Ahh - I think this was just an unexpected change on my part thanks for the catch - I looked back at the diff and the original format had the alias of syft/syft/formats/cyclonedxxml/format.go Lines 12 to 20 in 9de4129 There was a problem hiding this comment. @wagoodman this should now be reverted to where there are no ID changes and aliases are consistent across the new formats |
||
|
|
||
| var Format = Format1_4 | ||
|
|
||
| func Format1_0() sbom.Format { | ||
| return sbom.NewFormat( | ||
| cyclonedx.SpecVersion1_0.String(), | ||
| encoderV1_0, | ||
| cyclonedxhelpers.GetDecoder(cyclonedx.BOMFileFormatXML), | ||
| cyclonedxhelpers.GetValidator(cyclonedx.BOMFileFormatXML), | ||
| ID, "cyclonedx-xml", "cyclone-xml", | ||
| ) | ||
| } | ||
|
|
||
| func Format1_1() sbom.Format { | ||
| return sbom.NewFormat( | ||
| cyclonedx.SpecVersion1_1.String(), | ||
| encoderV1_1, | ||
| cyclonedxhelpers.GetDecoder(cyclonedx.BOMFileFormatXML), | ||
| cyclonedxhelpers.GetValidator(cyclonedx.BOMFileFormatXML), | ||
| ID, "cyclonedx-xml", "cyclone-xml", | ||
| ) | ||
| } | ||
|
|
||
| func Format1_2() sbom.Format { | ||
| return sbom.NewFormat( | ||
| cyclonedx.SpecVersion1_2.String(), | ||
| encoderV1_2, | ||
| cyclonedxhelpers.GetDecoder(cyclonedx.BOMFileFormatXML), | ||
| cyclonedxhelpers.GetValidator(cyclonedx.BOMFileFormatXML), | ||
| ID, "cyclonedx-xml", "cyclone-xml", | ||
| ) | ||
| } | ||
|
|
||
| func Format1_3() sbom.Format { | ||
| return sbom.NewFormat( | ||
| cyclonedx.SpecVersion1_3.String(), | ||
| encoderV1_3, | ||
| cyclonedxhelpers.GetDecoder(cyclonedx.BOMFileFormatXML), | ||
| cyclonedxhelpers.GetValidator(cyclonedx.BOMFileFormatXML), | ||
| ID, "cyclonedx-xml", "cyclone-xml", | ||
| ) | ||
| } | ||
|
|
||
| func Format1_4() sbom.Format { | ||
| return sbom.NewFormat( | ||
| cyclonedx.SpecVersion1_4.String(), | ||
| encoderV1_4, | ||
| cyclonedxhelpers.GetDecoder(cyclonedx.BOMFileFormatXML), | ||
| cyclonedxhelpers.GetValidator(cyclonedx.BOMFileFormatXML), | ||
| ID, "cyclonedx-xml", "cyclone-xml", | ||
| ) | ||
| } | ||
|
|
||
| func Format1_5() sbom.Format { | ||
| return sbom.NewFormat( | ||
| cyclonedx.SpecVersion1_5.String(), | ||
| encoderV1_5, | ||
| cyclonedxhelpers.GetDecoder(cyclonedx.BOMFileFormatXML), | ||
| cyclonedxhelpers.GetValidator(cyclonedx.BOMFileFormatXML), | ||
| ID, "cyclonedx-xml", "cyclone-xml", | ||
| ) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,17 +26,27 @@ import ( | |
| func Formats() []sbom.Format { | ||
| return []sbom.Format{ | ||
| syftjson.Format(), | ||
| github.Format(), | ||
| table.Format(), | ||
| text.Format(), | ||
| template.Format(), | ||
| cyclonedxxml.Format1_0(), | ||
| cyclonedxxml.Format1_1(), | ||
| cyclonedxxml.Format1_2(), | ||
| cyclonedxxml.Format1_3(), | ||
| cyclonedxxml.Format1_4(), | ||
| cyclonedxxml.Format1_5(), | ||
| cyclonedxjson.Format1_0(), | ||
| cyclonedxjson.Format1_1(), | ||
| cyclonedxjson.Format1_2(), | ||
| cyclonedxjson.Format1_3(), | ||
| cyclonedxjson.Format1_4(), | ||
| cyclonedxjson.Format1_5(), | ||
| spdxtagvalue.Format2_1(), | ||
| spdxtagvalue.Format2_2(), | ||
| spdxtagvalue.Format2_3(), | ||
| spdxjson.Format2_2(), | ||
| spdxjson.Format2_3(), | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -55,7 +65,7 @@ func Identify(by []byte) sbom.Format { | |
|
|
||
| // ByName accepts a name@version string, such as: | ||
| // | ||
| // [email protected] or cyclonedx@1.5 | ||
| func ByName(name string) sbom.Format { | ||
| parts := strings.SplitN(name, "@", 2) | ||
| version := sbom.AnyVersion | ||
|
|
@@ -71,6 +81,13 @@ func ByNameAndVersion(name string, version string) sbom.Format { | |
| for _, f := range Formats() { | ||
| for _, n := range f.IDs() { | ||
| if cleanFormatName(string(n)) == name && versionMatches(f.Version(), version) { | ||
| if version == sbom.AnyVersion && strings.Contains(string(n), "cyclonedx") { | ||
|
There was a problem hiding this comment. This is looking for the string At the same time, I feel that this format object is about to be rethought anyway. I think there are strong signs to split up for encoding vs decoding use cases, which means trying to settle this detail in this PR wouldn't be worth it. I think it's worth at least adding a comment here about the limitations of this approach and a TODO to refactor later. There was a problem hiding this comment. also, how does ByName work for spdx for the latest version (I didn't read up on that yet)? There was a problem hiding this comment. ^ Yea that sounds good to me
I had tried just setting up a default version for each format rather than The comment for this line should be below it I can move above and add a TODO: For SPDX if you pass in There was a problem hiding this comment. I would probably like to add a There was a problem hiding this comment. I can follow this PR up with a stab at that. I really want to keep this PR small and focused on integrating the released cyclonedx library. Other changes on how we handle formats, their defaults or how the latest is selected can be their own change set. The reason this line had to be included was because we don't necessarily want to be on latest for cyclonedx at this moment. Rather than rewrite ByNameAndVersion into a totally new thing I thought a simple edge case with comment would be the correct change here There was a problem hiding this comment. This check should probably be |
||
| // if the version is not specified and the format is cyclonedx, then we want to return the most recent version up to 1.4 | ||
| // https://github.com/CycloneDX/cyclonedx-go/pull/90 | ||
| if f.Version() == "1.5" { | ||
| continue | ||
| } | ||
| } | ||
| if mostRecentFormat == nil || f.Version() > mostRecentFormat.Version() { | ||
| mostRecentFormat = f | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| package syftjson | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/anchore/syft/internal" | ||
| ) | ||
|
|
||
| func TestFormat(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| version string | ||
| }{ | ||
| { | ||
| name: "default version should use latest internal version", | ||
| version: "", | ||
| }, | ||
| } | ||
|
|
||
| for _, c := range tests { | ||
| c := c | ||
| t.Run(c.name, func(t *testing.T) { | ||
| sbomFormat := Format() | ||
| if sbomFormat.ID() != ID { | ||
| t.Errorf("expected ID %q, got %q", ID, sbomFormat.ID()) | ||
| } | ||
|
|
||
| if sbomFormat.Version() != internal.JSONSchemaVersion { | ||
| t.Errorf("expected version %q, got %q", c.version, sbomFormat.Version()) | ||
| } | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be nice to programmatically build these formats, something like this could live in the
common/cyclonedxhelperspackage:This way there only needs to be a single
encodeCycloneDX()function and adding new versions is just updating the list, which would update both XML and JSON variants. E.g. the XML variant would have:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a good suggestion, but I think we wanted to save this kind of work for when we go back and take a look at Formats before syft v1.0. The purpose of this PR was to just keep this as close to the same pattern as we have with SPDX. Getting creative here and trying to build these programmatically (different from the current SPDX pattern) would eventually get swallowed by that new work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agreed to the both of you -- it's a good suggestion, but it would be better to be consistent with the current pattern first, then refactor to "the next" pattern.