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.

Sign up for GitHub

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
Closed

Implement multi-org config #524

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
0da91ed
Implement multi-org config
chadwhitacre Jul 13, 2023
d86f107
Update README and .env.example
chadwhitacre Jul 14, 2023
be1caa7
Fix regressions in 0da91edc42ea1c0ce24222b49ae82a0
chadwhitacre Jul 14, 2023
78f1fbf
Add a test for getClient(.., GETSENTRY_ORG)
chadwhitacre Jul 14, 2023
2a616cd
Tighten up & test error handling in getForPayload
chadwhitacre Jul 14, 2023
834c2b4
Implement multi-org in pubsub handler
chadwhitacre Jul 14, 2023
580ddd6
Satisfy CodeQL
chadwhitacre Jul 17, 2023
16166f5
Update getClient call in a couple tests
chadwhitacre Jul 18, 2023
18176b4
Consolidate a couple imports
chadwhitacre Jul 21, 2023
61d3556
Pick off a couple app.org locations
chadwhitacre Jul 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions .env.test
View file
Open in desktop
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"

# Slack
SLACK_SIGNING_SECRET="slacksigningsecret"
Expand Down
6 changes: 3 additions & 3 deletions src/api/github/getClient.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ describe('getClient(ClientType.User)', function () {
});
});

describe("getClient(ClientType.App, 'getsentry')", function () {
describe("getClient(ClientType.App, 'Enterprise')", function () {
beforeAll(async function () {
octokitClass.mockClear();
await getClient(ClientType.App, 'getsentry');
await getClient(ClientType.App, 'Enterprise');
});

it('is instantiated twice', async function () {
Expand All @@ -43,7 +43,7 @@ describe("getClient(ClientType.App, 'getsentry')", function () {
auth: {
appId: 1234,
privateKey: 'top \nsecret\n key',
installationId: 'installation-getsentry',
installationId: 'installation-Enterprise',
},
});
});
Expand Down
3 changes: 1 addition & 2 deletions src/api/github/getClient.ts
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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);
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
6 changes: 3 additions & 3 deletions src/brain/projectsHandler/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { projectsHandler } from '.';
describe('projectsHandler', function () {
let fastify: Fastify;
let octokit;
const app = GH_APPS.get('__tmp_org_placeholder__');
const app = GH_APPS.load('Enterprise');
const errors = jest.fn();

beforeAll(async function () {
Expand All @@ -38,7 +38,7 @@ describe('projectsHandler', function () {
beforeEach(async function () {
fastify = await buildServer(false);
await projectsHandler();
octokit = await getClient(ClientType.App, 'test-org');
octokit = await getClient(ClientType.App, 'Enterprise');
});

afterEach(async function () {
Expand All @@ -59,7 +59,7 @@ describe('projectsHandler', function () {
fieldNodeId?: string
) {
const projectPayload = {
organization: { login: 'test-org' },
organization: { login: 'Enterprise' },
projects_v2_item: {
project_node_id: projectNodeId || 'test-project-node-id',
node_id: 'test-node-id',
Expand Down
5 changes: 2 additions & 3 deletions src/brain/projectsHandler/project.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,7 @@ export async function syncLabelsWithProjectField({
return;
}

const owner = payload?.organization?.login || '';
const octokit = await getClient(ClientType.App, owner);
const octokit = await getClient(ClientType.App, app.org);
const fieldName = getFieldName(payload, app);
const fieldValue = await getKeyValueFromProjectField(
payload.projects_v2_item.node_id,
Expand All @@ -84,7 +83,7 @@ export async function syncLabelsWithProjectField({
);

await octokit.issues.addLabels({
owner,
owner: app.org,
repo: issueInfo.repo,
issue_number: issueInfo.number,
labels: [
Expand Down
3 changes: 2 additions & 1 deletion src/brain/requiredChecks/getAnnotations.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { GETSENTRY_ORG } from '@/config';
import { ClientType } from '@api/github/clientType';
import { getClient } from '@api/github/getClient';

Expand All @@ -7,7 +8,7 @@ describe('getAnnotations', function () {
let octokit;

beforeAll(async function () {
octokit = await getClient(ClientType.App, 'getsentry');
octokit = await getClient(ClientType.App, GETSENTRY_ORG);
});

beforeEach(function () {
Expand Down
3 changes: 2 additions & 1 deletion src/brain/requiredChecks/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { createGitHubEvent } from '@test/utils/github';
import { buildServer } from '@/buildServer';
import {
BuildStatus,
GETSENTRY_ORG,
REQUIRED_CHECK_CHANNEL,
REQUIRED_CHECK_NAME,
} from '@/config';
Expand Down Expand Up @@ -66,7 +67,7 @@ describe('requiredChecks', function () {
beforeEach(async function () {
fastify = await buildServer(false);
await requiredChecks();
octokit = await getClient(ClientType.App, 'getsentry');
octokit = await getClient(ClientType.App, GETSENTRY_ORG);

octokit.repos.getCommit.mockImplementation(({ repo, ref }) => {
const defaultPayload = require('@test/payloads/github/commit').default;
Expand Down
Loading