From 8e36bb959444c28ebd43766c102c2042fec35d2a Mon Sep 17 00:00:00 2001 From: Piotr Galar Date: Thu, 16 Jul 2026 18:18:03 +0100 Subject: [PATCH] Guard repo and membership deletes --- .github/workflows/apply.yml | 59 +++++- .github/workflows/plan.yml | 65 ++++++- .gitignore | 3 + CHANGELOG.md | 1 + docs/ABOUT.md | 2 + docs/SETUP.md | 7 +- .../actions/classify-allow-destroy.test.ts | 181 ++++++++++++++++++ scripts/src/actions/classify-allow-destroy.ts | 120 ++++++++++++ terraform/allow_destroy_override.tf.disabled | 11 ++ 9 files changed, 438 insertions(+), 11 deletions(-) create mode 100644 scripts/__tests__/actions/classify-allow-destroy.test.ts create mode 100644 scripts/src/actions/classify-allow-destroy.ts create mode 100644 terraform/allow_destroy_override.tf.disabled diff --git a/.github/workflows/apply.yml b/.github/workflows/apply.yml index 6dcaca9..21b3665 100644 --- a/.github/workflows/apply.yml +++ b/.github/workflows/apply.yml @@ -47,19 +47,65 @@ jobs: GITHUB_APP_PEM_FILE: ${{ secrets.RO_GITHUB_APP_PEM_FILE }} run: node lib/actions/find-sha-for-plan.js working-directory: scripts - apply: + classify: needs: [prepare] if: needs.prepare.outputs.sha != '' && needs.prepare.outputs.workspaces != '' + permissions: + contents: read + name: Classify + runs-on: ubuntu-latest + environment: read + outputs: + matrix: ${{ steps.classify.outputs.matrix }} + env: + TF_IN_AUTOMATION: 1 + TF_INPUT: 0 + AWS_ACCESS_KEY_ID: ${{ secrets.RO_AWS_ACCESS_KEY_ID }} + AWS_SECRET_ACCESS_KEY: ${{ secrets.RO_AWS_SECRET_ACCESS_KEY }} + WORKSPACES: ${{ needs.prepare.outputs.workspaces }} + defaults: + run: + shell: bash + steps: + - name: Checkout + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + - name: Setup terraform + uses: hashicorp/setup-terraform@5e8dbf3c6d9deaf4193ca7a8fb23f2ac83bb6c85 # v4.0.0 + with: + terraform_version: 1.12.0 + terraform_wrapper: false + - name: Initialize terraform + run: terraform init + working-directory: terraform + - name: Install pnpm + uses: pnpm/action-setup@91ab88e2619ed1f46221f0ba42d1492c02baf788 # v6.0.6 + with: + version: 10 + - name: Use Node.js lts/* + uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 + with: + node-version: lts/* + cache: '' + - run: pnpm install --frozen-lockfile && pnpm run build + working-directory: scripts + - name: Classify workspaces + id: classify + env: + MODE: write + run: node lib/actions/classify-allow-destroy.js + working-directory: scripts + apply: + needs: [prepare, classify] + if: needs.prepare.outputs.sha != '' && needs.prepare.outputs.workspaces != '' permissions: actions: read contents: read strategy: fail-fast: false - matrix: - workspace: ${{ fromJson(needs.prepare.outputs.workspaces) }} + matrix: ${{ fromJson(needs.classify.outputs.matrix) }} name: Apply runs-on: ubuntu-latest - environment: write + environment: ${{ matrix.environment }} env: TF_IN_AUTOMATION: 1 TF_INPUT: 0 @@ -84,6 +130,9 @@ jobs: terraform_wrapper: false - name: Initialize terraform run: terraform init + - name: Allow destroy in guarded environment + if: matrix.environment == 'write-allow-destroy' + run: cp allow_destroy_override.tf.disabled allow_destroy_override.tf - name: Download reviewed terraform plan env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} @@ -91,7 +140,7 @@ jobs: run: gh run download -n "${TF_WORKSPACE}_${SHA}.tfplan" --repo "${GITHUB_REPOSITORY}" - name: Replan merged commit run: | - terraform show -json > $TF_WORKSPACE.tfstate.json + terraform show -json > "$TF_WORKSPACE.tfstate.json" terraform plan -refresh=false -lock=false -out="${TF_WORKSPACE}.merged.tfplan" -no-color - name: Compare reviewed and merged plans run: | diff --git a/.github/workflows/plan.yml b/.github/workflows/plan.yml index 8b95d94..3dae02f 100644 --- a/.github/workflows/plan.yml +++ b/.github/workflows/plan.yml @@ -47,18 +47,69 @@ jobs: env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} timeout-minutes: 10 - plan: + classify: needs: [prepare] + permissions: + contents: read + pull-requests: read + name: Classify + runs-on: ubuntu-latest + environment: read + outputs: + matrix: ${{ steps.classify.outputs.matrix }} + env: + TF_IN_AUTOMATION: 1 + TF_INPUT: 0 + AWS_ACCESS_KEY_ID: ${{ secrets.RO_AWS_ACCESS_KEY_ID }} + AWS_SECRET_ACCESS_KEY: ${{ secrets.RO_AWS_SECRET_ACCESS_KEY }} + WORKSPACES: ${{ needs.prepare.outputs.workspaces }} + steps: + - name: Checkout + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + - if: github.event_name == 'pull_request_target' + env: + NUMBER: ${{ github.event.pull_request.number }} + SHA: ${{ github.event.pull_request.head.sha }} + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + git fetch origin "pull/${NUMBER}/head" + rm -rf github && git checkout "${SHA}" -- github + - name: Setup terraform + uses: hashicorp/setup-terraform@5e8dbf3c6d9deaf4193ca7a8fb23f2ac83bb6c85 # v4.0.0 + with: + terraform_version: 1.12.0 + terraform_wrapper: false + - name: Initialize terraform + run: terraform init + working-directory: terraform + - name: Install pnpm + uses: pnpm/action-setup@91ab88e2619ed1f46221f0ba42d1492c02baf788 # v6.0.6 + with: + version: 10 + - name: Use Node.js lts/* + uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 + with: + node-version: lts/* + cache: '' + - run: pnpm install --frozen-lockfile && pnpm run build + working-directory: scripts + - name: Classify workspaces + id: classify + env: + MODE: read + run: node lib/actions/classify-allow-destroy.js + working-directory: scripts + plan: + needs: [prepare, classify] permissions: contents: read pull-requests: read strategy: fail-fast: false - matrix: - workspace: ${{ fromJson(needs.prepare.outputs.workspaces || '[]') }} + matrix: ${{ fromJson(needs.classify.outputs.matrix) }} name: Plan runs-on: ubuntu-latest - environment: read + environment: ${{ matrix.environment }} env: TF_IN_AUTOMATION: 1 TF_INPUT: 0 @@ -88,9 +139,13 @@ jobs: - name: Initialize terraform run: terraform init working-directory: terraform + - name: Allow destroy in guarded environment + if: matrix.environment == 'read-allow-destroy' + run: cp allow_destroy_override.tf.disabled allow_destroy_override.tf + working-directory: terraform - name: Plan terraform run: | - terraform show -json > $TF_WORKSPACE.tfstate.json + terraform show -json > "$TF_WORKSPACE.tfstate.json" terraform plan -refresh=false -lock=false -out="${TF_WORKSPACE}.tfplan" -no-color working-directory: terraform - name: Upload terraform plan diff --git a/.gitignore b/.gitignore index 34adc7e..356c12f 100644 --- a/.gitignore +++ b/.gitignore @@ -28,3 +28,6 @@ crash.log # Include tfplan files to ignore the plan output of command: terraform plan -out=tfplan # example: *tfplan* *.tfplan + +# Enabled only by guarded allow-destroy workflow jobs +terraform/allow_destroy_override.tf diff --git a/CHANGELOG.md b/CHANGELOG.md index 5549e1d..59bfc4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Added +- allow-destroy workflow environments for guarded repository and membership deletion plans/applies - shared action for adding a collaborator to all repositories - clean workflow which removes resources from state - information on how to handle private GitHub Management repository diff --git a/docs/ABOUT.md b/docs/ABOUT.md index c7a44ef..d442054 100644 --- a/docs/ABOUT.md +++ b/docs/ABOUT.md @@ -27,6 +27,8 @@ The workflow for introducing changes to GitHub via YAML configuration file is as 1. Review the plan. 1. Merge the PR and wait for the GitHub Action workflow triggered on pushes to the default branch to apply it. +Plans that remove managed repositories or organization memberships are routed through the `read-allow-destroy` GitHub Actions environment before the PR plan is created. The matching apply is routed through `write-allow-destroy`. These environments must be protected separately from the normal `read` and `write` environments. Outside the allow-destroy environments, Terraform keeps `prevent_destroy` enabled for repository and membership resources. + Neither creating the terraform plan nor applying it refreshes the underlying terraform state i.e. going through this workflow does **NOT** ask GitHub if the actual GitHub configuration state has changed. This makes the workflow fast and rate limit friendly because the number of requests to GitHub is minimised. This can result in the plan failing to be applied, e.g. if the underlying resource has been deleted. This assumes that YAML configuration is the main source of truth for GitHub configuration state. The plans that are created during the PR GitHub Action workflow are compared against plans regenerated from the merged commit before applying. The workflow for synchronising the current GitHub configuration state with YAML configuration file is as follows: diff --git a/docs/SETUP.md b/docs/SETUP.md index 85cc586..32cbe5f 100644 --- a/docs/SETUP.md +++ b/docs/SETUP.md @@ -116,7 +116,12 @@ ## GitHub Actions Environments and Secrets -- [ ] Create GitHub Actions environments named `read`, `write`, and `push`, and configure protection rules such as required reviewers. Workflows that read organization state reference `read`; workflows that write organization state reference `write`; workflows that push generated changes to the GitHub Management repository reference `push`. +- [ ] Create GitHub Actions environments named `read`, `read-allow-destroy`, `write`, `write-allow-destroy`, and `push`, and configure protection rules such as required reviewers. Workflows that read organization state reference `read`; workflows that write organization state reference `write`; workflows that push generated changes to the GitHub Management repository reference `push`. +- [ ] Configure `read-allow-destroy` and `write-allow-destroy` with stricter protection rules for repository and membership deletion plans/applies: + - [ ] Require reviewers + - [ ] Prevent self-review + - [ ] Restrict deployment branches to `master` + - [ ] Disable administrator bypass where available - [ ] [Create encrypted secrets](https://docs.github.com/en/actions/security-guides/encrypted-secrets#creating-encrypted-secrets-for-an-organization) for the GitHub organization and allow the repository to access them (\*replace `$GITHUB_ORGANIZATION_NAME` with the GitHub organization name) - *these secrets are read by the GitHub Action workflows* - [ ] Go to `https://github.com/organizations/$GITHUB_ORGANIZATION_NAME/settings/apps/$GITHUB_APP_NAME` and copy the `App ID` - [ ] `RO_GITHUB_APP_ID` diff --git a/scripts/__tests__/actions/classify-allow-destroy.test.ts b/scripts/__tests__/actions/classify-allow-destroy.test.ts new file mode 100644 index 0000000..974911b --- /dev/null +++ b/scripts/__tests__/actions/classify-allow-destroy.test.ts @@ -0,0 +1,181 @@ +import 'reflect-metadata' + +import {describe, it} from 'node:test' +import assert from 'node:assert' +import { + getEnvironment, + hasAllowDestroyChange +} from '../../src/actions/classify-allow-destroy.js' +import {Config} from '../../src/yaml/config.js' +import {State} from '../../src/terraform/state.js' +import {Locals} from '../../src/terraform/locals.js' + +function setManagedResourceTypes(resourceTypes: string[]): void { + Locals.locals = { + resource_types: resourceTypes, + ignore: { + repositories: [], + teams: [], + users: [] + } + } +} + +function state(source: object): State { + return new State(JSON.stringify(source)) +} + +describe('allow destroy classification', () => { + it('routes repository deletes to allow-destroy environments', async () => { + setManagedResourceTypes(['github_repository', 'github_membership']) + + const allowDestroy = await hasAllowDestroyChange( + new Config(` +repositories: + kept: {} +`), + state({ + values: { + root_module: { + resources: [ + { + mode: 'managed', + type: 'github_repository', + values: {name: 'kept'} + }, + { + mode: 'managed', + type: 'github_repository', + values: {name: 'removed'} + } + ] + } + } + }) + ) + + assert.equal(allowDestroy, true) + assert.equal(getEnvironment('read', allowDestroy), 'read-allow-destroy') + assert.equal(getEnvironment('write', allowDestroy), 'write-allow-destroy') + }) + + it('routes membership deletes to allow-destroy environments', async () => { + setManagedResourceTypes(['github_repository', 'github_membership']) + + const allowDestroy = await hasAllowDestroyChange( + new Config(` +members: + admin: + - kept +`), + state({ + values: { + root_module: { + resources: [ + { + mode: 'managed', + type: 'github_membership', + values: {username: 'kept', role: 'admin'} + }, + { + mode: 'managed', + type: 'github_membership', + values: {username: 'removed', role: 'admin'} + } + ] + } + } + }) + ) + + assert.equal(allowDestroy, true) + }) + + it('keeps repository and membership updates in normal environments', async () => { + setManagedResourceTypes(['github_repository', 'github_membership']) + + const allowDestroy = await hasAllowDestroyChange( + new Config(` +members: + member: + - octocat +repositories: + github: + description: updated +`), + state({ + values: { + root_module: { + resources: [ + { + mode: 'managed', + type: 'github_repository', + values: {name: 'github', description: 'old'} + }, + { + mode: 'managed', + type: 'github_membership', + values: {username: 'octocat', role: 'member'} + } + ] + } + } + }) + ) + + assert.equal(allowDestroy, false) + assert.equal(getEnvironment('read', allowDestroy), 'read') + assert.equal(getEnvironment('write', allowDestroy), 'write') + }) + + it('ignores deletes for other resource types', async () => { + setManagedResourceTypes(['github_team']) + + const allowDestroy = await hasAllowDestroyChange( + new Config('{}'), + state({ + values: { + root_module: { + resources: [ + { + mode: 'managed', + type: 'github_team', + values: {name: 'removed'} + } + ] + } + } + }) + ) + + assert.equal(allowDestroy, false) + }) + + it('ignores repository and membership types that are not managed', async () => { + setManagedResourceTypes([]) + + const allowDestroy = await hasAllowDestroyChange( + new Config('{}'), + state({ + values: { + root_module: { + resources: [ + { + mode: 'managed', + type: 'github_repository', + values: {name: 'removed'} + }, + { + mode: 'managed', + type: 'github_membership', + values: {username: 'removed', role: 'member'} + } + ] + } + } + }) + ) + + assert.equal(allowDestroy, false) + }) +}) diff --git a/scripts/src/actions/classify-allow-destroy.ts b/scripts/src/actions/classify-allow-destroy.ts new file mode 100644 index 0000000..22fdc74 --- /dev/null +++ b/scripts/src/actions/classify-allow-destroy.ts @@ -0,0 +1,120 @@ +import * as core from '@actions/core' +import {pathToFileURL} from 'url' +import {Config} from '../yaml/config.js' +import {State} from '../terraform/state.js' +import { + Resource, + ResourceConstructor, + ResourceConstructors +} from '../resources/resource.js' +import {Member} from '../resources/member.js' +import {Repository} from '../resources/repository.js' + +const ALLOW_DESTROY_RESOURCE_CLASSES: ResourceConstructor[] = [ + Member, + Repository +] + +type Mode = 'read' | 'write' + +type Matrix = { + include: { + workspace: string + environment: string + }[] +} + +function getStateAddress(resource: Resource): string { + return resource.getStateAddress().toLowerCase() +} + +function hasMissingResources( + config: Config, + state: State, + resourceClass: ResourceConstructor +): boolean { + const desiredAddresses = new Set( + config.getResources(resourceClass).map(getStateAddress) + ) + return state + .getResources(resourceClass) + .some(resource => !desiredAddresses.has(getStateAddress(resource))) +} + +export async function hasAllowDestroyChange( + config: Config, + state: State +): Promise { + for (const resourceClass of ALLOW_DESTROY_RESOURCE_CLASSES) { + if ( + ResourceConstructors.includes(resourceClass) && + !(await state.isIgnored(resourceClass)) && + hasMissingResources(config, state, resourceClass) + ) { + return true + } + } + + return false +} + +export function getEnvironment(mode: Mode, allowDestroy: boolean): string { + return allowDestroy ? `${mode}-allow-destroy` : mode +} + +export async function classifyWorkspaces({ + mode, + workspaces, + githubDir +}: { + mode: Mode + workspaces: string[] + githubDir: string +}): Promise { + const include = [] + const originalWorkspace = process.env.TF_WORKSPACE + + try { + for (const workspace of workspaces) { + process.env.TF_WORKSPACE = workspace + const config = Config.FromPath(`${githubDir}/${workspace}.yml`) + const state = await State.New() + const allowDestroy = await hasAllowDestroyChange(config, state) + const environment = getEnvironment(mode, allowDestroy) + core.info(`${workspace}: ${environment}`) + include.push({workspace, environment}) + } + } finally { + if (originalWorkspace === undefined) { + delete process.env.TF_WORKSPACE + } else { + process.env.TF_WORKSPACE = originalWorkspace + } + } + + return {include} +} + +async function run(): Promise { + const mode = (process.env.MODE ?? 'read') as Mode + if (mode !== 'read' && mode !== 'write') { + throw new Error(`MODE must be one of "read" or "write", got "${mode}"`) + } + + const workspaces = JSON.parse(process.env.WORKSPACES?.trim() || '[]') + if (!Array.isArray(workspaces)) { + throw new Error('WORKSPACES must be a JSON array') + } + + const matrix = await classifyWorkspaces({ + mode, + workspaces, + githubDir: process.env.GITHUB_DIR ?? '../github' + }) + + core.setOutput('matrix', JSON.stringify(matrix)) +} + +if (import.meta.url === pathToFileURL(process.argv[1]).href) { + run().catch(error => core.setFailed(error)) +} diff --git a/terraform/allow_destroy_override.tf.disabled b/terraform/allow_destroy_override.tf.disabled new file mode 100644 index 0000000..012e584 --- /dev/null +++ b/terraform/allow_destroy_override.tf.disabled @@ -0,0 +1,11 @@ +resource "github_membership" "this" { + lifecycle { + prevent_destroy = false + } +} + +resource "github_repository" "this" { + lifecycle { + prevent_destroy = false + } +}