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

fix: dedupliate cataloging binary artifacts #2839

Merged
merged 1 commit into from Feb 25, 2022

Conversation

wagoodman
Copy link
Contributor

This PR makes the following adjustments:

  • primarily restores the capability to reference binary artifacts when sbom[].artifact == binary
  • adds the ability to have templates within sbom[].env values
  • only prepends dist config value to paths that relative paths (takes no action on absolute paths)
  • decomposes templating code further for the SBOM pipe and surround with more testing

Why make this change? primarily to unlock more workflows when generating SBOMs.

Not all tooling can work with encapsulations of go binaries (zip/tar) and instead need access to the binary directly. Since goreleaser manages creating the binary it is possible to expose the original binary to SBOM tooling even though it may be an encapsulation that is attached to a release (e.g. a zip or tar.gz)

Take for example using cyclonedx-gomod app:

#.goreleaser.yaml

archives:
  - format: tar.gz

sboms:
  - documents:
      - "{{ .Binary }}_{{ .Version }}_{{ .Os }}_{{ .Arch }}.cdx.sbom"
    artifacts: binary
    cmd: cyclonedx-gomod
    env: 
      - GOOS={{ .Os }}
      - GOARCH={{ .Arch }}
    args: ["app",  "-json", "-output", "$document", "$PWD"]

With artifacts: binary the SBOM tool gets access to the unarchived binary even though archives[0].format = tar.gz and there is no archives[].format = binary set. Additionally this uses goreleaser-sourced variables to seed the correct go-specific environment variables that the tool keys in on.

Relevant links:

@pull-request-size pull-request-size bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jan 19, 2022
Copy link
Member

@caarlos0 caarlos0 left a comment

Choose a reason for hiding this comment

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

LGTM, seems like its missing a go mod tidy though

@caarlos0
Copy link
Member

Thanks for the PR, its looking good to me overall!

The only pointers I have are:

  • the deduplication thing maybe is something worth moving to artifacts.go
  • there is a test failing
  • it looks like its missing a go mod tidy

other than that, lgtm!

PS: sorry for the delay reviewing this, was focusing on other things.
Thanks!

@wagoodman wagoodman marked this pull request as ready for review February 14, 2022 15:39
@wagoodman
Copy link
Contributor Author

@caarlos0 no worries -- this one fell off my radar too.

I went ahead and moved the filter function to artifacts.go, fixed the failing tests, rebased + go mod tidied. Should be good to go 👍

@codecov
Copy link

codecov bot commented Feb 14, 2022

Codecov Report

Merging #2839 (dd2ede7) into main (395ee0a) will increase coverage by 0.18%.
The diff coverage is 85.71%.

❗ Current head dd2ede7 differs from pull request most recent head efe1c4f. Consider uploading reports for the commit efe1c4f to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2839      +/-   ##
==========================================
+ Coverage   84.36%   84.55%   +0.18%     
==========================================
  Files         111      111              
  Lines        8924     8953      +29     
==========================================
+ Hits         7529     7570      +41     
+ Misses       1122     1111      -11     
+ Partials      273      272       -1     
Impacted Files Coverage Δ
internal/pipe/sbom/sbom.go 85.52% <82.69%> (+0.52%) ⬆️
internal/artifact/artifact.go 98.02% <100.00%> (+0.18%) ⬆️
internal/tmpl/tmpl.go 97.63% <0.00%> (-0.21%) ⬇️
pkg/context/context.go 100.00% <0.00%> (ø)
internal/pipe/aur/aur.go 80.65% <0.00%> (+0.49%) ⬆️
internal/pipe/git/git.go 84.53% <0.00%> (+1.37%) ⬆️
cmd/build.go 97.93% <0.00%> (+6.20%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 395ee0a...efe1c4f. Read the comment docs.

@wagoodman wagoodman force-pushed the dedupliate-binary-artifacts branch 5 times, most recently from 965129e to 028679b Compare February 21, 2022 15:31
@wagoodman wagoodman force-pushed the dedupliate-binary-artifacts branch 2 times, most recently from 649a7b3 to 9b25e49 Compare February 24, 2022 20:31
@wagoodman
Copy link
Contributor Author

wagoodman commented Feb 24, 2022

@caarlos0 friendly nudge --let me know if there is anything else you'd like changed/checked on this PR.

go.mod Outdated Show resolved Hide resolved
Copy link
Member

@caarlos0 caarlos0 left a comment

Choose a reason for hiding this comment

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

lgtm except the accidental dep downgrade

thanks again for the pr, sorry for the delayed review

@wagoodman
Copy link
Contributor Author

no worries at all --updated the go.mod/sum.

@caarlos0 caarlos0 merged commit c845063 into goreleaser:main Feb 25, 2022
@caarlos0
Copy link
Member

Thanks!

@github-actions github-actions bot added this to the v1.6.0 milestone Feb 25, 2022
caarlos0 added a commit that referenced this pull request Feb 25, 2022
For some reason the set lib was trowing an int overflow  error on snapshot
when there are no binary artifacts to sign.

Refs  #2839
See https://github.com/goreleaser/goreleaser/runs/5334854323?check_suite_focus=true#step:21:50

Signed-off-by: Carlos A Becker <caarlos0@gmail.com>
caarlos0 added a commit that referenced this pull request Feb 25, 2022
For some reason the set lib was trowing an int overflow  error on snapshot
when there are no binary artifacts to sign.

Refs  #2839
See https://github.com/goreleaser/goreleaser/runs/5334854323?check_suite_focus=true#step:21:50

Signed-off-by: Carlos A Becker <caarlos0@gmail.com>
@wagoodman wagoodman deleted the dedupliate-binary-artifacts branch March 8, 2022 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants