Implement multi-org config

[chadwhitacre]

Part of #482, after #523.

This implements parsing of multiple apps from the environment. I may decide to replace __tmp_org_placeholder__ on a separate PR first.

• replace __tmp_org_placeholder__ throughout
• possibly expand testing
• better error handling on config errors (envvars instead of path keys, better formatting)
• address reviews
• put protection around places where we want to require GETSENTRY_ORG
• clean up names (GH_APPS ⇒ GH_ORGS, envvars, etc.) punting

[azaslavsky]

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.

[chadwhitacre]

We are mocking the replies. 🐭

[chadwhitacre]

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.

[hubertdeng123]

Looks pretty good to me, pending some small comments that are made. Going to give the approval here since I'm out tomorrow

[hubertdeng123]

Actually, I guess I can't really approve if the env vars aren't yet in ci.yml yet since it'll break everything (Sorry Chad!). One thing there is that it would be nice if these env vars don't come from GH secrets in case we want to run GH graphql queries locally. They're not sensitive and it'd be nice to have those somewhere to use directly.

ISSUES_PROJECT_NODE_ID
PRODUCT_AREA_FIELD_ID
STATUS_FIELD_ID
RESPONSE_DUE_DATE_FIELD_ID

[chadwhitacre]

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.

[codecov]

Codecov Report

Patch coverage: 96.92% and project coverage change: +0.73 🎉

Comparison is base (d8a18c6) 84.65% compared to head (16166f5) 85.39%.

❗ Current head 16166f5 differs from pull request most recent head 61d3556. Consider uploading reports for the commit 61d3556 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #524      +/-   ##
==========================================
+ Coverage   84.65%   85.39%   +0.73%     
==========================================
  Files          97       97              
  Lines        2509     2533      +24     
  Branches      476      480       +4     
==========================================
+ Hits         2124     2163      +39     
+ Misses        378      363      -15     
  Partials        7        7              

Impacted Files	Coverage Δ
src/webhooks/pubsub/index.ts	96.00% <92.30%> (+58.50%)	⬆️
src/config/loadGitHubAppsFromEnvironment.ts	98.59% <97.72%> (+3.46%)	⬆️
src/api/github/getClient.ts	89.28% <100.00%> (ø)
src/brain/issueLabelHandler/followups.ts	100.00% <100.00%> (ø)
src/brain/issueLabelHandler/route.ts	97.29% <100.00%> (-0.08%)	⬇️
src/brain/issueLabelHandler/triage.ts	97.72% <100.00%> (-0.10%)	⬇️
src/brain/projectsHandler/project.ts	97.36% <100.00%> (-0.07%)	⬇️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[chadwhitacre]

put protection around places where we want to require GETSENTRY_ORG

I'm thinking about this one. The risk is that we might process a payload from a non-getsentry org through an endpoint that is only intended for getsentry—i.e., the endpoint operates on getsentry regardless of what org we might send from. 🤔

[chadwhitacre]

Here are the places where we call getClient. At the very least I need to audit these to make sure none of the ones where we call it with GETSENTRY_ORG operate on the org in the payload, they only touch getsentry.

$ ag --ignore \*.test.ts 'getClient\(ClientType\.App'
src/utils/db/getFailureMessages.ts
43:      const octokit = await getClient(ClientType.App, GETSENTRY_ORG);

src/api/github/getRelevantCommit.ts
16:    const octokit = await getClient(ClientType.App, GETSENTRY_ORG);

src/api/github/getChangedStack.ts
16:    const octokit = await getClient(ClientType.App, GETSENTRY_ORG);

src/brain/ghaCancel/index.ts
27:  const octokit = await getClient(ClientType.App, owner);

src/brain/issueLabelHandler/triage.ts
63:  const octokit = await getClient(ClientType.App, app.org);
116:  const octokit = await getClient(ClientType.App, app.org);

src/brain/issueLabelHandler/route.ts
84:  const octokit = await getClient(ClientType.App, app.org);
139:  const octokit = await getClient(ClientType.App, app.org);

src/brain/issueLabelHandler/followups.ts
80:  const octokit = await getClient(ClientType.App, app.org);
144:  const octokit = await getClient(ClientType.App, app.org);

src/brain/typescript/getProgress.ts
21:  const octokit = await getClient(ClientType.App, GETSENTRY_ORG);

src/brain/notifyOnGoCDStageEvent/index.ts
276:  const octokit = await getClient(ClientType.App, GETSENTRY_ORG);

src/brain/projectsHandler/project.ts
67:  const octokit = await getClient(ClientType.App, app.org);

src/brain/requiredChecks/getAnnotations.ts
85:  const octokit = await getClient(ClientType.App, GETSENTRY_ORG);

src/brain/pleaseDeployNotifier/actionViewUndeployedCommits.ts
71:  const octokit = await getClient(ClientType.App, GETSENTRY_ORG);

src/brain/gocdSlackFeeds/deployFeed.ts
81:    const octokit = await getClient(ClientType.App, GETSENTRY_ORG);

src/webhooks/pubsub/index.ts
72:      const octokit = await getClient(ClientType.App, org);

[chadwhitacre]

I guess the opposite could also happen, an Octokit created with something other than getsentry is used for getsentry. Does this generally result in a permissions failure? 🤔

const octokit = await getClient(ClientType.App, 'foo');
const { data } = await octokit.blah.blah({
  owner: 'bar',
  repo: 'baz',
});

[chadwhitacre]

Okay, I've audited all of the places where we call getClient. Findings:

1. GitHub's REST APIs seem to all take an org as an argument, whereas their GraphQL API seemingly does not (I guess the node IDs and whatnot are at least globally unique).
2. Where we instantiate with GETSENTRY_ORG, we always call with GETSENTRY_ORG. 👍
3. Where we instantiate with app.org, we sometimes call with GETSENTRY_ORG (stalebot), and other times it's not clear to me yet (everything using githubEventHelpers).
4. I'm struggling against the instinct to significantly refactor this so that app and especially the GitHub event helpers are properties on some sort of GitHubOrg object.

[chadwhitacre]

Scrapping for parts.