Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement multi-org config #524

Closed
wants to merge 10 commits into from
36 changes: 14 additions & 22 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -2,33 +2,25 @@
# Follow the instructions in README.md as how to add your personal
# tokens and secrets in this file

# GitHub App ID
GH_APP_IDENTIFIER=''
# Github App Webhook Secret to verify requests come from GitHub
GH_WEBHOOK_SECRET=''
# GitHub App Secret Key
# NOTE: this *must* be on a single line, otherwise it will break
# reading the secret key.
Copy link
Member Author

@chadwhitacre chadwhitacre Jul 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need docs in here since we tell people above to look for docs in the README. Let's only maintain docs in one place.

GH_APP_SECRET_KEY='-----BEGIN RSA PRIVATE KEY----------END RSA PRIVATE KEY-----'
# Personal (ie, for your own GitHub account) user access token with the `admin:org`,
# `admin:org_hook`, `admin: repo_hook`, `project`, `public_repo`, `repo:status`, and
# `repo_deployment` permissions enabled.
# GitHub
GH_USER_TOKEN=''
GH_WEBHOOK_SECRET=''

# Slack signing secret to verify requests come from Slack
SLACK_SIGNING_SECRET=
# Slack Bot user access token -> "Bot User OAuth Token"
SLACK_BOT_USER_ACCESS_TOKEN=
# GitHub Apps
GH_APP_1_ORG_SLUG=''
GH_APP_1_IDENTIFIER=''
GH_APP_1_SECRET_KEY='-----BEGIN RSA PRIVATE KEY----------END RSA PRIVATE KEY-----'
GH_APP_1_ISSUES_PROJECT_NODE_ID=''
GH_APP_1_PRODUCT_AREA_FIELD_ID=''
GH_APP_1_STATUS_FIELD_ID=''
GH_APP_1_RESPONSE_DUE_DATE_FIELD_ID=''

# Slack
SLACK_SIGNING_SECRET=''
SLACK_BOT_USER_ACCESS_TOKEN=''

# The name of the personal repository in your personal organization being used for development.
PERSONAL_TEST_REPO='eng-pipes-dev'

# Application-specific configuration
# Config for the "GitHub Issues Someone Else Cares About" project board
ISSUES_PROJECT_NODE_ID=''
STATUS_FIELD_ID=''
PRODUCT_AREA_FIELD_ID=''
RESPONSE_DUE_DATE_FIELD_ID=''

# Silence some GCP noise
DRY_RUN=true
20 changes: 18 additions & 2 deletions .env.test
Original file line number Diff line number Diff line change
@@ -1,9 +1,25 @@
# GitHub
FORCE_USER_TOKEN_GITHUB_CLIENT="false"
GH_USER_TOKEN="ghp_BLAHBLAHBLAH"
GH_APP_IDENTIFIER="1234"
GH_WEBHOOK_SECRET="webhooksecret"
GH_APP_SECRET_KEY="top \nsecret\n key"

# some functionality is org-agnostic
GH_APP_1_ORG_SLUG="Enterprise"
GH_APP_1_IDENTIFIER="1234"
GH_APP_1_SECRET_KEY="top \nsecret\n key"
GH_APP_1_ISSUES_PROJECT_NODE_ID="PVT_blahblah"
GH_APP_1_PRODUCT_AREA_FIELD_ID="PVTSSF_flooflah"
GH_APP_1_STATUS_FIELD_ID="PVTSSF_zimzimzim"
GH_APP_1_RESPONSE_DUE_DATE_FIELD_ID="PVTF_zamzamzam"

# other functionality is hardwired for getsentry
GH_APP_2_ORG_SLUG="getsentry"
GH_APP_2_IDENTIFIER="7890"
GH_APP_2_SECRET_KEY="super \nsecret\n key"
GH_APP_2_ISSUES_PROJECT_NODE_ID="PVT_flahflah"
GH_APP_2_PRODUCT_AREA_FIELD_ID="PVTSSF_moomah"
GH_APP_2_STATUS_FIELD_ID="PVTSSF_mizmizmi"
GH_APP_2_RESPONSE_DUE_DATE_FIELD_ID="PVTF_mazmaza"
chadwhitacre marked this conversation as resolved.
Show resolved Hide resolved

# Slack
SLACK_SIGNING_SECRET="slacksigningsecret"
Expand Down
39 changes: 31 additions & 8 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,20 +47,43 @@ Under Sentry's [webhooks](https://github.com/organizations/getsentry/settings/ho
### Setup Secrets

The following secrets are configured in GitHub for this app to function and to deploy to Google.
You can grab GitHub secrets in their respective configuration pages: [GitHub App](https://github.com/organizations/getsentry/settings/apps/getsantry)
You can grab GitHub secrets in their respective configuration pages: [getsantry](https://github.com/organizations/getsentry/settings/apps/getsantry), [covecod](https://github.com/organizations/codecov/settings/apps/covecod).

#### Local Secrets (required to run yarn dev)

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
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. See below for more instructions on [setting up a local environment](#configuring-a-test-environment).

##### GitHub

- `GH_WEBHOOK_SECRET` - GitHub webhook secret to confirm that webhooks come
from GitHub
- `GH_USER_TOKEN` - Personal (ie, for your own GitHub account) user access
token with the `read:org` and `read:user` permissions enabled.

##### GitHub Apps

You can configure multiple GitHub Apps to send events to one `eng-pipes`
installation. The config values are grouped together by number, so
`GH_APP_1_*`, `GH_APP_2_*`, etc.

- `GH_APP_1_ORG_SLUG` - Slug for the org where the GitHub app is installed
- `GH_APP_1_IDENTIFIER` - GitHub App identifier
- `GH_APP_1_SECRET_KEY` - GitHub App private key - NOTE: this *must* be on a single line, otherwise it will break reading the secret key (however, it can have literal `\n`s in it, which will be replaced with newlines)
- `GH_APP_1_ISSUES_PROJECT_NODE_ID` - node ID of the "GitHub Issues Someone Else Cares About" project board in the org
- `GH_APP_1_PRODUCT_AREA_FIELD_ID` - id of the "Product Area" field in the board
- `GH_APP_1_STATUS_FIELD_ID` - id of the "Status" field in the board
- `GH_APP_1_RESPONSE_DUE_DATE_FIELD_ID` - id of the "Response Due" field in the board

##### Slack

- `GH_APP_IDENTIFIER` - GitHub App identifier
- `GH_APP_SECRET_KEY` - GitHub App private key
- `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
- `SLACK_BOT_USER_ACCESS_TOKEN` - The Slack Bot User OAuth Access Token from the `Oauth & Permissions` section of your Slack app

Optional database configuration
##### CI

- `SENTRY_WEBPACK_WEBHOOK_SECRET` - Webhook secret that needs to match secret from CI. Records webpack asset sizes.

##### Database (Optional)

- `DB_USER` - Database user
- `DB_PASSWORD` - Database password
Expand Down Expand Up @@ -128,7 +151,7 @@ NOTE: These steps will cover more aspects over time. For now it focuses on testi

- In that organization, create a [new GitHub App](https://github.com/settings/apps/new)
- Set the webhook to your ngrok tunnel with the GH route (e.g. `<NGROK_INSTANCE>/webhooks/github`)
- Place the secrets in your `.env` (see [Setup Secrets](#setup-secrets) below)
- Place the secrets in your `.env` (see [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
Expand Down
20 changes: 18 additions & 2 deletions src/api/github/getClient.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { createAppAuth } from '@octokit/auth-app';

import { GETSENTRY_ORG } from '@/config';
import { ClientType } from '@api/github/clientType';
import { getClient } from '@api/github/getClient';
import { OctokitWithRetries as octokitClass } from '@api/github/octokitWithRetries';
Expand All @@ -23,10 +24,10 @@ describe('getClient(ClientType.User)', function () {
});
});

describe("getClient(ClientType.App, 'getsentry')", function () {
describe("getClient(ClientType.App, 'Enterprise')", function () {
chadwhitacre marked this conversation as resolved.
Show resolved Hide resolved
beforeAll(async function () {
octokitClass.mockClear();
await getClient(ClientType.App, 'getsentry');
await getClient(ClientType.App, 'Enterprise');
});

it('is instantiated twice', async function () {
Expand All @@ -43,6 +44,21 @@ describe("getClient(ClientType.App, 'getsentry')", function () {
auth: {
appId: 1234,
privateKey: 'top \nsecret\n key',
installationId: 'installation-Enterprise',
},
});
});
});

describe('getClient(ClientType.App, GETSENTRY_ORG)', function () {
it('also knows about getsentry', async function () {
octokitClass.mockClear();
const actual = await getClient(ClientType.App, GETSENTRY_ORG);
expect(octokitClass).toHaveBeenLastCalledWith({
authStrategy: createAppAuth,
auth: {
appId: 7890,
privateKey: 'super \nsecret\n key',
installationId: 'installation-getsentry',
},
});
Expand Down
3 changes: 1 addition & 2 deletions src/api/github/getClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,10 @@ export async function getClient(type: ClientType, org?: string | null) {
);
}

const app = GH_APPS.get('__tmp_org_placeholder__');

let client = _CLIENTS_BY_ORG.get(org);
if (client === undefined) {
// Bootstrap with a client not bound to an org.
const app = GH_APPS.get(org);
const appClient = _getAppClient(app.auth);

// Use the unbound client to hydrate a client bound to an org.
Expand Down
2 changes: 1 addition & 1 deletion src/brain/ghaCancel/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ describe('gha-test', function () {
});
beforeEach(async function () {
await db('users').delete();
octokit = await getClient(ClientType.App, 'getsentry');
octokit = await getClient(ClientType.App, 'Enterprise');
fastify = await buildServer(false);
ghaCancel();
// @ts-ignore
Expand Down
2 changes: 1 addition & 1 deletion src/brain/githubMetrics/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ describe('github webhook', function () {

beforeEach(async function () {
fastify = await buildServer(false);
octokit = await getClient(ClientType.App, 'getsentry');
octokit = await getClient(ClientType.App, 'Enterprise');
metrics();
});

Expand Down
4 changes: 2 additions & 2 deletions src/brain/gocdSlackFeeds/deployFeed.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,7 @@ describe('DeployFeed', () => {
});

it('post message with commits in deploy link for getsentry', async () => {
const octokit = await getClient(ClientType.App, GETSENTRY_ORG);
const octokit = await getClient(ClientType.App, 'Enterprise');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth parameterizing the somehow for the extra coverage? Not sure if we're mocking the replies, in which case it doesn't matter, but maybe worth considering.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are mocking the replies. 🐭

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oooh ... this one wants to be GETSENTRY_ORG though, good catch!

We end up testing both getClient cases pretty thoroughly since both are called a ton throughout the test suite.

octokit.repos.getContent.mockImplementation((args) => {
if (args.owner != 'getsentry') {
throw new Error(`Unexpected getContent() owner: ${args.owner}`);
Expand Down Expand Up @@ -647,7 +647,7 @@ describe('DeployFeed', () => {
});

it('handle error if get content fails', async () => {
const octokit = await getClient(ClientType.App, GETSENTRY_ORG);
const octokit = await getClient(ClientType.App, 'Enterprise');
octokit.repos.getContent.mockImplementation((args) => {
throw new Error('Injected error');
});
Expand Down
18 changes: 8 additions & 10 deletions src/brain/issueLabelHandler/followups.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,7 @@ export async function updateCommunityFollowups({
return;
}

const owner = payload.repository.owner.login;
const octokit = await getClient(ClientType.App, owner);
const octokit = await getClient(ClientType.App, app.org);
chadwhitacre marked this conversation as resolved.
Show resolved Hide resolved
const repo = payload.repository.name;
const issueNumber = payload.issue.number;

Expand All @@ -88,16 +87,16 @@ export async function updateCommunityFollowups({

if (isWaitingForCommunityLabelOnIssue) {
await octokit.issues.removeLabel({
owner,
repo: repo,
owner: app.org,
repo,
issue_number: issueNumber,
name: WAITING_FOR_COMMUNITY_LABEL,
});
}

await octokit.issues.addLabels({
owner,
repo: repo,
owner: app.org,
repo,
issue_number: issueNumber,
labels: [WAITING_FOR_PRODUCT_OWNER_LABEL],
});
Expand Down Expand Up @@ -142,8 +141,7 @@ export async function ensureOneWaitingForLabel({
}

const { issue, label } = payload;
const owner = payload.repository.owner.login;
const octokit = await getClient(ClientType.App, owner);
const octokit = await getClient(ClientType.App, app.org);
const repo = payload.repository.name;
const issueNumber = payload.issue.number;
// Here label will never be undefined, ts is erroring here but is handled in the shouldSkip above
Expand All @@ -156,8 +154,8 @@ export async function ensureOneWaitingForLabel({

if (labelToRemove != null) {
await octokit.issues.removeLabel({
owner: owner,
repo: repo,
owner: app.org,
repo,
issue_number: issueNumber,
name: labelToRemove,
});
Expand Down
2 changes: 1 addition & 1 deletion src/brain/issueLabelHandler/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { issueLabelHandler } from '.';
describe('issueLabelHandler', function () {
let fastify: Fastify;
let octokit;
const app = GH_APPS.get('__tmp_org_placeholder__');
const app = GH_APPS.get('Enterprise');
const errors = jest.fn();
let say, respond, client, ack;
let calculateSLOViolationRouteSpy, calculateSLOViolationTriageSpy;
Expand Down
20 changes: 9 additions & 11 deletions src/brain/issueLabelHandler/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,18 +81,17 @@ export async function markWaitingForSupport({
const issueNumber = payload.issue.number;

// New issues get a Waiting for: Support label.
const owner = payload.repository.owner.login;
const octokit = await getClient(ClientType.App, owner);
const octokit = await getClient(ClientType.App, app.org);
await octokit.issues.addLabels({
owner,
repo: repo,
owner: app.org,
repo,
issue_number: issueNumber,
labels: [WAITING_FOR_SUPPORT_LABEL],
});

await octokit.issues.createComment({
owner,
repo: repo,
owner: app.org,
repo,
issue_number: issueNumber,
body: `Assigning to @${GETSENTRY_ORG}/support for [routing](https://open.sentry.io/triage/#2-route) ⏲️`,
});
Expand Down Expand Up @@ -137,8 +136,7 @@ export async function markNotWaitingForSupport({
const { issue, label } = payload;
const productAreaLabel = label;
const productAreaLabelName = productAreaLabel?.name || PRODUCT_AREA_UNKNOWN;
const owner = payload.repository.owner.login;
const octokit = await getClient(ClientType.App, owner);
const octokit = await getClient(ClientType.App, app.org);
const labelsToRemove: string[] = [];
const labelNames = issue?.labels?.map((label) => label.name) || [];
const isBeingRoutedBySupport = labelNames.includes(WAITING_FOR_SUPPORT_LABEL);
Expand All @@ -153,7 +151,7 @@ export async function markNotWaitingForSupport({
for (const label of labelsToRemove) {
try {
await octokit.issues.removeLabel({
owner: owner,
owner: app.org,
repo: payload.repository.name,
issue_number: payload.issue.number,
name: label,
Expand All @@ -175,7 +173,7 @@ export async function markNotWaitingForSupport({
// Only retriage issues if support is routing
if (isBeingRoutedBySupport) {
await octokit.issues.addLabels({
owner: owner,
owner: app.org,
repo: payload.repository.name,
issue_number: payload.issue.number,
labels: [WAITING_FOR_PRODUCT_OWNER_LABEL],
Expand All @@ -185,7 +183,7 @@ export async function markNotWaitingForSupport({
const comment = await routeIssue(octokit, productAreaLabelName);

await octokit.issues.createComment({
owner,
owner: app.org,
repo: payload.repository.name,
issue_number: payload.issue.number,
body: comment,
Expand Down
12 changes: 5 additions & 7 deletions src/brain/issueLabelHandler/triage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,13 @@ export async function markWaitingForProductOwner({
}

// New issues get an Untriaged label.
const owner = payload.repository.owner.login;
const octokit = await getClient(ClientType.App, owner);
const octokit = await getClient(ClientType.App, app.org);
const repo = payload.repository.name;
const issueNumber = payload.issue.number;

await octokit.issues.addLabels({
owner,
repo: repo,
owner: app.org,
repo,
issue_number: issueNumber,
labels: [WAITING_FOR_PRODUCT_OWNER_LABEL],
});
Expand Down Expand Up @@ -114,11 +113,10 @@ export async function markNotWaitingForProductOwner({
}

// Remove Untriaged label when triaged.
const owner = payload.repository.owner.login;
const octokit = await getClient(ClientType.App, owner);
const octokit = await getClient(ClientType.App, app.org);
try {
await octokit.issues.removeLabel({
owner: owner,
owner: app.org,
repo: payload.repository.name,
issue_number: payload.issue.number,
name: WAITING_FOR_PRODUCT_OWNER_LABEL,
Expand Down
2 changes: 1 addition & 1 deletion src/brain/notifyOnGoCDStageEvent/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ describe('notifyOnGoCDStageEvent', function () {
await pleaseDeployNotifier();
await notifyOnGoCDStageEvent();

octokit = await getClient(ClientType.App, 'getsentry');
octokit = await getClient(ClientType.App, 'Enterprise');
octokit.paginate.mockImplementation(() => {
return [{ name: 'only frontend changes', conclusion: 'success' }];
});
Expand Down
2 changes: 1 addition & 1 deletion src/brain/pleaseDeployNotifier/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ describe('pleaseDeployNotifier', function () {
jest.spyOn(actions, 'actionViewUndeployedCommits');

pleaseDeployNotifier();
octokit = await getClient(ClientType.App, 'getsentry');
octokit = await getClient(ClientType.App, 'Enterprise');
octokit.repos.getCommit.mockImplementation(({ repo, ref }) => {
const defaultPayload = require('@test/payloads/github/commit').default;
if (repo === 'sentry') {
Expand Down
Loading