Skip to content

Migrate pipeline from CircleCI to GitHub Actions#270

Draft
phelma wants to merge 1 commit into
mainfrom
gha-migration-3
Draft

Migrate pipeline from CircleCI to GitHub Actions#270
phelma wants to merge 1 commit into
mainfrom
gha-migration-3

Conversation

@phelma

@phelma phelma commented Jul 10, 2026

Copy link
Copy Markdown

Cutover to GitHub Actions per the Variant A family plan (gem pilot).
Includes decommission — merging this PR completes the repo's migration.

  • main + pr workflows: loop-guard, check/test, prerelease, release environment gate
  • git-crypt unlock on the runner; encrypted CI GPG key moved to .github/
  • Slack notifications via rake_slack; dependabot auto-merge job
  • Rakefile provisioning swapped to rake_github secrets/environments; rake_circle_ci dropped
  • CircleCI pipeline removed: .circleci/, scripts/ci/, the CI SSH deploy
    key pair and its keys:deploy/deploy_keys provisioning

Deliberate decisions (not defects)

This cutover reproduces the CircleCI pipeline's behaviour, warts included;
fixing inherited hazards is post-migration work. In particular:

  • ./go release publishes to RubyGems before the version-bump commit is
    pushed — pre-existing ordering inside the untouched release logic.
  • Prerelease publishes on every push to main with no approval gate; only
    full releases are gated (environment: release).
  • Dependabot auto-merge accepts any update type that passes checks, and the
    merge does not trigger a release build — on CircleCI the merge commit
    carried [skip ci], so this matches. Updates ship with the next
    human-triggered release.
  • The release job pulls main at approval time, so a delayed approval
    publishes main as it stands then, not the SHA this run tested — parity with
    the old release.sh (which also pulled; prerelease.sh did not, so the
    prerelease job has no pull).
  • asdf_install@v1 is our own action (infrablocks/github-actions); we are
    happy tracking its major version tag.
  • Job scaffolding is repeated flat per job by design: the logic lives in the
    build system (./go/rake) and CI stays lean — it just triggers tasks and
    supplies secrets/context.
  • Gemfile.lock carries transitive major bumps — the unavoidable resolution
    of the targeted bundle lock --update, not scope creep.

Do not merge manually — cutover is: merge this PR, disable the CircleCI
project, delete the CircleCI deploy key from repo settings.

@phelma phelma left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Code Review: #270 - Migrate pipeline from CircleCI to GitHub Actions

Verdict: COMMENT

The cutover is plan-conformant: both workflow files match the Variant A family plan §4 verbatim, the Rakefile/gemspec/lockfile/README changes and CircleCI decommission are complete and correct, and an independent conformance check found no violations in either direction (git grep -i circleci clean; lockfile carries rake_git_crypt 0.4.0 / rake_github 0.17.0 / rake_slack 0.3.0; the GPG key move is a pure rename; no stray files). No in-scope defects were found. All surviving findings are plan concerns — observations against decisions the plan/PR description documents as deliberate — recorded so a human can revisit the plan; none should block this diff.

False positives (verified during review, not counted)

  • queue: max concurrency key (flagged by safety/correctness/standards lenses): verified valid — GitHub added queue: max to the concurrency block in May 2026 (changelog 2026-05-07); the plan's exact form (cancel-in-progress: false + queue: max) is the supported combination, keeping up to 100 queued runs FIFO.
  • Dependabot PR runs receive no SLACK_BOT_TOKEN: disproven by plan D2 — the token is provisioned org-wide into both the Actions and Dependabot secrets stores (issue 19), so dependabot-triggered runs resolve it and notifications fire.

Strengths

  • ✅ Least-privilege permissions with narrow job-level escalation; commit messages passed via env vars (no expression injection); release gated behind the release environment with reviewers provisioned as code.
  • ✅ Skip semantics propagate correctly via needs; dependabot merge guard fails safe; GITHUB_TOKEN pushes cannot recurse the pipeline.
  • ✅ Net simplification: 183 lines of anchored CircleCI YAML + 13 shell scripts replaced by two flat, readable workflows with rationale comments; Slack routing as a flat ordered rule table.
  • ✅ Decommission thorough; departures from the rake_slack reference are exactly the plan-authorised ones, not drift.

General Findings (all plan concerns — post-migration backlog)

  • 🟡 Security: the committed encrypted CI GPG key uses openssl -md sha1 (single-round SHA-1 KDF); re-encrypt with -pbkdf2 at next rotation or move to a native secret (inline comment).
  • 🔵 Safety: no timeout-minutes on main.yaml jobs; a hung publish holds the serialised main queue up to 6 h.
  • 🔵 Security: mutable action tags (checkout@v4, asdf_install@v1 — D6); SHA-pinning is post-migration hardening.
  • 🔵 Security: merge gate uses github.actor (plan's exact YAML, D3); github.event.pull_request.user.login is the stronger signal.
  • 🔵 Security: ENCRYPTION_PASSPHRASE is a repo-wide secret used in the ungated prerelease job (documented parity); consider environment-scoping / RubyGems trusted publishing later.
  • 🔵 Safety: release pulls main at approval time before unlocking git-crypt — untested post-approval commits run with unlocked secrets (documented parity).
  • 🔵 Safety: cutover runbook lacks a verify-then-disable step — confirm one full green main run (incl. prerelease publish) before disabling the CircleCI project.
  • 🔵 Correctness: skip-ci-check loop-guard is belt-and-braces per D4; omits native [skip actions]/[actions skip] markers.
  • 🔵 Correctness: residual publish-before-push race vs external pushes to main — pre-existing, D5.
  • 🔵 Code Quality: credential bootstrap duplicated inline across prerelease/release; five-step secrets bootstrap is a fleet-wide composite-action candidate; main.yaml has no build job (plan shape).

Review generated by /accelerator:review-pr

Comment thread README.md
openssl aes-256-cbc \
-e \
-md sha1 \
-in ./config/secrets/ci/gpg.private \

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🟡 Security (plan concern — inherited, plan says leave the openssl commands untouched)

The encrypted CI GPG private key (.github/gpg.private.enc, committed to a public repo) is produced with openssl aes-256-cbc -md sha1 — legacy EVP_BytesToKey with a single SHA-1 round and no iteration count, and the README continues to document this recipe post-migration.

Impact: single-round SHA-1 KDF makes offline brute-force of the passphrase orders of magnitude cheaper than a proper PBKDF; the ciphertext is world-readable, so passphrase strength is the only barrier.

Suggestion: post-migration — at next key rotation, re-encrypt with -pbkdf2 -iter 600000 (updating README and the git_crypt:unlock_with_encrypted_gpg_key decryption side), or move the raw key into a GitHub Actions secret and drop the committed ciphertext.

- name: Prerelease
run: ./go "version:bump[pre]" && ./go release
- name: Push release commit
run: git push && git push --tags

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🔵 Safety (plan concern — the plan's exact YAML sets no timeouts)

No job in main.yaml sets timeout-minutes, so a hung step runs for GitHub's 360-minute default. All main runs share the serialised main concurrency group, so one hung prerelease/release job stalls every queued run behind it for up to 6 hours.

Suggestion: consider a modest timeout-minutes (e.g. 20–30) per job — possibly as a plan-level amendment so the fleet stays consistent.

runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- name: Install tools

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🔵 Security (plan concern — tracking asdf_install@v1 is documented as deliberate, D6)

Actions are referenced by mutable tags (actions/checkout@v4, infrablocks/github-actions/asdf_install@v1). Tags are repointable: compromise of either repo lets an attacker run code in jobs holding unlocked RubyGems credentials and contents: write — a supply-chain path to publishing a malicious ruby_terraform.

Suggestion: post-migration hardening — pin to full commit SHAs with a tag comment and let dependabot manage bumps.

Comment thread .github/workflows/pr.yaml
merge-pull-request:
needs: [check, test, build]
runs-on: ubuntu-latest
if: github.actor == 'dependabot[bot]'

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🔵 Security (plan concern — github.actor == 'dependabot[bot]' is the plan's exact YAML, D3)

github.actor is the actor who triggered the event, not the PR author — it changes when a human re-runs the workflow, and is a weaker identity signal than the PR's author. Failure mode here is benign (a maintainer re-run disables auto-merge), but it's a known anti-pattern that can drift into exploitable territory if triggers broaden.

Suggestion: consider github.event.pull_request.user.login == 'dependabot[bot]' (or both) as a plan-level amendment.

- name: Install tools
uses: infrablocks/github-actions/asdf_install@v1
- name: Install secrets tools
run: sudo apt-get update && sudo apt-get install -y git-crypt gnupg

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🔵 Safety (plan concern — pull-at-approval-time is documented parity with the old release.sh)

Second-order effect worth recording: the git pull runs before git_crypt:unlock_with_encrypted_gpg_key and the credential copy, so commits landing between the tested SHA and approval can alter the release tooling (go, Rakefile) that then executes with unlocked secrets and publishes — and the approval window here can be much longer than a CircleCI hold.

Suggestion: post-migration — release the tested SHA (drop the pull) or fail the release if HEAD has advanced more than N commits/days past it.

run: ./go git_crypt:unlock_with_encrypted_gpg_key
env:
ENCRYPTION_PASSPHRASE: ${{ secrets.ENCRYPTION_PASSPHRASE }}
- name: Configure RubyGems credentials

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🔵 Code Quality (plan concern — this inline block is the plan's exact YAML)

The 'Configure RubyGems credentials' shell block is duplicated verbatim in the prerelease and release jobs, while sibling setup logic moved into the build system (repository:set_ci_author). A change to credentials handling must be edited in two YAML places and can't be exercised via ./go.

Suggestion: fleet-wide, consider a rake task (e.g. rubygems:configure_credentials) or a composite action in infrablocks/github-actions encapsulating the secrets bootstrap.

cancel-in-progress: false
queue: max

jobs:

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🔵 Correctness (plan concern — the loop-guard is documented belt-and-braces, D4)

GitHub natively skips push-triggered workflows for [skip ci]/[ci skip]/[no ci], and GITHUB_TOKEN pushes never trigger workflows — so this job never observes the strings it checks. If kept as belt-and-braces, note it omits the native [skip actions]/[actions skip] markers; adding them would match native semantics exactly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant