Skip to content

Commit

Permalink
Make stalebot multi-org
Browse files Browse the repository at this point in the history
  • Loading branch information
chadwhitacre committed Jul 25, 2023
1 parent 2246a3a commit 4c49ecd
Show file tree
Hide file tree
Showing 15 changed files with 139 additions and 131 deletions.
1 change: 0 additions & 1 deletion .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ GH_USER_TOKEN=''
GH_WEBHOOK_SECRET=''
GH_APP_PRIVATE_KEY='-----BEGIN RSA PRIVATE KEY----------END RSA PRIVATE KEY-----'
GH_ORGS_YAML='github-orgs.local.yml'
PERSONAL_TEST_REPO='testing-eng-pipes'

# Slack
SLACK_SIGNING_SECRET=''
Expand Down
7 changes: 6 additions & 1 deletion github-orgs.example.yml
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
your-org-CHANGEME:
appAuth:
appId: ''
privateKey: 'GH_APP_PRIVATE_KEY' # this is the name of an env var to pull from
privateKey: 'GH_APP_PRIVATE_KEY' # leave this, it names an env var to pull from
project:
nodeId: ''
fieldIds:
productArea: ''
status: ''
responseDue: ''
repos:
withRouting:
- 'foo'
withoutRouting:
- 'bar'
38 changes: 38 additions & 0 deletions github-orgs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,41 @@ getsentry:
productArea: 'PVTSSF_lADOABVQ184AOGW8zgJEBno'
status: 'PVTSSF_lADOABVQ184AOGW8zgI_7g0'
responseDue: 'PVTF_lADOABVQ184AOGW8zgLLxGg'
repos:
withRouting:
- 'sentry'
- 'sentry-docs'

withoutRouting:
- 'arroyo'
- 'cdc'
- 'craft'
- 'relay'
- 'responses'
- 'self-hosted'
- 'sentry-native'
- 'snuba'
- 'snuba-sdk'
- 'symbolic'
- 'symbolicator'
- 'test-ttt-simple'
- 'wal2json'

# Web team T1
- 'sentry-javascript'
- 'sentry-python'
- 'sentry-php'
- 'sentry-laravel'
- 'sentry-symfony'
- 'sentry-ruby'

# Mobile team T1
# www.notion.so/346452f21e7947b4bf515d5f3a4d497d?v=cad7f04cf9064e7483ab426a26d3923a
- 'sentry-cocoa'
- 'sentry-java'
- 'sentry-react-native'
- 'sentry-unity'
- 'sentry-dart'
- 'sentry-android-gradle-plugin'
- 'sentry-dotnet'
- 'sentry-dart-plugin'
19 changes: 19 additions & 0 deletions src/api/github/org.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,25 @@ describe('constructor', function () {
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: {
withRouting: ['cheese'],
withoutRouting: ['bread'],
},
});
expect(org.repos.all).toEqual(['cheese', 'bread']);
});

it('is fine without one of them', async function () {
const org = new GitHubOrg('barn', {
repos: {
withRouting: ['cheese', 'wine'],
},
});
expect(org.repos.all).toEqual(['cheese', 'wine']);
});
});

describe('bindAPI', function () {
Expand Down
7 changes: 7 additions & 0 deletions src/api/github/org.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
AppAuthStrategyOptions,
GitHubIssuesSomeoneElseCaresAbout,
GitHubOrgConfig,
GitHubOrgRepos,
} from '@/types';

// We can't use @ to import config here or we get an error from jest due to
Expand All @@ -19,6 +20,7 @@ export class GitHubOrg {
slug: string;
appAuth: AppAuthStrategyOptions;
project: GitHubIssuesSomeoneElseCaresAbout;
repos: GitHubOrgRepos;

// The docs say it's safe for Octokit instances to be long-lived:
//
Expand All @@ -33,6 +35,11 @@ export class GitHubOrg {
this.slug = orgSlug;
this.appAuth = config.appAuth;
this.project = config.project;
this.repos = config.repos || { withRouting: [], withoutRouting: [] };
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
Expand Down
5 changes: 2 additions & 3 deletions src/brain/issueLabelHandler/followups.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import moment from 'moment-timezone';

import {
GH_ORGS,
SENTRY_REPOS,
WAITING_FOR_COMMUNITY_LABEL,
WAITING_FOR_LABEL_PREFIX,
WAITING_FOR_PRODUCT_OWNER_LABEL,
Expand All @@ -18,8 +17,8 @@ import { isFromABot } from '@utils/isFromABot';
import { isNotFromAnExternalOrGTMUser } from '@utils/isNotFromAnExternalOrGTMUser';
import { shouldSkip } from '@utils/shouldSkip';

function isNotInARepoWeCareAboutForFollowups(payload) {
return !SENTRY_REPOS.has(payload.repository.name);
function isNotInARepoWeCareAboutForFollowups(payload, org) {
return !org.repos.all.includes(payload.repository.name);
}

function isNotWaitingForLabel(payload) {
Expand Down
5 changes: 2 additions & 3 deletions src/brain/issueLabelHandler/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import {
IN_PROGRESS_LABEL,
PRODUCT_AREA_LABEL_PREFIX,
PRODUCT_AREA_UNKNOWN,
SENTRY_REPOS_WITH_ROUTING,
WAITING_FOR_PRODUCT_OWNER_LABEL,
WAITING_FOR_SUPPORT_LABEL,
} from '@/config';
Expand All @@ -21,8 +20,8 @@ function isAlreadyWaitingForSupport(payload) {
);
}

function isNotInARepoWeCareAboutForRouting(payload) {
return !SENTRY_REPOS_WITH_ROUTING.has(payload.repository.name);
function isNotInARepoWeCareAboutForRouting(payload, org) {
return !org.repos.withRouting.includes(payload.repository.name);
}

function isValidLabel(payload) {
Expand Down
10 changes: 3 additions & 7 deletions src/brain/issueLabelHandler/triage.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,7 @@
import { EmitterWebhookEvent } from '@octokit/webhooks';
import * as Sentry from '@sentry/node';

import {
GH_ORGS,
SENTRY_REPOS_WITHOUT_ROUTING,
WAITING_FOR_PRODUCT_OWNER_LABEL,
} from '@/config';
import { GH_ORGS, WAITING_FOR_PRODUCT_OWNER_LABEL } from '@/config';
import { isFromABot } from '@utils/isFromABot';
import { isNotFromAnExternalOrGTMUser } from '@utils/isNotFromAnExternalOrGTMUser';
import { shouldSkip } from '@utils/shouldSkip';
Expand All @@ -20,8 +16,8 @@ function isAlreadyTriaged(payload) {
);
}

function isNotInARepoWeCareAboutForTriage(payload) {
return !SENTRY_REPOS_WITHOUT_ROUTING.has(payload.repository.name);
function isNotInARepoWeCareAboutForTriage(payload, org) {
return !org.repos.withoutRouting.includes(payload.repository.name);
}

function isWaitingForProductOwnerLabel(payload) {
Expand Down
52 changes: 0 additions & 52 deletions src/config/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,58 +106,6 @@ export const WAITING_FOR_PRODUCT_OWNER_LABEL = 'Waiting for: Product Owner';
export const MAX_TRIAGE_DAYS = 2;
export const MAX_ROUTE_DAYS = 1;

// Only add the `PERSONAL_TEST_REPO` to the array of `SENTRY_REPOS_WITH_ROUTING` if it has actually been set
// in the instantiating environment.
export const PERSONAL_TEST_REPO = process.env.PERSONAL_TEST_REPO;
export const PERSONAL_TEST_REPOS = PERSONAL_TEST_REPO
? [PERSONAL_TEST_REPO]
: [];

export const SENTRY_REPOS_WITH_ROUTING: Set<string> = new Set([
'sentry',
'sentry-docs',
...PERSONAL_TEST_REPOS,
]);
export const SENTRY_REPOS_WITHOUT_ROUTING: Set<string> = new Set([
'arroyo',
'cdc',
'craft',
'relay',
'responses',
'self-hosted',
'sentry-native',
'snuba',
'snuba-sdk',
'symbolic',
'symbolicator',
'test-ttt-simple',
'wal2json',

// Web team, T1
'sentry-javascript',
'sentry-python',
'sentry-php',
'sentry-laravel',
'sentry-symfony',
'sentry-ruby',

// Mobile team, T1
// https://www.notion.so/sentry/346452f21e7947b4bf515d5f3a4d497d?v=cad7f04cf9064e7483ab426a26d3923a
'sentry-cocoa',
'sentry-java',
'sentry-react-native',
'sentry-unity',
'sentry-dart',
'sentry-android-gradle-plugin',
'sentry-dotnet',
'sentry-dart-plugin',
]);

export const SENTRY_REPOS: Set<string> = new Set([
...SENTRY_REPOS_WITH_ROUTING,
...SENTRY_REPOS_WITHOUT_ROUTING,
]);

/**
* As far as we can tell, it's not possible to check private org membership
* from an app installation. Therefore, we use a Personal Access Token for a
Expand Down
1 change: 1 addition & 0 deletions src/config/loadGitHubOrgs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ describe('loadGitHubOrgs', function () {
}).get('getsentry');
expect(org.appAuth.privateKey).toEqual('cheese');
expect(org.appAuth.appId).toEqual(66573);
expect(org.repos.withRouting).toEqual(['sentry', 'sentry-docs']);
});

it('chokes on non-numeric appId', async function () {
Expand Down
8 changes: 7 additions & 1 deletion src/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,16 @@ export interface GitHubIssuesSomeoneElseCaresAbout {
}

export interface GitHubOrgConfig {
num: any;
slug: any;
appAuth: any;
project: any;
repos: any;
}

export interface GitHubOrgRepos {
all: string[];
withRouting: string[];
withoutRouting: string[];
}

export type CheckRun = EmitterWebhookEvent<'check_run'>['payload']['check_run'];
Expand Down
31 changes: 18 additions & 13 deletions src/webhooks/pubsub/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import * as Sentry from '@sentry/node';
import { FastifyReply, FastifyRequest } from 'fastify';
import moment from 'moment-timezone';

import { GETSENTRY_ORG } from '@/config';
import { GH_ORGS } from '@/config';

import { notifyProductOwnersForUntriagedIssues } from './slackNotifications';
import { triggerStaleBot } from './stalebot';
Expand Down Expand Up @@ -43,28 +43,33 @@ export const pubSubHandler = async (
Buffer.from(request.body.message.data, 'base64').toString().trim()
);

let org,
now,
code = 204;
let func = new Map([
let code, func;
const operation = new Map([
['stale-triage-notifier', notifyProductOwnersForUntriagedIssues],
['stale-bot', triggerStaleBot],
]).get(payload.name);

// Performing the following check seems to suppress GitHub's dynamic method
// call security warning.
// https://codeql.github.com/codeql-query-help/javascript/js-unvalidated-dynamic-method-call/
if (typeof func === 'function') {
org = GETSENTRY_ORG;
now = moment().utc();
if (operation) {
code = 204;
func = async () => {
const now = moment().utc();
for (const org of GH_ORGS.orgs.values()) {
// Performing the following check seems to suppress GitHub's dynamic method
// call security warning (as well as a Typescript error).
// https://codeql.github.com/codeql-query-help/javascript/js-unvalidated-dynamic-method-call/
if (typeof operation === 'function') {
operation(org, now);
}
}
};
} else {
func = async () => {}; // no-op
code = 400;
func = async () => {}; // no-op
}

reply.code(code);
reply.send(); // Respond early to not block the webhook sender
await func(org, now);
await func();
tx.finish();
};

Expand Down
23 changes: 8 additions & 15 deletions src/webhooks/pubsub/slackNotifications.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@ import moment from 'moment-timezone';
import { getLabelsTable } from '@/brain/issueNotifier';
import {
BACKLOG_LABEL,
GETSENTRY_ORG,
PRODUCT_AREA_LABEL_PREFIX,
SENTRY_REPOS_WITH_ROUTING,
WAITING_FOR_PRODUCT_OWNER_LABEL,
} from '@/config';
import { Issue } from '@/types';
Expand Down Expand Up @@ -393,16 +391,13 @@ export const notifyProductOwnersForUntriagedIssues = async (
const getIssueSLOInfoForRepo = async (
repo: string
): Promise<IssueSLOInfo[]> => {
const untriagedIssues = await GETSENTRY_ORG.api.paginate(
GETSENTRY_ORG.api.issues.listForRepo,
{
owner: GETSENTRY_ORG.slug,
repo,
state: 'open',
labels: WAITING_FOR_PRODUCT_OWNER_LABEL,
per_page: GH_API_PER_PAGE,
}
);
const untriagedIssues = await org.api.paginate(org.api.issues.listForRepo, {
owner: org.slug,
repo,
state: 'open',
labels: WAITING_FOR_PRODUCT_OWNER_LABEL,
per_page: GH_API_PER_PAGE,
});

const issuesWithSLOInfo = untriagedIssues
.filter(filterIssuesOnBacklog)
Expand All @@ -424,9 +419,7 @@ export const notifyProductOwnersForUntriagedIssues = async (
};

const issuesToNotifyAbout = (
await Promise.all(
[...SENTRY_REPOS_WITH_ROUTING].map(getIssueSLOInfoForRepo)
)
await Promise.all([...org.repos.withRouting].map(getIssueSLOInfoForRepo))
).flat();

// Get an N-to-N mapping of "Product Area: *" labels to issues
Expand Down
12 changes: 4 additions & 8 deletions src/webhooks/pubsub/stalebot.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,27 +4,23 @@ import { GETSENTRY_ORG, STALE_LABEL } from '@/config';

import { triggerStaleBot } from './stalebot';

jest.mock('@/config', () => {
const actualEnvVariables = jest.requireActual('@/config');
return {
...actualEnvVariables,
SENTRY_REPOS: ['test-sentry-repo'],
};
});

describe('Stalebot Tests', function () {
const org = GETSENTRY_ORG;
let origRepos;

const issueInfo = {
labels: [],
updated_at: '2023-04-05T15:51:22Z',
};

beforeEach(async function () {
origRepos = org.repos.all;
org.repos.all = ['test-sentry-repo'];
org.api.paginate = (a, b) => a(b);
});

afterEach(async function () {
org.repos.all = origRepos;
org.api.issues._labels = new Set([]);
org.api.issues.addLabels.mockClear();
org.api.issues.removeLabel.mockClear();
Expand Down
Loading

0 comments on commit 4c49ecd

Please sign in to comment.