Skip to content

chore: Bump oktokit/auth-app (#904) #1081

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion modules/runners/lambdas/runners/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
},
"dependencies": {
"@aws-sdk/client-ssm": "^3.25.0",
"@octokit/auth-app": "3.4.0",
"@octokit/auth-app": "3.6.0",
"@octokit/rest": "^18.3.5",
"@octokit/types": "^6.25.0",
"@types/aws-lambda": "^8.10.82",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { createOctoClient, createGithubAuth } from './gh-auth';
import { createOctoClient, createGithubAppAuth, createGithubInstallationAuth } from './gh-auth';
import nock from 'nock';
import { createAppAuth } from '@octokit/auth-app';

Expand Down Expand Up @@ -36,7 +36,7 @@ beforeEach(() => {
nock.disableNetConnect();
});

describe('Test createGithubAuth', () => {
describe('Test createOctoClient', () => {
test('Creates app client to GitHub public', async () => {
// Arrange
const token = '123456';
Expand All @@ -62,7 +62,7 @@ describe('Test createGithubAuth', () => {
});
});

describe('Test createGithubAuth', () => {
describe('Test createGithubAppAuth', () => {
const mockedCreatAppAuth = createAppAuth as unknown as jest.Mock;
const mockedDefaults = jest.spyOn(request, 'defaults');
let mockedRequestInterface: MockProxy<RequestInterface>;
Expand Down Expand Up @@ -99,7 +99,7 @@ describe('Test createGithubAuth', () => {
});

// Act
const result = await createGithubAuth(installationId, authType);
const result = await createGithubAppAuth(installationId);

// Assert
expect(getParameterValue).toBeCalledWith(PARAMETER_GITHUB_APP_ID_NAME);
Expand Down Expand Up @@ -144,7 +144,7 @@ describe('Test createGithubAuth', () => {
});

// Act
const result = await createGithubAuth(installationId, authType, githubServerUrl);
const result = await createGithubAppAuth(installationId, githubServerUrl);

// Assert
expect(getParameterValue).toBeCalledWith(PARAMETER_GITHUB_APP_ID_NAME);
Expand Down Expand Up @@ -189,7 +189,7 @@ describe('Test createGithubAuth', () => {
});

// Act
const result = await createGithubAuth(installationId, authType, githubServerUrl);
const result = await createGithubAppAuth(installationId, githubServerUrl);

// Assert
expect(getParameterValue).toBeCalledWith(PARAMETER_GITHUB_APP_ID_NAME);
Expand Down
33 changes: 27 additions & 6 deletions modules/runners/lambdas/runners/src/scale-runners/gh-auth.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,14 @@
import { Octokit } from '@octokit/rest';
import { request } from '@octokit/request';
import { createAppAuth } from '@octokit/auth-app';
import { StrategyOptions, AppAuthentication } from '@octokit/auth-app/dist-types/types';
import {
StrategyOptions,
AppAuthentication,
AppAuthOptions,
InstallationAuthOptions,
InstallationAccessTokenAuthentication,
AuthInterface,
} from '@octokit/auth-app/dist-types/types';
import { OctokitOptions } from '@octokit/core/dist-types/types';
import { getParameterValue } from './ssm';

Expand All @@ -16,13 +23,28 @@ export async function createOctoClient(token: string, ghesApiUrl = ''): Promise<
return new Octokit(ocktokitOptions);
}

export async function createGithubAuth(
export async function createGithubAppAuth(
installationId: number | undefined,
authType: 'app' | 'installation',
ghesApiUrl = '',
): Promise<AppAuthentication> {
const auth = await createAuth(installationId, ghesApiUrl);
const appAuthOptions: AppAuthOptions = { type: 'app' };
return await auth(appAuthOptions);
}

export async function createGithubInstallationAuth(
installationId: number | undefined,
ghesApiUrl = '',
): Promise<InstallationAccessTokenAuthentication> {
const auth = await createAuth(installationId, ghesApiUrl);
const installationAuthOptions: InstallationAuthOptions = { type: 'installation', installationId };
return await auth(installationAuthOptions);
}

async function createAuth(installationId: number | undefined, ghesApiUrl: string): Promise<AuthInterface> {
const appId = parseInt(await getParameterValue(process.env.PARAMETER_GITHUB_APP_ID_NAME));
let authOptions: StrategyOptions = {
appId: parseInt(await getParameterValue(process.env.PARAMETER_GITHUB_APP_ID_NAME)),
appId,
privateKey: Buffer.from(
await getParameterValue(process.env.PARAMETER_GITHUB_APP_KEY_BASE64_NAME),
'base64',
Expand All @@ -38,6 +60,5 @@ export async function createGithubAuth(
baseUrl: ghesApiUrl,
});
}
const result = (await createAppAuth(authOptions)({ type: authType })) as AppAuthentication;
return result;
return createAppAuth(authOptions);
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ jest.mock('./runners');
jest.mock('./gh-auth');

const mocktokit = Octokit as jest.MockedClass<typeof Octokit>;
const mockedAuth = mocked(ghAuth.createGithubAuth, true);
const mockedAppAuth = mocked(ghAuth.createGithubAppAuth, true);
const mockedInstallationAuth = mocked(ghAuth.createGithubInstallationAuth, true);
const mockCreateClient = mocked(ghAuth.createOctoClient, true);

export interface TestData {
Expand Down Expand Up @@ -153,12 +154,21 @@ describe('scaleDown', () => {

describe('no runners running', () => {
beforeAll(() => {
mockedAuth.mockResolvedValue({
mockedAppAuth.mockResolvedValue({
type: 'app',
token: 'token',
appId: 1,
expiresAt: 'some-date',
});
mockedInstallationAuth.mockResolvedValue({
type: 'token',
tokenType: 'installation',
token: 'token',
createdAt: 'some-date',
expiresAt: 'some-date',
permissions: {},
repositorySelection: 'all',
});
mockCreateClient.mockResolvedValue(new mocktokit());
const mockListRunners = mocked(listRunners);
mockListRunners.mockImplementation(async () => []);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import moment from 'moment';
import yn from 'yn';
import { listRunners, RunnerInfo, terminateRunner } from './runners';
import { getIdleRunnerCount, ScalingDownConfig } from './scale-down-config';
import { createOctoClient, createGithubAuth } from './gh-auth';
import { createOctoClient, createGithubAppAuth, createGithubInstallationAuth } from './gh-auth';

interface Repo {
repoName: string;
Expand Down Expand Up @@ -35,7 +35,7 @@ function createGitHubClientForRunnerFactory(): (runner: RunnerInfo, orgLevel: bo
if (ghesBaseUrl) {
ghesApiUrl = `${ghesBaseUrl}/api/v3`;
}
const ghAuth = await createGithubAuth(undefined, 'app', ghesApiUrl);
const ghAuth = await createGithubAppAuth(undefined, ghesApiUrl);
const githubClient = await createOctoClient(ghAuth.token, ghesApiUrl);
const installationId = orgLevel
? (
Expand All @@ -49,7 +49,7 @@ function createGitHubClientForRunnerFactory(): (runner: RunnerInfo, orgLevel: bo
repo: repo.repoName,
})
).data.id;
const ghAuth2 = await createGithubAuth(installationId, 'installation', ghesApiUrl);
const ghAuth2 = await createGithubInstallationAuth(installationId, ghesApiUrl);
const octokit = await createOctoClient(ghAuth2.token, ghesApiUrl);
cache.set(key, octokit);

Expand Down
82 changes: 46 additions & 36 deletions modules/runners/lambdas/runners/src/scale-runners/scale-up.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ jest.mock('./runners');
jest.mock('./gh-auth');

const mocktokit = Octokit as jest.MockedClass<typeof Octokit>;
const mockedAuth = mocked(ghAuth.createGithubAuth, true);
const mockedAppAuth = mocked(ghAuth.createGithubAppAuth, true);
const mockedInstallationAuth = mocked(ghAuth.createGithubInstallationAuth, true);
const mockCreateClient = mocked(ghAuth.createOctoClient, true);

const TEST_DATA: scaleUpModule.ActionRequestMessage = {
Expand Down Expand Up @@ -115,12 +116,21 @@ beforeEach(() => {

describe('scaleUp with GHES', () => {
beforeEach(() => {
mockedAuth.mockResolvedValue({
mockedAppAuth.mockResolvedValue({
type: 'app',
token: 'token',
appId: TEST_DATA.installationId,
expiresAt: 'some-date',
});
mockedInstallationAuth.mockResolvedValue({
type: 'token',
tokenType: 'installation',
token: 'token',
createdAt: 'some-date',
expiresAt: 'some-date',
permissions: {},
repositorySelection: 'all',
});

mockCreateClient.mockResolvedValue(new mocktokit());

Expand Down Expand Up @@ -180,26 +190,23 @@ describe('scaleUp with GHES', () => {
});

it('does not retrieve installation id if already set', async () => {
const spy = jest.spyOn(ghAuth, 'createGithubAuth');
const appSpy = jest.spyOn(ghAuth, 'createGithubAppAuth');
const installationSpy = jest.spyOn(ghAuth, 'createGithubInstallationAuth');
await scaleUpModule.scaleUp('aws:sqs', TEST_DATA);
expect(mockOctokit.apps.getOrgInstallation).not.toBeCalled();
expect(mockOctokit.apps.getRepoInstallation).not.toBeCalled();
expect(spy).toBeCalledWith(
TEST_DATA.installationId,
'installation',
'https://github.enterprise.something/api/v3',
);
expect(appSpy).not.toBeCalled();
expect(installationSpy).toBeCalledWith(TEST_DATA.installationId, 'https://github.enterprise.something/api/v3');
});

it('retrieves installation id if not set', async () => {
const spy = jest.spyOn(ghAuth, 'createGithubAuth');
const appSpy = jest.spyOn(ghAuth, 'createGithubAppAuth');
const installationSpy = jest.spyOn(ghAuth, 'createGithubInstallationAuth');
await scaleUpModule.scaleUp('aws:sqs', TEST_DATA_WITH_ZERO_INSTALL_ID);
expect(mockOctokit.apps.getRepoInstallation).not.toBeCalled();
expect(spy).toHaveBeenNthCalledWith(1, undefined, 'app', 'https://github.enterprise.something/api/v3');
expect(spy).toHaveBeenNthCalledWith(
2,
expect(appSpy).toHaveBeenCalledWith(undefined, 'https://github.enterprise.something/api/v3');
expect(installationSpy).toHaveBeenCalledWith(
TEST_DATA.installationId,
'installation',
'https://github.enterprise.something/api/v3',
);
});
Expand Down Expand Up @@ -271,7 +278,7 @@ describe('scaleUp with GHES', () => {
expect(mockOctokit.actions.createRegistrationTokenForOrg).not.toBeCalled();
expect(mockOctokit.actions.createRegistrationTokenForRepo).not.toBeCalled();
});

it('creates a token when maximum runners has not been reached', async () => {
await scaleUpModule.scaleUp('aws:sqs', TEST_DATA);
expect(mockOctokit.actions.createRegistrationTokenForOrg).not.toBeCalled();
Expand All @@ -291,30 +298,27 @@ describe('scaleUp with GHES', () => {
});

it('does not retrieve installation id if already set', async () => {
const spy = jest.spyOn(ghAuth, 'createGithubAuth');
const appSpy = jest.spyOn(ghAuth, 'createGithubAppAuth');
const installationSpy = jest.spyOn(ghAuth, 'createGithubInstallationAuth');
await scaleUpModule.scaleUp('aws:sqs', TEST_DATA);
expect(mockOctokit.apps.getOrgInstallation).not.toBeCalled();
expect(mockOctokit.apps.getRepoInstallation).not.toBeCalled();
expect(spy).toBeCalledWith(
TEST_DATA.installationId,
'installation',
'https://github.enterprise.something/api/v3',
);
expect(appSpy).not.toBeCalled();
expect(installationSpy).toBeCalledWith(TEST_DATA.installationId, 'https://github.enterprise.something/api/v3');
});

it('retrieves installation id if not set', async () => {
const spy = jest.spyOn(ghAuth, 'createGithubAuth');
const appSpy = jest.spyOn(ghAuth, 'createGithubAppAuth');
const installationSpy = jest.spyOn(ghAuth, 'createGithubInstallationAuth');
await scaleUpModule.scaleUp('aws:sqs', TEST_DATA_WITH_ZERO_INSTALL_ID);
expect(mockOctokit.apps.getOrgInstallation).not.toBeCalled();
expect(mockOctokit.apps.getRepoInstallation).toBeCalledWith({
owner: TEST_DATA.repositoryOwner,
repo: TEST_DATA.repositoryName,
});
expect(spy).toHaveBeenNthCalledWith(1, undefined, 'app', 'https://github.enterprise.something/api/v3');
expect(spy).toHaveBeenNthCalledWith(
2,
expect(appSpy).toHaveBeenCalledWith(undefined, 'https://github.enterprise.something/api/v3');
expect(installationSpy).toHaveBeenCalledWith(
TEST_DATA.installationId,
'installation',
'https://github.enterprise.something/api/v3',
);
});
Expand Down Expand Up @@ -371,20 +375,23 @@ describe('scaleUp with public GH', () => {
});

it('does not retrieve installation id if already set', async () => {
const spy = jest.spyOn(ghAuth, 'createGithubAuth');
const appSpy = jest.spyOn(ghAuth, 'createGithubAppAuth');
const installationSpy = jest.spyOn(ghAuth, 'createGithubInstallationAuth');
await scaleUpModule.scaleUp('aws:sqs', TEST_DATA);
expect(mockOctokit.apps.getOrgInstallation).not.toBeCalled();
expect(mockOctokit.apps.getRepoInstallation).not.toBeCalled();
expect(spy).toBeCalledWith(TEST_DATA.installationId, 'installation', '');
expect(appSpy).not.toBeCalled();
expect(installationSpy).toBeCalledWith(TEST_DATA.installationId, '');
});

it('retrieves installation id if not set', async () => {
const spy = jest.spyOn(ghAuth, 'createGithubAuth');
const appSpy = jest.spyOn(ghAuth, 'createGithubAppAuth');
const installationSpy = jest.spyOn(ghAuth, 'createGithubInstallationAuth');
await scaleUpModule.scaleUp('aws:sqs', TEST_DATA_WITH_ZERO_INSTALL_ID);
expect(mockOctokit.apps.getOrgInstallation).toBeCalled();
expect(mockOctokit.apps.getRepoInstallation).not.toBeCalled();
expect(spy).toHaveBeenNthCalledWith(1, undefined, 'app', '');
expect(spy).toHaveBeenNthCalledWith(2, TEST_DATA.installationId, 'installation', '');
expect(appSpy).toHaveBeenCalledWith(undefined, '');
expect(installationSpy).toHaveBeenCalledWith(TEST_DATA.installationId, '');
});

it('does not list runners when no workflows are queued', async () => {
Expand Down Expand Up @@ -481,7 +488,7 @@ describe('scaleUp with public GH', () => {
expect(mockOctokit.actions.createRegistrationTokenForOrg).not.toBeCalled();
expect(mockOctokit.actions.createRegistrationTokenForRepo).not.toBeCalled();
});

it('creates a token when maximum runners has not been reached', async () => {
await scaleUpModule.scaleUp('aws:sqs', TEST_DATA);
expect(mockOctokit.actions.createRegistrationTokenForOrg).not.toBeCalled();
Expand All @@ -492,20 +499,23 @@ describe('scaleUp with public GH', () => {
});

it('does not retrieve installation id if already set', async () => {
const spy = jest.spyOn(ghAuth, 'createGithubAuth');
const appSpy = jest.spyOn(ghAuth, 'createGithubAppAuth');
const installationSpy = jest.spyOn(ghAuth, 'createGithubInstallationAuth');
await scaleUpModule.scaleUp('aws:sqs', TEST_DATA);
expect(mockOctokit.apps.getOrgInstallation).not.toBeCalled();
expect(mockOctokit.apps.getRepoInstallation).not.toBeCalled();
expect(spy).toBeCalledWith(TEST_DATA.installationId, 'installation', '');
expect(appSpy).not.toBeCalled();
expect(installationSpy).toBeCalledWith(TEST_DATA.installationId, '');
});

it('retrieves installation id if not set', async () => {
const spy = jest.spyOn(ghAuth, 'createGithubAuth');
const appSpy = jest.spyOn(ghAuth, 'createGithubAppAuth');
const installationSpy = jest.spyOn(ghAuth, 'createGithubInstallationAuth');
await scaleUpModule.scaleUp('aws:sqs', TEST_DATA_WITH_ZERO_INSTALL_ID);
expect(mockOctokit.apps.getOrgInstallation).not.toBeCalled();
expect(mockOctokit.apps.getRepoInstallation).toBeCalled();
expect(spy).toHaveBeenNthCalledWith(1, undefined, 'app', '');
expect(spy).toHaveBeenNthCalledWith(2, TEST_DATA.installationId, 'installation', '');
expect(appSpy).toHaveBeenCalledWith(undefined, '');
expect(installationSpy).toHaveBeenCalledWith(TEST_DATA.installationId, '');
});

it('creates a runner with correct config and labels', async () => {
Expand Down
8 changes: 3 additions & 5 deletions modules/runners/lambdas/runners/src/scale-runners/scale-up.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { listRunners, createRunner, RunnerInputParameters } from './runners';
import { createOctoClient, createGithubAuth } from './gh-auth';
import { createOctoClient, createGithubAppAuth, createGithubInstallationAuth } from './gh-auth';
import yn from 'yn';
import { Octokit } from '@octokit/rest';

Expand Down Expand Up @@ -27,7 +27,7 @@ export const scaleUp = async (eventSource: string, payload: ActionRequestMessage

let installationId = payload.installationId;
if (installationId == 0) {
const ghAuth = await createGithubAuth(undefined, 'app', ghesApiUrl);
const ghAuth = await createGithubAppAuth(undefined, ghesApiUrl);
const githubClient = await createOctoClient(ghAuth.token, ghesApiUrl);
installationId = enableOrgLevel
? (
Expand All @@ -43,10 +43,8 @@ export const scaleUp = async (eventSource: string, payload: ActionRequestMessage
).data.id;
}

const ghAuth = await createGithubAuth(installationId, 'installation', ghesApiUrl);

const ghAuth = await createGithubInstallationAuth(installationId, ghesApiUrl);
const githubInstallationClient = await createOctoClient(ghAuth.token, ghesApiUrl);

const runnerType = enableOrgLevel ? 'Org' : 'Repo';
const runnerOwner = enableOrgLevel ? payload.repositoryOwner : `${payload.repositoryOwner}/${payload.repositoryName}`;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ beforeEach(() => {
nock.disableNetConnect();
});

describe('Test createGithubAuth', () => {
describe('Test getParameterValue', () => {
test('Gets parameters and returns string', async () => {
// Arrange
const parameterValue = 'test';
Expand Down
Loading