8000 feat: Parameterized DOCKER_METADATA_PR_HEAD_SHA to allow for PR-HEAD tagging by pk-nb · Pull Request #40 · cloudposse/github-action-docker-build-push · GitHub
[go: up one dir, main page]

Skip to content

feat: Parameterized DOCKER_METADATA_PR_HEAD_SHA to allow for PR-HEAD tagging #40

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

Merged
merged 2 commits into from
Jul 18, 2023

Conversation

pk-nb
Copy link
Contributor
@pk-nb pk-nb commented Jul 12, 2023

what

  • By default, Github Actions uses merge commits during PR events for checkout.
  • Similarly, the docker-metadata action will use the PR merge commit for the docker SHA tags (referenced via the github.sha variable).
  • This adds a parameter that allows users to configure docker metadata-action to compute the SHA tags to use github.event.pull_request.head.sha instead.
    • Unfortunately the metadata-action does not compute based on a checkout action of the code, so we have to tell it to use the correct event SHA explicitly with this environment variable.
  • (Folks will also need to update the checkout action in their own workflows to use github.event.pull_request.head.sha)

See https://github.com/docker/metadata-action#environment-variables for documentation.

why

  • Some users (like us!) find the the default behavior of merge-commits challenging
    • Since the merge commit contains code from the target branch, it becomes very difficult to build an image without main branch code in it for hotfixes (with a manual release workflow that we use)
    • Merge conflicts cause builds to not be triggered, which is annoying for large / long-running branches
    • Some find it unintuitive that the SHAs don't match the PR head
    • It is harder to reference tags in the ECR registry since the merge SHAs are more opaque
  • Parameterized it as other folks may like the tradeoffs of merge commits for "closer to merge" results for testing and don't want to break other workflows.

By default, Github Action uses merge commits during PR events for the
(referenced via the `github.sha` variable). Some users find this
challenging, since the merge commit contains code from the target branch,
which is troublesome for long-running branches, merge-conflicts, and
a hotfix workflow for deploying one-off containers.

This allows users to configure docker metadata-action to compute the SHA
tags appropriately. Unfortunately the metadata-action does not compute
based on a checkout action of the code, so we have to tell it to use
the correct event SHA explicitly with this environment variable.

See https://github.com/docker/metadata-action#environment-variables
Co-authored-by: Dan Miller <miller0daniel@gmail.com>
@goruha
Copy link
Member
goruha commented Jul 18, 2023

@pk-nb
I'll merge this PR this week.

Till that moment you can use the workaround:

      - name: Build
        id: build
        uses: cloudposse/github-action-docker-build-push@main
        with:
          registry: registry.hub.docker.com
          organization: "${{ github.event.repository.owner.login }}"
          repository: "${{ github.event.repository.name }}"
          login: "${{ secrets.DOCKERHUB_USERNAME }}"
          password: "${{ secrets.DOCKERHUB_PASSWORD }}"
          platforms: linux/amd64,linux/arm64
        env:
          DOCKER_METADATA_PR_HEAD_SHA=true

@goruha goruha changed the base branch from main to docker-metadata-pr-head-sha July 18, 2023 18:36
@goruha goruha merged commit 91868fd into cloudposse:docker-metadata-pr-head-sha Jul 18, 2023
@goruha
Copy link
Member
goruha commented Jul 18, 2023

@pk-nb Thanks for your contribution.

https://github.com/cloudposse/github-action-docker-build-push/releases/tag/1.13.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0