Simplify approval gate with dual-environment pattern#11300
Simplify approval gate with dual-environment pattern#11300
Conversation
sylvainsf
requested a deployment
to
external-contributor-approval
GitHub Actions
Waiting
There was a problem hiding this comment.
Pull request overview
This PR refactors the approval gate mechanism in the functional-test-cloud.yaml workflow from a skip-based approach to a dual-environment pattern. The previous implementation (fixed in PR #11189) conditionally skipped the approval-gate job for trusted users, requiring complex downstream conditions. The new approach always runs the approval-gate job for pull_request_target events but selects between two environments: pr-trusted (no protection rules) for org members and dependabot, and external-contributor-approval (manual approval required) for external contributors. This eliminates the falsy-value bug and simplifies the setup job's conditional logic.
Changes:
- Refactored
approval-gatejob to always run onpull_request_targetand use conditional environment selection instead of conditional job execution - Simplified
setupjob condition from checking multipleapproval-gateresults (success,skipped) to a single cleaner check (!cancelled() && result != 'failure') - Enhanced inline documentation explaining the dual-environment pattern and prerequisites
Replace skip-based approval gate with a dual-environment pattern adopted from SAP/crossplane-provider-btp. Instead of conditionally skipping the approval-gate job for trusted users and only running it for external contributors, the job now always runs on pull_request_target events and selects one of two environments: - 'pr-trusted': No protection rules, runs immediately for org members (OWNER/MEMBER/COLLABORATOR) and dependabot - 'external-contributor-approval': Requires manual reviewer approval for external contributors This eliminates the falsy-value bug where the old ternary pattern (condition && '' || 'environment') always evaluated to the environment name because empty string is falsy in GitHub Actions expressions. The setup job's if condition is also simplified from: always() && (event != 'pull_request_target' || result == 'suc always() && (event != 'pull_request_target' || result == 'suc always() && (event != 'pull_request_target' || result == 'suc always() && (event != 'pull_request_tarory settings. Signed-off-by: Sylvain Niles <sylvainniles@microsoft.com>
sylvainsf
force-pushed
the
simplify-approval-gate
branch
from
bc45ee1 to
a7fe173
Compare
sylvainsf
temporarily deployed
to
external-contributor-approval
GitHub Actions
Inactive
Radius functional test overviewClick here to see the test run details
Test Status⌛ Building Radius and pushing container images for functional tests... |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #11300 +/- ##
==========================================
- Coverage 51.00% 50.99% -0.02%
==========================================
Files 679 679
Lines 43174 43174
==========================================
- Hits 22023 22015 -8
- Misses 19033 19037 +4
- Partials 2118 2122 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Description
Simplifies the approval gate in
functional-test-cloud.yamlby adopting a dual-environment pattern (inspired by SAP/crossplane-provider-btp).Problem
The previous approach conditionally skipped the
approval-gatejob for trusted users and only ran it for external contributors. This led to a complexifcondition on thesetupjob that had to enumerate every possibleapproval-gateresult (success,skipped). The original implementation (before PR #11189) also had a falsy-value bug:(condition) && '' || 'environment'always returned'environment'because empty string is falsy in GitHub Actions expressions.Solution
Instead of skip-based gating, the
approval-gatejob now always runs onpull_request_targetevents and selects one of two GitHub environments:pr-trustedexternal-contributor-approvalBoth branches of the ternary expression are truthy strings, avoiding the falsy-value bug entirely.
The
setupjob condition is simplified fromalways() && (event != ... || result == 'success' || result == 'skipped')to!cancelled() && result != 'failure'.Prerequisites
pr-trustedGitHub environment in repository settings with NO protection rules.Type of change
Contributor checklist
Please verify that the PR meets the following requirements, where applicable: