From 7228b18c0ffdde85aefb73786902156ce820a47b Mon Sep 17 00:00:00 2001 From: Brian Fletcher Date: Fri, 17 Dec 2021 11:25:01 +0000 Subject: [PATCH 01/44] 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 02/44] 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 03/44] 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 1d260170907fb0cef2a4fc07e143919036897b6e Mon Sep 17 00:00:00 2001 From: "jean-philippe.blary" Date: Tue, 21 Dec 2021 13:42:20 +0100 Subject: [PATCH 04/44] fix(cli): skip findPackages if not inside a monorepo Signed-off-by: jean-philippe.blary --- .changeset/light-singers-double.md | 5 +++++ packages/cli/src/lib/config.ts | 14 +++++++++++--- 2 files changed, 16 insertions(+), 3 deletions(-) create mode 100644 .changeset/light-singers-double.md diff --git a/.changeset/light-singers-double.md b/.changeset/light-singers-double.md new file mode 100644 index 0000000000..e21017373d --- /dev/null +++ b/.changeset/light-singers-double.md @@ -0,0 +1,5 @@ +--- +'@backstage/cli': patch +--- + +Fix issue with plugin:serve for Plugins not using Lerna monorepo. diff --git a/packages/cli/src/lib/config.ts b/packages/cli/src/lib/config.ts index f85db276b2..986e409e58 100644 --- a/packages/cli/src/lib/config.ts +++ b/packages/cli/src/lib/config.ts @@ -44,9 +44,17 @@ export async function loadCliConfig(options: Options) { const project = new Project(paths.targetDir); const packages = await project.getPackages(); - const localPackageNames = options.fromPackage - ? findPackages(packages, options.fromPackage) - : packages.map((p: any) => p.name); + let localPackageNames; + if (options.fromPackage) { + if (packages.length) { + localPackageNames = findPackages(packages, options.fromPackage); + } else { + // No packages: it means that it's not a monorepo (e.g. standalone plugin) + localPackageNames = [options.fromPackage]; + } + } else { + localPackageNames = packages.map((p: any) => p.name); + } const schema = await loadConfigSchema({ dependencies: localPackageNames, From 3bc9681263b7cf8b6d1d35c16b10da8dd34d12c6 Mon Sep 17 00:00:00 2001 From: Suzanne Daniels Date: Wed, 22 Dec 2021 10:07:32 +0100 Subject: [PATCH 05/44] Fixing image for PAT creation Signed-off-by: Suzanne Daniels --- docs/getting-started/configuration.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/getting-started/configuration.md b/docs/getting-started/configuration.md index bcb792e7aa..55285e7f4e 100644 --- a/docs/getting-started/configuration.md +++ b/docs/getting-started/configuration.md @@ -215,7 +215,7 @@ days for expiration. If you have a hard time picking a number, we suggest to go for 7 days, it's a lucky number.

- Screenshot of the GitHub OAuth creation page + Screenshot of the GitHub Personal Access Token creation page

Set the scope to your likings. For this tutorial, selecting "repo" should be From 58b0262dd9fa9a5d1749b335233c8be5d477abbd Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 22 Dec 2021 10:12:31 +0100 Subject: [PATCH 06/44] cli: removed unused ts-loader and dashify deps Signed-off-by: Patrik Oldsberg --- .changeset/green-ladybugs-fold.md | 5 +++++ packages/cli/package.json | 2 -- yarn.lock | 20 ++------------------ 3 files changed, 7 insertions(+), 20 deletions(-) create mode 100644 .changeset/green-ladybugs-fold.md diff --git a/.changeset/green-ladybugs-fold.md b/.changeset/green-ladybugs-fold.md new file mode 100644 index 0000000000..b7be6680df --- /dev/null +++ b/.changeset/green-ladybugs-fold.md @@ -0,0 +1,5 @@ +--- +'@backstage/cli': patch +--- + +Pruned unused dependencies. diff --git a/packages/cli/package.json b/packages/cli/package.json index 4b59d46827..5ded9d79c9 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -60,7 +60,6 @@ "chokidar": "^3.3.1", "commander": "^6.1.0", "css-loader": "^5.2.6", - "dashify": "^2.0.0", "diff": "^5.0.0", "esbuild": "^0.14.1", "eslint": "^7.30.0", @@ -105,7 +104,6 @@ "sucrase": "^3.20.2", "tar": "^6.1.2", "terser-webpack-plugin": "^5.1.3", - "ts-loader": "^8.0.17", "typescript": "^4.0.3", "util": "^0.12.3", "webpack": "^5.48.0", diff --git a/yarn.lock b/yarn.lock index 422531d3a4..4f2c39b211 100644 --- a/yarn.lock +++ b/yarn.lock @@ -12931,11 +12931,6 @@ dashdash@^1.12.0: dependencies: assert-plus "^1.0.0" -dashify@^2.0.0: - version "2.0.0" - resolved "https://registry.npmjs.org/dashify/-/dashify-2.0.0.tgz#fff270ca2868ca427fee571de35691d6e437a648" - integrity sha512-hpA5C/YrPjucXypHPPc0oJ1l9Hf6wWbiOL7Ik42cxnsUOhWiCB/fylKbKqqJalW9FgkNQCw16YO8uW9Hs0Iy1A== - data-urls@^2.0.0: version "2.0.0" resolved "https://registry.npmjs.org/data-urls/-/data-urls-2.0.0.tgz#156485a72963a970f5d5821aaf642bef2bf2db9b" @@ -13796,7 +13791,7 @@ endent@^2.0.1: fast-json-parse "^1.0.3" objectorarray "^1.0.4" -enhanced-resolve@^4.0.0, enhanced-resolve@^4.5.0: +enhanced-resolve@^4.5.0: version "4.5.0" resolved "https://registry.npmjs.org/enhanced-resolve/-/enhanced-resolve-4.5.0.tgz#2f3cfd84dbe3b487f18f2db2ef1e064a571ca5ec" integrity sha512-Nv9m36S/vxpsI+Hc4/ZGRs0n9mXqSWGGq49zxb/cJfPAQMbUtttJAlNPS4AQzaBdw/pKskw5bMbekT/Y7W/Wlg== @@ -20993,7 +20988,7 @@ micromatch@^3.1.10, micromatch@^3.1.4: snapdragon "^0.8.1" to-regex "^3.0.2" -micromatch@^4.0.0, micromatch@^4.0.2, micromatch@^4.0.4: +micromatch@^4.0.2, micromatch@^4.0.4: version "4.0.4" resolved "https://registry.npmjs.org/micromatch/-/micromatch-4.0.4.tgz#896d519dfe9db25fce94ceb7a500919bf881ebf9" integrity sha512-pRmzw/XUcwXGpD9aI9q/0XOwLNygjETJ8y0ao0wdqprrzDa4YnxLcz7fQRZr8voh8V10kGhABbNcHVk5wHgWwg== @@ -28112,17 +28107,6 @@ ts-invariant@^0.4.0: dependencies: tslib "^1.9.3" -ts-loader@^8.0.17: - version "8.0.17" - resolved "https://registry.npmjs.org/ts-loader/-/ts-loader-8.0.17.tgz#98f2ccff9130074f4079fd89b946b4c637b1f2fc" - integrity sha512-OeVfSshx6ot/TCxRwpBHQ/4lRzfgyTkvi7ghDVrLXOHzTbSK413ROgu/xNqM72i3AFeAIJgQy78FwSMKmOW68w== - dependencies: - chalk "^4.1.0" - enhanced-resolve "^4.0.0" - loader-utils "^2.0.0" - micromatch "^4.0.0" - semver "^7.3.4" - ts-log@^2.2.3: version "2.2.3" resolved "https://registry.npmjs.org/ts-log/-/ts-log-2.2.3.tgz#4da5640fe25a9fb52642cd32391c886721318efb" From 700dfbe67e05e400fbc492a3791cdb3757697e51 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 22 Dec 2021 10:29:11 +0100 Subject: [PATCH 07/44] cli: fix index templating deprecation warning Signed-off-by: Patrik Oldsberg --- packages/cli/src/lib/bundler/config.ts | 56 ++++++++++++++------------ 1 file changed, 31 insertions(+), 25 deletions(-) diff --git a/packages/cli/src/lib/bundler/config.ts b/packages/cli/src/lib/bundler/config.ts index 398f2ace60..7ac230bb0e 100644 --- a/packages/cli/src/lib/bundler/config.ts +++ b/packages/cli/src/lib/bundler/config.ts @@ -131,36 +131,42 @@ export async function createConfig( }), ); - const deprecatedAppConfig = { - title: frontendConfig.getString('app.title'), - baseUrl: validBaseUrl.href, - googleAnalyticsTrackingId: frontendConfig.getOptionalString( - 'app.googleAnalyticsTrackingId', - ), - datadogRum: { - env: frontendConfig.getOptionalString('app.datadogRum.env'), - clientToken: frontendConfig.getOptionalString( - 'app.datadogRum.clientToken', - ), - applicationId: frontendConfig.getOptionalString( - 'app.datadogRum.applicationId', - ), - site: frontendConfig.getOptionalString('app.datadogRum.site'), - }, - }; - + const appParamDeprecationMsg = chalk.red( + 'DEPRECATION WARNING: using `app.` in the index.html template is deprecated, use `config.getString("app.")` instead.', + ); plugins.push( new HtmlWebpackPlugin({ template: paths.targetHtml, templateParameters: { publicPath: validBaseUrl.pathname.replace(/\/$/, ''), - get app() { - console.warn( - chalk.red( - 'DEPRECATION WARNING: using `app.` in the index.html template is deprecated, use `config.getString("app.")` instead.', - ), - ); - return deprecatedAppConfig; + app: { + get title() { + console.warn(appParamDeprecationMsg); + return frontendConfig.getString('app.title'); + }, + get baseUrl() { + console.warn(appParamDeprecationMsg); + return validBaseUrl.href; + }, + get googleAnalyticsTrackingId() { + console.warn(appParamDeprecationMsg); + return frontendConfig.getOptionalString( + 'app.googleAnalyticsTrackingId', + ); + }, + get datadogRum() { + console.warn(appParamDeprecationMsg); + return { + env: frontendConfig.getOptionalString('app.datadogRum.env'), + clientToken: frontendConfig.getOptionalString( + 'app.datadogRum.clientToken', + ), + applicationId: frontendConfig.getOptionalString( + 'app.datadogRum.applicationId', + ), + site: frontendConfig.getOptionalString('app.datadogRum.site'), + }; + }, }, config: frontendConfig, }, From d3f10b6e1e814bc4bc0c0b8e6f9f75f58fe5d10e Mon Sep 17 00:00:00 2001 From: Niall McCullagh Date: Wed, 22 Dec 2021 14:27:05 +0000 Subject: [PATCH 08/44] fix api docs routing clash with /api /api-docs was getting routed to the backend /api route. The documentation at https://kubernetes.io/docs/concepts/services-networking/ingress/ describes that the path should match on the full element so this was unexpected. Adding a trailing slash is not supposed to have an impact but in this case it solved the problem. Signed-off-by: Niall McCullagh --- contrib/chart/backstage/Chart.yaml | 2 +- contrib/chart/backstage/templates/ingress.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/contrib/chart/backstage/Chart.yaml b/contrib/chart/backstage/Chart.yaml index 8aeab9be3e..9154368475 100644 --- a/contrib/chart/backstage/Chart.yaml +++ b/contrib/chart/backstage/Chart.yaml @@ -5,7 +5,7 @@ type: application # This is the chart version. This version number should be incremented each time you make changes # to the chart and its templates, including the app version. -version: 0.1.1 +version: 0.1.2 # This is the version number of the application being deployed. This version number should be # incremented each time you make changes to the application. diff --git a/contrib/chart/backstage/templates/ingress.yaml b/contrib/chart/backstage/templates/ingress.yaml index 9340467800..3ec4990561 100644 --- a/contrib/chart/backstage/templates/ingress.yaml +++ b/contrib/chart/backstage/templates/ingress.yaml @@ -40,7 +40,7 @@ spec: servicePort: 80 {{/* Route the backend inside the same hostname as the frontend when they are the same */}} {{- if eq $frontendUrl.host $backendUrl.host}} - - path: /api + - path: /api/ backend: serviceName: {{ include "backend.serviceName" . }} servicePort: 80 From b29bf02ab66fe9ca8b8ec4ba7c2d14c7fe1749d7 Mon Sep 17 00:00:00 2001 From: Karan Shah Date: Wed, 22 Dec 2021 15:25:22 +0000 Subject: [PATCH 09/44] Add Simply Business as adopters Signed-off-by: Karan Shah --- ADOPTERS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/ADOPTERS.md b/ADOPTERS.md index 43e7fba68b..8e2ac92aaf 100644 --- a/ADOPTERS.md +++ b/ADOPTERS.md @@ -75,3 +75,4 @@ | [Telstra](https://www.telstra.com.au) | [@kiranpatel11](https://github.com/kiranpatel11), [JasonC](https://github.com/JasonC17) | Primary usage: software catalog and templates
Emerging usage : TechDocs, Explore Ecosystem, TechRadar, etc | | [Mosaico](https://www.mosaico.com.br/) | [Wédney Yuri](https://github.com/wedneyyuri),[@tino.milton](https://github.com/miltonjacomini) | A centralized service catalog of our documentation for our service engineers. | | [Mox Bank](https://www.mox.com/) | [Nick Laqua](https://github.com/nick-laqua-dragon), [Gauthier Roebroeck](https://github.com/gauthier-roebroeck-mox) | "Single pane of glass" developer portal for providing a best-in-class developer experience to our product teams and making Mox the best tech environment in Hongkong 🥰🚀 | +| [Simply Business](https://sbtech.simplybusiness.co.uk/) | [@addersuk](https://github.com/addersuk), [@LightningStairs](https://github.com/LightningStairs), [@punitcse](https://github.com/punitcse), [@moltenice](https://github.com/moltenice) | Central developer portal to access everything a developer needs such as docs, internal service catalog, and the ability to quickly create a new service from a template. Internally developed Backstage plugins allow us to customise the experience to how we work. | From 12c66ba076876584b5592dd2f102826f91e65836 Mon Sep 17 00:00:00 2001 From: Karan Shah Date: Wed, 22 Dec 2021 15:25:22 +0000 Subject: [PATCH 10/44] Add Simply Business as adopters Signed-off-by: Karan Shah --- ADOPTERS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/ADOPTERS.md b/ADOPTERS.md index 500dbb7559..18337c414c 100644 --- a/ADOPTERS.md +++ b/ADOPTERS.md @@ -76,3 +76,4 @@ | [Mosaico](https://www.mosaico.com.br/) | [Wédney Yuri](https://github.com/wedneyyuri),[@tino.milton](https://github.com/miltonjacomini) | A centralized service catalog of our documentation for our service engineers. | | [Mox Bank](https://www.mox.com/) | [Nick Laqua](https://github.com/nick-laqua-dragon), [Gauthier Roebroeck](https://github.com/gauthier-roebroeck-mox) | "Single pane of glass" developer portal for providing a best-in-class developer experience to our product teams and making Mox the best tech environment in Hongkong 🥰🚀 | | [Keyloop](https://www.keyloop.com/) | [Andre Wanlin](https://github.com/awanlin) | Future-motive Developer Portal to help our teams create technology to make everything about buying and owning a car better. 🚗 | +| [Simply Business](https://sbtech.simplybusiness.co.uk/) | [@addersuk](https://github.com/addersuk), [@LightningStairs](https://github.com/LightningStairs), [@punitcse](https://github.com/punitcse), [@moltenice](https://github.com/moltenice) | Central developer portal to access everything a developer needs such as docs, internal service catalog, and the ability to quickly create a new service from a template. Internally developed Backstage plugins allow us to customise the experience to how we work. | From a92d65bcb2389e5c47e18fa2b3c55e6bdd61ae3b Mon Sep 17 00:00:00 2001 From: Brian Fletcher Date: Wed, 22 Dec 2021 15:54:35 +0000 Subject: [PATCH 11/44] 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 27af6d996bcf2ab47a0492e6c6db779aabc7b93d Mon Sep 17 00:00:00 2001 From: blam Date: Wed, 22 Dec 2021 17:47:06 +0100 Subject: [PATCH 12/44] chore: lock `rc-progress` to a working version Signed-off-by: blam --- .changeset/gorgeous-hats-fold.md | 6 ++++++ packages/core-components/package.json | 2 +- plugins/sonarqube/package.json | 2 +- yarn.lock | 2 +- 4 files changed, 9 insertions(+), 3 deletions(-) create mode 100644 .changeset/gorgeous-hats-fold.md diff --git a/.changeset/gorgeous-hats-fold.md b/.changeset/gorgeous-hats-fold.md new file mode 100644 index 0000000000..4adaf50918 --- /dev/null +++ b/.changeset/gorgeous-hats-fold.md @@ -0,0 +1,6 @@ +--- +'@backstage/core-components': patch +'@backstage/plugin-sonarqube': patch +--- + +Locking `rc-progress` to the working version of 3.1.4 diff --git a/packages/core-components/package.json b/packages/core-components/package.json index b47a221227..fb91f9b3ce 100644 --- a/packages/core-components/package.json +++ b/packages/core-components/package.json @@ -51,7 +51,7 @@ "pluralize": "^8.0.0", "prop-types": "^15.7.2", "qs": "^6.9.4", - "rc-progress": "^3.0.0", + "rc-progress": "3.1.4", "react-helmet": "6.1.0", "react-hook-form": "^7.12.2", "react-markdown": "^7.0.1", diff --git a/plugins/sonarqube/package.json b/plugins/sonarqube/package.json index d1daf47b67..14980e796c 100644 --- a/plugins/sonarqube/package.json +++ b/plugins/sonarqube/package.json @@ -43,7 +43,7 @@ "@material-ui/lab": "4.0.0-alpha.57", "@material-ui/styles": "^4.10.0", "cross-fetch": "^3.0.6", - "rc-progress": "^3.0.0", + "rc-progress": "3.1.4", "react-use": "^17.2.4" }, "peerDependencies": { diff --git a/yarn.lock b/yarn.lock index 09e8eca9f1..c452457d06 100644 --- a/yarn.lock +++ b/yarn.lock @@ -24290,7 +24290,7 @@ raw-loader@^4.0.2: loader-utils "^2.0.0" schema-utils "^3.0.0" -rc-progress@^3.0.0: +rc-progress@3.1.4, rc-progress@^3.0.0: version "3.1.4" resolved "https://registry.npmjs.org/rc-progress/-/rc-progress-3.1.4.tgz#66040d0fae7d8ced2b38588378eccb2864bad615" integrity sha512-XBAif08eunHssGeIdxMXOmRQRULdHaDdIFENQ578CMb4dyewahmmfJRyab+hw4KH4XssEzzYOkAInTLS7JJG+Q== From 2462b9e27509e9e637aa5198f77a099ed44f25e4 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 22 Dec 2021 19:25:28 +0100 Subject: [PATCH 13/44] backend-common: make sure readTree temp dirs are cleaned up Signed-off-by: Patrik Oldsberg --- .changeset/friendly-seals-peel.md | 5 +++ .../reading/tree/TarArchiveResponse.test.ts | 36 +++++++++++++++++++ .../src/reading/tree/TarArchiveResponse.ts | 23 +++++++++++- 3 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 .changeset/friendly-seals-peel.md diff --git a/.changeset/friendly-seals-peel.md b/.changeset/friendly-seals-peel.md new file mode 100644 index 0000000000..a340d46f2b --- /dev/null +++ b/.changeset/friendly-seals-peel.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-common': patch +--- + +Ensure temporary directories are cleaned up if an error is thrown in the `filter` callback of the `UrlReader.readTree` options. diff --git a/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts b/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts index aa904c5522..f0cbc6a612 100644 --- a/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts +++ b/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts @@ -147,4 +147,40 @@ describe('TarArchiveResponse', () => { fs.pathExists(resolvePath(dir, 'docs/index.md')), ).resolves.toBe(false); }); + + it('should leave temporary directories in place in the case of an error', async () => { + const stream = fs.createReadStream('/test-archive.tar.gz'); + + const res = new TarArchiveResponse(stream, '', '/tmp', 'etag', () => { + throw new Error('NOPE'); + }); + + const tmpDir = await fs.mkdtemp('/tmp/test'); + // selects the wrong overload by default + const mkdtemp = jest.spyOn(fs, 'mkdtemp') as unknown as jest.SpyInstance< + Promise, + [] + >; + mkdtemp.mockResolvedValue(tmpDir); + + await expect(fs.pathExists(tmpDir)).resolves.toBe(true); + await expect(res.dir()).rejects.toThrow('NOPE'); + await expect(fs.pathExists(tmpDir)).resolves.toBe(false); + + mkdtemp.mockRestore(); + }); + + it('should leave directory in place if provided in the case of an error', async () => { + const stream = fs.createReadStream('/test-archive.tar.gz'); + + const res = new TarArchiveResponse(stream, '', '/tmp', 'etag', () => { + throw new Error('NOPE'); + }); + + const tmpDir = await fs.mkdtemp('/tmp/test'); + + await expect(fs.pathExists(tmpDir)).resolves.toBe(true); + await expect(res.dir({ targetDir: tmpDir })).rejects.toThrow('NOPE'); + await expect(fs.pathExists(tmpDir)).resolves.toBe(true); + }); }); diff --git a/packages/backend-common/src/reading/tree/TarArchiveResponse.ts b/packages/backend-common/src/reading/tree/TarArchiveResponse.ts index f7ab7f5349..493e283c5f 100644 --- a/packages/backend-common/src/reading/tree/TarArchiveResponse.ts +++ b/packages/backend-common/src/reading/tree/TarArchiveResponse.ts @@ -150,12 +150,19 @@ export class TarArchiveResponse implements ReadTreeResponse { // When no subPath is given, remove just 1 top level directory const strip = this.subPath ? this.subPath.split('/').length : 1; + let filterError: Error | undefined = undefined; + await pipeline( this.stream, tar.extract({ strip, cwd: dir, filter: (path, stat) => { + // Filter errors will short-circuit the rest of the filtering and then throw + if (filterError) { + return false; + } + // File path relative to the root extracted directory. Will remove the // top level dir name from the path since its name is hard to predetermine. const relativePath = stripFirstDirectoryFromPath(path); @@ -164,13 +171,27 @@ export class TarArchiveResponse implements ReadTreeResponse { } if (this.filter) { const innerPath = path.split('/').slice(strip).join('/'); - return this.filter(innerPath, { size: stat.size }); + try { + return this.filter(innerPath, { size: stat.size }); + } catch (error) { + filterError = error; + return false; + } } return true; }, }), ); + if (filterError) { + // If the dir was provided we don't want to remove it, but if it wasn't it means + // we created a temporary directory and we should remove it. + if (!options?.targetDir) { + await fs.remove(dir).catch(() => {}); + } + throw filterError; + } + return dir; } } From 457cfb9e2304e39ba54f8773791b860164c2cc5d Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 17 Dec 2021 04:17:29 +0000 Subject: [PATCH 14/44] build(deps): bump @roadiehq/backstage-plugin-github-insights Bumps [@roadiehq/backstage-plugin-github-insights](https://github.com/RoadieHQ/roadie-backstage-plugins/tree/HEAD/plugins/frontend/backstage-plugin-github-insights) from 1.2.2 to 1.3.3. - [Release notes](https://github.com/RoadieHQ/roadie-backstage-plugins/releases) - [Changelog](https://github.com/RoadieHQ/roadie-backstage-plugins/blob/main/plugins/frontend/backstage-plugin-github-insights/CHANGELOG.md) - [Commits](https://github.com/RoadieHQ/roadie-backstage-plugins/commits/HEAD/plugins/frontend/backstage-plugin-github-insights) --- updated-dependencies: - dependency-name: "@roadiehq/backstage-plugin-github-insights" dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- packages/app/package.json | 2 +- yarn.lock | 20 ++++++++++---------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/app/package.json b/packages/app/package.json index 1f609858f6..0aef9e7344 100644 --- a/packages/app/package.json +++ b/packages/app/package.json @@ -52,7 +52,7 @@ "@material-ui/icons": "^4.9.1", "@material-ui/lab": "4.0.0-alpha.57", "@octokit/rest": "^18.5.3", - "@roadiehq/backstage-plugin-github-insights": "^1.1.23", + "@roadiehq/backstage-plugin-github-insights": "^1.2.2", "@roadiehq/backstage-plugin-github-pull-requests": "^1.0.13", "@roadiehq/backstage-plugin-travis-ci": "^1.0.11", "history": "^5.0.0", diff --git a/yarn.lock b/yarn.lock index c452457d06..fe8a92d1a5 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2381,7 +2381,7 @@ remark-gfm "^1.0.0" zen-observable "^0.8.15" -"@backstage/core-components@^0.7.0", "@backstage/core-components@^0.7.6": +"@backstage/core-components@^0.7.6": version "0.7.6" resolved "https://registry.npmjs.org/@backstage/core-components/-/core-components-0.7.6.tgz#2d29480b13c607c8ea8a2821a06326bd05ca3db8" integrity sha512-aB0ndQDxjNW8Tkzs0rhABd8qF2wLC9f0PWKT3RMh5Tovmg6Y/OOhj+YDzlsPrYJFLwfIwFDC4iA+H1WdkFe7tg== @@ -5511,17 +5511,17 @@ resolved "https://registry.npmjs.org/@rjsf/material-ui/-/material-ui-3.2.1.tgz#84fbf322485aee3a84101e189161f0687779ec8d" integrity sha512-8UiDeDbjCImFSfOegGu13otQ7OdP9FOYpcLjeouppnhs+MPeIEAtYS+jCcBKmi3reyTagC15/KVSRhde1wS1vg== -"@roadiehq/backstage-plugin-github-insights@^1.1.23": - version "1.2.2" - resolved "https://registry.npmjs.org/@roadiehq/backstage-plugin-github-insights/-/backstage-plugin-github-insights-1.2.2.tgz#09d958ed15adbb598afda34187f45deedec2264d" - integrity sha512-ybomCU/3NIQJYahp/E78wl+JuzpwkRkhyeCcySLA45GaCz/mEqAGSs4xDZancpV2ls6nxV3mKUw/DL9afmMGAw== +"@roadiehq/backstage-plugin-github-insights@^1.2.2": + version "1.3.4" + resolved "https://registry.npmjs.org/@roadiehq/backstage-plugin-github-insights/-/backstage-plugin-github-insights-1.3.4.tgz#dc7b8e50186f52c704964c06a77604b512e83444" + integrity sha512-wVri2Z5sKgyVQDt68WAi3H2IdkAzRljMLnuwYu/CWnCQkprcIJlIknzZK5aNjtMnNhKZDY+mHMgEPydl85Tjxw== dependencies: - "@backstage/catalog-model" "^0.9.0" - "@backstage/core-app-api" "^0.1.3" - "@backstage/core-components" "^0.7.0" - "@backstage/core-plugin-api" "^0.1.3" + "@backstage/catalog-model" "^0.9.7" + "@backstage/core-app-api" "^0.2.0" + "@backstage/core-components" "^0.8.0" + "@backstage/core-plugin-api" "^0.3.0" "@backstage/integration-react" "^0.1.10" - "@backstage/plugin-catalog-react" "^0.6.0" + "@backstage/plugin-catalog-react" "^0.6.5" "@backstage/theme" "^0.2.7" "@date-io/core" "2.10.7" "@material-ui/core" "^4.11.0" From a86f5c17016632908296e6a8d7498350b3705c01 Mon Sep 17 00:00:00 2001 From: Erik Larsson Date: Wed, 22 Dec 2021 22:48:07 +0100 Subject: [PATCH 15/44] fixes api auth bug in tech-insights plugin Signed-off-by: Erik Larsson --- .changeset/fluffy-toys-tease.md | 5 +++++ .../tech-insights/src/api/TechInsightsClient.ts | 16 ++++++++++++++-- 2 files changed, 19 insertions(+), 2 deletions(-) create mode 100644 .changeset/fluffy-toys-tease.md diff --git a/.changeset/fluffy-toys-tease.md b/.changeset/fluffy-toys-tease.md new file mode 100644 index 0000000000..f20eaebb2c --- /dev/null +++ b/.changeset/fluffy-toys-tease.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-tech-insights': minor +--- + +fixes api auth in tech-insights plugin diff --git a/plugins/tech-insights/src/api/TechInsightsClient.ts b/plugins/tech-insights/src/api/TechInsightsClient.ts index 25002e30a1..43979e60b0 100644 --- a/plugins/tech-insights/src/api/TechInsightsClient.ts +++ b/plugins/tech-insights/src/api/TechInsightsClient.ts @@ -17,7 +17,7 @@ import { TechInsightsApi } from './TechInsightsApi'; import { CheckResult } from '@backstage/plugin-tech-insights-common'; import { Check } from './types'; -import { DiscoveryApi } from '@backstage/core-plugin-api'; +import { DiscoveryApi, IdentityApi } from '@backstage/core-plugin-api'; import { ResponseError } from '@backstage/errors'; import { EntityName } from '@backstage/catalog-model'; @@ -28,13 +28,16 @@ import { export type Options = { discoveryApi: DiscoveryApi; + identityApi: IdentityApi; }; export class TechInsightsClient implements TechInsightsApi { private readonly discoveryApi: DiscoveryApi; + private readonly identityApi: IdentityApi; constructor(options: Options) { this.discoveryApi = options.discoveryApi; + this.identityApi = options.identityApi; } getScorecardsDefinition( @@ -47,7 +50,14 @@ export class TechInsightsClient implements TechInsightsApi { async getAllChecks(): Promise { const url = await this.discoveryApi.getBaseUrl('tech-insights'); - const response = await fetch(`${url}/checks`); + const token = await this.identityApi.getIdToken(); + const response = await fetch(`${url}/checks`, { + headers: token + ? { + Authorization: `Bearer ${token}`, + } + : undefined, + }); if (!response.ok) { throw await ResponseError.fromResponse(response); } @@ -59,6 +69,7 @@ export class TechInsightsClient implements TechInsightsApi { checks: Check[], ): Promise { const url = await this.discoveryApi.getBaseUrl('tech-insights'); + const token = await this.identityApi.getIdToken(); const { namespace, kind, name } = entityParams; const allChecks = checks ? checks : await this.getAllChecks(); const checkIds = allChecks.map((check: Check) => check.id); @@ -71,6 +82,7 @@ export class TechInsightsClient implements TechInsightsApi { body: JSON.stringify({ checks: checkIds }), headers: { 'Content-Type': 'application/json', + ...(token && { Authorization: `Bearer ${token}` }), }, }, ); From da5751717e6daf9d0c9443850cbb647b49a55119 Mon Sep 17 00:00:00 2001 From: Erik Larsson Date: Wed, 22 Dec 2021 23:05:09 +0100 Subject: [PATCH 16/44] tsc Signed-off-by: Erik Larsson --- plugins/tech-insights/src/plugin.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/plugins/tech-insights/src/plugin.ts b/plugins/tech-insights/src/plugin.ts index f229b654f3..b4382d8361 100644 --- a/plugins/tech-insights/src/plugin.ts +++ b/plugins/tech-insights/src/plugin.ts @@ -31,8 +31,9 @@ export const techInsightsPlugin = createPlugin({ apis: [ createApiFactory({ api: techInsightsApiRef, - deps: { discoveryApi: discoveryApiRef }, - factory: ({ discoveryApi }) => new TechInsightsClient({ discoveryApi }), + deps: { discoveryApi: discoveryApiRef, identityApi: identityApiRef }, + factory: ({ discoveryApi, identityApi }) => + new TechInsightsClient({ discoveryApi, identityApi }), }), ], routes: { From 57a54eaed4ca87cfe7447ef9a07548290e9cbaca Mon Sep 17 00:00:00 2001 From: Erik Larsson Date: Wed, 22 Dec 2021 23:09:34 +0100 Subject: [PATCH 17/44] tsc Signed-off-by: Erik Larsson --- plugins/tech-insights/src/plugin.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/tech-insights/src/plugin.ts b/plugins/tech-insights/src/plugin.ts index b4382d8361..0c2da63670 100644 --- a/plugins/tech-insights/src/plugin.ts +++ b/plugins/tech-insights/src/plugin.ts @@ -18,6 +18,7 @@ import { createRoutableExtension, createApiFactory, discoveryApiRef, + identityApiRef, } from '@backstage/core-plugin-api'; import { rootRouteRef } from './routes'; import { techInsightsApiRef } from './api/TechInsightsApi'; From 489d491b8c9126dffc735d742402d46f5c4c7ac8 Mon Sep 17 00:00:00 2001 From: mufaddal motiwala Date: Thu, 23 Dec 2021 13:01:11 +0530 Subject: [PATCH 18/44] remove css from index.html Signed-off-by: mufaddal motiwala --- packages/app/public/index.html | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/packages/app/public/index.html b/packages/app/public/index.html index 8273576b01..885fb6c228 100644 --- a/packages/app/public/index.html +++ b/packages/app/public/index.html @@ -42,11 +42,6 @@ href="<%= publicPath %>/safari-pinned-tab.svg" color="#5bbad5" /> - <%= config.getString('app.title') %> <% if (config.has('app.googleAnalyticsTrackingId')) { %> @@ -103,7 +98,7 @@ <% } %> - +