diff --git a/modules/runners/lambdas/runners/package.json b/modules/runners/lambdas/runners/package.json index f5d47f7483..2bf7ce5e04 100644 --- a/modules/runners/lambdas/runners/package.json +++ b/modules/runners/lambdas/runners/package.json @@ -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", diff --git a/modules/runners/lambdas/runners/src/scale-runners/gh-auth.test.ts b/modules/runners/lambdas/runners/src/scale-runners/gh-auth.test.ts index 6d846a72bd..86ab1b6c08 100644 --- a/modules/runners/lambdas/runners/src/scale-runners/gh-auth.test.ts +++ b/modules/runners/lambdas/runners/src/scale-runners/gh-auth.test.ts @@ -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'; @@ -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'; @@ -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; @@ -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); @@ -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); @@ -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); diff --git a/modules/runners/lambdas/runners/src/scale-runners/gh-auth.ts b/modules/runners/lambdas/runners/src/scale-runners/gh-auth.ts index 4d74c669c9..cda6e5d3ab 100644 --- a/modules/runners/lambdas/runners/src/scale-runners/gh-auth.ts +++ b/modules/runners/lambdas/runners/src/scale-runners/gh-auth.ts @@ -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'; @@ -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 { + const auth = await createAuth(installationId, ghesApiUrl); + const appAuthOptions: AppAuthOptions = { type: 'app' }; + return await auth(appAuthOptions); +} + +export async function createGithubInstallationAuth( + installationId: number | undefined, + ghesApiUrl = '', +): Promise { + 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 { + 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', @@ -38,6 +60,5 @@ export async function createGithubAuth( baseUrl: ghesApiUrl, }); } - const result = (await createAppAuth(authOptions)({ type: authType })) as AppAuthentication; - return result; + return createAppAuth(authOptions); } diff --git a/modules/runners/lambdas/runners/src/scale-runners/scale-down.test.ts b/modules/runners/lambdas/runners/src/scale-runners/scale-down.test.ts index cd1ac53807..2dcc865da2 100644 --- a/modules/runners/lambdas/runners/src/scale-runners/scale-down.test.ts +++ b/modules/runners/lambdas/runners/src/scale-runners/scale-down.test.ts @@ -27,7 +27,8 @@ jest.mock('./runners'); jest.mock('./gh-auth'); const mocktokit = Octokit as jest.MockedClass; -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 { @@ -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 () => []); diff --git a/modules/runners/lambdas/runners/src/scale-runners/scale-down.ts b/modules/runners/lambdas/runners/src/scale-runners/scale-down.ts index 41b0226b1d..5412c98c7b 100644 --- a/modules/runners/lambdas/runners/src/scale-runners/scale-down.ts +++ b/modules/runners/lambdas/runners/src/scale-runners/scale-down.ts @@ -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; @@ -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 ? ( @@ -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); diff --git a/modules/runners/lambdas/runners/src/scale-runners/scale-up.test.ts b/modules/runners/lambdas/runners/src/scale-runners/scale-up.test.ts index b44e887b90..9c3049b3bd 100644 --- a/modules/runners/lambdas/runners/src/scale-runners/scale-up.test.ts +++ b/modules/runners/lambdas/runners/src/scale-runners/scale-up.test.ts @@ -26,7 +26,8 @@ jest.mock('./runners'); jest.mock('./gh-auth'); const mocktokit = Octokit as jest.MockedClass; -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 = { @@ -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()); @@ -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', ); }); @@ -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', ); }); @@ -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 () => { @@ -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 () => { diff --git a/modules/runners/lambdas/runners/src/scale-runners/scale-up.ts b/modules/runners/lambdas/runners/src/scale-runners/scale-up.ts index dcec191ae6..1de5e6f390 100644 --- a/modules/runners/lambdas/runners/src/scale-runners/scale-up.ts +++ b/modules/runners/lambdas/runners/src/scale-runners/scale-up.ts @@ -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'; @@ -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 ? ( @@ -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}`; diff --git a/modules/runners/lambdas/runners/src/scale-runners/ssm.test.ts b/modules/runners/lambdas/runners/src/scale-runners/ssm.test.ts index 1f91f1787f..6d54d43e91 100644 --- a/modules/runners/lambdas/runners/src/scale-runners/ssm.test.ts +++ b/modules/runners/lambdas/runners/src/scale-runners/ssm.test.ts @@ -13,7 +13,7 @@ beforeEach(() => { nock.disableNetConnect(); }); -describe('Test createGithubAuth', () => { +describe('Test getParameterValue', () => { test('Gets parameters and returns string', async () => { // Arrange const parameterValue = 'test'; diff --git a/modules/runners/lambdas/runners/yarn.lock b/modules/runners/lambdas/runners/yarn.lock index 83529d73ba..4515805514 100644 --- a/modules/runners/lambdas/runners/yarn.lock +++ b/modules/runners/lambdas/runners/yarn.lock @@ -1141,15 +1141,15 @@ "@nodelib/fs.scandir" "2.1.5" fastq "^1.6.0" -"@octokit/auth-app@3.4.0": - version "3.4.0" - resolved "https://registry.yarnpkg.com/@octokit/auth-app/-/auth-app-3.4.0.tgz#af9f68512e7b8dd071b49e1470a1ddf88ff6a3a3" - integrity sha512-zBVgTnLJb0uoNMGCpcDkkAbPeavHX7oAjJkaDv2nqMmsXSsCw4AbUhjl99EtJQG/JqFY/kLFHM9330Wn0k70+g== +"@octokit/auth-app@3.6.0": + version "3.6.0" + resolved "https://registry.yarnpkg.com/@octokit/auth-app/-/auth-app-3.6.0.tgz#79fc6f652d2362ce0bd3122f6f764a87cc813dea" + integrity sha512-A+tLuHEMXw+Xz9dmKO7Ho9i4EmMr4tThrwYTlmMNu8y93JxvvRjKFFElpCTS+Z0NlbfuyNdaTlJnAinFbVKm7g== dependencies: - "@octokit/auth-oauth-app" "^4.1.0" + "@octokit/auth-oauth-app" "^4.3.0" "@octokit/auth-oauth-user" "^1.2.3" - "@octokit/request" "^5.4.11" - "@octokit/request-error" "^2.0.0" + "@octokit/request" "^5.6.0" + "@octokit/request-error" "^2.1.0" "@octokit/types" "^6.0.3" "@types/lru-cache" "^5.1.0" deprecation "^2.3.1" @@ -1157,7 +1157,7 @@ universal-github-app-jwt "^1.0.1" universal-user-agent "^6.0.0" -"@octokit/auth-oauth-app@^4.1.0": +"@octokit/auth-oauth-app@^4.3.0": version "4.3.0" resolved "https://registry.yarnpkg.com/@octokit/auth-oauth-app/-/auth-oauth-app-4.3.0.tgz#de02f184360ffd7cfccef861053784fc4410e7ea" integrity sha512-cETmhmOQRHCz6cLP7StThlJROff3A/ln67Q961GuIr9zvyFXZ4lIJy9RE6Uw5O7D8IXWPU3jhDnG47FTSGQr8Q== @@ -1271,7 +1271,7 @@ "@octokit/types" "^6.24.0" deprecation "^2.3.1" -"@octokit/request-error@^2.0.0", "@octokit/request-error@^2.0.5", "@octokit/request-error@^2.1.0": +"@octokit/request-error@^2.0.5", "@octokit/request-error@^2.1.0": version "2.1.0" resolved "https://registry.yarnpkg.com/@octokit/request-error/-/request-error-2.1.0.tgz#9e150357831bfc788d13a4fd4b1913d60c74d677" integrity sha512-1VIvgXxs9WHSjicsRwq8PlR2LR2x6DwsJAaFgzdi0JfJoGSO8mYI/cHJQ+9FbN21aa+DrgNLnwObmyeSC8Rmpg== @@ -1280,7 +1280,7 @@ deprecation "^2.0.0" once "^1.4.0" -"@octokit/request@^5.3.0", "@octokit/request@^5.4.11", "@octokit/request@^5.4.14", "@octokit/request@^5.6.0": +"@octokit/request@^5.3.0", "@octokit/request@^5.4.14", "@octokit/request@^5.6.0": version "5.6.0" resolved "https://registry.yarnpkg.com/@octokit/request/-/request-5.6.0.tgz#6084861b6e4fa21dc40c8e2a739ec5eff597e672" integrity sha512-4cPp/N+NqmaGQwbh3vUsYqokQIzt7VjsgTYVXiwpUP2pxd5YiZB2XuTedbb0SPtv9XS7nzAKjAuQxmY8/aZkiA== diff --git a/modules/webhook/lambdas/webhook/src/ssm/index.test.ts b/modules/webhook/lambdas/webhook/src/ssm/index.test.ts index df249fac0e..40e68b6e84 100644 --- a/modules/webhook/lambdas/webhook/src/ssm/index.test.ts +++ b/modules/webhook/lambdas/webhook/src/ssm/index.test.ts @@ -14,7 +14,7 @@ beforeEach(() => { nock.disableNetConnect(); }); -describe('Test createGithubAuth', () => { +describe('Test getParameterValue', () => { test('Gets parameters and returns string', async () => { // Arrange const parameterValue = 'test';