Additional updates for SBOM Support #97
Conversation
dmikusa
added
type:enhancement
| return []string{"application/vnd.cyclonedx+json", "application/spdx+json", "application/vnd.syft+json", "unknown"}[b] | ||
| } | ||
|
|
||
| func SBOMFormatFromString(from string) (SBOMFormat, error) { |
There was a problem hiding this comment.
Should we create a slice of all known sbom formats? That way we can just iterate over them.
This will also help with the validate sbom function where we can check the file suffix to match with the known formats. Lmk what you think.
There was a problem hiding this comment.
Sounds reasonable. Go question for you about this. How would you achieve this? I thought you couldn't have a const slice. I know I could define the strings as const and use them in the slice, maybe that's better? Is there a common pattern for doing something like this that you've seen? Thanks
There was a problem hiding this comment.
I moved the strings to consts & used the consts in the array. I did a quick google search and const arrays/slices are not a thing, as far as I can tell, so I'm not sure we can do much more. If I'm missing something, which is totally possible, do let me know.
I thought about a global var, but I don't think using a global var would be good. I think that could potentially allow someone to modify the list, which wouldn't be desirable, IMHO.
There was a problem hiding this comment.
https://github.com/dmarkham/enumer looks like a good way to manage auto generation of all these methods.
There was a problem hiding this comment.
That project looks interesting, but I don't see a way to do custom transformations (let me know if I'm missing something). If we were going from SomeConst and wanted the string representation to be SomeConst or some_const or some-const, it looks like we could use this. From what I see in the docs, there's no custom transformation to make SomeConst turn into some.json and vise versa, which is what we need.
It also looks like it's generating a global var which would have the same limitation, someone could potentially mess with the list.
The lists are fairly small and shouldn't grow a ton over time. It seems like we could keep them hand-written and not take on too much burden, IMHO (adding dependencies for this functionality also has its own risks).
I'll acknowledge that there is some room for error with using the arrays to convert from enum to string, and a new type needs to be added. If you forget to update the arrays, you'll get a runtime panic. I could change that to use switch statements instead. It's more code, but when a type is added if you forget to update the conversion methods, you will end up with an unknown value instead of a runtime panic so it's a little safer. Would that be preferred?
- Adds SBOMFormats (maps to `sbom-formats` in buildpack.toml) to the BuildpackInfo struct. This makes the information accessible to buildpacks. - Modifies build such that it only writes the old-style build and launch BOM information if the buildpack API is less than 0.7. If it's 0.7+, it should not write the old style format as that can conflict with the new SBOM format and cause the lifecycle to fail. Omits a warning message if this occurs. - Adds validation of the new SBOM files that are written by a buildpack. We check that the extension matches up with a valid MIME type that is listed in buildpack.toml's `sbom-formats` field. If it does not match up, then it fails. This should not generally happen with published buildpacks. This check can be helpful while authoring buildpacks, to ensure everything is correctly setup. Signed-off-by: Daniel Mikusa <dmikusa@vmware.com> Co-authored-by: Sambhav Kothari <sambhavs.email@gmail.com> Signed-off-by: Daniel Mikusa <dmikusa@vmware.com>
| @@ -380,3 +398,27 @@ func contains(candidates []string, s string) bool { | |||
|
|
|||
| return false | |||
| } | |||
|
|
|||
| func ValidateSBOMFormats(layersPath string, acceptedSBOMFormats []string) error { | |||
There was a problem hiding this comment.
| func ValidateSBOMFormats(layersPath string, acceptedSBOMFormats []string) error { | |
| func validateSBOMFormats(layersPath string, acceptedSBOMFormats []string) error { |
There was a problem hiding this comment.
Technically yes. It makes testing much harder though because it's being used in build, so you have to set up and run the entire build to test it. Given what it's doing, I wanted to make sure the test coverage was solid so I opted to leave it public.
There was a problem hiding this comment.
Could we make it internal then? I would prefer to reduce our support surface area in terms of the public api to things that are strictly necessary - it makes it easier to modify/update code without breaking dependents.
There was a problem hiding this comment.
That is appearing to be difficult as well. We're using libcnb.SBOMFormatFromString from inside ValidateSBOMFormats which creates a import cycle, internal -> libcnb -> internal...
If I move SBOMFormatFromString into internal as well, I then get more references to libcnb, so it's not clear about how to break that import cycle.
There was a problem hiding this comment.
@dmikusa-pivotal couldn't find a good way either. Ended up just making it private and testing it the long way in #98. If that looks good, let's merge it.
Not required to use the new SBOM support, but here are some additional helpful bits.
sbom-formatsin buildpack.toml) to the BuildpackInfo struct. This makes the information accessible to buildpacks.buildpack.toml'ssbom-formatsfield. If it does not match up, then it fails. This should not generally happen with published buildpacks. This check can be helpful while authoring buildpacks, to ensure everything is correctly set up.Signed-off-by: Daniel Mikusa dmikusa@vmware.com