From 29aec1267d8758d99d32e1cc05590ae40a8013e3 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Sat, 23 May 2026 10:17:47 -1000 Subject: [PATCH] Document cpflow upstream testing --- .../docs/testing-cpflow-github-actions.md | 78 ++++++++++++- .controlplane/readme.md | 10 ++ .github/testing-github-actions.md | 103 +++++------------- bin/pin-cpflow-github-ref | 72 ++++++++++++ bin/test-cpflow-github-flow | 47 ++++++++ 5 files changed, 232 insertions(+), 78 deletions(-) create mode 100755 bin/pin-cpflow-github-ref diff --git a/.controlplane/docs/testing-cpflow-github-actions.md b/.controlplane/docs/testing-cpflow-github-actions.md index a64a180ec..ffd373958 100644 --- a/.controlplane/docs/testing-cpflow-github-actions.md +++ b/.controlplane/docs/testing-cpflow-github-actions.md @@ -60,6 +60,73 @@ workflow change was tested before a new `cpflow` gem release existed. That is safer than pinning a branch, but it should be treated as a temporary test pin until the next upstream release tag is available. +What is tied to the upstream repo: + +- The downstream workflow wrapper hardcodes `shakacode/control-plane-flow` in + `uses:`. +- The reusable workflow file comes from the ref after `@`. +- The reusable workflow checks out `control-plane-flow` at + `control_plane_flow_ref` to load shared composite actions. +- When `CPFLOW_VERSION` is empty, the setup action builds and installs the + `cpflow` gem from that checked-out repository ref. + +What is tied to RubyGems: + +- A released `cpflow` gem is the normal source used to generate downstream + workflow wrappers. +- The `CPFLOW_VERSION` repository variable is a runtime override that runs + `gem install cpflow -v ` inside workflows. + +For stable downstream automation, prefer the release path: generate from a +released gem and pin wrappers to the matching upstream release tag. For +pre-release validation, pin to a full commit SHA from the upstream PR, never a +moving branch. + +## Testing An Unmerged Upstream PR Downstream + +You can test an upstream `control-plane-flow` PR in this downstream app before +merging upstream, without publishing a gem. Use an immutable commit SHA from the +upstream PR branch: + +1. Push the upstream PR branch and copy its head commit SHA. +2. In a downstream test branch, pin every generated wrapper ref: + + ```sh + bin/pin-cpflow-github-ref + ``` + + The helper accepts release tags and full 40-character commit SHAs by default. + It rejects branch names such as `main` or `feature/foo`; use + `--allow-moving-ref` only for short-lived local experiments that will not be + committed. The resulting diff should replace both pins in each reusable + workflow call: + + ```yaml + uses: shakacode/control-plane-flow/.github/workflows/cpflow-deploy-review-app.yml@ + with: + control_plane_flow_ref: + ``` + +3. Keep `CPFLOW_VERSION` unset unless you intentionally want to test a released + RubyGems version instead of building `cpflow` from the upstream PR SHA. +4. Run `bin/test-cpflow-github-flow`. +5. Open a downstream PR and trigger a real review app with a comment whose body + is exactly: + + ```text + +review-app-deploy + ``` + +6. Verify the deploy logs show the expected upstream commit SHA, the setup step + prints the expected `cpflow` version/source, and the review app URL returns + HTTP 200. +7. After the upstream PR merges and releases, regenerate or repin downstream to + the release tag instead of leaving the temporary commit SHA forever. + +This tests the real reusable workflow and shared composite actions from the +upstream PR. It avoids merging upstream blind while also avoiding a mutable +branch ref in downstream automation. + ## Local Checks After regenerating the flow, run these checks from the repository root. If @@ -83,7 +150,10 @@ 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`. +checks that every generated wrapper keeps `uses:` and `control_plane_flow_ref` +on the same upstream ref across all `cpflow-*` wrappers, checks that any +secret-inheriting reusable workflow passes `control_plane_flow_ref`, and runs +`actionlint -ignore 'SC2129' .github/workflows/cpflow-*.yml`. ## PR Checks @@ -168,6 +238,12 @@ Create the first review app by commenting exactly: ## Ways To Make This Easier +- Extend `bin/pin-cpflow-github-ref` so it can also run + `bin/test-cpflow-github-flow`, open a downstream PR, and print or post the + exact `+review-app-deploy` command needed to start the canary deploy. +- Add CI coverage that runs `bin/test-cpflow-github-flow` on generated workflow + changes, so ref mismatches and action metadata parsing issues are caught + before review. - 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 diff --git a/.controlplane/readme.md b/.controlplane/readme.md index 9eb511d20..fff76eebb 100644 --- a/.controlplane/readme.md +++ b/.controlplane/readme.md @@ -437,6 +437,16 @@ setup action installs that RubyGems version instead. For normal release pins, either leave it unset while using the matching `v` workflow tag, or set it to the same gem version without the leading `v`. +To test unreleased upstream workflow changes before merging `control-plane-flow`, +pin a downstream PR to the upstream PR's full commit SHA in both `uses:` and +`control_plane_flow_ref`, leave `CPFLOW_VERSION` unset, and trigger a real review +app deploy. That tests the upstream reusable workflow, shared composite actions, +and source-built `cpflow` gem from the same immutable commit. After the upstream +change is released, regenerate or repin back to the matching release tag. +Use `bin/pin-cpflow-github-ref ` to update the generated wrapper refs +together. The helper accepts release tags and full commit SHAs by default; +`--allow-moving-ref` is only for short-lived local experiments. + For this app, validate a regenerated flow with: ```bash diff --git a/.github/testing-github-actions.md b/.github/testing-github-actions.md index 3b10fedc3..0366c6412 100644 --- a/.github/testing-github-actions.md +++ b/.github/testing-github-actions.md @@ -1,85 +1,34 @@ -# Developing and Testing Github Actions +# Developing and Testing GitHub Actions -Testing Github Actions on an existing repository is tricky. - -The main issue boils down to the fact that Github Actions uses the workflow files in the branch where the event originates. This is fine for push events, but it becomes a problem when you want to test workflows that are triggered by comments on a pull request. +GitHub Actions workflow testing depends on the event type: -Here's a summary of the behavior: - -Behavior of push and pull_request Events - 1. Push on a Branch: - • When you push changes to a branch (e.g., feature-branch), GitHub Actions uses the workflow files in that same branch. - • This is why changes to workflows work seamlessly when testing with push events. - 2. Pull Request Events: - • For pull_request events (e.g., a PR from feature-branch into master), GitHub Actions will always use the workflow files from the target branch (e.g., master), not the source branch (e.g., feature-branch). - • This is a security feature to prevent someone from introducing malicious code in a PR that modifies the workflow files themselves. +- `push` runs workflow files from the pushed branch. +- `pull_request` runs workflow files from the base branch for sensitive cases. +- `issue_comment` runs workflow files from the default branch. +- `workflow_dispatch --ref ` runs the workflow file from that ref. -Impact on Comment-Triggered Workflows +This matters for review-app automation because comment-triggered commands such +as `+review-app-deploy` use default-branch workflow code. A PR that changes +workflow files or action wiring is not fully proven by commenting on that same +PR until the trusted default-branch code has those changes. -When you want to trigger workflows via comments (issue_comment) in a pull request: - • The workflow code used will always come from the master branch (or the default branch), regardless of the branch where the PR originates. - • This means the PR’s changes to the workflow won’t be used, and the action invoked by the comment will also use code from master. +For cpflow review-app, staging, and promotion workflows, use the cpflow-specific +guide: -Workarounds to Test Comment-Triggered Workflows +[Testing cpflow GitHub Actions Changes](../.controlplane/docs/testing-cpflow-github-actions.md) -If you want to test workflows in a way that uses the changes in the pull request, here are your options: +## Practical Pattern -1. Use Push Events for Testing - • Test your changes on a branch with push triggers. - • Use workflow_dispatch to simulate the events you need (like invoking actions via comments). +1. Validate generated files locally with `bin/test-cpflow-github-flow`. +2. Open a PR and let regular CI prove GitHub can parse the workflow YAML. +3. For top-level workflow-file experiments, run `workflow_dispatch --ref`. +4. For comment-triggered review-app commands, test a real `+review-app-deploy` + after the trusted default-branch wrapper points at the code under test. +5. When testing unreleased upstream `control-plane-flow` changes downstream, pin + both the reusable workflow `uses:` ref and `control_plane_flow_ref` to the + same upstream commit SHA. -This allows you to confirm that your changes to the workflow file or actions behave as expected before merging into master. - -2. Merge the Workflow to master Temporarily - -If you absolutely need the workflow to run as part of a pull_request event: - 1. Merge your workflow changes into master temporarily. - 2. Open a PR to test your comment-triggered workflows. - 3. Revert the changes in master if necessary. - -This ensures the workflow changes are active in master while still testing with the pull_request context. - -3. Add Logic to Detect the Source Branch - -Use github.event.pull_request.head.ref to add custom logic in your workflow that behaves differently based on the source branch. - • Example: - -jobs: - test-pr: - runs-on: ubuntu-latest - if: ${{ github.event.pull_request.head.ref == 'feature-branch' }} - steps: - - name: Checkout Code - uses: actions/checkout@v3 - - - name: Debug - run: echo "Testing workflow changes in feature-branch" - -However, this still requires the workflow itself to exist in master. - -4. Use a Fork or a Temporary Repo - -Create a temporary repository or a fork to test workflows in isolation: - • Push your workflow changes to master in the test repository. - • Open a PR in the fork to test how workflows behave with issue_comment events and PR contexts. - -Once confirmed, you can replicate the changes in your main repository. - -6. Alternative Approach: Split Workflows - -If your workflow includes comment-based triggers (issue_comment), consider splitting your workflows: - • A base workflow in master that handles triggering. - • A test-specific workflow for validating changes on a branch. - -For example: - 1. The base workflow triggers when a comment like /run-tests is added. - 2. The test-specific workflow runs in response to the base workflow but uses the branch’s code. - -Summary - • For push events: The branch-specific workflow is used, so testing changes is easy. - • For pull_request and issue_comment events: GitHub always uses workflows from the master branch, and there’s no direct way to bypass this. - -To test comment-triggered workflows: - 1. Use push or workflow_dispatch to validate changes. - 2. Merge workflow changes temporarily into master to test with pull_request events. - 3. Use tools like act for local simulation. +Avoid testing production automation against moving branch refs such as `main` or +a feature branch. Use release tags for normal operation and full commit SHAs for +temporary pre-release validation. `bin/pin-cpflow-github-ref` enforces that +default and requires `--allow-moving-ref` for one-off local branch experiments. diff --git a/bin/pin-cpflow-github-ref b/bin/pin-cpflow-github-ref new file mode 100755 index 000000000..41f643f3b --- /dev/null +++ b/bin/pin-cpflow-github-ref @@ -0,0 +1,72 @@ +#!/usr/bin/env ruby +# frozen_string_literal: true + +require "pathname" + +USAGE = <<~USAGE + Usage: bin/pin-cpflow-github-ref [--allow-moving-ref] + + Use a release tag for normal operation, e.g. v5.0.0. + Use a full 40-character commit SHA for temporary unreleased upstream testing. + Use --allow-moving-ref only for short-lived local branch/ref experiments. +USAGE + +ALLOWED_OPTIONS = ["--allow-moving-ref"].freeze +FULL_COMMIT_SHA = /\A[0-9a-f]{40}\z/i +RELEASE_TAG = /\Av\d+\.\d+\.\d+(?:[-.][0-9A-Za-z][0-9A-Za-z.-]*)?\z/ + +options, positional = ARGV.partition { |arg| arg.start_with?("--") } +unknown_options = options - ALLOWED_OPTIONS + +unless unknown_options.empty? + warn "Unknown option(s): #{unknown_options.join(', ')}" + warn USAGE + exit 1 +end + +unless positional.length == 1 + warn USAGE + exit 1 +end + +ref = positional.first +allow_moving_ref = options.include?("--allow-moving-ref") + +unless ref.match?(/\A[0-9A-Za-z._\/-]+\z/) + warn "Ref contains unsupported characters: #{ref.inspect}" + exit 1 +end + +unless ref.match?(FULL_COMMIT_SHA) || ref.match?(RELEASE_TAG) || allow_moving_ref + warn "Ref must be a full 40-character commit SHA or a v-prefixed release tag: #{ref.inspect}" + warn "Use --allow-moving-ref only for a short-lived branch/ref experiment." + exit 1 +end + +root = Pathname.new(__dir__).join("..").expand_path +workflow_paths = Dir[root.join(".github/workflows/cpflow-*.yml")].sort + +if workflow_paths.empty? + warn "No cpflow workflow wrappers found." + exit 1 +end + +changed = [] +workflow_paths.each do |path| + text = File.read(path) + updated = text + .gsub(%r{(uses:\s+shakacode/control-plane-flow/\.github/workflows/[^@\s]+@)[^\s]+}, "\\1#{ref}") + .gsub(/(\bcontrol_plane_flow_ref:\s*)\S+/, "\\1#{ref}") + + next if updated == text + + File.write(path, updated) + changed << Pathname.new(path).relative_path_from(root).to_s +end + +puts "Pinned cpflow GitHub ref to #{ref}" +if changed.empty? + puts "No files changed." +else + changed.each { |path| puts "updated #{path}" } +end diff --git a/bin/test-cpflow-github-flow b/bin/test-cpflow-github-flow index 593211bf6..91c8ed461 100755 --- a/bin/test-cpflow-github-flow +++ b/bin/test-cpflow-github-flow @@ -38,5 +38,52 @@ abort bad.join("\n") unless bad.empty? puts "no action metadata descriptions contain GitHub expressions" RUBY +echo "==> check cpflow reusable workflow refs" +bin/conductor-exec ruby <<'RUBY' +require "yaml" + +CONTROL_PLANE_FLOW_WORKFLOW = %r{\Ashakacode/control-plane-flow/\.github/workflows/[^@\s]+@([^\s]+)\z} + +refs = Hash.new { |hash, key| hash[key] = [] } + +Dir[".github/workflows/cpflow-*.yml"].sort.each do |path| + doc = YAML.load_file(path, aliases: true) + doc.fetch("jobs", {}).each do |job_name, job| + next unless job.is_a?(Hash) + + with = job["with"].is_a?(Hash) ? job["with"] : {} + input_ref = with["control_plane_flow_ref"] + uses_match = job["uses"].to_s.match(CONTROL_PLANE_FLOW_WORKFLOW) + + unless uses_match + abort "#{path}:#{job_name} has control_plane_flow_ref but no control-plane-flow reusable workflow" if input_ref + + next + end + + uses_ref = uses_match[1] + refs[uses_ref] << "#{path}:#{job_name}" + + if input_ref + refs[input_ref] << "#{path}:#{job_name}" + abort "#{path}:#{job_name} mismatched cpflow refs: #{uses_ref}, #{input_ref}" if uses_ref != input_ref + elsif job.key?("secrets") + abort "#{path}:#{job_name} inherits secrets but is missing control_plane_flow_ref for #{uses_ref}" + end + end +end + +if refs.empty? + puts "no upstream cpflow reusable workflow refs found" +elsif refs.length > 1 + refs.each do |ref, paths| + puts "#{ref}: #{paths.uniq.sort.join(', ')}" + end + abort "cpflow workflow wrappers use multiple upstream refs: #{refs.keys.sort.join(', ')}" +else + puts "cpflow refs: #{refs.keys.sort.join(', ')}" +end +RUBY + echo "==> actionlint" actionlint -ignore "SC2129" .github/workflows/cpflow-*.yml