Skip to content

Conversation

@crazy-max
Copy link
Member

fixes #206

Signed-off-by: CrazyMax [email protected]

@crazy-max crazy-max marked this pull request as ready for review October 7, 2022 22:35
@crazy-max crazy-max merged commit 8ed470c into docker:master Oct 7, 2022
@crazy-max crazy-max deleted the fix-sha-pr branch October 7, 2022 22:36
@gustavovalverde
Copy link

This might seem as a fix in versioning, but it's a breaking change. For example, this breaks all our workflows which depend on the built image sha

Now we have to include extra conditions as the sha won't be the same when there's a PR and a push to main. It would have been nice to increase the version to 5.0.0 considering this would break most scenarios being based on GITHUB_SHA or ${{ github.sha }}

This now requires implementations to use ( ${{ github.event.pull_request.head.sha }} || {{ github.sha }} ) for a single pull to work on both GitHub events (PRs and push)

@crazy-max
Copy link
Member Author

@gustavovalverde From what I see on ZcashFoundation/zebra#5390, you're pushing an image on PR and others jobs depend on it using github.sha as tag.

I didn't think about this use case as pushing on PR is not possible in majority of cases as secrets can't be exposed. But in your case I see you're using google-github-actions/auth action to connect to your provider using the OIDC token so you're able to push to its registry.

Even if you can share image between jobs without pushing them, it still sounds like a valid use case to use a registry. I will revert this change, sorry about that.

context.ref = `refs/pull/${context.payload.number}/merge`;
}

if ((/pull_request/.test(context.eventName) || /pull_request_target/.test(context.eventName)) && context.payload?.pull_request?.head?.sha != undefined) {
Copy link

Choose a reason for hiding this comment

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

This appears to check that substring “pull_request” appears anywhere in the event name. Maybe the intention was to check if the name starts with a certain substring? Or matches exactly if that’s the entire event name?

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.

Use correct commit sha for PRs?

3 participants