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

Exporter when caching/exporting layers for a buildpack should also cache/export sbom as a separate layer #741

Closed
wants to merge 4 commits into from
Closed

Exporter when caching/exporting layers for a buildpack should also cache/export sbom as a separate layer #741

wants to merge 4 commits into from

Conversation

natalieparellano
Copy link
Member

@natalieparellano natalieparellano commented Oct 5, 2021 •
edited
Loading

Fixes #736 and #735

This is rough and still needs tests, adding this draft PR so that it could be discussed.

@natalieparellano
Exporter and cacher add a single layer per buildpack containing unmer…
11d2a73 
…ged layer boms

Signed-off-by: Natalie Arellano <narellano@vmware.com>
@natalieparellano
Test that launch bom layers are re-used on export
ee96dcf 
Signed-off-by: Natalie Arellano <narellano@vmware.com>
@natalieparellano natalieparellano changed the title Cacher when caching a layer should also cache sbom as a separate layer Exporter when caching/exporting layers for a buildpack should also cache/export sbom as a separate layer Oct 7, 2021
@natalieparellano
Copy link
Member Author

Some findings:

Because the exporter and cacher generally assume that launch and cache layers "behave the same", it would make the code a lot simpler if the Builder in #738 were to also copy sbom files for cache=true layers to /layers/config/sbom/cache/buildpack.id/layer.id/bom.<type>.json. Then the restorer in #733 could do the reverse. We might be able to avoid this extra copy just for cache layers, but at the expense of making the code much more complicated.

@samj1912 WDYT?

@natalieparellano
Copy link
Member Author

It might make sense to create a structured-sbom branch for the lifecycle and throw all of the PRs for https://github.com/buildpacks/lifecycle/issues?q=is%3Aissue+is%3Aopen+label%3Aepic%2Fsbom on there since it will be hard to test the feature end-to-end without all of them.

@natalieparellano
Include BOM sha when writing toml and json
de98d3f 
Use a pointer so that we can omit it completely when empty

Signed-off-by: Natalie Arellano <narellano@vmware.com>
@natalieparellano
Update comment
ce6e24e 
Signed-off-by: Natalie Arellano <narellano@vmware.com>
@natalieparellano natalieparellano mentioned this pull request Oct 8, 2021
Closed
mboldt
mboldt reviewed Oct 13, 2021
View reviewed changes
cache.go
Comment on lines +61 to +64
bpMD.BOM.SHA, err = e.addOrReuseCacheLayer(cacheStore, sbomLayer, origBOMSHA)
if err != nil {
return err
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this warn and move on instead of returning an error, like the lines 44-47 above?

@aemengo aemengo mentioned this pull request Oct 21, 2021
Merged
@natalieparellano
Copy link
Member Author

Closing as this is covered in #749

@natalieparellano natalieparellano deleted the cache-sbom branch March 30, 2022 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mboldt mboldt mboldt left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[RFC #0095] Exporter should cache bom files for cached layers
2 participants
@natalieparellano @mboldt