ci(docs-preview): Acquire PR context via `gh` CLI by polarathene · Pull Request #4267 · docker-mailserver/docker-mailserver · GitHub
Skip to content

ci(docs-preview): Acquire PR context via gh CLI#4267

Merged
polarathene merged 3 commits into
masterfrom
ci/docs-preview-use-gh-cli
Nov 20, 2024
Merged

ci(docs-preview): Acquire PR context via gh CLI#4267
polarathene merged 3 commits into
masterfrom
ci/docs-preview-use-gh-cli

Conversation

@polarathene

@polarathene polarathene commented Nov 18, 2024

Copy link
Copy Markdown
Member

Description

A pull_request + workflow_run solution that should work well with PRs from forks.

My preference is still to pull_request_target + workflow_call, but that is awaiting confirmation that it was implemented securely (EDIT: It is not secure, unless the untrusted code is only executed in an environment like a container).

UPDATE: Due to review feedback of #4264 (Solution C), while I do prefer that approach I am not comfortable moving forward with it for the project and will favor this PR (Solution B).

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change that does improve existing functionality)

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • New and existing unit tests pass locally with my changes
  • I have added information about changes made in this PR to CHANGELOG.md

@polarathene

polarathene commented Nov 19, 2024

Copy link
Copy Markdown
Member Author

@davidlj95

Copy link
Copy Markdown

This is mostly for my benefit to document all of this research in one place along with full workflow configs to compare. It might also be a helpful resource to others weighing up which approach to implement on their projects.

Thanks a lot for this!! ⭐ Really helpful

Just a small correction: there are two Solution B , I can't C see the C one 😛

@polarathene

Copy link
Copy Markdown
Member Author

Thanks a lot for this!! ⭐ Really helpful

Glad to hear that 🎉

Just a small correction: there are two Solution B , I can't C see the C one 😛

Whoops! Thanks for pointing that out, I've corrected it 😅

casperklein
casperklein previously approved these changes Nov 19, 2024
@polarathene polarathene self-assigned this Nov 20, 2024
@polarathene polarathene added area/ci kind/improvement Improve an existing feature, configuration file or the documentation kind/bug/fix A fix (PR) for a confirmed bug labels Nov 20, 2024
@polarathene polarathene added this to the v15.0.0 milestone Nov 20, 2024
@polarathene polarathene merged commit 02f1894 into master Nov 20, 2024
@polarathene polarathene deleted the ci/docs-preview-use-gh-cli branch November 20, 2024 03:37
# Required by `set-pr-context`:
contents: read
# Required by `marocchino/sticky-pull-request-comment` (write) + `set-pr-context` (read):
pull-requests: write

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Better to move the write permissions to the job that needs them (deploy-preview)

&& github.event.workflow_run.event == 'pull_request'
&& contains(github.event.workflow_run.pull_requests.*.head.sha, github.event.workflow_run.head_sha)
PR_HEADSHA: ${{ steps.set-pr-context.outputs.head-sha }}
PR_NUMBER: ${{ steps.set-pr-context.outputs.number }}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/ci kind/bug/fix A fix (PR) for a confirmed bug kind/improvement Improve an existing feature, configuration file or the documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants