Skip to content

fix: work around legacy checks not having write on statuses#81

Merged
chris11-taylor-nttd merged 1 commit into
mainfrom
legacy-status-check-workaround
May 7, 2026
Merged

fix: work around legacy checks not having write on statuses#81
chris11-taylor-nttd merged 1 commit into
mainfrom
legacy-status-check-workaround

Conversation

@chris11-taylor-nttd
Copy link
Copy Markdown
Contributor

@chris11-taylor-nttd chris11-taylor-nttd commented May 6, 2026

Works around an issue with legacy repos trying to consume updated versions of launch-workflows and failing workflow invocation due to a missing statuses: write permission.

One option would have been to just not call these status checks at all, but if we don't, we lose out on the opportunity to put the new status checks in evaluate mode and use them to measure completeness of the migration.

A narrowly-scoped GitHub app has been created to allow these legacy workflows to publish status checks, which allows us to drop the requirement for statuses: write on the legacy workflows.

Validated this fix with a failed Dependabot branch here: https://github.com/launchbynttdata/tf-azurerm-module_primitive-container_registry_token/actions/runs/25461504514/job/74704568650?pr=44

@chris11-taylor-nttd chris11-taylor-nttd marked this pull request as ready for review May 6, 2026 21:32
Copy link
Copy Markdown
Contributor

@bryce-lynn-nttd bryce-lynn-nttd left a comment

Choose a reason for hiding this comment

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

PR Review

Overall: Well-scoped fix for a real problem — caller-workflow permissions act as a ceiling, so legacy (un-migrated) repos calling these per-cloud workflows from jobs that don't grant statuses: write were unable to emit status checks. Adding a narrowly-scoped GitHub App token escape hatch is the right pattern, and targeting only the legacy reusable-terraform-check-aws.yml / reusable-terraform-check-azure.yml (leaving the unified reusable-terraform-check.yml alone, since its skeleton-templated callers already grant statuses: write) is the right scope.

Verifications:

  • update-status-check action declares a github_token input (line 26 of action.yml at the head SHA), so the threading is valid syntax.
  • The linked validation run on tf-azurerm-module_primitive-container_registry_token (Dependabot branch, commit "test: workflow status checks workaround") completed all 14 steps green, including Run actions/create-github-app-token@1b10c78c… (step 2) and the four status-check steps (8/10/12/14). Empirically confirmed.
  • LAUNCH_STATUS_CHECK_* is a separate App from LAUNCH_SKELETON_UPDATE_* (used in #79). Narrowly-scoped per use case — right pattern.
  • actions/create-github-app-token@1b10c78c… SHA pin matches the convention from #79.

Non-blocking

  1. Docs not updated. docs/reusable-terraform-check-aws.md and docs/reusable-terraform-check-azure.md exist but aren't touched by this PR. Operators of legacy repos won't see the new status_check_app_id input, the LAUNCH_STATUS_CHECK_KEY required secret, or the App-installation requirement. Worth a follow-up doc PR (or amending this one).

  2. Org-level config assumption isn't called out in the PR body. The default ${{ vars.LAUNCH_STATUS_CHECK_APP_ID }} and the required LAUNCH_STATUS_CHECK_KEY secret rely on org-level variable/secret being defined, plus the App needing to be installed on legacy repos. PR body confirms the App was created but doesn't explicitly confirm the org-level var/secret are provisioned and the App is installed everywhere it'll be used. If any of those are missing, callers hit a workflow-startup error rather than the fix taking effect. Worth a one-line confirmation in the PR body or a comment.

  3. LAUNCH_STATUS_CHECK_KEY is now required: true. Callers using secrets: inherit (the standard pattern) get it transparently; any caller passing secrets explicitly would need to update. Not a real concern given the standard pattern, just worth being aware of.

  4. Status check attribution change. Status checks will now be attributed to the new App rather than github-actions[bot] in the GitHub UI ("Set by [App Name]"). Cosmetic, just visible.

Generated with Claude Code (Opus 4.7)

@chris11-taylor-nttd chris11-taylor-nttd merged commit fa7957f into main May 7, 2026
4 checks passed
@chris11-taylor-nttd chris11-taylor-nttd deleted the legacy-status-check-workaround branch May 7, 2026 18:27
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.

2 participants