Skip to content

Conversation

@NickLarsenNZ
Copy link
Member

@NickLarsenNZ NickLarsenNZ commented Jul 24, 2024

maltesander
maltesander previously approved these changes Jul 24, 2024
Copy link
Member

@maltesander maltesander left a comment

Choose a reason for hiding this comment

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

LGTM!

@sbernauer
Copy link
Member

@dervoeti will review as well

@dervoeti
Copy link
Member

dervoeti commented Aug 5, 2024

When I build an image locally with bake, push it and then run docker images --digests "$(< bake-target-tags)" --format '{{.Digest}}', it prints <none>. But that might be because my local build was in some way different from what we do in CI.
Also, I think .Digest is the local digest and the digest we are interested in is actually in .RepoDigests, which is an array (it should only have one element in our case though). As far as I remember, the "remote" digest in the repo you pushed to is different from the local one.

@NickLarsenNZ
Copy link
Member Author

I would have to look into the Digest meaning. When I ran it, it was the same as the remote, and I never saw arrays of digests, only the final.

Maybe for now, I can keep it the current way, and just print the output of this command (or even compare it), and we can check it over time to see if it matched.

@NickLarsenNZ NickLarsenNZ marked this pull request as draft August 5, 2024 09:10
@dervoeti
Copy link
Member

dervoeti commented Aug 5, 2024

Sounds good! Just want to make sure that we use the correct digest, otherwise signings / SBOMs will break for new builds. If it's the same digest then it's fine for me, printing the digests to compare them sounds like a good next step. I just remembered that I saw some differences when I tried it that way, but that was about a year ago, things might have changed.

@NickLarsenNZ
Copy link
Member Author

NickLarsenNZ commented Aug 5, 2024

On second thought @dervoeti, I'd like to keep my change. I took another look, and it appears to be fine (see explanation below).

We are also a while away from the next release, so doing this earlier gives us plenty of time to fix any issues that might appear.


Explanation

The digest used to come from the last line of this output (STDOUT):

Using default tag: latest
The push refers to repository [docker.stackable.tech/sandbox/nick/alpine]
78561cef0761: Preparing
78561cef0761: Pushed
latest: digest: sha256:eddacbc7e24bf8799a4ed3cdcfa50d4b88a323695ad80f317b6629883b2c2a78 size: 528

This matches what I get from:

❯ docker images --digests docker.stackable.tech/sandbox/nick/alpine --format '{{.Digest}}'                                         
sha256:eddacbc7e24bf8799a4ed3cdcfa50d4b88a323695ad80f317b6629883b2c2a78

This doesn't work at all: docker images --digests docker.stackable.tech/sandbox/nick/alpine --format '{{.RepoDigests}}'.

@dervoeti
Copy link
Member

dervoeti commented Aug 5, 2024

Yep okay, fine with me. Maybe we can run the build once with the new workflow for one of the images? For example: https://github.com/stackabletech/docker-images/actions/workflows/dev_opa.yaml?query=branch%3Afixes-for-the-jilted-generation

@NickLarsenNZ NickLarsenNZ marked this pull request as ready for review August 6, 2024 10:10
@NickLarsenNZ
Copy link
Member Author

@dervoeti we can test it on #797

Copy link
Member

@dervoeti dervoeti left a comment

Choose a reason for hiding this comment

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

LGTM. I'll check some signatures and SBOMs after the builds are done to verify it works as intended.

@NickLarsenNZ NickLarsenNZ added this pull request to the merge queue Aug 6, 2024
Merged via the queue into main with commit 7a4af85 Aug 6, 2024
@NickLarsenNZ NickLarsenNZ deleted the fixes-for-the-jilted-generation branch August 6, 2024 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants