Skip to content

Commit 0047ae8

Browse files
committed
fix: Fix dual GitHub App installation auth fallback for regional runners
Handle 404 installation token failures by resolving the installation from the active regional app and retrying auth. Add tests and avoid eager app-client creation in shared octokit auth.
1 parent 0a0c0e8 commit 0047ae8

4 files changed

Lines changed: 179 additions & 22 deletions

File tree

lambdas/functions/control-plane/src/github/octokit.test.ts

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { Octokit } from '@octokit/rest';
22
import { ActionRequestMessage } from '../scale-runners/scale-up';
33
import { getOctokit } from './octokit';
44
import { describe, it, expect, beforeEach, vi } from 'vitest';
5+
import { createGithubAppAuth, createGithubInstallationAuth } from '../github/auth';
56

67
const mockOctokit = {
78
apps: {
@@ -11,7 +12,7 @@ const mockOctokit = {
1112
};
1213

1314
vi.mock('../github/auth', async () => ({
14-
createGithubInstallationAuth: vi.fn().mockImplementation(async (installationId) => {
15+
createGithubInstallationAuth: vi.fn().mockImplementation(async (installationId: number) => {
1516
return { token: 'token', type: 'installation', installationId: installationId };
1617
}),
1718
createOctokitClient: vi.fn().mockImplementation(() => new Octokit()),
@@ -27,7 +28,11 @@ vi.mock('@octokit/rest', async () => ({
2728
// We've already mocked '../github/auth' above
2829

2930
describe('Test getOctokit', () => {
30-
const data = [
31+
const data: Array<{
32+
description: string;
33+
input: { orgLevelRunner: boolean; installationId: number };
34+
output: { callReposInstallation: boolean; callOrgInstallation: boolean };
35+
}> = [
3136
{
3237
description: 'Should look-up org installation if installationId is 0.',
3338
input: { orgLevelRunner: false, installationId: 0 },
@@ -49,7 +54,7 @@ describe('Test getOctokit', () => {
4954
vi.clearAllMocks();
5055
});
5156

52-
it.each(data)(`$description`, async ({ input, output }) => {
57+
it.each(data)(`$description`, async ({ input, output }: (typeof data)[number]) => {
5358
const payload = {
5459
eventType: 'workflow_job',
5560
id: 0,
@@ -74,6 +79,31 @@ describe('Test getOctokit', () => {
7479
} else if (output.callReposInstallation) {
7580
expect(mockOctokit.apps.getRepoInstallation).toHaveBeenCalled();
7681
expect(mockOctokit.apps.getOrgInstallation).not.toHaveBeenCalled();
82+
} else {
83+
expect(createGithubAppAuth).not.toHaveBeenCalled();
7784
}
7885
});
86+
87+
it('Should resolve installation again when event installation belongs to another app', async () => {
88+
const payload = {
89+
eventType: 'workflow_job',
90+
id: 0,
91+
installationId: 999,
92+
repositoryOwner: 'owner',
93+
repositoryName: 'repo',
94+
} as ActionRequestMessage;
95+
96+
mockOctokit.apps.getOrgInstallation.mockResolvedValue({ data: { id: 123 } });
97+
98+
vi.mocked(createGithubInstallationAuth)
99+
.mockRejectedValueOnce({ status: 404 })
100+
.mockResolvedValueOnce({ token: 'token', type: 'installation', installationId: 123 });
101+
102+
await expect(getOctokit('', true, payload)).resolves.toBeDefined();
103+
104+
expect(createGithubAppAuth).toHaveBeenCalledTimes(1);
105+
expect(mockOctokit.apps.getOrgInstallation).toHaveBeenCalledWith({ org: 'owner' });
106+
expect(createGithubInstallationAuth).toHaveBeenNthCalledWith(1, 999, '');
107+
expect(createGithubInstallationAuth).toHaveBeenNthCalledWith(2, 123, '');
108+
});
79109
});

lambdas/functions/control-plane/src/github/octokit.ts

Lines changed: 57 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,20 @@ import { Octokit } from '@octokit/rest';
22
import { ActionRequestMessage } from '../scale-runners/scale-up';
33
import { createGithubAppAuth, createGithubInstallationAuth, createOctokitClient } from './auth';
44

5-
export async function getInstallationId(
6-
ghesApiUrl: string,
5+
function getErrorStatus(error: unknown): number | undefined {
6+
if (typeof error !== 'object' || error === null) {
7+
return undefined;
8+
}
9+
10+
const errorWithStatus = error as { status?: number; response?: { status?: number } };
11+
return errorWithStatus.status ?? errorWithStatus.response?.status;
12+
}
13+
14+
async function resolveInstallationId(
15+
githubClient: Octokit,
716
enableOrgLevel: boolean,
817
payload: ActionRequestMessage,
918
): Promise<number> {
10-
if (payload.installationId !== 0) {
11-
return payload.installationId;
12-
}
13-
14-
const ghAuth = await createGithubAppAuth(undefined, ghesApiUrl);
15-
const githubClient = await createOctokitClient(ghAuth.token, ghesApiUrl);
1619
return enableOrgLevel
1720
? (
1821
await githubClient.apps.getOrgInstallation({
@@ -27,6 +30,20 @@ export async function getInstallationId(
2730
).data.id;
2831
}
2932

33+
export async function getInstallationId(
34+
ghesApiUrl: string,
35+
enableOrgLevel: boolean,
36+
payload: ActionRequestMessage,
37+
): Promise<number> {
38+
if (payload.installationId !== 0) {
39+
return payload.installationId;
40+
}
41+
42+
const ghAuth = await createGithubAppAuth(undefined, ghesApiUrl);
43+
const githubClient = await createOctokitClient(ghAuth.token, ghesApiUrl);
44+
return resolveInstallationId(githubClient, enableOrgLevel, payload);
45+
}
46+
3047
/**
3148
*
3249
* Util method to get an octokit client based on provided installation id. This method should
@@ -40,7 +57,36 @@ export async function getOctokit(
4057
enableOrgLevel: boolean,
4158
payload: ActionRequestMessage,
4259
): Promise<Octokit> {
43-
const installationId = await getInstallationId(ghesApiUrl, enableOrgLevel, payload);
44-
const ghAuth = await createGithubInstallationAuth(installationId, ghesApiUrl);
45-
return await createOctokitClient(ghAuth.token, ghesApiUrl);
60+
let githubAppClient: Octokit | undefined;
61+
let installationId = payload.installationId !== 0 ? payload.installationId : undefined;
62+
63+
const getGithubAppClient = async (): Promise<Octokit> => {
64+
if (githubAppClient === undefined) {
65+
const appAuth = await createGithubAppAuth(undefined, ghesApiUrl);
66+
githubAppClient = await createOctokitClient(appAuth.token, ghesApiUrl);
67+
}
68+
69+
return githubAppClient;
70+
};
71+
72+
try {
73+
if (installationId === undefined) {
74+
installationId = await resolveInstallationId(await getGithubAppClient(), enableOrgLevel, payload);
75+
}
76+
77+
const ghAuth = await createGithubInstallationAuth(installationId, ghesApiUrl);
78+
return await createOctokitClient(ghAuth.token, ghesApiUrl);
79+
} catch (error) {
80+
if (payload.installationId === 0 || getErrorStatus(error) !== 404) {
81+
throw error;
82+
}
83+
84+
const resolvedInstallationId = await resolveInstallationId(await getGithubAppClient(), enableOrgLevel, payload);
85+
if (resolvedInstallationId === payload.installationId) {
86+
throw error;
87+
}
88+
89+
const ghAuth = await createGithubInstallationAuth(resolvedInstallationId, ghesApiUrl);
90+
return await createOctokitClient(ghAuth.token, ghesApiUrl);
91+
}
4692
}

lambdas/functions/control-plane/src/scale-runners/scale-up.test.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -842,6 +842,34 @@ describe('scaleUp with GHES', () => {
842842
expect(mockedInstallationAuth).toHaveBeenCalledWith(200, 'https://github.enterprise.something/api/v3');
843843
});
844844

845+
it('Should resolve installation again when event installation belongs to another app', async () => {
846+
mockOctokit.apps.getOrgInstallation.mockReset();
847+
mockOctokit.apps.getOrgInstallation.mockImplementation(() => ({
848+
data: {
849+
id: 123,
850+
},
851+
}));
852+
853+
mockedInstallationAuth
854+
.mockRejectedValueOnce({ status: 404 })
855+
.mockResolvedValueOnce({
856+
type: 'token',
857+
tokenType: 'installation',
858+
token: 'token',
859+
createdAt: 'some-date',
860+
expiresAt: 'some-date',
861+
permissions: {},
862+
repositorySelection: 'all',
863+
installationId: 123,
864+
});
865+
866+
await scaleUpModule.scaleUp(TEST_DATA);
867+
868+
expect(mockOctokit.apps.getOrgInstallation).toHaveBeenCalledWith({ org: TEST_DATA_SINGLE.repositoryOwner });
869+
expect(mockedInstallationAuth).toHaveBeenNthCalledWith(1, TEST_DATA_SINGLE.installationId, 'https://github.enterprise.something/api/v3');
870+
expect(mockedInstallationAuth).toHaveBeenNthCalledWith(2, 123, 'https://github.enterprise.something/api/v3');
871+
});
872+
845873
it('Should reuse GitHub clients for same installation', async () => {
846874
const messages = createTestMessages(3, [
847875
{ repositoryOwner: 'same-org' },

lambdas/functions/control-plane/src/scale-runners/scale-up.ts

Lines changed: 61 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -149,15 +149,20 @@ function removeTokenFromLogging(config: string[]): string[] {
149149
return result;
150150
}
151151

152-
export async function getInstallationId(
152+
function getErrorStatus(error: unknown): number | undefined {
153+
if (typeof error !== 'object' || error === null) {
154+
return undefined;
155+
}
156+
157+
const errorWithStatus = error as { status?: number; response?: { status?: number } };
158+
return errorWithStatus.status ?? errorWithStatus.response?.status;
159+
}
160+
161+
async function resolveInstallationId(
153162
githubAppClient: Octokit,
154163
enableOrgLevel: boolean,
155164
payload: ActionRequestMessage,
156165
): Promise<number> {
157-
if (payload.installationId !== 0) {
158-
return payload.installationId;
159-
}
160-
161166
return enableOrgLevel
162167
? (
163168
await githubAppClient.apps.getOrgInstallation({
@@ -172,6 +177,18 @@ export async function getInstallationId(
172177
).data.id;
173178
}
174179

180+
export async function getInstallationId(
181+
githubAppClient: Octokit,
182+
enableOrgLevel: boolean,
183+
payload: ActionRequestMessage,
184+
): Promise<number> {
185+
if (payload.installationId !== 0) {
186+
return payload.installationId;
187+
}
188+
189+
return resolveInstallationId(githubAppClient, enableOrgLevel, payload);
190+
}
191+
175192
export async function isJobQueued(githubInstallationClient: Octokit, payload: ActionRequestMessage): Promise<boolean> {
176193
let isQueued = false;
177194
if (payload.eventType === 'workflow_job') {
@@ -288,6 +305,39 @@ export async function createRunners(
288305
return instances;
289306
}
290307

308+
async function createGithubInstallationClient(
309+
githubAppClient: Octokit,
310+
enableOrgLevel: boolean,
311+
payload: ActionRequestMessage,
312+
ghesApiUrl: string,
313+
): Promise<Octokit> {
314+
let installationId = await getInstallationId(githubAppClient, enableOrgLevel, payload);
315+
316+
try {
317+
const ghAuth = await createGithubInstallationAuth(installationId, ghesApiUrl);
318+
return await createOctokitClient(ghAuth.token, ghesApiUrl);
319+
} catch (error) {
320+
if (payload.installationId === 0 || getErrorStatus(error) !== 404) {
321+
throw error;
322+
}
323+
324+
installationId = await resolveInstallationId(githubAppClient, enableOrgLevel, payload);
325+
if (installationId === payload.installationId) {
326+
throw error;
327+
}
328+
329+
logger.warn('Retrying GitHub installation auth with installation resolved for current app', {
330+
eventInstallationId: payload.installationId,
331+
resolvedInstallationId: installationId,
332+
repositoryOwner: payload.repositoryOwner,
333+
repositoryName: payload.repositoryName,
334+
});
335+
336+
const ghAuth = await createGithubInstallationAuth(installationId, ghesApiUrl);
337+
return await createOctokitClient(ghAuth.token, ghesApiUrl);
338+
}
339+
}
340+
291341
export async function scaleUp(payloads: ActionRequestMessageSQS[]): Promise<string[]> {
292342
logger.info('Received scale up requests', {
293343
n_requests: payloads.length,
@@ -373,9 +423,12 @@ export async function scaleUp(payloads: ActionRequestMessageSQS[]): Promise<stri
373423
// If we've not seen this owner/repo before, we'll need to create a GitHub
374424
// client for it.
375425
if (entry === undefined) {
376-
const installationId = await getInstallationId(githubAppClient, enableOrgLevel, payload);
377-
const ghAuth = await createGithubInstallationAuth(installationId, ghesApiUrl);
378-
const githubInstallationClient = await createOctokitClient(ghAuth.token, ghesApiUrl);
426+
const githubInstallationClient = await createGithubInstallationClient(
427+
githubAppClient,
428+
enableOrgLevel,
429+
payload,
430+
ghesApiUrl,
431+
);
379432

380433
entry = {
381434
messages: [],

0 commit comments

Comments
 (0)