From 7228b18c0ffdde85aefb73786902156ce820a47b Mon Sep 17 00:00:00 2001 From: Brian Fletcher Date: Fri, 17 Dec 2021 11:25:01 +0000 Subject: [PATCH 1/5] create an interface for gh creds provider We plan to build on this later to allow the credentials provider to be passed into the scaffolder tasks, the processors, and the url readers. Signed-off-by: Brian Fletcher --- .../src/reading/GithubUrlReader.ts | 3 +- packages/integration/api-report.md | 14 +- ... DefaultGithubCredentialsProvider.test.ts} | 12 +- .../DefaultGithubCredentialsProvider.ts | 287 ++++++++++++++++++ .../src/github/GithubCredentialsProvider.ts | 259 +--------------- packages/integration/src/github/index.ts | 5 +- .../processors/GithubDiscoveryProcessor.ts | 4 +- .../GithubMultiOrgReaderProcessor.ts | 5 +- .../GithubOrgReaderProcessor.test.ts | 6 +- .../processors/GithubOrgReaderProcessor.ts | 5 +- .../providers/GitHubOrgEntityProvider.test.ts | 4 +- .../providers/GitHubOrgEntityProvider.ts | 3 +- .../actions/builtin/github/OctokitProvider.ts | 5 +- .../builtin/publish/githubPullRequest.ts | 4 +- 14 files changed, 334 insertions(+), 282 deletions(-) rename packages/integration/src/github/{GithubCredentialsProvider.test.ts => DefaultGithubCredentialsProvider.test.ts} (95%) create mode 100644 packages/integration/src/github/DefaultGithubCredentialsProvider.ts diff --git a/packages/backend-common/src/reading/GithubUrlReader.ts b/packages/backend-common/src/reading/GithubUrlReader.ts index d0fbf6d8ae..e6c0572ca7 100644 --- a/packages/backend-common/src/reading/GithubUrlReader.ts +++ b/packages/backend-common/src/reading/GithubUrlReader.ts @@ -16,6 +16,7 @@ import { getGitHubFileFetchUrl, + DefaultGithubCredentialsProvider, GithubCredentialsProvider, GitHubIntegration, ScmIntegrations, @@ -58,7 +59,7 @@ export class GithubUrlReader implements UrlReader { static factory: ReaderFactory = ({ config, treeResponseFactory }) => { const integrations = ScmIntegrations.fromConfig(config); return integrations.github.list().map(integration => { - const credentialsProvider = GithubCredentialsProvider.create( + const credentialsProvider = DefaultGithubCredentialsProvider.create( integration.config, ); const reader = new GithubUrlReader(integration, { diff --git a/packages/integration/api-report.md b/packages/integration/api-report.md index fcaa39b285..385d337e0d 100644 --- a/packages/integration/api-report.md +++ b/packages/integration/api-report.md @@ -92,6 +92,17 @@ export type BitbucketIntegrationConfig = { appPassword?: string; }; +// @public +export class DefaultGithubCredentialsProvider + implements GithubCredentialsProvider +{ + // (undocumented) + static create( + config: GitHubIntegrationConfig, + ): DefaultGithubCredentialsProvider; + getCredentials(opts: { url: string }): Promise; +} + // @public export function defaultScmResolveUrl(options: { url: string; @@ -198,9 +209,8 @@ export type GithubCredentials = { }; // @public -export class GithubCredentialsProvider { +export interface GithubCredentialsProvider { // (undocumented) - static create(config: GitHubIntegrationConfig): GithubCredentialsProvider; getCredentials(opts: { url: string }): Promise; } diff --git a/packages/integration/src/github/GithubCredentialsProvider.test.ts b/packages/integration/src/github/DefaultGithubCredentialsProvider.test.ts similarity index 95% rename from packages/integration/src/github/GithubCredentialsProvider.test.ts rename to packages/integration/src/github/DefaultGithubCredentialsProvider.test.ts index 0f51db85be..4ed682aea1 100644 --- a/packages/integration/src/github/GithubCredentialsProvider.test.ts +++ b/packages/integration/src/github/DefaultGithubCredentialsProvider.test.ts @@ -31,11 +31,11 @@ jest.doMock('@octokit/rest', () => { return { Octokit }; }); -import { GithubCredentialsProvider } from './GithubCredentialsProvider'; +import { DefaultGithubCredentialsProvider } from './DefaultGithubCredentialsProvider'; import { RestEndpointMethodTypes } from '@octokit/rest'; import { DateTime } from 'luxon'; -const github = GithubCredentialsProvider.create({ +const github = DefaultGithubCredentialsProvider.create({ host: 'github.com', apps: [ { @@ -49,7 +49,7 @@ const github = GithubCredentialsProvider.create({ token: 'hardcoded_token', }); -describe('GithubCredentialsProvider tests', () => { +describe('DefaultGithubCredentialsProvider tests', () => { beforeEach(() => { jest.resetAllMocks(); }); @@ -204,7 +204,7 @@ describe('GithubCredentialsProvider tests', () => { }); it('should return the default token if no app is configured', async () => { - const githubProvider = GithubCredentialsProvider.create({ + const githubProvider = DefaultGithubCredentialsProvider.create({ host: 'github.com', apps: [], token: 'fallback_token', @@ -218,7 +218,7 @@ describe('GithubCredentialsProvider tests', () => { }); it('should return the configured token if there are no installations', async () => { - const githubProvider = GithubCredentialsProvider.create({ + const githubProvider = DefaultGithubCredentialsProvider.create({ host: 'github.com', apps: [ { @@ -243,7 +243,7 @@ describe('GithubCredentialsProvider tests', () => { }); it('should return undefined if no token or apps are configured', async () => { - const githubProvider = GithubCredentialsProvider.create({ + const githubProvider = DefaultGithubCredentialsProvider.create({ host: 'github.com', }); diff --git a/packages/integration/src/github/DefaultGithubCredentialsProvider.ts b/packages/integration/src/github/DefaultGithubCredentialsProvider.ts new file mode 100644 index 0000000000..6d9944329c --- /dev/null +++ b/packages/integration/src/github/DefaultGithubCredentialsProvider.ts @@ -0,0 +1,287 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import parseGitUrl from 'git-url-parse'; +import { GithubAppConfig, GitHubIntegrationConfig } from './config'; +import { createAppAuth } from '@octokit/auth-app'; +import { Octokit, RestEndpointMethodTypes } from '@octokit/rest'; +import { DateTime } from 'luxon'; +import { + GithubCredentials, + GithubCredentialsProvider, + GithubCredentialType, +} from './GithubCredentialsProvider'; + +type InstallationData = { + installationId: number; + suspended: boolean; +}; + +class Cache { + private readonly tokenCache = new Map< + string, + { token: string; expiresAt: DateTime } + >(); + + async getOrCreateToken( + key: string, + supplier: () => Promise<{ token: string; expiresAt: DateTime }>, + ): Promise<{ accessToken: string }> { + const item = this.tokenCache.get(key); + if (item && this.isNotExpired(item.expiresAt)) { + return { accessToken: item.token }; + } + + const result = await supplier(); + this.tokenCache.set(key, result); + return { accessToken: result.token }; + } + + // consider timestamps older than 50 minutes to be expired. + private isNotExpired = (date: DateTime) => + date.diff(DateTime.local(), 'minutes').minutes > 50; +} + +/** + * This accept header is required when calling App APIs in GitHub Enterprise. + * It has no effect on calls to github.com and can probably be removed entirely + * once GitHub Apps is out of preview. + */ +const HEADERS = { + Accept: 'application/vnd.github.machine-man-preview+json', +}; + +/** + * GithubAppManager issues and caches tokens for a specific GitHub App. + */ +class GithubAppManager { + private readonly appClient: Octokit; + private readonly baseUrl?: string; + private readonly baseAuthConfig: { appId: number; privateKey: string }; + private readonly cache = new Cache(); + private readonly allowedInstallationOwners: string[] | undefined; // undefined allows all installations + + constructor(config: GithubAppConfig, baseUrl?: string) { + this.allowedInstallationOwners = config.allowedInstallationOwners; + this.baseUrl = baseUrl; + this.baseAuthConfig = { + appId: config.appId, + privateKey: config.privateKey.replace(/\\n/gm, '\n'), + }; + this.appClient = new Octokit({ + baseUrl, + headers: HEADERS, + authStrategy: createAppAuth, + auth: this.baseAuthConfig, + }); + } + + async getInstallationCredentials( + owner: string, + repo?: string, + ): Promise<{ accessToken: string }> { + const { installationId, suspended } = await this.getInstallationData(owner); + if (this.allowedInstallationOwners) { + if (!this.allowedInstallationOwners?.includes(owner)) { + throw new Error( + `The GitHub application for ${owner} is not included in the allowed installation list (${installationId}).`, + ); + } + } + 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 () => { + const result = await this.appClient.apps.createInstallationAccessToken({ + installation_id: installationId, + headers: HEADERS, + }); + if (repo && result.data.repository_selection === 'selected') { + const installationClient = new Octokit({ + baseUrl: this.baseUrl, + auth: result.data.token, + }); + const repos = await installationClient.paginate( + installationClient.apps.listReposAccessibleToInstallation, + ); + const hasRepo = repos.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}`, + ); + } + } + return { + token: result.data.token, + expiresAt: DateTime.fromISO(result.data.expires_at), + }; + }); + } + + getInstallations(): Promise< + RestEndpointMethodTypes['apps']['listInstallations']['response']['data'] + > { + return this.appClient.paginate(this.appClient.apps.listInstallations); + } + + private async getInstallationData(owner: string): Promise { + const allInstallations = await this.getInstallations(); + const installation = allInstallations.find( + inst => + inst.account?.login?.toLocaleLowerCase('en-US') === + owner.toLocaleLowerCase('en-US'), + ); + if (installation) { + return { + installationId: installation.id, + suspended: Boolean(installation.suspended_by), + }; + } + const notFoundError = new Error( + `No app installation found for ${owner} in ${this.baseAuthConfig.appId}`, + ); + notFoundError.name = 'NotFoundError'; + throw notFoundError; + } +} + +/** + * Corresponds to a Github installation which internally could hold several GitHub Apps. + * + * @public + */ +export class GithubAppCredentialsMux { + private readonly apps: GithubAppManager[]; + + constructor(config: GitHubIntegrationConfig) { + this.apps = + config.apps?.map(ac => new GithubAppManager(ac, config.apiBaseUrl)) ?? []; + } + + async getAllInstallations(): Promise< + RestEndpointMethodTypes['apps']['listInstallations']['response']['data'] + > { + if (!this.apps.length) { + return []; + } + + const installs = await Promise.all( + this.apps.map(app => app.getInstallations()), + ); + + return installs.flat(); + } + + async getAppToken(owner: string, repo?: string): Promise { + if (this.apps.length === 0) { + return undefined; + } + + const results = await Promise.all( + this.apps.map(app => + app.getInstallationCredentials(owner, repo).then( + credentials => ({ credentials, error: undefined }), + error => ({ credentials: undefined, error }), + ), + ), + ); + + const result = results.find(resultItem => resultItem.credentials); + if (result) { + return result.credentials!.accessToken; + } + + const errors = results.map(r => r.error); + const notNotFoundError = errors.find(err => err.name !== 'NotFoundError'); + if (notNotFoundError) { + throw notNotFoundError; + } + + return undefined; + } +} + +/** + * Handles the creation and caching of credentials for GitHub integrations. + * + * @public + * @remarks + * + * TODO: Possibly move this to a backend only package so that it's not used in the frontend by mistake + */ +export class DefaultGithubCredentialsProvider + implements GithubCredentialsProvider +{ + static create( + config: GitHubIntegrationConfig, + ): DefaultGithubCredentialsProvider { + return new DefaultGithubCredentialsProvider( + new GithubAppCredentialsMux(config), + config.token, + ); + } + + private constructor( + private readonly githubAppCredentialsMux: GithubAppCredentialsMux, + private readonly token?: string, + ) {} + + /** + * Returns {@link GithubCredentials} for a given URL. + * + * @remarks + * + * Consecutive calls to this method with the same URL will return cached + * credentials. + * + * The shortest lifetime for a token returned is 10 minutes. + * + * @example + * ```ts + * const { token, headers } = await getCredentials({ + * url: 'github.com/backstage/foobar' + * }) + * ``` + * + * @param opts - The organization or repository URL + * @returns A promise of {@link GithubCredentials}. + */ + async getCredentials(opts: { url: string }): Promise { + const parsed = parseGitUrl(opts.url); + + const owner = parsed.owner || parsed.name; + const repo = parsed.owner ? parsed.name : undefined; + + let type: GithubCredentialType = 'app'; + let token = await this.githubAppCredentialsMux.getAppToken(owner, repo); + if (!token) { + type = 'token'; + token = this.token; + } + + return { + headers: token ? { Authorization: `Bearer ${token}` } : undefined, + token, + type, + }; + } +} diff --git a/packages/integration/src/github/GithubCredentialsProvider.ts b/packages/integration/src/github/GithubCredentialsProvider.ts index ece692fecc..15f5375570 100644 --- a/packages/integration/src/github/GithubCredentialsProvider.ts +++ b/packages/integration/src/github/GithubCredentialsProvider.ts @@ -14,207 +14,6 @@ * limitations under the License. */ -import parseGitUrl from 'git-url-parse'; -import { GithubAppConfig, GitHubIntegrationConfig } from './config'; -import { createAppAuth } from '@octokit/auth-app'; -import { Octokit, RestEndpointMethodTypes } from '@octokit/rest'; -import { DateTime } from 'luxon'; - -type InstallationData = { - installationId: number; - suspended: boolean; -}; - -class Cache { - private readonly tokenCache = new Map< - string, - { token: string; expiresAt: DateTime } - >(); - - async getOrCreateToken( - key: string, - supplier: () => Promise<{ token: string; expiresAt: DateTime }>, - ): Promise<{ accessToken: string }> { - const item = this.tokenCache.get(key); - if (item && this.isNotExpired(item.expiresAt)) { - return { accessToken: item.token }; - } - - const result = await supplier(); - this.tokenCache.set(key, result); - return { accessToken: result.token }; - } - - // consider timestamps older than 50 minutes to be expired. - private isNotExpired = (date: DateTime) => - date.diff(DateTime.local(), 'minutes').minutes > 50; -} - -/** - * This accept header is required when calling App APIs in GitHub Enterprise. - * It has no effect on calls to github.com and can probably be removed entirely - * once GitHub Apps is out of preview. - */ -const HEADERS = { - Accept: 'application/vnd.github.machine-man-preview+json', -}; - -/** - * GithubAppManager issues and caches tokens for a specific GitHub App. - */ -class GithubAppManager { - private readonly appClient: Octokit; - private readonly baseUrl?: string; - private readonly baseAuthConfig: { appId: number; privateKey: string }; - private readonly cache = new Cache(); - private readonly allowedInstallationOwners: string[] | undefined; // undefined allows all installations - - constructor(config: GithubAppConfig, baseUrl?: string) { - this.allowedInstallationOwners = config.allowedInstallationOwners; - this.baseUrl = baseUrl; - this.baseAuthConfig = { - appId: config.appId, - privateKey: config.privateKey.replace(/\\n/gm, '\n'), - }; - this.appClient = new Octokit({ - baseUrl, - headers: HEADERS, - authStrategy: createAppAuth, - auth: this.baseAuthConfig, - }); - } - - async getInstallationCredentials( - owner: string, - repo?: string, - ): Promise<{ accessToken: string }> { - const { installationId, suspended } = await this.getInstallationData(owner); - if (this.allowedInstallationOwners) { - if (!this.allowedInstallationOwners?.includes(owner)) { - throw new Error( - `The GitHub application for ${owner} is not included in the allowed installation list (${installationId}).`, - ); - } - } - 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 () => { - const result = await this.appClient.apps.createInstallationAccessToken({ - installation_id: installationId, - headers: HEADERS, - }); - if (repo && result.data.repository_selection === 'selected') { - const installationClient = new Octokit({ - baseUrl: this.baseUrl, - auth: result.data.token, - }); - const repos = await installationClient.paginate( - installationClient.apps.listReposAccessibleToInstallation, - ); - const hasRepo = repos.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}`, - ); - } - } - return { - token: result.data.token, - expiresAt: DateTime.fromISO(result.data.expires_at), - }; - }); - } - - getInstallations(): Promise< - RestEndpointMethodTypes['apps']['listInstallations']['response']['data'] - > { - return this.appClient.paginate(this.appClient.apps.listInstallations); - } - - private async getInstallationData(owner: string): Promise { - const allInstallations = await this.getInstallations(); - const installation = allInstallations.find( - inst => - inst.account?.login?.toLocaleLowerCase('en-US') === - owner.toLocaleLowerCase('en-US'), - ); - if (installation) { - return { - installationId: installation.id, - suspended: Boolean(installation.suspended_by), - }; - } - const notFoundError = new Error( - `No app installation found for ${owner} in ${this.baseAuthConfig.appId}`, - ); - notFoundError.name = 'NotFoundError'; - throw notFoundError; - } -} - -/** - * Corresponds to a Github installation which internally could hold several GitHub Apps. - * - * @public - */ -export class GithubAppCredentialsMux { - private readonly apps: GithubAppManager[]; - - constructor(config: GitHubIntegrationConfig) { - this.apps = - config.apps?.map(ac => new GithubAppManager(ac, config.apiBaseUrl)) ?? []; - } - - async getAllInstallations(): Promise< - RestEndpointMethodTypes['apps']['listInstallations']['response']['data'] - > { - if (!this.apps.length) { - return []; - } - - const installs = await Promise.all( - this.apps.map(app => app.getInstallations()), - ); - - return installs.flat(); - } - - async getAppToken(owner: string, repo?: string): Promise { - if (this.apps.length === 0) { - return undefined; - } - - const results = await Promise.all( - this.apps.map(app => - app.getInstallationCredentials(owner, repo).then( - credentials => ({ credentials, error: undefined }), - error => ({ credentials: undefined, error }), - ), - ), - ); - - const result = results.find(resultItem => resultItem.credentials); - if (result) { - return result.credentials!.accessToken; - } - - const errors = results.map(r => r.error); - const notNotFoundError = errors.find(err => err.name !== 'NotFoundError'); - if (notNotFoundError) { - throw notNotFoundError; - } - - return undefined; - } -} - /** * The type of credentials produced by the credential provider. * @@ -234,63 +33,11 @@ export type GithubCredentials = { }; /** - * Handles the creation and caching of credentials for GitHub integrations. + * This allows implementations to be provided to retrieve GitHub credentials. * * @public - * @remarks * - * TODO: Possibly move this to a backend only package so that it's not used in the frontend by mistake */ -export class GithubCredentialsProvider { - static create(config: GitHubIntegrationConfig): GithubCredentialsProvider { - return new GithubCredentialsProvider( - new GithubAppCredentialsMux(config), - config.token, - ); - } - - private constructor( - private readonly githubAppCredentialsMux: GithubAppCredentialsMux, - private readonly token?: string, - ) {} - - /** - * Returns {@link GithubCredentials} for a given URL. - * - * @remarks - * - * Consecutive calls to this method with the same URL will return cached - * credentials. - * - * The shortest lifetime for a token returned is 10 minutes. - * - * @example - * ```ts - * const { token, headers } = await getCredentials({ - * url: 'github.com/backstage/foobar' - * }) - * ``` - * - * @param opts - The organization or repository URL - * @returns A promise of {@link GithubCredentials}. - */ - async getCredentials(opts: { url: string }): Promise { - const parsed = parseGitUrl(opts.url); - - const owner = parsed.owner || parsed.name; - const repo = parsed.owner ? parsed.name : undefined; - - let type: GithubCredentialType = 'app'; - let token = await this.githubAppCredentialsMux.getAppToken(owner, repo); - if (!token) { - type = 'token'; - token = this.token; - } - - return { - headers: token ? { Authorization: `Bearer ${token}` } : undefined, - token, - type, - }; - } +export interface GithubCredentialsProvider { + getCredentials(opts: { url: string }): Promise; } diff --git a/packages/integration/src/github/index.ts b/packages/integration/src/github/index.ts index 9c13f13135..440691c436 100644 --- a/packages/integration/src/github/index.ts +++ b/packages/integration/src/github/index.ts @@ -22,10 +22,11 @@ export type { GithubAppConfig, GitHubIntegrationConfig } from './config'; export { getGitHubFileFetchUrl, getGitHubRequestOptions } from './core'; export { GithubAppCredentialsMux, - GithubCredentialsProvider, -} from './GithubCredentialsProvider'; + DefaultGithubCredentialsProvider, +} from './DefaultGithubCredentialsProvider'; export type { GithubCredentials, + GithubCredentialsProvider, GithubCredentialType, } from './GithubCredentialsProvider'; export { GitHubIntegration, replaceGitHubUrlType } from './GitHubIntegration'; diff --git a/plugins/catalog-backend/src/ingestion/processors/GithubDiscoveryProcessor.ts b/plugins/catalog-backend/src/ingestion/processors/GithubDiscoveryProcessor.ts index 9bd6cc775e..d2498a39d8 100644 --- a/plugins/catalog-backend/src/ingestion/processors/GithubDiscoveryProcessor.ts +++ b/plugins/catalog-backend/src/ingestion/processors/GithubDiscoveryProcessor.ts @@ -17,7 +17,7 @@ import { LocationSpec } from '@backstage/catalog-model'; import { Config } from '@backstage/config'; import { - GithubCredentialsProvider, + DefaultGithubCredentialsProvider, ScmIntegrations, } from '@backstage/integration'; import { graphql } from '@octokit/graphql'; @@ -84,7 +84,7 @@ export class GithubDiscoveryProcessor implements CatalogProcessor { // about how to handle the wild card which is special for this processor. const orgUrl = `https://${host}/${org}`; - const { headers } = await GithubCredentialsProvider.create( + const { headers } = await DefaultGithubCredentialsProvider.create( gitHubConfig, ).getCredentials({ url: orgUrl }); diff --git a/plugins/catalog-backend/src/ingestion/processors/GithubMultiOrgReaderProcessor.ts b/plugins/catalog-backend/src/ingestion/processors/GithubMultiOrgReaderProcessor.ts index 00b173777d..ba346fa0c6 100644 --- a/plugins/catalog-backend/src/ingestion/processors/GithubMultiOrgReaderProcessor.ts +++ b/plugins/catalog-backend/src/ingestion/processors/GithubMultiOrgReaderProcessor.ts @@ -18,7 +18,7 @@ import { LocationSpec } from '@backstage/catalog-model'; import { Config } from '@backstage/config'; import { GithubAppCredentialsMux, - GithubCredentialsProvider, + DefaultGithubCredentialsProvider, GitHubIntegrationConfig, ScmIntegrations, } from '@backstage/integration'; @@ -86,7 +86,8 @@ export class GithubMultiOrgReaderProcessor implements CatalogProcessor { const allUsersMap = new Map(); const baseUrl = new URL(location.target).origin; - const credentialsProvider = GithubCredentialsProvider.create(gitHubConfig); + const credentialsProvider = + DefaultGithubCredentialsProvider.create(gitHubConfig); const orgsToProcess = this.orgs.length ? this.orgs diff --git a/plugins/catalog-backend/src/ingestion/processors/GithubOrgReaderProcessor.test.ts b/plugins/catalog-backend/src/ingestion/processors/GithubOrgReaderProcessor.test.ts index 87a910ee06..0e1a65df93 100644 --- a/plugins/catalog-backend/src/ingestion/processors/GithubOrgReaderProcessor.test.ts +++ b/plugins/catalog-backend/src/ingestion/processors/GithubOrgReaderProcessor.test.ts @@ -17,7 +17,7 @@ import { getVoidLogger } from '@backstage/backend-common'; import { LocationSpec } from '@backstage/catalog-model'; import { ConfigReader } from '@backstage/config'; import { - GithubCredentialsProvider, + DefaultGithubCredentialsProvider, ScmIntegrations, } from '@backstage/integration'; import { graphql } from '@octokit/graphql'; @@ -87,7 +87,7 @@ describe('GithubOrgReaderProcessor', () => { (graphql.defaults as jest.Mock).mockReturnValue(mockClient); - jest.spyOn(GithubCredentialsProvider, 'create').mockReturnValue({ + jest.spyOn(DefaultGithubCredentialsProvider, 'create').mockReturnValue({ getCredentials: mockGetCredentials, } as any); @@ -135,7 +135,7 @@ describe('GithubOrgReaderProcessor', () => { (graphql.defaults as jest.Mock).mockReturnValue(mockClient); - jest.spyOn(GithubCredentialsProvider, 'create').mockReturnValue({ + jest.spyOn(DefaultGithubCredentialsProvider, 'create').mockReturnValue({ getCredentials: mockGetCredentials, } as any); diff --git a/plugins/catalog-backend/src/ingestion/processors/GithubOrgReaderProcessor.ts b/plugins/catalog-backend/src/ingestion/processors/GithubOrgReaderProcessor.ts index 244dbd8104..cb455005fe 100644 --- a/plugins/catalog-backend/src/ingestion/processors/GithubOrgReaderProcessor.ts +++ b/plugins/catalog-backend/src/ingestion/processors/GithubOrgReaderProcessor.ts @@ -17,7 +17,7 @@ import { LocationSpec } from '@backstage/catalog-model'; import { Config } from '@backstage/config'; import { - GithubCredentialsProvider, + DefaultGithubCredentialsProvider, GithubCredentialType, ScmIntegrations, } from '@backstage/integration'; @@ -107,7 +107,8 @@ export class GithubOrgReaderProcessor implements CatalogProcessor { ); } - const credentialsProvider = GithubCredentialsProvider.create(gitHubConfig); + const credentialsProvider = + DefaultGithubCredentialsProvider.create(gitHubConfig); const { headers, type: tokenType } = await credentialsProvider.getCredentials({ url: orgUrl, diff --git a/plugins/catalog-backend/src/ingestion/providers/GitHubOrgEntityProvider.test.ts b/plugins/catalog-backend/src/ingestion/providers/GitHubOrgEntityProvider.test.ts index 75edea6909..17c0b7054b 100644 --- a/plugins/catalog-backend/src/ingestion/providers/GitHubOrgEntityProvider.test.ts +++ b/plugins/catalog-backend/src/ingestion/providers/GitHubOrgEntityProvider.test.ts @@ -17,7 +17,7 @@ import { getVoidLogger } from '@backstage/backend-common'; import { GroupEntity, UserEntity } from '@backstage/catalog-model'; import { - GithubCredentialsProvider, + DefaultGithubCredentialsProvider, GitHubIntegrationConfig, } from '@backstage/integration'; import { GitHubOrgEntityProvider } from '.'; @@ -93,7 +93,7 @@ describe('GitHubOrgEntityProvider', () => { type: 'app', }); - jest.spyOn(GithubCredentialsProvider, 'create').mockReturnValue({ + jest.spyOn(DefaultGithubCredentialsProvider, 'create').mockReturnValue({ getCredentials: mockGetCredentials, } as any); diff --git a/plugins/catalog-backend/src/ingestion/providers/GitHubOrgEntityProvider.ts b/plugins/catalog-backend/src/ingestion/providers/GitHubOrgEntityProvider.ts index ecd0d153d1..63d915d817 100644 --- a/plugins/catalog-backend/src/ingestion/providers/GitHubOrgEntityProvider.ts +++ b/plugins/catalog-backend/src/ingestion/providers/GitHubOrgEntityProvider.ts @@ -20,6 +20,7 @@ import { } from '@backstage/catalog-model'; import { Config } from '@backstage/config'; import { + DefaultGithubCredentialsProvider, GithubCredentialsProvider, GitHubIntegrationConfig, ScmIntegrations, @@ -77,7 +78,7 @@ export class GitHubOrgEntityProvider implements EntityProvider { logger: Logger; }, ) { - this.credentialsProvider = GithubCredentialsProvider.create( + this.credentialsProvider = DefaultGithubCredentialsProvider.create( options.gitHubConfig, ); } diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/OctokitProvider.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/OctokitProvider.ts index bea027a91b..4fbd45c0ee 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/OctokitProvider.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/OctokitProvider.ts @@ -16,6 +16,7 @@ import { InputError } from '@backstage/errors'; import { + DefaultGithubCredentialsProvider, GithubCredentialsProvider, ScmIntegrationRegistry, } from '@backstage/integration'; @@ -40,7 +41,9 @@ export class OctokitProvider { this.integrations = integrations; this.credentialsProviders = new Map( integrations.github.list().map(integration => { - const provider = GithubCredentialsProvider.create(integration.config); + const provider = DefaultGithubCredentialsProvider.create( + integration.config, + ); return [integration.config.host, provider]; }), ); diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts index 3a9dd01686..79b393a16a 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts @@ -18,7 +18,7 @@ import fs from 'fs-extra'; import { parseRepoUrl, isExecutable } from './util'; import { - GithubCredentialsProvider, + DefaultGithubCredentialsProvider, ScmIntegrationRegistry, } from '@backstage/integration'; import { zipObject } from 'lodash'; @@ -76,7 +76,7 @@ export const defaultClientFactory = async ({ } const credentialsProvider = - GithubCredentialsProvider.create(integrationConfig); + DefaultGithubCredentialsProvider.create(integrationConfig); if (!credentialsProvider) { throw new InputError( From 6984c4988a8e4bf9c3451b8de39efc286582278c Mon Sep 17 00:00:00 2001 From: Brian Fletcher Date: Mon, 20 Dec 2021 11:05:58 +0000 Subject: [PATCH 2/5] moves types and interfaces to specific types file Signed-off-by: Brian Fletcher --- .../integration/src/github/DefaultGithubCredentialsProvider.ts | 2 +- packages/integration/src/github/core.test.ts | 2 +- packages/integration/src/github/core.ts | 2 +- packages/integration/src/github/index.ts | 2 +- .../src/github/{GithubCredentialsProvider.ts => types.ts} | 0 5 files changed, 4 insertions(+), 4 deletions(-) rename packages/integration/src/github/{GithubCredentialsProvider.ts => types.ts} (100%) diff --git a/packages/integration/src/github/DefaultGithubCredentialsProvider.ts b/packages/integration/src/github/DefaultGithubCredentialsProvider.ts index 6d9944329c..259fcf71a8 100644 --- a/packages/integration/src/github/DefaultGithubCredentialsProvider.ts +++ b/packages/integration/src/github/DefaultGithubCredentialsProvider.ts @@ -23,7 +23,7 @@ import { GithubCredentials, GithubCredentialsProvider, GithubCredentialType, -} from './GithubCredentialsProvider'; +} from './types'; type InstallationData = { installationId: number; diff --git a/packages/integration/src/github/core.test.ts b/packages/integration/src/github/core.test.ts index 53733b40d0..c5d04f5b4f 100644 --- a/packages/integration/src/github/core.test.ts +++ b/packages/integration/src/github/core.test.ts @@ -16,7 +16,7 @@ import { GitHubIntegrationConfig } from './config'; import { getGitHubFileFetchUrl, getGitHubRequestOptions } from './core'; -import { GithubCredentials } from './GithubCredentialsProvider'; +import { GithubCredentials } from './types'; describe('github core', () => { const appCredentials: GithubCredentials = { diff --git a/packages/integration/src/github/core.ts b/packages/integration/src/github/core.ts index 76e54fd544..5b630d5182 100644 --- a/packages/integration/src/github/core.ts +++ b/packages/integration/src/github/core.ts @@ -16,7 +16,7 @@ import parseGitUrl from 'git-url-parse'; import { GitHubIntegrationConfig } from './config'; -import { GithubCredentials } from './GithubCredentialsProvider'; +import { GithubCredentials } from './types'; /** * Given a URL pointing to a file on a provider, returns a URL that is suitable diff --git a/packages/integration/src/github/index.ts b/packages/integration/src/github/index.ts index 440691c436..142249b0a7 100644 --- a/packages/integration/src/github/index.ts +++ b/packages/integration/src/github/index.ts @@ -28,5 +28,5 @@ export type { GithubCredentials, GithubCredentialsProvider, GithubCredentialType, -} from './GithubCredentialsProvider'; +} from './types'; export { GitHubIntegration, replaceGitHubUrlType } from './GitHubIntegration'; diff --git a/packages/integration/src/github/GithubCredentialsProvider.ts b/packages/integration/src/github/types.ts similarity index 100% rename from packages/integration/src/github/GithubCredentialsProvider.ts rename to packages/integration/src/github/types.ts From b2d67bde044a332f1db034b7ef3f0669d598211a Mon Sep 17 00:00:00 2001 From: Brian Fletcher Date: Mon, 20 Dec 2021 13:54:19 +0000 Subject: [PATCH 3/5] adds factory type for creds provider and rename default Signed-off-by: Brian Fletcher --- .../src/reading/GithubUrlReader.ts | 7 +++--- packages/integration/api-report.md | 25 +++++++++++-------- .../DefaultGithubCredentialsProvider.test.ts | 10 ++++---- ...ingleInstanceGithubCredentialsProvider.ts} | 11 ++++---- packages/integration/src/github/index.ts | 5 ++-- packages/integration/src/github/types.ts | 12 +++++++++ .../processors/GithubDiscoveryProcessor.ts | 4 +-- .../GithubMultiOrgReaderProcessor.ts | 4 +-- .../GithubOrgReaderProcessor.test.ts | 18 +++++++------ .../processors/GithubOrgReaderProcessor.ts | 4 +-- .../providers/GitHubOrgEntityProvider.test.ts | 10 +++++--- .../providers/GitHubOrgEntityProvider.ts | 4 +-- .../actions/builtin/github/OctokitProvider.ts | 4 +-- .../builtin/publish/githubPullRequest.ts | 4 +-- 14 files changed, 71 insertions(+), 51 deletions(-) rename packages/integration/src/github/{DefaultGithubCredentialsProvider.ts => SingleInstanceGithubCredentialsProvider.ts} (97%) diff --git a/packages/backend-common/src/reading/GithubUrlReader.ts b/packages/backend-common/src/reading/GithubUrlReader.ts index e6c0572ca7..6b18cfbe1a 100644 --- a/packages/backend-common/src/reading/GithubUrlReader.ts +++ b/packages/backend-common/src/reading/GithubUrlReader.ts @@ -16,7 +16,7 @@ import { getGitHubFileFetchUrl, - DefaultGithubCredentialsProvider, + SingleInstanceGithubCredentialsProvider, GithubCredentialsProvider, GitHubIntegration, ScmIntegrations, @@ -59,9 +59,8 @@ export class GithubUrlReader implements UrlReader { static factory: ReaderFactory = ({ config, treeResponseFactory }) => { const integrations = ScmIntegrations.fromConfig(config); return integrations.github.list().map(integration => { - const credentialsProvider = DefaultGithubCredentialsProvider.create( - integration.config, - ); + const credentialsProvider = + SingleInstanceGithubCredentialsProvider.create(integration.config); const reader = new GithubUrlReader(integration, { treeResponseFactory, credentialsProvider, diff --git a/packages/integration/api-report.md b/packages/integration/api-report.md index 385d337e0d..ca24848db8 100644 --- a/packages/integration/api-report.md +++ b/packages/integration/api-report.md @@ -92,17 +92,6 @@ export type BitbucketIntegrationConfig = { appPassword?: string; }; -// @public -export class DefaultGithubCredentialsProvider - implements GithubCredentialsProvider -{ - // (undocumented) - static create( - config: GitHubIntegrationConfig, - ): DefaultGithubCredentialsProvider; - getCredentials(opts: { url: string }): Promise; -} - // @public export function defaultScmResolveUrl(options: { url: string; @@ -214,6 +203,11 @@ export interface GithubCredentialsProvider { getCredentials(opts: { url: string }): Promise; } +// @public +export type GithubCredentialsProviderFactory = ( + config: GitHubIntegrationConfig, +) => GithubCredentialsProvider; + // @public export type GithubCredentialType = 'app' | 'token'; @@ -433,6 +427,15 @@ export interface ScmIntegrationsGroup { list(): T[]; } +// @public +export class SingleInstanceGithubCredentialsProvider + implements GithubCredentialsProvider +{ + // (undocumented) + static create: GithubCredentialsProviderFactory; + getCredentials(opts: { url: string }): Promise; +} + // Warnings were encountered during analysis: // // src/gitlab/config.d.ts:29:68 - (tsdoc-escape-right-brace) The "}" character should be escaped using a backslash to avoid confusion with a TSDoc inline tag diff --git a/packages/integration/src/github/DefaultGithubCredentialsProvider.test.ts b/packages/integration/src/github/DefaultGithubCredentialsProvider.test.ts index 4ed682aea1..6bea9647a1 100644 --- a/packages/integration/src/github/DefaultGithubCredentialsProvider.test.ts +++ b/packages/integration/src/github/DefaultGithubCredentialsProvider.test.ts @@ -31,11 +31,11 @@ jest.doMock('@octokit/rest', () => { return { Octokit }; }); -import { DefaultGithubCredentialsProvider } from './DefaultGithubCredentialsProvider'; +import { SingleInstanceGithubCredentialsProvider } from './SingleInstanceGithubCredentialsProvider'; import { RestEndpointMethodTypes } from '@octokit/rest'; import { DateTime } from 'luxon'; -const github = DefaultGithubCredentialsProvider.create({ +const github = SingleInstanceGithubCredentialsProvider.create({ host: 'github.com', apps: [ { @@ -204,7 +204,7 @@ describe('DefaultGithubCredentialsProvider tests', () => { }); it('should return the default token if no app is configured', async () => { - const githubProvider = DefaultGithubCredentialsProvider.create({ + const githubProvider = SingleInstanceGithubCredentialsProvider.create({ host: 'github.com', apps: [], token: 'fallback_token', @@ -218,7 +218,7 @@ describe('DefaultGithubCredentialsProvider tests', () => { }); it('should return the configured token if there are no installations', async () => { - const githubProvider = DefaultGithubCredentialsProvider.create({ + const githubProvider = SingleInstanceGithubCredentialsProvider.create({ host: 'github.com', apps: [ { @@ -243,7 +243,7 @@ describe('DefaultGithubCredentialsProvider tests', () => { }); it('should return undefined if no token or apps are configured', async () => { - const githubProvider = DefaultGithubCredentialsProvider.create({ + const githubProvider = SingleInstanceGithubCredentialsProvider.create({ host: 'github.com', }); diff --git a/packages/integration/src/github/DefaultGithubCredentialsProvider.ts b/packages/integration/src/github/SingleInstanceGithubCredentialsProvider.ts similarity index 97% rename from packages/integration/src/github/DefaultGithubCredentialsProvider.ts rename to packages/integration/src/github/SingleInstanceGithubCredentialsProvider.ts index 259fcf71a8..5b6fa39e75 100644 --- a/packages/integration/src/github/DefaultGithubCredentialsProvider.ts +++ b/packages/integration/src/github/SingleInstanceGithubCredentialsProvider.ts @@ -22,6 +22,7 @@ import { DateTime } from 'luxon'; import { GithubCredentials, GithubCredentialsProvider, + GithubCredentialsProviderFactory, GithubCredentialType, } from './types'; @@ -228,17 +229,15 @@ export class GithubAppCredentialsMux { * * TODO: Possibly move this to a backend only package so that it's not used in the frontend by mistake */ -export class DefaultGithubCredentialsProvider +export class SingleInstanceGithubCredentialsProvider implements GithubCredentialsProvider { - static create( - config: GitHubIntegrationConfig, - ): DefaultGithubCredentialsProvider { - return new DefaultGithubCredentialsProvider( + static create: GithubCredentialsProviderFactory = config => { + return new SingleInstanceGithubCredentialsProvider( new GithubAppCredentialsMux(config), config.token, ); - } + }; private constructor( private readonly githubAppCredentialsMux: GithubAppCredentialsMux, diff --git a/packages/integration/src/github/index.ts b/packages/integration/src/github/index.ts index 142249b0a7..fa8dfb4b86 100644 --- a/packages/integration/src/github/index.ts +++ b/packages/integration/src/github/index.ts @@ -22,11 +22,12 @@ export type { GithubAppConfig, GitHubIntegrationConfig } from './config'; export { getGitHubFileFetchUrl, getGitHubRequestOptions } from './core'; export { GithubAppCredentialsMux, - DefaultGithubCredentialsProvider, -} from './DefaultGithubCredentialsProvider'; + SingleInstanceGithubCredentialsProvider, +} from './SingleInstanceGithubCredentialsProvider'; export type { GithubCredentials, GithubCredentialsProvider, + GithubCredentialsProviderFactory, GithubCredentialType, } from './types'; export { GitHubIntegration, replaceGitHubUrlType } from './GitHubIntegration'; diff --git a/packages/integration/src/github/types.ts b/packages/integration/src/github/types.ts index 15f5375570..48609d46f0 100644 --- a/packages/integration/src/github/types.ts +++ b/packages/integration/src/github/types.ts @@ -14,6 +14,8 @@ * limitations under the License. */ +import { GitHubIntegrationConfig } from './config'; + /** * The type of credentials produced by the credential provider. * @@ -41,3 +43,13 @@ export type GithubCredentials = { export interface GithubCredentialsProvider { getCredentials(opts: { url: string }): Promise; } + +/** + * This allows implementations to provide factories to create credential providers + * + * @public + * + */ +export type GithubCredentialsProviderFactory = ( + config: GitHubIntegrationConfig, +) => GithubCredentialsProvider; diff --git a/plugins/catalog-backend/src/ingestion/processors/GithubDiscoveryProcessor.ts b/plugins/catalog-backend/src/ingestion/processors/GithubDiscoveryProcessor.ts index d2498a39d8..046455c484 100644 --- a/plugins/catalog-backend/src/ingestion/processors/GithubDiscoveryProcessor.ts +++ b/plugins/catalog-backend/src/ingestion/processors/GithubDiscoveryProcessor.ts @@ -17,7 +17,7 @@ import { LocationSpec } from '@backstage/catalog-model'; import { Config } from '@backstage/config'; import { - DefaultGithubCredentialsProvider, + SingleInstanceGithubCredentialsProvider, ScmIntegrations, } from '@backstage/integration'; import { graphql } from '@octokit/graphql'; @@ -84,7 +84,7 @@ export class GithubDiscoveryProcessor implements CatalogProcessor { // about how to handle the wild card which is special for this processor. const orgUrl = `https://${host}/${org}`; - const { headers } = await DefaultGithubCredentialsProvider.create( + const { headers } = await SingleInstanceGithubCredentialsProvider.create( gitHubConfig, ).getCredentials({ url: orgUrl }); diff --git a/plugins/catalog-backend/src/ingestion/processors/GithubMultiOrgReaderProcessor.ts b/plugins/catalog-backend/src/ingestion/processors/GithubMultiOrgReaderProcessor.ts index ba346fa0c6..5ad735ceeb 100644 --- a/plugins/catalog-backend/src/ingestion/processors/GithubMultiOrgReaderProcessor.ts +++ b/plugins/catalog-backend/src/ingestion/processors/GithubMultiOrgReaderProcessor.ts @@ -18,7 +18,7 @@ import { LocationSpec } from '@backstage/catalog-model'; import { Config } from '@backstage/config'; import { GithubAppCredentialsMux, - DefaultGithubCredentialsProvider, + SingleInstanceGithubCredentialsProvider, GitHubIntegrationConfig, ScmIntegrations, } from '@backstage/integration'; @@ -87,7 +87,7 @@ export class GithubMultiOrgReaderProcessor implements CatalogProcessor { const allUsersMap = new Map(); const baseUrl = new URL(location.target).origin; const credentialsProvider = - DefaultGithubCredentialsProvider.create(gitHubConfig); + SingleInstanceGithubCredentialsProvider.create(gitHubConfig); const orgsToProcess = this.orgs.length ? this.orgs diff --git a/plugins/catalog-backend/src/ingestion/processors/GithubOrgReaderProcessor.test.ts b/plugins/catalog-backend/src/ingestion/processors/GithubOrgReaderProcessor.test.ts index 0e1a65df93..9124637e52 100644 --- a/plugins/catalog-backend/src/ingestion/processors/GithubOrgReaderProcessor.test.ts +++ b/plugins/catalog-backend/src/ingestion/processors/GithubOrgReaderProcessor.test.ts @@ -17,7 +17,7 @@ import { getVoidLogger } from '@backstage/backend-common'; import { LocationSpec } from '@backstage/catalog-model'; import { ConfigReader } from '@backstage/config'; import { - DefaultGithubCredentialsProvider, + SingleInstanceGithubCredentialsProvider, ScmIntegrations, } from '@backstage/integration'; import { graphql } from '@octokit/graphql'; @@ -87,9 +87,11 @@ describe('GithubOrgReaderProcessor', () => { (graphql.defaults as jest.Mock).mockReturnValue(mockClient); - jest.spyOn(DefaultGithubCredentialsProvider, 'create').mockReturnValue({ - getCredentials: mockGetCredentials, - } as any); + jest + .spyOn(SingleInstanceGithubCredentialsProvider, 'create') + .mockReturnValue({ + getCredentials: mockGetCredentials, + } as any); const processor = new GithubOrgReaderProcessor({ integrations, @@ -135,9 +137,11 @@ describe('GithubOrgReaderProcessor', () => { (graphql.defaults as jest.Mock).mockReturnValue(mockClient); - jest.spyOn(DefaultGithubCredentialsProvider, 'create').mockReturnValue({ - getCredentials: mockGetCredentials, - } as any); + jest + .spyOn(SingleInstanceGithubCredentialsProvider, 'create') + .mockReturnValue({ + getCredentials: mockGetCredentials, + } as any); const processor = new GithubOrgReaderProcessor({ integrations, diff --git a/plugins/catalog-backend/src/ingestion/processors/GithubOrgReaderProcessor.ts b/plugins/catalog-backend/src/ingestion/processors/GithubOrgReaderProcessor.ts index cb455005fe..95db344818 100644 --- a/plugins/catalog-backend/src/ingestion/processors/GithubOrgReaderProcessor.ts +++ b/plugins/catalog-backend/src/ingestion/processors/GithubOrgReaderProcessor.ts @@ -17,7 +17,7 @@ import { LocationSpec } from '@backstage/catalog-model'; import { Config } from '@backstage/config'; import { - DefaultGithubCredentialsProvider, + SingleInstanceGithubCredentialsProvider, GithubCredentialType, ScmIntegrations, } from '@backstage/integration'; @@ -108,7 +108,7 @@ export class GithubOrgReaderProcessor implements CatalogProcessor { } const credentialsProvider = - DefaultGithubCredentialsProvider.create(gitHubConfig); + SingleInstanceGithubCredentialsProvider.create(gitHubConfig); const { headers, type: tokenType } = await credentialsProvider.getCredentials({ url: orgUrl, diff --git a/plugins/catalog-backend/src/ingestion/providers/GitHubOrgEntityProvider.test.ts b/plugins/catalog-backend/src/ingestion/providers/GitHubOrgEntityProvider.test.ts index 17c0b7054b..6fde975a2b 100644 --- a/plugins/catalog-backend/src/ingestion/providers/GitHubOrgEntityProvider.test.ts +++ b/plugins/catalog-backend/src/ingestion/providers/GitHubOrgEntityProvider.test.ts @@ -17,7 +17,7 @@ import { getVoidLogger } from '@backstage/backend-common'; import { GroupEntity, UserEntity } from '@backstage/catalog-model'; import { - DefaultGithubCredentialsProvider, + SingleInstanceGithubCredentialsProvider, GitHubIntegrationConfig, } from '@backstage/integration'; import { GitHubOrgEntityProvider } from '.'; @@ -93,9 +93,11 @@ describe('GitHubOrgEntityProvider', () => { type: 'app', }); - jest.spyOn(DefaultGithubCredentialsProvider, 'create').mockReturnValue({ - getCredentials: mockGetCredentials, - } as any); + jest + .spyOn(SingleInstanceGithubCredentialsProvider, 'create') + .mockReturnValue({ + getCredentials: mockGetCredentials, + } as any); const entityProvider = new GitHubOrgEntityProvider({ id: 'my-id', diff --git a/plugins/catalog-backend/src/ingestion/providers/GitHubOrgEntityProvider.ts b/plugins/catalog-backend/src/ingestion/providers/GitHubOrgEntityProvider.ts index 63d915d817..b8059a783c 100644 --- a/plugins/catalog-backend/src/ingestion/providers/GitHubOrgEntityProvider.ts +++ b/plugins/catalog-backend/src/ingestion/providers/GitHubOrgEntityProvider.ts @@ -20,7 +20,7 @@ import { } from '@backstage/catalog-model'; import { Config } from '@backstage/config'; import { - DefaultGithubCredentialsProvider, + SingleInstanceGithubCredentialsProvider, GithubCredentialsProvider, GitHubIntegrationConfig, ScmIntegrations, @@ -78,7 +78,7 @@ export class GitHubOrgEntityProvider implements EntityProvider { logger: Logger; }, ) { - this.credentialsProvider = DefaultGithubCredentialsProvider.create( + this.credentialsProvider = SingleInstanceGithubCredentialsProvider.create( options.gitHubConfig, ); } diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/OctokitProvider.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/OctokitProvider.ts index 4fbd45c0ee..9fdc6ced17 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/OctokitProvider.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/OctokitProvider.ts @@ -16,7 +16,7 @@ import { InputError } from '@backstage/errors'; import { - DefaultGithubCredentialsProvider, + SingleInstanceGithubCredentialsProvider, GithubCredentialsProvider, ScmIntegrationRegistry, } from '@backstage/integration'; @@ -41,7 +41,7 @@ export class OctokitProvider { this.integrations = integrations; this.credentialsProviders = new Map( integrations.github.list().map(integration => { - const provider = DefaultGithubCredentialsProvider.create( + const provider = SingleInstanceGithubCredentialsProvider.create( integration.config, ); return [integration.config.host, provider]; diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts index 79b393a16a..299eb07f2e 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts @@ -18,7 +18,7 @@ import fs from 'fs-extra'; import { parseRepoUrl, isExecutable } from './util'; import { - DefaultGithubCredentialsProvider, + SingleInstanceGithubCredentialsProvider, ScmIntegrationRegistry, } from '@backstage/integration'; import { zipObject } from 'lodash'; @@ -76,7 +76,7 @@ export const defaultClientFactory = async ({ } const credentialsProvider = - DefaultGithubCredentialsProvider.create(integrationConfig); + SingleInstanceGithubCredentialsProvider.create(integrationConfig); if (!credentialsProvider) { throw new InputError( From a92d65bcb2389e5c47e18fa2b3c55e6bdd61ae3b Mon Sep 17 00:00:00 2001 From: Brian Fletcher Date: Wed, 22 Dec 2021 15:54:35 +0000 Subject: [PATCH 4/5] create changeset file Signed-off-by: Brian Fletcher --- .changeset/five-cameras-hammer.md | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 .changeset/five-cameras-hammer.md diff --git a/.changeset/five-cameras-hammer.md b/.changeset/five-cameras-hammer.md new file mode 100644 index 0000000000..7e1cdda0c9 --- /dev/null +++ b/.changeset/five-cameras-hammer.md @@ -0,0 +1,10 @@ +--- +'@backstage/backend-common': patch +'@backstage/integration': patch +'@backstage/plugin-catalog-backend': patch +'@backstage/plugin-catalog': patch +'@backstage/plugin-scaffolder-backend': patch +'@backstage/plugin-scaffolder': patch +--- + +Create an interface for the GitHub credentials provider in order to support providing implementations. From 7d4b4e937c14bf300d5a94aa4eb5976c03b3e8bc Mon Sep 17 00:00:00 2001 From: Brian Fletcher Date: Thu, 23 Dec 2021 12:30:48 +0000 Subject: [PATCH 5/5] addressing review comments Signed-off-by: Brian Fletcher --- ...five-cameras-hammer.md => lucky-bats-reflect.md} | 7 +++---- .changeset/short-schools-heal.md | 13 +++++++++++++ packages/integration/api-report.md | 7 +------ ...SingleInstanceGithubCredentialsProvider.test.ts} | 0 .../SingleInstanceGithubCredentialsProvider.ts | 5 +++-- packages/integration/src/github/index.ts | 1 - packages/integration/src/github/types.ts | 12 ------------ 7 files changed, 20 insertions(+), 25 deletions(-) rename .changeset/{five-cameras-hammer.md => lucky-bats-reflect.md} (60%) create mode 100644 .changeset/short-schools-heal.md rename packages/integration/src/github/{DefaultGithubCredentialsProvider.test.ts => SingleInstanceGithubCredentialsProvider.test.ts} (100%) diff --git a/.changeset/five-cameras-hammer.md b/.changeset/lucky-bats-reflect.md similarity index 60% rename from .changeset/five-cameras-hammer.md rename to .changeset/lucky-bats-reflect.md index 7e1cdda0c9..ad3171382e 100644 --- a/.changeset/five-cameras-hammer.md +++ b/.changeset/lucky-bats-reflect.md @@ -1,10 +1,9 @@ --- '@backstage/backend-common': patch -'@backstage/integration': patch -'@backstage/plugin-catalog-backend': patch '@backstage/plugin-catalog': patch -'@backstage/plugin-scaffolder-backend': patch +'@backstage/plugin-catalog-backend': patch '@backstage/plugin-scaffolder': patch +'@backstage/plugin-scaffolder-backend': patch --- -Create an interface for the GitHub credentials provider in order to support providing implementations. +Uptake changes to the GitHub Credentials Provider interface. diff --git a/.changeset/short-schools-heal.md b/.changeset/short-schools-heal.md new file mode 100644 index 0000000000..3976ea3a0c --- /dev/null +++ b/.changeset/short-schools-heal.md @@ -0,0 +1,13 @@ +--- +'@backstage/integration': minor +--- + +Create an interface for the GitHub credentials provider in order to support providing implementations. + +We have changed the name of the `GithubCredentialsProvider` to `SingleInstanceGithubCredentialsProvider`. + +`GithubCredentialsProvider` is now an interface that maybe implemented to provide a custom mechanism to retrieve GitHub credentials. + +In a later release we will support configuring URL readers, scaffolder tasks, and processors with customer GitHub credentials providers. + +If you want to uptake this release, you will need to replace all references to `GithubCredentialsProvider.create` with `SingleInstanceGithubCredentialsProvider.create`. diff --git a/packages/integration/api-report.md b/packages/integration/api-report.md index ca24848db8..00623dc917 100644 --- a/packages/integration/api-report.md +++ b/packages/integration/api-report.md @@ -203,11 +203,6 @@ export interface GithubCredentialsProvider { getCredentials(opts: { url: string }): Promise; } -// @public -export type GithubCredentialsProviderFactory = ( - config: GitHubIntegrationConfig, -) => GithubCredentialsProvider; - // @public export type GithubCredentialType = 'app' | 'token'; @@ -432,7 +427,7 @@ export class SingleInstanceGithubCredentialsProvider implements GithubCredentialsProvider { // (undocumented) - static create: GithubCredentialsProviderFactory; + static create: (config: GitHubIntegrationConfig) => GithubCredentialsProvider; getCredentials(opts: { url: string }): Promise; } diff --git a/packages/integration/src/github/DefaultGithubCredentialsProvider.test.ts b/packages/integration/src/github/SingleInstanceGithubCredentialsProvider.test.ts similarity index 100% rename from packages/integration/src/github/DefaultGithubCredentialsProvider.test.ts rename to packages/integration/src/github/SingleInstanceGithubCredentialsProvider.test.ts diff --git a/packages/integration/src/github/SingleInstanceGithubCredentialsProvider.ts b/packages/integration/src/github/SingleInstanceGithubCredentialsProvider.ts index 5b6fa39e75..408c5b1348 100644 --- a/packages/integration/src/github/SingleInstanceGithubCredentialsProvider.ts +++ b/packages/integration/src/github/SingleInstanceGithubCredentialsProvider.ts @@ -22,7 +22,6 @@ import { DateTime } from 'luxon'; import { GithubCredentials, GithubCredentialsProvider, - GithubCredentialsProviderFactory, GithubCredentialType, } from './types'; @@ -232,7 +231,9 @@ export class GithubAppCredentialsMux { export class SingleInstanceGithubCredentialsProvider implements GithubCredentialsProvider { - static create: GithubCredentialsProviderFactory = config => { + static create: ( + config: GitHubIntegrationConfig, + ) => GithubCredentialsProvider = config => { return new SingleInstanceGithubCredentialsProvider( new GithubAppCredentialsMux(config), config.token, diff --git a/packages/integration/src/github/index.ts b/packages/integration/src/github/index.ts index fa8dfb4b86..429a2b1c76 100644 --- a/packages/integration/src/github/index.ts +++ b/packages/integration/src/github/index.ts @@ -27,7 +27,6 @@ export { export type { GithubCredentials, GithubCredentialsProvider, - GithubCredentialsProviderFactory, GithubCredentialType, } from './types'; export { GitHubIntegration, replaceGitHubUrlType } from './GitHubIntegration'; diff --git a/packages/integration/src/github/types.ts b/packages/integration/src/github/types.ts index 48609d46f0..15f5375570 100644 --- a/packages/integration/src/github/types.ts +++ b/packages/integration/src/github/types.ts @@ -14,8 +14,6 @@ * limitations under the License. */ -import { GitHubIntegrationConfig } from './config'; - /** * The type of credentials produced by the credential provider. * @@ -43,13 +41,3 @@ export type GithubCredentials = { export interface GithubCredentialsProvider { getCredentials(opts: { url: string }): Promise; } - -/** - * This allows implementations to provide factories to create credential providers - * - * @public - * - */ -export type GithubCredentialsProviderFactory = ( - config: GitHubIntegrationConfig, -) => GithubCredentialsProvider;