diff --git a/.github/workflows/apply.yml b/.github/workflows/apply.yml
index 4b619f5..7087b02 100644
--- a/.github/workflows/apply.yml
+++ b/.github/workflows/apply.yml
@@ -140,19 +140,73 @@ jobs:
exit 1
fi
cp allow_destroy_override.tf.disabled allow_destroy_override.tf
+ - name: Summarize apply target
+ env:
+ REVIEWED_SHA: ${{ needs.prepare.outputs.sha }}
+ ENVIRONMENT_REASONS: ${{ toJson(matrix.environmentReasons) }}
+ run: |
+ {
+ echo '## Apply target'
+ echo ''
+ echo "- Reviewed SHA: \`${REVIEWED_SHA}\`"
+ echo "- Workspace: \`${TF_WORKSPACE}\`"
+ echo "- Environment: \`${{ matrix.environment }}\`"
+ if [[ "$(jq 'length' <<< "${ENVIRONMENT_REASONS}")" == '0' ]]; then
+ echo "- Environment reason: no allow-destroy changes detected"
+ else
+ echo "- Environment reason:"
+ jq -r '.[] | " - " + .' <<< "${ENVIRONMENT_REASONS}"
+ fi
+ echo "- Reviewed plan artifact: \`${TF_WORKSPACE}_${REVIEWED_SHA}.tfplan\`"
+ } >> "$GITHUB_STEP_SUMMARY"
- name: Download reviewed terraform plan
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
SHA: ${{ needs.prepare.outputs.sha }}
run: gh run download -n "${TF_WORKSPACE}_${SHA}.tfplan" --repo "${GITHUB_REPOSITORY}"
+ - name: Show reviewed terraform plan
+ run: |
+ terraform show -no-color "${TF_WORKSPACE}.tfplan" > "${TF_WORKSPACE}.reviewed.txt"
+ {
+ echo '## Reviewed Terraform plan'
+ echo ''
+ echo "${TF_WORKSPACE}.tfplan
"
+ echo ''
+ echo '~~~~terraform'
+ sed 's/^~~~~/~~~~ /' "${TF_WORKSPACE}.reviewed.txt"
+ echo '~~~~'
+ echo ''
+ echo '${TF_WORKSPACE}.merged.tfplan
"
+ echo ''
+ echo '~~~~terraform'
+ sed 's/^~~~~/~~~~ /' "${TF_WORKSPACE}.merged.txt"
+ echo '~~~~'
+ echo ''
+ echo 'Access Changes<\/summary>/)
assert.doesNotMatch(comment, /Potential no members/)
- assert.match(report, /Potential no members<\/summary>/)
+ assert.match(
+ report,
+ /Post-change potential no members<\/summary>/
+ )
} finally {
if (originalPath === undefined) {
delete process.env.ACCESS_REPORT_PATH
diff --git a/scripts/__tests__/actions/classify-allow-destroy.test.ts b/scripts/__tests__/actions/classify-allow-destroy.test.ts
index 6d070a2..67969af 100644
--- a/scripts/__tests__/actions/classify-allow-destroy.test.ts
+++ b/scripts/__tests__/actions/classify-allow-destroy.test.ts
@@ -3,7 +3,9 @@ import 'reflect-metadata'
import {describe, it} from 'node:test'
import assert from 'node:assert'
import {
+ describeWorkspaceClassification,
getEnvironment,
+ getAllowDestroyReasons,
hasAllowDestroyChange,
validateRemovedMembersHaveNoDanglingAccess
} from '../../src/actions/classify-allow-destroy.js'
@@ -92,6 +94,66 @@ members:
assert.equal(allowDestroy, true)
})
+ it('describes why allow-destroy environments are selected', async () => {
+ setManagedResourceTypes(['github_repository', 'github_membership'])
+
+ const reasons = await getAllowDestroyReasons(
+ new Config(`
+members:
+ admin:
+ - kept
+repositories:
+ 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'}
+ },
+ {
+ mode: 'managed',
+ type: 'github_repository',
+ values: {name: 'kept'}
+ },
+ {
+ mode: 'managed',
+ type: 'github_repository',
+ values: {name: 'removed'}
+ }
+ ]
+ }
+ }
+ })
+ )
+ const summary = describeWorkspaceClassification({
+ include: [
+ {
+ workspace: 'default',
+ environment: 'read-allow-destroy',
+ environmentReasons: reasons
+ }
+ ]
+ })
+
+ assert.deepEqual(reasons, [
+ 'removes organization member removed',
+ 'removes repository removed'
+ ])
+ assert.match(summary, /Workspace classification/)
+ assert.match(summary, /read-allow-destroy/)
+ assert.match(summary, /removes organization member removed/)
+ assert.match(summary, /removes repository removed/)
+ })
+
it('keeps repository and membership updates in normal environments', async () => {
setManagedResourceTypes(['github_repository', 'github_membership'])
diff --git a/scripts/__tests__/workflows.test.ts b/scripts/__tests__/workflows.test.ts
index 993d53e..ad24070 100644
--- a/scripts/__tests__/workflows.test.ts
+++ b/scripts/__tests__/workflows.test.ts
@@ -85,4 +85,89 @@ describe('workflows', () => {
assert.equal(downloadYamlStep.with?.['merge-multiple'], true)
assert.equal(copyYamlStep.run, 'cp artifacts/*.yml head/github')
})
+
+ it('publishes planned terraform targets and rendered plan summaries', () => {
+ const plan = workflow('plan.yml')
+ const planSteps = plan.jobs.plan.steps
+ const commentSteps = plan.jobs.comment.steps
+ const targetStep = planSteps.find(
+ step => step.name === 'Summarize plan target'
+ )
+ const publishStep = commentSteps.find(
+ step => step.name === 'Publish terraform plans summary'
+ )
+ const uploadStep = commentSteps.find(
+ step => step.name === 'Upload terraform plans summary'
+ )
+
+ assert.ok(targetStep)
+ assert.equal(
+ targetStep.env?.ENVIRONMENT_REASONS,
+ '${{ toJson(matrix.environmentReasons) }}'
+ )
+ assert.match(targetStep.run ?? '', /## Plan target/)
+ assert.match(targetStep.run ?? '', /Pull request/)
+ assert.match(targetStep.run ?? '', /Source SHA/)
+ assert.match(targetStep.run ?? '', /Environment reason/)
+ assert.match(targetStep.run ?? '', /Terraform plan artifact/)
+ assert.ok(publishStep)
+ assert.equal(
+ publishStep.run,
+ 'cat TERRAFORM_PLANS.md >> "$GITHUB_STEP_SUMMARY"'
+ )
+ assert.ok(uploadStep)
+ assert.equal(
+ uploadStep.with?.name,
+ 'terraform-plans-${{ github.event.pull_request.head.sha || github.sha }}'
+ )
+ assert.equal(uploadStep.with?.path, 'terraform/TERRAFORM_PLANS.md')
+ })
+
+ it('publishes apply targets and reviewed plan summaries', () => {
+ const apply = workflow('apply.yml')
+ const steps = apply.jobs.apply.steps
+ const targetStep = steps.find(
+ step => step.name === 'Summarize apply target'
+ )
+ const reviewedStep = steps.find(
+ step => step.name === 'Show reviewed terraform plan'
+ )
+ const mergedStep = steps.find(
+ step => step.name === 'Show merged terraform plan'
+ )
+ const uploadStep = steps.find(
+ step => step.name === 'Upload apply plan summaries'
+ )
+ const compareStep = steps.find(
+ step => step.name === 'Compare reviewed and merged plans'
+ )
+
+ assert.ok(targetStep)
+ assert.equal(
+ targetStep.env?.ENVIRONMENT_REASONS,
+ '${{ toJson(matrix.environmentReasons) }}'
+ )
+ assert.match(targetStep.run ?? '', /## Apply target/)
+ assert.match(targetStep.run ?? '', /Reviewed SHA/)
+ assert.match(targetStep.run ?? '', /Environment reason/)
+ assert.match(targetStep.run ?? '', /Reviewed plan artifact/)
+ assert.ok(reviewedStep)
+ assert.match(reviewedStep.run ?? '', /## Reviewed Terraform plan/)
+ assert.match(reviewedStep.run ?? '', /\.reviewed\.txt/)
+ assert.ok(mergedStep)
+ assert.match(mergedStep.run ?? '', /## Merged Terraform plan/)
+ assert.match(mergedStep.run ?? '', /\.merged\.txt/)
+ assert.ok(uploadStep)
+ assert.equal(
+ uploadStep.with?.name,
+ 'apply-plans-${{ env.TF_WORKSPACE }}-${{ needs.prepare.outputs.sha }}'
+ )
+ assert.match(String(uploadStep.with?.path), /\.reviewed\.txt/)
+ assert.match(String(uploadStep.with?.path), /\.merged\.txt/)
+ assert.ok(compareStep)
+ assert.equal(
+ compareStep.run,
+ 'diff -u "${TF_WORKSPACE}.reviewed.txt" "${TF_WORKSPACE}.merged.txt"\n'
+ )
+ })
})
diff --git a/scripts/src/actions/classify-allow-destroy.ts b/scripts/src/actions/classify-allow-destroy.ts
index a4828c5..bea8b79 100644
--- a/scripts/src/actions/classify-allow-destroy.ts
+++ b/scripts/src/actions/classify-allow-destroy.ts
@@ -2,6 +2,7 @@ import 'reflect-metadata'
import * as core from '@actions/core'
import {pathToFileURL} from 'url'
+import * as fs from 'fs'
import {Config} from '../yaml/config.js'
import {State} from '../terraform/state.js'
import {
@@ -25,6 +26,7 @@ type Matrix = {
include: {
workspace: string
environment: string
+ environmentReasons: string[]
}[]
}
@@ -32,34 +34,58 @@ function getStateAddress(resource: Resource): string {
return resource.getStateAddress().toLowerCase()
}
-function hasMissingResources(
+function formatAllowDestroyReason(resource: Resource): string {
+ if (resource instanceof Member) {
+ return `removes organization member ${resource.username.toLowerCase()}`
+ }
+
+ if (resource instanceof Repository) {
+ return `removes repository ${resource.name.toLowerCase()}`
+ }
+
+ return `removes ${resource.getStateAddress().toLowerCase()}`
+}
+
+function getMissingResources(
config: Config,
state: State,
resourceClass: ResourceConstructor
-): boolean {
+): T[] {
const desiredAddresses = new Set(
config.getResources(resourceClass).map(getStateAddress)
)
return state
.getResources(resourceClass)
- .some(resource => !desiredAddresses.has(getStateAddress(resource)))
+ .filter(resource => !desiredAddresses.has(getStateAddress(resource)))
}
-export async function hasAllowDestroyChange(
+export async function getAllowDestroyReasons(
config: Config,
state: State
-): Promise {
+): Promise {
+ const reasons = []
+
for (const resourceClass of ALLOW_DESTROY_RESOURCE_CLASSES) {
if (
ResourceConstructors.includes(resourceClass) &&
- !(await state.isIgnored(resourceClass)) &&
- hasMissingResources(config, state, resourceClass)
+ !(await state.isIgnored(resourceClass))
) {
- return true
+ reasons.push(
+ ...getMissingResources(config, state, resourceClass).map(
+ formatAllowDestroyReason
+ )
+ )
}
}
- return false
+ return reasons.sort()
+}
+
+export async function hasAllowDestroyChange(
+ config: Config,
+ state: State
+): Promise {
+ return (await getAllowDestroyReasons(config, state)).length > 0
}
export async function validateRemovedMembersHaveNoDanglingAccess(
@@ -140,6 +166,32 @@ export function getEnvironment(mode: Mode, allowDestroy: boolean): string {
return allowDestroy ? `${mode}-allow-destroy` : mode
}
+export function describeWorkspaceClassification(matrix: Matrix): string {
+ const lines = [
+ '## Workspace classification',
+ '',
+ '| Workspace | Environment | Reason |',
+ '| --- | --- | --- |'
+ ]
+
+ for (const item of matrix.include) {
+ const reasons =
+ item.environmentReasons.length === 0
+ ? 'No allow-destroy changes detected.'
+ : item.environmentReasons.join('
')
+ lines.push(`| ${item.workspace} | ${item.environment} | ${reasons} |`)
+ }
+
+ return lines.join('\n')
+}
+
+function writeStepSummary(markdown: string): void {
+ const summaryPath = process.env.GITHUB_STEP_SUMMARY
+ if (summaryPath !== undefined) {
+ fs.appendFileSync(summaryPath, `${markdown}\n`)
+ }
+}
+
export async function classifyWorkspaces({
mode,
workspaces,
@@ -158,10 +210,13 @@ export async function classifyWorkspaces({
const config = Config.FromPath(`${githubDir}/${workspace}.yml`)
const state = await State.New()
await validateRemovedMembersHaveNoDanglingAccess(config, state)
- const allowDestroy = await hasAllowDestroyChange(config, state)
- const environment = getEnvironment(mode, allowDestroy)
+ const environmentReasons = await getAllowDestroyReasons(config, state)
+ const environment = getEnvironment(mode, environmentReasons.length > 0)
core.info(`${workspace}: ${environment}`)
- include.push({workspace, environment})
+ for (const reason of environmentReasons) {
+ core.info(`- ${reason}`)
+ }
+ include.push({workspace, environment, environmentReasons})
}
} finally {
if (originalWorkspace === undefined) {
@@ -191,6 +246,7 @@ async function run(): Promise {
githubDir: process.env.GITHUB_DIR ?? '../github'
})
+ writeStepSummary(describeWorkspaceClassification(matrix))
core.setOutput('matrix', JSON.stringify(matrix))
}
diff --git a/scripts/src/actions/shared/describe-access-changes.ts b/scripts/src/actions/shared/describe-access-changes.ts
index 3a163d7..ea3093c 100644
--- a/scripts/src/actions/shared/describe-access-changes.ts
+++ b/scripts/src/actions/shared/describe-access-changes.ts
@@ -85,26 +85,28 @@ export function describeAccessReport(state: State, config: Config): string {
'',
'