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: migrate image building to buildx #724

Merged
merged 2 commits into from
Sep 11, 2024

Conversation

salasberryfin
Copy link
Contributor

@salasberryfin salasberryfin commented Sep 5, 2024

What this PR does / why we need it:

A few notes on the changes required for moving to buildx for image building and provenance attestation (thanks @alexander-demicev for the help):

  • The former image creation mechanism was based on creating different images for different architectures, which meant we had different Makefile actions for single or multi-platform image building. With buildx we move to a multi architecture image as we need to create the builder and its build capabilities (see https://docs.docker.com/reference/cli/docker/buildx/build/). The way buildx build command works, we create a single multi architecture image which is pushed to the specified registry. In Makefile this corresponds to action docker-build-and-push.
  • Images built with buildx and provenance attestation cannot be stored in the local registry, so we need to have an extra action that is platform-specific to which we can apply the --load flag that stores the resulting image locally. This applies to some CI workflows (e.g. lint). In Makefile this corresponds to action docker-build.
  • Provenance attestation will be added to the image during the build process, as buildx build accepts --attest type=provenance as input parameter (see https://docs.docker.com/build/metadata/attestations/slsa-provenance/). After the image is built and pushed, provenance attestation can be validated via:
docker buildx imagetools inspect <registry>/turtles:<tag> --format "{{ json .Provenance }}"

This means we can get rid of the former provenance attestation GitHub Action.

  • As we don't need to push a manifest to manage multiarch images, we can remove this step from release workflow. The image created with buildx includes different SHA256 values to identify each platform.

Which issue(s) this PR fixes:
Fixes #683

Special notes for your reviewer:

Checklist:

  • squashed commits into logical changes
  • includes documentation
  • adds unit tests
  • adds or updates e2e tests

@salasberryfin salasberryfin requested a review from a team as a code owner September 5, 2024 09:30
@salasberryfin salasberryfin force-pushed the migrate-to-buildx branch 5 times, most recently from 97480a0 to 9df2387 Compare September 5, 2024 13:19
Makefile Outdated Show resolved Hide resolved
@salasberryfin salasberryfin force-pushed the migrate-to-buildx branch 2 times, most recently from c00d640 to 60a4fa9 Compare September 5, 2024 13:59
@salasberryfin salasberryfin changed the title WIP: feat: migrate image building to buildx feat: migrate image building to buildx Sep 5, 2024
Copy link
Contributor

@Danil-Grigorev Danil-Grigorev left a comment

Choose a reason for hiding this comment

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

Some naming suggestions, which disambiguate the new usage.

Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Signed-off-by: Carlos Salas <carlos.salas@suse.com>
Signed-off-by: Carlos Salas <carlos.salas@suse.com>
Copy link
Contributor

@furkatgofurov7 furkatgofurov7 left a comment

Choose a reason for hiding this comment

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

Small nit, otherwise LGTM

@Danil-Grigorev
Copy link
Contributor

LGTM, a question though. Have you tested the published provenance with something like https://github.com/slsa-framework/slsa-verifier to check it still passes? It seems it is following a different format: docker/buildx#1741

@furkatgofurov7
Copy link
Contributor

furkatgofurov7 commented Sep 9, 2024

Based on offline discussion, @alexander-demicev @Danil-Grigorev should we mark this with "DON'T Merge" until we are ready for buildx migration and to avoid accidental merge?

@alexander-demicev
Copy link
Member

@salasberryfin some context for you, we figured out that with docker buildx we can only sign manifest that references each architecture-specific image. Currently, we sign every image for every architecture and this seems more complicated with buildx

@alexander-demicev alexander-demicev merged commit a35e256 into rancher:main Sep 11, 2024
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move to docker buildx for building Turtles
4 participants