Skip to content

TRU-342: Migrate CI/CD from Azure Pipelines to GitHub Actions - #34

Merged
jtcrde merged 5 commits into
masterfrom
feature/TRU-342
Aug 6, 2026
Merged

TRU-342: Migrate CI/CD from Azure Pipelines to GitHub Actions#34
jtcrde merged 5 commits into
masterfrom
feature/TRU-342

Conversation

@jtcrde

@jtcrde jtcrde commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Track A of TRU-342: replaces the Azure Pipelines definition with consumers of quantum-sec/actions under the split pin policy — validation (secret-free) rides the floating @v1 major tag; the release and auto-bump workflows (secret-bearing) are exact-pinned @v1.4.0.

Changes

  • ci.yaml: validation (terraform-module-validation@v1), release (semantic-release@v1.4.0), auto-bump (update-source-reference@v1.4.0)
  • .releaserc.json + package.json/lockfile: @quantum-sec@cncsc release stack (semantic-release 21)
  • azure-pipelines.yml deleted in the same commit (the AzDO agent's global npm-link of the old @quantum-sec semantic-release v19 conflicts with the local v21 stack — they cannot coexist)

@jtcrde
jtcrde requested a review from a team as a code owner August 6, 2026 02:22
@jtcrde

jtcrde commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

Supersedes #33 (May's self-contained migration attempt) — this version consumes the central quantum-sec/actions reusables instead of stamping full workflows, matching the fleet pattern. #33 can be closed.

zhianchow
zhianchow previously approved these changes Aug 6, 2026

@zhianchow zhianchow left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Approving — checked the new ci.yaml against quantum-sec/actions@v1.4.0 and against what the deleted azure-pipelines.yml actually did. No functional gaps; notes below are non-blocking.

Verified

  • Interfaces line up: terraform-module-validation.yaml takes no secrets and defaults to .terraform-version; semantic-release.yaml requires GIT_TOKEN_BASIC and runs npm ci (hence the lockfile); update-source-reference.yaml requires package_name plus exactly the four secrets passed here.
  • devDependencies are identical to the package-pki set that already released successfully, all three are on public npm, and package.json keeps "private": true — no accidental publish path.
  • Existing tags here are bare semver (latest 1.7.3), which satisfies the strict-semver guard in update-source-reference.yaml.
  • ?ref= pins for package-log-analytics do exist in the default target (infrastructure-modules, default branch master), in the ?ref=X.Y.Z" form the sed expects — so the auto-bump job has something to rewrite rather than failing on an empty commit. Chain proven upstream: package-pki 1.1.0 → Update package-pki references to 1.1.0 (minor) (dd14fde).
  • Parity with the old pipeline: pre-commit ✅, Checkov soft-fail ✅, semantic-release gated to master ✅, infrastructure-modules auto-bump ✅. Terratest is not a loss — the old definition set runFunctionalTests: false. No .gitmodules here, so dropping the submodule-init step is fine. Validating PRs is a net gain over the old pr: none.

Non-blocking

  1. The description says @v1.3.0, but the file pins @v1 for validation and @v1.4.0 for the two secret-bearing workflows. The floating major tag is the deliberate split-pin policy from quantum-sec/actions#6 — worth stating that in the body rather than describing it as "version-pinned … @v1.3.0", since the floating pin is the part a reviewer would want to weigh.
  2. push: feature/*, fix/* combined with pull_request: master, main means every commit on a branch with an open PR validates twice. This PR has done exactly that — all three commits produced a push run and a pull_request run of the full suite. The old definition avoided it with pr: none. Dropping feature/*/fix/* from the push trigger reclaims the duplicate half.
  3. No concurrency: stanza, where the old trigger had batch: true. Two pushes to master in quick succession could run two semantic-release jobs at once. Low risk at this repo's traffic, but it's a one-stanza guard.

zhianchow
zhianchow previously approved these changes Aug 6, 2026

@zhianchow zhianchow left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Re-approving after the CI hygiene commit. Both non-blocking notes from my earlier review are addressed, and I've confirmed the effect rather than just the diff: this PR's head now produces a single pull_request run instead of the push/pull_request pair, with all checks green.

  • Feature-branch push triggers dropped — validation runs once per commit via pull_request, and pushes to master/main are unchanged, so the release and auto-bump gating is untouched.
  • concurrency per workflow+ref added with cancel-in-progress: false, the batch: true equivalent.

One carry-over nit, no need to hold the merge: the comment's "superseded runs are not cancelled" isn't what GitHub does — cancel-in-progress: false protects the in-progress run, but a previously pending run in the group is cancelled when a newer one queues. Three quick master pushes run the first, drop the second, run the third. No practical consequence here (semantic-release is cumulative, so the surviving run covers the dropped commit), but the sentence now appears in eleven files across the fleet and reads as a guarantee that no run is ever dropped.

@zhianchow zhianchow left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Re-approving after the concurrency-comment correction. Comment-only — one file, 5+/3-, no change to any YAML key — and the new wording is accurate: cancel-in-progress: false protects the in-progress run, and a superseded pending run is replaced rather than queued.

I also checked that the stated rationale actually holds for this repo, rather than taking it as boilerplate: validation here runs pre-commit run --all-files over the full tree, and releases are cumulative, so a surviving run genuinely does cover a replaced one's content. Checks green on the new head.

@jtcrde
jtcrde merged commit 8dca49c into master Aug 6, 2026
4 checks passed
@jtcrde
jtcrde deleted the feature/TRU-342 branch August 6, 2026 08:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants