Skip to content

Commit ce23e0a

Browse files
committed
Address cpflow review workflow feedback
1 parent fdda08d commit ce23e0a

5 files changed

Lines changed: 22 additions & 2 deletions

File tree

.github/cpflow-help.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444

4545
- Review apps are opt-in and created with `/deploy-review-app`
4646
- New commits redeploy existing review apps automatically
47+
- Slash command workflows run from the default branch until merged. Test PR-branch edits with `gh workflow run cpflow-deploy-review-app.yml --ref <branch> -f pr_number=<pr-number>`.
4748
- Pushes to the staging branch deploy staging automatically
4849
- Promotion to production is manual via the Actions tab
4950
- A nightly workflow removes stale review apps

.github/workflows/cpflow-delete-review-app.yml

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
name: Delete Review App
22

33
on:
4+
# SECURITY: pull_request_target is intentional for PR-close cleanup from forks.
5+
# This workflow must keep checking out trusted base-branch code only; do not
6+
# change checkout to a PR head ref without re-evaluating secret exposure.
47
pull_request_target:
58
types: [closed]
69
issue_comment:
@@ -53,6 +56,7 @@ jobs:
5356
uses: actions/checkout@v4
5457

5558
- name: Validate required secrets and variables
59+
id: config
5660
uses: ./.github/actions/cpflow-validate-config
5761
env:
5862
CPLN_TOKEN_STAGING: ${{ secrets.CPLN_TOKEN_STAGING }}
@@ -66,6 +70,7 @@ jobs:
6670
pull_request_friendly: "true"
6771

6872
- name: Setup environment
73+
if: steps.config.outputs.ready == 'true'
6974
uses: ./.github/actions/cpflow-setup-environment
7075
with:
7176
token: ${{ secrets.CPLN_TOKEN_STAGING }}
@@ -74,6 +79,7 @@ jobs:
7479
cpflow_version: ${{ vars.CPFLOW_VERSION }}
7580

7681
- name: Set workflow links
82+
if: steps.config.outputs.ready == 'true'
7783
uses: actions/github-script@v7
7884
with:
7985
script: |
@@ -85,6 +91,7 @@ jobs:
8591
);
8692
8793
- name: Create initial PR comment
94+
if: steps.config.outputs.ready == 'true'
8895
id: create-comment
8996
uses: actions/github-script@v7
9097
with:
@@ -98,13 +105,15 @@ jobs:
98105
core.setOutput("comment-id", comment.data.id);
99106
100107
- name: Delete review app
108+
if: steps.config.outputs.ready == 'true'
101109
uses: ./.github/actions/cpflow-delete-control-plane-app
102110
with:
103111
app_name: ${{ env.APP_NAME }}
104112
cpln_org: ${{ vars.CPLN_ORG_STAGING }}
105113
review_app_prefix: ${{ vars.REVIEW_APP_PREFIX }}
106114

107115
- name: Mark GitHub deployment inactive
116+
if: steps.config.outputs.ready == 'true'
108117
uses: actions/github-script@v7
109118
with:
110119
script: |
@@ -132,7 +141,7 @@ jobs:
132141
}
133142
134143
- name: Finalize delete status
135-
if: always()
144+
if: always() && steps.config.outputs.ready == 'true'
136145
uses: actions/github-script@v7
137146
with:
138147
script: |

.github/workflows/cpflow-deploy-review-app.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ jobs:
126126
id: source
127127
env:
128128
EVENT_NAME: ${{ github.event_name }}
129+
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
129130
SAME_REPO: ${{ steps.resolve-pr.outputs.same_repo }}
130131
shell: bash
131132
run: |
@@ -147,10 +148,14 @@ jobs:
147148
148149
if [[ "${EVENT_NAME}" == "issue_comment" ]]; then
149150
echo "allowed=false" >> "$GITHUB_OUTPUT"
151+
message="Review app deploys from fork pull requests require a branch in ${GITHUB_REPOSITORY}. This workflow builds Docker images with repository secrets, so comment-triggered deploys only run for branches in the base repository."
150152
{
151153
echo "Review app deploys from fork pull requests require a branch in ${GITHUB_REPOSITORY}."
152154
echo "This workflow builds Docker images with repository secrets, so comment-triggered deploys only run for branches in the base repository."
153155
} >> "$GITHUB_STEP_SUMMARY"
156+
if ! gh pr comment "${PR_NUMBER}" --body "${message}"; then
157+
echo "::warning::Could not post fork-PR deployment rejection comment."
158+
fi
154159
exit 0
155160
fi
156161

.github/workflows/cpflow-deploy-staging.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ concurrency:
2929
jobs:
3030
validate-branch:
3131
runs-on: ubuntu-latest
32-
timeout-minutes: 5
32+
timeout-minutes: 10
3333
outputs:
3434
is_deployable: ${{ steps.check-branch.outputs.is_deployable }}
3535
steps:

.github/workflows/cpflow-promote-staging-to-production.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ env:
2626
# (e.g. "200" for a plain /health, or "200 401 403" for apps that auth-gate / without
2727
# redirecting).
2828
HEALTH_CHECK_ACCEPTED_STATUSES: ${{ vars.HEALTH_CHECK_ACCEPTED_STATUSES || '200 301 302' }}
29+
# Rollback readiness checks run per workload in sequence, so worst-case wait is
30+
# workload count times ROLLBACK_READINESS_RETRIES times ROLLBACK_READINESS_INTERVAL.
2931
ROLLBACK_READINESS_RETRIES: ${{ vars.ROLLBACK_READINESS_RETRIES || 24 }}
3032
ROLLBACK_READINESS_INTERVAL: ${{ vars.ROLLBACK_READINESS_INTERVAL || 15 }}
3133
PRIMARY_WORKLOAD: ${{ vars.PRIMARY_WORKLOAD }}
@@ -287,8 +289,11 @@ jobs:
287289
run: |
288290
# Best-effort rollback: try every workload, aggregate failures, exit non-zero at the end
289291
# if any failed. A single cpln hiccup shouldn't leave other workloads mid-promotion.
292+
# Intentionally omit -e so one workload failure does not skip the rest.
290293
set -uo pipefail
291294
295+
echo "::warning::Rollback restores workload images only. Database migrations and release_script side effects are not reversed; verify database state manually before re-promoting."
296+
292297
rollback_failures=0
293298
if ! rollback_entries="$(echo "${ROLLBACK_STATE}" | jq -r 'to_entries[] | "\(.key)\t\(.value.containers | @json)"')"; then
294299
echo "::error::Could not parse rollback state; manual recovery may be required." >&2

0 commit comments

Comments
 (0)