diff --git a/.github/workflows/sync-automation-template.yml b/.github/workflows/sync-automation-template.yml index d1fd0a5..195935a 100644 --- a/.github/workflows/sync-automation-template.yml +++ b/.github/workflows/sync-automation-template.yml @@ -9,6 +9,10 @@ on: description: 'Report drift without opening pull requests' type: boolean default: false + only: + description: 'Limit the run to one repo, for testing' + type: string + default: '' push: branches: - main @@ -43,4 +47,9 @@ jobs: - name: Sync consumers env: GITHUB_TOKEN: ${{ steps.generate-token.outputs.token }} - run: node scripts/sync-automation-template.js ${{ inputs.dry_run && '--dry-run' || '' }} + ONLY: ${{ inputs.only }} + DRY_RUN: ${{ inputs.dry_run && 'yes' || '' }} + run: | + node scripts/sync-automation-template.js \ + ${DRY_RUN:+--dry-run} \ + ${ONLY:+--only="$ONLY"} diff --git a/docs/automation.md b/docs/automation.md index ed516b0..2ca1737 100644 --- a/docs/automation.md +++ b/docs/automation.md @@ -123,3 +123,8 @@ a Changelog section whose Description is not the placeholder its own template sh plain explanation has no such section. For that repo the sync reads its template and fills each field, so the body is not a half-filled form. Nothing about that template is stored here, so it stays current as they change it. + +## Testing a change + +See [testing the automations](./testing-automations.md). Test in `learningequality/test-actions`, +where the bot app and the secrets are already in place. diff --git a/docs/community-automations.md b/docs/community-automations.md index f18516f..3bb48da 100644 --- a/docs/community-automations.md +++ b/docs/community-automations.md @@ -73,4 +73,4 @@ Sends a holiday message to community pull requests and issue comments. In `scripts/contants.js` set: - `HOLIDAY_MESSAGE`: Message text -Before/after holidays, toggle `enabled:` for `holiday-message` in [`automation-registry.yml`](../automation-registry.yml) and regenerate (see [`docs/automation.md`](./automation.md)) - no per-repo changes needed. +Before/after holidays, toggle `enabled:` for `holiday-message` in [`automation-registry.yml`](../automation-registry.yml) and regenerate (see [the automation entry point](./automation.md)) - no per-repo changes needed. diff --git a/docs/testing-automations.md b/docs/testing-automations.md new file mode 100644 index 0000000..70d873b --- /dev/null +++ b/docs/testing-automations.md @@ -0,0 +1,69 @@ +# Testing the automations + +Test in [`learningequality/test-actions`](https://github.com/learningequality/test-actions). The bot +app is installed there and the secrets resolve, so there is nothing to set up. + +It holds a copy of `automation-template.yml` at `.github/workflows/automation.yml`, making it a +consumer like any other. Every automation runs there against the reusable workflows on `main`. + +## Testing an automation + +Trigger the event you want to test: open a pull request, comment on an issue, add a label, etc. Then +check the run in the Actions tab. + +Jobs that do not match the event are reported as skipped, which is normal. If everything is skipped, +the `if:` condition on each job did not match, so check the event first. + +### Automations that need an outside contributor + +Some automations run only when `is-contributor` is true, meaning the author is not a member of the +organization. An org member cannot trigger them, so they skip. These are `review-requested`, +`pull-request-label`, `contributor-pr-reply`, `contributor-issue-comment`, and +`update-pr-spreadsheet`, plus `holiday-message` when enabled. + +Use a second GitHub account that is not in the organization. It needs no permissions, secrets, or +app. To open a pull request, fork `test-actions` and open one back to it. `pull_request_target` then +runs in `test-actions` with `test-actions`' secrets rather than the fork's. For an issue or comment, +no fork is needed. + +Keep that account out of the organization. Adding it makes these automations skip again. + +`update-pr-spreadsheet` writes to a test sheet rather than the production sheet because +`CONTRIBUTIONS_SPREADSHEET_ID` and `CONTRIBUTIONS_SHEET_NAME` are set as repository secrets there. A +repository secret takes precedence over an organization secret with the same name. + +## Testing the sync workflow + +Run `Sync automation template` from the Actions tab with `only` set to `test-actions`. This limits +the run to one repo, so it cannot open a pull request anywhere else. Leave `dry_run` on to see what +it would do, or turn it off to let it open a pull request. + +Without `only`, the workflow considers every consumer in the organization, which is how it runs on +its schedule. + +To see it propose a change, modify `.github/workflows/automation.yml` in `test-actions` so that it +differs from the template, then run it with `only: test-actions`. + +## Testing a change to a reusable workflow + +Point a caller at your branch, for example: + +`uses: learningequality/.github/.github/workflows/automation.yml@my-branch` + +Put that caller at a path other than `.github/workflows/automation.yml`. The sync reads exactly that +path, so a branch-pinned caller there looks like drift and gets a pull request on every run. + +## Clearing up + +Close test pull requests and delete their branches. Leave `.github/workflows/automation.yml` in +place, since the repo is only a consumer while that file exists. + +If you drifted the copy to make the sync propose something, either merge the sync pull request or +revert the drift. Closing it without merging leaves test-actions out of sync, so the workflow +reports it as declined and stops proposing updates until the template changes. + +## Testing in a separate organization + +Changing the bot app, using different secret values, or testing discovery across several repos +requires an organization you control. See +[testing in a separate organization](./testing-in-a-separate-org.md). diff --git a/docs/testing-in-a-separate-org.md b/docs/testing-in-a-separate-org.md new file mode 100644 index 0000000..eb49313 --- /dev/null +++ b/docs/testing-in-a-separate-org.md @@ -0,0 +1,44 @@ +# Testing in a separate organization + +Most testing belongs in `learningequality/test-actions`, where the app and secrets are already in +place. See [testing the automations](./testing-automations.md). + +A separate organization is worth the setup when: + +* You are changing the bot app itself, its permissions, or its installation scope. Those changes + cannot be tested on the production app. +* You need different secret values, such as another Slack webhook, without changing what everyone + else testing in `test-actions` sees. +* You are changing discovery. `--only` covers one repo, while an organization lets you arrange + several repos: in sync, drifted, archived, and a fork. + +## Setting one up + +1. Create an organization, or use one you own. +2. Register a GitHub App in it with repository permissions for contents, issues, and pull requests + set to read and write, and metadata set to read. Install it on the repos you will test with. +3. Generate a private key for the app. Set `LE_BOT_APP_ID` to the app's ID and `LE_BOT_PRIVATE_KEY` + to the contents of the key file, at the organization or repository level. +4. Add `SLACK_WEBHOOK_URL` and `SLACK_COMMUNITY_NOTIFICATIONS_WEBHOOK_URL` if you want to test the + Slack automations. Without them, those steps fail, but the rest still runs. +5. For the spreadsheet, create a Google Cloud project, enable the Google Sheets API, and create a + service account. On the service account, open the Keys tab, add a key, and choose JSON. Set + `GH_UPLOADER_GCP_SA_CREDENTIALS` to the contents of the file. +6. Create a Google Sheet and share it with the service account's `client_email` as an editor. That + sharing is the only permission that matters, since no IAM role is involved. Set + `CONTRIBUTIONS_SPREADSHEET_ID` to the ID in the sheet's URL and `CONTRIBUTIONS_SHEET_NAME` to the + tab name. +7. Copy `automation-template.yml` into a repo there as `.github/workflows/automation.yml`. + +## Running against it + +Set `SYNC_ORG` to the organization: + +```bash +SYNC_ORG= GITHUB_TOKEN= node scripts/sync-automation-template.js --dry-run +``` + +`SYNC_ORG` and `--only` compose, so a run can still be narrowed to one repo within the organization. + +Installation is per repo and separate from the app's permissions. A repo the app is not installed on +never appears because discovery only sees what the token can see. diff --git a/scripts/sync-automation-template.js b/scripts/sync-automation-template.js index 58b1ac0..826ff67 100644 --- a/scripts/sync-automation-template.js +++ b/scripts/sync-automation-template.js @@ -4,8 +4,9 @@ * drifted from automation-template.yml. * * Usage: - * node scripts/sync-automation-template.js open or update pull requests - * node scripts/sync-automation-template.js --dry-run report only, change nothing + * node scripts/sync-automation-template.js open or update pull requests + * node scripts/sync-automation-template.js --dry-run report only, change nothing + * node scripts/sync-automation-template.js --only=repo limit the run to one repo * * Requires GITHUB_TOKEN with contents:write and pull-requests:write on each * consumer repo. It never commits to a default branch and never merges. @@ -136,32 +137,52 @@ async function readCopy(api, repo, ref) { * skipped because Actions do not run on them, and forks because their copy * belongs to the upstream repo. */ -async function findConsumers(api) { +async function findConsumers(api, only) { const repos = []; - for (let page = 1; ; page += 1) { + if (only) { + const r = await api('GET', `/repos/${ORG}/${only}`); + if (!r.ok) throw new Error(`could not read ${only} (${detail(r)})`); + repos.push(r.data); + } + for (let page = 1; !only; page += 1) { const r = await api('GET', `/orgs/${ORG}/repos?per_page=100&type=all&page=${page}`); if (!r.ok) throw new Error(`could not list the org's repos (${detail(r)})`); repos.push(...r.data); if (r.data.length < 100) break; } + // Skipping a named repo would report no consumers and exit 0, which reads as + // "in sync". Say why instead. + const skip = (repo, why) => { + if (only) throw new Error(`${repo.name} is not a consumer: ${why}`); + }; + const consumers = []; for (const repo of repos) { - if (repo.archived || repo.fork) continue; + if (repo.archived || repo.fork) { + skip(repo, repo.archived ? 'archived' : 'a fork'); + continue; + } let copy; try { copy = await readCopy(api, repo.name, repo.default_branch); } catch (err) { copy = { error: err.message }; } - if (copy.missing) continue; + if (copy.missing) { + skip(repo, `no ${TARGET_PATH}`); + continue; + } if (copy.error) { consumers.push({ repo: repo.name, base: repo.default_branch, unreadable: copy.error }); continue; } // This repo's own reusable automation.yml sits at the same path and does not // call the shared workflow, so this excludes it without a special case. - if (!copy.content.includes(CONSUMER_MARKER)) continue; + if (!copy.content.includes(CONSUMER_MARKER)) { + skip(repo, 'its automation.yml does not call the shared workflow'); + continue; + } consumers.push({ repo: repo.name, base: repo.default_branch }); } return consumers; @@ -272,7 +293,7 @@ async function syncRepo(api, consumer, template, { dryRun }) { } async function run(api, template, options) { - const consumers = await findConsumers(api); + const consumers = await findConsumers(api, options.only); const results = []; for (const consumer of consumers) { @@ -299,6 +320,18 @@ function report(results) { return problems.length; } +// Both spellings, because an --only nobody parses is a full run against every +// consumer, which is the opposite of what it asks for. +function parseArgs(argv) { + const i = argv.findIndex((a) => a === '--only' || a.startsWith('--only=')); + let only; + if (i !== -1) { + only = argv[i] === '--only' ? argv[i + 1] : argv[i].slice('--only='.length); + if (!only || only.startsWith('--')) throw new Error('--only needs a repo name'); + } + return { dryRun: argv.includes('--dry-run'), only }; +} + async function main() { const token = process.env.GITHUB_TOKEN; if (!token) { @@ -306,10 +339,25 @@ async function main() { process.exit(1); } const template = fs.readFileSync(TEMPLATE_PATH, 'utf8'); - const results = await run(httpApi(token), template, { dryRun: process.argv.includes('--dry-run') }); + const results = await run(httpApi(token), template, parseArgs(process.argv.slice(2))); process.exit(report(results) ? 1 : 0); } -if (require.main === module) main(); +if (require.main === module) { + main().catch((err) => { + console.error(err.message); + process.exit(1); + }); +} -module.exports = { run, syncRepo, classifyDrift, findConsumers, report, PROBLEM_STATES, BRANCH, TARGET_PATH }; +module.exports = { + run, + syncRepo, + classifyDrift, + findConsumers, + parseArgs, + report, + PROBLEM_STATES, + BRANCH, + TARGET_PATH, +}; diff --git a/scripts/sync-automation-template.test.js b/scripts/sync-automation-template.test.js index 0d99dc4..c92f413 100644 --- a/scripts/sync-automation-template.test.js +++ b/scripts/sync-automation-template.test.js @@ -1,7 +1,15 @@ const test = require('node:test'); const assert = require('node:assert/strict'); -const { run, classifyDrift, findConsumers, report, PROBLEM_STATES, BRANCH } = require('./sync-automation-template'); +const { + run, + classifyDrift, + findConsumers, + parseArgs, + report, + PROBLEM_STATES, + BRANCH, +} = require('./sync-automation-template'); const USES = 'jobs:\n automation:\n uses: learningequality/.github/.github/workflows/automation.yml@main\n'; const TEMPLATE = `name: Automation\non: {}\n${USES}`; @@ -375,6 +383,68 @@ test('a stale branch is reset to base when no pull request is open', async () => assert.equal(reset.body.force, true); }); +test('both spellings of --only are parsed, because an ignored one runs against everything', () => { + assert.equal(parseArgs(['--only=demo']).only, 'demo'); + assert.equal(parseArgs(['--only', 'demo']).only, 'demo'); + assert.equal(parseArgs(['--dry-run', '--only', 'demo']).dryRun, true); + assert.equal(parseArgs(['--dry-run']).only, undefined); +}); + +test('--only without a repo name is an error, not a full run', () => { + assert.throws(() => parseArgs(['--only']), /needs a repo name/); + assert.throws(() => parseArgs(['--only=']), /needs a repo name/); + assert.throws(() => parseArgs(['--only', '--dry-run']), /needs a repo name/); +}); + +test('a named repo that is not a consumer says why, rather than reporting none', async () => { + const cases = [ + [repo({ archived: true }), /archived/], + [repo({ fork: true }), /a fork/], + ]; + for (const [r, expected] of cases) { + const routes = [...baseRoutes(TEMPLATE), ['GET =/repos/learningequality/demo', ok(r)]]; + await assert.rejects(() => run(makeApi(routes), TEMPLATE, { only: 'demo' }), expected); + } +}); + +test('a named repo without the marker says why', async () => { + const routes = [ + ...baseRoutes(TEMPLATE), + ['GET =/repos/learningequality/demo', ok(repo())], + ['GET contents/.github/workflows/automation.yml', ok({ sha: 'x', content: encode('name: other\n') })], + ]; + await assert.rejects(() => run(makeApi(routes), TEMPLATE, { only: 'demo' }), /does not call the shared workflow/); +}); + +test('only limits the run to that repo, and never lists the org', async () => { + const calls = []; + const others = [repo({ name: 'demo' }), repo({ name: 'production-repo' })]; + const routes = [...baseRoutes(TEMPLATE, others), ['GET =/repos/learningequality/demo', ok(repo())]]; + const results = await run(makeApi(routes, calls), TEMPLATE, { only: 'demo' }); + assert.deepEqual( + results.map((r) => r.repo), + ['demo'] + ); + assert.ok( + !calls.some((c) => c.url.includes('/orgs/')), + 'the org listing is the path to every other repo, so it must not be walked' + ); +}); + +test('without only, every repo in the listing is considered', async () => { + const others = [repo({ name: 'demo' }), repo({ name: 'production-repo' })]; + const results = await run(makeApi(baseRoutes(TEMPLATE, others)), TEMPLATE, {}); + assert.deepEqual( + results.map((r) => r.repo), + ['demo', 'production-repo'] + ); +}); + +test('only reports an error when the repo cannot be read', async () => { + const routes = [...baseRoutes(TEMPLATE), ['GET =/repos/learningequality/missing', fail(404, 'Not Found')]]; + await assert.rejects(() => run(makeApi(routes), TEMPLATE, { only: 'missing' }), /could not read missing/); +}); + test('findConsumers pages through the org listing', async () => { const calls = []; const many = Array.from({ length: 100 }, (_, i) => repo({ name: `r${i}` }));