diff --git a/.changeset/sweet-grapes-explain.md b/.changeset/sweet-grapes-explain.md new file mode 100644 index 0000000000..014fbb3d41 --- /dev/null +++ b/.changeset/sweet-grapes-explain.md @@ -0,0 +1,8 @@ +--- +'@backstage/integration': patch +--- + +Improved caching around github app tokens. +Tokens are now cached for 50 minutes, not 10. +Calls to get app installations are also included in this cache. +If you have more than one github app configured, consider adding `allowedInstallationOwners` to your apps configuration to gain the most benefit from these performance changes. diff --git a/docs/integrations/github/github-apps.md b/docs/integrations/github/github-apps.md index 4f2c58bbec..b526c306e7 100644 --- a/docs/integrations/github/github-apps.md +++ b/docs/integrations/github/github-apps.md @@ -98,7 +98,9 @@ integrations: ### Limiting the GitHub App installations If you want to limit the GitHub app installations visible to backstage you may -optionally include the `allowedInstallationOwners` option. +optionally include the `allowedInstallationOwners` option. If you configure +multiple apps, specifying this will bring some small performance benefits +as backstage can more easily select which app to use for a URL. ```yaml appId: app id diff --git a/packages/integration/src/github/SingleInstanceGithubCredentialsProvider.test.ts b/packages/integration/src/github/SingleInstanceGithubCredentialsProvider.test.ts index 1dcc45c1ef..a1544fbc68 100644 --- a/packages/integration/src/github/SingleInstanceGithubCredentialsProvider.test.ts +++ b/packages/integration/src/github/SingleInstanceGithubCredentialsProvider.test.ts @@ -57,6 +57,7 @@ describe('SingleInstanceGithubCredentialsProvider tests', () => { token: 'hardcoded_token', }); }); + it('create repository specific tokens', async () => { octokit.apps.listInstallations.mockResolvedValue({ headers: { @@ -66,11 +67,6 @@ describe('SingleInstanceGithubCredentialsProvider tests', () => { { id: 1, repository_selection: 'selected', - account: null, - }, - { - id: 2, - repository_selection: 'selected', account: { login: 'backstage', }, @@ -195,9 +191,14 @@ describe('SingleInstanceGithubCredentialsProvider tests', () => { data: { expires_at: DateTime.local().plus({ hours: 1 }).toString(), token: 'secret_token', + repository_selection: 'selected', }, } as RestEndpointMethodTypes['apps']['createInstallationAccessToken']['response']); + octokit.apps.listReposAccessibleToInstallation.mockReturnValue({ + data: [{ name: 'some-repo' }], + } as unknown as RestEndpointMethodTypes['apps']['listReposAccessibleToInstallation']['response']); + const { token, headers } = await github.getCredentials({ url: 'https://github.com/backstage', }); @@ -400,4 +401,70 @@ describe('SingleInstanceGithubCredentialsProvider tests', () => { }), ).resolves.not.toThrow(); }); + + it('should cache access token', async () => { + octokit.apps.listInstallations.mockReturnValue({ + headers: { + etag: '123', + }, + data: [ + { + id: 1, + repository_selection: 'all', + account: { + login: 'backstage', + }, + }, + ], + } as RestEndpointMethodTypes['apps']['listInstallations']['response']); + + octokit.apps.createInstallationAccessToken.mockReturnValue({ + data: { + expires_at: DateTime.local().plus({ minutes: 11 }).toString(), + token: 'secret_token', + }, + } as RestEndpointMethodTypes['apps']['createInstallationAccessToken']['response']); + + await github.getCredentials({ url: 'https://github.com/backstage' }); + await github.getCredentials({ url: 'https://github.com/backstage' }); + + expect(octokit.apps.listInstallations.mock.calls.length).toBe(1); + expect(octokit.apps.createInstallationAccessToken.mock.calls.length).toBe( + 1, + ); + }); + + it('should expire access token cache when less than 10 mins before token expires', async () => { + octokit.apps.listInstallations.mockReturnValue({ + headers: { + etag: '123', + }, + data: [ + { + id: 1, + repository_selection: 'all', + account: { + login: 'backstage', + }, + }, + ], + } as RestEndpointMethodTypes['apps']['listInstallations']['response']); + + octokit.apps.createInstallationAccessToken.mockReturnValue({ + data: { + expires_at: DateTime.local() + .plus({ minutes: 9, seconds: 59, milliseconds: 999 }) + .toString(), + token: 'secret_token', + }, + } as RestEndpointMethodTypes['apps']['createInstallationAccessToken']['response']); + + await github.getCredentials({ url: 'https://github.com/backstage' }); + await github.getCredentials({ url: 'https://github.com/backstage' }); + + expect(octokit.apps.listInstallations.mock.calls.length).toBe(2); + expect(octokit.apps.createInstallationAccessToken.mock.calls.length).toBe( + 2, + ); + }); }); diff --git a/packages/integration/src/github/SingleInstanceGithubCredentialsProvider.ts b/packages/integration/src/github/SingleInstanceGithubCredentialsProvider.ts index 2fd2e7f7dc..02f050f69f 100644 --- a/packages/integration/src/github/SingleInstanceGithubCredentialsProvider.ts +++ b/packages/integration/src/github/SingleInstanceGithubCredentialsProvider.ts @@ -30,29 +30,56 @@ type InstallationData = { suspended: boolean; }; +type InstallationTokenData = { + token: string; + expiresAt: DateTime; + repositories?: String[]; +}; + class Cache { - private readonly tokenCache = new Map< - string, - { token: string; expiresAt: DateTime } - >(); + private readonly tokenCache = new Map(); async getOrCreateToken( - key: string, - supplier: () => Promise<{ token: string; expiresAt: DateTime }>, + owner: string, + repo: string | undefined, + supplier: () => Promise, ): Promise<{ accessToken: string }> { - const item = this.tokenCache.get(key); - if (item && this.isNotExpired(item.expiresAt)) { - return { accessToken: item.token }; + let existingInstallationData = this.tokenCache.get(owner); + + if ( + !existingInstallationData || + this.isExpired(existingInstallationData.expiresAt) + ) { + existingInstallationData = await supplier(); + // Allow 10 minutes grace to account for clock skew + existingInstallationData.expiresAt = + existingInstallationData.expiresAt.minus({ minutes: 10 }); + this.tokenCache.set(owner, existingInstallationData); } - const result = await supplier(); - this.tokenCache.set(key, result); - return { accessToken: result.token }; + if (!this.appliesToRepo(existingInstallationData, repo)) { + throw new Error( + `The Backstage GitHub application used in the ${owner} organization does not have access to a repository with the name ${repo}`, + ); + } + + return { accessToken: existingInstallationData.token }; } - // consider timestamps older than 50 minutes to be expired. - private isNotExpired = (date: DateTime) => - date.diff(DateTime.local(), 'minutes').minutes > 50; + private isExpired = (date: DateTime) => DateTime.local() > date; + + private appliesToRepo(tokenData: InstallationTokenData, repo?: string) { + // If no specific repo has been requested the token is applicable + if (repo === undefined) { + return true; + } + // If the token is restricted to repositories, the token only applies if the repo is in the allow list + if (tokenData.repositories !== undefined) { + return tokenData.repositories.includes(repo); + } + // Otherwise the token is applicable + return true; + } } /** @@ -93,25 +120,29 @@ class GithubAppManager { owner: string, repo?: string, ): Promise<{ accessToken: string | undefined }> { - const { installationId, suspended } = await this.getInstallationData(owner); if (this.allowedInstallationOwners) { if (!this.allowedInstallationOwners?.includes(owner)) { return { accessToken: undefined }; // An empty token allows anonymous access to public repos } } - if (suspended) { - throw new Error(`The GitHub application for ${owner} is suspended`); - } - - const cacheKey = repo ? `${owner}/${repo}` : owner; // Go and grab an access token for the app scoped to a repository if provided, if not use the organisation installation. - return this.cache.getOrCreateToken(cacheKey, async () => { + return this.cache.getOrCreateToken(owner, repo, async () => { + const { installationId, suspended } = await this.getInstallationData( + owner, + ); + if (suspended) { + throw new Error(`The GitHub application for ${owner} is suspended`); + } + const result = await this.appClient.apps.createInstallationAccessToken({ installation_id: installationId, headers: HEADERS, }); - if (repo && result.data.repository_selection === 'selected') { + + let repositoryNames; + + if (result.data.repository_selection === 'selected') { const installationClient = new Octokit({ baseUrl: this.baseUrl, auth: result.data.token, @@ -122,18 +153,13 @@ class GithubAppManager { // The return type of the paginate method is incorrect. const repositories: RestEndpointMethodTypes['apps']['listReposAccessibleToInstallation']['response']['data']['repositories'] = repos.repositories ?? repos; - const hasRepo = repositories.some(repository => { - return repository.name === repo; - }); - if (!hasRepo) { - throw new Error( - `The Backstage GitHub application used in the ${owner} organization does not have access to a repository with the name ${repo}`, - ); - } + + repositoryNames = repositories.map(repository => repository.name); } return { token: result.data.token, expiresAt: DateTime.fromISO(result.data.expires_at), + repositories: repositoryNames, }; }); }