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

Use correct commit sha for PRs? #206

Closed
evandam opened this issue May 11, 2022 · 11 comments · Fixed by #234
Closed

Use correct commit sha for PRs? #206

evandam opened this issue May 11, 2022 · 11 comments · Fixed by #234

Comments

@evandam
Copy link

evandam commented May 11, 2022

Behaviour

Hey folks, similar to #191 a bit, when running a workflow on a PR event, the {{ sha }} refers to the temporary merge commit to the target branch, not the commit that was pushed to trigger the event.

I realize this is a pretty common situation with GitHub actions, but it ends up with Docker tags/labels that don't map back to any real commits. It would be really useful if this action automatically used github.event.pull_request.head.sha for PR events. I think it would have to be set for the tags {{ sha }} value and the auto-generated label org.opencontainers.image.revision.

I'm aware we can work around the tags situation by using type=raw and passing a value based on github.event.pull_request.head.sha, but it feels a bit error-prone.

I'm unaware of a workaround for overriding the org.opencontainers.image.revision label, though.

Steps to reproduce this issue

See example workflow and run on a PR

Expected behaviour

Tags with the correct sha in a PR (same with org.opencontainers.image.revision label)

Actual behaviour

Tags are created with the merge commit of the PR to the target branch (same with org.opencontainers.image.revision)

name: AWS

on:
  push:
    branches: ["main"]
    tags: ["v*"]
  pull_request:
    branches: ["*"]

jobs:
  docker:
    runs-on: ubuntu-latest
    steps:
      - name: Checkout
        uses: actions/checkout@v3
      - name: Docker meta
        id: meta
        uses: docker/metadata-action@v4
        with:
          images: "my-repo/my-image"
          tags: |
            type=ref,event=branch,suffix=-{{ sha }}
            type=ref,event=pr,suffix=-{{ sha }}
@crazy-max
Copy link
Member

the {{ sha }} refers to the temporary merge commit to the target branch, not the commit that was pushed to trigger the event.

No, {{sha}} refers to the commit SHA that triggered the workflow run. It's the same as GITHUB_SHA or ${{ github.sha }} in github context.

You can check this behavior by adding the following step in your workflow to trace the context:

      - name: Dump context
        if: always()
        uses: crazy-max/ghaction-dump-context@v1

In your case you might want ${{ github.event.pull_request.head.sha }} or ${{ github.event.pull_request.base.sha }}:

jobs:
  docker:
    runs-on: ubuntu-latest
    steps:
      - name: Checkout
        uses: actions/checkout@v3
      - name: Docker meta
        id: meta
        uses: docker/metadata-action@v4
        with:
          images: "my-repo/my-image"
          tags: |
            type=ref,event=branch,suffix=-{{ sha }}
            type=ref,event=pr,suffix=-${{ github.event.pull_request.head.sha }}

when running a workflow on a PR event

Also on PR event your don't have access to secrets so you don't push your image anyway? Maybe it's a private repo and you're using OIDC?

@evandam
Copy link
Author

evandam commented May 11, 2022

Sure but the point remains the same, it's a commit that doesn't exist in the repo 😅

Right I mentioned the workaround like you mentioned works for tags, but unaware of anything for labels and it's just a tricky thing to have to account for whenever we want to reference the git sha.

I was just hoping it would be possible to tweak https://github.com/docker/metadata-action/blob/master/src/main.ts#L19 to be something like context.eventName == 'pull_request' ? context.event.pull_request.head.sha : context.sha in one place and then we can safely use it wherever needed.

@crazy-max
Copy link
Member

crazy-max commented May 11, 2022

but unaware of anything for labels and it's just a tricky thing to have to account for whenever we want to reference the git sha.

Sorry I missed your case with labels. So yes you can override them with:

jobs:
  docker:
    runs-on: ubuntu-latest
    steps:
      - name: Checkout
        uses: actions/checkout@v3
      - name: Docker meta
        id: meta
        uses: docker/metadata-action@v4
        with:
          images: "my-repo/my-image"
          tags: |
            type=ref,event=branch,suffix=-{{ sha }}
            type=ref,event=pr,suffix=-${{ github.event.pull_request.head.sha }}
          labels:
            org.opencontainers.image.revision=${{ github.event.pull_request.head.sha }}

See https://github.com/docker/metadata-action#overwrite-labels

@evandam
Copy link
Author

evandam commented May 11, 2022

Oh gotcha thanks for the link. Not sure if this is an issue though? Looks like it adds a label rather than replacing

Docker labels
  org.opencontainers.image.title=my_repo
  org.opencontainers.image.description=
  org.opencontainers.image.url=https://github.com/my_org/my_repo
  org.opencontainers.image.source=https://github.com/my_org/my_repo
  org.opencontainers.image.version=pr-123
  org.opencontainers.image.created=2022-05-11T19:15:32.381Z
  org.opencontainers.image.revision=b0609399d9ffb14f42dd7097df4a12fc81a1234f
  org.opencontainers.image.licenses=
  org.opencontainers.image.revision=d48678cbb01422b60f8fe3cc2e1c1438ab6789e

@crazy-max
Copy link
Member

#125 (comment)

@evandam
Copy link
Author

evandam commented May 11, 2022

Got it, that works thank you!

Any thoughts on being able to set the commit sha for PRs specifically? I think it would be a great quality of life improvement.

@scalp42
Copy link

scalp42 commented May 12, 2022

what @evandam said, it'd be great if we could fetch the "real" SHA of the PR vs some temporary SHA that doesn't really match to anything 😅

@LMS007
Copy link

LMS007 commented May 23, 2022

I also have this issue. I think its because the workflow in my case is triggered from the merged PR and then an auto commit and tag is performed after that. So the SHA then points to the second to the last commit which is what triggered the workflow, but the SHA does not match the tag.

@abhijit838
Copy link

abhijit838 commented Sep 29, 2022

Agree @scalp42, It is not event respecting the checkout commit.

I have used ref: ${{ github.event.pull_request.head.sha }} to checkout the PR head and it works but the meta is still taking future merge commit id.

@dylanANZ
Copy link

Should this issue be re-opened since the change was reverted?

@scalp42
Copy link

scalp42 commented Nov 29, 2022

@dylanANZ they reverted the default behavior (it was a breaking change associated with a minor bump which was not the best move) but the feature is available (see #239):

User can still set associated head sha on PR by setting the env var DOCKER_METADATA_PR_HEAD_SHA=true.

l3d00m added a commit to reims2/actions that referenced this issue Jul 20, 2023
l3d00m added a commit to reims2/reims2 that referenced this issue Aug 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants