Document cpflow downstream setup#747
Conversation
🚀 Quick Review App CommandsWelcome! Here are the commands you can use in this PR:
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (1)
WalkthroughAdds a "GitHub Review App Setup" section and clarifies cpflow secrets/variables and advanced testing; repins all cpflow wrapper workflows to a newer immutable upstream commit SHA, and annotates promotion workflow permissions and environment secret usage. ChangesControl Plane Documentation and Workflow Reference Update
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is ON. A cloud agent has been kicked off to fix the reported issue.
Reviewed by Cursor Bugbot for commit d5e1fd6. Configure here.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| @@ -15,4 +15,4 @@ permissions: | |||
| jobs: | |||
| show-help: | |||
| if: vars.REVIEW_APP_PREFIX != '' | |||
There was a problem hiding this comment.
This guard contradicts the new documentation. The README and cpflow-help.md now say REVIEW_APP_PREFIX is optional and inferred from controlplane.yml, but this condition means the help workflow will be silently skipped when REVIEW_APP_PREFIX is unset — which is the new recommended default.
If the upstream reusable workflow now supports inferring the prefix, this guard should be removed (or updated to a condition that always evaluates to true for authorized callers). As-is, teams following the new "leave REVIEW_APP_PREFIX unset" guidance will never see the help command fire.
| if: vars.REVIEW_APP_PREFIX != '' | |
| if: true |
| # upstream workflow. The upstream workflow only reads the named secrets it | ||
| # references, but GitHub does not enforce that boundary. Strict consumers can | ||
| # set CPFLOW_GITHUB_ACTIONS_REF to an immutable commit SHA. | ||
| secrets: inherit |
There was a problem hiding this comment.
The comment mentions CPFLOW_GITHUB_ACTIONS_REF as an env var for strict consumers, but the actual input used everywhere else in these workflows is control_plane_flow_ref. This looks like a variable name that doesn't exist — it may confuse readers trying to act on this guidance.
Consider referencing the correct mechanism (the control_plane_flow_ref workflow input or the @<sha> ref in uses:) instead.
Code Review: Document cpflow downstream setupOverviewThis PR documents the minimal GitHub/Control Plane setup for review apps, staging, and production, and bumps all generated cpflow workflow wrappers from commit IssuesBug: Minor: Misleading variable name in Security
Documentation QualityThe new sections in One minor note: the README update changes the review app naming convention from SummaryThe |
Greptile SummaryThis PR documents the minimal GitHub and Control Plane setup required for generated cpflow review apps, staging deploys, and production promotion, and bumps all workflow SHA pins from
Confidence Score: 4/5Safe to merge with one follow-up: the REVIEW_APP_PREFIX guard in cpflow-review-app-help.yml should be removed or relaxed now that the variable is documented as optional. The documentation and SHA-pin changes are accurate and consistent. The only functional mismatch is in cpflow-review-app-help.yml, where a pre-existing guard silently suppresses the onboarding help comment for any repo that follows the newly documented standard setup without setting REVIEW_APP_PREFIX. Deploys still work — only the help message is lost — but it directly contradicts the intent of this PR. .github/workflows/cpflow-review-app-help.yml — the REVIEW_APP_PREFIX guard needs to be removed or updated to match the new optional-variable documentation.
|
| Filename | Overview |
|---|---|
| .github/workflows/cpflow-review-app-help.yml | SHA bumped to db013e1; the pre-existing if: vars.REVIEW_APP_PREFIX != '' guard now contradicts the updated docs that say REVIEW_APP_PREFIX is optional, meaning the help job is silently skipped for standard setups. |
| .github/workflows/cpflow-promote-staging-to-production.yml | Added production_environment: production input to gate production secrets behind the protected GitHub Environment; added explanatory comments; SHA bumped. The acknowledged secrets: inherit scope is worth verifying against org policy. |
| .controlplane/readme.md | Large documentation addition covering GitHub secrets/variables, Control Plane org/app/secret-dictionary setup, and production promotion isolation. Content is accurate and consistent with the workflow changes. |
| .github/cpflow-help.md | Updated secrets/variables tables to mark REVIEW_APP_PREFIX optional, clarified CPLN_TOKEN_PRODUCTION placement, and added an advanced section on testing upstream PRs with commit-SHA pins. |
| .github/workflows/cpflow-cleanup-stale-review-apps.yml | SHA bumped from 3e0e7e1 to db013e1 in both uses: and control_plane_flow_ref; no other changes. |
| .github/workflows/cpflow-delete-review-app.yml | SHA bumped consistently; no other changes. |
| .github/workflows/cpflow-deploy-review-app.yml | SHA bumped consistently; no other changes. No REVIEW_APP_PREFIX guard here (intentional asymmetry with cpflow-review-app-help.yml). |
| .github/workflows/cpflow-deploy-staging.yml | SHA bumped consistently; no other changes. |
| .github/workflows/cpflow-help-command.yml | SHA bumped consistently; no other changes. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
PR[Pull Request opened/sync] --> DeployJob[cpflow-deploy-review-app\nno REVIEW_APP_PREFIX guard]
PR --> HelpJob{cpflow-review-app-help\nif REVIEW_APP_PREFIX != ''}
HelpJob -- "var set" --> PostHelp[Post help comment on PR]
HelpJob -- "var unset\n(new standard setup)" --> Skip[Job skipped — no help posted]
DeployJob --> UpstreamDeploy[Upstream reusable workflow\ninfers prefix from controlplane.yml]
Master[Push to master] --> StagingDeploy[cpflow-deploy-staging]
StagingDeploy --> UpstreamStaging[Upstream staging workflow]
ManualTrigger[workflow_dispatch\nconfirm_promotion=promote] --> PromoteJob[cpflow-promote-staging-to-production]
PromoteJob --> EnvGate{Protected GitHub Environment\nproduction — requires reviewer approval}
EnvGate -- approved --> UpstreamPromote[Upstream promote workflow\nreads CPLN_TOKEN_PRODUCTION]
EnvGate -- rejected --> Blocked[Promotion blocked]
Reviews (1): Last reviewed commit: "Document cpflow downstream setup" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @@ -15,4 +15,4 @@ permissions: | |||
| jobs: | |||
| show-help: | |||
| if: vars.REVIEW_APP_PREFIX != '' | |||
There was a problem hiding this comment.
The updated docs explicitly say REVIEW_APP_PREFIX is optional and inferred from controlplane.yml, but the guard if: vars.REVIEW_APP_PREFIX != '' remains. Any repo that follows the new standard setup (leaving REVIEW_APP_PREFIX unset) will never see the help comment posted on opened PRs — the job is skipped entirely. The deploy workflow (cpflow-deploy-review-app.yml) has no such guard, so deploys work but onboarding help is silently absent.
| # `secrets: inherit` passes all caller repository secrets to the trusted | ||
| # upstream workflow. The upstream workflow only reads the named secrets it | ||
| # references, but GitHub does not enforce that boundary. Strict consumers can | ||
| # set CPFLOW_GITHUB_ACTIONS_REF to an immutable commit SHA. | ||
| secrets: inherit |
There was a problem hiding this comment.
secrets: inherit exposes all repository secrets to the upstream workflow
The inline comment correctly notes that secrets: inherit passes every caller repository secret to the upstream reusable workflow, and that GitHub does not enforce the named-secrets boundary. In the current setup this includes CPLN_TOKEN_STAGING, any DOCKER_BUILD_SSH_KEY, and any other repository secrets. If the upstream SHA (db013e...) ever changes to include malicious or compromised code, those secrets are exposed during a production-promotion run. The PR already pins to an immutable SHA which limits supply-chain risk, but consider whether your org's policy requires explicit secret whitelisting via environment-level scoping rather than secrets: inherit.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.controlplane/readme.md:
- Around line 489-493: The README guidance and shakacode-team doc disagree about
review-app naming: update .controlplane/shakacode-team.md to match
.controlplane/readme.md by changing the instructions that set REVIEW_APP_PREFIX
to instead instruct leaving REVIEW_APP_PREFIX unset (or to document the inferred
name qa-react-webpack-rails-tutorial) and reference
.controlplane/controlplane.yml for app name alignment; ensure the text mentions
the generated review app pattern qa-react-webpack-rails-tutorial-<PR number> and
remove the conflicting example
REVIEW_APP_PREFIX=qa-react-webpack-rails-tutorial-pr so both docs are
consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 339f15eb-4b09-48f3-897d-86bcf9c18dec
📒 Files selected for processing (9)
.controlplane/readme.md.github/cpflow-help.md.github/workflows/cpflow-cleanup-stale-review-apps.yml.github/workflows/cpflow-delete-review-app.yml.github/workflows/cpflow-deploy-review-app.yml.github/workflows/cpflow-deploy-staging.yml.github/workflows/cpflow-help-command.yml.github/workflows/cpflow-promote-staging-to-production.yml.github/workflows/cpflow-review-app-help.yml
Code Review: Document cpflow downstream setupOverviewThis PR improves the setup experience for downstream cpflow users by consolidating documentation and fixing the production token exposure model. The security change—moving SecurityPositive: Gating Concern — Breaking Change: Review App Naming ConventionThe PR changes the review app naming scheme from Documentation QualityThe new The advanced section in Workflow Changes
Summary
Overall this is a solid improvement. Address the naming-convention migration note and clarify the |
| # references, but GitHub does not enforce that boundary. Strict consumers can | ||
| # keep both the `uses:` ref and `control_plane_flow_ref` pinned to an | ||
| # immutable commit SHA. | ||
| secrets: inherit |
There was a problem hiding this comment.
The comment says the upstream workflow "only reads the named secrets it references," which is accurate for repository secrets. Worth clarifying here that environment secrets (e.g. CPLN_TOKEN_PRODUCTION) are not passed via secrets: inherit—they're only injected when the job itself directly references the production environment. So the production token isolation is actually stronger than this comment implies; a reader might incorrectly assume CPLN_TOKEN_PRODUCTION leaks through secrets: inherit.
| `qa-react-webpack-rails-tutorial` and staging org | ||
| `shakacode-open-source-examples-staging` from `.controlplane/controlplane.yml` | ||
| because that file defines exactly one app with | ||
| `match_if_app_name_starts_with: true`. |
There was a problem hiding this comment.
This inference assumption is fragile: if controlplane.yml ever gains a second app entry with match_if_app_name_starts_with: true, the inference will fail (or pick the wrong one) with no clear error. Consider adding a sentence here directing readers to set CPLN_ORG_STAGING and REVIEW_APP_PREFIX explicitly if they hit ambiguous inference, so the failure mode is less opaque.
| - Leave `REVIEW_APP_PREFIX` unset for the standard path. The workflow infers | ||
| `qa-react-webpack-rails-tutorial` from `.controlplane/controlplane.yml`, so | ||
| generated review apps use names like | ||
| `qa-react-webpack-rails-tutorial-1234`. |
There was a problem hiding this comment.
Breaking change — naming convention. The old scheme was qa-react-webpack-rails-tutorial-pr-<N>; the new scheme omits the -pr segment. Any live review apps created under the old convention become orphaned after this merges: the cleanup and delete workflows query by the new prefix and won't find them. Recommend adding a migration note (e.g. in the PR description or a one-time manual cleanup step) so operators know to delete old-style apps before merging.
Code Review: Document cpflow downstream setupOverall assessment: Low-risk, well-structured documentation PR. The shift of `CPLN_TOKEN_PRODUCTION` from a repository secret to a protected GitHub Environment is a genuine security improvement. SHA pinning and the two-pin comment in the promotion workflow are both good practices. ✅ Strengths
Issues1. Contradictory `Required` column for production variables in `cpflow-help.md` The table rows for `CPLN_ORG_PRODUCTION` and `PRODUCTION_APP_NAME` say `yes (for promote)` in the Required column — implying repository variables — but the Notes column says "Prefer a `production` environment variable." The recommendation and the column label contradict each other. Readers who follow the `yes` column will put these in repo variables; readers who follow the note will put them in the environment. The table should be updated to say something like `yes (for promote) — as environment variable` or the Required column should use a different marker for environment variables. (See inline comment.) 2. `STAGING_APP_BRANCH` / `PRIMARY_WORKLOAD` required vs. optional discrepancy `shakacode-team.md` lists both under "Required repository variables for staging deploys", while `cpflow-help.md` marks both as optional. At minimum `PRIMARY_WORKLOAD` defaults to `rails`, so listing it as required in one doc and optional in the other will confuse fork maintainers. 3. `SECRET_KEY_BASE` test placeholder The readme documents that the review/staging template "currently contains a test placeholder for `SECRET_KEY_BASE`" and asks readers to replace it before production. This is a hard security requirement, not just guidance — a placeholder key in staging means any staging environment is running with a known, weak secret. Consider either removing the placeholder in a follow-up PR and replacing it with a proper secret reference, or at minimum adding a CI check that fails if the placeholder value is detected. 4. Minor `secrets: inherit` comment improvement The acknowledgment in the promotion workflow is transparent and appreciated. One thing worth making explicit: because `CPLN_TOKEN_PRODUCTION` is now an environment secret (not a repo secret), `secrets: inherit` in the staging and review-app workflows won't inadvertently expose it — the environment gate is the actual isolation boundary, not just the upstream workflow's access pattern. Adding that note would help future readers understand why the security model holds. |
| | `CPLN_ORG_STAGING` | yes | Control Plane org on controlplane.com for staging and review apps. | | ||
| | `CPLN_ORG_PRODUCTION` | yes (for promote) | Control Plane org on controlplane.com for production. | | ||
| | `CPLN_ORG_STAGING` | optional for review apps; yes for staging | Override the staging/review Control Plane org inferred from `controlplane.yml`. | | ||
| | `CPLN_ORG_PRODUCTION` | yes (for promote) | Control Plane org on controlplane.com for production. Prefer a `production` environment variable. | |
There was a problem hiding this comment.
The Required column says yes (for promote) — which implies a repository variable — but the Notes column says "Prefer a production environment variable." These two signals contradict each other. Readers who scan the Required column will put this in repo variables; readers who follow the note will put it in the protected environment. Suggest picking one canonical placement and stating it clearly, e.g. updating the Required cell to yes — environment variable or splitting the secrets and environment-variable rows into separate tables.
| | `PRODUCTION_APP_NAME` | yes (for promote) | App name in `controlplane.yml` used as the production deploy target. | | ||
| | `REVIEW_APP_PREFIX` | yes | Prefix for per-PR review app names (e.g. `review-app`). | | ||
| | `REVIEW_APP_DEPLOYING_ICON_URL` | optional | Custom image URL for the animated deploying icon in review-app PR comments. Set to `none` to use the text fallback icon. | | ||
| | `PRODUCTION_APP_NAME` | yes (for promote) | App name in `controlplane.yml` used as the production deploy target. Prefer a `production` environment variable. | |
There was a problem hiding this comment.
Same issue as CPLN_ORG_PRODUCTION above: yes (for promote) implies a repo variable, but the Notes column recommends a production environment variable. Both entries need a consistent placement signal.
| # `secrets: inherit` passes all caller repository secrets to the trusted | ||
| # upstream workflow. The upstream workflow only reads the named secrets it | ||
| # references, but GitHub does not enforce that boundary. Strict consumers can | ||
| # keep both the `uses:` ref and `control_plane_flow_ref` pinned to an | ||
| # immutable commit SHA. |
There was a problem hiding this comment.
The transparency here is good. One clarification worth adding to the comment: because CPLN_TOKEN_PRODUCTION is now an environment secret (not a repo secret), secrets: inherit in the staging/review-app caller workflows won't expose it — those jobs never acquire the production environment context, so GitHub never injects the environment secret into them. The environment gate is the actual isolation boundary, not just the upstream workflow's declared access pattern. This makes the security model more robust than the comment currently implies.
|
|
||
| Generate `SECRET_KEY_BASE` with `openssl rand -hex 64` and | ||
| `RENDERER_PASSWORD` with `openssl rand -hex 32`. The review/staging template | ||
| currently contains a test placeholder for `SECRET_KEY_BASE`; replace it with a |
There was a problem hiding this comment.
Documenting the placeholder is good, but this phrasing ("replace it with a secret-backed value before promoting production") leaves staging running with a known, weak SECRET_KEY_BASE. If the staging environment is internet-accessible (which review apps typically are), that's a real risk — anyone with the placeholder value can forge session cookies. Consider either removing the placeholder from the template in a follow-up and requiring an explicit secret reference, or adding a CI/readiness check that rejects deploys when SECRET_KEY_BASE matches the placeholder value.
Code ReviewOverview: Documentation-heavy PR that improves the cpflow downstream setup story — moves the production token into a protected GitHub Environment, infers review-app config from Issues1.
2. Removing 3. Naming change from The PR removes the 4. The inline comment acknowledges the issue but leaves it as-is. Minor
Positives
|
| contains(fromJson('["OWNER","MEMBER","COLLABORATOR"]'), github.event.comment.author_association)) || | ||
| github.event_name == 'workflow_dispatch' | ||
| uses: shakacode/control-plane-flow/.github/workflows/cpflow-help-command.yml@3e0e7e1f0a35c15648cc9254b573b058d77ca8c4 | ||
| uses: shakacode/control-plane-flow/.github/workflows/cpflow-help-command.yml@cfe494bf32925d49508380e03856d97bd71f6689 |
There was a problem hiding this comment.
All the operational cpflow workflows (cleanup, delete, deploy, staging, promote) pass control_plane_flow_ref as a with: input alongside the uses: SHA. This one does not. If the upstream help workflow has a setup action that uses control_plane_flow_ref to check out and build the gem, it will silently fall back to its own default rather than the pinned SHA.
Unless the upstream cpflow-help-command.yml workflow is known to not have a gem-build step, add:
| uses: shakacode/control-plane-flow/.github/workflows/cpflow-help-command.yml@cfe494bf32925d49508380e03856d97bd71f6689 | |
| uses: shakacode/control-plane-flow/.github/workflows/cpflow-help-command.yml@cfe494bf32925d49508380e03856d97bd71f6689 | |
| with: | |
| control_plane_flow_ref: cfe494bf32925d49508380e03856d97bd71f6689 |
Or add a comment explaining why this workflow doesn't need it.
| show-help: | ||
| if: vars.REVIEW_APP_PREFIX != '' | ||
| uses: shakacode/control-plane-flow/.github/workflows/cpflow-review-app-help.yml@3e0e7e1f0a35c15648cc9254b573b058d77ca8c4 | ||
| uses: shakacode/control-plane-flow/.github/workflows/cpflow-review-app-help.yml@cfe494bf32925d49508380e03856d97bd71f6689 |
There was a problem hiding this comment.
Same as cpflow-help-command.yml — no with: block means control_plane_flow_ref is not passed, unlike all the operational workflows. If the upstream help workflow doesn't need it, a short comment here would prevent future maintainers from treating this as an oversight during the next SHA repin.
| # references, but GitHub does not enforce that boundary. Strict consumers can | ||
| # keep both the `uses:` ref and `control_plane_flow_ref` pinned to an | ||
| # immutable commit SHA. | ||
| secrets: inherit |
There was a problem hiding this comment.
secrets: inherit sends all caller repository secrets — including CPLN_TOKEN_STAGING, SSH keys, and any other repository secrets — to the upstream reusable workflow. The SHA pin limits exposure to a known-good commit, but it is still a broader grant than strictly necessary for a production promotion job.
If the upstream cpflow-promote-staging-to-production.yml workflow supports receiving named secrets explicitly (e.g., secrets: { CPLN_TOKEN_PRODUCTION: ${{ secrets.CPLN_TOKEN_PRODUCTION }} }), that would be strictly tighter. If it requires secrets: inherit, the current approach is the practical maximum — just worth documenting explicitly so the next maintainer doesn't widen the SHA pin without realizing the implication.
| `qa-react-webpack-rails-tutorial-pr-1234`. | ||
| The GitHub settings and Control Plane resources must match the app names in | ||
| `.controlplane/controlplane.yml`. For the standard review-app path, leave | ||
| `REVIEW_APP_PREFIX` unset and let the workflow infer |
There was a problem hiding this comment.
This silently changes the naming convention from the previously documented qa-react-webpack-rails-tutorial-pr-<N> (with the -pr suffix) to qa-react-webpack-rails-tutorial-<N>. Any review apps currently running under the old names, bookmark links, or CI output referencing them will break without warning.
Consider adding a migration note here, even one sentence: "If you previously had REVIEW_APP_PREFIX set to qa-react-webpack-rails-tutorial-pr, delete existing review apps before switching — the old names will become orphaned resources in Control Plane."
1 participant
Summary
Validation
Note
Medium Risk
Repins all deployment workflows to a new upstream commit (behavior change) and documents production token handling via a protected environment; mostly docs plus CI wrapper changes, not application runtime code.
Overview
This PR expands downstream cpflow setup documentation and aligns generated GitHub Actions with the newer upstream flow.
Documentation now describes a minimal review-app path: only
CPLN_TOKEN_STAGINGis required, withREVIEW_APP_PREFIXand staging org inferred from.controlplane/controlplane.yml(app keyqa-react-webpack-rails-tutorial, not the old-prsuffix). It adds tables for optional GitHub overrides, staging variables, protectedproductionenvironment secrets (CPLN_TOKEN_PRODUCTION), and Control Plane orgs, apps, and secret dictionaries (SECRET_KEY_BASE, etc.). Internal team notes and.github/cpflow-help.mdmatch this model and add guidance on pinning unreleasedcontrol-plane-flowvia commit SHA vs release tags.Workflows repin all
cpflow-*reusable workflows from3e0e7e1…tocfe494bf…, pass the samecontrol_plane_flow_ref, wire promotion throughproduction_environment: productionwith comments on permissions andsecrets: inherit, and remove thevars.REVIEW_APP_PREFIX != ''guard on PR-open help so help posts without setting that variable.Reviewed by Cursor Bugbot for commit 8e2722f. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit