Fix /run-acceptance-tests for bridged providers - #1793
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for community pull requests by adding optional sha parameters to reusable workflows and implementing a status check mechanism for the Sentinel job when running acceptance tests via repository dispatch events.
- Adds optional
shainput parameter to all reusable workflow files to support building from specific commits - Replaces environment variable-based commit SHA references with the new input parameter
- Implements community PR workflow jobs that create and update GitHub status checks for Sentinel
Reviewed Changes
Copilot reviewed 69 out of 69 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| provider-ci/test-providers//workflows/.yml | Added sha input parameter and updated checkout actions to use it instead of environment variables |
| provider-ci/internal/pkg/templates//workflows/.yml | Updated workflow templates with sha parameter support |
| provider-ci/test-providers/*/workflows/run-acceptance-tests.yml | Added start-community-pr and finish-community-pr jobs for status check management |
| provider-ci/test-providers/*/workflows/command-dispatch.yml | Added pull-requests write permission and reorganized workflow structure |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| contents: read | ||
| id-token: write # For ESC secrets. | ||
| with: | ||
| sha: ${{ github.event.client_payload.pull_request.merge_commit_sha }} |
There was a problem hiding this comment.
Earlier we have:
PR_COMMIT_SHA: ${{ github.event.client_payload.pull_request.head.sha }}
It is intentional that the checkout use the merge commit rather than the head commit?
|
|
||
| env: | ||
| PR_COMMIT_SHA: ${{ github.event.client_payload.pull_request.head.sha }} | ||
| MISE_ENV: test |
There was a problem hiding this comment.
What is the impact of removing MISE_ENV
| uses: #{{ .Config.ActionVersions.Checkout }}# | ||
| with: | ||
| ref: ${{ env.PR_COMMIT_SHA }} | ||
| ref: ${{ inputs.sha }} |
There was a problem hiding this comment.
I'm a bit concerned that the ref is changing from head to merge commit.
| with: | ||
| commands: | | ||
| run-acceptance-tests | ||
| release |
There was a problem hiding this comment.
Explanation for why release is being dropped?
| description: Version of the provider to build | ||
| sha: | ||
| type: string | ||
| description: The commit to build, primarily for community PRs. |
There was a problem hiding this comment.
Might be nice to clarify the default behavior.
| repository_dispatch: | ||
| types: | ||
| - run-acceptance-tests-command | ||
| workflow_dispatch: {} |
There was a problem hiding this comment.
Is workflow_dispatch actually being used?
This adds an optional
shaargument to our reusable workflows. When we get a community PR, we propagate the PR's merge commit SHA to those workflows.run-acceptance-tests.ymlis modified to create and update a status check for Sentinel for these community PR workflows.We aren't able to use the native "Approve workflows to run" button because our ESC secrets require
id-token: writepermission but PRs from forks can only haveread.Fixes #1409