diff --git a/.env.example b/.env.example index d98f7d4b..cfd7b0a3 100644 --- a/.env.example +++ b/.env.example @@ -1,6 +1,9 @@ # The organization where you want to register the app in the app creation manifest flow. # If set, the app is registered for an organization (https://github.com/organizations/ORGANIZATION/settings/apps/new), # if not set, the GitHub app would be registered for the user account (https://github.com/settings/apps/new). +# Also selects the account for CLI and cron full syncs (case-insensitive). +# If set without a matching installation, the sync fails instead of falling back. +# If unset, full sync processes all repository-owning installations sequentially. # GH_ORG= # The ID of your GitHub App diff --git a/README.md b/README.md index 88d67004..322f2492 100644 --- a/README.md +++ b/README.md @@ -951,6 +951,22 @@ You can pass environment variables; the easiest way to do it is via a `.env` fil ``` CREATE_DEFAULT_BRANCH=true ``` +1. Scope CLI and cron full syncs to one account using `GH_ORG`. For e.g. + ``` + GH_ORG=my-org + ``` + A full sync (`CRON` or `npm run full-sync`) selects the installation whose + account login matches `GH_ORG` case-insensitively and reads configuration from + that account's ``. If no installation matches, including when the + app has no installations, the sync fails instead of syncing another account. + This also applies to dry runs (`FULL_SYNC_NOP=true`); the CLI exits nonzero on + a targeting error, and cron reports the execution as failed. + When `GH_ORG` is unset, all repository-owning installations are synced + sequentially. Set it to restrict full sync to one account; enterprise + installations supply app-management context but are not repository sync targets. + Webhook events still use their own installation context. `GH_ORG` is also used + by the [manifest flow](https://docs.github.com/en/apps/sharing-github-apps/registering-a-github-app-from-a-manifest) + to choose where the app is registered. ### Runtime Settings @@ -1037,6 +1053,9 @@ node smoke-test.js --phase 1,3,5 # Bypass actor apply/NOP convergence (Phase 1 creates the required test repo) node smoke-test.js --phase 1,19 +# Explicit GH_ORG full-sync CLI dry run (setup + Phase 24 + teardown) +node smoke-test.js --phase 24 + # Repository and environment variables, including pagination and no-op convergence node smoke-test.js --phase 1,13,21 @@ -1072,6 +1091,7 @@ The smoke test runs the following phases: | **Phase 19** | Tests ignored `OrganizationAdmin`/`DeployKey` IDs, order-independent NOP convergence, real bypass-mode/role-ID changes, and no redundant updates (requires Phase 1) | | **Phase 21** | Reads all pages of 101 repository variables and 100 environment variables, verifies unchanged NOP/apply writes nothing, and updates only the two boundary variables (requires Phases 1 and 13) | | **Phase 22** | Runs real full-sync NOP against the verified `GH_ORG` installation with controlled enumeration/auth and read-only, org-scoped requests. Requires only Setup; multi-installation fanout and failure isolation are covered by unit/CLI tests, not this single-org smoke | +| **Phase 24** | Runs the real full-sync CLI with explicit `GH_ORG` and `FULL_SYNC_NOP=true`, verifying the selected installation and successful completion; multi-account, no-match, and cron failure scenarios use local mocked tests | | **Teardown** | Shuts down safe-settings, deletes test repos, teams, custom roles, and rulesets | The harness rejects a nonempty `CRON` before authentication or setup. However, diff --git a/docs/github-action.md b/docs/github-action.md index 0cf63c67..737c61a1 100644 --- a/docs/github-action.md +++ b/docs/github-action.md @@ -10,21 +10,24 @@ Follow the [Create the GitHub App](deploy.md#create-the-github-app) guide to cre ## Defining the GitHub Action Workflow Running a full-sync with `safe-settings` can be done via `npm run full-sync`. This requires installing Node, such as with [actions/setup-node](https://github.com/actions/setup-node) (see example below). When doing so, the appropriate environment variables must be set (see the [Environment variables](#environment-variables) document for more details). -Manual and scheduled (`CRON`) full syncs process every repository-owning App -installation sequentially, using each installation's token and account login for -its admin repository. Enterprise installations are not repository sync targets; +When `GH_ORG` is unset, manual and scheduled (`CRON`) full syncs process every +repository-owning App installation sequentially, using each installation's token +and account login for its admin repository. Enterprise installations are not repository sync targets; they remain available for enterprise app-management context enrichment. An authentication, configuration, or sync failure does not stop later installations. The returned `{ results, errors }` retains each returned Settings result (including partial results) and aggregates their errors with thrown and missing-result failures. An info-level summary counts installations with errors as failed. `FULL_SYNC_NOP=true` -previews every installation and still reports failures with a nonzero CLI exit. -No eligible installations returns `null` from `syncInstallation` and exits the CLI -nonzero with an explicit diagnostic. +previews the selected installations and still reports failures with a nonzero CLI exit. +With `GH_ORG` unset, no eligible installations returns `null` from +`syncInstallation` and exits the CLI nonzero with an explicit diagnostic. -`GH_ORG` does not restrict this full-sync path. Do not use a multi-installation App -for a live single-organization test by setting `GH_ORG` alone; the smoke test's -Phase 22 uses a controlled installation list and auth boundary instead. +`GH_ORG` restricts full sync to the matching account and fails if no eligible +installation matches. Startup `info()` still authenticates independently to read +the app slug, and webhook events use their own installation contexts. A live +single-organization smoke test therefore also needs an authentication/API boundary +and verified runtime configuration, not just `GH_ORG`. Phase 22 uses a controlled +installation list and auth boundary. Installation repositories are processed in batches of up to ten, with each batch settling before the next starts. A repository failure does not stop later @@ -38,6 +41,12 @@ for suborg configuration changes; existing repository restrictions still apply. ### Example GHA Workflow The below example uses the GHA "cron" feature to run a full-sync every 4 hours. While not required, this example uses the `.github` repo as the `admin` repo (set via `ADMIN_REPO` env var) and the safe-settings configurations are stored in the `safe-settings/` directory (set via `CONFIG_PATH` and `DEPLOYMENT_CONFIG_FILE`). +`GH_ORG` selects the account to sync, matching its login case-insensitively. +Set it when the App is installed on multiple accounts; without it, full sync +processes all repository-owning installations. If no installation matches, +the sync exits nonzero rather than syncing another account, including in dry +runs (`FULL_SYNC_NOP=true`). + ```yaml name: Safe Settings Sync on: diff --git a/full-sync.js b/full-sync.js index 028660ac..c349e952 100644 --- a/full-sync.js +++ b/full-sync.js @@ -3,7 +3,7 @@ const { FULL_SYNC_NOP } = require('./lib/env') const { createProbot } = require('probot') async function performFullSync (appFn, nop) { - const probot = createProbot() + const probot = await createProbot().ready() probot.log.info(`Starting full sync with NOP=${nop}`) try { diff --git a/index.js b/index.js index 9f5fa48f..4ed6c239 100644 --- a/index.js +++ b/index.js @@ -416,7 +416,7 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => async function syncInstallation (nop = false) { robot.log.trace('Fetching installations') - const installations = (await listAllInstallations()).filter(installation => { + let installations = (await listAllInstallations()).filter(installation => { // Enterprise installations supply app-management credentials, not repos. if (installation.target_type === 'Enterprise') { robot.log.debug(`Skipping enterprise installation ${installation.id} for repository sync`) @@ -425,6 +425,17 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => return true }) + if (env.GH_ORG) { + const installation = installations.find(i => + typeof i.account?.login === 'string' && i.account.login.toLowerCase() === env.GH_ORG.toLowerCase() + ) + if (!installation) { + const accounts = installations.map(i => i.account?.login).join(', ') + throw new Error(`No app installation found for GH_ORG '${env.GH_ORG}'. Installed on: [${accounts}]`) + } + installations = [installation] + } + if (installations.length === 0) { return null } @@ -439,7 +450,7 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => if (typeof owner !== 'string' || !owner.trim()) { throw new Error(`Installation ${installation.id} has no account login for repository sync`) } - robot.log.debug(`Syncing installation ${installation.id} for ${owner}`) + robot.log.info(`Syncing installation ${installation.id} on account ${owner}`) const github = await robot.auth(installation.id) const context = { payload: { @@ -1055,9 +1066,10 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => cron.schedule(process.env.CRON, async () => { robot.log.debug('running a task every minute') try { - await syncInstallation() + return await syncInstallation() } catch (e) { robot.log.error(`Scheduled full sync failed: ${e}`) + throw e } }) } diff --git a/lib/env.js b/lib/env.js index 7cdc467c..252e976b 100644 --- a/lib/env.js +++ b/lib/env.js @@ -8,6 +8,7 @@ module.exports = { BLOCK_REPO_RENAME_BY_HUMAN: process.env.BLOCK_REPO_RENAME_BY_HUMAN || 'false', CREATE_DEFAULT_BRANCH: process.env.CREATE_DEFAULT_BRANCH === 'true', FULL_SYNC_NOP: process.env.FULL_SYNC_NOP === 'true', + GH_ORG: process.env.GH_ORG, GHE_HOST: process.env.GHE_HOST, GHE_PROTOCOL: process.env.GHE_PROTOCOL, } diff --git a/smoke-test.js b/smoke-test.js index 6fe13959..91abd893 100644 --- a/smoke-test.js +++ b/smoke-test.js @@ -148,6 +148,7 @@ class InteractiveExit extends Error { // ─── Octokit client (initialized in main) ──────────────────────────────────── let octokit = null +let orgInstallation = null // Enterprise-installation-authenticated client (Phase 17). Null when the app // is not installed on the enterprise or GH_ENTERPRISE is unset. let entOctokit = null @@ -3539,6 +3540,47 @@ async function phase19BypassActorConvergence () { } } +async function phase24OrganizationSyncTargeting () { + logPhase('Phase 24: Organization-targeted full-sync dry run') + if (!assert(process.env.GH_ORG && orgInstallation?.account?.login?.toLowerCase() === ORG.toLowerCase(), + '24: explicit GH_ORG matches the authenticated test installation')) { + throw new Error('Phase 24 requires an explicitly configured and verified test organization') + } + + const result = await new Promise((resolve, reject) => { + const child = spawn(process.execPath, [path.join(__dirname, 'full-sync.js')], { + cwd: __dirname, + env: { + ...process.env, + GH_ORG: ORG, + FULL_SYNC_NOP: 'true', + CRON: '', + LOG_LEVEL: 'info' + }, + stdio: ['ignore', 'pipe', 'pipe'], + timeout: 120000 + }) + let output = '' + child.stdout.on('data', data => { output += data }) + child.stderr.on('data', data => { output += data }) + child.on('error', error => { + logFail(`24: could not start full-sync CLI: ${error.message}`) + reject(error) + }) + child.on('close', (code, signal) => resolve({ code, signal, output })) + }) + + const selected = [...result.output.matchAll(/Syncing installation (\d+) on account ([A-Za-z0-9-]+)/g)] + assert(result.code === 0, `24: full-sync CLI exited successfully (code=${result.code}, signal=${result.signal})`) + assert(result.output.includes('Starting full sync with NOP=true'), '24: full-sync CLI ran in NOP mode') + assert(selected.length === 1 && Number(selected[0][1]) === orgInstallation.id && + selected[0][2].toLowerCase() === ORG.toLowerCase(), + '24: sync selected exactly the authenticated test organization installation') + assert(result.output.includes('Full sync completed successfully.'), '24: actual full-sync entrypoint completed') + assert(!/Unexpected error during full sync|Fatal error during full sync|Errors occurred during full sync/.test(result.output), + '24: full-sync CLI reported no failure') +} + async function phase22InstallationFullSync (app, installationId) { logPhase('Phase 22: Test-org installation full-sync NOP') if (!process.env.GH_ORG || process.env.GH_ORG.toLowerCase() !== ORG.toLowerCase() || process.env.CRON) { @@ -3629,6 +3671,7 @@ async function main () { // Org/user installations key off account.login (only enterprise accounts have a slug). if (installation.account && installation.account.login && installation.account.login.toLowerCase() === ORG.toLowerCase()) { installationId = installation.id + orgInstallation = installation break } } @@ -3691,7 +3734,8 @@ async function main () { ['Phase 19: Bypass actor convergence', phase19BypassActorConvergence], ['Phase 20: Custom property exclusions', phase20CustomPropertyExclusions], ['Phase 21: Variable pagination', phase21VariablePagination], - ['Phase 22: Test-org installation full-sync NOP', () => phase22InstallationFullSync(app, installationId)] + ['Phase 22: Test-org installation full-sync NOP', () => phase22InstallationFullSync(app, installationId)], + ['Phase 24: Organization sync targeting', phase24OrganizationSyncTargeting] ] // When --phase is given, only run setup (phase 0) + the requested phase(s). diff --git a/test/unit/full-sync.test.js b/test/unit/full-sync.test.js index e9020f5b..41b9b075 100644 --- a/test/unit/full-sync.test.js +++ b/test/unit/full-sync.test.js @@ -1,4 +1,106 @@ +/* eslint-disable no-undef */ const { spawnSync } = require('child_process') +const { generateKeyPairSync } = require('crypto') +const path = require('path') + +describe('full-sync CLI with the installed Probot runtime', () => { + let privateKey + + beforeAll(() => { + privateKey = generateKeyPairSync('rsa', { + modulusLength: 2048, + privateKeyEncoding: { type: 'pkcs1', format: 'pem' }, + publicKeyEncoding: { type: 'pkcs1', format: 'pem' } + }).privateKey + }) + + it.each([ + [false, 'success'], + [true, 'success'], + [false, 'missing-target'], + [true, 'missing-target'], + [false, 'sync-errors'], + [true, 'sync-errors'] + ])('initializes before logging and preserves NOP=%s %s reporting', (nop, scenario) => { + const cli = spawnSync(process.execPath, ['-e', ` + const forbidNetwork = () => { throw new Error('Network access forbidden in CLI regression') } + global.fetch = forbidNetwork + require('http').request = forbidNetwork + require('https').request = forbidNetwork + require('net').Socket.prototype.connect = forbidNetwork + + const { Probot } = require('probot') + Probot.prototype.auth = async function (id) { + await this.ready() + if (id !== undefined && ![1, 2].includes(id)) throw new Error('Unexpected installation') + return { + paginate: async () => [ + { id: 1, account: { login: 'other-org' } }, + { id: 2, account: { login: 'test-org' } } + ], + rest: { + apps: { + listInstallations: { endpoint: { merge: options => options } }, + getAuthenticated: async () => ({ data: { slug: 'test-app' } }) + }, + repos: { getContent: async () => ({ data: { content: '' } }) } + } + } + } + const Module = require('module') + const load = Module._load + Module._load = function (name, parent, main) { + if (name === './lib/settings' && parent.filename === require.resolve('./index')) { + return { + syncAll: async (nop, context, repo) => { + console.log('SYNC_RESULT ' + JSON.stringify({ nop, owner: repo.owner, installation: context.payload.installation.id })) + return { errors: process.env.TEST_SCENARIO === 'sync-errors' ? [{ msg: 'test sync failure' }] : [] } + } + } + } + return load.call(this, name, parent, main) + } + require('./full-sync') + `], { + cwd: path.resolve(__dirname, '../..'), + encoding: 'utf8', + timeout: 10000, + env: { + PATH: process.env.PATH, + APP_ID: '1', + PRIVATE_KEY: privateKey, + GH_ORG: scenario === 'missing-target' ? 'missing-org' : 'TeSt-OrG', + FULL_SYNC_NOP: String(nop), + TEST_SCENARIO: scenario, + LOG_FORMAT: 'json', + LOG_LEVEL: 'info', + DEPLOYMENT_CONFIG_FILE: path.join(__dirname, 'no-such-deployment-settings.yml') + } + }) + const output = cli.stdout + cli.stderr + + expect(cli.error).toBeUndefined() + expect(output).not.toContain('Network access forbidden') + expect(output).not.toContain('"owner":"other-org"') + expect(output).toContain(`Starting full sync with NOP=${nop}`) + expect(cli.status).toBe(scenario === 'success' ? 0 : 1) + if (scenario === 'missing-target') { + expect(output).toContain("No app installation found for GH_ORG 'missing-org'") + expect(output).not.toContain('SYNC_RESULT') + } else { + expect(output).toContain('SYNC_RESULT ' + JSON.stringify({ nop, owner: 'test-org', installation: 2 })) + expect(output).toContain('Syncing installation 2 on account test-org') + } + if (scenario === 'success') { + expect(output).toContain('Full sync completed successfully.') + } else { + expect(output).not.toContain('Full sync completed successfully.') + } + if (scenario === 'sync-errors') { + expect(output).toContain('Errors occurred during full sync.') + } + }) +}) // Run both real entrypoints in a child process, replacing only external clients // and Settings work. No App credentials or caller environment reach the child. @@ -19,6 +121,7 @@ const script = ` rest: { apps: { listInstallations: { endpoint: { merge: x => x } } } } } const robot = { + ready: async () => robot, on: () => {}, log: { trace: () => {}, debug: () => {}, info: console.log, error: console.error }, auth: async id => { diff --git a/test/unit/lib/env.test.js b/test/unit/lib/env.test.js index d345fe54..7daec359 100644 --- a/test/unit/lib/env.test.js +++ b/test/unit/lib/env.test.js @@ -1,48 +1,43 @@ /* eslint-disable no-undef */ describe('env', () => { - describe('load default values without override', () => { - const envTest = require('../../../lib/env') - - it('loads default ADMIN_REPO if not passed', () => { - const ADMIN_REPO = envTest.ADMIN_REPO - expect(ADMIN_REPO).toEqual('admin') - }) - - it('loads default CONFIG_PATH if not passed', () => { - const CONFIG_PATH = envTest.CONFIG_PATH - expect(CONFIG_PATH).toEqual('.github') - }) - - it('loads default SETTINGS_FILE_PATH if not passed', () => { - const SETTINGS_FILE_PATH = envTest.SETTINGS_FILE_PATH - expect(SETTINGS_FILE_PATH).toEqual('settings.yml') - }) - - it('loads default DEPLOYMENT_CONFIG_FILE_PATH if not passed', () => { - const SETTINGS_FILE_PATH = envTest.DEPLOYMENT_CONFIG_FILE_PATH - expect(SETTINGS_FILE_PATH).toEqual('deployment-settings.yml') - }) + const originalEnv = process.env + const defaults = { + ADMIN_REPO: 'admin', + CONFIG_PATH: '.github', + SETTINGS_FILE_PATH: 'settings.yml', + DEPLOYMENT_CONFIG_FILE_PATH: 'deployment-settings.yml', + CREATE_PR_COMMENT: 'true', + FULL_SYNC_NOP: false, + GH_ORG: undefined + } + + beforeEach(() => { + jest.resetModules() + process.env = { ...originalEnv } + for (const key of [...Object.keys(defaults), 'DEPLOYMENT_CONFIG_FILE']) { + delete process.env[key] + } + }) - it('loads default CREATE_PR_COMMENT if not passed', () => { - const CREATE_PR_COMMENT = envTest.CREATE_PR_COMMENT - expect(CREATE_PR_COMMENT).toEqual('true') - }) + afterEach(() => { + process.env = originalEnv + }) - it('loads default FULL_SYNC_NOP if not passed', () => { - const FULL_SYNC_NOP = envTest.FULL_SYNC_NOP - expect(FULL_SYNC_NOP).toEqual(false) + describe('load default values without override', () => { + it.each(Object.entries(defaults))('loads default %s if not passed', (key, value) => { + expect(require('../../../lib/env')[key]).toEqual(value) }) }) describe('load override values', () => { - beforeAll(() => { - jest.resetModules() + beforeEach(() => { process.env.ADMIN_REPO = '.github' process.env.CONFIG_PATH = '.config' process.env.SETTINGS_FILE_PATH = 'safe-settings.yml' process.env.DEPLOYMENT_CONFIG_FILE = 'safe-settings-deployment.yml' process.env.CREATE_PR_COMMENT = 'false' - process.env.FULL_SYNC_NOP = false + process.env.FULL_SYNC_NOP = 'true' + process.env.GH_ORG = 'My-Org' }) it('loads override values if passed', () => { @@ -58,7 +53,8 @@ describe('env', () => { const CREATE_PR_COMMENT = envTest.CREATE_PR_COMMENT expect(CREATE_PR_COMMENT).toEqual('false') const FULL_SYNC_NOP = envTest.FULL_SYNC_NOP - expect(FULL_SYNC_NOP).toEqual(false) + expect(FULL_SYNC_NOP).toEqual(true) + expect(envTest.GH_ORG).toEqual('My-Org') }) }) }) diff --git a/test/unit/lib/settings-batching.test.js b/test/unit/lib/settings-batching.test.js index d97f4785..9c78fb3c 100644 --- a/test/unit/lib/settings-batching.test.js +++ b/test/unit/lib/settings-batching.test.js @@ -327,7 +327,7 @@ describe('Repository sync batching', () => { require: name => { if (name === './') return () => ({ syncInstallation: async () => settings }) if (name === './lib/env') return { FULL_SYNC_NOP: true } - if (name === 'probot') return { createProbot: () => ({ log: console }) } + if (name === 'probot') return { createProbot: () => ({ ready: async () => ({ log: console }) }) } throw new Error('Unexpected dependency: ' + name) }, process, diff --git a/test/unit/smoke-organization-targeting.test.js b/test/unit/smoke-organization-targeting.test.js new file mode 100644 index 00000000..1948afca --- /dev/null +++ b/test/unit/smoke-organization-targeting.test.js @@ -0,0 +1,88 @@ +/* eslint-disable no-undef */ +const fs = require('fs') +const path = require('path') +const vm = require('vm') +const { EventEmitter } = require('events') + +const smokePath = path.resolve(__dirname, '../../smoke-test.js') +const source = fs.readFileSync(smokePath, 'utf8') +const phaseSource = source.slice( + source.indexOf('async function phase24OrganizationSyncTargeting ()'), + source.indexOf('\n}\n', source.indexOf('async function phase24OrganizationSyncTargeting ()')) + 2 +) + +describe('organization-targeted smoke phase', () => { + let child, spawn, check, logFail, run, env + const success = 'Starting full sync with NOP=true\nSyncing installation 2 on account My-Org\nFull sync completed successfully.' + + beforeEach(() => { + child = new EventEmitter() + child.stdout = new EventEmitter() + child.stderr = new EventEmitter() + spawn = jest.fn(() => child) + check = jest.fn(condition => Boolean(condition)) + logFail = jest.fn() + env = { GH_ORG: 'my-org', FULL_SYNC_NOP: 'false', CRON: '* * * * *' } + // Execute only this phase, never the live harness setup or its .env loader. + run = vm.runInNewContext(`(${phaseSource})`, { + spawn, + assert: check, + logPhase: jest.fn(), + logFail, + process: { execPath: process.execPath, env }, + path, + __dirname: path.dirname(smokePath), + ORG: 'my-org', + orgInstallation: { id: 2, account: { login: 'My-Org' } } + }) + }) + + it('runs only the scoped CLI dry run with no background cron', async () => { + const pending = run() + child.stdout.emit('data', success) + child.emit('close', 0, null) + await pending + + expect(spawn).toHaveBeenCalledWith(process.execPath, [path.resolve(__dirname, '../../full-sync.js')], expect.objectContaining({ + timeout: 120000, + env: expect.objectContaining({ GH_ORG: 'my-org', FULL_SYNC_NOP: 'true', CRON: '', LOG_LEVEL: 'info' }) + })) + expect(check).toHaveBeenCalledTimes(6) + expect(check.mock.results.every(result => result.value)).toBe(true) + expect(env.FULL_SYNC_NOP).toBe('false') + expect(env.CRON).toBe('* * * * *') + }) + + it.each([ + ['wrong account', success.replace('My-Org', 'other-org'), 0], + ['wrong installation', success.replace('installation 2', 'installation 1'), 0], + ['extra installation', success + '\nSyncing installation 1 on account other-org', 0], + ['apply instead of NOP', success.replace('NOP=true', 'NOP=false'), 0], + ['incomplete execution', success.replace('Full sync completed successfully.', ''), 0], + ['CLI failure', success, 1], + ['timeout', success, null], + ['fatal output', success + '\nUnexpected error during full sync: failure', 0] + ])('fails its assertions for %s', async (_name, output, code) => { + const pending = run() + child.stdout.emit('data', output) + child.emit('close', code, code === null ? 'SIGTERM' : null) + await pending + + expect(check.mock.results.some(result => !result.value)).toBe(true) + }) + + it('refuses to run with an implicit test organization', async () => { + delete env.GH_ORG + + await expect(run()).rejects.toThrow('explicitly configured and verified test organization') + expect(spawn).not.toHaveBeenCalled() + }) + + it('reports process startup errors as failures', async () => { + const pending = run() + child.emit('error', new Error('spawn failed')) + + await expect(pending).rejects.toThrow('spawn failed') + expect(logFail).toHaveBeenCalledWith('24: could not start full-sync CLI: spawn failed') + }) +}) diff --git a/test/unit/sync-installation.test.js b/test/unit/sync-installation.test.js index 2eec3194..49f70b66 100644 --- a/test/unit/sync-installation.test.js +++ b/test/unit/sync-installation.test.js @@ -1,21 +1,313 @@ -const plugin = require('../../index') -const ConfigManager = require('../../lib/configManager') -const cron = require('node-cron') +/* eslint-disable no-undef */ +const path = require('path') +const { spawnSync } = require('child_process') jest.mock('node-cron', () => ({ schedule: jest.fn() })) -jest.mock('../../lib/env', () => ({ - ADMIN_REPO: 'admin', - CONFIG_PATH: '.github', - SETTINGS_FILE_PATH: 'settings.yml', - DEPLOYMENT_CONFIG_FILE_PATH: 'test/fixtures/no-deployment-settings.yml' -})) -const flush = () => new Promise(resolve => setImmediate(resolve)) -const installation = (id, login, targetType = 'Organization') => ({ - id, target_type: targetType, account: { login, type: targetType } -}) +function installation (id, login) { + return { id, account: { login } } +} + +function client () { + return { + rest: { + apps: { + getAuthenticated: jest.fn().mockResolvedValue({ data: { slug: 'safe-settings' } }) + }, + repos: { + getContent: jest.fn().mockResolvedValue({ data: { content: '' } }) + } + } + } +} describe('syncInstallation', () => { + const originalEnv = process.env + let robot, appClient, clients, syncAll, handleError + + beforeEach(() => { + jest.resetModules() + process.env = { ...originalEnv } + for (const key of ['GH_ORG', 'GH_ENTERPRISE', 'CRON', 'ADMIN_REPO', 'CONFIG_PATH', 'SETTINGS_FILE_PATH']) { + delete process.env[key] + } + process.env.DEPLOYMENT_CONFIG_FILE = path.join(__dirname, 'no-such-deployment-settings.yml') + appClient = { + paginate: jest.fn().mockResolvedValue([]), + rest: { + apps: { + listInstallations: { endpoint: { merge: jest.fn(options => options) } } + } + } + } + clients = new Map([[1, client()], [2, client()], [3, client()]]) + robot = { + auth: jest.fn(async id => { + if (id === undefined) return appClient + if (!clients.has(id)) throw new Error(`Unexpected installation ${id}`) + return clients.get(id) + }), + log: { debug: jest.fn(), info: jest.fn(), warn: jest.fn(), error: jest.fn(), trace: jest.fn() }, + on: jest.fn() + } + syncAll = jest.fn().mockResolvedValue({ errors: [] }) + handleError = jest.fn() + }) + + afterEach(() => { + process.env = originalEnv + }) + + async function load (ghOrg, installations) { + if (ghOrg !== undefined) process.env.GH_ORG = ghOrg + appClient.paginate.mockResolvedValue(installations) + const app = require('../../index')(robot, {}, { syncAll, handleError }) + // info() intentionally authenticates independently to read the app slug. + await new Promise(resolve => setImmediate(resolve)) + robot.auth.mockClear() + appClient.paginate.mockClear() + return app + } + + function expectSync (selected, nop = false) { + const repo = { owner: selected.account.login, repo: process.env.ADMIN_REPO || 'admin' } + expect(syncAll).toHaveBeenCalledTimes(1) + expect(syncAll).toHaveBeenCalledWith(nop, expect.objectContaining({ + payload: { installation: selected }, + octokit: clients.get(selected.id), + log: robot.log + }), repo, expect.any(Object)) + expect(syncAll.mock.calls[0][1].repo()).toEqual(repo) + expect(clients.get(selected.id).rest.repos.getContent).toHaveBeenCalledWith(expect.objectContaining(repo)) + expect(appClient.rest.apps.listInstallations.endpoint.merge).toHaveBeenCalledWith({ per_page: 100 }) + } + + it.each(['my-org', 'MY-ORG', 'My-Org'])('selects only the matching installation for GH_ORG=%s', async ghOrg => { + const selected = installation(2, 'my-org') + process.env.ADMIN_REPO = 'config' + const app = await load(ghOrg, [installation(1, 'other-org'), selected]) + + await app.syncInstallation() + + expectSync(selected) + expect(robot.auth.mock.calls).toEqual([[], [2]]) + expect(clients.get(1).rest.repos.getContent).not.toHaveBeenCalled() + expect(clients.get(1).rest.apps.getAuthenticated).toHaveBeenCalledTimes(1) + expect(robot.log.info).toHaveBeenCalledWith('Syncing installation 2 on account my-org') + }) + + it.each([false, true])('rejects an absent target without authenticating or syncing another installation (nop=%s)', async nop => { + const app = await load('my-org', [installation(1, 'other-org'), installation(2, 'another-org')]) + + await expect(app.syncInstallation(nop)).rejects.toThrow( + "No app installation found for GH_ORG 'my-org'. Installed on: [other-org, another-org]" + ) + + expect(robot.auth.mock.calls).toEqual([[]]) + expect(syncAll).not.toHaveBeenCalled() + expect(handleError).not.toHaveBeenCalled() + }) + + it.each([false, true])('rejects an empty installation list with a configured target (nop=%s)', async nop => { + const app = await load('my-org', []) + + await expect(app.syncInstallation(nop)).rejects.toThrow( + "No app installation found for GH_ORG 'my-org'. Installed on: []" + ) + expect(robot.auth.mock.calls).toEqual([[]]) + expect(syncAll).not.toHaveBeenCalled() + }) + + it.each([undefined, ''])('syncs every repository-owning installation with GH_ORG=%s', async ghOrg => { + const installations = [installation(1, 'first-org'), installation(2, 'second-org')] + const app = await load(ghOrg, installations) + + const result = await app.syncInstallation() + + expect(result).toEqual({ results: [{ errors: [] }, { errors: [] }], errors: [] }) + expect(robot.auth.mock.calls).toEqual([[], [1], [2]]) + expect(syncAll.mock.calls.map(call => call[2].owner)).toEqual(['first-org', 'second-org']) + for (const selected of installations) { + expect(clients.get(selected.id).rest.repos.getContent).toHaveBeenCalledWith(expect.objectContaining({ + owner: selected.account.login, repo: 'admin' + })) + } + }) + + it('still returns null when there are no installations and no configured org', async () => { + const app = await load(undefined, []) + + await expect(app.syncInstallation()).resolves.toBeNull() + expect(syncAll).not.toHaveBeenCalled() + }) + + it('passes the NOP flag and retains the selected result and errors in the aggregate', async () => { + const selected = installation(2, 'my-org') + const result = { errors: [{ msg: 'existing sync error' }] } + syncAll.mockResolvedValue(result) + const app = await load('my-org', [installation(1, 'other-org'), selected]) + + const aggregate = await app.syncInstallation(true) + expect(aggregate).toEqual({ results: [result], errors: result.errors }) + expect(aggregate.results[0]).toBe(result) + expect(robot.auth.mock.calls).toEqual([[], [2]]) + expectSync(selected, true) + }) + + it('preserves enterprise enrichment and selects an org rather than an enterprise account', async () => { + const selected = { ...installation(2, 'my-org'), enterprise: { slug: 'My-Enterprise' } } + const enterprise = { id: 3, target_type: 'Enterprise', account: { slug: 'my-enterprise' } } + const app = await load('my-org', [installation(1, 'other-org'), enterprise, selected]) + + await app.syncInstallation() + + expectSync(selected) + expect(syncAll.mock.calls[0][1]).toMatchObject({ + enterpriseSlug: 'My-Enterprise', + appGithub: clients.get(3) + }) + expect(robot.auth.mock.calls).toEqual([[], [2], [], [3]]) + expect(clients.get(1).rest.repos.getContent).not.toHaveBeenCalled() + expect(clients.get(3).rest.repos.getContent).not.toHaveBeenCalled() + }) + + it.each([ + [{ id: 1, target_type: 'Enterprise', account: { login: 'my-org', slug: 'my-org' } }], + [{ id: 1, account: { login: 42 } }] + ])('does not select enterprise or malformed accounts for a configured org: %j', async entries => { + const app = await load('my-org', [entries]) + + await expect(app.syncInstallation()).rejects.toThrow("No app installation found for GH_ORG 'my-org'") + expect(robot.auth.mock.calls).toEqual([[]]) + expect(syncAll).not.toHaveBeenCalled() + }) + + it.each(['listing', 'authentication', 'config', 'sync'])('retains %s failures without syncing an untargeted installation', async stage => { + const error = new Error(`${stage} failed`) + const app = await load('my-org', [installation(1, 'other-org'), installation(2, 'my-org')]) + if (stage === 'listing') appClient.paginate.mockRejectedValue(error) + if (stage === 'authentication') { + robot.auth.mockImplementation(async id => { + if (id !== undefined) throw error + return appClient + }) + } + if (stage === 'config') clients.get(2).rest.repos.getContent.mockRejectedValue(error) + if (stage === 'sync') syncAll.mockRejectedValue(error) + + if (stage === 'listing') { + await expect(app.syncInstallation()).rejects.toBe(error) + } else { + await expect(app.syncInstallation()).resolves.toEqual({ results: [], errors: [error] }) + } + if (stage !== 'sync') expect(syncAll).not.toHaveBeenCalled() + expect(clients.get(1).rest.repos.getContent).not.toHaveBeenCalled() + }) + + it('returns the scheduled promise so node-cron reports targeting failures instead of success', async () => { + process.env.CRON = '* * * * *' + await load('my-org', [installation(1, 'other-org')]) + const schedule = require('node-cron').schedule + expect(schedule).toHaveBeenCalledWith('* * * * *', expect.any(Function)) + + await expect(schedule.mock.calls[0][1]()).rejects.toThrow("No app installation found for GH_ORG 'my-org'") + expect(syncAll).not.toHaveBeenCalled() + }) + + it('reports a targeted NOP configuration failure without falling back to another installation', async () => { + const failure = new Error('invalid target configuration') + const app = await load('my-org', [installation(1, 'other-org'), installation(2, 'my-org')]) + clients.get(2).rest.repos.getContent.mockRejectedValue(failure) + + const result = await app.syncInstallation(true) + + expect(result.results).toEqual([]) + expect(result.errors).toHaveLength(1) + expect(result.errors[0].message).toContain('installation 2 for my-org returned no result') + expect(handleError).toHaveBeenCalledTimes(1) + expect(handleError.mock.calls[0][2]).toEqual({ owner: 'my-org', repo: 'admin' }) + expect(robot.auth.mock.calls).toEqual([[], [2]]) + expect(clients.get(1).rest.repos.getContent).not.toHaveBeenCalled() + expect(syncAll).not.toHaveBeenCalled() + }) + + it('awaits successful scheduled syncs', async () => { + process.env.CRON = '* * * * *' + await load('my-org', [installation(2, 'my-org')]) + const result = { errors: [] } + syncAll.mockResolvedValue(result) + + await expect(require('node-cron').schedule.mock.calls[0][1]()).resolves.toEqual({ results: [result], errors: [] }) + }) + + it('reports a failed execution through the real node-cron task lifecycle', async () => { + process.env.CRON = '* * * * *' + await load('my-org', [installation(1, 'other-org')]) + const callback = require('node-cron').schedule.mock.calls[0][1] + const task = jest.requireActual('node-cron').createTask('* * * * *', callback) + const failed = jest.fn() + const finished = jest.fn() + task.on('execution:failed', failed) + task.on('execution:finished', finished) + const logged = jest.spyOn(console, 'error').mockImplementation(() => {}) + + try { + await expect(task.execute()).rejects.toThrow("No app installation found for GH_ORG 'my-org'") + expect(failed).toHaveBeenCalledTimes(1) + expect(finished).not.toHaveBeenCalled() + expect(logged).toHaveBeenCalled() + } finally { + task.destroy() + } + }) + + it.each([false, true])('exits the real full-sync CLI nonzero on targeting failure (nop=%s)', nop => { + const cli = spawnSync(process.execPath, ['-e', ` + const fs = require('fs') + const vm = require('vm') + const plugin = require(process.argv[1]) + const github = { + paginate: async () => [{ id: 1, account: { login: 'other-org' } }], + rest: { apps: { + listInstallations: { endpoint: { merge: options => options } }, + getAuthenticated: async () => ({ data: { slug: 'safe-settings' } }) + } } + } + const robot = { auth: async () => github, on: () => {}, log: console, ready: async () => robot } + vm.runInNewContext(fs.readFileSync(process.argv[2], 'utf8'), { + require: name => { + if (name === './') return (robot, options) => plugin(robot, options, { + syncAll: () => { throw new Error('Wrong account synced') } + }) + if (name === './lib/env') return { FULL_SYNC_NOP: process.env.FULL_SYNC_NOP === 'true' } + if (name === 'probot') return { createProbot: () => robot } + throw new Error('Unexpected dependency: ' + name) + }, + process, + console + }) + `, require.resolve('../../index'), require.resolve('../../full-sync')], { + encoding: 'utf8', + env: { ...process.env, GH_ORG: 'my-org', FULL_SYNC_NOP: String(nop) }, + timeout: 10000 + }) + + expect(cli.error).toBeUndefined() + expect(cli.status).toBe(1) + expect(cli.stdout).toContain(`Starting full sync with NOP=${nop}`) + expect(cli.stdout).toContain("No app installation found for GH_ORG 'my-org'") + expect(cli.stdout).not.toContain('Wrong account synced') + expect(cli.stdout).not.toContain('Full sync completed successfully.') + }) +}) + +const flush = () => new Promise(resolve => setImmediate(resolve)) + +describe('syncInstallation fanout', () => { + const installation = (id, login, targetType = 'Organization') => ({ + id, target_type: targetType, account: { login, type: targetType } + }) + let plugin, ConfigManager, cron let robot, Settings, installations, appGithub, clients, loadConfig, savedEnv const createApp = async () => { @@ -29,10 +321,19 @@ describe('syncInstallation', () => { } beforeEach(() => { - savedEnv = { ...process.env } + jest.resetModules() + savedEnv = process.env + process.env = { ...savedEnv } delete process.env.CRON delete process.env.GH_ENTERPRISE delete process.env.GH_ORG + process.env.ADMIN_REPO = 'admin' + process.env.CONFIG_PATH = '.github' + process.env.SETTINGS_FILE_PATH = 'settings.yml' + process.env.DEPLOYMENT_CONFIG_FILE = 'test/fixtures/no-deployment-settings.yml' + plugin = require('../../index') + ConfigManager = require('../../lib/configManager') + cron = require('node-cron') installations = [installation(1, 'org-one'), installation(2, 'org-two')] clients = new Map() appGithub = { @@ -82,7 +383,11 @@ describe('syncInstallation', () => { expect(loadConfig.mock.instances[index].context).toBe(context) } expect(result).toEqual({ results: [{ errors: [] }, { errors: [] }], errors: [] }) - expect(robot.log.info.mock.calls).toEqual([['Synced 2 of 2 installation(s); 0 failed']]) + expect(robot.log.info.mock.calls).toEqual([ + ['Syncing installation 1 on account org-one'], + ['Syncing installation 2 on account org-two'], + ['Synced 2 of 2 installation(s); 0 failed'] + ]) }) it('waits for each authentication and sync before starting the next installation', async () => { @@ -307,12 +612,12 @@ describe('syncInstallation', () => { expect(robot.log.info).toHaveBeenCalledWith('Synced 1 of 2 installation(s); 1 failed') }) - it('logs scheduled enumeration failures without leaving an unhandled rejection', async () => { + it('logs and rejects scheduled enumeration failures for node-cron reporting', async () => { process.env.CRON = '* * * * *' await createApp() const failure = new Error('enumeration failed') appGithub.paginate.mockRejectedValueOnce(failure) - await cron.schedule.mock.calls[0][1]() + await expect(cron.schedule.mock.calls[0][1]()).rejects.toBe(failure) await flush() expect(robot.log.error).toHaveBeenCalledWith(expect.stringContaining('Scheduled full sync failed: Error: enumeration failed')) expect(Settings.syncAll).not.toHaveBeenCalled()