From eb2449d30d8c0fd741a5ab18bf9b37f924bce092 Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Mon, 24 Jul 2023 09:47:11 -0400 Subject: [PATCH] Store installationId in config --- README.md | 18 ++++++++------ github-orgs.yml | 1 + src/api/github/org.test.ts | 40 +++---------------------------- src/api/github/org.ts | 22 +---------------- src/buildServer.ts | 5 +--- src/config/loadGitHubOrgs.test.ts | 12 +++++++--- src/config/loadGitHubOrgs.ts | 16 +++++-------- src/types/index.ts | 2 +- test/github-orgs.good.yml | 2 ++ test/github-orgs.yml | 1 + 10 files changed, 36 insertions(+), 83 deletions(-) diff --git a/README.md b/README.md index a5365cf9..fdac069d 100644 --- a/README.md +++ b/README.md @@ -53,7 +53,7 @@ You can grab GitHub secrets in their respective configuration pages: [GitHub App You will also need to set up some of these environment variables if you want to test this locally, e.g. using `direnv` or something similar -- `GH_APP_PRIVATE_KEY` - GitHub App private key for your test app. It needs to all be on one line, but it can include literal '\n' which will be converted to newlines. +- `GH_APP_PRIVATE_KEY` - GitHub App private key for your test app. It needs to all be on one line, with literal `\n`s instead of newlines (these seem to be required). - `GH_WEBHOOK_SECRET` - GitHub webhook secret to confirm that webhooks come from GitHub - `SENTRY_WEBPACK_WEBHOOK_SECRET` - Webhook secret that needs to match secret from CI. Records webpack asset sizes. - `SLACK_SIGNING_SECRET` - Slack webhook secret to verify payload @@ -123,21 +123,21 @@ You'll also need to create a private key for the service account (it should down 1. [Create a GitHub organization](https://github.com/account/organizations/new?plan=free). Name it what you like. - - In your organization, create a new GitHub repository named something like `testing-eng-pipes`. - After the organization has been created, go to its `Settings`, then select `Personal access tokens` from the sidebar to enable tokens. - In the setup menu, select `Allow access via fine-grained personal access tokens`, then `Do not require administrator approval`, and finally `Allow access via personal access tokens (classic)`. + - In your organization, create a new repository named something like `testing-eng-pipes`. 1. [Create a personal access token](https://github.com/settings/tokens/new). - Title the new token `Eng-pipes development token`, give this token 90 days until expiration, and enable the following permissions: `read:org` and `read:user`. - On the next page, copy the displayed token into the `GH_USER_TOKEN` field of your `.env` file. - > :warning: **You are giving this token permissions to all orgs across GitHub that you are a member of (though some, like `getsentry`, are configured to require approval before PATs have access)). Be careful and ensure it does not leave your machine!** + > :warning: **You are giving this token permissions to all orgs across GitHub that you are a member of (though some, like `getsentry`, are configured to require approval before PATs have access). Be careful and ensure it does not leave your machine!** 1. [Create a GitHub App](https://github.com/settings/apps/new). - Set the webhook to your ngrok tunnel with the GH route (e.g. `/webhooks/github`) - - Create and download a private key and add it to your `.env` under `GH_APP_PRIVATE_KEY`. You'll need to strip newlines (or convert them to literal `\n`). (See [Setup Secrets](#setup-secrets) above.) + - Create and download a private key and add it to your `.env` under `GH_APP_PRIVATE_KEY`. You'll need to convert newlines to literal `\n`. (See also [Setup Secrets](#setup-secrets) above.) - Go to the `Permissions & events` sidebar menu entry of the GitHub app configuration, and grant maximum non-`Admin` access (`Read and write` where possible, `Read only` everywhere else) for every line in `Repository permissions` (NOTE: We use a more constrained permission-set in production, but for initial setup enabling maximal permissions is fine; permissions can be whittled down later as needed.) - For `Organization permissions`, grant `Read and write` for `Members` and `Projects` - In the `Subscribe to events` section, check every possible box @@ -159,15 +159,19 @@ You'll also need to create a private key for the service account (it should down - Set an environment variable, `GH_ORGS_YML=github-orgs.local.yml`. - - Modify it with the slug of your organization and the ID of your app. + - Modify the top-level key to the slug of your organization. + + - Set `appId` to the "App ID" from General > About in the UI for your app. + + - Set `installationId` to the ID at the end of the URL for your app's installation on your org (confused yet?). - Leave the `privateKey` as-is, it's the name of an environment variable to pull from (the main `github-orgs.yml` holds public config and is checked into version control). - In a terminal, log into the Github CLI using `gh auth login`. - - Use [this](https://docs.github.com/en/issues/planning-and-tracking-with-projects/automating-your-project/using-the-api-to-manage-projects#finding-the-node-id-of-an-organization-project) GraphQL query to identify the node ID of the project you made earlier; set `nodeId` to match. + - Use [this](https://docs.github.com/en/issues/planning-and-tracking-with-projects/automating-your-project/using-the-api-to-manage-projects#finding-the-node-id-of-an-organization-project) GraphQL query to identify the node ID of the project you made earlier; set `project.nodeId` to match. - - Use [this](https://docs.github.com/en/issues/planning-and-tracking-with-projects/automating-your-project/using-the-api-to-manage-projects#finding-the-node-id-of-a-field) GraphQL query to identify the IDs of the project fields you set up, and use those to populate `fieldIds`. + - Use [this](https://docs.github.com/en/issues/planning-and-tracking-with-projects/automating-your-project/using-the-api-to-manage-projects#finding-the-node-id-of-a-field) GraphQL query to identify the IDs of the project fields you set up, and use those to populate `project.fieldIds`. 1. Follow the steps of the "Development & tests" section below to get the server running. diff --git a/github-orgs.yml b/github-orgs.yml index 1d171faf..29ed8c45 100644 --- a/github-orgs.yml +++ b/github-orgs.yml @@ -2,6 +2,7 @@ getsentry: appAuth: appId: 66573 privateKey: 'GH_APP_PRIVATE_KEY_FOR_GETSENTRY' + installationId: 9303463 project: nodeId: 'PVT_kwDOABVQ184AOGW8' fieldIds: diff --git a/src/api/github/org.test.ts b/src/api/github/org.test.ts index 7dc54163..3d31e25d 100644 --- a/src/api/github/org.test.ts +++ b/src/api/github/org.test.ts @@ -12,6 +12,7 @@ describe('constructor', function () { appAuth: { appId: 423, privateKey: 'so secret', + installationId: 432, }, }); }); @@ -20,20 +21,17 @@ describe('constructor', function () { expect(octokitClass).toHaveBeenCalledTimes(1); }); - it('is instantiated with unbound appAuth', async function () { + it('is instantiated with appAuth', async function () { expect(octokitClass).toHaveBeenCalledWith({ authStrategy: createAppAuth, auth: { appId: 423, privateKey: 'so secret', + installationId: 432, }, }); }); - it('does not try to get an org installation', async function () { - expect(octokitClass.apps.getOrgInstallation).toHaveBeenCalledTimes(0); - }); - it('combines repos into .all', async function () { const org = new GitHubOrg('barn', { repos: { @@ -56,38 +54,6 @@ describe('constructor', function () { }); }); -describe('bindAPI', function () { - beforeAll(async function () { - const org = new GitHubOrg('banana', { - appAuth: { - appId: 422, - privateKey: 'so private', - }, - }); - octokitClass.mockClear(); - org.bindAPI(); - }); - - it('is instantiated once again', async function () { - expect(octokitClass).toHaveBeenCalledTimes(1); - }); - - it('tries to get an org installation', async function () { - expect(octokitClass.apps.getOrgInstallation).toHaveBeenCalledTimes(1); - }); - - it('is instantiated the second time with authStrategy and auth', async function () { - expect(octokitClass).toHaveBeenLastCalledWith({ - authStrategy: createAppAuth, - auth: { - appId: 422, - privateKey: 'so private', - installationId: 'installation-banana', - }, - }); - }); -}); - describe('helpers', function () { const org = GETSENTRY_ORG; diff --git a/src/api/github/org.ts b/src/api/github/org.ts index f5700527..de473d20 100644 --- a/src/api/github/org.ts +++ b/src/api/github/org.ts @@ -44,30 +44,10 @@ export class GitHubOrg { } this.repos.all = [...this.repos.withRouting, ...this.repos.withoutRouting]; - // Call bindAPI ASAP. We can't call it here because constructors can't be - // async. Note that in testing this ends up being mocked as if it were - // bound to an org installation, even though (afaict) we generally don't - // call bindAPI in test. - this.api = new OctokitWithRetries({ - authStrategy: createAppAuth, - auth: this.appAuth, - }); - } - - async bindAPI() { if (FORCE_USER_TOKEN_GITHUB_CLIENT) { // Hack for easier dev, avoids setting up a test org. this.api = makeUserTokenClient(); - return; - } - - // Use the unbound Octokit instantiated in the constructor to make an - // Octokit bound to our org, now that we can await. - if (this.appAuth.installationId === undefined) { - const installation = await this.api.apps.getOrgInstallation({ - org: this.slug, - }); - this.appAuth.installationId = installation.data.id; + } else { this.api = new OctokitWithRetries({ authStrategy: createAppAuth, auth: this.appAuth, diff --git a/src/buildServer.ts b/src/buildServer.ts index c7f2e980..6a9ed193 100644 --- a/src/buildServer.ts +++ b/src/buildServer.ts @@ -14,16 +14,13 @@ import { bolt } from '@api/slack'; import { loadBrain } from '@utils/loadBrain'; import * as PubSub from './webhooks/pubsub'; -import { GH_ORGS, SENTRY_DSN } from './config'; +import { SENTRY_DSN } from './config'; export async function buildServer( logger: boolean | { prettyPrint: boolean } = { prettyPrint: process.env.NODE_ENV === 'development', } ) { - // Oh, for top-level await! - await GH_ORGS.bindAPIs(); - const server: Fastify = fastify({ logger, disableRequestLogging: true, diff --git a/src/config/loadGitHubOrgs.test.ts b/src/config/loadGitHubOrgs.test.ts index cc8fbc12..db20c2ff 100644 --- a/src/config/loadGitHubOrgs.test.ts +++ b/src/config/loadGitHubOrgs.test.ts @@ -16,12 +16,18 @@ describe('loadGitHubOrgs', function () { expect(orgs.get('hurple').appAuth.appId).toEqual(42); }); - it('mixes in a private key from the environment', async function () { + it('loads the main github-orgs.yml in a prod simulation', async function () { const org = loadGitHubOrgs({ GH_APP_PRIVATE_KEY_FOR_GETSENTRY: 'cheese', + // GH_ORGS_YML - not set in prod }).get('getsentry'); - expect(org.appAuth.privateKey).toEqual('cheese'); - expect(org.appAuth.appId).toEqual(66573); + expect(org.appAuth).toEqual({ + appId: 66573, + privateKey: 'cheese', + installationId: 9303463, + }); + + // Spot-check repos for good measure. expect(org.repos.withRouting).toEqual(['sentry', 'sentry-docs']); }); diff --git a/src/config/loadGitHubOrgs.ts b/src/config/loadGitHubOrgs.ts index bf4be105..fe601aac 100644 --- a/src/config/loadGitHubOrgs.ts +++ b/src/config/loadGitHubOrgs.ts @@ -20,12 +20,6 @@ export class GitHubOrgs { } } - async bindAPIs() { - for (const org of this.orgs.values()) { - await org.bindAPI(); - } - } - get(orgSlug) { const org = this.orgs.get(orgSlug); if (org === undefined) { @@ -75,11 +69,13 @@ export function loadGitHubOrgs(env) { const config = _config as GitHubOrgConfig; // IDs - const appId = parseInt(config.appAuth.appId, 10); - if (Number.isNaN(appId)) { - throw `appId '${config.appAuth.appId}' is not a number`; + for (const idVar of ['appId', 'installationId']) { + const tmp = parseInt(config.appAuth[idVar], 10); + if (Number.isNaN(tmp)) { + throw `${idVar} '${config.appAuth[idVar]}' is not a number`; + } + config.appAuth[idVar] = tmp; } - config.appAuth.appId = appId; // Key const keyEnvVarName = config.appAuth.privateKey; diff --git a/src/types/index.ts b/src/types/index.ts index b01462f6..f60c67e0 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -32,7 +32,7 @@ export type Annotation = GetResponseDataTypeFromEndpointMethod< export interface AppAuthStrategyOptions { appId: number; privateKey: string; - installationId?: number; + installationId: number; } export interface GitHubIssuesSomeoneElseCaresAbout { diff --git a/test/github-orgs.good.yml b/test/github-orgs.good.yml index 6c7d2590..e0ab197b 100644 --- a/test/github-orgs.good.yml +++ b/test/github-orgs.good.yml @@ -2,6 +2,7 @@ zer-ner: appAuth: appId: '53' privateKey: 'GH_KEY_BLAH' + installationId: 54 project: nodeId: 'bloo' fieldIds: @@ -12,6 +13,7 @@ hurple: appAuth: appId: 42 privateKey: 'GH_KEY_ZAR' + installationId: 420 project: nodeId: 'zer' fieldIds: diff --git a/test/github-orgs.yml b/test/github-orgs.yml index 0f42a7f9..3cf989e3 100644 --- a/test/github-orgs.yml +++ b/test/github-orgs.yml @@ -2,6 +2,7 @@ getsentry: appAuth: appId: -1 privateKey: 'STUB' + installationId: -1 project: nodeId: '' fieldIds: