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.

Sign up for GitHub

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

Jump to bottom

Additional updates for SBOM Support #97

Merged
merged 1 commit into from Nov 24, 2021
Merged

Additional updates for SBOM Support #97

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
42 changes: 42 additions & 0 deletions build.go
View file
Open in desktop
Expand Up @@ -306,6 +306,11 @@ func Build(builder Builder, options ...Option) {
}
}

if err := ValidateSBOMFormats(ctx.Layers.Path, ctx.Buildpack.Info.SBOMFormats); err != nil {
config.exitHandler.Error(fmt.Errorf("unable to validate SBOM\n%w", err))
return
}

// Deprecated: as of Buildpack API 0.7, to be removed in a future version
var launchBOM, buildBOM []BOMEntry
if result.BOM != nil {
Expand Down Expand Up @@ -338,6 +343,12 @@ func Build(builder Builder, options ...Option) {
}
}

// even if there is data, do not write a BOM if we have buildpack API 0.7, that will cause a lifecycle error
if API == "0.7" {
logger.Info("Warning: this buildpack is including both old and new format SBOM information, which is an invalid state. To prevent the lifecycle from failing, libcnb is discarding the old SBOM information.")
launch.BOM = nil
dmikusa marked this conversation as resolved.
Show resolved Hide resolved
}

if err = config.tomlWriter.Write(file, launch); err != nil {
config.exitHandler.Error(fmt.Errorf("unable to write application metadata %s\n%w", file, err))
return
Expand All @@ -352,6 +363,13 @@ func Build(builder Builder, options ...Option) {
if !build.isEmpty() {
file = filepath.Join(ctx.Layers.Path, "build.toml")
logger.Debugf("Writing build metadata: %s <= %+v", file, build)

// even if there is data, do not write a BOM if we have buildpack API 0.7, that will cause a lifecycle error
if API == "0.7" {
logger.Info("Warning: this buildpack is including both old and new format SBOM information, which is an invalid state. To prevent the lifecycle from failing, libcnb is discarding the old SBOM information.")
build.BOM = nil
}

if err = config.tomlWriter.Write(file, build); err != nil {
config.exitHandler.Error(fmt.Errorf("unable to write build metadata %s\n%w", file, err))
return
Expand Down Expand Up @@ -380,3 +398,27 @@ func contains(candidates []string, s string) bool {

return false
}

func ValidateSBOMFormats(layersPath string, acceptedSBOMFormats []string) error {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
func ValidateSBOMFormats(layersPath string, acceptedSBOMFormats []string) error {
func validateSBOMFormats(layersPath string, acceptedSBOMFormats []string) error {

Copy link
Member

Choose a reason for hiding this comment

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

Can we make this private?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

@samj1912 samj1912 Nov 22, 2021 •
edited

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

@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.

sbomFiles, err := filepath.Glob(filepath.Join(layersPath, "*.sbom.*"))
if err != nil {
return fmt.Errorf("unable find SBOM files\n%w", err)
}

for _, sbomFile := range sbomFiles {
parts := strings.Split(filepath.Base(sbomFile), ".")
if len(parts) <= 2 {
return fmt.Errorf("invalid format %s", filepath.Base(sbomFile))
}
sbomFormat, err := SBOMFormatFromString(strings.Join(parts[len(parts)-2:], "."))
if err != nil {
return fmt.Errorf("unable to parse SBOM %s\n%w", sbomFormat, err)
}

if !contains(acceptedSBOMFormats, sbomFormat.MediaType()) {
return fmt.Errorf("unable to find actual SBOM Type %s in list of supported SBOM types %s", sbomFormat.MediaType(), acceptedSBOMFormats)
}
}

return nil
}
26 changes: 26 additions & 0 deletions build_test.go
View file
Open in desktop
Expand Up @@ -625,4 +625,30 @@ version = "1.1.1"
},
}))
})

context("Validates SBOM entries", func() {
it("has no SBOM files", func() {
Expect(libcnb.ValidateSBOMFormats(layersPath, []string{})).To(BeNil())
})

it("has no accepted formats", func() {
Expect(ioutil.WriteFile(filepath.Join(layersPath, "launch.sbom.spdx.json"), []byte{}, 0600)).To(Succeed())
Expect(libcnb.ValidateSBOMFormats(layersPath, []string{})).To(MatchError("unable to find actual SBOM Type application/spdx+json in list of supported SBOM types []"))
})

it("has no matching formats", func() {
Expect(ioutil.WriteFile(filepath.Join(layersPath, "launch.sbom.spdx.json"), []byte{}, 0600)).To(Succeed())
Expect(libcnb.ValidateSBOMFormats(layersPath, []string{"application/vnd.cyclonedx+json"})).To(MatchError("unable to find actual SBOM Type application/spdx+json in list of supported SBOM types [application/vnd.cyclonedx+json]"))
})

it("has a matching format", func() {
Expect(ioutil.WriteFile(filepath.Join(layersPath, "launch.sbom.spdx.json"), []byte{}, 0600)).To(Succeed())
Expect(libcnb.ValidateSBOMFormats(layersPath, []string{"application/vnd.cyclonedx+json", "application/spdx+json"})).To(BeNil())
})

it("has an invalid format", func() {
Expect(ioutil.WriteFile(filepath.Join(layersPath, "launch.sbom.junk.json"), []byte{}, 0600)).To(Succeed())
Expect(libcnb.ValidateSBOMFormats(layersPath, []string{})).To(MatchError("unable to parse SBOM unknown\nunable to translate from junk.json to SBOMFormat"))
})
})
}
3 changes: 3 additions & 0 deletions buildpack.go
View file
Open in desktop
Expand Up @@ -41,6 +41,9 @@ type BuildpackInfo struct {

// Licenses a list of buildpack licenses.
Licenses []License `toml:"licenses"`

// SBOM is the list of supported SBOM media types
SBOMFormats []string `toml:"sbom-formats"`
}

// License contains information about a Software License
Expand Down
38 changes: 37 additions & 1 deletion layer.go
View file
Open in desktop
Expand Up @@ -26,6 +26,16 @@ import (
"github.com/buildpacks/libcnb/internal"
)

const (
BOMFormatCycloneDXExtension = "cdx.json"
BOMFormatSPDXExtension = "spdx.json"
BOMFormatSyftExtension = "syft.json"
BOMMediaTypeCycloneDX = "application/vnd.cyclonedx+json"
BOMMediaTypeSPDX = "application/spdx+json"
BOMMediaTypeSyft = "application/vnd.syft+json"
BOMUnknown = "unknown"
)

// Exec represents the exec.d layer location
type Exec struct {
// Path is the path to the exec.d directory.
Expand Down Expand Up @@ -74,10 +84,36 @@ const (
CycloneDXJSON SBOMFormat = iota
SPDXJSON
SyftJSON
UnknownFormat
)

func (b SBOMFormat) String() string {
return []string{"cdx.json", "spdx.json", "syft.json"}[b]
return []string{
BOMFormatCycloneDXExtension,
BOMFormatSPDXExtension,
BOMFormatSyftExtension,
BOMUnknown}[b]
}

func (b SBOMFormat) MediaType() string {
return []string{
BOMMediaTypeCycloneDX,
BOMMediaTypeSPDX,
BOMMediaTypeSyft,
BOMUnknown}[b]
}

func SBOMFormatFromString(from string) (SBOMFormat, error) {
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

@dmikusa dmikusa Nov 20, 2021 •
edited

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

https://github.com/dmarkham/enumer looks like a good way to manage auto generation of all these methods.

Copy link
Contributor Author

@dmikusa dmikusa Nov 21, 2021 •
edited

Choose a reason for hiding this comment

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

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?

switch from {
case CycloneDXJSON.String():
return CycloneDXJSON, nil
case SPDXJSON.String():
return SPDXJSON, nil
case SyftJSON.String():
return SyftJSON, nil
}

return UnknownFormat, fmt.Errorf("unable to translate from %s to SBOMFormat", from)
}

// Layer represents a layer managed by the buildpack.
Expand Down
20 changes: 19 additions & 1 deletion layer_test.go
View file
Open in desktop
Expand Up @@ -110,7 +110,7 @@ func testLayer(t *testing.T, context spec.G, it spec.S) {
Expect(l.Profile).To(Equal(libcnb.Profile{}))
})

it("generates BOM paths", func() {
it("generates SBOM paths", func() {
l, err := layers.Layer("test-name")
Expect(err).NotTo(HaveOccurred())

Expand All @@ -122,6 +122,24 @@ func testLayer(t *testing.T, context spec.G, it spec.S) {
Expect(l.SBOMPath(libcnb.SyftJSON)).To(Equal(filepath.Join(path, "test-name.sbom.syft.json")))
})

it("maps from string to SBOM Format", func() {
fmt, err := libcnb.SBOMFormatFromString("cdx.json")
Expect(err).ToNot(HaveOccurred())
Expect(fmt).To(Equal(libcnb.CycloneDXJSON))

fmt, err = libcnb.SBOMFormatFromString("spdx.json")
Expect(err).ToNot(HaveOccurred())
Expect(fmt).To(Equal(libcnb.SPDXJSON))

fmt, err = libcnb.SBOMFormatFromString("syft.json")
Expect(err).ToNot(HaveOccurred())
Expect(fmt).To(Equal(libcnb.SyftJSON))

fmt, err = libcnb.SBOMFormatFromString("foobar.json")
Expect(err).To(MatchError("unable to translate from foobar.json to SBOMFormat"))
Expect(fmt).To(Equal(libcnb.UnknownFormat))
})

it("reads existing 0.5 metadata", func() {
Expect(ioutil.WriteFile(
filepath.Join(path, "test-name.toml"),
Expand Down