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

feat: add digest to artifacts info of published docker images #3540

Merged
merged 4 commits into from Nov 12, 2022

Conversation

gal-legit
Copy link
Contributor

Extract the digest (sha256) of docker images from the docker push command for dockers published to the docker registry.
Outputting the digest is required to avoid a race condition when referencing the image, where the image tag is being modified before the reference is done.
See this blog post for more info.
This PR fixes #3496.

Note that the 'publish' pipe now must run before the 'metadata' pipe, so that the information extracted during the 'publish' pipe would appear in the metadata.
Previously, the published docker images metadata wasn't printed (because of the order). It made sense because the content of the published image was just a subset of the local one.
Now that it is printed to the metadata, it should have a different name to avoid confusion.
As I mentioned, it wasn't printed before - so there shouldn't be any backward-compatibility issues.


Local tests:

go test -v .
=== RUN   TestVersion
=== RUN   TestVersion/only_version
=== RUN   TestVersion/version_and_date
=== RUN   TestVersion/version,_date,_built_by
=== RUN   TestVersion/all_empty
=== RUN   TestVersion/complete
--- PASS: TestVersion (0.00s)
    --- PASS: TestVersion/only_version (0.00s)
    --- PASS: TestVersion/version_and_date (0.00s)
    --- PASS: TestVersion/version,_date,_built_by (0.00s)
    --- PASS: TestVersion/all_empty (0.00s)
    --- PASS: TestVersion/complete (0.00s)
PASS
ok      github.com/goreleaser/goreleaser        0.764s

Output example:

  {
    "name": "gallegit/hello-world:latest",
    "path": "gallegit/hello-world:latest",
    "goos": "linux",
    "goarch": "amd64",
    "internal_type": 10,
    "type": "Published Docker Image",
    "extra": {
      "digest": "sha256:c3f7dd196a046dc061236d3c6ae1e2946269e90da30b0a959240ca799750e632"
    }
  }

@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Nov 9, 2022
@codecov
Copy link

codecov bot commented Nov 12, 2022

Codecov Report

Merging #3540 (5965c86) into main (778f099) will increase coverage by 0.00%.
The diff coverage is 85.71%.

@@           Coverage Diff           @@
##             main    #3540   +/-   ##
=======================================
  Coverage   83.63%   83.64%           
=======================================
  Files         116      116           
  Lines        9656     9692   +36     
=======================================
+ Hits         8076     8107   +31     
- Misses       1280     1284    +4     
- Partials      300      301    +1     
Impacted Files Coverage Δ
internal/artifact/artifact.go 84.91% <0.00%> (-0.61%) ⬇️
internal/pipe/docker/api_docker.go 88.23% <70.00%> (-5.10%) ⬇️
internal/pipe/docker/api.go 100.00% <100.00%> (ø)
internal/pipe/docker/docker.go 91.32% <100.00%> (+0.13%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@caarlos0
Copy link
Member

caarlos0 commented Nov 12, 2022

hey @gal-legit ! this looks awesome! Thank you!

can you cherry-pick this commit? d389030

so we ensure this is not broken at some point :)

internal/pipe/docker/api.go Outdated Show resolved Hide resolved
Co-authored-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
caarlos0 and others added 2 commits November 12, 2022 11:54
Signed-off-by: Carlos A Becker <caarlos0@users.noreply.github.com>
@gal-legit
Copy link
Contributor Author

gal-legit commented Nov 12, 2022

@caarlos0 done. Thanks for the feedback! :)

@caarlos0 caarlos0 merged commit 5eb1e4a into goreleaser:main Nov 12, 2022
11 of 12 checks passed
@caarlos0
Copy link
Member

awesome, thank you @gal-legit !

@github-actions github-actions bot added this to the v1.13.0 milestone Nov 12, 2022
@caarlos0 caarlos0 added the bug Something isn't working label Nov 12, 2022
caarlos0 added a commit that referenced this pull request Nov 14, 2022
Following #3540, output artifacts' checksums to the artifact info.
This addition makes it easier to consume the checksums, especially when
running from e.g. GitHub Actions.

New tests:
1. Add a check for the checksum in the extra field. 
2. Add a test for every checksum algorithm (to see that it doesn't break
for any algo's output).
3. Add a case of a binary and an extra file (to see that the logic
doesn't break when there's a mix).

p.s.
While working on that, I noticed that the convention for extra fields is
actually to use UpperCamelCase rather than lower.
I was mistaken because I looked at the subfields of the "DockerConfig"
extra field.
I think it's a good idea to fix it quickly, before the next release
rolls and it becomes a compatibility issue.
I took the liberty to fix it here as an extra commit. Please let me know
if you want it to be in a separate PR.

---
Tests:
```
go test
  • refreshing checksums                             file=binary_bar_checksums.txt
  • refreshing checksums                             file=binary_bar_checksums.txt
  • refreshing checksums                             file=binary_bar_checksums.txt
PASS
ok  	github.com/goreleaser/goreleaser/internal/pipe/checksums	0.184s
```

Co-authored-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
caarlos0 added a commit that referenced this pull request Nov 15, 2022
this drives it home by using the actual images/manifest digests to sign
with cosign by default.

the default signing command is changing in this PR, but since `digest`
should be always there (if not, the pipeline will fail way earlier), it
should be fine.

refs #3496
refs #3540

Signed-off-by: Carlos A Becker <caarlos0@users.noreply.github.com>
caarlos0 added a commit that referenced this pull request Nov 15, 2022
this use the digests on the manifest creation.
Another PR will add it to signing too.

refs #3496
refs #3540

Signed-off-by: Carlos A Becker <caarlos0@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
2 participants