From 808153c0f78828ead4a09622f89b15407510155e Mon Sep 17 00:00:00 2001 From: ebarrios Date: Fri, 12 Feb 2021 16:18:30 +0100 Subject: [PATCH 01/39] Added GithubApp authentication to the scaffolder plugin --- packages/integration/src/github/config.ts | 7 ++++ .../src/scaffolder/stages/publish/github.ts | 38 +++++++++++++++++-- 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/packages/integration/src/github/config.ts b/packages/integration/src/github/config.ts index 94ed00731e..4b80a9aa60 100644 --- a/packages/integration/src/github/config.ts +++ b/packages/integration/src/github/config.ts @@ -59,6 +59,13 @@ export type GitHubIntegrationConfig = { */ token?: string; + /** + * The accessType (oAuth|githubApp) to use for requests to this provider. + * + * If no user is specified, oAuth access is used. + */ + accessType?: string; + /** * The GitHub Apps configuration to use for requests to this provider. * diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts b/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts index 13aab7ff34..667764dc1e 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts @@ -17,6 +17,7 @@ import { PublisherBase, PublisherOptions, PublisherResult } from './types'; import { initRepoAndPush } from './helpers'; import { GitHubIntegrationConfig } from '@backstage/integration'; +import { createAppAuth } from '@octokit/auth-app'; import parseGitUrl from 'git-url-parse'; import { Octokit } from '@octokit/rest'; import path from 'path'; @@ -28,8 +29,23 @@ export class GithubPublisher implements PublisherBase { config: GitHubIntegrationConfig, { repoVisibility }: { repoVisibility: RepoVisibilityOptions }, ) { + config.accessType = 'oAuth'; + if (!config.token) { - return undefined; + if (!config.apps) { + return undefined + } + const auth = createAppAuth({ + appId: config.apps[0].appId, + privateKey: config.apps[0].privateKey, + installationId: process.env.GITHUB_INSTALLATION_ID, + clientId: config.apps[0].clientId, + clientSecret: config.apps[0].clientSecret, + }); + const appAuthentication = await auth({ type: 'installation' }); + + config.token = appAuthentication.token; + config.accessType = 'githubApp' } const githubClient = new Octokit({ @@ -39,6 +55,7 @@ export class GithubPublisher implements PublisherBase { return new GithubPublisher({ token: config.token, + accessType: config.accessType, client: githubClient, repoVisibility, }); @@ -47,6 +64,7 @@ export class GithubPublisher implements PublisherBase { constructor( private readonly config: { token: string; + accessType: string; client: Octokit; repoVisibility: RepoVisibilityOptions; }, @@ -68,15 +86,27 @@ export class GithubPublisher implements PublisherBase { owner, }); - await initRepoAndPush({ + if (this.config.accessType === 'githubApp') { + await initRepoAndPush({ dir: path.join(workspacePath, 'result'), remoteUrl, auth: { - username: this.config.token, - password: 'x-oauth-basic', + username: 'x-access-token', + password: this.config.token, }, logger, }); + } else if (this.config.accessType === 'oAuth'){ + await initRepoAndPush({ + dir: path.join(workspacePath, 'result'), + remoteUrl, + auth: { + username: this.config.token, + password: 'x-oauth-basic', + }, + logger, + }); + } const catalogInfoUrl = remoteUrl.replace( /\.git$/, From 7a4d468d974e023e19588bdccb7317a92c842a44 Mon Sep 17 00:00:00 2001 From: ebarrios Date: Mon, 15 Feb 2021 11:53:38 +0100 Subject: [PATCH 02/39] Used GithubCredentialsProvider as requested by benjdlambert in the pr comments --- .../src/scaffolder/stages/publish/github.ts | 69 ++++++++++--------- 1 file changed, 37 insertions(+), 32 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts b/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts index 667764dc1e..4dd872ad8f 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts @@ -16,8 +16,10 @@ import { PublisherBase, PublisherOptions, PublisherResult } from './types'; import { initRepoAndPush } from './helpers'; -import { GitHubIntegrationConfig } from '@backstage/integration'; -import { createAppAuth } from '@octokit/auth-app'; +import { + GitHubIntegrationConfig, + GithubCredentialsProvider, +} from '@backstage/integration'; import parseGitUrl from 'git-url-parse'; import { Octokit } from '@octokit/rest'; import path from 'path'; @@ -31,21 +33,13 @@ export class GithubPublisher implements PublisherBase { ) { config.accessType = 'oAuth'; + const credentialsProvider = GithubCredentialsProvider.create(config); + if (!config.token) { if (!config.apps) { - return undefined + return undefined; } - const auth = createAppAuth({ - appId: config.apps[0].appId, - privateKey: config.apps[0].privateKey, - installationId: process.env.GITHUB_INSTALLATION_ID, - clientId: config.apps[0].clientId, - clientSecret: config.apps[0].clientSecret, - }); - const appAuthentication = await auth({ type: 'installation' }); - - config.token = appAuthentication.token; - config.accessType = 'githubApp' + config.accessType = 'githubApp'; } const githubClient = new Octokit({ @@ -54,10 +48,12 @@ export class GithubPublisher implements PublisherBase { }); return new GithubPublisher({ - token: config.token, + token: config.token || '', accessType: config.accessType, + credentialsProvider, client: githubClient, repoVisibility, + apiBaseUrl: config.apiBaseUrl, }); } @@ -65,8 +61,10 @@ export class GithubPublisher implements PublisherBase { private readonly config: { token: string; accessType: string; + credentialsProvider: GithubCredentialsProvider; client: Octokit; repoVisibility: RepoVisibilityOptions; + apiBaseUrl: string | undefined; }, ) {} @@ -77,6 +75,28 @@ export class GithubPublisher implements PublisherBase { }: PublisherOptions): Promise { const { owner, name } = parseGitUrl(values.storePath); + let auth = { + username: this.config.token, + password: 'x-oauth-basic', + }; + + if (this.config.accessType === 'githubApp') { + this.config.token = + ( + await this.config.credentialsProvider.getCredentials({ + url: values.storePath, + }) + ).token || ''; + this.config.client = new Octokit({ + auth: this.config.token, + baseUrl: this.config.apiBaseUrl, + }); + auth = { + username: 'x-access-token', + password: this.config.token, + }; + } + const description = values.description as string; const access = values.access as string; const remoteUrl = await this.createRemote({ @@ -86,27 +106,12 @@ export class GithubPublisher implements PublisherBase { owner, }); - if (this.config.accessType === 'githubApp') { - await initRepoAndPush({ + await initRepoAndPush({ dir: path.join(workspacePath, 'result'), remoteUrl, - auth: { - username: 'x-access-token', - password: this.config.token, - }, + auth, logger, }); - } else if (this.config.accessType === 'oAuth'){ - await initRepoAndPush({ - dir: path.join(workspacePath, 'result'), - remoteUrl, - auth: { - username: this.config.token, - password: 'x-oauth-basic', - }, - logger, - }); - } const catalogInfoUrl = remoteUrl.replace( /\.git$/, From 18ed99c0e5f6d376f780d8098ead15ec98837242 Mon Sep 17 00:00:00 2001 From: ebarrios Date: Mon, 15 Feb 2021 14:45:40 +0100 Subject: [PATCH 03/39] Used credentialsProvider as to determine which kind of authentication is going to be used instead of adding an accessType top the GitHubIntegrationConfig --- packages/integration/package.json | 1 + packages/integration/src/github/config.ts | 7 ++++--- .../src/scaffolder/stages/publish/github.ts | 12 ++++-------- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/packages/integration/package.json b/packages/integration/package.json index b4134ab00e..254b4d3fe5 100644 --- a/packages/integration/package.json +++ b/packages/integration/package.json @@ -30,6 +30,7 @@ }, "dependencies": { "@backstage/config": "^0.1.2", + "@backstage/integration": "^0.4.0", "cross-fetch": "^3.0.6", "git-url-parse": "^11.4.4", "@octokit/rest": "^18.0.12", diff --git a/packages/integration/src/github/config.ts b/packages/integration/src/github/config.ts index 4b80a9aa60..2a998ef18b 100644 --- a/packages/integration/src/github/config.ts +++ b/packages/integration/src/github/config.ts @@ -16,6 +16,7 @@ import { Config } from '@backstage/config'; import { isValidHost } from '../helpers'; +import { GithubCredentialsProvider } from '@backstage/integration'; const GITHUB_HOST = 'github.com'; const GITHUB_API_BASE_URL = 'https://api.github.com'; @@ -60,11 +61,11 @@ export type GitHubIntegrationConfig = { token?: string; /** - * The accessType (oAuth|githubApp) to use for requests to this provider. + * The credentialsProvider to use for requests to this provider. * - * If no user is specified, oAuth access is used. + * If no credentialsProvider is created, undefined is used. */ - accessType?: string; + credentialsProvider?: GithubCredentialsProvider | undefined; /** * The GitHub Apps configuration to use for requests to this provider. diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts b/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts index 4dd872ad8f..4d038ed5b9 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts @@ -31,15 +31,13 @@ export class GithubPublisher implements PublisherBase { config: GitHubIntegrationConfig, { repoVisibility }: { repoVisibility: RepoVisibilityOptions }, ) { - config.accessType = 'oAuth'; - - const credentialsProvider = GithubCredentialsProvider.create(config); + let credentialsProvider: GithubCredentialsProvider | undefined = undefined; if (!config.token) { if (!config.apps) { return undefined; } - config.accessType = 'githubApp'; + credentialsProvider = GithubCredentialsProvider.create(config); } const githubClient = new Octokit({ @@ -49,7 +47,6 @@ export class GithubPublisher implements PublisherBase { return new GithubPublisher({ token: config.token || '', - accessType: config.accessType, credentialsProvider, client: githubClient, repoVisibility, @@ -60,8 +57,7 @@ export class GithubPublisher implements PublisherBase { constructor( private readonly config: { token: string; - accessType: string; - credentialsProvider: GithubCredentialsProvider; + credentialsProvider: GithubCredentialsProvider | undefined; client: Octokit; repoVisibility: RepoVisibilityOptions; apiBaseUrl: string | undefined; @@ -80,7 +76,7 @@ export class GithubPublisher implements PublisherBase { password: 'x-oauth-basic', }; - if (this.config.accessType === 'githubApp') { + if (this.config.credentialsProvider) { this.config.token = ( await this.config.credentialsProvider.getCredentials({ From 09e8baf31b617c20ee1d85ff5e1887762fa96334 Mon Sep 17 00:00:00 2001 From: ebarrios Date: Mon, 15 Feb 2021 14:57:58 +0100 Subject: [PATCH 04/39] Removed the githubCredentials from the github config because it is not needed there --- packages/integration/src/github/config.ts | 8 -------- 1 file changed, 8 deletions(-) diff --git a/packages/integration/src/github/config.ts b/packages/integration/src/github/config.ts index 2a998ef18b..94ed00731e 100644 --- a/packages/integration/src/github/config.ts +++ b/packages/integration/src/github/config.ts @@ -16,7 +16,6 @@ import { Config } from '@backstage/config'; import { isValidHost } from '../helpers'; -import { GithubCredentialsProvider } from '@backstage/integration'; const GITHUB_HOST = 'github.com'; const GITHUB_API_BASE_URL = 'https://api.github.com'; @@ -60,13 +59,6 @@ export type GitHubIntegrationConfig = { */ token?: string; - /** - * The credentialsProvider to use for requests to this provider. - * - * If no credentialsProvider is created, undefined is used. - */ - credentialsProvider?: GithubCredentialsProvider | undefined; - /** * The GitHub Apps configuration to use for requests to this provider. * From c2dfd1cf0135ddef22a1dc647a0b7e75cc6e133d Mon Sep 17 00:00:00 2001 From: Taras Date: Mon, 15 Feb 2021 12:18:25 -0500 Subject: [PATCH 05/39] Added fail test for error reporting when accountKey is missing --- .../stages/publish/azureBlobStorage.test.ts | 95 +++++++++++++++---- 1 file changed, 76 insertions(+), 19 deletions(-) diff --git a/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts b/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts index 80f3cb0cf7..18724a84f9 100644 --- a/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts +++ b/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts @@ -19,6 +19,7 @@ import { getVoidLogger } from '@backstage/backend-common'; import { AzureBlobStoragePublish } from './azureBlobStorage'; import { PublisherBase } from './types'; import type { Entity } from '@backstage/catalog-model'; +import type { Logger } from 'winston'; const createMockEntity = (annotations = {}) => { return { @@ -43,33 +44,40 @@ const getEntityRootDir = (entity: Entity) => { return entityRootDir; }; -const logger = getVoidLogger(); -jest.spyOn(logger, 'info').mockReturnValue(logger); -jest.spyOn(logger, 'error').mockReturnValue(logger); +function createLogger() { + const logger = getVoidLogger(); + jest.spyOn(logger, 'info').mockReturnValue(logger); + jest.spyOn(logger, 'error').mockReturnValue(logger); + return logger; +} let publisher: PublisherBase; -beforeEach(async () => { - const mockConfig = new ConfigReader({ - techdocs: { - requestUrl: 'http://localhost:7000', - publisher: { - type: 'azureBlobStorage', - azureBlobStorage: { - credentials: { - accountName: 'accountName', - accountKey: 'accountKey', +describe('publishing with valid credentials', () => { + let logger: Logger; + + beforeEach(async () => { + const mockConfig = new ConfigReader({ + techdocs: { + requestUrl: 'http://localhost:7000', + publisher: { + type: 'azureBlobStorage', + azureBlobStorage: { + credentials: { + accountName: 'accountName', + accountKey: 'accountKey', + }, + containerName: 'containerName', }, - containerName: 'containerName', }, }, - }, + }); + + logger = createLogger(); + + publisher = await AzureBlobStoragePublish.fromConfig(mockConfig, logger); }); - publisher = await AzureBlobStoragePublish.fromConfig(mockConfig, logger); -}); - -describe('AzureBlobStoragePublish', () => { describe('publish', () => { it('should publish a directory', async () => { const entity = createMockEntity(); @@ -145,3 +153,52 @@ describe('AzureBlobStoragePublish', () => { }); }); }); + +describe('attempting to publish with invalid credentials', () => { + let logger: Logger; + + describe('accountKey is not specified', () => { + beforeEach(async () => { + const mockConfig = new ConfigReader({ + techdocs: { + requestUrl: 'http://localhost:7000', + publisher: { + type: 'azureBlobStorage', + azureBlobStorage: { + credentials: { + accountName: 'accountName', + }, + containerName: 'containerName', + }, + }, + }, + }); + + logger = createLogger(); + + publisher = await AzureBlobStoragePublish.fromConfig(mockConfig, logger); + }); + + describe('without Azure environment variables', () => { + it('should log an error', async () => { + const entity = createMockEntity(); + const entityRootDir = getEntityRootDir(entity); + + mockFs({ + [entityRootDir]: { + 'index.html': '', + }, + }); + + await publisher.publish({ + entity, + directory: entityRootDir, + }); + + expect(logger.error).toHaveBeenCalled(); + + mockFs.restore(); + }); + }); + }); +}); From c7e8e7cf058a5b0bed942c6b0eaddaab48aed08c Mon Sep 17 00:00:00 2001 From: Esteban Barrios Date: Mon, 15 Feb 2021 23:28:00 +0100 Subject: [PATCH 06/39] Removed unused import Removed unused import --- packages/integration/package.json | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/integration/package.json b/packages/integration/package.json index 254b4d3fe5..b4134ab00e 100644 --- a/packages/integration/package.json +++ b/packages/integration/package.json @@ -30,7 +30,6 @@ }, "dependencies": { "@backstage/config": "^0.1.2", - "@backstage/integration": "^0.4.0", "cross-fetch": "^3.0.6", "git-url-parse": "^11.4.4", "@octokit/rest": "^18.0.12", From ef5a18a9f9be74cdcdccf9ae3e93522dec7a0bfb Mon Sep 17 00:00:00 2001 From: Taras Date: Mon, 15 Feb 2021 18:30:55 -0500 Subject: [PATCH 07/39] Remove unnecessary array of promises --- .../src/stages/publish/azureBlobStorage.ts | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts b/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts index a02639e63f..e36254b164 100644 --- a/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts +++ b/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts @@ -122,8 +122,6 @@ export class AzureBlobStoragePublish implements PublisherBase { // So collecting path of only the files is good enough. const allFilesToUpload = await getFileTreeRecursively(directory); - const uploadPromises: Array> = []; - // Bound the number of concurrent batches. We want a bit of concurrency for // performance reasons, but not so much that we starve the connection pool // or start thrashing. @@ -139,13 +137,11 @@ export class AzureBlobStoragePublish implements PublisherBase { `${entityRootDir}/${relativeFilePath}`, ); // Azure Blob Storage Container file relative path - return limiter(async () => { - await uploadPromises.push( - this.storageClient - .getContainerClient(this.containerName) - .getBlockBlobClient(destination) - .uploadFile(filePath), - ); + return limiter(() => { + return this.storageClient + .getContainerClient(this.containerName) + .getBlockBlobClient(destination) + .uploadFile(filePath); }); }); From 4d483cbc10a827df2b1f66dbf6d5ad81c179907c Mon Sep 17 00:00:00 2001 From: Taras Date: Mon, 15 Feb 2021 21:52:54 -0500 Subject: [PATCH 08/39] Add tests for error reporting --- .../__mocks__/@azure/storage-blob.ts | 72 ++++++++++---- .../stages/publish/azureBlobStorage.test.ts | 96 +++++++++++-------- .../src/stages/publish/azureBlobStorage.ts | 49 +++++----- 3 files changed, 141 insertions(+), 76 deletions(-) diff --git a/packages/techdocs-common/__mocks__/@azure/storage-blob.ts b/packages/techdocs-common/__mocks__/@azure/storage-blob.ts index 60a705f6a7..29f3178efb 100644 --- a/packages/techdocs-common/__mocks__/@azure/storage-blob.ts +++ b/packages/techdocs-common/__mocks__/@azure/storage-blob.ts @@ -14,6 +14,10 @@ * limitations under the License. */ import fs from 'fs'; +import type { + BlobUploadCommonResponse, + ContainerGetPropertiesResponse, +} from '@azure/storage-blob'; export class BlockBlobClient { private readonly blobName; @@ -22,23 +26,29 @@ export class BlockBlobClient { this.blobName = blobName; } - uploadFile(source: string) { - return new Promise((resolve, reject) => { - if (!fs.existsSync(source)) { - reject(''); - } else { - resolve(''); - } + uploadFile(source: string): Promise { + return Promise.resolve({ + _response: { + request: {} as any, + status: 200, + headers: {} as any, + }, }); } exists() { - return new Promise((resolve, reject) => { - if (fs.existsSync(this.blobName)) { - resolve(true); - } else { - reject({ message: 'The object doest not exist !' }); - } + return Promise.resolve(fs.existsSync(this.blobName)); + } +} + +class BlockBlobClientFailUpload extends BlockBlobClient { + uploadFile(source: string): Promise { + return Promise.resolve({ + _response: { + request: {} as any, + status: 500, + headers: {} as any, + }, }); } } @@ -50,9 +60,14 @@ export class ContainerClient { this.containerName = containerName; } - getProperties() { - return new Promise(resolve => { - resolve(''); + getProperties(): Promise { + return Promise.resolve({ + _response: { + request: {} as any, + status: 200, + headers: {} as any, + parsedHeaders: {}, + }, }); } @@ -61,6 +76,25 @@ export class ContainerClient { } } +class ContainerClientFailGetProperties extends ContainerClient { + getProperties(): Promise { + return Promise.resolve({ + _response: { + request: {} as any, + status: 404, + headers: {} as any, + parsedHeaders: {}, + }, + }); + } +} + +class ContainerClientFailUpload extends ContainerClient { + getBlockBlobClient(blobName: string) { + return new BlockBlobClientFailUpload(blobName); + } +} + export class BlobServiceClient { private readonly url; private readonly credential; @@ -71,6 +105,12 @@ export class BlobServiceClient { } getContainerClient(containerName: string) { + if (containerName === 'bad_container') { + return new ContainerClientFailGetProperties(containerName); + } + if (this.credential.accountName === 'failupload') { + return new ContainerClientFailUpload(containerName); + } return new ContainerClient(containerName); } } diff --git a/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts b/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts index 18724a84f9..b37d6c9379 100644 --- a/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts +++ b/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts @@ -154,51 +154,71 @@ describe('publishing with valid credentials', () => { }); }); -describe('attempting to publish with invalid credentials', () => { - let logger: Logger; - - describe('accountKey is not specified', () => { - beforeEach(async () => { - const mockConfig = new ConfigReader({ - techdocs: { - requestUrl: 'http://localhost:7000', - publisher: { - type: 'azureBlobStorage', - azureBlobStorage: { - credentials: { - accountName: 'accountName', - }, - containerName: 'containerName', +describe('error reporting', () => { + it('reports an error when unable to read container properties', async () => { + const mockConfig = new ConfigReader({ + techdocs: { + requestUrl: 'http://localhost:7000', + publisher: { + type: 'azureBlobStorage', + azureBlobStorage: { + credentials: { + accountName: 'accountName', }, + containerName: 'bad_container', }, }, - }); - - logger = createLogger(); - - publisher = await AzureBlobStoragePublish.fromConfig(mockConfig, logger); + }, }); - describe('without Azure environment variables', () => { - it('should log an error', async () => { - const entity = createMockEntity(); - const entityRootDir = getEntityRootDir(entity); + const logger = createLogger(); - mockFs({ - [entityRootDir]: { - 'index.html': '', + publisher = await AzureBlobStoragePublish.fromConfig(mockConfig, logger); + + expect(logger.error).toHaveBeenCalledWith( + `Could not read Azure Blob Storage container properties`, + ); + }); + + it('reports an error when bad account credentials', async () => { + const mockConfig = new ConfigReader({ + techdocs: { + requestUrl: 'http://localhost:7000', + publisher: { + type: 'azureBlobStorage', + azureBlobStorage: { + credentials: { + accountName: 'failupload', + accountKey: 'accountKey', + }, + containerName: 'containerName', }, - }); - - await publisher.publish({ - entity, - directory: entityRootDir, - }); - - expect(logger.error).toHaveBeenCalled(); - - mockFs.restore(); - }); + }, + }, }); + + const logger = createLogger(); + + publisher = await AzureBlobStoragePublish.fromConfig(mockConfig, logger); + + const entity = createMockEntity(); + const entityRootDir = getEntityRootDir(entity); + + mockFs({ + [entityRootDir]: { + 'index.html': '', + }, + }); + + await publisher.publish({ + entity, + directory: entityRootDir, + }); + + expect(logger.error).toHaveBeenCalledWith( + `Unable to upload 1 file(s) to Azure Blob Storage.`, + ); + + mockFs.restore(); }); }); diff --git a/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts b/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts index e36254b164..b0b2bfd855 100644 --- a/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts +++ b/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts @@ -82,10 +82,16 @@ export class AzureBlobStoragePublish implements PublisherBase { await storageClient .getContainerClient(containerName) .getProperties() - .then(() => { - logger.info( - `Successfully connected to the Azure Blob Storage container ${containerName}.`, - ); + .then(result => { + if (result?._response?.status >= 400) { + logger.error( + 'Could not read Azure Blob Storage container properties', + ); + } else { + logger.info( + `Successfully connected to the Azure Blob Storage container ${containerName}.`, + ); + } }) .catch(reason => { logger.error( @@ -145,16 +151,23 @@ export class AzureBlobStoragePublish implements PublisherBase { }); }); - await Promise.all(promises).then(() => { + let responses: BlobUploadCommonResponse[] = []; + + responses = await Promise.all(promises); + + const failed = responses.filter(r => r?._response?.status >= 400); + if (failed.length === 0) { this.logger.info( `Successfully uploaded all the generated files for Entity ${entity.metadata.name}. Total number of files: ${allFilesToUpload.length}`, ); - }); - return; + } else { + const errorMessage = `Unable to upload ${failed.length} file(s) to Azure Blob Storage.`; + this.logger.error(errorMessage); + } } catch (e) { const errorMessage = `Unable to upload file(s) to Azure Blob Storage. Error ${e.message}`; this.logger.error(errorMessage); - throw new Error(errorMessage); + return; } } @@ -237,19 +250,11 @@ export class AzureBlobStoragePublish implements PublisherBase { * A helper function which checks if index.html of an Entity's docs site is available. This * can be used to verify if there are any pre-generated docs available to serve. */ - async hasDocsBeenGenerated(entity: Entity): Promise { - return new Promise(resolve => { - const entityRootDir = `${entity.metadata.namespace}/${entity.kind}/${entity.metadata.name}`; - this.storageClient - .getContainerClient(this.containerName) - .getBlockBlobClient(`${entityRootDir}/index.html`) - .exists() - .then((response: boolean) => { - resolve(response); - }) - .catch(() => { - resolve(false); - }); - }); + hasDocsBeenGenerated(entity: Entity): Promise { + const entityRootDir = `${entity.metadata.namespace}/${entity.kind}/${entity.metadata.name}`; + return this.storageClient + .getContainerClient(this.containerName) + .getBlockBlobClient(`${entityRootDir}/index.html`) + .exists(); } } From e518093dceac901d2cf9c6e68af54c7124da75c6 Mon Sep 17 00:00:00 2001 From: Taras Mankovski Date: Mon, 15 Feb 2021 21:57:11 -0500 Subject: [PATCH 09/39] Create quiet-dots-mix.md --- .changeset/quiet-dots-mix.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/quiet-dots-mix.md diff --git a/.changeset/quiet-dots-mix.md b/.changeset/quiet-dots-mix.md new file mode 100644 index 0000000000..b1186b320f --- /dev/null +++ b/.changeset/quiet-dots-mix.md @@ -0,0 +1,5 @@ +--- +"@backstage/techdocs-common": patch +--- + +Changed AzureBlobStorage to show errors when upload fails From 79f7bb4665d950104783a82203a6052b507cf671 Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Tue, 16 Feb 2021 08:23:08 +0100 Subject: [PATCH 10/39] Update .changeset/quiet-dots-mix.md --- .changeset/quiet-dots-mix.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/quiet-dots-mix.md b/.changeset/quiet-dots-mix.md index b1186b320f..a83278671e 100644 --- a/.changeset/quiet-dots-mix.md +++ b/.changeset/quiet-dots-mix.md @@ -1,5 +1,5 @@ --- -"@backstage/techdocs-common": patch +'@backstage/techdocs-common': patch --- Changed AzureBlobStorage to show errors when upload fails From 3a15d2941ebae2a23a8bead606a55971efc21990 Mon Sep 17 00:00:00 2001 From: ebarrios Date: Tue, 16 Feb 2021 10:18:14 +0100 Subject: [PATCH 11/39] Addresed all coments from Rugvip to remove all mutable objects from the fromConfig function --- .../src/scaffolder/stages/publish/github.ts | 55 ++++++++----------- 1 file changed, 23 insertions(+), 32 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts b/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts index 98d16e6bb7..1846cb90d1 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts @@ -31,24 +31,16 @@ export class GithubPublisher implements PublisherBase { config: GitHubIntegrationConfig, { repoVisibility }: { repoVisibility: RepoVisibilityOptions }, ) { - let credentialsProvider: GithubCredentialsProvider | undefined = undefined; - if (!config.token) { if (!config.apps) { return undefined; } - credentialsProvider = GithubCredentialsProvider.create(config); } - const githubClient = new Octokit({ - auth: config.token, - baseUrl: config.apiBaseUrl, - }); + const credentialsProvider = GithubCredentialsProvider.create(config); return new GithubPublisher({ - token: config.token || '', credentialsProvider, - client: githubClient, repoVisibility, apiBaseUrl: config.apiBaseUrl, }); @@ -56,9 +48,7 @@ export class GithubPublisher implements PublisherBase { constructor( private readonly config: { - token: string; - credentialsProvider: GithubCredentialsProvider | undefined; - client: Octokit; + credentialsProvider: GithubCredentialsProvider; repoVisibility: RepoVisibilityOptions; apiBaseUrl: string | undefined; }, @@ -70,23 +60,23 @@ export class GithubPublisher implements PublisherBase { logger, }: PublisherOptions): Promise { const { owner, name } = parseGitUrl(values.storePath); - - if (this.config.credentialsProvider) { - this.config.token = - ( - await this.config.credentialsProvider.getCredentials({ - url: values.storePath, - }) - ).token || ''; - this.config.client = new Octokit({ - auth: this.config.token, - baseUrl: this.config.apiBaseUrl, - }); - } + + const token = + ( + await this.config.credentialsProvider.getCredentials({ + url: values.storePath, + }) + ).token || ''; + + const client = new Octokit({ + auth: token, + baseUrl: this.config.apiBaseUrl, + }); const description = values.description as string; const access = values.access as string; const remoteUrl = await this.createRemote({ + client, description, access, name, @@ -98,7 +88,7 @@ export class GithubPublisher implements PublisherBase { remoteUrl, auth: { username: 'x-access-token', - password: this.config.token, + password: token, }, logger, }); @@ -111,27 +101,28 @@ export class GithubPublisher implements PublisherBase { } private async createRemote(opts: { + client: Octokit; access: string; name: string; owner: string; description: string; }) { - const { access, description, owner, name } = opts; + const { client, access, description, owner, name } = opts; - const user = await this.config.client.users.getByUsername({ + const user = await client.users.getByUsername({ username: owner, }); const repoCreationPromise = user.data.type === 'Organization' - ? this.config.client.repos.createInOrg({ + ? client.repos.createInOrg({ name, org: owner, private: this.config.repoVisibility !== 'public', visibility: this.config.repoVisibility, description, }) - : this.config.client.repos.createForAuthenticatedUser({ + : client.repos.createForAuthenticatedUser({ name, private: this.config.repoVisibility === 'private', description, @@ -141,7 +132,7 @@ export class GithubPublisher implements PublisherBase { if (access?.startsWith(`${owner}/`)) { const [, team] = access.split('/'); - await this.config.client.teams.addOrUpdateRepoPermissionsInOrg({ + await client.teams.addOrUpdateRepoPermissionsInOrg({ org: owner, team_slug: team, owner, @@ -150,7 +141,7 @@ export class GithubPublisher implements PublisherBase { }); // no need to add access if it's the person who own's the personal account } else if (access && access !== owner) { - await this.config.client.repos.addCollaborator({ + await client.repos.addCollaborator({ owner, repo: name, username: access, From edbc27bfdc1c0ba4b3f93cf3f109f49804170853 Mon Sep 17 00:00:00 2001 From: ebarrios Date: Tue, 16 Feb 2021 10:59:04 +0100 Subject: [PATCH 12/39] Added changeset --- .changeset/loud-owls-beam.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/loud-owls-beam.md diff --git a/.changeset/loud-owls-beam.md b/.changeset/loud-owls-beam.md new file mode 100644 index 0000000000..f17b23223b --- /dev/null +++ b/.changeset/loud-owls-beam.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder-backend': minor +--- + +Added githubApp authentication to the scaffolder-backend plugin From 3fc738a55c494da8c577da1e9b021ad8c517c041 Mon Sep 17 00:00:00 2001 From: ebarrios Date: Tue, 16 Feb 2021 12:58:15 +0100 Subject: [PATCH 13/39] modify change set to path, simplify to one expresion the check for config.token and config.apps and deconstructed the response fron the credentialProvider.getCredentials function --- .changeset/loud-owls-beam.md | 2 +- .../src/scaffolder/stages/publish/github.ts | 19 +++++++++---------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/.changeset/loud-owls-beam.md b/.changeset/loud-owls-beam.md index f17b23223b..764bfdf72a 100644 --- a/.changeset/loud-owls-beam.md +++ b/.changeset/loud-owls-beam.md @@ -1,5 +1,5 @@ --- -'@backstage/plugin-scaffolder-backend': minor +'@backstage/plugin-scaffolder-backend': patch --- Added githubApp authentication to the scaffolder-backend plugin diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts b/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts index 1846cb90d1..365b3f14cd 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts @@ -31,10 +31,8 @@ export class GithubPublisher implements PublisherBase { config: GitHubIntegrationConfig, { repoVisibility }: { repoVisibility: RepoVisibilityOptions }, ) { - if (!config.token) { - if (!config.apps) { - return undefined; - } + if (!config.token && !config.apps) { + return undefined; } const credentialsProvider = GithubCredentialsProvider.create(config); @@ -61,12 +59,13 @@ export class GithubPublisher implements PublisherBase { }: PublisherOptions): Promise { const { owner, name } = parseGitUrl(values.storePath); - const token = - ( - await this.config.credentialsProvider.getCredentials({ - url: values.storePath, - }) - ).token || ''; + const { token } = await this.config.credentialsProvider.getCredentials({ + url: values.storePath, + }); + + if (!token) { + return { remoteUrl: '', catalogInfoUrl: undefined }; + } const client = new Octokit({ auth: token, From 4de00ead3887a1dd8307745b1ab46d3e40a1f58a Mon Sep 17 00:00:00 2001 From: Taras Date: Tue, 16 Feb 2021 09:00:17 -0500 Subject: [PATCH 14/39] Report errors when creating AzureBlobPublisher and requirements are not satisfied --- .../__mocks__/@azure/storage-blob.ts | 16 ++++++-- .../stages/publish/azureBlobStorage.test.ts | 13 +++++- .../src/stages/publish/azureBlobStorage.ts | 40 ++++++++----------- 3 files changed, 40 insertions(+), 29 deletions(-) diff --git a/packages/techdocs-common/__mocks__/@azure/storage-blob.ts b/packages/techdocs-common/__mocks__/@azure/storage-blob.ts index 29f3178efb..6633a71b2c 100644 --- a/packages/techdocs-common/__mocks__/@azure/storage-blob.ts +++ b/packages/techdocs-common/__mocks__/@azure/storage-blob.ts @@ -29,7 +29,9 @@ export class BlockBlobClient { uploadFile(source: string): Promise { return Promise.resolve({ _response: { - request: {} as any, + request: { + url: `https://example.blob.core.windows.net`, + } as any, status: 200, headers: {} as any, }, @@ -45,7 +47,9 @@ class BlockBlobClientFailUpload extends BlockBlobClient { uploadFile(source: string): Promise { return Promise.resolve({ _response: { - request: {} as any, + request: { + url: `https://example.blob.core.windows.net`, + } as any, status: 500, headers: {} as any, }, @@ -63,7 +67,9 @@ export class ContainerClient { getProperties(): Promise { return Promise.resolve({ _response: { - request: {} as any, + request: { + url: `https://example.blob.core.windows.net`, + } as any, status: 200, headers: {} as any, parsedHeaders: {}, @@ -80,7 +86,9 @@ class ContainerClientFailGetProperties extends ContainerClient { getProperties(): Promise { return Promise.resolve({ _response: { - request: {} as any, + request: { + url: `https://example.blob.core.windows.net`, + } as any, status: 404, headers: {} as any, parsedHeaders: {}, diff --git a/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts b/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts index b37d6c9379..baeb644301 100644 --- a/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts +++ b/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts @@ -173,10 +173,19 @@ describe('error reporting', () => { const logger = createLogger(); - publisher = await AzureBlobStoragePublish.fromConfig(mockConfig, logger); + let error; + try { + publisher = await AzureBlobStoragePublish.fromConfig(mockConfig, logger); + } catch (e) { + error = e; + } + + expect(error).toBeInstanceOf(Error); expect(logger.error).toHaveBeenCalledWith( - `Could not read Azure Blob Storage container properties`, + expect.stringContaining( + `Could not retrieve metadata about the Azure Blob Storage container bad_container.`, + ), ); }); diff --git a/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts b/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts index b0b2bfd855..f73417156a 100644 --- a/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts +++ b/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts @@ -79,31 +79,25 @@ export class AzureBlobStoragePublish implements PublisherBase { credential, ); - await storageClient - .getContainerClient(containerName) - .getProperties() - .then(result => { - if (result?._response?.status >= 400) { - logger.error( - 'Could not read Azure Blob Storage container properties', - ); - } else { - logger.info( - `Successfully connected to the Azure Blob Storage container ${containerName}.`, - ); - } - }) - .catch(reason => { - logger.error( - `Could not retrieve metadata about the Azure Blob Storage container ${containerName}. ` + - 'Make sure that the Azure project and container exist and the access key is setup correctly ' + - 'techdocs.publisher.azureBlobStorage.credentials defined in app config has correct permissions. ' + - 'Refer to https://backstage.io/docs/features/techdocs/using-cloud-storage', - ); + try { + const metadata = await storageClient + .getContainerClient(containerName) + .getProperties(); + + if (metadata._response.status >= 400) { throw new Error( - `from Azure Blob Storage client library: ${reason.message}`, + `Failed to retrieve metadata from ${metadata._response.request.url} with status code ${metadata._response.status}.`, ); - }); + } + } catch (e) { + logger.error( + `Could not retrieve metadata about the Azure Blob Storage container ${containerName}. ` + + 'Make sure that the Azure project and container exist and the access key is setup correctly ' + + 'techdocs.publisher.azureBlobStorage.credentials defined in app config has correct permissions. ' + + 'Refer to https://backstage.io/docs/features/techdocs/using-cloud-storage', + ); + throw new Error(`from Azure Blob Storage client library: ${e.message}`); + } return new AzureBlobStoragePublish(storageClient, containerName, logger); } From fdd33af1cf79e71d35f550f353a7d5cac126eddb Mon Sep 17 00:00:00 2001 From: ebarrios Date: Tue, 16 Feb 2021 15:10:00 +0100 Subject: [PATCH 15/39] Added githubApp authentication for the scaffolder prepare stage --- .../src/scaffolder/stages/prepare/github.ts | 28 ++++++++++++------- .../src/scaffolder/stages/publish/github.ts | 1 + 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.ts b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.ts index 5b74ee81ca..d2b29b5d0b 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.ts @@ -18,14 +18,20 @@ import path from 'path'; import { Git } from '@backstage/backend-common'; import { PreparerBase, PreparerOptions } from './types'; import parseGitUrl from 'git-url-parse'; -import { GitHubIntegrationConfig } from '@backstage/integration'; +import { + GitHubIntegrationConfig, + GithubCredentialsProvider, +} from '@backstage/integration'; export class GithubPreparer implements PreparerBase { static fromConfig(config: GitHubIntegrationConfig) { - return new GithubPreparer({ token: config.token }); + const credentialsProvider = GithubCredentialsProvider.create(config); + return new GithubPreparer({ credentialsProvider }); } - constructor(private readonly config: { token?: string }) {} + constructor( + private readonly config: { credentialsProvider: GithubCredentialsProvider }, + ) {} async prepare({ url, workspacePath, logger }: PreparerOptions) { const parsedGitUrl = parseGitUrl(url); @@ -36,13 +42,15 @@ export class GithubPreparer implements PreparerBase { parsedGitUrl.filepath ?? '', ); - const git = this.config.token - ? Git.fromAuth({ - username: 'x-access-token', - password: this.config.token, - logger, - }) - : Git.fromAuth({ logger }); + const { token } = await this.config.credentialsProvider.getCredentials({ + url, + }); + + const git = Git.fromAuth({ + username: 'x-access-token', + password: token, + logger, + }); await git.clone({ url: parsedGitUrl.toString('https'), diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts b/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts index 365b3f14cd..e71da7fb84 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts @@ -64,6 +64,7 @@ export class GithubPublisher implements PublisherBase { }); if (!token) { + logger.error(`Unable to adquire credentials for ${owner}/${name}`); return { remoteUrl: '', catalogInfoUrl: undefined }; } From 549a859ac8ac201cab66ff3519fc95f310cab721 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gustaf=20R=C3=A4ntil=C3=A4?= Date: Tue, 16 Feb 2021 14:06:58 +0100 Subject: [PATCH 16/39] Improve pagerduty plugin --- .changeset/sour-shoes-perform.md | 5 + .../app/src/components/catalog/EntityPage.tsx | 6 +- .../components/Incident/IncidentListItem.tsx | 53 +++++++---- .../src/components/PagerDutyCard.tsx | 60 ++---------- .../src/components/TriggerButton/index.tsx | 94 +++++++++++++++++++ .../TriggerDialog/TriggerDialog.tsx | 18 ++-- plugins/pagerduty/src/components/constants.ts | 16 ++++ plugins/pagerduty/src/index.ts | 1 + 8 files changed, 177 insertions(+), 76 deletions(-) create mode 100644 .changeset/sour-shoes-perform.md create mode 100644 plugins/pagerduty/src/components/TriggerButton/index.tsx create mode 100644 plugins/pagerduty/src/components/constants.ts diff --git a/.changeset/sour-shoes-perform.md b/.changeset/sour-shoes-perform.md new file mode 100644 index 0000000000..e2361cb815 --- /dev/null +++ b/.changeset/sour-shoes-perform.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-pagerduty': minor +--- + +Improved the UI of the pagerduty plugin, and added a standalone TriggerButton diff --git a/packages/app/src/components/catalog/EntityPage.tsx b/packages/app/src/components/catalog/EntityPage.tsx index e849667c19..dcd0cb4c05 100644 --- a/packages/app/src/components/catalog/EntityPage.tsx +++ b/packages/app/src/components/catalog/EntityPage.tsx @@ -33,7 +33,7 @@ import { EntityLinksCard, EntityPageLayout, } from '@backstage/plugin-catalog'; -import { useEntity } from '@backstage/plugin-catalog-react'; +import { EntityProvider, useEntity } from '@backstage/plugin-catalog-react'; import { isPluginApplicableToEntity as isCircleCIAvailable, Router as CircleCIRouter, @@ -178,7 +178,9 @@ const ComponentOverviewContent = ({ entity }: { entity: Entity }) => ( {isPagerDutyAvailable(entity) && ( - + + + )} diff --git a/plugins/pagerduty/src/components/Incident/IncidentListItem.tsx b/plugins/pagerduty/src/components/Incident/IncidentListItem.tsx index bf0d5d2587..2d4afa53cd 100644 --- a/plugins/pagerduty/src/components/Incident/IncidentListItem.tsx +++ b/plugins/pagerduty/src/components/Incident/IncidentListItem.tsx @@ -17,7 +17,6 @@ import React from 'react'; import { ListItem, - ListItemIcon, ListItemSecondaryAction, Tooltip, ListItemText, @@ -25,13 +24,16 @@ import { IconButton, Link, Typography, + Chip, } from '@material-ui/core'; -import { StatusError, StatusWarning } from '@backstage/core'; +import Done from '@material-ui/icons/Done'; +import Warning from '@material-ui/icons/Warning'; import { DateTime, Duration } from 'luxon'; import { Incident } from '../types'; import OpenInBrowserIcon from '@material-ui/icons/OpenInBrowser'; +import { BackstageTheme } from '@backstage/theme'; -const useStyles = makeStyles({ +const useStyles = makeStyles(theme => ({ denseListIcon: { marginRight: 0, display: 'flex', @@ -42,10 +44,21 @@ const useStyles = makeStyles({ listItemPrimary: { fontWeight: 'bold', }, - listItemIcon: { - minWidth: '1em', + warning: { + borderColor: theme.palette.status.warning, + color: theme.palette.status.warning, + '& *': { + color: theme.palette.status.warning, + }, }, -}); + error: { + borderColor: theme.palette.status.error, + color: theme.palette.status.error, + '& *': { + color: theme.palette.status.error, + }, + }, +})); type Props = { incident: Incident; @@ -62,19 +75,23 @@ export const IncidentListItem = ({ incident }: Props) => { return ( - - -
- {incident.status === 'triggered' ? ( - - ) : ( - - )} -
-
-
+ : } + className={ + incident.status === 'triggered' + ? classes.error + : classes.warning + } + /> + {incident.title} + + } primaryTypographyProps={{ variant: 'body1', className: classes.listItemPrimary, diff --git a/plugins/pagerduty/src/components/PagerDutyCard.tsx b/plugins/pagerduty/src/components/PagerDutyCard.tsx index e03fb0fdbe..5f2ae42aee 100644 --- a/plugins/pagerduty/src/components/PagerDutyCard.tsx +++ b/plugins/pagerduty/src/components/PagerDutyCard.tsx @@ -19,7 +19,6 @@ import { Entity } from '@backstage/catalog-model'; import { useEntity } from '@backstage/plugin-catalog-react'; import { Button, - makeStyles, Card, CardHeader, Divider, @@ -31,54 +30,31 @@ import { useAsync } from 'react-use'; import { Alert } from '@material-ui/lab'; import { pagerDutyApiRef, UnauthorizedError } from '../api'; import AlarmAddIcon from '@material-ui/icons/AlarmAdd'; -import { TriggerDialog } from './TriggerDialog'; import { MissingTokenError } from './Errors/MissingTokenError'; import WebIcon from '@material-ui/icons/Web'; - -const useStyles = makeStyles({ - triggerAlarm: { - paddingTop: 0, - paddingBottom: 0, - fontSize: '0.7rem', - textTransform: 'uppercase', - fontWeight: 600, - letterSpacing: 1.2, - lineHeight: 1.5, - '&:hover, &:focus, &.focus': { - backgroundColor: 'transparent', - textDecoration: 'none', - }, - }, -}); - -export const PAGERDUTY_INTEGRATION_KEY = 'pagerduty.com/integration-key'; +import { PAGERDUTY_INTEGRATION_KEY } from './constants'; +import { TriggerButton, useShowDialog } from './TriggerButton'; export const isPluginApplicableToEntity = (entity: Entity) => Boolean(entity.metadata.annotations?.[PAGERDUTY_INTEGRATION_KEY]); -type Props = { - /** @deprecated The entity is now grabbed from context instead */ - entity?: Entity; -}; - -export const PagerDutyCard = (_props: Props) => { - const classes = useStyles(); +export const PagerDutyCard = () => { const { entity } = useEntity(); const api = useApi(pagerDutyApiRef); - const [showDialog, setShowDialog] = useState(false); const [refreshIncidents, setRefreshIncidents] = useState(false); const integrationKey = entity.metadata.annotations![ PAGERDUTY_INTEGRATION_KEY ]; + const setShowDialog = useShowDialog()[1]; + + const showDialog = useCallback(() => { + setShowDialog(true); + }, [setShowDialog]); const handleRefresh = useCallback(() => { setRefreshIncidents(x => !x); }, []); - const handleDialog = useCallback(() => { - setShowDialog(x => !x); - }, []); - const { value: service, loading, error } = useAsync(async () => { const services = await api.getServiceByIntegrationKey(integrationKey); @@ -114,17 +90,8 @@ export const PagerDutyCard = (_props: Props) => { const triggerLink = { label: 'Create Incident', - action: ( - - ), - icon: , + action: , + icon: , }; return ( @@ -140,13 +107,6 @@ export const PagerDutyCard = (_props: Props) => { refreshIncidents={refreshIncidents} /> - ); diff --git a/plugins/pagerduty/src/components/TriggerButton/index.tsx b/plugins/pagerduty/src/components/TriggerButton/index.tsx new file mode 100644 index 0000000000..833e01d8aa --- /dev/null +++ b/plugins/pagerduty/src/components/TriggerButton/index.tsx @@ -0,0 +1,94 @@ +/* + * Copyright 2021 Spotify AB + * + * 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 React, { useCallback, PropsWithChildren } from 'react'; +import { createGlobalState } from 'react-use'; +import { makeStyles, Button } from '@material-ui/core'; +import { useEntity } from '@backstage/plugin-catalog-react'; +import { BackstageTheme } from '@backstage/theme'; + +import { TriggerDialog } from '../TriggerDialog'; +import { PAGERDUTY_INTEGRATION_KEY } from '../constants'; + +export interface TriggerButtonProps { + design: 'link' | 'button'; + onIncidentCreated?: () => void; +} + +const useStyles = makeStyles(theme => ({ + buttonStyle: { + backgroundColor: theme.palette.error.main, + color: theme.palette.error.contrastText, + '&:hover': { + backgroundColor: theme.palette.error.dark, + }, + }, + triggerAlarm: { + paddingTop: 0, + paddingBottom: 0, + fontSize: '0.7rem', + textTransform: 'uppercase', + fontWeight: 600, + letterSpacing: 1.2, + lineHeight: 1.5, + '&:hover, &:focus, &.focus': { + backgroundColor: 'transparent', + textDecoration: 'none', + }, + }, +})); + +export const useShowDialog = createGlobalState(false); + +export function TriggerButton({ + design, + onIncidentCreated, + children, +}: PropsWithChildren) { + const { buttonStyle, triggerAlarm } = useStyles(); + const { entity } = useEntity(); + const [dialogShown = false, setDialogShown] = useShowDialog(); + + const showDialog = useCallback(() => { + setDialogShown(true); + }, [setDialogShown]); + const hideDialog = useCallback(() => { + setDialogShown(false); + }, [setDialogShown]); + + const integrationKey = entity.metadata.annotations![ + PAGERDUTY_INTEGRATION_KEY + ]; + + return ( + <> + + + + ); +} diff --git a/plugins/pagerduty/src/components/TriggerDialog/TriggerDialog.tsx b/plugins/pagerduty/src/components/TriggerDialog/TriggerDialog.tsx index c390ed1f82..c2084a41ed 100644 --- a/plugins/pagerduty/src/components/TriggerDialog/TriggerDialog.tsx +++ b/plugins/pagerduty/src/components/TriggerDialog/TriggerDialog.tsx @@ -35,7 +35,7 @@ type Props = { integrationKey: string; showDialog: boolean; handleDialog: () => void; - onIncidentCreated: () => void; + onIncidentCreated?: () => void; }; export const TriggerDialog = ({ @@ -69,11 +69,17 @@ export const TriggerDialog = ({ useEffect(() => { if (value) { - alertApi.post({ - message: `Alarm successfully triggered by ${userName}`, - }); - onIncidentCreated(); - handleDialog(); + (async () => { + alertApi.post({ + message: `Alarm successfully triggered by ${userName}`, + }); + + handleDialog(); + + // The pager duty API isn't always returning the newly created alarm immediately + await new Promise(resolve => setTimeout(resolve, 1000)); + onIncidentCreated?.(); + })(); } }, [value, alertApi, handleDialog, userName, onIncidentCreated]); diff --git a/plugins/pagerduty/src/components/constants.ts b/plugins/pagerduty/src/components/constants.ts new file mode 100644 index 0000000000..a7c32a362e --- /dev/null +++ b/plugins/pagerduty/src/components/constants.ts @@ -0,0 +1,16 @@ +/* + * Copyright 2021 Spotify AB + * + * 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. + */ +export const PAGERDUTY_INTEGRATION_KEY = 'pagerduty.com/integration-key'; diff --git a/plugins/pagerduty/src/index.ts b/plugins/pagerduty/src/index.ts index 2dfedab164..92c3c8e73c 100644 --- a/plugins/pagerduty/src/index.ts +++ b/plugins/pagerduty/src/index.ts @@ -23,6 +23,7 @@ export { isPluginApplicableToEntity as isPagerDutyAvailable, PagerDutyCard, } from './components/PagerDutyCard'; +export { TriggerButton } from './components/TriggerButton'; export { PagerDutyClient, pagerDutyApiRef, From a98637c1533c8545d75b17c22849b1eef522c996 Mon Sep 17 00:00:00 2001 From: Rob Long Date: Tue, 16 Feb 2021 16:55:51 +0000 Subject: [PATCH 17/39] Update Percentage.tsx useTheme() on line 32 returns null so I tested this change to the import on my local copy, which gets the sonar plugin working. Other plugins seem to be importing from '@material-ui/core', e.g. https://github.com/backstage/backstage/blob/7ee15d2c5b40a8984aacabe7bdf5ccab759d95a6/plugins/catalog/src/components/CatalogFilter/AllServicesCount.tsx#L19 --- plugins/sonarqube/src/components/SonarQubeCard/Percentage.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/sonarqube/src/components/SonarQubeCard/Percentage.tsx b/plugins/sonarqube/src/components/SonarQubeCard/Percentage.tsx index 9d1fc415ff..838e3d6609 100644 --- a/plugins/sonarqube/src/components/SonarQubeCard/Percentage.tsx +++ b/plugins/sonarqube/src/components/SonarQubeCard/Percentage.tsx @@ -16,7 +16,7 @@ import { BackstageTheme } from '@backstage/theme'; import { makeStyles } from '@material-ui/core/styles'; -import { useTheme } from '@material-ui/styles'; +import { useTheme } from '@material-ui/core'; import { Circle } from 'rc-progress'; import React from 'react'; From 02c5ce979c49aac15b6e4dde159905598d75e79a Mon Sep 17 00:00:00 2001 From: Taras Date: Tue, 16 Feb 2021 12:52:32 -0500 Subject: [PATCH 18/39] Throwing errors --- .../stages/publish/azureBlobStorage.test.ts | 23 +++++++--- .../src/stages/publish/azureBlobStorage.ts | 43 +++++++++++++------ 2 files changed, 46 insertions(+), 20 deletions(-) diff --git a/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts b/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts index baeb644301..62c81df5c4 100644 --- a/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts +++ b/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts @@ -124,7 +124,7 @@ describe('publishing with valid credentials', () => { }) .catch(error => expect(error.message).toContain( - 'Unable to upload file(s) to Azure Blob Storage. Error Failed to read template directory: ENOENT, no such file or directory', + 'Unable to upload file(s) to Azure Blob Storage. Failed to read template directory: ENOENT, no such file or directory', ), ); mockFs.restore(); @@ -219,13 +219,24 @@ describe('error reporting', () => { }, }); - await publisher.publish({ - entity, - directory: entityRootDir, - }); + let error; + try { + await publisher.publish({ + entity, + directory: entityRootDir, + }); + } catch (e) { + error = e; + } + + expect(error.message).toContain( + `Unable to upload file(s) to Azure Blob Storage.`, + ); expect(logger.error).toHaveBeenCalledWith( - `Unable to upload 1 file(s) to Azure Blob Storage.`, + expect.stringContaining( + `Unable to upload file(s) to Azure Blob Storage. Upload failed for test-namespace/TestKind/test-component-name/index.html with status code 500`, + ), ); mockFs.restore(); diff --git a/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts b/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts index f73417156a..07e1053b99 100644 --- a/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts +++ b/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts @@ -80,13 +80,13 @@ export class AzureBlobStoragePublish implements PublisherBase { ); try { - const metadata = await storageClient + const response = await storageClient .getContainerClient(containerName) .getProperties(); - if (metadata._response.status >= 400) { + if (response._response.status >= 400) { throw new Error( - `Failed to retrieve metadata from ${metadata._response.request.url} with status code ${metadata._response.status}.`, + `Failed to retrieve metadata from ${response._response.request.url} with status code ${response._response.status}.`, ); } } catch (e) { @@ -137,31 +137,46 @@ export class AzureBlobStoragePublish implements PublisherBase { `${entityRootDir}/${relativeFilePath}`, ); // Azure Blob Storage Container file relative path - return limiter(() => { - return this.storageClient + return limiter(async () => { + const response = await this.storageClient .getContainerClient(this.containerName) .getBlockBlobClient(destination) .uploadFile(filePath); + + if (response._response.status >= 400) { + return { + ...response, + error: new Error( + `Upload failed for ${filePath} with status code ${response._response.status}`, + ), + }; + } + return { + ...response, + error: undefined, + }; }); }); - let responses: BlobUploadCommonResponse[] = []; + const responses = await Promise.all(promises); - responses = await Promise.all(promises); - - const failed = responses.filter(r => r?._response?.status >= 400); + const failed = responses.filter(r => r.error); if (failed.length === 0) { this.logger.info( - `Successfully uploaded all the generated files for Entity ${entity.metadata.name}. Total number of files: ${allFilesToUpload.length}`, + `Successfully uploaded the ${responses.length} generated file(s) for Entity ${entity.metadata.name}. Total number of files: ${allFilesToUpload.length}`, ); } else { - const errorMessage = `Unable to upload ${failed.length} file(s) to Azure Blob Storage.`; - this.logger.error(errorMessage); + throw new Error( + failed + .map(r => r.error?.message) + .filter(Boolean) + .join(' '), + ); } } catch (e) { - const errorMessage = `Unable to upload file(s) to Azure Blob Storage. Error ${e.message}`; + const errorMessage = `Unable to upload file(s) to Azure Blob Storage. ${e.message}`; this.logger.error(errorMessage); - return; + throw new Error(errorMessage); } } From d1af7b4fbc71622c2cdb7e0903e5805a8191b345 Mon Sep 17 00:00:00 2001 From: Taras Date: Tue, 16 Feb 2021 12:58:26 -0500 Subject: [PATCH 19/39] Remove unnecessary import --- packages/techdocs-common/src/stages/publish/azureBlobStorage.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts b/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts index 07e1053b99..7000e9f28c 100644 --- a/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts +++ b/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts @@ -17,7 +17,6 @@ import platformPath from 'path'; import express from 'express'; import { BlobServiceClient, - BlobUploadCommonResponse, StorageSharedKeyCredential, } from '@azure/storage-blob'; import { DefaultAzureCredential } from '@azure/identity'; From 76db06fa5d730ef6002edaed7a40af0972124449 Mon Sep 17 00:00:00 2001 From: Taras Mankovski Date: Tue, 16 Feb 2021 13:00:14 -0500 Subject: [PATCH 20/39] Update quiet-dots-mix.md --- .changeset/quiet-dots-mix.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/quiet-dots-mix.md b/.changeset/quiet-dots-mix.md index a83278671e..b847c040c5 100644 --- a/.changeset/quiet-dots-mix.md +++ b/.changeset/quiet-dots-mix.md @@ -2,4 +2,4 @@ '@backstage/techdocs-common': patch --- -Changed AzureBlobStorage to show errors when upload fails +Improved error reporting in AzureBlobStorage to better surface failures From 8721f06ebea5ef979bd15698e52a244b2feebce8 Mon Sep 17 00:00:00 2001 From: Taras Mankovski Date: Tue, 16 Feb 2021 13:01:35 -0500 Subject: [PATCH 21/39] Update quiet-dots-mix.md --- .changeset/quiet-dots-mix.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.changeset/quiet-dots-mix.md b/.changeset/quiet-dots-mix.md index b847c040c5..97926c9d0f 100644 --- a/.changeset/quiet-dots-mix.md +++ b/.changeset/quiet-dots-mix.md @@ -2,4 +2,5 @@ '@backstage/techdocs-common': patch --- -Improved error reporting in AzureBlobStorage to better surface failures +Improved error reporting in AzureBlobStorage to surface errors when fetching metadata and uploading files fails. + From a2518e3dbe4aab77dfb4e050a21c376d976d38ee Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Tue, 16 Feb 2021 19:03:07 +0100 Subject: [PATCH 22/39] docs(TechDocs): Update docs around AWS SDK downgrade to v2 --- docs/features/techdocs/using-cloud-storage.md | 26 ++++++++++--------- .../src/stages/publish/awsS3.ts | 15 ++++++----- 2 files changed, 22 insertions(+), 19 deletions(-) diff --git a/docs/features/techdocs/using-cloud-storage.md b/docs/features/techdocs/using-cloud-storage.md index de2d505c87..7d735c72ab 100644 --- a/docs/features/techdocs/using-cloud-storage.md +++ b/docs/features/techdocs/using-cloud-storage.md @@ -142,6 +142,8 @@ variables** You should follow the [AWS security best practices guide for authentication](https://docs.aws.amazon.com/general/latest/gr/aws-access-keys-best-practices.html). +TechDocs needs access to read files and metadata of the S3 bucket. + If the environment variables - `AWS_ACCESS_KEY_ID` @@ -149,15 +151,21 @@ If the environment variables - `AWS_REGION` are set and can be used to access the bucket you created in step 2, they will be -used by the AWS SDK v3 Node.js client for authentication. -[Refer to the official documentation.](https://docs.aws.amazon.com/sdk-for-javascript/v3/developer-guide/loading-node-credentials-environment.html) +used by the AWS SDK V2 Node.js client for authentication. +[Refer to the official documentation.](https://docs.aws.amazon.com/sdk-for-javascript/v2/developer-guide/loading-node-credentials-environment.html) If the environment variables are missing, the AWS SDK tries to read the `~/.aws/credentials` file for credentials. -[Refer to the official documentation.](https://docs.aws.amazon.com/sdk-for-javascript/v3/developer-guide/loading-node-credentials-shared.html) +[Refer to the official documentation.](https://docs.aws.amazon.com/sdk-for-javascript/v2/developer-guide/loading-node-credentials-shared.html) -Note that the region of the bucket has to be set for the AWS SDK to work. -[See this](https://docs.aws.amazon.com/sdk-for-javascript/v3/developer-guide/setting-region.html). +If you are using Amazon EC2 instance to deploy Backstage, you do not need to +obtain the access keys separately. They can be made available in the environment +automatically by defining appropriate IAM role with access to the bucket. Read +more +[here](https://docs.aws.amazon.com/general/latest/gr/aws-access-keys-best-practices.html#use-roles). + +The AWS Region of the bucket is optional since TechDocs uses AWS SDK V2 and not +V3. **3b. Authentication using app-config.yaml** @@ -181,13 +189,7 @@ techdocs: ``` Refer to the -[official AWS documentation for obtaining the credentials](https://docs.aws.amazon.com/sdk-for-javascript/v3/developer-guide/getting-your-credentials.html). - -Note: If you are using Amazon EC2 instance to deploy Backstage, you do not need -to obtain the access keys separately. They can be made available in the -environment automatically by defining appropriate IAM role with access to the -bucket. Read more -[here](https://docs.aws.amazon.com/general/latest/gr/aws-access-keys-best-practices.html#use-roles). +[official AWS documentation for obtaining the credentials](https://docs.aws.amazon.com/sdk-for-javascript/v2/developer-guide/getting-your-credentials.html). **4. That's it!** diff --git a/packages/techdocs-common/src/stages/publish/awsS3.ts b/packages/techdocs-common/src/stages/publish/awsS3.ts index 6c0cadcfae..b0b29e8654 100644 --- a/packages/techdocs-common/src/stages/publish/awsS3.ts +++ b/packages/techdocs-common/src/stages/publish/awsS3.ts @@ -52,10 +52,13 @@ export class AwsS3Publish implements PublisherBase { ); } - // Credentials is an optional config. If missing, default AWS environment variables - // or AWS shared credentials file at ~/.aws/credentials will be used to authenticate - // https://docs.aws.amazon.com/sdk-for-javascript/v3/developer-guide/loading-node-credentials-environment.html - // https://docs.aws.amazon.com/sdk-for-javascript/v3/developer-guide/loading-node-credentials-shared.html + // Credentials is an optional config. If missing, the default ways of authenticating AWS SDK V2 will be used. + // 1. AWS environment variables + // https://docs.aws.amazon.com/sdk-for-javascript/v2/developer-guide/loading-node-credentials-environment.html + // 2. AWS shared credentials file at ~/.aws/credentials + // https://docs.aws.amazon.com/sdk-for-javascript/v2/developer-guide/loading-node-credentials-shared.html + // 3. IAM Roles for EC2 + // https://docs.aws.amazon.com/sdk-for-javascript/v2/developer-guide/loading-node-credentials-iam.html const credentials = config.getOptionalConfig( 'techdocs.publisher.awsS3.credentials', ); @@ -67,9 +70,7 @@ export class AwsS3Publish implements PublisherBase { } // AWS Region is an optional config. If missing, default AWS env variable AWS_REGION - // or AWS shared credentials file at ~/.aws/credentials will be used. Any way, AWS SDK v3 client needs - // to have the AWS Region information for it to work. - // https://docs.aws.amazon.com/sdk-for-javascript/v3/developer-guide/setting-region.html + // or AWS shared credentials file at ~/.aws/credentials will be used. const region = config.getOptionalString('techdocs.publisher.awsS3.region'); const storageClient = new aws.S3({ From 874864e38c486e9c45c74aafe639763a48715fc7 Mon Sep 17 00:00:00 2001 From: Taras Date: Tue, 16 Feb 2021 13:12:14 -0500 Subject: [PATCH 23/39] Fix formatting of changeset --- .changeset/quiet-dots-mix.md | 1 - 1 file changed, 1 deletion(-) diff --git a/.changeset/quiet-dots-mix.md b/.changeset/quiet-dots-mix.md index 97926c9d0f..fc4e550afd 100644 --- a/.changeset/quiet-dots-mix.md +++ b/.changeset/quiet-dots-mix.md @@ -3,4 +3,3 @@ --- Improved error reporting in AzureBlobStorage to surface errors when fetching metadata and uploading files fails. - From c6655413db29c70e0984177b0c1f675c582f05ef Mon Sep 17 00:00:00 2001 From: Taras Date: Tue, 16 Feb 2021 13:15:04 -0500 Subject: [PATCH 24/39] Renamed changeset to aply code owners --- .../{quiet-dots-mix.md => techdocs-improve-error-reporting.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename .changeset/{quiet-dots-mix.md => techdocs-improve-error-reporting.md} (100%) diff --git a/.changeset/quiet-dots-mix.md b/.changeset/techdocs-improve-error-reporting.md similarity index 100% rename from .changeset/quiet-dots-mix.md rename to .changeset/techdocs-improve-error-reporting.md From cc2c047f15791f90c5635acd81a190a5d7c62f5f Mon Sep 17 00:00:00 2001 From: ebarrios Date: Tue, 16 Feb 2021 19:50:56 +0100 Subject: [PATCH 25/39] Reverted change to preparer function to use Git.fromAuth({ logger }) when the credentials provider can not get a token --- .../src/scaffolder/stages/prepare/github.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.ts b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.ts index d2b29b5d0b..81ccdeb026 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.ts @@ -46,11 +46,13 @@ export class GithubPreparer implements PreparerBase { url, }); - const git = Git.fromAuth({ - username: 'x-access-token', - password: token, - logger, - }); + const git = token + ? Git.fromAuth({ + username: 'x-access-token', + password: token, + logger, + }) + : Git.fromAuth({ logger }); await git.clone({ url: parsedGitUrl.toString('https'), From 36a4ede27516becd0683ef59393bcd82dd3ea39c Mon Sep 17 00:00:00 2001 From: ebarrios Date: Tue, 16 Feb 2021 19:56:18 +0100 Subject: [PATCH 26/39] Change error message in the publish func for when the credentials provider can not get a token --- .../scaffolder-backend/src/scaffolder/stages/publish/github.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts b/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts index e71da7fb84..4de928c32b 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts @@ -64,7 +64,7 @@ export class GithubPublisher implements PublisherBase { }); if (!token) { - logger.error(`Unable to adquire credentials for ${owner}/${name}`); + logger.error(`No token could be acquired for URL: ${values.storePath}`); return { remoteUrl: '', catalogInfoUrl: undefined }; } From 3a82293dadf363efe063941b0797c004d7e8604c Mon Sep 17 00:00:00 2001 From: bobalong79 Date: Tue, 16 Feb 2021 22:19:22 +0000 Subject: [PATCH 27/39] Add changeset --- .changeset/blue-lions-worry.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/blue-lions-worry.md diff --git a/.changeset/blue-lions-worry.md b/.changeset/blue-lions-worry.md new file mode 100644 index 0000000000..911e1507b2 --- /dev/null +++ b/.changeset/blue-lions-worry.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-sonarqube': patch +--- + +Fix bug retrieving current theme From 36c154541cdb04d55ba77ef52dc01b0534f6d6f9 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 17 Feb 2021 04:54:48 +0000 Subject: [PATCH 28/39] chore(deps-dev): bump @graphql-codegen/typescript-resolvers Bumps [@graphql-codegen/typescript-resolvers](https://github.com/dotansimha/graphql-code-generator/tree/HEAD/packages/plugins/typescript/resolvers) from 1.17.12 to 1.18.2. - [Release notes](https://github.com/dotansimha/graphql-code-generator/releases) - [Changelog](https://github.com/dotansimha/graphql-code-generator/blob/master/packages/plugins/typescript/resolvers/CHANGELOG.md) - [Commits](https://github.com/dotansimha/graphql-code-generator/commits/@graphql-codegen/typescript-resolvers@1.18.2/packages/plugins/typescript/resolvers) Signed-off-by: dependabot[bot] --- yarn.lock | 85 +++++++++++++++++++++++++++++++++++++------------------ 1 file changed, 58 insertions(+), 27 deletions(-) diff --git a/yarn.lock b/yarn.lock index fb8fcdcad1..ac3a9e4a28 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2487,31 +2487,31 @@ upper-case "2.0.1" "@graphql-codegen/typescript-resolvers@^1.17.7": - version "1.17.12" - resolved "https://registry.npmjs.org/@graphql-codegen/typescript-resolvers/-/typescript-resolvers-1.17.12.tgz#1f8f608d68b46780351c8224a61750b9b9307497" - integrity sha512-qICKxl06R1yCBh03cdyiOYlHTebQ1n/FOiQPCxo4bsiF6HfbrgpMS8fdGEZ8v+VBPxQFZZ7HQ2KcuPNsuw5R6g== + version "1.18.2" + resolved "https://registry.npmjs.org/@graphql-codegen/typescript-resolvers/-/typescript-resolvers-1.18.2.tgz#7513b92df7c5a0d3c27342c591ada7340696cf8f" + integrity sha512-aWfRR5y1gXCPUNK7zaUiSbmceqidvZ38mNHIBvXmZArRigyz1QZgN26kKpXjJjtFbPvROPnlGsYcZBReybvZXA== dependencies: "@graphql-codegen/plugin-helpers" "^1.18.2" - "@graphql-codegen/typescript" "^1.18.1" - "@graphql-codegen/visitor-plugin-common" "^1.17.20" - "@graphql-tools/utils" "^6" - auto-bind "~4.0.0" - tslib "~2.0.1" - -"@graphql-codegen/typescript@^1.17.7", "@graphql-codegen/typescript@^1.18.1": - version "1.20.2" - resolved "https://registry.npmjs.org/@graphql-codegen/typescript/-/typescript-1.20.2.tgz#223327a8aea929e772f481e264c5aa3a36707fcb" - integrity sha512-D/DMUz4O2UEoFucUVu2K2xoaMPAn68BwYGnCAKnSDqtFKsOEqmTjHFwcgyEnpucQ5xFeG0pKGMb1SlS2Go9J8Q== - dependencies: - "@graphql-codegen/plugin-helpers" "^1.18.2" - "@graphql-codegen/visitor-plugin-common" "^1.18.2" + "@graphql-codegen/typescript" "^1.21.0" + "@graphql-codegen/visitor-plugin-common" "^1.18.3" + "@graphql-tools/utils" "^7.0.0" auto-bind "~4.0.0" tslib "~2.1.0" -"@graphql-codegen/visitor-plugin-common@^1.17.20", "@graphql-codegen/visitor-plugin-common@^1.18.2": - version "1.18.2" - resolved "https://registry.npmjs.org/@graphql-codegen/visitor-plugin-common/-/visitor-plugin-common-1.18.2.tgz#1a30bd33f011b4fb976e0f8462d160126db875ea" - integrity sha512-A8yBJGq7A7gxaVVXK4QXwV1ZpzZ64fH7U7JTGeq86o3jA7QNV2rmCRXCY0JttS2fu+olV18NjsWRwAXuAb1g9A== +"@graphql-codegen/typescript@^1.17.7", "@graphql-codegen/typescript@^1.21.0": + version "1.21.0" + resolved "https://registry.npmjs.org/@graphql-codegen/typescript/-/typescript-1.21.0.tgz#301b1851cd278bedd1f49e1b3d654f4dc0af2943" + integrity sha512-23YttnZ+87dA/3lbCvPKdsrpEOx142dCT9xSh6XkSeyCvn+vUtETN2MhamCYB87G7Nu2EcLDFKDZjgXH73f4fg== + dependencies: + "@graphql-codegen/plugin-helpers" "^1.18.2" + "@graphql-codegen/visitor-plugin-common" "^1.18.3" + auto-bind "~4.0.0" + tslib "~2.1.0" + +"@graphql-codegen/visitor-plugin-common@^1.18.3": + version "1.18.3" + resolved "https://registry.npmjs.org/@graphql-codegen/visitor-plugin-common/-/visitor-plugin-common-1.18.3.tgz#9d2c4449c3bdaffe3e782e2321fe0cb998b8a91d" + integrity sha512-6xJzt8hszCTKt3rTlcCURpuiAFuaiaZgStlVeRE1OrKEDiY1T3vwF3/7TonhfnEjqBWtZdMmXvNx3ArXkRUV4w== dependencies: "@graphql-codegen/plugin-helpers" "^1.18.2" "@graphql-tools/optimize" "^1.0.1" @@ -2808,14 +2808,14 @@ camel-case "4.1.1" tslib "~2.0.1" -"@graphql-tools/utils@^7.1.0": - version "7.1.0" - resolved "https://registry.npmjs.org/@graphql-tools/utils/-/utils-7.1.0.tgz#0cbcfa7b6e7741650f78e8bde4823780ed9e8c57" - integrity sha512-lMTgy08BdqQ31zyyCRrxcKZ6gAKQB9amGKJGg0mb3b4I3iJQKeaw9a7T96yGe1frGak1UK9y7OoYqx8RG7KitA== +"@graphql-tools/utils@^7.0.0", "@graphql-tools/utils@^7.1.0": + version "7.2.6" + resolved "https://registry.npmjs.org/@graphql-tools/utils/-/utils-7.2.6.tgz#5d974cbdec5ddf4d7fdc593816335512ee5fe4de" + integrity sha512-/kY7Nb+cCHi/MvU3tjz3KrXzuJNWMlnn7EoWazLmpDvl6b2Qt69hlVoPd5zQtKlGib35zZw9NZ5zs5qTAw8Y9g== dependencies: "@ardatan/aggregate-error" "0.0.6" - camel-case "4.1.1" - tslib "~2.0.1" + camel-case "4.1.2" + tslib "~2.1.0" "@graphql-tools/wrap@^6.2.4": version "6.2.4" @@ -9225,6 +9225,14 @@ camel-case@4.1.1, camel-case@^4.1.1: pascal-case "^3.1.1" tslib "^1.10.0" +camel-case@4.1.2: + version "4.1.2" + resolved "https://registry.npmjs.org/camel-case/-/camel-case-4.1.2.tgz#9728072a954f805228225a6deea6b38461e1bd5a" + integrity sha512-gxGWBrTT1JuMx6R+o5PTXMmUnhnVzLQ9SNutD4YqKtI6ap897t3tKECYla6gCWEkplXnlNybEkZg9GEGxKFCgw== + dependencies: + pascal-case "^3.1.2" + tslib "^2.0.3" + camelcase-keys@^2.0.0: version "2.1.0" resolved "https://registry.npmjs.org/camelcase-keys/-/camelcase-keys-2.1.0.tgz#308beeaffdf28119051efa1d932213c91b8f92e7" @@ -17455,6 +17463,13 @@ lower-case@2.0.1, lower-case@^2.0.1: dependencies: tslib "^1.10.0" +lower-case@^2.0.2: + version "2.0.2" + resolved "https://registry.npmjs.org/lower-case/-/lower-case-2.0.2.tgz#6fa237c63dbdc4a82ca0fd882e4722dc5e634e28" + integrity sha512-7fm3l3NAF9WfN6W3JOmf5drwpVqX78JtoGJ3A6W0a6ZnldM41w2fV5D490psKFTpMds8TJse/eHLFFsNHHjHgg== + dependencies: + tslib "^2.0.3" + lowercase-keys@^1.0.0, lowercase-keys@^1.0.1: version "1.0.1" resolved "https://registry.npmjs.org/lowercase-keys/-/lowercase-keys-1.0.1.tgz#6f9e30b47084d971a7c820ff15a6c5167b74c26f" @@ -18561,6 +18576,14 @@ no-case@^3.0.3: lower-case "^2.0.1" tslib "^1.10.0" +no-case@^3.0.4: + version "3.0.4" + resolved "https://registry.npmjs.org/no-case/-/no-case-3.0.4.tgz#d361fd5c9800f558551a8369fc0dcd4662b6124d" + integrity sha512-fgAN3jGAh+RoxUGZHTSOLJIqUc2wmoBwGR4tbpNAKmmovFoWq0OdRkb0VkldReO2a2iBT/OEulG9XSUc10r3zg== + dependencies: + lower-case "^2.0.2" + tslib "^2.0.3" + nock@^13.0.5: version "13.0.5" resolved "https://registry.npmjs.org/nock/-/nock-13.0.5.tgz#a618c6f86372cb79fac04ca9a2d1e4baccdb2414" @@ -19762,6 +19785,14 @@ pascal-case@3.1.1, pascal-case@^3.1.1: no-case "^3.0.3" tslib "^1.10.0" +pascal-case@^3.1.2: + version "3.1.2" + resolved "https://registry.npmjs.org/pascal-case/-/pascal-case-3.1.2.tgz#b48e0ef2b98e205e7c1dae747d0b1508237660eb" + integrity sha512-uWlGT3YSnK9x3BQJaOdcZwrnV6hPpd8jFH1/ucpiLRPh/2zCVJKS19E4GvYHvaCcACn3foXZ0cLB9Wrx1KGe5g== + dependencies: + no-case "^3.0.4" + tslib "^2.0.3" + pascalcase@^0.1.1: version "0.1.1" resolved "https://registry.npmjs.org/pascalcase/-/pascalcase-0.1.1.tgz#b363e55e8006ca6fe21784d2db22bd15d7917f14" @@ -24825,7 +24856,7 @@ tslib@^1.10.0, tslib@^1.11.1, tslib@^1.8.0, tslib@^1.8.1, tslib@^1.9.0, tslib@^1 resolved "https://registry.npmjs.org/tslib/-/tslib-1.14.1.tgz#cf2d38bdc34a134bcaf1091c41f6619e2f672d00" integrity sha512-Xni35NKzjgMrwevysHTCArtLDpPvye8zV/0E4EyYn43P7/7qvQwPh9BGkHewbMulVntbigmcT7rdX3BNo9wRJg== -tslib@^2.0.0, tslib@^2.0.1, tslib@~2.1.0: +tslib@^2.0.0, tslib@^2.0.1, tslib@^2.0.3, tslib@~2.1.0: version "2.1.0" resolved "https://registry.npmjs.org/tslib/-/tslib-2.1.0.tgz#da60860f1c2ecaa5703ab7d39bc05b6bf988b97a" integrity sha512-hcVC3wYEziELGGmEEXue7D75zbwIIVUMWAVbHItGPx0ziyXxrOMQx4rQEVEV45Ut/1IotuEvwqPopzIOkDMf0A== From f358d119107f639b1824306e62244d3f1a639428 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gustaf=20R=C3=A4ntil=C3=A4?= Date: Wed, 17 Feb 2021 10:16:52 +0100 Subject: [PATCH 29/39] Removed unused import --- plugins/pagerduty/src/components/PagerDutyCard.tsx | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/plugins/pagerduty/src/components/PagerDutyCard.tsx b/plugins/pagerduty/src/components/PagerDutyCard.tsx index 5f2ae42aee..7ec425a062 100644 --- a/plugins/pagerduty/src/components/PagerDutyCard.tsx +++ b/plugins/pagerduty/src/components/PagerDutyCard.tsx @@ -17,13 +17,7 @@ import React, { useState, useCallback } from 'react'; import { useApi, Progress, HeaderIconLinkRow } from '@backstage/core'; import { Entity } from '@backstage/catalog-model'; import { useEntity } from '@backstage/plugin-catalog-react'; -import { - Button, - Card, - CardHeader, - Divider, - CardContent, -} from '@material-ui/core'; +import { Card, CardHeader, Divider, CardContent } from '@material-ui/core'; import { Incidents } from './Incident'; import { EscalationPolicy } from './Escalation'; import { useAsync } from 'react-use'; From 010bfcca4388b7b2df4a665d82541e7bc84efed4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gustaf=20R=C3=A4ntil=C3=A4?= Date: Wed, 17 Feb 2021 11:01:16 +0100 Subject: [PATCH 30/39] Added missing dep on @types/react --- plugins/pagerduty/package.json | 1 + yarn.lock | 13 +++++++++++++ 2 files changed, 14 insertions(+) diff --git a/plugins/pagerduty/package.json b/plugins/pagerduty/package.json index c430075b95..b009a90e03 100644 --- a/plugins/pagerduty/package.json +++ b/plugins/pagerduty/package.json @@ -37,6 +37,7 @@ "@material-ui/core": "^4.11.0", "@material-ui/icons": "^4.9.1", "@material-ui/lab": "4.0.0-alpha.45", + "@types/react": "^17.0.2", "classnames": "^2.2.6", "luxon": "1.25.0", "react": "^16.13.1", diff --git a/yarn.lock b/yarn.lock index e2c8103d8e..042bf77d46 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6620,6 +6620,14 @@ dependencies: csstype "^2.2.0" +"@types/react@^17.0.2": + version "17.0.2" + resolved "https://registry.npmjs.org/@types/react/-/react-17.0.2.tgz#3de24c4efef902dd9795a49c75f760cbe4f7a5a8" + integrity sha512-Xt40xQsrkdvjn1EyWe1Bc0dJLcil/9x2vAuW7ya+PuQip4UYUaXyhzWmAbwRsdMgwOFHpfp7/FFZebDU6Y8VHA== + dependencies: + "@types/prop-types" "*" + csstype "^3.0.2" + "@types/reactcss@*": version "1.2.3" resolved "https://registry.npmjs.org/@types/reactcss/-/reactcss-1.2.3.tgz#af28ae11bbb277978b99d04d1eedfd068ca71834" @@ -10695,6 +10703,11 @@ csstype@^2.2.0, csstype@^2.5.2, csstype@^2.5.5, csstype@^2.5.7, csstype@^2.6.5, resolved "https://registry.npmjs.org/csstype/-/csstype-2.6.9.tgz#05141d0cd557a56b8891394c1911c40c8a98d098" integrity sha512-xz39Sb4+OaTsULgUERcCk+TJj8ylkL4aSVDQiX/ksxbELSqwkgt4d4RD7fovIdgJGSuNYqwZEiVjYY5l0ask+Q== +csstype@^3.0.2: + version "3.0.6" + resolved "https://registry.npmjs.org/csstype/-/csstype-3.0.6.tgz#865d0b5833d7d8d40f4e5b8a6d76aea3de4725ef" + integrity sha512-+ZAmfyWMT7TiIlzdqJgjMb7S4f1beorDbWbsocyK4RaiqA5RTX3K14bnBWmmA9QEM0gRdsjyyrEmcyga8Zsxmw== + csv-generate@^3.2.4: version "3.2.4" resolved "https://registry.npmjs.org/csv-generate/-/csv-generate-3.2.4.tgz#440dab9177339ee0676c9e5c16f50e2b3463c019" From b0501f8348f5988f93175ff1ce8ea81eed087dbd Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Wed, 17 Feb 2021 11:08:27 +0100 Subject: [PATCH 31/39] docs(TechDocs): Add more context with AWS docs hyperlinks --- docs/features/techdocs/using-cloud-storage.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/features/techdocs/using-cloud-storage.md b/docs/features/techdocs/using-cloud-storage.md index 7d735c72ab..32c44e9595 100644 --- a/docs/features/techdocs/using-cloud-storage.md +++ b/docs/features/techdocs/using-cloud-storage.md @@ -152,7 +152,7 @@ If the environment variables are set and can be used to access the bucket you created in step 2, they will be used by the AWS SDK V2 Node.js client for authentication. -[Refer to the official documentation.](https://docs.aws.amazon.com/sdk-for-javascript/v2/developer-guide/loading-node-credentials-environment.html) +[Refer to the official documentation for loading credentials in Node.js from environment variables](https://docs.aws.amazon.com/sdk-for-javascript/v2/developer-guide/loading-node-credentials-environment.html). If the environment variables are missing, the AWS SDK tries to read the `~/.aws/credentials` file for credentials. @@ -161,8 +161,8 @@ If the environment variables are missing, the AWS SDK tries to read the If you are using Amazon EC2 instance to deploy Backstage, you do not need to obtain the access keys separately. They can be made available in the environment automatically by defining appropriate IAM role with access to the bucket. Read -more -[here](https://docs.aws.amazon.com/general/latest/gr/aws-access-keys-best-practices.html#use-roles). +more in +[official AWS documentation for using IAM roles.](https://docs.aws.amazon.com/general/latest/gr/aws-access-keys-best-practices.html#use-roles). The AWS Region of the bucket is optional since TechDocs uses AWS SDK V2 and not V3. From db6ff2daa24109843e6ae4a6af2cd0eafacf7ebc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gustaf=20R=C3=A4ntil=C3=A4?= Date: Wed, 17 Feb 2021 12:11:42 +0100 Subject: [PATCH 32/39] @types/react 16 not 17 --- plugins/pagerduty/package.json | 2 +- yarn.lock | 13 ------------- 2 files changed, 1 insertion(+), 14 deletions(-) diff --git a/plugins/pagerduty/package.json b/plugins/pagerduty/package.json index b009a90e03..93a9ce3e4b 100644 --- a/plugins/pagerduty/package.json +++ b/plugins/pagerduty/package.json @@ -37,7 +37,7 @@ "@material-ui/core": "^4.11.0", "@material-ui/icons": "^4.9.1", "@material-ui/lab": "4.0.0-alpha.45", - "@types/react": "^17.0.2", + "@types/react": "^16.9", "classnames": "^2.2.6", "luxon": "1.25.0", "react": "^16.13.1", diff --git a/yarn.lock b/yarn.lock index 042bf77d46..e2c8103d8e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6620,14 +6620,6 @@ dependencies: csstype "^2.2.0" -"@types/react@^17.0.2": - version "17.0.2" - resolved "https://registry.npmjs.org/@types/react/-/react-17.0.2.tgz#3de24c4efef902dd9795a49c75f760cbe4f7a5a8" - integrity sha512-Xt40xQsrkdvjn1EyWe1Bc0dJLcil/9x2vAuW7ya+PuQip4UYUaXyhzWmAbwRsdMgwOFHpfp7/FFZebDU6Y8VHA== - dependencies: - "@types/prop-types" "*" - csstype "^3.0.2" - "@types/reactcss@*": version "1.2.3" resolved "https://registry.npmjs.org/@types/reactcss/-/reactcss-1.2.3.tgz#af28ae11bbb277978b99d04d1eedfd068ca71834" @@ -10703,11 +10695,6 @@ csstype@^2.2.0, csstype@^2.5.2, csstype@^2.5.5, csstype@^2.5.7, csstype@^2.6.5, resolved "https://registry.npmjs.org/csstype/-/csstype-2.6.9.tgz#05141d0cd557a56b8891394c1911c40c8a98d098" integrity sha512-xz39Sb4+OaTsULgUERcCk+TJj8ylkL4aSVDQiX/ksxbELSqwkgt4d4RD7fovIdgJGSuNYqwZEiVjYY5l0ask+Q== -csstype@^3.0.2: - version "3.0.6" - resolved "https://registry.npmjs.org/csstype/-/csstype-3.0.6.tgz#865d0b5833d7d8d40f4e5b8a6d76aea3de4725ef" - integrity sha512-+ZAmfyWMT7TiIlzdqJgjMb7S4f1beorDbWbsocyK4RaiqA5RTX3K14bnBWmmA9QEM0gRdsjyyrEmcyga8Zsxmw== - csv-generate@^3.2.4: version "3.2.4" resolved "https://registry.npmjs.org/csv-generate/-/csv-generate-3.2.4.tgz#440dab9177339ee0676c9e5c16f50e2b3463c019" From 233b78a42ef566aa10df6b6a08397ec7dbe54682 Mon Sep 17 00:00:00 2001 From: ebarrios Date: Wed, 17 Feb 2021 12:23:16 +0100 Subject: [PATCH 33/39] Replace logging erro and return undefined for a throw new Error --- .../src/scaffolder/stages/publish/github.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts b/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts index 4de928c32b..a449eddd5d 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts @@ -64,8 +64,9 @@ export class GithubPublisher implements PublisherBase { }); if (!token) { - logger.error(`No token could be acquired for URL: ${values.storePath}`); - return { remoteUrl: '', catalogInfoUrl: undefined }; + throw new Error( + `No token could be acquired for URL: ${values.storePath}`, + ); } const client = new Octokit({ From 5795f2dc936abd68daababba1ed119d73bf9152e Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Wed, 17 Feb 2021 12:40:29 +0100 Subject: [PATCH 34/39] TechDocs: Pass user and group ID when invoking docker container Without this, files generated by Docker container will be restricted to the root user --- .../src/stages/generate/helpers.test.ts | 29 +++++++++++++++++-- .../src/stages/generate/helpers.ts | 18 ++++++++++++ 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/packages/techdocs-common/src/stages/generate/helpers.test.ts b/packages/techdocs-common/src/stages/generate/helpers.test.ts index c4bf102610..fbca6d1ed2 100644 --- a/packages/techdocs-common/src/stages/generate/helpers.test.ts +++ b/packages/techdocs-common/src/stages/generate/helpers.test.ts @@ -30,6 +30,7 @@ import { patchMkdocsYmlPreBuild, runDockerContainer, storeEtagMetadata, + UserOptions, } from './helpers'; const mockEntity = { @@ -114,7 +115,7 @@ describe('helpers', () => { imageName, args, expect.any(Stream), - { + expect.objectContaining({ Volumes: { '/content': {}, '/result': {}, @@ -123,7 +124,7 @@ describe('helpers', () => { HostConfig: { Binds: [`${docsDir}:/content`, `${outputDir}:/result`], }, - }, + }), ); }); @@ -139,6 +140,30 @@ describe('helpers', () => { expect(mockDocker.ping).toHaveBeenCalled(); }); + it('should pass through the user and group id from the host machine and set the home dir', async () => { + await runDockerContainer({ + imageName, + args, + docsDir, + outputDir, + dockerClient: mockDocker, + }); + + const userOptions: UserOptions = {}; + if (process.getuid && process.getgid) { + userOptions.User = `${process.getuid()}:${process.getgid()}`; + } + + expect(mockDocker.run).toHaveBeenCalledWith( + imageName, + args, + expect.any(Stream), + expect.objectContaining({ + ...userOptions, + }), + ); + }); + describe('where docker is unavailable', () => { const dockerError = 'a docker error'; diff --git a/packages/techdocs-common/src/stages/generate/helpers.ts b/packages/techdocs-common/src/stages/generate/helpers.ts index 32da5d19e8..a88ed13231 100644 --- a/packages/techdocs-common/src/stages/generate/helpers.ts +++ b/packages/techdocs-common/src/stages/generate/helpers.ts @@ -51,6 +51,12 @@ export type RunCommandOptions = { logStream?: Writable; }; +export type UserOptions = { + User?: string; +}; + +// To be replaced by a runDockerContainer from backend-common +// shared between Scaffolder and TechDocs and any other plugin. export async function runDockerContainer({ imageName, args, @@ -78,6 +84,17 @@ export async function runDockerContainer({ }); }); + const userOptions: UserOptions = {}; + // @ts-ignore + if (process.getuid && process.getgid) { + // Files that are created inside the Docker container will be owned by + // root on the host system on non Mac systems, because of reasons. Mainly the fact that + // volume sharing is done using NFS on Mac and actual mounts in Linux world. + // So we set the user in the container as the same user and group id as the host. + // On Windows we don't have process.getuid nor process.getgid + userOptions.User = `${process.getuid()}:${process.getgid()}`; + } + const [{ Error: error, StatusCode: statusCode }] = await dockerClient.run( imageName, args, @@ -91,6 +108,7 @@ export async function runDockerContainer({ HostConfig: { Binds: [`${docsDir}:/content`, `${outputDir}:/result`], }, + ...userOptions, ...createOptions, }, ); From 1743239d034d7763d89c8cc487b265af17472faa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gustaf=20R=C3=A4ntil=C3=A4?= Date: Wed, 17 Feb 2021 13:40:36 +0100 Subject: [PATCH 35/39] Updated unit tests for the new UI --- .../src/components/Incident/IncidentListItem.tsx | 1 + .../src/components/Incident/Incidents.test.tsx | 16 +++++----------- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/plugins/pagerduty/src/components/Incident/IncidentListItem.tsx b/plugins/pagerduty/src/components/Incident/IncidentListItem.tsx index 2d4afa53cd..3d2e4d0edd 100644 --- a/plugins/pagerduty/src/components/Incident/IncidentListItem.tsx +++ b/plugins/pagerduty/src/components/Incident/IncidentListItem.tsx @@ -79,6 +79,7 @@ export const IncidentListItem = ({ incident }: Props) => { primary={ <> { }, ] as Incident[], ); - const { - getByText, - getByTitle, - getAllByTitle, - getByLabelText, - queryByTestId, - } = render( + const { getByText, getAllByTitle, queryByTestId } = render( wrapInTestApp( @@ -102,10 +96,10 @@ describe('Incidents', () => { expect(getByText('title2')).toBeInTheDocument(); expect(getByText('person1')).toBeInTheDocument(); expect(getByText('person2')).toBeInTheDocument(); - expect(getByTitle('triggered')).toBeInTheDocument(); - expect(getByTitle('acknowledged')).toBeInTheDocument(); - expect(getByLabelText('Status error')).toBeInTheDocument(); - expect(getByLabelText('Status warning')).toBeInTheDocument(); + expect(getByText('triggered')).toBeInTheDocument(); + expect(getByText('acknowledged')).toBeInTheDocument(); + expect(queryByTestId('chip-triggered')).toBeInTheDocument(); + expect(queryByTestId('chip-acknowledged')).toBeInTheDocument(); // assert links, mailto and hrefs, date calculation expect(getAllByTitle('View in PagerDuty').length).toEqual(2); From 44414239f2e7c57ab7626c224493d6cefdf7e4f9 Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Wed, 17 Feb 2021 13:49:59 +0100 Subject: [PATCH 36/39] TechDocs: Add changeset about Docker permission fix --- .changeset/techdocs-metal-turkeys-sleep.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/techdocs-metal-turkeys-sleep.md diff --git a/.changeset/techdocs-metal-turkeys-sleep.md b/.changeset/techdocs-metal-turkeys-sleep.md new file mode 100644 index 0000000000..79cf11ce53 --- /dev/null +++ b/.changeset/techdocs-metal-turkeys-sleep.md @@ -0,0 +1,5 @@ +--- +'@backstage/techdocs-common': patch +--- + +Pass user and group ID when invoking docker container. When TechDocs invokes Docker, docker could be run as a `root` user which results in generation of files by applications run by non-root user (e.g. TechDocs) will not have access to modify. This PR passes in current user and group ID to docker so that the file permissions of the generated files and folders are correct. From e308a89ade8e129fb81e34ef18d9b55cc95e689c Mon Sep 17 00:00:00 2001 From: blam Date: Wed, 17 Feb 2021 13:53:44 +0100 Subject: [PATCH 37/39] docs: fixing custom implementations of utitiy apis --- docs/api/utility-apis.md | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/docs/api/utility-apis.md b/docs/api/utility-apis.md index 5b5656f1b1..c039f33bf5 100644 --- a/docs/api/utility-apis.md +++ b/docs/api/utility-apis.md @@ -181,7 +181,25 @@ The `IgnoringErrorApi` would then be imported in the app, and wired up like this: ```ts -builder.add(errorApiRef, new IgnoringErrorApi()); +const app = createApp({ + apis: [ + /* ApiFactories */ + createApiFactory(errorApiRef, new IgnoringErrorApi()) + + // OR + // if you have dependencies inside your additional API you can use the object form + createApiFactory({ + api: errorApiRef, + deps: { configApi: configApiRef }, + factory({ configApi }) { + return new IgnoringErrorApi({ + reportingUrl: configApi.getString('error.reportingUrl'), + }); + }, + }), + ], + // ... other options +}); ``` Note that the above line will cause an error if `IgnoreErrorApi` does not fully From 8425f6d10fc8825c68db821cfa36d0d7f7f6ba86 Mon Sep 17 00:00:00 2001 From: blam Date: Wed, 17 Feb 2021 13:56:29 +0100 Subject: [PATCH 38/39] chore: fixing syntax --- docs/api/utility-apis.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/api/utility-apis.md b/docs/api/utility-apis.md index c039f33bf5..904848b664 100644 --- a/docs/api/utility-apis.md +++ b/docs/api/utility-apis.md @@ -184,7 +184,7 @@ this: const app = createApp({ apis: [ /* ApiFactories */ - createApiFactory(errorApiRef, new IgnoringErrorApi()) + createApiFactory(errorApiRef, new IgnoringErrorApi()), // OR // if you have dependencies inside your additional API you can use the object form From 758bde62663891101bf087c50ecb51946cd8ed29 Mon Sep 17 00:00:00 2001 From: blam Date: Wed, 17 Feb 2021 13:57:32 +0100 Subject: [PATCH 39/39] chore: fix code review --- docs/api/utility-apis.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/api/utility-apis.md b/docs/api/utility-apis.md index 904848b664..308544e0f1 100644 --- a/docs/api/utility-apis.md +++ b/docs/api/utility-apis.md @@ -187,7 +187,7 @@ const app = createApp({ createApiFactory(errorApiRef, new IgnoringErrorApi()), // OR - // if you have dependencies inside your additional API you can use the object form + // If your API has dependencies, you use the object form createApiFactory({ api: errorApiRef, deps: { configApi: configApiRef },