Skip to content

Commit

Permalink
chore: Bump oktokit/auth-app (philips-labs#904)
Browse files Browse the repository at this point in the history 
The newer version of the library requires different input data for yyy in the createAppAuth(xxx)(yyy) function, and also returns a different data structure for the 'app' and 'installation' scenarios.

- Set dependency to version 3.6.0
- Fix some (misleading) test descriptions
  • Loading branch information
@aadrijnberg
aadrijnberg committed Aug 12, 2021
1 parent 946b2ea commit 2306df3
Show file tree
Hide file tree
Showing 10 changed files with 110 additions and 71 deletions.
2 changes: 1 addition & 1 deletion modules/runners/lambdas/runners/package.json
View file
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
12 changes: 6 additions & 6 deletions modules/runners/lambdas/runners/src/scale-runners/gh-auth.test.ts
View file
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
View file
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);
}
14 changes: 12 additions & 2 deletions modules/runners/lambdas/runners/src/scale-runners/scale-down.test.ts
View file
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
6 changes: 3 additions & 3 deletions modules/runners/lambdas/runners/src/scale-runners/scale-down.ts
View file
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
View file
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
View file
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
2 changes: 1 addition & 1 deletion modules/runners/lambdas/runners/src/scale-runners/ssm.test.ts
View file
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

0 comments on commit 2306df3

Please sign in to comment.