Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
140 changes: 140 additions & 0 deletions .controlplane/docs/testing-cpflow-github-actions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
# Testing cpflow GitHub Actions Changes

Use this guide when changing generated `cpflow-*` GitHub Actions, updating the
`cpflow` generator version, or debugging review-app automation.

## What To Test

Test the flow in three layers:

1. Local generated-file checks catch YAML, metadata, and lint problems before a PR.
2. GitHub workflow checks prove GitHub can load the workflow and run CI.
3. A real review-app deploy proves the default-branch trusted actions, GitHub
secrets, Docker build, and Control Plane deploy all work together.

The third layer matters because the review-app workflow intentionally checks out
trusted workflow sources from the repository default branch before passing
Control Plane secrets to local composite actions. A PR branch can contain fixed
`.github/actions/*` files, but the deploy job still loads those local actions
from `master` until the fix is merged there.

## Local Checks
Comment on lines +19 to +21
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The preamble introduces the unreleased-checkout case, but every command in the block below uses a hard-coded path — there is no hint for readers using a normally-installed gem. Consider clarifying:

Suggested change
from `master` until the fix is merged there.
## Local Checks
After regenerating the flow, run these checks from the repository root. If
`cpflow` is installed as a gem, use `bin/conductor-exec cpflow <subcommand>`
directly. When testing an unreleased upstream `control-plane-flow` checkout,
replace `cpflow` with `ruby /path/to/control-plane-flow/bin/cpflow`:


After regenerating the flow, run these checks from the repository root. If
`cpflow` is installed as a gem, use `cpflow` directly:

```sh
bin/conductor-exec cpflow generate-github-actions --staging-branch master
bin/test-cpflow-github-flow
```

When testing an unreleased upstream `control-plane-flow` checkout, replace
`cpflow` with that checkout's `bin/cpflow`:

```sh
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This one-liner is ~200 characters and will wrap unpredictably in rendered Markdown, making it easy to mis-copy (a truncated command silently does nothing or raises a syntax error). The "Ways To Make This Easier" section already suggests extracting these checks into bin/test-cpflow-github-flow. Moving this command there now — even as a thin wrapper — would be more robust and reduce copy-paste risk for future readers.

bin/conductor-exec ruby /path/to/control-plane-flow/bin/cpflow generate-github-actions --staging-branch master
bin/test-cpflow-github-flow ruby /path/to/control-plane-flow/bin/cpflow
```

Why the explicit description check exists: GitHub parses expression-like snippets
inside composite action metadata, including `description:` fields. Literal
examples such as `${{ vars.SOME_VALUE }}` can fail action loading before any
shell step starts. The wrapper runs `cpflow github-flow-readiness`, parses the
generated YAML, checks action input descriptions for literal GitHub expressions,
and runs `actionlint -ignore 'SC2129' .github/workflows/cpflow-*.yml`.

## PR Checks

Open a normal PR for the generated-file diff and wait for CI. The workflow PR
itself is useful for syntax and CI validation, but it does not fully prove
review-app deployment changes that live under `.github/actions/`.

For top-level workflow edits, you can manually dispatch the PR branch workflow:

```sh
gh workflow run cpflow-deploy-review-app.yml --ref <branch> -f pr_number=<pr-number>
```

This loads the workflow file from `<branch>`, but the deploy workflow's
`Checkout trusted workflow sources` step still checks out `master` before using
local composite actions with secrets. Treat this as a partial smoke test, not as
proof that PR-branch composite action changes work.

## Post-Merge Review-App Test

After the workflow PR merges to `master`, test a real review-app deployment:

1. Pick a same-repository PR to use as the canary.
2. If the review app does not exist yet, comment exactly `+review-app-deploy` on
that PR.
3. If a previous deploy run failed, rerun the failed deploy run after the
workflow PR is merged.
4. Confirm the deploy run checks out `master` at the merge commit in
`Checkout trusted workflow sources`.
5. Confirm `Setup environment` succeeds and prints the expected `cpflow` version.
6. Confirm `Check if review app exists`, `Build Docker image`, and
`Deploy to Control Plane` all run as expected.
7. Open the review-app URL from the PR comment or deployment status and verify
it returns HTTP 200.

Use the generated app name from the workflow log:

```text
APP_NAME: ${REVIEW_APP_PREFIX}-${PR_NUMBER}
```

This is a template from the workflow output, not a literal command to evaluate
unless those environment variables are already set. For this repo, verify the
actual `REVIEW_APP_PREFIX` repository variable before assuming the final app
name.

## Troubleshooting Signals

### Composite action metadata fails before setup
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The APP_NAME: ${REVIEW_APP_PREFIX}-${PR_NUMBER} line is shell-interpolation syntax inside a fenced text block, so it renders literally in markdown — which is exactly what the following paragraph explains. No change needed, just confirming the intent is clear.


Error shape:

```text
Unrecognized named-value: 'vars'
Failed to load ./.github/actions/cpflow-setup-environment/action.yml
```

Cause: GitHub parsed a literal expression inside composite action metadata,
usually an input description. Because trusted local actions come from `master`,
fix and merge the generated action metadata on `master`, then rerun the deploy.

### Setup succeeds, then `cpflow exists` reports token format

Error shape:

```text
ERROR: Unknown API token format. Please re-run 'cpln profile login' or set the correct CPLN_TOKEN env variable.
```

Cause: the workflow can read `CPLN_TOKEN_STAGING`, but the value is not a valid
Control Plane service-account token for the installed Control Plane CLI. Rotate
the GitHub secret, then rerun the failed deploy job.

### PR pushes do not create a new review app

This is expected. Pushes redeploy only after the review app already exists.
Create the first review app by commenting exactly:

```text
+review-app-deploy
```

## Ways To Make This Easier

- Add a no-secret GitHub Actions smoke workflow that loads generated local
composite actions from the PR branch and fails fast on action metadata parsing.
- Extend `bin/test-cpflow-github-flow` as more local cpflow GitHub Actions
checks become worth standardizing.
- Add an early token sanity step after `Setup environment` so invalid
`CPLN_TOKEN_STAGING` and `CPLN_TOKEN_PRODUCTION` values fail with a named
"validate Control Plane token" step instead of surfacing later during
`cpflow exists`.
- Keep a tiny canary PR open for review-app workflow testing so post-merge
deploy verification does not depend on whichever feature PR happens to exist.
- Upstream the metadata-description check to `cpflow github-flow-readiness` so
downstream repos get the guard automatically.
19 changes: 14 additions & 5 deletions .controlplane/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -424,24 +424,33 @@ repository variables, secrets, and docs aligned with `.controlplane/controlplane
For this app, validate a regenerated flow with:

```bash
bundle exec ruby /path/to/control-plane-flow/bin/cpflow generate-github-actions --staging-branch master
bundle exec ruby /path/to/control-plane-flow/bin/cpflow github-flow-readiness
actionlint .github/workflows/cpflow-*.yml
bundle exec rubocop
bin/conductor-exec ruby /path/to/control-plane-flow/bin/cpflow generate-github-actions --staging-branch master
bin/conductor-exec ruby /path/to/control-plane-flow/bin/cpflow github-flow-readiness
actionlint -ignore 'SC2129' .github/workflows/cpflow-*.yml
bin/conductor-exec bundle exec rubocop
```
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The new testing-cpflow-github-actions.md guide runs actionlint -ignore 'SC2129', but this line still omits that flag. The two docs will produce different output for the same workflow files, which is confusing for anyone switching between them.

Suggested change
```
actionlint -ignore 'SC2129' .github/workflows/cpflow-*.yml


Then open a normal PR and let GitHub Actions prove the generated review-app,
staging, lint, JS, and RSpec workflows before merging. For review-app workflow
changes, test both the local workflow syntax and a real deployment. GitHub runs
`issue_comment` workflows from the default branch, so a `+review-app-deploy`
comment on the PR does not fully exercise command changes that are only on the
PR branch. Before merge, run the PR branch workflow explicitly:
PR branch. For top-level workflow edits, run the PR branch workflow explicitly:

```bash
gh workflow run cpflow-deploy-review-app.yml --ref <branch> -f pr_number=<pr-number>
```

This loads the workflow file from `<branch>`, but trusted local composite
actions still come from the default branch before secrets are used. Treat it as
a partial smoke test, then verify a real deploy after the workflow changes land
on `master`.

After the workflow reports a review-app URL, verify the URL returns HTTP 200.
If a project needs to track generator changes automatically, use a scheduled
maintenance PR or Renovate-style workflow that bumps the `cpflow` version,
regenerates these files, and runs the same validation commands.

For a fuller checklist, including the gotcha that review-app deploys load local
composite actions from `master` before using Control Plane secrets, see
[Testing cpflow GitHub Actions Changes](docs/testing-cpflow-github-actions.md).
11 changes: 8 additions & 3 deletions .controlplane/shakacode-team.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,14 @@ flow, regenerate the `cpflow-*` actions/workflows in this repo from the target
`cpflow` version or branch using `--staging-branch master`, review the diff, and
keep the repository variables above aligned with `.controlplane/controlplane.yml`. Validate with
`cpflow github-flow-readiness`, `actionlint .github/workflows/cpflow-*.yml`, and
the normal CI checks before merging.

See [readme.md](readme.md) for more details.
the normal CI checks before merging. For review-app workflow changes, remember
that the deploy workflow checks out trusted local actions from `master` before
passing Control Plane secrets; PR-branch composite action changes are not fully
tested until they land on `master` and a real review-app deploy is rerun.

See [readme.md](readme.md) and
[Testing cpflow GitHub Actions Changes](docs/testing-cpflow-github-actions.md)
for more details.

## Links

Expand Down
4 changes: 2 additions & 2 deletions .github/cpflow-help.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,14 @@ You asked for review app help. These commands are generated by [cpflow](https://
<details>
<summary>Advanced: testing changes to generated workflows</summary>

When iterating on the generated workflow YAML on a PR branch, comment-triggered runs (`+review-app-deploy`, `+review-app-delete`, `+review-app-help`) execute the workflow code from the repository's default branch — not your PR branch. To exercise the PR-branch workflow code before merging, dispatch the workflow manually with `gh`:
When iterating on the generated workflow YAML on a PR branch, comment-triggered runs (`+review-app-deploy`, `+review-app-delete`, `+review-app-help`) execute the workflow code from the repository's default branch — not your PR branch. To exercise the top-level PR-branch workflow file before merging, dispatch the workflow manually with `gh`:

```sh
gh workflow run cpflow-deploy-review-app.yml --ref <your-pr-branch> -f pr_number=<pr-number>
gh workflow run cpflow-delete-review-app.yml --ref <your-pr-branch> -f pr_number=<pr-number>
gh workflow run cpflow-help-command.yml --ref <your-pr-branch> -f pr_number=<pr-number>
```

`workflow_dispatch` runs use the workflow file from the `--ref` you pass, so this is the supported way to test PR-branch workflow edits before merge. After merge, comment triggers go back to running the default-branch workflow code as usual.
`workflow_dispatch` runs use the workflow file from the `--ref` you pass, but workflows that intentionally check out trusted local actions from the default branch will still load those local composite actions from the default branch before using secrets. Treat this as a partial smoke test for top-level workflow edits. For changes under `.github/actions/`, merge the generated fix to the default branch and rerun a real review-app deploy.

</details>
42 changes: 42 additions & 0 deletions bin/test-cpflow-github-flow
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
#!/usr/bin/env bash
set -euo pipefail

ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
cd "$ROOT"

cpflow_cmd=(cpflow)
if [[ $# -gt 0 ]]; then
cpflow_cmd=("$@")
Comment on lines +6 to +9
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

When no arguments are supplied the script silently falls back to looking up cpflow in $PATH. If it isn't installed, conductor-exec will fail with a generic "not found" error.

A small guard here mirrors what you're already doing for actionlint (or could do — see other comment) and makes the failure message more actionable:

Suggested change
cpflow_cmd=(cpflow)
if [[ $# -gt 0 ]]; then
cpflow_cmd=("$@")
cpflow_cmd=(cpflow)
if [[ $# -gt 0 ]]; then
cpflow_cmd=("$@")
elif ! command -v cpflow &>/dev/null; then
echo "cpflow not found in PATH — install it or pass the path as an argument: $0 ruby /path/to/cpflow" >&2
exit 1
fi

fi

echo "==> cpflow github-flow-readiness"
bin/conductor-exec "${cpflow_cmd[@]}" github-flow-readiness

echo "==> parse generated GitHub Actions YAML"
bin/conductor-exec ruby <<'RUBY'
require "yaml"

Dir[".github/actions/**/action.yml", ".github/workflows/*.yml"].sort.each do |path|
YAML.load_file(path, aliases: true)
puts "parsed #{path}"
end
RUBY

echo "==> check composite action input descriptions"
bin/conductor-exec ruby <<'RUBY'
require "yaml"

bad = []
Dir[".github/actions/**/action.yml"].sort.each do |path|
doc = YAML.load_file(path, aliases: true)
doc.fetch("inputs", {}).each do |name, spec|
bad << "#{path}:#{name}" if spec["description"].to_s.include?("${{")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ruby fetch returns nil for nil-valued YAML keys

Low Severity

doc.fetch("inputs", {}) only falls back to {} when the "inputs" key is entirely absent. If an action YAML file contains inputs: with no sub-keys, the key exists but maps to nil, so fetch returns nil and nil.each raises a NoMethodError. Similarly, if an individual input spec is nil (e.g. token: with no properties), spec["description"] crashes on nil. Using (doc["inputs"] || {}) and guarding spec would prevent both crashes.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit e4302c5. Configure here.

end
end

abort bad.join("\n") unless bad.empty?
puts "no action metadata descriptions contain GitHub expressions"
RUBY

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If no cpflow-*.yml files exist (e.g. in a fresh checkout or on a branch before generation), bash passes the literal glob string cpflow-*.yml to actionlint with set -euo pipefail active, causing a confusing "no such file" failure rather than a clean "nothing to lint" exit.

Consider guarding with shopt -s nullglob or an explicit check:

Suggested change
shopt -s nullglob
workflow_files=(.github/workflows/cpflow-*.yml)
if [[ ${#workflow_files[@]} -eq 0 ]]; then
echo "no cpflow workflow files found, skipping actionlint"
else
actionlint -ignore "SC2129" "${workflow_files[@]}"
fi

Comment on lines +38 to +40
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No check that actionlint is installed before invoking it. A missing binary produces command not found with no hint about where to get it. A quick guard would give a clearer developer experience:

Suggested change
puts "no action metadata descriptions contain GitHub expressions"
RUBY
echo "==> actionlint"
if ! command -v actionlint &>/dev/null; then
echo "actionlint not found — install it (https://github.com/rhysd/actionlint#installation) and rerun" >&2
exit 1
fi
actionlint -ignore "SC2129" .github/workflows/cpflow-*.yml

echo "==> actionlint"
actionlint -ignore "SC2129" .github/workflows/cpflow-*.yml
Loading