From 09a37042643341a4f6f001554748e438573fb67f Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 30 Dec 2020 13:52:48 +0100 Subject: [PATCH 01/75] backend-common: remove deprecated HTTPS config --- .changeset/wise-mice-invite.md | 5 + packages/backend-common/config.d.ts | 35 ++---- .../backend-common/src/service/lib/config.ts | 36 +----- .../src/service/lib/hostFactory.ts | 117 +++++++++--------- 4 files changed, 82 insertions(+), 111 deletions(-) create mode 100644 .changeset/wise-mice-invite.md diff --git a/.changeset/wise-mice-invite.md b/.changeset/wise-mice-invite.md new file mode 100644 index 0000000000..9021336c07 --- /dev/null +++ b/.changeset/wise-mice-invite.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-common': minor +--- + +Remove support for HTTPS certificate generation parameters. Use `backend.https = true` instead. diff --git a/packages/backend-common/config.d.ts b/packages/backend-common/config.d.ts index b7241bcc03..96dd71d41b 100644 --- a/packages/backend-common/config.d.ts +++ b/packages/backend-common/config.d.ts @@ -41,31 +41,16 @@ export interface Config { https?: | true | { - /** - * Certificate configuration or parameters for generating a self-signed certificate - * - * Setting parameters for self-signed certificates is deprecated and will be removed in - * the future, set `backend.https = true` instead. - */ - certificate?: - | { - /** Algorithm to use to generate a self-signed certificate */ - algorithm?: string; - keySize?: number; - days?: number; - attributes: { - commonName: string; - }; - } - | { - /** PEM encoded certificate. Use $file to load in a file */ - cert: string; - /** - * PEM encoded certificate key. Use $file to load in a file. - * @visibility secret - */ - key: string; - }; + /** Certificate configuration */ + certificate?: { + /** PEM encoded certificate. Use $file to load in a file */ + cert: string; + /** + * PEM encoded certificate key. Use $file to load in a file. + * @visibility secret + */ + key: string; + }; }; /** Database connection configuration, select database type using the `client` field */ diff --git a/packages/backend-common/src/service/lib/config.ts b/packages/backend-common/src/service/lib/config.ts index 6abea97454..5d9d12658d 100644 --- a/packages/backend-common/src/service/lib/config.ts +++ b/packages/backend-common/src/service/lib/config.ts @@ -22,23 +22,8 @@ export type BaseOptions = { listenHost?: string; }; -export type CertificateOptions = { - key?: CertificateKeyOptions; - attributes?: CertificateAttributeOptions; -}; - -export type CertificateKeyOptions = { - size?: number; - algorithm?: string; - days?: number; -}; - -export type CertificateAttributeOptions = { - commonName?: string; -}; - export type HttpsSettings = { - certificate: CertificateSigningOptions | CertificateReferenceOptions; + certificate: CertificateGenerationOptions | CertificateReferenceOptions; }; export type CertificateReferenceOptions = { @@ -46,11 +31,8 @@ export type CertificateReferenceOptions = { cert: string; }; -export type CertificateSigningOptions = { - algorithm?: string; - size?: number; - days?: number; - attributes: CertificateAttributes; +export type CertificateGenerationOptions = { + hostname: string; }; export type CertificateAttributes = { @@ -196,20 +178,14 @@ export function readHttpsSettings(config: Config): HttpsSettings | undefined { const https = config.get('https'); if (https === true) { const baseUrl = config.getString('baseUrl'); - let commonName; + let hostname; try { - commonName = new URL(baseUrl).hostname; + hostname = new URL(baseUrl).hostname; } catch (error) { throw new Error(`Invalid backend.baseUrl "${baseUrl}"`); } - return { - certificate: { - attributes: { - commonName, - }, - }, - }; + return { certificate: { hostname } }; } const cc = config.getOptionalConfig('https'); diff --git a/packages/backend-common/src/service/lib/hostFactory.ts b/packages/backend-common/src/service/lib/hostFactory.ts index 656f160c31..db202a84ab 100644 --- a/packages/backend-common/src/service/lib/hostFactory.ts +++ b/packages/backend-common/src/service/lib/hostFactory.ts @@ -20,10 +20,12 @@ import express from 'express'; import * as http from 'http'; import * as https from 'https'; import { Logger } from 'winston'; -import { CertificateSigningOptions, HttpsSettings } from './config'; +import { HttpsSettings } from './config'; const ALMOST_MONTH_IN_MS = 25 * 24 * 60 * 60 * 1000; +const IP_HOSTNAME_REGEX = /:|^\d+\.\d+\.\d+\.\d+$/; + /** * Creates a Http server instance based on an Express application. * @@ -59,17 +61,17 @@ export async function createHttpsServer( let credentials: { key: string | Buffer; cert: string | Buffer }; - const signingOptions: any = httpsSettings?.certificate; - - // TODO(Rugvip): remove support for generated certificate params and make this a more straightforward check - if (signingOptions?.attributes) { - credentials = await getGeneratedCertificate(signingOptions, logger); + if ('hostname' in httpsSettings?.certificate) { + credentials = await getGeneratedCertificate( + httpsSettings.certificate.hostname, + logger, + ); } else { logger?.info('Loading certificate from config'); credentials = { - key: signingOptions?.key, - cert: signingOptions?.cert, + key: httpsSettings?.certificate?.key, + cert: httpsSettings?.certificate?.cert, }; } @@ -80,16 +82,7 @@ export async function createHttpsServer( return https.createServer(credentials, app) as http.Server; } -async function getGeneratedCertificate( - options: CertificateSigningOptions, - logger?: Logger, -) { - if (options?.algorithm) { - logger?.warn( - 'Certificate generation configuration with parameters in backend.https.certificate is deprecated, set backend.https = true instead', - ); - } - +async function getGeneratedCertificate(hostname: string, logger?: Logger) { const hasModules = await fs.pathExists('node_modules'); let certPath; if (hasModules) { @@ -119,20 +112,61 @@ async function getGeneratedCertificate( } logger?.info('Generating new self-signed certificate'); - const newCert = await createCertificate(options); + const newCert = await createCertificate(hostname); await fs.writeFile(certPath, newCert.cert + newCert.key, 'utf8'); return newCert; } -async function createCertificate(options: CertificateSigningOptions) { - const attributes: Array = Object.entries( - options.attributes, - ).map(([name, value]) => ({ name, value })); +async function createCertificate(hostname: string) { + const attributes = [ + { + name: 'commonName', + value: 'dev-cert', + }, + ]; + + const sans = [ + { + type: 2, // DNS + value: 'localhost', + }, + { + type: 2, + value: 'localhost.localdomain', + }, + { + type: 2, + value: '[::1]', + }, + { + type: 7, // IP + ip: '127.0.0.1', + }, + { + type: 7, + ip: 'fe80::1', + }, + ]; + + // Add hostname from backend.baseUrl if it doesn't already exist in our list of SANs + if (!sans.find(({ value, ip }) => value === hostname || ip === hostname)) { + sans.push( + IP_HOSTNAME_REGEX.test(hostname) + ? { + type: 7, + ip: hostname, + } + : { + type: 2, + value: hostname, + }, + ); + } const params = { - algorithm: options?.algorithm || 'sha256', - keySize: options?.size || 2048, - days: options?.days || 30, + algorithm: 'sha256', + keySize: 2048, + days: 30, extensions: [ { name: 'keyUsage', @@ -151,36 +185,7 @@ async function createCertificate(options: CertificateSigningOptions) { }, { name: 'subjectAltName', - altNames: [ - { - type: 2, // DNS - value: 'localhost', - }, - { - type: 2, - value: 'localhost.localdomain', - }, - { - type: 2, - value: '[::1]', - }, - { - type: 7, // IP - ip: '127.0.0.1', - }, - { - type: 7, - ip: 'fe80::1', - }, - ...(options.attributes.commonName - ? [ - { - type: 2, // DNS - value: options.attributes.commonName, - }, - ] - : []), - ], + altNames: sans, }, ], }; From bd18e6528ef5824eaacf74aec38155d3e9dcfb36 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 15 Jan 2021 04:59:23 +0000 Subject: [PATCH 02/75] chore(deps): bump @rjsf/material-ui from 2.4.0 to 2.4.1 Bumps [@rjsf/material-ui](https://github.com/rjsf-team/react-jsonschema-form) from 2.4.0 to 2.4.1. - [Release notes](https://github.com/rjsf-team/react-jsonschema-form/releases) - [Commits](https://github.com/rjsf-team/react-jsonschema-form/compare/v2.4.0...v2.4.1) Signed-off-by: dependabot[bot] --- yarn.lock | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/yarn.lock b/yarn.lock index 955b6305c3..9abad80835 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2436,7 +2436,7 @@ to-fast-properties "^2.0.0" "@backstage/catalog-model@^0.2.0": - version "0.6.0" + version "0.6.1" dependencies: "@backstage/config" "^0.1.2" "@types/json-schema" "^7.0.5" @@ -2447,7 +2447,7 @@ yup "^0.29.3" "@backstage/catalog-model@^0.3.0": - version "0.6.0" + version "0.6.1" dependencies: "@backstage/config" "^0.1.2" "@types/json-schema" "^7.0.5" @@ -2458,7 +2458,7 @@ yup "^0.29.3" "@backstage/core@^0.3.0": - version "0.4.3" + version "0.4.4" dependencies: "@backstage/config" "^0.1.2" "@backstage/core-api" "^0.2.8" @@ -5024,9 +5024,9 @@ shortid "^2.2.14" "@rjsf/material-ui@^2.4.0": - version "2.4.0" - resolved "https://registry.npmjs.org/@rjsf/material-ui/-/material-ui-2.4.0.tgz#1b5859298bf3f61137d7b05084f058a775d6fd73" - integrity sha512-U8F/suzg4MuV+8mK1/ufs0Y6c3O8hc1wnuD2IKoOVJvegGfz5JCafyoyGAW6iyuT1DZBMPzVWEqfiuYPmoE7pw== + version "2.4.1" + resolved "https://registry.npmjs.org/@rjsf/material-ui/-/material-ui-2.4.1.tgz#b0dedff8877b114147e298966ca3faba895a7a62" + integrity sha512-pZaWL5Dw+km8S0hFLIK1lRHaeNtheMxTF2mZrWhf6HlGWCTGkQJnXta2UgshJN/nKtZPgO1L4FKz42Eun9nnhg== "@roadiehq/backstage-plugin-buildkite@^0.1.3": version "0.1.3" From 8d950d0ae7bbbce10a413f23262888b6195997ff Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Fri, 15 Jan 2021 18:02:37 +0100 Subject: [PATCH 03/75] Stream files directly from GCS instead of buffering and sending. --- .../src/stages/publish/googleStorage.ts | 25 +++++++------------ 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/packages/techdocs-common/src/stages/publish/googleStorage.ts b/packages/techdocs-common/src/stages/publish/googleStorage.ts index e6b2c26943..f92b905f73 100644 --- a/packages/techdocs-common/src/stages/publish/googleStorage.ts +++ b/packages/techdocs-common/src/stages/publish/googleStorage.ts @@ -169,30 +169,23 @@ export class GoogleGCSPublish implements PublisherBase { // Files with different extensions (CSS, HTML) need to be served with different headers const fileExtension = path.extname(filePath); const responseHeaders = getHeadersForFileExtension(fileExtension); + res.writeHead(200, { + ...{ + 'Transfer-Encoding': 'chunked', + }, + ...responseHeaders, + }); - const fileStreamChunks: Array = []; + // Pipe file chunks directly from storage to client. this.storageClient .bucket(this.bucketName) .file(filePath) .createReadStream() .on('error', err => { this.logger.warn(err.message); - res.status(404).send(err.message); + res.destroy(err); }) - .on('data', chunk => { - fileStreamChunks.push(chunk); - }) - .on('end', () => { - const fileContent = Buffer.concat(fileStreamChunks); - // Inject response headers - for (const [headerKey, headerValue] of Object.entries( - responseHeaders, - )) { - res.setHeader(headerKey, headerValue); - } - - res.send(fileContent); - }); + .pipe(res); }; } From ce9d5ac220d9a22dab46d0d97e278fe3a0c5f0a7 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Fri, 15 Jan 2021 19:17:01 +0100 Subject: [PATCH 04/75] Remove unnecessary chunk transfer encoding header. --- .../techdocs-common/src/stages/publish/googleStorage.ts | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/packages/techdocs-common/src/stages/publish/googleStorage.ts b/packages/techdocs-common/src/stages/publish/googleStorage.ts index f92b905f73..df9d1120bf 100644 --- a/packages/techdocs-common/src/stages/publish/googleStorage.ts +++ b/packages/techdocs-common/src/stages/publish/googleStorage.ts @@ -169,12 +169,7 @@ export class GoogleGCSPublish implements PublisherBase { // Files with different extensions (CSS, HTML) need to be served with different headers const fileExtension = path.extname(filePath); const responseHeaders = getHeadersForFileExtension(fileExtension); - res.writeHead(200, { - ...{ - 'Transfer-Encoding': 'chunked', - }, - ...responseHeaders, - }); + res.writeHead(200, responseHeaders); // Pipe file chunks directly from storage to client. this.storageClient From bba8b99255de6c3a3c9e67932a6b212cec0356db Mon Sep 17 00:00:00 2001 From: Alan Crosswell Date: Fri, 15 Jan 2021 16:25:06 -0500 Subject: [PATCH 05/75] document using locally-installed cookiecutter This avoids running Docker in Docker. See https://github.com/backstage/backstage/blob/57460cda021bddaf04e7c3351c3881a3f24859b0/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.ts#L56-L79 See also https://backstage.io/docs/features/techdocs/getting-started#disabling-docker-in-docker-situation-optional where it is configurable. --- .../extending/create-your-own-templater.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/features/software-templates/extending/create-your-own-templater.md b/docs/features/software-templates/extending/create-your-own-templater.md index 63acd38286..28eb980bff 100644 --- a/docs/features/software-templates/extending/create-your-own-templater.md +++ b/docs/features/software-templates/extending/create-your-own-templater.md @@ -87,9 +87,10 @@ follows: _note_ Currently the templaters that we provide are basically Docker action containers that are run on top of the skeleton folder. This keeps dependencies to a minimal for running backstage scaffolder, but you don't /have/ to use -Docker. You could create your own templater that spins up an EC2 instance and +Docker. You can `pip install cookiecutter` to run it locally in your backend. +You could create your own templater that spins up an EC2 instance and downloads the folder and does everything using an AMI if you want. It's entirely -up to you! +up to you! Now it's up to you to implement the `run` function, and then return a `TemplaterRunResult` which is `{ resultDir: string }`. From 614ee0d1db49b4c7549dbe61d99aa5a5b4712e1f Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 18 Jan 2021 05:02:30 +0000 Subject: [PATCH 06/75] chore(deps-dev): bump @storybook/react from 6.1.11 to 6.1.14 Bumps [@storybook/react](https://github.com/storybookjs/storybook/tree/HEAD/app/react) from 6.1.11 to 6.1.14. - [Release notes](https://github.com/storybookjs/storybook/releases) - [Changelog](https://github.com/storybookjs/storybook/blob/next/CHANGELOG.md) - [Commits](https://github.com/storybookjs/storybook/commits/v6.1.14/app/react) Signed-off-by: dependabot[bot] --- yarn.lock | 288 +++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 199 insertions(+), 89 deletions(-) diff --git a/yarn.lock b/yarn.lock index 7fdc2c3a62..4f10347ff1 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1989,20 +1989,6 @@ "@babel/helper-plugin-utils" "^7.10.4" "@babel/plugin-syntax-jsx" "^7.12.1" -"@babel/plugin-transform-react-jsx-self@^7.12.1": - version "7.12.1" - resolved "https://registry.npmjs.org/@babel/plugin-transform-react-jsx-self/-/plugin-transform-react-jsx-self-7.12.1.tgz#ef43cbca2a14f1bd17807dbe4376ff89d714cf28" - integrity sha512-FbpL0ieNWiiBB5tCldX17EtXgmzeEZjFrix72rQYeq9X6nUK38HCaxexzVQrZWXanxKJPKVVIU37gFjEQYkPkA== - dependencies: - "@babel/helper-plugin-utils" "^7.10.4" - -"@babel/plugin-transform-react-jsx-source@^7.12.1": - version "7.12.1" - resolved "https://registry.npmjs.org/@babel/plugin-transform-react-jsx-source/-/plugin-transform-react-jsx-source-7.12.1.tgz#d07de6863f468da0809edcf79a1aa8ce2a82a26b" - integrity sha512-keQ5kBfjJNRc6zZN1/nVHCd6LLIHq4aUKcVnvE/2l+ZZROSbqoiGFRtT5t3Is89XJxBQaP7NLZX2jgGHdZvvFQ== - dependencies: - "@babel/helper-plugin-utils" "^7.10.4" - "@babel/plugin-transform-react-jsx@^7.0.0": version "7.10.4" resolved "https://registry.npmjs.org/@babel/plugin-transform-react-jsx/-/plugin-transform-react-jsx-7.10.4.tgz#673c9f913948764a4421683b2bef2936968fddf2" @@ -2023,16 +2009,6 @@ "@babel/helper-plugin-utils" "^7.10.4" "@babel/plugin-syntax-jsx" "^7.12.1" -"@babel/plugin-transform-react-jsx@^7.12.7": - version "7.12.7" - resolved "https://registry.npmjs.org/@babel/plugin-transform-react-jsx/-/plugin-transform-react-jsx-7.12.7.tgz#8b14d45f6eccd41b7f924bcb65c021e9f0a06f7f" - integrity sha512-YFlTi6MEsclFAPIDNZYiCRbneg1MFGao9pPG9uD5htwE0vDbPaMUMeYd6itWjw7K4kro4UbdQf3ljmFl9y48dQ== - dependencies: - "@babel/helper-builder-react-jsx" "^7.10.4" - "@babel/helper-builder-react-jsx-experimental" "^7.12.4" - "@babel/helper-plugin-utils" "^7.10.4" - "@babel/plugin-syntax-jsx" "^7.12.1" - "@babel/plugin-transform-react-pure-annotations@^7.12.1": version "7.12.1" resolved "https://registry.npmjs.org/@babel/plugin-transform-react-pure-annotations/-/plugin-transform-react-pure-annotations-7.12.1.tgz#05d46f0ab4d1339ac59adf20a1462c91b37a1a42" @@ -2229,7 +2205,7 @@ "@babel/types" "^7.4.4" esutils "^2.0.2" -"@babel/preset-react@^7.12.1": +"@babel/preset-react@^7.12.1", "@babel/preset-react@^7.12.5", "@babel/preset-react@^7.9.4": version "7.12.10" resolved "https://registry.npmjs.org/@babel/preset-react/-/preset-react-7.12.10.tgz#4fed65f296cbb0f5fb09de6be8cddc85cc909be9" integrity sha512-vtQNjaHRl4DUpp+t+g4wvTHsLQuye+n0H/wsXIZRn69oz/fvNC7gQ4IK73zGJBaxvHoxElDvnYCthMcT7uzFoQ== @@ -2240,19 +2216,6 @@ "@babel/plugin-transform-react-jsx-development" "^7.12.7" "@babel/plugin-transform-react-pure-annotations" "^7.12.1" -"@babel/preset-react@^7.12.5", "@babel/preset-react@^7.9.4": - version "7.12.7" - resolved "https://registry.npmjs.org/@babel/preset-react/-/preset-react-7.12.7.tgz#36d61d83223b07b6ac4ec55cf016abb0f70be83b" - integrity sha512-wKeTdnGUP5AEYCYQIMeXMMwU7j+2opxrG0WzuZfxuuW9nhKvvALBjl67653CWamZJVefuJGI219G591RSldrqQ== - dependencies: - "@babel/helper-plugin-utils" "^7.10.4" - "@babel/plugin-transform-react-display-name" "^7.12.1" - "@babel/plugin-transform-react-jsx" "^7.12.7" - "@babel/plugin-transform-react-jsx-development" "^7.12.7" - "@babel/plugin-transform-react-jsx-self" "^7.12.1" - "@babel/plugin-transform-react-jsx-source" "^7.12.1" - "@babel/plugin-transform-react-pure-annotations" "^7.12.1" - "@babel/preset-typescript@^7.12.1": version "7.12.7" resolved "https://registry.npmjs.org/@babel/preset-typescript/-/preset-typescript-7.12.7.tgz#fc7df8199d6aae747896f1e6c61fc872056632a3" @@ -5292,7 +5255,7 @@ react-syntax-highlighter "^13.5.0" regenerator-runtime "^0.13.7" -"@storybook/addons@6.1.11", "@storybook/addons@^6.1.11": +"@storybook/addons@6.1.11": version "6.1.11" resolved "https://registry.npmjs.org/@storybook/addons/-/addons-6.1.11.tgz#cb4578411ca00ccb206b484df5a171ccaca34719" integrity sha512-OZXsdmn60dVe482l9zWxzOqqJApD2jggk/8QJKn3/Ub9posmqdqg712bW6v71BBe0UXXG/QfkZA7gcyiyEENbw== @@ -5307,6 +5270,21 @@ global "^4.3.2" regenerator-runtime "^0.13.7" +"@storybook/addons@6.1.14", "@storybook/addons@^6.1.11": + version "6.1.14" + resolved "https://registry.npmjs.org/@storybook/addons/-/addons-6.1.14.tgz#2b81304bbe696923df95cdcf85cfc592d10f4065" + integrity sha512-HlpmV7aejp/MeW8bo/WKME3i71gi0men9qcwoovjDjnSF6jXoNLT336a5udKXdHqYSZgzdyURlgLtilCWkWaJQ== + dependencies: + "@storybook/api" "6.1.14" + "@storybook/channels" "6.1.14" + "@storybook/client-logger" "6.1.14" + "@storybook/core-events" "6.1.14" + "@storybook/router" "6.1.14" + "@storybook/theming" "6.1.14" + core-js "^3.0.1" + global "^4.3.2" + regenerator-runtime "^0.13.7" + "@storybook/api@6.1.11": version "6.1.11" resolved "https://registry.npmjs.org/@storybook/api/-/api-6.1.11.tgz#1e0b798203df823ac21184386258cf8b5f17f440" @@ -5332,6 +5310,31 @@ ts-dedent "^2.0.0" util-deprecate "^1.0.2" +"@storybook/api@6.1.14": + version "6.1.14" + resolved "https://registry.npmjs.org/@storybook/api/-/api-6.1.14.tgz#20035dd336aba1c5a0f8c83c8c14a2edaf4db891" + integrity sha512-gWcC/xEW8HL5DsocLujHBUdoNsl4YW1Zx1Y4SBbLCyrhj8v4JudJpylwJpOUBDe/GESXq1zqvNKvUPtI8DQNyw== + dependencies: + "@reach/router" "^1.3.3" + "@storybook/channels" "6.1.14" + "@storybook/client-logger" "6.1.14" + "@storybook/core-events" "6.1.14" + "@storybook/csf" "0.0.1" + "@storybook/router" "6.1.14" + "@storybook/semver" "^7.3.2" + "@storybook/theming" "6.1.14" + "@types/reach__router" "^1.3.5" + core-js "^3.0.1" + fast-deep-equal "^3.1.1" + global "^4.3.2" + lodash "^4.17.15" + memoizerific "^1.11.3" + regenerator-runtime "^0.13.7" + store2 "^2.7.1" + telejson "^5.0.2" + ts-dedent "^2.0.0" + util-deprecate "^1.0.2" + "@storybook/channel-postmessage@6.1.11": version "6.1.11" resolved "https://registry.npmjs.org/@storybook/channel-postmessage/-/channel-postmessage-6.1.11.tgz#62c1079f04870dd27925bd538a2020e7380daa2e" @@ -5345,6 +5348,19 @@ qs "^6.6.0" telejson "^5.0.2" +"@storybook/channel-postmessage@6.1.14": + version "6.1.14" + resolved "https://registry.npmjs.org/@storybook/channel-postmessage/-/channel-postmessage-6.1.14.tgz#41f3115895010dad9fb30f4ac381e4f904b1e50c" + integrity sha512-If83dXXW9mKIRuvuWhWa/zkEw/F0FDgikp33x8436J3rWCh3recp27kffFRrKG0YDMpFSk/Ci5G47E9zn9SCjw== + dependencies: + "@storybook/channels" "6.1.14" + "@storybook/client-logger" "6.1.14" + "@storybook/core-events" "6.1.14" + core-js "^3.0.1" + global "^4.3.2" + qs "^6.6.0" + telejson "^5.0.2" + "@storybook/channels@6.1.11": version "6.1.11" resolved "https://registry.npmjs.org/@storybook/channels/-/channels-6.1.11.tgz#a93a83746ad78dd40e1c056029f6d93b17bb66bc" @@ -5354,6 +5370,15 @@ ts-dedent "^2.0.0" util-deprecate "^1.0.2" +"@storybook/channels@6.1.14": + version "6.1.14" + resolved "https://registry.npmjs.org/@storybook/channels/-/channels-6.1.14.tgz#c479190ebb853a603f3ed90fc470534a02eb46eb" + integrity sha512-vP19IB2FXj8SiFbQ9ETljEBienL+KRMLgMzz3Ta3nZj/OfjJJbIuj42ZfexQGV4mS0Bo+OW+qT7VMIY6fulnFw== + dependencies: + core-js "^3.0.1" + ts-dedent "^2.0.0" + util-deprecate "^1.0.2" + "@storybook/client-api@6.1.11": version "6.1.11" resolved "https://registry.npmjs.org/@storybook/client-api/-/client-api-6.1.11.tgz#d25aac484ca84a1acb01d450e756a62408f00c1a" @@ -5378,6 +5403,30 @@ ts-dedent "^2.0.0" util-deprecate "^1.0.2" +"@storybook/client-api@6.1.14": + version "6.1.14" + resolved "https://registry.npmjs.org/@storybook/client-api/-/client-api-6.1.14.tgz#6daf56743cc72e13f05fff3d2ac554897cc9f9fd" + integrity sha512-pIDSlS48bhJdtgNg7sXV1NmLJtB0ebRHJI9htIiqtL7EGQenb4+Bbwflhj1j51OEkuM+bQsAAZxq5deiUQEGVw== + dependencies: + "@storybook/addons" "6.1.14" + "@storybook/channel-postmessage" "6.1.14" + "@storybook/channels" "6.1.14" + "@storybook/client-logger" "6.1.14" + "@storybook/core-events" "6.1.14" + "@storybook/csf" "0.0.1" + "@types/qs" "^6.9.0" + "@types/webpack-env" "^1.15.3" + core-js "^3.0.1" + global "^4.3.2" + lodash "^4.17.15" + memoizerific "^1.11.3" + qs "^6.6.0" + regenerator-runtime "^0.13.7" + stable "^0.1.8" + store2 "^2.7.1" + ts-dedent "^2.0.0" + util-deprecate "^1.0.2" + "@storybook/client-logger@6.1.11": version "6.1.11" resolved "https://registry.npmjs.org/@storybook/client-logger/-/client-logger-6.1.11.tgz#5dd092e4293e5f58f7e89ddbc6eb2511b7d60954" @@ -5386,6 +5435,14 @@ core-js "^3.0.1" global "^4.3.2" +"@storybook/client-logger@6.1.14": + version "6.1.14" + resolved "https://registry.npmjs.org/@storybook/client-logger/-/client-logger-6.1.14.tgz#216b9c1332ffa3a3473dad837780a3b14f686bae" + integrity sha512-NSO8nVsp6o0eoQ1Drlu66KXpl6DPuq02Kj8AhttGzvqSYB50SV4CV+wceBcg77tIVu5QmQ+71hAEVXhx7sjRHA== + dependencies: + core-js "^3.0.1" + global "^4.3.2" + "@storybook/components@6.1.11": version "6.1.11" resolved "https://registry.npmjs.org/@storybook/components/-/components-6.1.11.tgz#edd5db7fe43f47b5a7ab515840795a89d931512e" @@ -5412,6 +5469,32 @@ react-textarea-autosize "^8.1.1" ts-dedent "^2.0.0" +"@storybook/components@6.1.14": + version "6.1.14" + resolved "https://registry.npmjs.org/@storybook/components/-/components-6.1.14.tgz#4ea47edfa0a3e4a26882aa5a1eb90c1ec86e6f71" + integrity sha512-Nxsp/9o1tqfY8s6RBWNHyM03A5D9k56Kr/4VNa++CbDrz1+TIxpYlDgS4sllUlXyvICLfk3sUtg3KS5CPl2iZA== + dependencies: + "@popperjs/core" "^2.5.4" + "@storybook/client-logger" "6.1.14" + "@storybook/csf" "0.0.1" + "@storybook/theming" "6.1.14" + "@types/overlayscrollbars" "^1.9.0" + "@types/react-color" "^3.0.1" + "@types/react-syntax-highlighter" "11.0.4" + core-js "^3.0.1" + fast-deep-equal "^3.1.1" + global "^4.3.2" + lodash "^4.17.15" + markdown-to-jsx "^6.11.4" + memoizerific "^1.11.3" + overlayscrollbars "^1.10.2" + polished "^3.4.4" + react-color "^2.17.0" + react-popper-tooltip "^3.1.1" + react-syntax-highlighter "^13.5.0" + react-textarea-autosize "^8.1.1" + ts-dedent "^2.0.0" + "@storybook/core-events@6.1.11": version "6.1.11" resolved "https://registry.npmjs.org/@storybook/core-events/-/core-events-6.1.11.tgz#d50e8ec90490f9a7180a8c8a83afb6dcfe47ed66" @@ -5419,10 +5502,17 @@ dependencies: core-js "^3.0.1" -"@storybook/core@6.1.11": - version "6.1.11" - resolved "https://registry.npmjs.org/@storybook/core/-/core-6.1.11.tgz#ed9d3b513794c604ab11180f6a014924b871179e" - integrity sha512-pYOOQwiNJ5myLRn6p6nnLUjjjISHK/N55vS4HFnETYSaRLA++h1coN1jk7Zwt89dOQTdF0EsTJn+6snYOC+lxQ== +"@storybook/core-events@6.1.14": + version "6.1.14" + resolved "https://registry.npmjs.org/@storybook/core-events/-/core-events-6.1.14.tgz#a3165e32cefd6be7326bbad4b8140653bdfa0426" + integrity sha512-tpM3VDvzqgRY7S17CRglgt1625rxNoyEwrLQiNcZkUPyO0rpaacPqVEbPCtcTmUeboI1bLdnSQIjT9B0/Y2Pww== + dependencies: + core-js "^3.0.1" + +"@storybook/core@6.1.14": + version "6.1.14" + resolved "https://registry.npmjs.org/@storybook/core/-/core-6.1.14.tgz#17e724a5b94d6e1bb557e213b8176660d2d14762" + integrity sha512-lHKZmfLAo2VGtF/yrZkkWMYgmFRNKbzIDxYJGp8USyUQyTfEpz2qqJlBdoD6rxr1hFPM2954tIKwh8iPhT2PFQ== dependencies: "@babel/core" "^7.12.3" "@babel/plugin-proposal-class-properties" "^7.12.1" @@ -5446,20 +5536,20 @@ "@babel/preset-react" "^7.12.1" "@babel/preset-typescript" "^7.12.1" "@babel/register" "^7.12.1" - "@storybook/addons" "6.1.11" - "@storybook/api" "6.1.11" - "@storybook/channel-postmessage" "6.1.11" - "@storybook/channels" "6.1.11" - "@storybook/client-api" "6.1.11" - "@storybook/client-logger" "6.1.11" - "@storybook/components" "6.1.11" - "@storybook/core-events" "6.1.11" + "@storybook/addons" "6.1.14" + "@storybook/api" "6.1.14" + "@storybook/channel-postmessage" "6.1.14" + "@storybook/channels" "6.1.14" + "@storybook/client-api" "6.1.14" + "@storybook/client-logger" "6.1.14" + "@storybook/components" "6.1.14" + "@storybook/core-events" "6.1.14" "@storybook/csf" "0.0.1" - "@storybook/node-logger" "6.1.11" - "@storybook/router" "6.1.11" + "@storybook/node-logger" "6.1.14" + "@storybook/router" "6.1.14" "@storybook/semver" "^7.3.2" - "@storybook/theming" "6.1.11" - "@storybook/ui" "6.1.11" + "@storybook/theming" "6.1.14" + "@storybook/ui" "6.1.14" "@types/glob-base" "^0.3.0" "@types/micromatch" "^4.0.1" "@types/node-fetch" "^2.5.4" @@ -5533,10 +5623,10 @@ dependencies: lodash "^4.17.15" -"@storybook/node-logger@6.1.11": - version "6.1.11" - resolved "https://registry.npmjs.org/@storybook/node-logger/-/node-logger-6.1.11.tgz#8e0d058b4804f2fea03c9d7d331b8e2d02f3b7ff" - integrity sha512-MASonXDWpSMU9HF9mqbGOR1Ps/DTJ8AVmYD50+OnB9kXl4M42Dliobeq7JwKFMnZ42RelUCCSXdWW80hGrUKKA== +"@storybook/node-logger@6.1.14": + version "6.1.14" + resolved "https://registry.npmjs.org/@storybook/node-logger/-/node-logger-6.1.14.tgz#e5294f986e3ec5c67b2738895b9d16c9a2b667fa" + integrity sha512-3jrw7coAwFXZu4qK1vm54bCPhNRvxjG+7jISbhhocDoNIv0nLWL3+tJyrC5/k/XHQiUlLkhEzpMaASADmkttNw== dependencies: "@types/npmlog" "^4.1.2" chalk "^4.0.0" @@ -5545,16 +5635,16 @@ pretty-hrtime "^1.0.3" "@storybook/react@^6.1.11": - version "6.1.11" - resolved "https://registry.npmjs.org/@storybook/react/-/react-6.1.11.tgz#e94403cd878c66b445df993bad9bec9023db3ebe" - integrity sha512-EmR7yvVW6z6AYhfzAgJMGR/5+igeBGa1EePaEIibn51r5uboSB72N12NaADyF2OaycIdV+0sW6vP9Zvlvexa/w== + version "6.1.14" + resolved "https://registry.npmjs.org/@storybook/react/-/react-6.1.14.tgz#436e9b90096b1d7c83f7f073b5baf47212b2e425" + integrity sha512-M99wHjc/5z+Wz1FdFaScVs6dyAi/6PdcIx5Fyip6Qd8aKwm1XyYoOMql5Vu3Cf560feDYCKS4phzyEZ7EJy+EQ== dependencies: "@babel/preset-flow" "^7.12.1" "@babel/preset-react" "^7.12.1" "@pmmmwh/react-refresh-webpack-plugin" "^0.4.2" - "@storybook/addons" "6.1.11" - "@storybook/core" "6.1.11" - "@storybook/node-logger" "6.1.11" + "@storybook/addons" "6.1.14" + "@storybook/core" "6.1.14" + "@storybook/node-logger" "6.1.14" "@storybook/semver" "^7.3.2" "@types/webpack-env" "^1.15.3" babel-plugin-add-react-displayname "^0.0.5" @@ -5583,6 +5673,18 @@ memoizerific "^1.11.3" qs "^6.6.0" +"@storybook/router@6.1.14": + version "6.1.14" + resolved "https://registry.npmjs.org/@storybook/router/-/router-6.1.14.tgz#f6aef8c9dabf19bf06dddd80907e66369261fdde" + integrity sha512-rMaUCYzgfVLwFWo3A1Q/weSv8FBqCLmHY+3+t6ao7OV6NYjR0XgLKRzHrXq1uYdbMxWeIKhN2tIt/LR43bmDjQ== + dependencies: + "@reach/router" "^1.3.3" + "@types/reach__router" "^1.3.5" + core-js "^3.0.1" + global "^4.3.2" + memoizerific "^1.11.3" + qs "^6.6.0" + "@storybook/semver@^7.3.2": version "7.3.2" resolved "https://registry.npmjs.org/@storybook/semver/-/semver-7.3.2.tgz#f3b9c44a1c9a0b933c04e66d0048fcf2fa10dac0" @@ -5626,21 +5728,39 @@ resolve-from "^5.0.0" ts-dedent "^2.0.0" -"@storybook/ui@6.1.11": - version "6.1.11" - resolved "https://registry.npmjs.org/@storybook/ui/-/ui-6.1.11.tgz#2e5a5df010f2bb75a09a0fd0439fc8e62f8c89e5" - integrity sha512-Qth2dxS5+VbKHcqgkiKpeD+xr/hRUuUIDUA/2Ierh/BaA8Up/krlso/mCLaQOa5E8Og9WJAdDFO0cUbt939c2Q== +"@storybook/theming@6.1.14": + version "6.1.14" + resolved "https://registry.npmjs.org/@storybook/theming/-/theming-6.1.14.tgz#fecb66cab22d3b3218b4a98a9c210eb8a7be91e8" + integrity sha512-S+t30y4FqBTXWoVr+dtxVJ/ywiQGHBclBd9aUunbdCV4mMFra5InNo2CWn+RJlNEauLZ93gRIEzSFchIbzLk1A== dependencies: "@emotion/core" "^10.1.1" - "@storybook/addons" "6.1.11" - "@storybook/api" "6.1.11" - "@storybook/channels" "6.1.11" - "@storybook/client-logger" "6.1.11" - "@storybook/components" "6.1.11" - "@storybook/core-events" "6.1.11" - "@storybook/router" "6.1.11" + "@emotion/is-prop-valid" "^0.8.6" + "@emotion/styled" "^10.0.23" + "@storybook/client-logger" "6.1.14" + core-js "^3.0.1" + deep-object-diff "^1.1.0" + emotion-theming "^10.0.19" + global "^4.3.2" + memoizerific "^1.11.3" + polished "^3.4.4" + resolve-from "^5.0.0" + ts-dedent "^2.0.0" + +"@storybook/ui@6.1.14": + version "6.1.14" + resolved "https://registry.npmjs.org/@storybook/ui/-/ui-6.1.14.tgz#766d696480ee6f6a5a0454ccb2f101c38a0eb9d2" + integrity sha512-DTW2TM05jTMKxh8LzUGk3g5a528PgJxrtgODFU6zzwSg2+LwdmSDtd1HAxopt2vpfTyQyX+6WN2H+lMNwfQTAQ== + dependencies: + "@emotion/core" "^10.1.1" + "@storybook/addons" "6.1.14" + "@storybook/api" "6.1.14" + "@storybook/channels" "6.1.14" + "@storybook/client-logger" "6.1.14" + "@storybook/components" "6.1.14" + "@storybook/core-events" "6.1.14" + "@storybook/router" "6.1.14" "@storybook/semver" "^7.3.2" - "@storybook/theming" "6.1.11" + "@storybook/theming" "6.1.14" "@types/markdown-to-jsx" "^6.11.0" copy-to-clipboard "^3.0.8" core-js "^3.0.1" @@ -7257,12 +7377,7 @@ "@types/serve-static" "*" "@types/webpack" "*" -"@types/webpack-env@^1.15.2": - version "1.15.3" - resolved "https://registry.npmjs.org/@types/webpack-env/-/webpack-env-1.15.3.tgz#fb602cd4c2f0b7c0fb857e922075fdf677d25d84" - integrity sha512-5oiXqR7kwDGZ6+gmzIO2lTC+QsriNuQXZDWNYRV3l2XRN/zmPgnC21DLSx2D05zvD8vnXW6qUg7JnXZ4I6qLVQ== - -"@types/webpack-env@^1.15.3": +"@types/webpack-env@^1.15.2", "@types/webpack-env@^1.15.3": version "1.16.0" resolved "https://registry.npmjs.org/@types/webpack-env/-/webpack-env-1.16.0.tgz#8c0a9435dfa7b3b1be76562f3070efb3f92637b4" integrity sha512-Fx+NpfOO0CpeYX2g9bkvX8O5qh9wrU1sOF4g8sft4Mu7z+qfe387YlyY8w8daDyDsKY5vUxM0yxkAYnbkRbZEw== @@ -22121,12 +22236,7 @@ regenerator-runtime@^0.11.0: resolved "https://registry.npmjs.org/regenerator-runtime/-/regenerator-runtime-0.11.1.tgz#be05ad7f9bf7d22e056f9726cee5017fbf19e2e9" integrity sha512-MguG95oij0fC3QV3URf4V2SDYGJhJnJGqvIIgdECeODCT98wSWDAJ94SSuVpYQUoTcGUIL6L4yNB7j1DFFHSBg== -regenerator-runtime@^0.13.3, regenerator-runtime@^0.13.4, regenerator-runtime@^0.13.5: - version "0.13.5" - resolved "https://registry.npmjs.org/regenerator-runtime/-/regenerator-runtime-0.13.5.tgz#d878a1d094b4306d10b9096484b33ebd55e26697" - integrity sha512-ZS5w8CpKFinUzOwW3c83oPeVXoNsrLsaCoLtJvAClH135j/R77RuymhiSErhm2lKcwSCIpmvIWSbDkIfAqKQlA== - -regenerator-runtime@^0.13.7: +regenerator-runtime@^0.13.3, regenerator-runtime@^0.13.4, regenerator-runtime@^0.13.5, regenerator-runtime@^0.13.7: version "0.13.7" resolved "https://registry.npmjs.org/regenerator-runtime/-/regenerator-runtime-0.13.7.tgz#cac2dacc8a1ea675feaabaeb8ae833898ae46f55" integrity sha512-a54FxoJDIr27pgf7IgeQGxmqUNYrcV338lf/6gH456HZ/PhX+5BcwHXG9ajESmwe6WRO0tAzRUrRmNONWgkrew== From 5345a1f983ca48cf82cfad5f46e0b29fc46c5dcf Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 18 Jan 2021 13:17:50 +0100 Subject: [PATCH 07/75] backend-common: lock down UrlReader to only read from allowed URLs --- .changeset/rich-turtles-roll.md | 19 ++++++ app-config.yaml | 4 ++ .../software-catalog/descriptor-format.md | 13 +++++ packages/backend-common/config.d.ts | 19 ++++++ .../src/reading/FetchUrlReader.test.ts | 58 +++++++++++++++++++ .../src/reading/FetchUrlReader.ts | 29 +++++++++- .../src/reading/UrlReaderPredicateMux.ts | 27 ++------- .../backend-common/src/reading/UrlReaders.ts | 19 ++---- .../processors/UrlReaderProcessor.test.ts | 16 ++++- 9 files changed, 163 insertions(+), 41 deletions(-) create mode 100644 .changeset/rich-turtles-roll.md create mode 100644 packages/backend-common/src/reading/FetchUrlReader.test.ts diff --git a/.changeset/rich-turtles-roll.md b/.changeset/rich-turtles-roll.md new file mode 100644 index 0000000000..9015161369 --- /dev/null +++ b/.changeset/rich-turtles-roll.md @@ -0,0 +1,19 @@ +--- +'@backstage/backend-common': minor +--- + +Remove fallback option from `UrlReaders.create` and `UrlReaders.default`, as well as the default fallback reader. + +To be able to read data from endpoints outside of the configured integrations, you now need to explicitly allow it by +adding an entry in the `backend.reading.allow` list. For example: + +```yml +backend: + baseUrl: ... + reading: + allow: + - host: example.com + - host: '*.examples.org' +``` + +Apart from adding the above configuration, most projects should not need to take any action to migrate existing code. If you do happen to have your own fallback reader configured, this needs to be replaced with a reader factory that selects a specific set of URLs to work with. If you where wrapping the existing fallback reader, the new one that handles the allow list is created using `FetchUrlReader.factory`. diff --git a/app-config.yaml b/app-config.yaml index 5866354a87..741991877e 100644 --- a/app-config.yaml +++ b/app-config.yaml @@ -16,6 +16,10 @@ backend: credentials: true csp: connect-src: ["'self'", 'http:', 'https:'] + reading: + allow: + - host: example.com + - host: '*.mozilla.org' # workingDirectory: /tmp # Use this to configure a working directory for the scaffolder, defaults to the OS temp-dir # See README.md in the proxy-backend plugin for information on the configuration format diff --git a/docs/features/software-catalog/descriptor-format.md b/docs/features/software-catalog/descriptor-format.md index 77feef45e2..5b7c82152d 100644 --- a/docs/features/software-catalog/descriptor-format.md +++ b/docs/features/software-catalog/descriptor-format.md @@ -131,6 +131,19 @@ spec: $text: https://petstore.swagger.io/v2/swagger.json ``` +Note that to be able to read from targets that are outside of the normal +integration points such as `github.com`, you'll need to explicitly allow it by +adding an entry in the `backend.reading.allow` list. For example: + +```yml +backend: + baseUrl: ... + reading: + allow: + - host: example.com + - host: '*.examples.org' +``` + ## Common to All Kinds: The Envelope The root envelope object has the following structure. diff --git a/packages/backend-common/config.d.ts b/packages/backend-common/config.d.ts index b7241bcc03..e5d9b294f8 100644 --- a/packages/backend-common/config.d.ts +++ b/packages/backend-common/config.d.ts @@ -94,6 +94,25 @@ export interface Config { optionsSuccessStatus?: number; }; + /** + * Configuration related to URL reading, used for example for reading catalog info + * files, scaffolder templates, and techdocs content. + */ + reading?: { + /** + * A list of targets to allow outgoing requests to. Users will be able to make + * requests on behalf of the backend to the targets that are allowed by this list. + */ + allow?: Array<{ + /** + * A hostname to allow outgoing requests to, being either a full hostname or + * a subdomain wildcard pattern with a leading `*`. For example `example.com` + * and `*.example.com` are valid values, `prod.*.example.com` is not. + */ + host: string; + }>; + }; + /** * Content Security Policy options. * diff --git a/packages/backend-common/src/reading/FetchUrlReader.test.ts b/packages/backend-common/src/reading/FetchUrlReader.test.ts new file mode 100644 index 0000000000..6579926e3b --- /dev/null +++ b/packages/backend-common/src/reading/FetchUrlReader.test.ts @@ -0,0 +1,58 @@ +/* + * Copyright 2020 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 { ConfigReader } from '@backstage/config'; +import { msw } from '@backstage/test-utils'; +import { setupServer } from 'msw/node'; +import { getVoidLogger } from '../logging'; +import { FetchUrlReader } from './FetchUrlReader'; +import { ReadTreeResponseFactory } from './tree'; + +describe('FetchUrlReader', () => { + const worker = setupServer(); + + msw.setupDefaultHandlers(worker); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('factory should create a single entry with a predicate that matches config', async () => { + const entries = FetchUrlReader.factory({ + config: new ConfigReader({ + backend: { + reading: { + allow: [{ host: 'example.com' }, { host: '*.examples.org' }], + }, + }, + }), + logger: getVoidLogger(), + treeResponseFactory: ReadTreeResponseFactory.create({ + config: new ConfigReader({}), + }), + }); + + expect(entries.length).toBe(1); + const [{ predicate }] = entries; + + expect(predicate(new URL('https://example.com/test'))).toBe(true); + expect(predicate(new URL('https://a.example.com/test'))).toBe(false); + expect(predicate(new URL('https://other.com/test'))).toBe(false); + expect(predicate(new URL('https://examples.org/test'))).toBe(false); + expect(predicate(new URL('https://a.examples.org/test'))).toBe(true); + expect(predicate(new URL('https://a.b.examples.org/test'))).toBe(true); + }); +}); diff --git a/packages/backend-common/src/reading/FetchUrlReader.ts b/packages/backend-common/src/reading/FetchUrlReader.ts index 1d1784590c..aec399a06c 100644 --- a/packages/backend-common/src/reading/FetchUrlReader.ts +++ b/packages/backend-common/src/reading/FetchUrlReader.ts @@ -16,12 +16,39 @@ import fetch from 'cross-fetch'; import { NotFoundError } from '../errors'; -import { ReadTreeResponse, UrlReader } from './types'; +import { ReaderFactory, ReadTreeResponse, UrlReader } from './types'; /** * A UrlReader that does a plain fetch of the URL. */ export class FetchUrlReader implements UrlReader { + /** + * The factory creates a single reader that will be used for reading any URL that's listed + * in configuration at `backend.reading.allow`. The allow list contains a list of objects describing + * targets to allow, containing the following fields: + * + * `host`: + * Either full hostnames to match, or subdomain wildcard matchers with a leading `*`. + * For example `example.com` and `*.example.com` are valid values, `prod.*.example.com` is not. + */ + static factory: ReaderFactory = ({ config }) => { + const predicates = + config + .getOptionalConfigArray('backend.reading.allow') + ?.map(allowConfig => { + const host = allowConfig.getString('host'); + if (host.startsWith('*.')) { + const suffix = host.slice(1); + return (url: URL) => url.hostname.endsWith(suffix); + } + return (url: URL) => url.hostname === host; + }) ?? []; + + const reader = new FetchUrlReader(); + const predicate = (url: URL) => predicates.some(p => p(url)); + return [{ reader, predicate }]; + }; + async read(url: string): Promise { let response: Response; try { diff --git a/packages/backend-common/src/reading/UrlReaderPredicateMux.ts b/packages/backend-common/src/reading/UrlReaderPredicateMux.ts index 465c125fda..ca11400e4a 100644 --- a/packages/backend-common/src/reading/UrlReaderPredicateMux.ts +++ b/packages/backend-common/src/reading/UrlReaderPredicateMux.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import { NotAllowedError } from '../errors'; import { ReadTreeOptions, ReadTreeResponse, @@ -21,22 +22,12 @@ import { UrlReaderPredicateTuple, } from './types'; -type Options = { - // UrlReader to fall back to if no other reader is matched - fallback?: UrlReader; -}; - /** * A UrlReader implementation that selects from a set of UrlReaders * based on a predicate tied to each reader. */ export class UrlReaderPredicateMux implements UrlReader { private readonly readers: UrlReaderPredicateTuple[] = []; - private readonly fallback?: UrlReader; - - constructor({ fallback }: Options) { - this.fallback = fallback; - } register(tuple: UrlReaderPredicateTuple): void { this.readers.push(tuple); @@ -51,11 +42,7 @@ export class UrlReaderPredicateMux implements UrlReader { } } - if (this.fallback) { - return this.fallback.read(url); - } - - throw new Error(`No reader found that could handle '${url}'`); + throw new NotAllowedError(`Reading from '${url}' is not allowed`); } readTree(url: string, options?: ReadTreeOptions): Promise { @@ -67,16 +54,10 @@ export class UrlReaderPredicateMux implements UrlReader { } } - if (this.fallback) { - return this.fallback.readTree(url, options); - } - - throw new Error(`No reader found that could handle '${url}'`); + throw new NotAllowedError(`Reading from '${url}' is not allowed`); } toString() { - return `predicateMux{readers=${this.readers - .map(t => t.reader) - .join(',')},fallback=${this.fallback}}`; + return `predicateMux{readers=${this.readers.map(t => t.reader).join(',')}`; } } diff --git a/packages/backend-common/src/reading/UrlReaders.ts b/packages/backend-common/src/reading/UrlReaders.ts index 2bb5617907..2f233463bb 100644 --- a/packages/backend-common/src/reading/UrlReaders.ts +++ b/packages/backend-common/src/reading/UrlReaders.ts @@ -22,8 +22,8 @@ import { AzureUrlReader } from './AzureUrlReader'; import { BitbucketUrlReader } from './BitbucketUrlReader'; import { GithubUrlReader } from './GithubUrlReader'; import { GitlabUrlReader } from './GitlabUrlReader'; -import { FetchUrlReader } from './FetchUrlReader'; import { ReadTreeResponseFactory } from './tree'; +import { FetchUrlReader } from './FetchUrlReader'; type CreateOptions = { /** Root config object */ @@ -32,8 +32,6 @@ type CreateOptions = { logger: Logger; /** A list of factories used to construct individual readers that match on URLs */ factories?: ReaderFactory[]; - /** Fallback reader to use if none of the readers created by the factories match */ - fallback?: UrlReader; }; /** @@ -43,13 +41,8 @@ export class UrlReaders { /** * Creates a UrlReader without any known types. */ - static create({ - logger, - config, - factories, - fallback, - }: CreateOptions): UrlReader { - const mux = new UrlReaderPredicateMux({ fallback: fallback }); + static create({ logger, config, factories }: CreateOptions): UrlReader { + const mux = new UrlReaderPredicateMux(); const treeResponseFactory = ReadTreeResponseFactory.create({ config }); for (const factory of factories ?? []) { @@ -67,10 +60,8 @@ export class UrlReaders { * Creates a UrlReader that includes all the default factories from this package. * * Any additional factories passed will be loaded before the default ones. - * - * If no fallback reader is passed, a plain fetch reader will be used. */ - static default({ logger, config, factories = [], fallback }: CreateOptions) { + static default({ logger, config, factories = [] }: CreateOptions) { return UrlReaders.create({ logger, config, @@ -79,8 +70,8 @@ export class UrlReaders { BitbucketUrlReader.factory, GithubUrlReader.factory, GitlabUrlReader.factory, + FetchUrlReader.factory, ]), - fallback: fallback ?? new FetchUrlReader(), }); } } diff --git a/plugins/catalog-backend/src/ingestion/processors/UrlReaderProcessor.test.ts b/plugins/catalog-backend/src/ingestion/processors/UrlReaderProcessor.test.ts index 9bf72d620b..aac4235e3a 100644 --- a/plugins/catalog-backend/src/ingestion/processors/UrlReaderProcessor.test.ts +++ b/plugins/catalog-backend/src/ingestion/processors/UrlReaderProcessor.test.ts @@ -27,13 +27,18 @@ import { } from './types'; describe('UrlReaderProcessor', () => { - const mockApiOrigin = 'http://localhost:23000'; + const mockApiOrigin = 'http://localhost'; const server = setupServer(); msw.setupDefaultHandlers(server); it('should load from url', async () => { const logger = getVoidLogger(); - const reader = UrlReaders.default({ logger, config: new ConfigReader({}) }); + const reader = UrlReaders.default({ + logger, + config: new ConfigReader({ + backend: { reading: { allow: [{ host: 'localhost' }] } }, + }), + }); const processor = new UrlReaderProcessor({ reader, logger }); const spec = { type: 'url', @@ -57,7 +62,12 @@ describe('UrlReaderProcessor', () => { it('should fail load from url with error', async () => { const logger = getVoidLogger(); - const reader = UrlReaders.default({ logger, config: new ConfigReader({}) }); + const reader = UrlReaders.default({ + logger, + config: new ConfigReader({ + backend: { reading: { allow: [{ host: 'localhost' }] } }, + }), + }); const processor = new UrlReaderProcessor({ reader, logger }); const spec = { type: 'url', From a283dcc43b2912e89e80b96e49abbf95fedaae6d Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 18 Jan 2021 14:21:34 +0100 Subject: [PATCH 08/75] backend-common: include port in url reader allow list check --- packages/backend-common/config.d.ts | 3 ++- .../src/reading/FetchUrlReader.test.ts | 17 ++++++++++++++++- .../src/reading/FetchUrlReader.ts | 4 ++-- 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/packages/backend-common/config.d.ts b/packages/backend-common/config.d.ts index e5d9b294f8..08b746f96d 100644 --- a/packages/backend-common/config.d.ts +++ b/packages/backend-common/config.d.ts @@ -105,9 +105,10 @@ export interface Config { */ allow?: Array<{ /** - * A hostname to allow outgoing requests to, being either a full hostname or + * A host to allow outgoing requests to, being either a full host or * a subdomain wildcard pattern with a leading `*`. For example `example.com` * and `*.example.com` are valid values, `prod.*.example.com` is not. + * The host may also contain a port, for example `example.com:8080`. */ host: string; }>; diff --git a/packages/backend-common/src/reading/FetchUrlReader.test.ts b/packages/backend-common/src/reading/FetchUrlReader.test.ts index 6579926e3b..8dc4aba29b 100644 --- a/packages/backend-common/src/reading/FetchUrlReader.test.ts +++ b/packages/backend-common/src/reading/FetchUrlReader.test.ts @@ -35,7 +35,12 @@ describe('FetchUrlReader', () => { config: new ConfigReader({ backend: { reading: { - allow: [{ host: 'example.com' }, { host: '*.examples.org' }], + allow: [ + { host: 'example.com' }, + { host: 'example.com:700' }, + { host: '*.examples.org' }, + { host: '*.examples.org:700' }, + ], }, }, }), @@ -50,9 +55,19 @@ describe('FetchUrlReader', () => { expect(predicate(new URL('https://example.com/test'))).toBe(true); expect(predicate(new URL('https://a.example.com/test'))).toBe(false); + expect(predicate(new URL('https://example.com:600/test'))).toBe(false); + expect(predicate(new URL('https://a.example.com:600/test'))).toBe(false); + expect(predicate(new URL('https://example.com:700/test'))).toBe(true); + expect(predicate(new URL('https://a.example.com:700/test'))).toBe(false); expect(predicate(new URL('https://other.com/test'))).toBe(false); expect(predicate(new URL('https://examples.org/test'))).toBe(false); expect(predicate(new URL('https://a.examples.org/test'))).toBe(true); expect(predicate(new URL('https://a.b.examples.org/test'))).toBe(true); + expect(predicate(new URL('https://examples.org:600/test'))).toBe(false); + expect(predicate(new URL('https://a.examples.org:600/test'))).toBe(false); + expect(predicate(new URL('https://a.b.examples.org:600/test'))).toBe(false); + expect(predicate(new URL('https://examples.org:700/test'))).toBe(false); + expect(predicate(new URL('https://a.examples.org:700/test'))).toBe(true); + expect(predicate(new URL('https://a.b.examples.org:700/test'))).toBe(true); }); }); diff --git a/packages/backend-common/src/reading/FetchUrlReader.ts b/packages/backend-common/src/reading/FetchUrlReader.ts index aec399a06c..81f1dfa90e 100644 --- a/packages/backend-common/src/reading/FetchUrlReader.ts +++ b/packages/backend-common/src/reading/FetchUrlReader.ts @@ -39,9 +39,9 @@ export class FetchUrlReader implements UrlReader { const host = allowConfig.getString('host'); if (host.startsWith('*.')) { const suffix = host.slice(1); - return (url: URL) => url.hostname.endsWith(suffix); + return (url: URL) => url.host.endsWith(suffix); } - return (url: URL) => url.hostname === host; + return (url: URL) => url.host === host; }) ?? []; const reader = new FetchUrlReader(); From f04db53d736cc372dcbd38c481112d7e43a5bf1e Mon Sep 17 00:00:00 2001 From: Oliver Sand Date: Mon, 18 Jan 2021 15:08:51 +0100 Subject: [PATCH 09/75] Display systems in catalog table and make both owner and system link to the entity pages --- .changeset/strong-ligers-lay.md | 6 + .../components/CatalogTable/CatalogTable.tsx | 79 +++++++++++-- .../EntityRefLink/EntityRefLink.test.tsx | 50 ++++++++- .../EntityRefLink/EntityRefLink.tsx | 16 +-- .../components/EntityRefLink/format.test.ts | 106 ++++++++++++++++++ .../src/components/EntityRefLink/format.ts | 54 +++++++++ .../src/components/EntityRefLink/index.ts | 1 + 7 files changed, 287 insertions(+), 25 deletions(-) create mode 100644 .changeset/strong-ligers-lay.md create mode 100644 plugins/catalog/src/components/EntityRefLink/format.test.ts create mode 100644 plugins/catalog/src/components/EntityRefLink/format.ts diff --git a/.changeset/strong-ligers-lay.md b/.changeset/strong-ligers-lay.md new file mode 100644 index 0000000000..56f4a91a9d --- /dev/null +++ b/.changeset/strong-ligers-lay.md @@ -0,0 +1,6 @@ +--- +'@backstage/plugin-catalog': patch +--- + +Display systems in catalog table and make both owner and system link to the entity pages. +The owner field is now taken from the relations of the entity instead of its spec. diff --git a/plugins/catalog/src/components/CatalogTable/CatalogTable.tsx b/plugins/catalog/src/components/CatalogTable/CatalogTable.tsx index 684e0e3916..8741cb6157 100644 --- a/plugins/catalog/src/components/CatalogTable/CatalogTable.tsx +++ b/plugins/catalog/src/components/CatalogTable/CatalogTable.tsx @@ -13,7 +13,12 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { Entity } from '@backstage/catalog-model'; +import { + Entity, + EntityName, + RELATION_OWNED_BY, + RELATION_PART_OF, +} from '@backstage/catalog-model'; import { Table, TableColumn, TableProps } from '@backstage/core'; import { Chip, Link } from '@material-ui/core'; import Edit from '@material-ui/icons/Edit'; @@ -22,20 +27,31 @@ import { Alert } from '@material-ui/lab'; import React from 'react'; import { generatePath, Link as RouterLink } from 'react-router-dom'; import { findLocationForEntityMeta } from '../../data/utils'; -import { createEditLink } from '../createEditLink'; import { useStarredEntities } from '../../hooks/useStarredEntities'; import { entityRoute, entityRouteParams } from '../../routes'; +import { createEditLink } from '../createEditLink'; +import { EntityRefLink, formatEntityRefTitle } from '../EntityRefLink'; import { favouriteEntityIcon, favouriteEntityTooltip, } from '../FavouriteEntity/FavouriteEntity'; +import { getEntityRelations } from '../getEntityRelations'; -const columns: TableColumn[] = [ +type EntityRow = Entity & { + row: { + partOfSystemRelationTitle?: string; + partOfSystemRelation?: EntityName; + ownedByRelationsTitle?: string; + ownedByRelations: EntityName[]; + }; +}; + +const columns: TableColumn[] = [ { title: 'Name', field: 'metadata.name', highlight: true, - render: (entity: any) => ( + render: entity => ( [] = [ ), }, + { + title: 'System', + field: 'row.partOfSystemRelationTitle', + render: entity => ( + <> + {entity.row.partOfSystemRelation && ( + + )} + + ), + }, { title: 'Owner', - field: 'spec.owner', + field: 'row.ownedByRelationsTitle', + render: entity => ( + <> + {entity.row.ownedByRelations.map((t, i) => ( + + {i > 0 && ', '} + + + ))} + + ), }, { title: 'Lifecycle', @@ -65,7 +105,7 @@ const columns: TableColumn[] = [ cellStyle: { padding: '0px 16px 0px 20px', }, - render: (entity: Entity) => ( + render: entity => ( <> {entity.metadata.tags && entity.metadata.tags.map(t => ( @@ -141,8 +181,31 @@ export const CatalogTable = ({ }, ]; + const rows = entities.map(e => { + const [partOfSystemRelation] = getEntityRelations(e, RELATION_PART_OF, { + kind: 'system', + }); + const ownedByRelations = getEntityRelations(e, RELATION_OWNED_BY); + + return { + ...e, + row: { + ownedByRelationsTitle: ownedByRelations + .map(r => formatEntityRefTitle(r, { defaultKind: 'group' })) + .join(', '), + ownedByRelations, + partOfSystemRelationTitle: partOfSystemRelation + ? formatEntityRefTitle(partOfSystemRelation, { + defaultKind: 'system', + }) + : undefined, + partOfSystemRelation, + }, + }; + }); + return ( - + isLoading={loading} columns={columns} options={{ @@ -155,7 +218,7 @@ export const CatalogTable = ({ pageSizeOptions: [20, 50, 100], }} title={`${titlePreamble} (${(entities && entities.length) || 0})`} - data={entities} + data={rows} actions={actions} /> ); diff --git a/plugins/catalog/src/components/EntityRefLink/EntityRefLink.test.tsx b/plugins/catalog/src/components/EntityRefLink/EntityRefLink.test.tsx index 28f5caca00..b6fe8a77d8 100644 --- a/plugins/catalog/src/components/EntityRefLink/EntityRefLink.test.tsx +++ b/plugins/catalog/src/components/EntityRefLink/EntityRefLink.test.tsx @@ -37,7 +37,10 @@ describe('', () => { wrapper: MemoryRouter, }); - expect(getByText('component:software')).toBeInTheDocument(); + expect(getByText('component:software')).toHaveAttribute( + 'href', + '/catalog/default/component/software', + ); }); it('renders link for entity in other namespace', () => { @@ -57,7 +60,10 @@ describe('', () => { const { getByText } = render(, { wrapper: MemoryRouter, }); - expect(getByText('component:test/software')).toBeInTheDocument(); + expect(getByText('component:test/software')).toHaveAttribute( + 'href', + '/catalog/test/component/software', + ); }); it('renders link for entity and hides default kind', () => { @@ -80,7 +86,10 @@ describe('', () => { wrapper: MemoryRouter, }, ); - expect(getByText('test/software')).toBeInTheDocument(); + expect(getByText('test/software')).toHaveAttribute( + 'href', + '/catalog/test/component/software', + ); }); it('renders link for entity name in default namespace', () => { @@ -92,7 +101,10 @@ describe('', () => { const { getByText } = render(, { wrapper: MemoryRouter, }); - expect(getByText('component:software')).toBeInTheDocument(); + expect(getByText('component:software')).toHaveAttribute( + 'href', + '/catalog/default/component/software', + ); }); it('renders link for entity name in other namespace', () => { @@ -104,7 +116,10 @@ describe('', () => { const { getByText } = render(, { wrapper: MemoryRouter, }); - expect(getByText('component:test/software')).toBeInTheDocument(); + expect(getByText('component:test/software')).toHaveAttribute( + 'href', + '/catalog/test/component/software', + ); }); it('renders link for entity name and hides default kind', () => { @@ -119,6 +134,29 @@ describe('', () => { wrapper: MemoryRouter, }, ); - expect(getByText('test/software')).toBeInTheDocument(); + expect(getByText('test/software')).toHaveAttribute( + 'href', + '/catalog/test/component/software', + ); + }); + + it('renders link with custom children', () => { + const entityName = { + kind: 'Component', + namespace: 'test', + name: 'software', + }; + const { getByText } = render( + + Custom Children + , + { + wrapper: MemoryRouter, + }, + ); + expect(getByText('Custom Children')).toHaveAttribute( + 'href', + '/catalog/test/component/software', + ); }); }); diff --git a/plugins/catalog/src/components/EntityRefLink/EntityRefLink.tsx b/plugins/catalog/src/components/EntityRefLink/EntityRefLink.tsx index a06ba69fbe..64b2bb8615 100644 --- a/plugins/catalog/src/components/EntityRefLink/EntityRefLink.tsx +++ b/plugins/catalog/src/components/EntityRefLink/EntityRefLink.tsx @@ -17,17 +17,18 @@ import { Entity, EntityName, ENTITY_DEFAULT_NAMESPACE, - serializeEntityRef, } from '@backstage/catalog-model'; import { Link } from '@material-ui/core'; import React from 'react'; import { generatePath } from 'react-router'; import { Link as RouterLink } from 'react-router-dom'; import { entityRoute } from '../../routes'; +import { formatEntityRefTitle } from './format'; type EntityRefLinkProps = { entityRef: Entity | EntityName; defaultKind?: string; + children?: React.ReactNode; }; // TODO: This component is private for now, as it should probably belong into @@ -36,6 +37,7 @@ type EntityRefLinkProps = { export const EntityRefLink = ({ entityRef, defaultKind, + children, }: EntityRefLinkProps) => { let kind; let namespace; @@ -51,17 +53,8 @@ export const EntityRefLink = ({ name = entityRef.name; } - if (namespace === ENTITY_DEFAULT_NAMESPACE) { - namespace = undefined; - } - kind = kind.toLowerCase(); - const title = `${serializeEntityRef({ - kind: defaultKind && defaultKind.toLowerCase() === kind ? undefined : kind, - name, - namespace, - })}`; const routeParams = { kind, namespace: namespace?.toLowerCase() ?? ENTITY_DEFAULT_NAMESPACE, @@ -74,7 +67,8 @@ export const EntityRefLink = ({ component={RouterLink} to={generatePath(`/catalog/${entityRoute.path}`, routeParams)} > - {title} + {children} + {!children && formatEntityRefTitle(entityRef, { defaultKind })} ); }; diff --git a/plugins/catalog/src/components/EntityRefLink/format.test.ts b/plugins/catalog/src/components/EntityRefLink/format.test.ts new file mode 100644 index 0000000000..142c914453 --- /dev/null +++ b/plugins/catalog/src/components/EntityRefLink/format.test.ts @@ -0,0 +1,106 @@ +/* + * Copyright 2020 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 { formatEntityRefTitle } from './format'; + +describe('formatEntityRefTitle', () => { + it('formats entity in default namespace', () => { + const entity = { + apiVersion: 'v1', + kind: 'Component', + metadata: { + name: 'software', + }, + spec: { + owner: 'guest', + type: 'service', + lifecycle: 'production', + }, + }; + const title = formatEntityRefTitle(entity); + expect(title).toEqual('component:software'); + }); + + it('formats entity in other namespace', () => { + const entity = { + apiVersion: 'v1', + kind: 'Component', + metadata: { + name: 'software', + namespace: 'test', + }, + spec: { + owner: 'guest', + type: 'service', + lifecycle: 'production', + }, + }; + const title = formatEntityRefTitle(entity); + expect(title).toEqual('component:test/software'); + }); + + it('formats entity and hides default kind', () => { + const entity = { + apiVersion: 'v1', + kind: 'Component', + metadata: { + name: 'software', + namespace: 'test', + }, + spec: { + owner: 'guest', + type: 'service', + lifecycle: 'production', + }, + }; + const title = formatEntityRefTitle(entity, { defaultKind: 'Component' }); + expect(title).toEqual('test/software'); + }); + + it('formats entity name in default namespace', () => { + const entityName = { + kind: 'Component', + namespace: 'default', + name: 'software', + }; + const title = formatEntityRefTitle(entityName); + expect(title).toEqual('component:software'); + }); + + it('formats entity name in other namespace', () => { + const entityName = { + kind: 'Component', + namespace: 'test', + name: 'software', + }; + + const title = formatEntityRefTitle(entityName); + expect(title).toEqual('component:test/software'); + }); + + it('renders link for entity name and hides default kind', () => { + const entityName = { + kind: 'Component', + namespace: 'test', + name: 'software', + }; + + const title = formatEntityRefTitle(entityName, { + defaultKind: 'component', + }); + expect(title).toEqual('test/software'); + }); +}); diff --git a/plugins/catalog/src/components/EntityRefLink/format.ts b/plugins/catalog/src/components/EntityRefLink/format.ts new file mode 100644 index 0000000000..28ba1bd22d --- /dev/null +++ b/plugins/catalog/src/components/EntityRefLink/format.ts @@ -0,0 +1,54 @@ +/* + * Copyright 2020 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 { + Entity, + EntityName, + ENTITY_DEFAULT_NAMESPACE, + serializeEntityRef, +} from '@backstage/catalog-model'; + +export function formatEntityRefTitle( + entityRef: Entity | EntityName, + opts?: { defaultKind?: string }, +) { + const defaultKind = opts?.defaultKind; + let kind; + let namespace; + let name; + + if ('metadata' in entityRef) { + kind = entityRef.kind; + namespace = entityRef.metadata.namespace; + name = entityRef.metadata.name; + } else { + kind = entityRef.kind; + namespace = entityRef.namespace; + name = entityRef.name; + } + + if (namespace === ENTITY_DEFAULT_NAMESPACE) { + namespace = undefined; + } + + kind = kind.toLowerCase(); + + return `${serializeEntityRef({ + kind: defaultKind && defaultKind.toLowerCase() === kind ? undefined : kind, + name, + namespace, + })}`; +} diff --git a/plugins/catalog/src/components/EntityRefLink/index.ts b/plugins/catalog/src/components/EntityRefLink/index.ts index aa2c6641ef..76a7da38c5 100644 --- a/plugins/catalog/src/components/EntityRefLink/index.ts +++ b/plugins/catalog/src/components/EntityRefLink/index.ts @@ -14,3 +14,4 @@ * limitations under the License. */ export { EntityRefLink } from './EntityRefLink'; +export { formatEntityRefTitle } from './format'; From a93f42213210c0bc35790728b07731d5f5900830 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Mon, 18 Jan 2021 16:50:03 +0100 Subject: [PATCH 10/75] catalog-model: remove special merge treatment of annotations --- .changeset/yellow-ties-switch.md | 5 ++ .../catalog-model/src/entity/util.test.ts | 10 +--- packages/catalog-model/src/entity/util.ts | 51 +++++-------------- 3 files changed, 19 insertions(+), 47 deletions(-) create mode 100644 .changeset/yellow-ties-switch.md diff --git a/.changeset/yellow-ties-switch.md b/.changeset/yellow-ties-switch.md new file mode 100644 index 0000000000..784d2a18ae --- /dev/null +++ b/.changeset/yellow-ties-switch.md @@ -0,0 +1,5 @@ +--- +'@backstage/catalog-model': minor +--- + +The catalog no longer attempts to merge old and new annotations, when updating an entity from a remote location. This was a behavior that was copied from kubernetes, and catered to use cases where you wanted to use HTTP POST to update an entity in-place, outside of what the refresh loop does. This has proved to be a mistake, because as a side effect, the refresh loop effectively is unable to ever delete annotations when they are removed from source YAML. This is obviously a breaking change, but we believe that this is not a behavior that is relied upon in the wild, and it has never been an actually supported use flow of the catalog. We therefore choose to break the behavior outright, and instead just store updated annotations verbatim - just like we already do for example for labels diff --git a/packages/catalog-model/src/entity/util.test.ts b/packages/catalog-model/src/entity/util.test.ts index e4399bbe05..c7e2c036b5 100644 --- a/packages/catalog-model/src/entity/util.test.ts +++ b/packages/catalog-model/src/entity/util.test.ts @@ -96,18 +96,12 @@ describe('util', () => { b = lodash.cloneDeep(a); b.metadata.labels.labelKey += 'a'; expect(entityHasChanges(a, b)).toBe(true); - }); - - it('detects annotation changes, but not removals', () => { - let b: any = lodash.cloneDeep(a); + b = lodash.cloneDeep(a); b.metadata.annotations.annotationKey += 'a'; expect(entityHasChanges(a, b)).toBe(true); b = lodash.cloneDeep(a); - b.metadata.annotations.n = 'n'; - expect(entityHasChanges(a, b)).toBe(true); - b = lodash.cloneDeep(a); delete b.metadata.annotations.annotationKey; - expect(entityHasChanges(a, b)).toBe(false); + expect(entityHasChanges(a, b)).toBe(true); }); it('detects spec changes', () => { diff --git a/packages/catalog-model/src/entity/util.ts b/packages/catalog-model/src/entity/util.ts index ed68339a99..2c63cc4c49 100644 --- a/packages/catalog-model/src/entity/util.ts +++ b/packages/catalog-model/src/entity/util.ts @@ -54,10 +54,6 @@ export function generateEntityEtag(): string { * @param next The new state of the entity */ export function entityHasChanges(previous: Entity, next: Entity): boolean { - if (entityHasAnnotationChanges(previous, next)) { - return true; - } - const e1 = lodash.cloneDeep(previous); const e2 = lodash.cloneDeep(next); @@ -67,6 +63,18 @@ export function entityHasChanges(previous: Entity, next: Entity): boolean { if (!e2.metadata.labels) { e2.metadata.labels = {}; } + if (!e1.metadata.annotations) { + e1.metadata.annotations = {}; + } + if (!e2.metadata.annotations) { + e2.metadata.annotations = {}; + } + if (!e1.metadata.tags) { + e1.metadata.tags = []; + } + if (!e2.metadata.tags) { + e2.metadata.tags = []; + } // Remove generated fields delete e1.metadata.uid; @@ -76,10 +84,6 @@ export function entityHasChanges(previous: Entity, next: Entity): boolean { delete e2.metadata.etag; delete e2.metadata.generation; - // Remove already compared things - delete e1.metadata.annotations; - delete e2.metadata.annotations; - // Remove things that we explicitly do not compare delete e1.relations; delete e2.relations; @@ -106,14 +110,6 @@ export function generateUpdatedEntity(previous: Entity, next: Entity): Entity { const result = lodash.cloneDeep(next); - // Annotations are merged, with the new ones taking precedence - if (previous.metadata.annotations) { - next.metadata.annotations = { - ...previous.metadata.annotations, - ...next.metadata.annotations, - }; - } - // Generated fields are copied and updated const bumpEtag = entityHasChanges(previous, result); const bumpGeneration = !lodash.isEqual(previous.spec, result.spec); @@ -123,26 +119,3 @@ export function generateUpdatedEntity(previous: Entity, next: Entity): Entity { return result; } - -function entityHasAnnotationChanges(previous: Entity, next: Entity): boolean { - // Since the next annotations get merged into the previous, extract only - // the overlapping keys and check if their values match. - if (next.metadata.annotations) { - if (!previous.metadata.annotations) { - return true; - } - if ( - !lodash.isEqual( - next.metadata.annotations, - lodash.pick( - previous.metadata.annotations, - Object.keys(next.metadata.annotations), - ), - ) - ) { - return true; - } - } - - return false; -} From da6a29b6d9b71cad303e00069f7f6ceaeaece8d1 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Mon, 18 Jan 2021 18:48:34 +0100 Subject: [PATCH 11/75] Ensure error handling happens the same way. --- .../src/stages/publish/googleStorage.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/techdocs-common/src/stages/publish/googleStorage.ts b/packages/techdocs-common/src/stages/publish/googleStorage.ts index df9d1120bf..9c28de4130 100644 --- a/packages/techdocs-common/src/stages/publish/googleStorage.ts +++ b/packages/techdocs-common/src/stages/publish/googleStorage.ts @@ -169,16 +169,23 @@ export class GoogleGCSPublish implements PublisherBase { // Files with different extensions (CSS, HTML) need to be served with different headers const fileExtension = path.extname(filePath); const responseHeaders = getHeadersForFileExtension(fileExtension); - res.writeHead(200, responseHeaders); // Pipe file chunks directly from storage to client. this.storageClient .bucket(this.bucketName) .file(filePath) .createReadStream() + .on('pipe', () => { + res.writeHead(200, responseHeaders); + }) .on('error', err => { this.logger.warn(err.message); - res.destroy(err); + // Send a 404 with a meaningful message if possible. + if (!res.headersSent) { + res.status(404).send(err.message); + } else { + res.destroy(); + } }) .pipe(res); }; From e019935409afed46941bdc197ae8b00422dc37bf Mon Sep 17 00:00:00 2001 From: Alan Crosswell Date: Mon, 18 Jan 2021 14:24:56 -0500 Subject: [PATCH 12/75] prettier --- .../extending/create-your-own-templater.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/features/software-templates/extending/create-your-own-templater.md b/docs/features/software-templates/extending/create-your-own-templater.md index 28eb980bff..450892d914 100644 --- a/docs/features/software-templates/extending/create-your-own-templater.md +++ b/docs/features/software-templates/extending/create-your-own-templater.md @@ -88,9 +88,9 @@ _note_ Currently the templaters that we provide are basically Docker action containers that are run on top of the skeleton folder. This keeps dependencies to a minimal for running backstage scaffolder, but you don't /have/ to use Docker. You can `pip install cookiecutter` to run it locally in your backend. -You could create your own templater that spins up an EC2 instance and -downloads the folder and does everything using an AMI if you want. It's entirely -up to you! +You could create your own templater that spins up an EC2 instance and downloads +the folder and does everything using an AMI if you want. It's entirely up to +you! Now it's up to you to implement the `run` function, and then return a `TemplaterRunResult` which is `{ resultDir: string }`. From 2305296073b4a32ca096ceba46dd49c66173385f Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Sat, 16 Jan 2021 22:10:43 +0100 Subject: [PATCH 13/75] backend-common: Support SHA based caching in URL Reader's readTree readTree now takes a SHA in options, and throws a NotModifiedError if the SHA matches with the latest commit SHA on the tree. readTree response now contains a new 'sha' parameter. Also, the default branch will now be used if no branch is provided in the url's target --- catalog-info.yaml | 2 +- packages/backend-common/src/errors.ts | 5 + .../src/reading/AzureUrlReader.ts | 17 +- .../src/reading/BitbucketUrlReader.ts | 17 +- .../src/reading/GithubUrlReader.test.ts | 272 +++++++++++------- .../src/reading/GithubUrlReader.ts | 81 ++++-- .../src/reading/GitlabUrlReader.ts | 17 +- .../src/reading/UrlReaderPredicateMux.ts | 7 +- .../src/reading/__fixtures__/mock-main.tar.gz | Bin 0 -> 230 bytes .../src/reading/__fixtures__/repo.tar.gz | Bin 232 -> 0 bytes .../reading/tree/ReadTreeResponseFactory.ts | 10 +- .../reading/tree/TarArchiveResponse.test.ts | 16 +- .../src/reading/tree/TarArchiveResponse.ts | 4 +- .../src/reading/tree/ZipArchiveResponse.ts | 4 +- packages/backend-common/src/reading/types.ts | 19 +- packages/techdocs-common/src/helpers.test.ts | 1 + 16 files changed, 305 insertions(+), 167 deletions(-) create mode 100644 packages/backend-common/src/reading/__fixtures__/mock-main.tar.gz delete mode 100644 packages/backend-common/src/reading/__fixtures__/repo.tar.gz diff --git a/catalog-info.yaml b/catalog-info.yaml index 2144d1709c..7e60af5755 100644 --- a/catalog-info.yaml +++ b/catalog-info.yaml @@ -6,7 +6,7 @@ metadata: Backstage is an open-source developer portal that puts the developer experience first. annotations: github.com/project-slug: backstage/backstage - backstage.io/techdocs-ref: url:https://github.com/backstage/backstage/tree/master + backstage.io/techdocs-ref: url:https://github.com/backstage/backstage lighthouse.com/website-url: https://backstage.io spec: type: library diff --git a/packages/backend-common/src/errors.ts b/packages/backend-common/src/errors.ts index b68dc8e3f0..39703127e6 100644 --- a/packages/backend-common/src/errors.ts +++ b/packages/backend-common/src/errors.ts @@ -75,3 +75,8 @@ export class NotFoundError extends CustomErrorBase {} * resource. */ export class ConflictError extends CustomErrorBase {} + +/** + * The requested resource has not changed since last request. + */ +export class NotModifiedError extends CustomErrorBase {} diff --git a/packages/backend-common/src/reading/AzureUrlReader.ts b/packages/backend-common/src/reading/AzureUrlReader.ts index ea716a1d4a..4535b329d7 100644 --- a/packages/backend-common/src/reading/AzureUrlReader.ts +++ b/packages/backend-common/src/reading/AzureUrlReader.ts @@ -75,22 +75,27 @@ export class AzureUrlReader implements UrlReader { url: string, options?: ReadTreeOptions, ): Promise { - const response = await fetch( + const archiveAzureResponse = await fetch( getAzureDownloadUrl(url), getAzureRequestOptions(this.options, { Accept: 'application/zip' }), ); - if (!response.ok) { - const message = `Failed to read tree from ${url}, ${response.status} ${response.statusText}`; - if (response.status === 404) { + if (!archiveAzureResponse.ok) { + const message = `Failed to read tree from ${url}, ${archiveAzureResponse.status} ${archiveAzureResponse.statusText}`; + if (archiveAzureResponse.status === 404) { throw new NotFoundError(message); } throw new Error(message); } - return this.deps.treeResponseFactory.fromZipArchive({ - stream: (response.body as unknown) as Readable, + const archiveResponse = await this.deps.treeResponseFactory.fromZipArchive({ + stream: (archiveAzureResponse.body as unknown) as Readable, filter: options?.filter, }); + + const response = archiveResponse as ReadTreeResponse; + // TODO: Just a placeholder for now. + response.sha = ''; + return response; } toString() { diff --git a/packages/backend-common/src/reading/BitbucketUrlReader.ts b/packages/backend-common/src/reading/BitbucketUrlReader.ts index 4578436521..7f462e5bf0 100644 --- a/packages/backend-common/src/reading/BitbucketUrlReader.ts +++ b/packages/backend-common/src/reading/BitbucketUrlReader.ts @@ -108,13 +108,13 @@ export class BitbucketUrlReader implements UrlReader { const isHosted = resource === 'bitbucket.org'; const downloadUrl = await getBitbucketDownloadUrl(url, this.config); - const response = await fetch( + const archiveBitbucketResponse = await fetch( downloadUrl, getBitbucketRequestOptions(this.config), ); - if (!response.ok) { - const message = `Failed to read tree from ${url}, ${response.status} ${response.statusText}`; - if (response.status === 404) { + if (!archiveBitbucketResponse.ok) { + const message = `Failed to read tree from ${url}, ${archiveBitbucketResponse.status} ${archiveBitbucketResponse.statusText}`; + if (archiveBitbucketResponse.status === 404) { throw new NotFoundError(message); } throw new Error(message); @@ -126,11 +126,16 @@ export class BitbucketUrlReader implements UrlReader { folderPath = `${project}-${repoName}-${lastCommitShortHash}`; } - return this.treeResponseFactory.fromZipArchive({ - stream: (response.body as unknown) as Readable, + const archiveResponse = await this.treeResponseFactory.fromZipArchive({ + stream: (archiveBitbucketResponse.body as unknown) as Readable, path: `${folderPath}/${filepath}`, filter: options?.filter, }); + + const response = archiveResponse as ReadTreeResponse; + // TODO: Just a placeholder for now. + response.sha = ''; + return response; } toString() { diff --git a/packages/backend-common/src/reading/GithubUrlReader.test.ts b/packages/backend-common/src/reading/GithubUrlReader.test.ts index 1a0f27d28f..6fdab9dfa1 100644 --- a/packages/backend-common/src/reading/GithubUrlReader.test.ts +++ b/packages/backend-common/src/reading/GithubUrlReader.test.ts @@ -21,6 +21,7 @@ import fs from 'fs'; import { rest } from 'msw'; import { setupServer } from 'msw/node'; import path from 'path'; +import { NotModifiedError } from '../errors'; import { GithubUrlReader } from './GithubUrlReader'; import { ReadTreeResponseFactory } from './tree'; @@ -28,11 +29,27 @@ const treeResponseFactory = ReadTreeResponseFactory.create({ config: new ConfigReader({}), }); -describe('GithubUrlReader', () => { - const mockCredentialsProvider = ({ - getCredentials: jest.fn().mockResolvedValue({ headers: {} }), - } as unknown) as GithubCredentialsProvider; +const mockCredentialsProvider = ({ + getCredentials: jest.fn().mockResolvedValue({ headers: {} }), +} as unknown) as GithubCredentialsProvider; +const githubProcessor = new GithubUrlReader( + { + host: 'github.com', + apiBaseUrl: 'https://api.github.com', + }, + { treeResponseFactory, credentialsProvider: mockCredentialsProvider }, +); + +const gheProcessor = new GithubUrlReader( + { + host: 'ghe.github.com', + apiBaseUrl: 'https://ghe.github.com/api/v3', + }, + { treeResponseFactory, credentialsProvider: mockCredentialsProvider }, +); + +describe('GithubUrlReader', () => { const worker = setupServer(); msw.setupDefaultHandlers(worker); @@ -43,15 +60,8 @@ describe('GithubUrlReader', () => { describe('implementation', () => { it('rejects unknown targets', async () => { - const processor = new GithubUrlReader( - { - host: 'github.com', - apiBaseUrl: 'https://api.github.com', - }, - { treeResponseFactory, credentialsProvider: mockCredentialsProvider }, - ); await expect( - processor.read('https://not.github.com/apa'), + githubProcessor.read('https://not.github.com/apa'), ).rejects.toThrow( 'Incorrect URL: https://not.github.com/apa, Error: Invalid GitHub URL or file path', ); @@ -73,7 +83,7 @@ describe('GithubUrlReader', () => { worker.use( rest.get( - 'https://api.github.com/repos/backstage/mock/tree/contents/?ref=repo', + 'https://api.github.com/repos/backstage/mock/tree/contents/?ref=main', (req, res, ctx) => { expect(req.headers.get('authorization')).toBe( mockHeaders.Authorization, @@ -90,28 +100,43 @@ describe('GithubUrlReader', () => { ), ); - const processor = new GithubUrlReader( - { - host: 'ghe.github.com', - apiBaseUrl: 'https://api.github.com', - }, - { treeResponseFactory, credentialsProvider: mockCredentialsProvider }, - ); - await processor.read( - 'https://ghe.github.com/backstage/mock/tree/blob/repo', + await githubProcessor.read( + 'https://github.com/backstage/mock/tree/blob/main', ); }); }); describe('readTree', () => { const repoBuffer = fs.readFileSync( - path.resolve('src', 'reading', '__fixtures__', 'repo.tar.gz'), + path.resolve('src', 'reading', '__fixtures__', 'mock-main.tar.gz'), ); + const reposGithubApiResponse = { + id: '123', + full_name: 'backstage/mock', + default_branch: 'main', + branches_url: + 'https://api.github.com/repos/backstage/mock/branches{/branch}', + }; + + const reposGheApiResponse = { + ...reposGithubApiResponse, + branches_url: + 'https://ghe.github.com/api/v3/repos/backstage/mock/branches{/branch}', + }; + + const branchesApiResponse = { + name: 'main', + commit: { + sha: 'sha123abc', + }, + }; + beforeEach(() => { + // For github.com host worker.use( rest.get( - 'https://github.com/backstage/mock/archive/repo.tar.gz', + 'https://github.com/backstage/mock/archive/main.tar.gz', (_, res, ctx) => res( ctx.status(200), @@ -120,20 +145,73 @@ describe('GithubUrlReader', () => { ), ), ); + + worker.use( + rest.get('https://api.github.com/repos/backstage/mock', (_, res, ctx) => + res( + ctx.status(200), + ctx.set('Content-Type', 'application/json'), + ctx.json(reposGithubApiResponse), + ), + ), + ); + + worker.use( + rest.get( + 'https://api.github.com/repos/backstage/mock/branches/main', + (_, res, ctx) => + res( + ctx.status(200), + ctx.set('Content-Type', 'application/json'), + ctx.json(branchesApiResponse), + ), + ), + ); + + // For a GHE host + worker.use( + rest.get( + 'https://ghe.github.com/backstage/mock/archive/main.tar.gz', + (_, res, ctx) => + res( + ctx.status(200), + ctx.set('Content-Type', 'application/x-gzip'), + ctx.body(repoBuffer), + ), + ), + ); + + worker.use( + rest.get( + 'https://ghe.github.com/api/v3/repos/backstage/mock', + (_, res, ctx) => + res( + ctx.status(200), + ctx.set('Content-Type', 'application/json'), + ctx.json(reposGheApiResponse), + ), + ), + ); + + worker.use( + rest.get( + 'https://ghe.github.com/api/v3/repos/backstage/mock/branches/main', + (_, res, ctx) => + res( + ctx.status(200), + ctx.set('Content-Type', 'application/json'), + ctx.json(branchesApiResponse), + ), + ), + ); }); it('returns the wanted files from an archive', async () => { - const processor = new GithubUrlReader( - { - host: 'github.com', - apiBaseUrl: 'https://api.github.com', - }, - { treeResponseFactory, credentialsProvider: mockCredentialsProvider }, + const response = await githubProcessor.readTree( + 'https://github.com/backstage/mock/tree/main', ); - const response = await processor.readTree( - 'https://github.com/backstage/mock/tree/repo', - ); + expect(response.sha).toBe('sha123abc'); const files = await response.files(); @@ -145,40 +223,6 @@ describe('GithubUrlReader', () => { expect(indexMarkdownFile.toString()).toBe('# Test\n'); }); - it('includes the subdomain in the github url', async () => { - worker.resetHandlers(); - worker.use( - rest.get( - 'https://ghe.github.com/backstage/mock/archive/repo.tar.gz', - (_, res, ctx) => - res( - ctx.status(200), - ctx.set('Content-Type', 'application/x-gzip'), - ctx.body(repoBuffer), - ), - ), - ); - - const processor = new GithubUrlReader( - { - host: 'ghe.github.com', - apiBaseUrl: 'https://api.github.com', - }, - { treeResponseFactory, credentialsProvider: mockCredentialsProvider }, - ); - - const response = await processor.readTree( - 'https://ghe.github.com/backstage/mock/tree/repo/docs', - ); - - const files = await response.files(); - - expect(files.length).toBe(1); - const indexMarkdownFile = await files[0].content(); - - expect(indexMarkdownFile.toString()).toBe('# Test\n'); - }); - it('should use the headers from the credentials provider to the fetch request', async () => { expect.assertions(2); @@ -193,7 +237,7 @@ describe('GithubUrlReader', () => { worker.use( rest.get( - 'https://ghe.github.com/backstage/mock/archive/repo.tar.gz', + 'https://ghe.github.com/backstage/mock/archive/main.tar.gz', (req, res, ctx) => { expect(req.headers.get('authorization')).toBe( mockHeaders.Authorization, @@ -210,46 +254,14 @@ describe('GithubUrlReader', () => { ), ); - const processor = new GithubUrlReader( - { - host: 'ghe.github.com', - apiBaseUrl: 'https://api.github.com', - }, - { treeResponseFactory, credentialsProvider: mockCredentialsProvider }, - ); - - await processor.readTree( - 'https://ghe.github.com/backstage/mock/tree/repo/docs', + await gheProcessor.readTree( + 'https://ghe.github.com/backstage/mock/tree/main', ); }); - it('must specify a branch', async () => { - const processor = new GithubUrlReader( - { - host: 'github.com', - apiBaseUrl: 'https://api.github.com', - }, - { treeResponseFactory, credentialsProvider: mockCredentialsProvider }, - ); - - await expect( - processor.readTree('https://github.com/backstage/mock'), - ).rejects.toThrow( - 'GitHub URL must contain branch to be able to fetch tree', - ); - }); - - it('returns the wanted files from an archive with a subpath', async () => { - const processor = new GithubUrlReader( - { - host: 'github.com', - apiBaseUrl: 'https://api.github.com', - }, - { treeResponseFactory, credentialsProvider: mockCredentialsProvider }, - ); - - const response = await processor.readTree( - 'https://github.com/backstage/mock/tree/repo/docs', + it('includes the subdomain in the github url', async () => { + const response = await gheProcessor.readTree( + 'https://ghe.github.com/backstage/mock/tree/main/docs', ); const files = await response.files(); @@ -259,5 +271,55 @@ describe('GithubUrlReader', () => { expect(indexMarkdownFile.toString()).toBe('# Test\n'); }); + + it('returns the wanted files from an archive with a subpath', async () => { + const response = await githubProcessor.readTree( + 'https://github.com/backstage/mock/tree/main/docs', + ); + + const files = await response.files(); + + expect(files.length).toBe(1); + const indexMarkdownFile = await files[0].content(); + + expect(indexMarkdownFile.toString()).toBe('# Test\n'); + }); + + it('throws a NotModifiedError when given a sha in options', async () => { + const fnGithub = async () => { + await githubProcessor.readTree('https://github.com/backstage/mock', { + sha: 'sha123abc', + }); + }; + + const fnGhe = async () => { + await gheProcessor.readTree( + 'https://ghe.github.com/backstage/mock/tree/main/docs', + { + sha: 'sha123abc', + }, + ); + }; + + await expect(fnGithub).rejects.toThrow(NotModifiedError); + await expect(fnGhe).rejects.toThrow(NotModifiedError); + }); + + it('should not throw error when given an outdated sha in options', async () => { + const response = await githubProcessor.readTree( + 'https://github.com/backstage/mock/tree/main', + { + sha: 'outdatedSha123abc', + }, + ); + expect((await response.files()).length).toBe(2); + }); + + it('should detect the default branch', async () => { + const response = await githubProcessor.readTree( + 'https://github.com/backstage/mock', + ); + expect((await response.files()).length).toBe(2); + }); }); }); diff --git a/packages/backend-common/src/reading/GithubUrlReader.ts b/packages/backend-common/src/reading/GithubUrlReader.ts index 45ce8336e6..2acca10d4f 100644 --- a/packages/backend-common/src/reading/GithubUrlReader.ts +++ b/packages/backend-common/src/reading/GithubUrlReader.ts @@ -23,7 +23,7 @@ import { import fetch from 'cross-fetch'; import parseGitUrl from 'git-url-parse'; import { Readable } from 'stream'; -import { InputError, NotFoundError } from '../errors'; +import { NotFoundError, NotModifiedError } from '../errors'; import { ReadTreeResponseFactory } from './tree'; import { ReaderFactory, @@ -99,52 +99,83 @@ export class GithubUrlReader implements UrlReader { options?: ReadTreeOptions, ): Promise { const { - name: repoName, - ref, protocol, resource, - full_name, + name: repoName, + ref, filepath, + full_name, } = parseGitUrl(url); - if (!ref) { - // TODO(Rugvip): We should add support for defaulting to the default branch - throw new InputError( - 'GitHub URL must contain branch to be able to fetch tree', - ); - } - const { headers } = await this.deps.credentialsProvider.getCredentials({ url, }); - // TODO(Rugvip): use API to fetch URL instead - const response = await fetch( - new URL( - `${protocol}://${resource}/${full_name}/archive/${ref}.tar.gz`, - ).toString(), + + // Get GitHub API urls for the repository + const repoGitHubResponse = await fetch( + new URL(`${this.config.apiBaseUrl}/repos/${full_name}`).toString(), { - headers: { - ...headers, - }, + headers, }, ); - if (!response.ok) { - const message = `Failed to read tree from ${url}, ${response.status} ${response.statusText}`; - if (response.status === 404) { + if (!repoGitHubResponse.ok) { + const message = `Failed to read tree from ${url}, ${repoGitHubResponse.status} ${repoGitHubResponse.statusText}`; + if (repoGitHubResponse.status === 404) { throw new NotFoundError(message); } throw new Error(message); } - const path = `${repoName}-${ref}/${filepath}`; + const repoResponseJson = await repoGitHubResponse.json(); - return this.deps.treeResponseFactory.fromTarArchive({ + // ref is an empty string if no branch is set in provided url to readTree. + // Use GitHub API to get the default branch of the repository. + const branch = ref === '' ? repoResponseJson.default_branch : ref; + const branchesApiUrl = repoResponseJson.branches_url; + + // Fetch the latest commit in the provided or default branch to compare against + // the provided sha. + const branchGitHubResponse = await fetch( + // branchesApiUrl looks like "https://api.github.com/repos/owner/repo/branches{/branch}" + branchesApiUrl.replace('{/branch}', `/${branch}`), + { + headers, + }, + ); + const commitSha = (await branchGitHubResponse.json()).commit.sha; + + if (options?.sha && options.sha === commitSha) { + throw new NotModifiedError(); + } + + // Note: the API way of downloading an archive URL does not return a real time archive. + // https://github.community/t/archive-downloaded-via-v3-rest-api-is-not-real-time/14827 + // It looks like this https://api.github.com/repos/owner/repo/{archive_format}{/ref} + // and can be used from `repoResponseJson.archive_url`. + // Continue using the "direct" way i.e. https://github.com/:owner/:repo/archive/branch.tar.gz + // until the bug? is fixed. + const archive = await fetch( + new URL( + `${protocol}://${resource}/${full_name}/archive/${branch}.tar.gz`, + ).toString(), + { + headers, + }, + ); + + const path = `${repoName}-${branch}/${filepath}`; + + const archiveResponse = await this.deps.treeResponseFactory.fromTarArchive({ // TODO(Rugvip): Underlying implementation of fetch will be node-fetch, we probably want // to stick to using that in exclusively backend code. - stream: (response.body as unknown) as Readable, + stream: (archive.body as unknown) as Readable, path, filter: options?.filter, }); + + const response = archiveResponse as ReadTreeResponse; + response.sha = commitSha; + return response; } toString() { diff --git a/packages/backend-common/src/reading/GitlabUrlReader.ts b/packages/backend-common/src/reading/GitlabUrlReader.ts index 299bbc783b..76cfea7c38 100644 --- a/packages/backend-common/src/reading/GitlabUrlReader.ts +++ b/packages/backend-common/src/reading/GitlabUrlReader.ts @@ -94,13 +94,13 @@ export class GitlabUrlReader implements UrlReader { } const archive = `${protocol}://${resource}/${full_name}/-/archive/${ref}/${repoName}-${ref}.zip`; - const response = await fetch( + const archiveGitLabResponse = await fetch( archive, getGitLabRequestOptions(this.options), ); - if (!response.ok) { - const msg = `Failed to read tree from ${url}, ${response.status} ${response.statusText}`; - if (response.status === 404) { + if (!archiveGitLabResponse.ok) { + const msg = `Failed to read tree from ${url}, ${archiveGitLabResponse.status} ${archiveGitLabResponse.statusText}`; + if (archiveGitLabResponse.status === 404) { throw new NotFoundError(msg); } throw new Error(msg); @@ -108,11 +108,16 @@ export class GitlabUrlReader implements UrlReader { const path = filepath ? `${repoName}-${ref}/${filepath}/` : ''; - return this.treeResponseFactory.fromZipArchive({ - stream: (response.body as unknown) as Readable, + const archiveResponse = await this.treeResponseFactory.fromZipArchive({ + stream: (archiveGitLabResponse.body as unknown) as Readable, path, filter: options?.filter, }); + + const response = archiveResponse as ReadTreeResponse; + // TODO: Just a placeholder for now. + response.sha = ''; + return response; } toString() { diff --git a/packages/backend-common/src/reading/UrlReaderPredicateMux.ts b/packages/backend-common/src/reading/UrlReaderPredicateMux.ts index ca11400e4a..3183aa0c28 100644 --- a/packages/backend-common/src/reading/UrlReaderPredicateMux.ts +++ b/packages/backend-common/src/reading/UrlReaderPredicateMux.ts @@ -45,12 +45,15 @@ export class UrlReaderPredicateMux implements UrlReader { throw new NotAllowedError(`Reading from '${url}' is not allowed`); } - readTree(url: string, options?: ReadTreeOptions): Promise { + async readTree( + url: string, + options?: ReadTreeOptions, + ): Promise { const parsed = new URL(url); for (const { predicate, reader } of this.readers) { if (predicate(parsed)) { - return reader.readTree(url, options); + return await reader.readTree(url, options); } } diff --git a/packages/backend-common/src/reading/__fixtures__/mock-main.tar.gz b/packages/backend-common/src/reading/__fixtures__/mock-main.tar.gz new file mode 100644 index 0000000000000000000000000000000000000000..291690b447eae0995eda9c4215ffc21636d516c1 GIT binary patch literal 230 zcmV>lPUp5kJIk}HdFl55zWvjD0$=4n1-t*;egQ83=b-1m(n)&pF&VYT g$NUG`^WT<-F8}9X?PA~Ia5xsp4OF6Z-vAH*0G5MqdH?_b literal 0 HcmV?d00001 diff --git a/packages/backend-common/src/reading/__fixtures__/repo.tar.gz b/packages/backend-common/src/reading/__fixtures__/repo.tar.gz deleted file mode 100644 index 7a8e9902a24232a5f7130637a05304134abc3fec..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 232 zcmb2|=3oE==C@Zbay1!XSu&B+beL(LRxe2^Idk!eV446aOJ+o+S-o>~??20b^`}#YbLB3%Cll?@@AuK( zf9m={{U?{EUwv`;^MMVwzTe;Tuda?M|Lgvh`?I~>7XNQAm|j0i{htM|=koGBZ|a}E f`_yk@`Q^XBu@$utzd*@``2}nqTB%DJG#D5FAn { + async fromTarArchive( + options: FromArchiveOptions, + ): Promise { return new TarArchiveResponse( options.stream, options.path ?? '', @@ -49,7 +51,9 @@ export class ReadTreeResponseFactory { ); } - async fromZipArchive(options: FromArchiveOptions): Promise { + async fromZipArchive( + options: FromArchiveOptions, + ): Promise { return new ZipArchiveResponse( options.stream, options.path ?? '', diff --git a/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts b/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts index 1bc0d3a386..bbeee00c70 100644 --- a/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts +++ b/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts @@ -20,7 +20,7 @@ import { resolve as resolvePath } from 'path'; import { TarArchiveResponse } from './TarArchiveResponse'; const archiveData = fs.readFileSync( - resolvePath(__filename, '../../__fixtures__/repo.tar.gz'), + resolvePath(__filename, '../../__fixtures__/mock-main.tar.gz'), ); describe('TarArchiveResponse', () => { @@ -38,7 +38,7 @@ describe('TarArchiveResponse', () => { it('should read files', async () => { const stream = fs.createReadStream('/test-archive.tar.gz'); - const res = new TarArchiveResponse(stream, 'mock-repo/', '/tmp'); + const res = new TarArchiveResponse(stream, 'mock-main/', '/tmp'); const files = await res.files(); expect(files).toEqual([ @@ -61,7 +61,7 @@ describe('TarArchiveResponse', () => { it('should read files with filter', async () => { const stream = fs.createReadStream('/test-archive.tar.gz'); - const res = new TarArchiveResponse(stream, 'mock-repo/', '/tmp', path => + const res = new TarArchiveResponse(stream, 'mock-main/', '/tmp', path => path.endsWith('.yml'), ); const files = await res.files(); @@ -79,7 +79,7 @@ describe('TarArchiveResponse', () => { it('should read as archive and files', async () => { const stream = fs.createReadStream('/test-archive.tar.gz'); - const res = new TarArchiveResponse(stream, 'mock-repo/', '/tmp'); + const res = new TarArchiveResponse(stream, 'mock-main/', '/tmp'); const buffer = await res.archive(); await expect(res.archive()).rejects.toThrow( @@ -113,17 +113,17 @@ describe('TarArchiveResponse', () => { const dir = await res.dir(); await expect( - fs.readFile(resolvePath(dir, 'mock-repo/mkdocs.yml'), 'utf8'), + fs.readFile(resolvePath(dir, 'mock-main/mkdocs.yml'), 'utf8'), ).resolves.toBe('site_name: Test\n'); await expect( - fs.readFile(resolvePath(dir, 'mock-repo/docs/index.md'), 'utf8'), + fs.readFile(resolvePath(dir, 'mock-main/docs/index.md'), 'utf8'), ).resolves.toBe('# Test\n'); }); it('should extract archive into directory with a subpath', async () => { const stream = fs.createReadStream('/test-archive.tar.gz'); - const res = new TarArchiveResponse(stream, 'mock-repo/docs/', '/tmp'); + const res = new TarArchiveResponse(stream, 'mock-main/docs/', '/tmp'); const dir = await res.dir(); expect(dir).toMatch(/^[\/\\]tmp[\/\\].*$/); @@ -135,7 +135,7 @@ describe('TarArchiveResponse', () => { it('should extract archive into directory with a subpath and filter', async () => { const stream = fs.createReadStream('/test-archive.tar.gz'); - const res = new TarArchiveResponse(stream, 'mock-repo/', '/tmp', path => + const res = new TarArchiveResponse(stream, 'mock-main/', '/tmp', path => path.endsWith('.yml'), ); const dir = await res.dir({ targetDir: '/tmp' }); diff --git a/packages/backend-common/src/reading/tree/TarArchiveResponse.ts b/packages/backend-common/src/reading/tree/TarArchiveResponse.ts index 5d18ec7dc6..e8929fbe9b 100644 --- a/packages/backend-common/src/reading/tree/TarArchiveResponse.ts +++ b/packages/backend-common/src/reading/tree/TarArchiveResponse.ts @@ -21,7 +21,7 @@ import { Readable, pipeline as pipelineCb } from 'stream'; import { promisify } from 'util'; import concatStream from 'concat-stream'; import { - ReadTreeResponse, + ReadTreeArchiveResponse, ReadTreeResponseFile, ReadTreeResponseDirOptions, } from '../types'; @@ -34,7 +34,7 @@ const pipeline = promisify(pipelineCb); /** * Wraps a tar archive stream into a tree response reader. */ -export class TarArchiveResponse implements ReadTreeResponse { +export class TarArchiveResponse implements ReadTreeArchiveResponse { private read = false; constructor( diff --git a/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts b/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts index 4106d49a11..48ba4f19ee 100644 --- a/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts +++ b/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts @@ -20,7 +20,7 @@ import unzipper, { Entry } from 'unzipper'; import archiver from 'archiver'; import { Readable } from 'stream'; import { - ReadTreeResponse, + ReadTreeArchiveResponse, ReadTreeResponseFile, ReadTreeResponseDirOptions, } from '../types'; @@ -28,7 +28,7 @@ import { /** * Wraps a zip archive stream into a tree response reader. */ -export class ZipArchiveResponse implements ReadTreeResponse { +export class ZipArchiveResponse implements ReadTreeArchiveResponse { private read = false; constructor( diff --git a/packages/backend-common/src/reading/types.ts b/packages/backend-common/src/reading/types.ts index f9dca3e1d5..c727ba078b 100644 --- a/packages/backend-common/src/reading/types.ts +++ b/packages/backend-common/src/reading/types.ts @@ -32,6 +32,19 @@ export type ReadTreeOptions = { * If no filter is provided all files are extracted. */ filter?(path: string): boolean; + + /** + * A commit SHA can be provided to check whether readTree's response has changed from a previous execution. + * + * In the readTree() response, a SHA is returned along with the tree blob. The SHA belongs to the + * latest commit on the target repository's branch that was used to read the blob. + * + * When a SHA is given in ReadTreeOptions, readTree will first compare the SHA against the latest commit + * on the target branch. If they match, it will throw a NotModifiedError indicating that the readTree + * response will not differ from the previous response which included this particular SHA. If they mismatch, + * readTree will return a new SHA along with the rest of ReadTreeResponse. + */ + sha?: string; }; /** @@ -67,8 +80,12 @@ export type ReadTreeResponseDirOptions = { targetDir?: string; }; -export type ReadTreeResponse = { +export type ReadTreeArchiveResponse = { files(): Promise; archive(): Promise; dir(options?: ReadTreeResponseDirOptions): Promise; }; + +export interface ReadTreeResponse extends ReadTreeArchiveResponse { + sha: string; +} diff --git a/packages/techdocs-common/src/helpers.test.ts b/packages/techdocs-common/src/helpers.test.ts index 88c4d8829e..88fdeed498 100644 --- a/packages/techdocs-common/src/helpers.test.ts +++ b/packages/techdocs-common/src/helpers.test.ts @@ -135,6 +135,7 @@ describe('getDocFilesFromRepository', () => { archive: async () => { return Readable.from(''); }, + sha: '', }; } } From 294a70caba78729627b96759a61881b454069546 Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Sun, 17 Jan 2021 09:28:54 +0100 Subject: [PATCH 14/75] backend-common: Add changeset about SHA based caching in URL Reader --- .changeset/khaki-icons-trade.md | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 .changeset/khaki-icons-trade.md diff --git a/.changeset/khaki-icons-trade.md b/.changeset/khaki-icons-trade.md new file mode 100644 index 0000000000..37bafd8d33 --- /dev/null +++ b/.changeset/khaki-icons-trade.md @@ -0,0 +1,22 @@ +--- +'@backstage/backend-common': patch +--- + +1. URL Reader's `readTree` method now returns a `sha` in the response along with the blob. The SHA belongs to the latest commit on the target. `readTree` also takes an optional `sha` in its options and throws a `NotModifiedError` if the SHA matches with the latest commit SHA on the url's target. This can be used in building a cache when working with URL Reader. + +An example - + +```ts +const response = await reader.readTree( + 'https://github.com/backstage/backstage', +); + +const commitSha = response.sha; + +// Will throw a new NotModifiedError (exported from @backstage/backstage-common) +await reader.readTree('https://github.com/backstage/backstage', { + sha: commitSha, +}); +``` + +2. URL Reader's readTree method can now detect the default branch. So, `url:https://github.com/org/repo/tree/master` can be replaced with `url:https://github.com/org/repo` in places like `backstage.io/techdocs-ref`. From fa8ba330a86b518981fd6177203dc45590b65d3e Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Sun, 17 Jan 2021 10:43:39 +0100 Subject: [PATCH 15/75] integration: GitLab API should be added for default and should have a protocol --- .changeset/nervous-mails-repair.md | 5 +++++ packages/integration/src/gitlab/config.test.ts | 14 +++++++++++++- packages/integration/src/gitlab/config.ts | 4 ++-- 3 files changed, 20 insertions(+), 3 deletions(-) create mode 100644 .changeset/nervous-mails-repair.md diff --git a/.changeset/nervous-mails-repair.md b/.changeset/nervous-mails-repair.md new file mode 100644 index 0000000000..f72e2b5e66 --- /dev/null +++ b/.changeset/nervous-mails-repair.md @@ -0,0 +1,5 @@ +--- +'@backstage/integration': patch +--- + +Fix GitLab API base URL and add it by default to the gitlab.com host diff --git a/packages/integration/src/gitlab/config.test.ts b/packages/integration/src/gitlab/config.test.ts index e817465b83..31643d74a6 100644 --- a/packages/integration/src/gitlab/config.test.ts +++ b/packages/integration/src/gitlab/config.test.ts @@ -43,7 +43,18 @@ describe('readGitLabIntegrationConfig', () => { const output = readGitLabIntegrationConfig(buildConfig({})); expect(output).toEqual({ host: 'gitlab.com', - apiBaseUrl: 'gitlab.com/api/v4', + apiBaseUrl: 'https://gitlab.com/api/v4', + }); + }); + + it('injects the correct GitLab API base URL when missing', () => { + const output = readGitLabIntegrationConfig( + buildConfig({ host: 'gitlab.com' }), + ); + + expect(output).toEqual({ + host: 'gitlab.com', + apiBaseUrl: 'https://gitlab.com/api/v4', }); }); @@ -86,6 +97,7 @@ describe('readGitLabIntegrationConfigs', () => { expect(output).toEqual([ { host: 'gitlab.com', + apiBaseUrl: 'https://gitlab.com/api/v4', }, ]); }); diff --git a/packages/integration/src/gitlab/config.ts b/packages/integration/src/gitlab/config.ts index 2d9f4b39ab..fd52a436b6 100644 --- a/packages/integration/src/gitlab/config.ts +++ b/packages/integration/src/gitlab/config.ts @@ -18,7 +18,7 @@ import { Config } from '@backstage/config'; import { isValidHost } from '../helpers'; const GITLAB_HOST = 'gitlab.com'; -const GITLAB_API_BASE_URL = 'gitlab.com/api/v4'; +const GITLAB_API_BASE_URL = 'https://gitlab.com/api/v4'; /** * The configuration parameters for a single GitLab integration. @@ -89,7 +89,7 @@ export function readGitLabIntegrationConfigs( // As a convenience we always make sure there's at least an unauthenticated // reader for public gitlab repos. if (!result.some(c => c.host === GITLAB_HOST)) { - result.push({ host: GITLAB_HOST }); + result.push({ host: GITLAB_HOST, apiBaseUrl: GITLAB_API_BASE_URL }); } return result; From f9ca2a3769989f653d3a439607099dd6369b4c71 Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Sun, 17 Jan 2021 12:10:32 +0100 Subject: [PATCH 16/75] backend-common: UrlReader/GitLab implement Sha based caching Also use API to fetch archive.zip --- .../src/reading/AzureUrlReader.test.ts | 6 +- .../src/reading/GithubUrlReader.test.ts | 18 +- .../src/reading/GithubUrlReader.ts | 16 +- .../src/reading/GitlabUrlReader.test.ts | 197 ++++++++++++++---- .../src/reading/GitlabUrlReader.ts | 99 ++++++--- .../reading/__fixtures__/gitlab-archive.zip | Bin 0 -> 857 bytes .../src/reading/__fixtures__/mock-main.zip | Bin 0 -> 777 bytes .../src/reading/__fixtures__/repo.zip | Bin 387 -> 0 bytes .../reading/tree/ZipArchiveResponse.test.ts | 28 +-- packages/backend-common/src/reading/types.ts | 4 + 10 files changed, 275 insertions(+), 93 deletions(-) create mode 100644 packages/backend-common/src/reading/__fixtures__/gitlab-archive.zip create mode 100644 packages/backend-common/src/reading/__fixtures__/mock-main.zip delete mode 100644 packages/backend-common/src/reading/__fixtures__/repo.zip diff --git a/packages/backend-common/src/reading/AzureUrlReader.test.ts b/packages/backend-common/src/reading/AzureUrlReader.test.ts index 616cbaaadc..f729316223 100644 --- a/packages/backend-common/src/reading/AzureUrlReader.test.ts +++ b/packages/backend-common/src/reading/AzureUrlReader.test.ts @@ -139,7 +139,7 @@ describe('AzureUrlReader', () => { describe('readTree', () => { const repoBuffer = fs.readFileSync( - path.resolve('src', 'reading', '__fixtures__', 'repo.zip'), + path.resolve('src', 'reading', '__fixtures__', 'mock-main.zip'), ); beforeEach(() => { @@ -169,8 +169,8 @@ describe('AzureUrlReader', () => { const files = await response.files(); expect(files.length).toBe(2); - const mkDocsFile = await files[1].content(); - const indexMarkdownFile = await files[0].content(); + const mkDocsFile = await files[0].content(); + const indexMarkdownFile = await files[1].content(); expect(mkDocsFile.toString()).toBe('site_name: Test\n'); expect(indexMarkdownFile.toString()).toBe('# Test\n'); diff --git a/packages/backend-common/src/reading/GithubUrlReader.test.ts b/packages/backend-common/src/reading/GithubUrlReader.test.ts index 6fdab9dfa1..24a49cf81c 100644 --- a/packages/backend-common/src/reading/GithubUrlReader.test.ts +++ b/packages/backend-common/src/reading/GithubUrlReader.test.ts @@ -21,7 +21,7 @@ import fs from 'fs'; import { rest } from 'msw'; import { setupServer } from 'msw/node'; import path from 'path'; -import { NotModifiedError } from '../errors'; +import { NotFoundError, NotModifiedError } from '../errors'; import { GithubUrlReader } from './GithubUrlReader'; import { ReadTreeResponseFactory } from './tree'; @@ -168,6 +168,13 @@ describe('GithubUrlReader', () => { ), ); + worker.use( + rest.get( + 'https://api.github.com/repos/backstage/mock/branches/branchDoesNotExist', + (_, res, ctx) => res(ctx.status(404)), + ), + ); + // For a GHE host worker.use( rest.get( @@ -321,5 +328,14 @@ describe('GithubUrlReader', () => { ); expect((await response.files()).length).toBe(2); }); + + it('should throw error on missing branch', async () => { + const fnGithub = async () => { + await githubProcessor.readTree( + 'https://github.com/backstage/mock/tree/branchDoesNotExist', + ); + }; + await expect(fnGithub).rejects.toThrow(NotFoundError); + }); }); }); diff --git a/packages/backend-common/src/reading/GithubUrlReader.ts b/packages/backend-common/src/reading/GithubUrlReader.ts index 2acca10d4f..8e0410642b 100644 --- a/packages/backend-common/src/reading/GithubUrlReader.ts +++ b/packages/backend-common/src/reading/GithubUrlReader.ts @@ -119,7 +119,7 @@ export class GithubUrlReader implements UrlReader { }, ); if (!repoGitHubResponse.ok) { - const message = `Failed to read tree from ${url}, ${repoGitHubResponse.status} ${repoGitHubResponse.statusText}`; + const message = `Failed to read tree (repository) from ${url}, ${repoGitHubResponse.status} ${repoGitHubResponse.statusText}`; if (repoGitHubResponse.status === 404) { throw new NotFoundError(message); } @@ -142,6 +142,13 @@ export class GithubUrlReader implements UrlReader { headers, }, ); + if (!branchGitHubResponse.ok) { + const message = `Failed to read tree (branch) from ${url}, ${branchGitHubResponse.status} ${branchGitHubResponse.statusText}`; + if (branchGitHubResponse.status === 404) { + throw new NotFoundError(message); + } + throw new Error(message); + } const commitSha = (await branchGitHubResponse.json()).commit.sha; if (options?.sha && options.sha === commitSha) { @@ -162,6 +169,13 @@ export class GithubUrlReader implements UrlReader { headers, }, ); + if (!archive.ok) { + const message = `Failed to read tree (archive) from ${url}, ${archive.status} ${archive.statusText}`; + if (archive.status === 404) { + throw new NotFoundError(message); + } + throw new Error(message); + } const path = `${repoName}-${branch}/${filepath}`; diff --git a/packages/backend-common/src/reading/GitlabUrlReader.test.ts b/packages/backend-common/src/reading/GitlabUrlReader.test.ts index 6cc5e30dbd..a2dda48b16 100644 --- a/packages/backend-common/src/reading/GitlabUrlReader.test.ts +++ b/packages/backend-common/src/reading/GitlabUrlReader.test.ts @@ -23,6 +23,7 @@ import path from 'path'; import { getVoidLogger } from '../logging'; import { GitlabUrlReader } from './GitlabUrlReader'; import { ReadTreeResponseFactory } from './tree'; +import { NotModifiedError, NotFoundError } from '../errors'; const logger = getVoidLogger(); @@ -30,6 +31,22 @@ const treeResponseFactory = ReadTreeResponseFactory.create({ config: new ConfigReader({}), }); +const gitlabProcessor = new GitlabUrlReader( + { + host: 'gitlab.com', + apiBaseUrl: 'https://gitlab.com/api/v4', + }, + { treeResponseFactory }, +); + +const hostedGitlabProcessor = new GitlabUrlReader( + { + host: 'gitlab.mycompany.com', + apiBaseUrl: 'https://gitlab.mycompany.com/api/v4', + }, + { treeResponseFactory }, +); + describe('GitlabUrlReader', () => { const worker = setupServer(); msw.setupDefaultHandlers(worker); @@ -136,39 +153,112 @@ describe('GitlabUrlReader', () => { }); describe('readTree', () => { - const repoBuffer = fs.readFileSync( - path.resolve('src', 'reading', '__fixtures__', 'repo.zip'), + const archiveBuffer = fs.readFileSync( + path.resolve('src', 'reading', '__fixtures__', 'gitlab-archive.zip'), ); + const projectGitlabApiResponse = { + id: 11111111, + default_branch: 'main', + }; + + const branchGitlabApiResponse = { + commit: { + id: 'sha123abc', + }, + }; + beforeEach(() => { worker.use( rest.get( - 'https://gitlab.com/backstage/mock/-/archive/repo/mock-repo.zip', + 'https://gitlab.com/api/v4/projects/backstage%2Fmock/repository/archive.zip?sha=main', (_, res, ctx) => res( ctx.status(200), ctx.set('Content-Type', 'application/zip'), - ctx.body(repoBuffer), + ctx.body(archiveBuffer), + ), + ), + ); + + worker.use( + rest.get( + 'https://gitlab.com/api/v4/projects/backstage%2Fmock', + (_, res, ctx) => + res( + ctx.status(200), + ctx.set('Content-Type', 'application/json'), + ctx.json(projectGitlabApiResponse), + ), + ), + ); + + worker.use( + rest.get( + 'https://gitlab.com/api/v4/projects/backstage%2Fmock/repository/branches/main', + (_, res, ctx) => + res( + ctx.status(200), + ctx.set('Content-Type', 'application/json'), + ctx.json(branchGitlabApiResponse), + ), + ), + ); + + worker.use( + rest.get( + 'https://gitlab.com/api/v4/projects/backstage%2Fmock/repository/branches/branchDoesNotExist', + (_, res, ctx) => res(ctx.status(404)), + ), + ); + + worker.use( + rest.get( + 'https://gitlab.mycompany.com/api/v4/projects/backstage%2Fmock', + (_, res, ctx) => + res( + ctx.status(200), + ctx.set('Content-Type', 'application/json'), + ctx.json(projectGitlabApiResponse), + ), + ), + ); + + worker.use( + rest.get( + 'https://gitlab.mycompany.com/api/v4/projects/backstage%2Fmock/repository/branches/main', + (_, res, ctx) => + res( + ctx.status(200), + ctx.set('Content-Type', 'application/json'), + ctx.json(branchGitlabApiResponse), + ), + ), + ); + + worker.use( + rest.get( + 'https://gitlab.mycompany.com/api/v4/projects/backstage%2Fmock/repository/archive.zip?sha=main', + (_, res, ctx) => + res( + ctx.status(200), + ctx.set('Content-Type', 'application/zip'), + ctx.body(archiveBuffer), ), ), ); }); it('returns the wanted files from an archive', async () => { - const processor = new GitlabUrlReader( - { host: 'gitlab.com' }, - { treeResponseFactory }, - ); - - const response = await processor.readTree( - 'https://gitlab.com/backstage/mock/tree/repo', + const response = await gitlabProcessor.readTree( + 'https://gitlab.com/backstage/mock/tree/main', ); const files = await response.files(); expect(files.length).toBe(2); - const indexMarkdownFile = await files[0].content(); - const mkDocsFile = await files[1].content(); + const mkDocsFile = await files[0].content(); + const indexMarkdownFile = await files[1].content(); expect(mkDocsFile.toString()).toBe('site_name: Test\n'); expect(indexMarkdownFile.toString()).toBe('# Test\n'); @@ -177,23 +267,18 @@ describe('GitlabUrlReader', () => { it('returns the wanted files from hosted gitlab', async () => { worker.use( rest.get( - 'https://git.mycompany.com/backstage/mock/-/archive/repo/mock-repo.zip', + 'https://gitlab.mycompany.com/backstage/mock/-/archive/main.zip', (_, res, ctx) => res( ctx.status(200), ctx.set('Content-Type', 'application/zip'), - ctx.body(repoBuffer), + ctx.body(archiveBuffer), ), ), ); - const processor = new GitlabUrlReader( - { host: 'git.mycompany.com' }, - { treeResponseFactory }, - ); - - const response = await processor.readTree( - 'https://git.mycompany.com/backstage/mock/tree/repo/docs', + const response = await hostedGitlabProcessor.readTree( + 'https://gitlab.mycompany.com/backstage/mock/tree/main/docs', ); const files = await response.files(); @@ -204,27 +289,9 @@ describe('GitlabUrlReader', () => { expect(indexMarkdownFile.toString()).toBe('# Test\n'); }); - it('throws an error when branch is not specified', async () => { - const processor = new GitlabUrlReader( - { host: 'gitlab.com' }, - { treeResponseFactory }, - ); - - await expect( - processor.readTree('https://gitlab.com/backstage/mock'), - ).rejects.toThrow( - 'GitLab URL must contain a branch to be able to fetch its tree', - ); - }); - it('returns the wanted files from an archive with a subpath', async () => { - const processor = new GitlabUrlReader( - { host: 'gitlab.com' }, - { treeResponseFactory }, - ); - - const response = await processor.readTree( - 'https://gitlab.com/backstage/mock/tree/repo/docs', + const response = await gitlabProcessor.readTree( + 'https://gitlab.com/backstage/mock/tree/main/docs', ); const files = await response.files(); @@ -234,5 +301,51 @@ describe('GitlabUrlReader', () => { expect(indexMarkdownFile.toString()).toBe('# Test\n'); }); + + it('throws a NotModifiedError when given a sha in options', async () => { + const fnGitlab = async () => { + await gitlabProcessor.readTree('https://gitlab.com/backstage/mock', { + sha: 'sha123abc', + }); + }; + + const fnHostedGitlab = async () => { + await hostedGitlabProcessor.readTree( + 'https://gitlab.mycompany.com/backstage/mock', + { + sha: 'sha123abc', + }, + ); + }; + + await expect(fnGitlab).rejects.toThrow(NotModifiedError); + await expect(fnHostedGitlab).rejects.toThrow(NotModifiedError); + }); + + it('should not throw error when given an outdated sha in options', async () => { + const response = await gitlabProcessor.readTree( + 'https://gitlab.com/backstage/mock/tree/main', + { + sha: 'outdatedSha123abc', + }, + ); + expect((await response.files()).length).toBe(2); + }); + + it('should detect the default branch', async () => { + const response = await gitlabProcessor.readTree( + 'https://gitlab.com/backstage/mock', + ); + expect((await response.files()).length).toBe(2); + }); + + it('should throw error on missing branch', async () => { + const fnGithub = async () => { + await gitlabProcessor.readTree( + 'https://gitlab.com/backstage/mock/tree/branchDoesNotExist', + ); + }; + await expect(fnGithub).rejects.toThrow(NotFoundError); + }); }); }); diff --git a/packages/backend-common/src/reading/GitlabUrlReader.ts b/packages/backend-common/src/reading/GitlabUrlReader.ts index 76cfea7c38..0c0ce4f565 100644 --- a/packages/backend-common/src/reading/GitlabUrlReader.ts +++ b/packages/backend-common/src/reading/GitlabUrlReader.ts @@ -21,7 +21,7 @@ import { readGitLabIntegrationConfigs, } from '@backstage/integration'; import fetch from 'cross-fetch'; -import { InputError, NotFoundError } from '../errors'; +import { NotFoundError, NotModifiedError } from '../errors'; import { ReadTreeResponseFactory } from './tree'; import { ReaderFactory, @@ -39,26 +39,26 @@ export class GitlabUrlReader implements UrlReader { const configs = readGitLabIntegrationConfigs( config.getOptionalConfigArray('integrations.gitlab') ?? [], ); - return configs.map(options => { - const reader = new GitlabUrlReader(options, { treeResponseFactory }); - const predicate = (url: URL) => url.host === options.host; + return configs.map(provider => { + const reader = new GitlabUrlReader(provider, { treeResponseFactory }); + const predicate = (url: URL) => url.host === provider.host; return { reader, predicate }; }); }; constructor( - private readonly options: GitLabIntegrationConfig, + private readonly config: GitLabIntegrationConfig, deps: { treeResponseFactory: ReadTreeResponseFactory }, ) { this.treeResponseFactory = deps.treeResponseFactory; } async read(url: string): Promise { - const builtUrl = await getGitLabFileFetchUrl(url, this.options); + const builtUrl = await getGitLabFileFetchUrl(url, this.config); let response: Response; try { - response = await fetch(builtUrl, getGitLabRequestOptions(this.options)); + response = await fetch(builtUrl, getGitLabRequestOptions(this.config)); } catch (e) { throw new Error(`Unable to read ${url}, ${e}`); } @@ -78,35 +78,71 @@ export class GitlabUrlReader implements UrlReader { url: string, options?: ReadTreeOptions, ): Promise { - const { - name: repoName, - ref, - protocol, - resource, - full_name, - filepath, - } = parseGitUrl(url); + const { name: repoName, ref, full_name, filepath } = parseGitUrl(url); - if (!ref) { - throw new InputError( - 'GitLab URL must contain a branch to be able to fetch its tree', - ); - } - - const archive = `${protocol}://${resource}/${full_name}/-/archive/${ref}/${repoName}-${ref}.zip`; - const archiveGitLabResponse = await fetch( - archive, - getGitLabRequestOptions(this.options), + // Use GitLab API to get the default branch + // encodeURIComponent is required for GitLab API + // https://docs.gitlab.com/ee/api/README.html#namespaced-path-encoding + const projectGitlabResponse = await fetch( + new URL( + `${this.config.apiBaseUrl}/projects/${encodeURIComponent(full_name)}`, + ).toString(), + getGitLabRequestOptions(this.config), ); - if (!archiveGitLabResponse.ok) { - const msg = `Failed to read tree from ${url}, ${archiveGitLabResponse.status} ${archiveGitLabResponse.statusText}`; - if (archiveGitLabResponse.status === 404) { + if (!projectGitlabResponse.ok) { + const msg = `Failed to read tree from ${url}, ${projectGitlabResponse.status} ${projectGitlabResponse.statusText}`; + if (projectGitlabResponse.status === 404) { throw new NotFoundError(msg); } throw new Error(msg); } + const projectGitlabResponseJson = await projectGitlabResponse.json(); - const path = filepath ? `${repoName}-${ref}/${filepath}/` : ''; + // ref is an empty string if no branch is set in provided url to readTree. + const branch = ref === '' ? projectGitlabResponseJson.default_branch : ref; + + // Fetch the latest commit in the provided or default branch to compare against + // the provided sha. + const branchGitlabResponse = await fetch( + new URL( + `${this.config.apiBaseUrl}/projects/${encodeURIComponent( + full_name, + )}/repository/branches/${branch}`, + ).toString(), + getGitLabRequestOptions(this.config), + ); + if (!branchGitlabResponse.ok) { + const message = `Failed to read tree (branch) from ${url}, ${branchGitlabResponse.status} ${branchGitlabResponse.statusText}`; + if (branchGitlabResponse.status === 404) { + throw new NotFoundError(message); + } + throw new Error(message); + } + + const commitSha = (await branchGitlabResponse.json()).commit.id; + + if (options?.sha && options.sha === commitSha) { + throw new NotModifiedError(); + } + + // https://docs.gitlab.com/ee/api/repositories.html#get-file-archive + const archiveGitLabResponse = await fetch( + `${this.config.apiBaseUrl}/projects/${encodeURIComponent( + full_name, + )}/repository/archive.zip?sha=${branch}`, + getGitLabRequestOptions(this.config), + ); + if (!archiveGitLabResponse.ok) { + const message = `Failed to read tree (archive) from ${url}, ${archiveGitLabResponse.status} ${archiveGitLabResponse.statusText}`; + if (archiveGitLabResponse.status === 404) { + throw new NotFoundError(message); + } + throw new Error(message); + } + + const path = filepath + ? `${repoName}-${branch}-${commitSha}/${filepath}/` + : ''; const archiveResponse = await this.treeResponseFactory.fromZipArchive({ stream: (archiveGitLabResponse.body as unknown) as Readable, @@ -115,13 +151,12 @@ export class GitlabUrlReader implements UrlReader { }); const response = archiveResponse as ReadTreeResponse; - // TODO: Just a placeholder for now. - response.sha = ''; + response.sha = commitSha; return response; } toString() { - const { host, token } = this.options; + const { host, token } = this.config; return `gitlab{host=${host},authed=${Boolean(token)}}`; } } diff --git a/packages/backend-common/src/reading/__fixtures__/gitlab-archive.zip b/packages/backend-common/src/reading/__fixtures__/gitlab-archive.zip new file mode 100644 index 0000000000000000000000000000000000000000..884ec200048ae719af7b6e9c352ce5ab062c0da2 GIT binary patch literal 857 zcmWIWW@Zs#0D;aZ!yqsNN{BEhFy!VZXY1xBX6ES@XCxXL87C$s>xYK$GO#D}vm}7< z32~N$(h6<{MwYLP3=CkC0>CD6FmN!euZ<3bnJ55c$l)+CH#;Rixmd3>OX)-s}?snh&xAVmruIbpJ@=upMMK zs;976jPTSpBu}vetx?2hY-V0cYK2~I3fNyWfc^p*jm7xjFeL9x6FDEj2{ajGdVn`0 zlL#~J2m&ergSU<#ioEE8*Z_+#paHV_|u_`hUFdG-N3j!RBdQ0mScmYyHY5+Q z0}U6)G%PbOCAC5?HwEm689+b4LI%wb!C^>FpC)oXf)i*S$jkt5MkWzv+yM_%0tRm# zK@=&`05KO95y-&>iU=53(&&L=F7eTV&*h+Chk>__dr)j3G7=EZ2So#Nkb$BB29`8( dG9m{H*l=PaCBU1N4P+1t5Y_;VDF0=yB1 jV>%w$@CX#ck-Y*m8RQiVlUdn74q^hr?Lc}Ph{FH?Ktx*D diff --git a/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts b/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts index 6c2592ffce..345ec7a783 100644 --- a/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts +++ b/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts @@ -20,7 +20,7 @@ import { resolve as resolvePath } from 'path'; import { ZipArchiveResponse } from './ZipArchiveResponse'; const archiveData = fs.readFileSync( - resolvePath(__filename, '../../__fixtures__/repo.zip'), + resolvePath(__filename, '../../__fixtures__/mock-main.zip'), ); describe('ZipArchiveResponse', () => { @@ -38,30 +38,30 @@ describe('ZipArchiveResponse', () => { it('should read files', async () => { const stream = fs.createReadStream('/test-archive.zip'); - const res = new ZipArchiveResponse(stream, 'mock-repo/', '/tmp'); + const res = new ZipArchiveResponse(stream, 'mock-main/', '/tmp'); const files = await res.files(); expect(files).toEqual([ { - path: 'docs/index.md', + path: 'mkdocs.yml', content: expect.any(Function), }, { - path: 'mkdocs.yml', + path: 'docs/index.md', content: expect.any(Function), }, ]); const contents = await Promise.all(files.map(f => f.content())); expect(contents.map(c => c.toString('utf8').trim())).toEqual([ - '# Test', 'site_name: Test', + '# Test', ]); }); it('should read files with filter', async () => { const stream = fs.createReadStream('/test-archive.zip'); - const res = new ZipArchiveResponse(stream, 'mock-repo/', '/tmp', path => + const res = new ZipArchiveResponse(stream, 'mock-main/', '/tmp', path => path.endsWith('.yml'), ); const files = await res.files(); @@ -79,7 +79,7 @@ describe('ZipArchiveResponse', () => { it('should read as archive and files', async () => { const stream = fs.createReadStream('/test-archive.zip'); - const res = new ZipArchiveResponse(stream, 'mock-repo/', '/tmp'); + const res = new ZipArchiveResponse(stream, 'mock-main/', '/tmp'); const buffer = await res.archive(); await expect(res.archive()).rejects.toThrow( @@ -91,18 +91,18 @@ describe('ZipArchiveResponse', () => { expect(files).toEqual([ { - path: 'docs/index.md', + path: 'mkdocs.yml', content: expect.any(Function), }, { - path: 'mkdocs.yml', + path: 'docs/index.md', content: expect.any(Function), }, ]); const contents = await Promise.all(files.map(f => f.content())); expect(contents.map(c => c.toString('utf8').trim())).toEqual([ - '# Test', 'site_name: Test', + '# Test', ]); }); @@ -113,17 +113,17 @@ describe('ZipArchiveResponse', () => { const dir = await res.dir(); await expect( - fs.readFile(resolvePath(dir, 'mock-repo/mkdocs.yml'), 'utf8'), + fs.readFile(resolvePath(dir, 'mock-main/mkdocs.yml'), 'utf8'), ).resolves.toBe('site_name: Test\n'); await expect( - fs.readFile(resolvePath(dir, 'mock-repo/docs/index.md'), 'utf8'), + fs.readFile(resolvePath(dir, 'mock-main/docs/index.md'), 'utf8'), ).resolves.toBe('# Test\n'); }); it('should extract archive into directory with a subpath', async () => { const stream = fs.createReadStream('/test-archive.zip'); - const res = new ZipArchiveResponse(stream, 'mock-repo/docs/', '/tmp'); + const res = new ZipArchiveResponse(stream, 'mock-main/docs/', '/tmp'); const dir = await res.dir(); expect(dir).toMatch(/^[\/\\]tmp[\/\\].*$/); @@ -135,7 +135,7 @@ describe('ZipArchiveResponse', () => { it('should extract archive into directory with a subpath and filter', async () => { const stream = fs.createReadStream('/test-archive.zip'); - const res = new ZipArchiveResponse(stream, 'mock-repo/', '/tmp', path => + const res = new ZipArchiveResponse(stream, 'mock-main/', '/tmp', path => path.endsWith('.yml'), ); const dir = await res.dir({ targetDir: '/tmp' }); diff --git a/packages/backend-common/src/reading/types.ts b/packages/backend-common/src/reading/types.ts index c727ba078b..d5c24ed1db 100644 --- a/packages/backend-common/src/reading/types.ts +++ b/packages/backend-common/src/reading/types.ts @@ -83,6 +83,10 @@ export type ReadTreeResponseDirOptions = { export type ReadTreeArchiveResponse = { files(): Promise; archive(): Promise; + + /** + * dir() extracts the tree response into a directory and returns the path of the directory. + */ dir(options?: ReadTreeResponseDirOptions): Promise; }; From 7078d35e0af4c0ef44cd63ae47f2f1361bab460c Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Sun, 17 Jan 2021 14:03:43 +0100 Subject: [PATCH 17/75] backend-common: UrlReader/Bitbucket implement SHA based caching Default branch detection was already implemented --- .../src/reading/BitbucketUrlReader.test.ts | 157 ++++++++++-------- .../src/reading/BitbucketUrlReader.ts | 13 +- .../src/reading/GithubUrlReader.test.ts | 20 --- .../src/reading/GitlabUrlReader.test.ts | 18 -- 4 files changed, 96 insertions(+), 112 deletions(-) diff --git a/packages/backend-common/src/reading/BitbucketUrlReader.test.ts b/packages/backend-common/src/reading/BitbucketUrlReader.test.ts index e327770114..111aff753c 100644 --- a/packages/backend-common/src/reading/BitbucketUrlReader.test.ts +++ b/packages/backend-common/src/reading/BitbucketUrlReader.test.ts @@ -20,6 +20,7 @@ import fs from 'fs'; import { rest } from 'msw'; import { setupServer } from 'msw/node'; import path from 'path'; +import { NotModifiedError } from '../errors'; import { BitbucketUrlReader } from './BitbucketUrlReader'; import { ReadTreeResponseFactory } from './tree'; @@ -27,15 +28,24 @@ const treeResponseFactory = ReadTreeResponseFactory.create({ config: new ConfigReader({}), }); +const bitbucketProcessor = new BitbucketUrlReader( + { host: 'bitbucket.org', apiBaseUrl: 'https://api.bitbucket.org/2.0' }, + { treeResponseFactory }, +); + +const hostedBitbucketProcessor = new BitbucketUrlReader( + { + host: 'bitbucket.mycompany.net', + apiBaseUrl: 'https://api.bitbucket.mycompany.net/rest/api/1.0', + }, + { treeResponseFactory }, +); + describe('BitbucketUrlReader', () => { describe('implementation', () => { it('rejects unknown targets', async () => { - const processor = new BitbucketUrlReader( - { host: 'bitbucket.org', apiBaseUrl: 'https://api.bitbucket.org/2.0' }, - { treeResponseFactory }, - ); await expect( - processor.read('https://not.bitbucket.com/apa'), + bitbucketProcessor.read('https://not.bitbucket.com/apa'), ).rejects.toThrow( 'Incorrect URL: https://not.bitbucket.com/apa, Error: Invalid Bitbucket URL or file path', ); @@ -55,8 +65,30 @@ describe('BitbucketUrlReader', () => { ), ); - it('returns the wanted files from an archive', async () => { + const privateBitbucketRepoBuffer = fs.readFileSync( + path.resolve( + 'src', + 'reading', + '__fixtures__', + 'bitbucket-server-repo.zip', + ), + ); + + beforeEach(() => { worker.use( + rest.get( + 'https://api.bitbucket.org/2.0/repositories/backstage/mock', + (_, res, ctx) => + res( + ctx.status(200), + ctx.json({ + mainbranch: { + type: 'branch', + name: 'master', + }, + }), + ), + ), rest.get( 'https://bitbucket.org/backstage/mock/get/master.zip', (_, res, ctx) => @@ -76,17 +108,35 @@ describe('BitbucketUrlReader', () => { }), ), ), + rest.get( + 'https://api.bitbucket.mycompany.net/rest/api/1.0/projects/backstage/repos/mock/archive?format=zip&prefix=mock&path=docs', + (_, res, ctx) => + res( + ctx.status(200), + ctx.set('Content-Type', 'application/zip'), + ctx.body(privateBitbucketRepoBuffer), + ), + ), + rest.get( + 'https://api.bitbucket.mycompany.net/rest/api/1.0/repositories/backstage/mock/commits/some-branch', + (_, res, ctx) => + res( + ctx.status(200), + ctx.json({ + values: [{ hash: '12ab34cd56ef78gh90ij12kl34mn56op78qr90st' }], + }), + ), + ), ); + }); - const processor = new BitbucketUrlReader( - { host: 'bitbucket.org', apiBaseUrl: 'https://api.bitbucket.org/2.0' }, - { treeResponseFactory }, - ); - - const response = await processor.readTree( + it('returns the wanted files from an archive', async () => { + const response = await bitbucketProcessor.readTree( 'https://bitbucket.org/backstage/mock/src/master', ); + expect(response.sha).toBe('12ab34cd56ef'); + const files = await response.files(); expect(files.length).toBe(2); @@ -98,38 +148,12 @@ describe('BitbucketUrlReader', () => { }); it('uses private bitbucket host', async () => { - const privateBitbucketRepoBuffer = fs.readFileSync( - path.resolve( - 'src', - 'reading', - '__fixtures__', - 'bitbucket-server-repo.zip', - ), - ); - worker.use( - rest.get( - 'https://api.bitbucket.mycompany.net/rest/api/1.0/projects/backstage/repos/mock/archive?format=zip&prefix=mock&path=docs', - (_, res, ctx) => - res( - ctx.status(200), - ctx.set('Content-Type', 'application/zip'), - ctx.body(privateBitbucketRepoBuffer), - ), - ), - ); - - const processor = new BitbucketUrlReader( - { - host: 'bitbucket.mycompany.net', - apiBaseUrl: 'https://api.bitbucket.mycompany.net/rest/api/1.0', - }, - { treeResponseFactory }, - ); - - const response = await processor.readTree( + const response = await hostedBitbucketProcessor.readTree( 'https://bitbucket.mycompany.net/projects/backstage/repos/mock/browse/docs?at=some-branch', ); + expect(response.sha).toBe('12ab34cd56ef'); + const files = await response.files(); expect(files.length).toBe(1); @@ -139,37 +163,12 @@ describe('BitbucketUrlReader', () => { }); it('returns the wanted files from an archive with a subpath', async () => { - worker.use( - rest.get( - 'https://bitbucket.org/backstage/mock/get/master.zip', - (_, res, ctx) => - res( - ctx.status(200), - ctx.set('Content-Type', 'application/zip'), - ctx.body(repoBuffer), - ), - ), - rest.get( - 'https://api.bitbucket.org/2.0/repositories/backstage/mock/commits/master', - (_, res, ctx) => - res( - ctx.status(200), - ctx.json({ - values: [{ hash: '12ab34cd56ef78gh90ij12kl34mn56op78qr90st' }], - }), - ), - ), - ); - - const processor = new BitbucketUrlReader( - { host: 'bitbucket.org', apiBaseUrl: 'https://api.bitbucket.org/2.0' }, - { treeResponseFactory }, - ); - - const response = await processor.readTree( + const response = await bitbucketProcessor.readTree( 'https://bitbucket.org/backstage/mock/src/master/docs', ); + expect(response.sha).toBe('12ab34cd56ef'); + const files = await response.files(); expect(files.length).toBe(1); @@ -177,5 +176,25 @@ describe('BitbucketUrlReader', () => { expect(indexMarkdownFile.toString()).toBe('# Test\n'); }); + + it('throws a NotModifiedError when given a sha in options', async () => { + const fnBitbucket = async () => { + await bitbucketProcessor.readTree( + 'https://bitbucket.org/backstage/mock', + { sha: '12ab34cd56ef' }, + ); + }; + + await expect(fnBitbucket).rejects.toThrow(NotModifiedError); + }); + + it('should not throw a NotModifiedError when given an outdated sha in options', async () => { + const response = await bitbucketProcessor.readTree( + 'https://bitbucket.org/backstage/mock', + { sha: 'outdatedSha123abc' }, + ); + + expect(response.sha).toBe('12ab34cd56ef'); + }); }); }); diff --git a/packages/backend-common/src/reading/BitbucketUrlReader.ts b/packages/backend-common/src/reading/BitbucketUrlReader.ts index 7f462e5bf0..606aecd023 100644 --- a/packages/backend-common/src/reading/BitbucketUrlReader.ts +++ b/packages/backend-common/src/reading/BitbucketUrlReader.ts @@ -25,7 +25,7 @@ import { import fetch from 'cross-fetch'; import parseGitUrl from 'git-url-parse'; import { Readable } from 'stream'; -import { NotFoundError } from '../errors'; +import { NotFoundError, NotModifiedError } from '../errors'; import { ReadTreeResponseFactory } from './tree'; import { ReaderFactory, @@ -105,6 +105,11 @@ export class BitbucketUrlReader implements UrlReader { url, ); + const lastCommitShortHash = await this.getLastCommitShortHash(url); + if (options?.sha && options.sha === lastCommitShortHash) { + throw new NotModifiedError(); + } + const isHosted = resource === 'bitbucket.org'; const downloadUrl = await getBitbucketDownloadUrl(url, this.config); @@ -122,7 +127,6 @@ export class BitbucketUrlReader implements UrlReader { let folderPath = `${project}-${repoName}`; if (isHosted) { - const lastCommitShortHash = await this.getLastCommitShortHash(url); folderPath = `${project}-${repoName}-${lastCommitShortHash}`; } @@ -133,8 +137,7 @@ export class BitbucketUrlReader implements UrlReader { }); const response = archiveResponse as ReadTreeResponse; - // TODO: Just a placeholder for now. - response.sha = ''; + response.sha = lastCommitShortHash; return response; } @@ -147,7 +150,7 @@ export class BitbucketUrlReader implements UrlReader { return `bitbucket{host=${host},authed=${authed}}`; } - private async getLastCommitShortHash(url: string): Promise { + private async getLastCommitShortHash(url: string): Promise { const { name: repoName, owner: project, ref } = parseGitUrl(url); let branch = ref; diff --git a/packages/backend-common/src/reading/GithubUrlReader.test.ts b/packages/backend-common/src/reading/GithubUrlReader.test.ts index 24a49cf81c..a311da910a 100644 --- a/packages/backend-common/src/reading/GithubUrlReader.test.ts +++ b/packages/backend-common/src/reading/GithubUrlReader.test.ts @@ -133,7 +133,6 @@ describe('GithubUrlReader', () => { }; beforeEach(() => { - // For github.com host worker.use( rest.get( 'https://github.com/backstage/mock/archive/main.tar.gz', @@ -144,9 +143,6 @@ describe('GithubUrlReader', () => { ctx.body(repoBuffer), ), ), - ); - - worker.use( rest.get('https://api.github.com/repos/backstage/mock', (_, res, ctx) => res( ctx.status(200), @@ -154,9 +150,6 @@ describe('GithubUrlReader', () => { ctx.json(reposGithubApiResponse), ), ), - ); - - worker.use( rest.get( 'https://api.github.com/repos/backstage/mock/branches/main', (_, res, ctx) => @@ -166,17 +159,10 @@ describe('GithubUrlReader', () => { ctx.json(branchesApiResponse), ), ), - ); - - worker.use( rest.get( 'https://api.github.com/repos/backstage/mock/branches/branchDoesNotExist', (_, res, ctx) => res(ctx.status(404)), ), - ); - - // For a GHE host - worker.use( rest.get( 'https://ghe.github.com/backstage/mock/archive/main.tar.gz', (_, res, ctx) => @@ -186,9 +172,6 @@ describe('GithubUrlReader', () => { ctx.body(repoBuffer), ), ), - ); - - worker.use( rest.get( 'https://ghe.github.com/api/v3/repos/backstage/mock', (_, res, ctx) => @@ -198,9 +181,6 @@ describe('GithubUrlReader', () => { ctx.json(reposGheApiResponse), ), ), - ); - - worker.use( rest.get( 'https://ghe.github.com/api/v3/repos/backstage/mock/branches/main', (_, res, ctx) => diff --git a/packages/backend-common/src/reading/GitlabUrlReader.test.ts b/packages/backend-common/src/reading/GitlabUrlReader.test.ts index a2dda48b16..823555d37d 100644 --- a/packages/backend-common/src/reading/GitlabUrlReader.test.ts +++ b/packages/backend-common/src/reading/GitlabUrlReader.test.ts @@ -179,9 +179,6 @@ describe('GitlabUrlReader', () => { ctx.body(archiveBuffer), ), ), - ); - - worker.use( rest.get( 'https://gitlab.com/api/v4/projects/backstage%2Fmock', (_, res, ctx) => @@ -191,9 +188,6 @@ describe('GitlabUrlReader', () => { ctx.json(projectGitlabApiResponse), ), ), - ); - - worker.use( rest.get( 'https://gitlab.com/api/v4/projects/backstage%2Fmock/repository/branches/main', (_, res, ctx) => @@ -203,16 +197,10 @@ describe('GitlabUrlReader', () => { ctx.json(branchGitlabApiResponse), ), ), - ); - - worker.use( rest.get( 'https://gitlab.com/api/v4/projects/backstage%2Fmock/repository/branches/branchDoesNotExist', (_, res, ctx) => res(ctx.status(404)), ), - ); - - worker.use( rest.get( 'https://gitlab.mycompany.com/api/v4/projects/backstage%2Fmock', (_, res, ctx) => @@ -222,9 +210,6 @@ describe('GitlabUrlReader', () => { ctx.json(projectGitlabApiResponse), ), ), - ); - - worker.use( rest.get( 'https://gitlab.mycompany.com/api/v4/projects/backstage%2Fmock/repository/branches/main', (_, res, ctx) => @@ -234,9 +219,6 @@ describe('GitlabUrlReader', () => { ctx.json(branchGitlabApiResponse), ), ), - ); - - worker.use( rest.get( 'https://gitlab.mycompany.com/api/v4/projects/backstage%2Fmock/repository/archive.zip?sha=main', (_, res, ctx) => From 79f9a97428ea781136f46773c23769a52dfd460b Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Sun, 17 Jan 2021 15:02:55 +0100 Subject: [PATCH 18/75] backend-common: UrlReader/Azure implement SHA based caching --- .../src/reading/AzureUrlReader.test.ts | 62 +++++++++++++++++-- .../src/reading/AzureUrlReader.ts | 25 +++++++- packages/integration/src/azure/core.ts | 56 +++++++++++++++++ packages/integration/src/azure/index.ts | 1 + 4 files changed, 136 insertions(+), 8 deletions(-) diff --git a/packages/backend-common/src/reading/AzureUrlReader.test.ts b/packages/backend-common/src/reading/AzureUrlReader.test.ts index f729316223..b492b26609 100644 --- a/packages/backend-common/src/reading/AzureUrlReader.test.ts +++ b/packages/backend-common/src/reading/AzureUrlReader.test.ts @@ -23,6 +23,7 @@ import { getVoidLogger } from '../logging'; import { AzureUrlReader } from './AzureUrlReader'; import { msw } from '@backstage/test-utils'; import { ReadTreeResponseFactory } from './tree'; +import { NotModifiedError } from '../errors'; const logger = getVoidLogger(); @@ -142,6 +143,11 @@ describe('AzureUrlReader', () => { path.resolve('src', 'reading', '__fixtures__', 'mock-main.zip'), ); + const processor = new AzureUrlReader( + { host: 'dev.azure.com' }, + { treeResponseFactory }, + ); + beforeEach(() => { worker.use( rest.get( @@ -153,19 +159,65 @@ describe('AzureUrlReader', () => { ctx.body(repoBuffer), ), ), + rest.get( + // https://docs.microsoft.com/en-us/rest/api/azure/devops/git/commits/get%20commits?view=azure-devops-rest-6.0#on-a-branch + 'https://dev.azure.com/organization/project/_apis/git/repositories/repository/commits', + (_, res, ctx) => + res( + ctx.status(200), + ctx.json({ + count: 2, + value: [ + { + commitId: '123abc2', + comment: 'second commit', + }, + { + commitId: '123abc1', + comment: 'first commit', + }, + ], + }), + ), + ), ); }); it('returns the wanted files from an archive', async () => { - const processor = new AzureUrlReader( - { host: 'dev.azure.com' }, - { treeResponseFactory }, - ); - const response = await processor.readTree( 'https://dev.azure.com/organization/project/_git/repository', ); + expect(response.sha).toBe('123abc2'); + + const files = await response.files(); + + expect(files.length).toBe(2); + const mkDocsFile = await files[0].content(); + const indexMarkdownFile = await files[1].content(); + + expect(mkDocsFile.toString()).toBe('site_name: Test\n'); + expect(indexMarkdownFile.toString()).toBe('# Test\n'); + }); + + it('throws a NotModifiedError when given a sha in options', async () => { + const fnAzure = async () => { + await processor.readTree( + 'https://dev.azure.com/organization/project/_git/repository', + { sha: '123abc2' }, + ); + }; + + await expect(fnAzure).rejects.toThrow(NotModifiedError); + }); + + it('should not throw a NotModifiedError when given an outdated sha in options', async () => { + const response = await processor.readTree( + 'https://dev.azure.com/organization/project/_git/repository', + { sha: 'outdated123abc' }, + ); + + expect(response.sha).toBe('123abc2'); const files = await response.files(); expect(files.length).toBe(2); diff --git a/packages/backend-common/src/reading/AzureUrlReader.ts b/packages/backend-common/src/reading/AzureUrlReader.ts index 4535b329d7..c40291ec85 100644 --- a/packages/backend-common/src/reading/AzureUrlReader.ts +++ b/packages/backend-common/src/reading/AzureUrlReader.ts @@ -20,10 +20,11 @@ import { getAzureFileFetchUrl, getAzureDownloadUrl, getAzureRequestOptions, + getAzureCommitsUrl, } from '@backstage/integration'; import fetch from 'cross-fetch'; import { Readable } from 'stream'; -import { NotFoundError } from '../errors'; +import { NotFoundError, NotModifiedError } from '../errors'; import { ReaderFactory, ReadTreeOptions, @@ -75,6 +76,25 @@ export class AzureUrlReader implements UrlReader { url: string, options?: ReadTreeOptions, ): Promise { + // Get latest commit SHA + + const commitsAzureResponse = await fetch( + getAzureCommitsUrl(url), + getAzureRequestOptions(this.options), + ); + if (!commitsAzureResponse.ok) { + const message = `Failed to read tree from ${url}, ${commitsAzureResponse.status} ${commitsAzureResponse.statusText}`; + if (commitsAzureResponse.status === 404) { + throw new NotFoundError(message); + } + throw new Error(message); + } + + const commitSha = (await commitsAzureResponse.json()).value[0].commitId; + if (options?.sha && options.sha === commitSha) { + throw new NotModifiedError(); + } + const archiveAzureResponse = await fetch( getAzureDownloadUrl(url), getAzureRequestOptions(this.options, { Accept: 'application/zip' }), @@ -93,8 +113,7 @@ export class AzureUrlReader implements UrlReader { }); const response = archiveResponse as ReadTreeResponse; - // TODO: Just a placeholder for now. - response.sha = ''; + response.sha = commitSha; return response; } diff --git a/packages/integration/src/azure/core.ts b/packages/integration/src/azure/core.ts index 7900774c79..73ce4f83d7 100644 --- a/packages/integration/src/azure/core.ts +++ b/packages/integration/src/azure/core.ts @@ -108,6 +108,62 @@ export function getAzureDownloadUrl(url: string): string { return `${protocol}://${resource}/${organization}/${project}/_apis/git/repositories/${repoName}/items?recursionLevel=full&download=true&api-version=6.0${scopePath}`; } +/** + * Given a URL, return the API URL to fetch commits on the branch. + * + * @param url A URL pointing to a repository or a sub-path + */ +export function getAzureCommitsUrl(url: string): string { + try { + const parsedUrl = new URL(url); + + const [ + empty, + userOrOrg, + project, + srcKeyword, + repoName, + ] = parsedUrl.pathname.split('/'); + + // Remove the "GB" from "GBmain" for example. + const ref = parsedUrl.searchParams.get('version')?.substr(2); + + if ( + empty !== '' || + userOrOrg === '' || + project === '' || + srcKeyword !== '_git' || + repoName === '' + ) { + throw new Error('Wrong Azure Devops URL'); + } + + // transform to commits api + parsedUrl.pathname = [ + empty, + userOrOrg, + project, + '_apis', + 'git', + 'repositories', + repoName, + 'commits', + ].join('/'); + + const queryParams = []; + if (ref) { + queryParams.push(`searchCriteria.itemVersion.version=${ref}`); + } + parsedUrl.search = queryParams.join('&'); + + parsedUrl.protocol = 'https'; + + return parsedUrl.toString(); + } catch (e) { + throw new Error(`Incorrect URL: ${url}, ${e}`); + } +} + /** * Gets the request options necessary to make requests to a given provider. * diff --git a/packages/integration/src/azure/index.ts b/packages/integration/src/azure/index.ts index 365e4cdcdc..6d57437779 100644 --- a/packages/integration/src/azure/index.ts +++ b/packages/integration/src/azure/index.ts @@ -23,4 +23,5 @@ export { getAzureDownloadUrl, getAzureFileFetchUrl, getAzureRequestOptions, + getAzureCommitsUrl, } from './core'; From 8565ad2279c157cd0c404499efe51b6942a6b685 Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Mon, 18 Jan 2021 22:50:47 +0100 Subject: [PATCH 19/75] 1. Use etag over sha for the identifier 2. Re-combine Resonse types into one --- .changeset/khaki-icons-trade.md | 10 ++++-- .github/styles/vocab.txt | 1 + .../src/reading/AzureUrlReader.test.ts | 12 +++---- .../src/reading/AzureUrlReader.ts | 9 ++---- .../src/reading/BitbucketUrlReader.test.ts | 16 +++++----- .../src/reading/BitbucketUrlReader.ts | 9 ++---- .../src/reading/GithubUrlReader.test.ts | 14 ++++----- .../src/reading/GithubUrlReader.ts | 9 ++---- .../src/reading/GitlabUrlReader.test.ts | 10 +++--- .../src/reading/GitlabUrlReader.ts | 9 ++---- .../reading/tree/ReadTreeResponseFactory.ts | 14 ++++----- .../reading/tree/TarArchiveResponse.test.ts | 31 +++++++++++++------ .../src/reading/tree/TarArchiveResponse.ts | 8 +++-- .../reading/tree/ZipArchiveResponse.test.ts | 31 +++++++++++++------ .../src/reading/tree/ZipArchiveResponse.ts | 8 +++-- packages/backend-common/src/reading/types.ts | 27 ++++++++-------- packages/techdocs-common/src/helpers.test.ts | 2 +- 17 files changed, 124 insertions(+), 96 deletions(-) diff --git a/.changeset/khaki-icons-trade.md b/.changeset/khaki-icons-trade.md index 37bafd8d33..4d30c3977e 100644 --- a/.changeset/khaki-icons-trade.md +++ b/.changeset/khaki-icons-trade.md @@ -2,7 +2,11 @@ '@backstage/backend-common': patch --- -1. URL Reader's `readTree` method now returns a `sha` in the response along with the blob. The SHA belongs to the latest commit on the target. `readTree` also takes an optional `sha` in its options and throws a `NotModifiedError` if the SHA matches with the latest commit SHA on the url's target. This can be used in building a cache when working with URL Reader. +1. URL Reader's `readTree` method now returns an `etag` in the response along with the blob. The etag is an identifier of the blob and will only change if the blob is modified on the target. Usually it is set to the latest commit SHA on the target. + +`readTree` also takes an optional `etag` in its options and throws a `NotModifiedError` if the etag matches with the etag of the resource. + +So, the `etag` can be used in building a cache when working with URL Reader. An example - @@ -11,11 +15,11 @@ const response = await reader.readTree( 'https://github.com/backstage/backstage', ); -const commitSha = response.sha; +const etag = response.etag; // Will throw a new NotModifiedError (exported from @backstage/backstage-common) await reader.readTree('https://github.com/backstage/backstage', { - sha: commitSha, + etag, }); ``` diff --git a/.github/styles/vocab.txt b/.github/styles/vocab.txt index c3c67ab66e..96581d4b79 100644 --- a/.github/styles/vocab.txt +++ b/.github/styles/vocab.txt @@ -67,6 +67,7 @@ Dominik dtuite dzolotusky Ek +etag env Env eslint diff --git a/packages/backend-common/src/reading/AzureUrlReader.test.ts b/packages/backend-common/src/reading/AzureUrlReader.test.ts index b492b26609..20f8feba42 100644 --- a/packages/backend-common/src/reading/AzureUrlReader.test.ts +++ b/packages/backend-common/src/reading/AzureUrlReader.test.ts @@ -188,7 +188,7 @@ describe('AzureUrlReader', () => { 'https://dev.azure.com/organization/project/_git/repository', ); - expect(response.sha).toBe('123abc2'); + expect(response.etag).toBe('123abc2'); const files = await response.files(); @@ -200,24 +200,24 @@ describe('AzureUrlReader', () => { expect(indexMarkdownFile.toString()).toBe('# Test\n'); }); - it('throws a NotModifiedError when given a sha in options', async () => { + it('throws a NotModifiedError when given a etag in options', async () => { const fnAzure = async () => { await processor.readTree( 'https://dev.azure.com/organization/project/_git/repository', - { sha: '123abc2' }, + { etag: '123abc2' }, ); }; await expect(fnAzure).rejects.toThrow(NotModifiedError); }); - it('should not throw a NotModifiedError when given an outdated sha in options', async () => { + it('should not throw a NotModifiedError when given an outdated etag in options', async () => { const response = await processor.readTree( 'https://dev.azure.com/organization/project/_git/repository', - { sha: 'outdated123abc' }, + { etag: 'outdated123abc' }, ); - expect(response.sha).toBe('123abc2'); + expect(response.etag).toBe('123abc2'); const files = await response.files(); expect(files.length).toBe(2); diff --git a/packages/backend-common/src/reading/AzureUrlReader.ts b/packages/backend-common/src/reading/AzureUrlReader.ts index c40291ec85..59e437607f 100644 --- a/packages/backend-common/src/reading/AzureUrlReader.ts +++ b/packages/backend-common/src/reading/AzureUrlReader.ts @@ -91,7 +91,7 @@ export class AzureUrlReader implements UrlReader { } const commitSha = (await commitsAzureResponse.json()).value[0].commitId; - if (options?.sha && options.sha === commitSha) { + if (options?.etag && options.etag === commitSha) { throw new NotModifiedError(); } @@ -107,14 +107,11 @@ export class AzureUrlReader implements UrlReader { throw new Error(message); } - const archiveResponse = await this.deps.treeResponseFactory.fromZipArchive({ + return await this.deps.treeResponseFactory.fromZipArchive({ stream: (archiveAzureResponse.body as unknown) as Readable, + etag: commitSha, filter: options?.filter, }); - - const response = archiveResponse as ReadTreeResponse; - response.sha = commitSha; - return response; } toString() { diff --git a/packages/backend-common/src/reading/BitbucketUrlReader.test.ts b/packages/backend-common/src/reading/BitbucketUrlReader.test.ts index 111aff753c..3542e822e1 100644 --- a/packages/backend-common/src/reading/BitbucketUrlReader.test.ts +++ b/packages/backend-common/src/reading/BitbucketUrlReader.test.ts @@ -135,7 +135,7 @@ describe('BitbucketUrlReader', () => { 'https://bitbucket.org/backstage/mock/src/master', ); - expect(response.sha).toBe('12ab34cd56ef'); + expect(response.etag).toBe('12ab34cd56ef'); const files = await response.files(); @@ -152,7 +152,7 @@ describe('BitbucketUrlReader', () => { 'https://bitbucket.mycompany.net/projects/backstage/repos/mock/browse/docs?at=some-branch', ); - expect(response.sha).toBe('12ab34cd56ef'); + expect(response.etag).toBe('12ab34cd56ef'); const files = await response.files(); @@ -167,7 +167,7 @@ describe('BitbucketUrlReader', () => { 'https://bitbucket.org/backstage/mock/src/master/docs', ); - expect(response.sha).toBe('12ab34cd56ef'); + expect(response.etag).toBe('12ab34cd56ef'); const files = await response.files(); @@ -177,24 +177,24 @@ describe('BitbucketUrlReader', () => { expect(indexMarkdownFile.toString()).toBe('# Test\n'); }); - it('throws a NotModifiedError when given a sha in options', async () => { + it('throws a NotModifiedError when given a etag in options', async () => { const fnBitbucket = async () => { await bitbucketProcessor.readTree( 'https://bitbucket.org/backstage/mock', - { sha: '12ab34cd56ef' }, + { etag: '12ab34cd56ef' }, ); }; await expect(fnBitbucket).rejects.toThrow(NotModifiedError); }); - it('should not throw a NotModifiedError when given an outdated sha in options', async () => { + it('should not throw a NotModifiedError when given an outdated etag in options', async () => { const response = await bitbucketProcessor.readTree( 'https://bitbucket.org/backstage/mock', - { sha: 'outdatedSha123abc' }, + { etag: 'outdatedetag123abc' }, ); - expect(response.sha).toBe('12ab34cd56ef'); + expect(response.etag).toBe('12ab34cd56ef'); }); }); }); diff --git a/packages/backend-common/src/reading/BitbucketUrlReader.ts b/packages/backend-common/src/reading/BitbucketUrlReader.ts index 606aecd023..869870f248 100644 --- a/packages/backend-common/src/reading/BitbucketUrlReader.ts +++ b/packages/backend-common/src/reading/BitbucketUrlReader.ts @@ -106,7 +106,7 @@ export class BitbucketUrlReader implements UrlReader { ); const lastCommitShortHash = await this.getLastCommitShortHash(url); - if (options?.sha && options.sha === lastCommitShortHash) { + if (options?.etag && options.etag === lastCommitShortHash) { throw new NotModifiedError(); } @@ -130,15 +130,12 @@ export class BitbucketUrlReader implements UrlReader { folderPath = `${project}-${repoName}-${lastCommitShortHash}`; } - const archiveResponse = await this.treeResponseFactory.fromZipArchive({ + return await this.treeResponseFactory.fromZipArchive({ stream: (archiveBitbucketResponse.body as unknown) as Readable, path: `${folderPath}/${filepath}`, + etag: lastCommitShortHash, filter: options?.filter, }); - - const response = archiveResponse as ReadTreeResponse; - response.sha = lastCommitShortHash; - return response; } toString() { diff --git a/packages/backend-common/src/reading/GithubUrlReader.test.ts b/packages/backend-common/src/reading/GithubUrlReader.test.ts index a311da910a..26ba91bd29 100644 --- a/packages/backend-common/src/reading/GithubUrlReader.test.ts +++ b/packages/backend-common/src/reading/GithubUrlReader.test.ts @@ -128,7 +128,7 @@ describe('GithubUrlReader', () => { const branchesApiResponse = { name: 'main', commit: { - sha: 'sha123abc', + sha: 'etag123abc', }, }; @@ -198,7 +198,7 @@ describe('GithubUrlReader', () => { 'https://github.com/backstage/mock/tree/main', ); - expect(response.sha).toBe('sha123abc'); + expect(response.etag).toBe('etag123abc'); const files = await response.files(); @@ -272,10 +272,10 @@ describe('GithubUrlReader', () => { expect(indexMarkdownFile.toString()).toBe('# Test\n'); }); - it('throws a NotModifiedError when given a sha in options', async () => { + it('throws a NotModifiedError when given a etag in options', async () => { const fnGithub = async () => { await githubProcessor.readTree('https://github.com/backstage/mock', { - sha: 'sha123abc', + etag: 'etag123abc', }); }; @@ -283,7 +283,7 @@ describe('GithubUrlReader', () => { await gheProcessor.readTree( 'https://ghe.github.com/backstage/mock/tree/main/docs', { - sha: 'sha123abc', + etag: 'etag123abc', }, ); }; @@ -292,11 +292,11 @@ describe('GithubUrlReader', () => { await expect(fnGhe).rejects.toThrow(NotModifiedError); }); - it('should not throw error when given an outdated sha in options', async () => { + it('should not throw error when given an outdated etag in options', async () => { const response = await githubProcessor.readTree( 'https://github.com/backstage/mock/tree/main', { - sha: 'outdatedSha123abc', + etag: 'outdatedetag123abc', }, ); expect((await response.files()).length).toBe(2); diff --git a/packages/backend-common/src/reading/GithubUrlReader.ts b/packages/backend-common/src/reading/GithubUrlReader.ts index 8e0410642b..1a8133fceb 100644 --- a/packages/backend-common/src/reading/GithubUrlReader.ts +++ b/packages/backend-common/src/reading/GithubUrlReader.ts @@ -151,7 +151,7 @@ export class GithubUrlReader implements UrlReader { } const commitSha = (await branchGitHubResponse.json()).commit.sha; - if (options?.sha && options.sha === commitSha) { + if (options?.etag && options.etag === commitSha) { throw new NotModifiedError(); } @@ -179,17 +179,14 @@ export class GithubUrlReader implements UrlReader { const path = `${repoName}-${branch}/${filepath}`; - const archiveResponse = await this.deps.treeResponseFactory.fromTarArchive({ + return await this.deps.treeResponseFactory.fromTarArchive({ // TODO(Rugvip): Underlying implementation of fetch will be node-fetch, we probably want // to stick to using that in exclusively backend code. stream: (archive.body as unknown) as Readable, path, + etag: commitSha, filter: options?.filter, }); - - const response = archiveResponse as ReadTreeResponse; - response.sha = commitSha; - return response; } toString() { diff --git a/packages/backend-common/src/reading/GitlabUrlReader.test.ts b/packages/backend-common/src/reading/GitlabUrlReader.test.ts index 823555d37d..2e66794397 100644 --- a/packages/backend-common/src/reading/GitlabUrlReader.test.ts +++ b/packages/backend-common/src/reading/GitlabUrlReader.test.ts @@ -284,10 +284,10 @@ describe('GitlabUrlReader', () => { expect(indexMarkdownFile.toString()).toBe('# Test\n'); }); - it('throws a NotModifiedError when given a sha in options', async () => { + it('throws a NotModifiedError when given a etag in options', async () => { const fnGitlab = async () => { await gitlabProcessor.readTree('https://gitlab.com/backstage/mock', { - sha: 'sha123abc', + etag: 'sha123abc', }); }; @@ -295,7 +295,7 @@ describe('GitlabUrlReader', () => { await hostedGitlabProcessor.readTree( 'https://gitlab.mycompany.com/backstage/mock', { - sha: 'sha123abc', + etag: 'sha123abc', }, ); }; @@ -304,11 +304,11 @@ describe('GitlabUrlReader', () => { await expect(fnHostedGitlab).rejects.toThrow(NotModifiedError); }); - it('should not throw error when given an outdated sha in options', async () => { + it('should not throw error when given an outdated etag in options', async () => { const response = await gitlabProcessor.readTree( 'https://gitlab.com/backstage/mock/tree/main', { - sha: 'outdatedSha123abc', + etag: 'outdatedsha123abc', }, ); expect((await response.files()).length).toBe(2); diff --git a/packages/backend-common/src/reading/GitlabUrlReader.ts b/packages/backend-common/src/reading/GitlabUrlReader.ts index 0c0ce4f565..647c6fd644 100644 --- a/packages/backend-common/src/reading/GitlabUrlReader.ts +++ b/packages/backend-common/src/reading/GitlabUrlReader.ts @@ -121,7 +121,7 @@ export class GitlabUrlReader implements UrlReader { const commitSha = (await branchGitlabResponse.json()).commit.id; - if (options?.sha && options.sha === commitSha) { + if (options?.etag && options.etag === commitSha) { throw new NotModifiedError(); } @@ -144,15 +144,12 @@ export class GitlabUrlReader implements UrlReader { ? `${repoName}-${branch}-${commitSha}/${filepath}/` : ''; - const archiveResponse = await this.treeResponseFactory.fromZipArchive({ + return await this.treeResponseFactory.fromZipArchive({ stream: (archiveGitLabResponse.body as unknown) as Readable, path, + etag: commitSha, filter: options?.filter, }); - - const response = archiveResponse as ReadTreeResponse; - response.sha = commitSha; - return response; } toString() { diff --git a/packages/backend-common/src/reading/tree/ReadTreeResponseFactory.ts b/packages/backend-common/src/reading/tree/ReadTreeResponseFactory.ts index cc229e9d5a..7332154d09 100644 --- a/packages/backend-common/src/reading/tree/ReadTreeResponseFactory.ts +++ b/packages/backend-common/src/reading/tree/ReadTreeResponseFactory.ts @@ -17,7 +17,7 @@ import os from 'os'; import { Readable } from 'stream'; import { Config } from '@backstage/config'; -import { ReadTreeArchiveResponse } from '../types'; +import { ReadTreeResponse } from '../types'; import { TarArchiveResponse } from './TarArchiveResponse'; import { ZipArchiveResponse } from './ZipArchiveResponse'; @@ -26,6 +26,8 @@ type FromArchiveOptions = { stream: Readable; // If set, the root of the tree will be set to the given directory path. path?: string; + // etag of the blob + etag: string; // Filter passed on from the ReadTreeOptions filter?: (path: string) => boolean; }; @@ -40,24 +42,22 @@ export class ReadTreeResponseFactory { constructor(private readonly workDir: string) {} - async fromTarArchive( - options: FromArchiveOptions, - ): Promise { + async fromTarArchive(options: FromArchiveOptions): Promise { return new TarArchiveResponse( options.stream, options.path ?? '', this.workDir, + options.etag, options.filter, ); } - async fromZipArchive( - options: FromArchiveOptions, - ): Promise { + async fromZipArchive(options: FromArchiveOptions): Promise { return new ZipArchiveResponse( options.stream, options.path ?? '', this.workDir, + options.etag, options.filter, ); } diff --git a/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts b/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts index bbeee00c70..2cbfc4a89e 100644 --- a/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts +++ b/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts @@ -38,7 +38,7 @@ describe('TarArchiveResponse', () => { it('should read files', async () => { const stream = fs.createReadStream('/test-archive.tar.gz'); - const res = new TarArchiveResponse(stream, 'mock-main/', '/tmp'); + const res = new TarArchiveResponse(stream, 'mock-main/', '/tmp', 'etag'); const files = await res.files(); expect(files).toEqual([ @@ -61,8 +61,12 @@ describe('TarArchiveResponse', () => { it('should read files with filter', async () => { const stream = fs.createReadStream('/test-archive.tar.gz'); - const res = new TarArchiveResponse(stream, 'mock-main/', '/tmp', path => - path.endsWith('.yml'), + const res = new TarArchiveResponse( + stream, + 'mock-main/', + '/tmp', + 'etag', + path => path.endsWith('.yml'), ); const files = await res.files(); @@ -79,14 +83,14 @@ describe('TarArchiveResponse', () => { it('should read as archive and files', async () => { const stream = fs.createReadStream('/test-archive.tar.gz'); - const res = new TarArchiveResponse(stream, 'mock-main/', '/tmp'); + const res = new TarArchiveResponse(stream, 'mock-main/', '/tmp', 'etag'); const buffer = await res.archive(); await expect(res.archive()).rejects.toThrow( 'Response has already been read', ); - const res2 = new TarArchiveResponse(buffer, '', '/tmp'); + const res2 = new TarArchiveResponse(buffer, '', '/tmp', 'etag'); const files = await res2.files(); expect(files).toEqual([ @@ -109,7 +113,7 @@ describe('TarArchiveResponse', () => { it('should extract entire archive into directory', async () => { const stream = fs.createReadStream('/test-archive.tar.gz'); - const res = new TarArchiveResponse(stream, '', '/tmp'); + const res = new TarArchiveResponse(stream, '', '/tmp', 'etag'); const dir = await res.dir(); await expect( @@ -123,7 +127,12 @@ describe('TarArchiveResponse', () => { it('should extract archive into directory with a subpath', async () => { const stream = fs.createReadStream('/test-archive.tar.gz'); - const res = new TarArchiveResponse(stream, 'mock-main/docs/', '/tmp'); + const res = new TarArchiveResponse( + stream, + 'mock-main/docs/', + '/tmp', + 'etag', + ); const dir = await res.dir(); expect(dir).toMatch(/^[\/\\]tmp[\/\\].*$/); @@ -135,8 +144,12 @@ describe('TarArchiveResponse', () => { it('should extract archive into directory with a subpath and filter', async () => { const stream = fs.createReadStream('/test-archive.tar.gz'); - const res = new TarArchiveResponse(stream, 'mock-main/', '/tmp', path => - path.endsWith('.yml'), + const res = new TarArchiveResponse( + stream, + 'mock-main/', + '/tmp', + 'etag', + path => path.endsWith('.yml'), ); const dir = await res.dir({ targetDir: '/tmp' }); diff --git a/packages/backend-common/src/reading/tree/TarArchiveResponse.ts b/packages/backend-common/src/reading/tree/TarArchiveResponse.ts index e8929fbe9b..f44adcc7b2 100644 --- a/packages/backend-common/src/reading/tree/TarArchiveResponse.ts +++ b/packages/backend-common/src/reading/tree/TarArchiveResponse.ts @@ -21,7 +21,7 @@ import { Readable, pipeline as pipelineCb } from 'stream'; import { promisify } from 'util'; import concatStream from 'concat-stream'; import { - ReadTreeArchiveResponse, + ReadTreeResponse, ReadTreeResponseFile, ReadTreeResponseDirOptions, } from '../types'; @@ -34,13 +34,15 @@ const pipeline = promisify(pipelineCb); /** * Wraps a tar archive stream into a tree response reader. */ -export class TarArchiveResponse implements ReadTreeArchiveResponse { +export class TarArchiveResponse implements ReadTreeResponse { private read = false; + public readonly etag; constructor( private readonly stream: Readable, private readonly subPath: string, private readonly workDir: string, + etag: string, private readonly filter?: (path: string) => boolean, ) { if (subPath) { @@ -53,6 +55,8 @@ export class TarArchiveResponse implements ReadTreeArchiveResponse { ); } } + + this.etag = etag; } // Make sure the input stream is only read once diff --git a/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts b/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts index 345ec7a783..b42ec79d81 100644 --- a/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts +++ b/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts @@ -38,7 +38,7 @@ describe('ZipArchiveResponse', () => { it('should read files', async () => { const stream = fs.createReadStream('/test-archive.zip'); - const res = new ZipArchiveResponse(stream, 'mock-main/', '/tmp'); + const res = new ZipArchiveResponse(stream, 'mock-main/', '/tmp', 'etag'); const files = await res.files(); expect(files).toEqual([ @@ -61,8 +61,12 @@ describe('ZipArchiveResponse', () => { it('should read files with filter', async () => { const stream = fs.createReadStream('/test-archive.zip'); - const res = new ZipArchiveResponse(stream, 'mock-main/', '/tmp', path => - path.endsWith('.yml'), + const res = new ZipArchiveResponse( + stream, + 'mock-main/', + '/tmp', + 'etag', + path => path.endsWith('.yml'), ); const files = await res.files(); @@ -79,14 +83,14 @@ describe('ZipArchiveResponse', () => { it('should read as archive and files', async () => { const stream = fs.createReadStream('/test-archive.zip'); - const res = new ZipArchiveResponse(stream, 'mock-main/', '/tmp'); + const res = new ZipArchiveResponse(stream, 'mock-main/', '/tmp', 'etag'); const buffer = await res.archive(); await expect(res.archive()).rejects.toThrow( 'Response has already been read', ); - const res2 = new ZipArchiveResponse(buffer, '', '/tmp'); + const res2 = new ZipArchiveResponse(buffer, '', '/tmp', 'etag'); const files = await res2.files(); expect(files).toEqual([ @@ -109,7 +113,7 @@ describe('ZipArchiveResponse', () => { it('should extract entire archive into directory', async () => { const stream = fs.createReadStream('/test-archive.zip'); - const res = new ZipArchiveResponse(stream, '', '/tmp'); + const res = new ZipArchiveResponse(stream, '', '/tmp', 'etag'); const dir = await res.dir(); await expect( @@ -123,7 +127,12 @@ describe('ZipArchiveResponse', () => { it('should extract archive into directory with a subpath', async () => { const stream = fs.createReadStream('/test-archive.zip'); - const res = new ZipArchiveResponse(stream, 'mock-main/docs/', '/tmp'); + const res = new ZipArchiveResponse( + stream, + 'mock-main/docs/', + '/tmp', + 'etag', + ); const dir = await res.dir(); expect(dir).toMatch(/^[\/\\]tmp[\/\\].*$/); @@ -135,8 +144,12 @@ describe('ZipArchiveResponse', () => { it('should extract archive into directory with a subpath and filter', async () => { const stream = fs.createReadStream('/test-archive.zip'); - const res = new ZipArchiveResponse(stream, 'mock-main/', '/tmp', path => - path.endsWith('.yml'), + const res = new ZipArchiveResponse( + stream, + 'mock-main/', + '/tmp', + 'etag', + path => path.endsWith('.yml'), ); const dir = await res.dir({ targetDir: '/tmp' }); diff --git a/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts b/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts index 48ba4f19ee..7550dc1259 100644 --- a/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts +++ b/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts @@ -20,7 +20,7 @@ import unzipper, { Entry } from 'unzipper'; import archiver from 'archiver'; import { Readable } from 'stream'; import { - ReadTreeArchiveResponse, + ReadTreeResponse, ReadTreeResponseFile, ReadTreeResponseDirOptions, } from '../types'; @@ -28,13 +28,15 @@ import { /** * Wraps a zip archive stream into a tree response reader. */ -export class ZipArchiveResponse implements ReadTreeArchiveResponse { +export class ZipArchiveResponse implements ReadTreeResponse { private read = false; + public readonly etag; constructor( private readonly stream: Readable, private readonly subPath: string, private readonly workDir: string, + etag: string, private readonly filter?: (path: string) => boolean, ) { if (subPath) { @@ -47,6 +49,8 @@ export class ZipArchiveResponse implements ReadTreeArchiveResponse { ); } } + + this.etag = etag; } // Make sure the input stream is only read once diff --git a/packages/backend-common/src/reading/types.ts b/packages/backend-common/src/reading/types.ts index d5c24ed1db..e98f760d8f 100644 --- a/packages/backend-common/src/reading/types.ts +++ b/packages/backend-common/src/reading/types.ts @@ -34,17 +34,17 @@ export type ReadTreeOptions = { filter?(path: string): boolean; /** - * A commit SHA can be provided to check whether readTree's response has changed from a previous execution. + * An etag can be provided to check whether readTree's response has changed from a previous execution. * - * In the readTree() response, a SHA is returned along with the tree blob. The SHA belongs to the - * latest commit on the target repository's branch that was used to read the blob. + * In the readTree() response, an etag is returned along with the tree blob. The etag is a unique identifer + * of the tree blob, usually the commit SHA or etag from the target. * - * When a SHA is given in ReadTreeOptions, readTree will first compare the SHA against the latest commit - * on the target branch. If they match, it will throw a NotModifiedError indicating that the readTree - * response will not differ from the previous response which included this particular SHA. If they mismatch, - * readTree will return a new SHA along with the rest of ReadTreeResponse. + * When a etag is given in ReadTreeOptions, readTree will first compare the etag against the etag + * on the target branch. If they match, readTree will throw a NotModifiedError indicating that the readTree + * response will not differ from the previous response which included this particular etag. If they mismatch, + * readTree will return the rest of ReadTreeResponse along with a new etag. */ - sha?: string; + etag?: string; }; /** @@ -80,7 +80,7 @@ export type ReadTreeResponseDirOptions = { targetDir?: string; }; -export type ReadTreeArchiveResponse = { +export type ReadTreeResponse = { files(): Promise; archive(): Promise; @@ -88,8 +88,9 @@ export type ReadTreeArchiveResponse = { * dir() extracts the tree response into a directory and returns the path of the directory. */ dir(options?: ReadTreeResponseDirOptions): Promise; -}; -export interface ReadTreeResponse extends ReadTreeArchiveResponse { - sha: string; -} + /** + * A unique identifer of the tree blob, usually the commit SHA or etag from the target. + */ + etag: string; +}; diff --git a/packages/techdocs-common/src/helpers.test.ts b/packages/techdocs-common/src/helpers.test.ts index 88fdeed498..740a08aaa2 100644 --- a/packages/techdocs-common/src/helpers.test.ts +++ b/packages/techdocs-common/src/helpers.test.ts @@ -135,7 +135,7 @@ describe('getDocFilesFromRepository', () => { archive: async () => { return Readable.from(''); }, - sha: '', + etag: '', }; } } From baeed36324f6a275f3885da8a86aea78b0cbd8d8 Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Mon, 18 Jan 2021 22:59:36 +0100 Subject: [PATCH 20/75] Convert NotModifiedError to 304 HTTP status code --- packages/backend-common/src/middleware/errorHandler.test.ts | 4 ++++ packages/backend-common/src/middleware/errorHandler.ts | 2 ++ 2 files changed, 6 insertions(+) diff --git a/packages/backend-common/src/middleware/errorHandler.test.ts b/packages/backend-common/src/middleware/errorHandler.test.ts index a7a3d64bd1..6d22c2f175 100644 --- a/packages/backend-common/src/middleware/errorHandler.test.ts +++ b/packages/backend-common/src/middleware/errorHandler.test.ts @@ -72,6 +72,9 @@ describe('errorHandler', () => { it('handles well-known error classes', async () => { const app = express(); + app.use('/NotModifiedError', () => { + throw new errors.NotModifiedError(); + }); app.use('/InputError', () => { throw new errors.InputError(); }); @@ -90,6 +93,7 @@ describe('errorHandler', () => { app.use(errorHandler()); const r = request(app); + expect((await r.get('/NotModifiedError')).status).toBe(304); expect((await r.get('/InputError')).status).toBe(400); expect((await r.get('/AuthenticationError')).status).toBe(401); expect((await r.get('/NotAllowedError')).status).toBe(403); diff --git a/packages/backend-common/src/middleware/errorHandler.ts b/packages/backend-common/src/middleware/errorHandler.ts index 7365ce8b93..a08849813d 100644 --- a/packages/backend-common/src/middleware/errorHandler.ts +++ b/packages/backend-common/src/middleware/errorHandler.ts @@ -101,6 +101,8 @@ function getStatusCode(error: Error): number { // Handle well-known error types switch (error.name) { + case errors.NotModifiedError.name: + return 304; case errors.InputError.name: return 400; case errors.AuthenticationError.name: From 380dd626fb1f42c24c8ce77eddfc5b93caa389e5 Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Mon, 18 Jan 2021 23:04:31 +0100 Subject: [PATCH 21/75] Safe nullish check with git-url-parse library responses --- packages/backend-common/src/reading/GithubUrlReader.ts | 2 +- packages/backend-common/src/reading/GitlabUrlReader.ts | 2 +- packages/integration/src/azure/core.ts | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/backend-common/src/reading/GithubUrlReader.ts b/packages/backend-common/src/reading/GithubUrlReader.ts index 1a8133fceb..d6612a9da2 100644 --- a/packages/backend-common/src/reading/GithubUrlReader.ts +++ b/packages/backend-common/src/reading/GithubUrlReader.ts @@ -130,7 +130,7 @@ export class GithubUrlReader implements UrlReader { // ref is an empty string if no branch is set in provided url to readTree. // Use GitHub API to get the default branch of the repository. - const branch = ref === '' ? repoResponseJson.default_branch : ref; + const branch = ref || repoResponseJson.default_branch; const branchesApiUrl = repoResponseJson.branches_url; // Fetch the latest commit in the provided or default branch to compare against diff --git a/packages/backend-common/src/reading/GitlabUrlReader.ts b/packages/backend-common/src/reading/GitlabUrlReader.ts index 647c6fd644..72db901ac1 100644 --- a/packages/backend-common/src/reading/GitlabUrlReader.ts +++ b/packages/backend-common/src/reading/GitlabUrlReader.ts @@ -99,7 +99,7 @@ export class GitlabUrlReader implements UrlReader { const projectGitlabResponseJson = await projectGitlabResponse.json(); // ref is an empty string if no branch is set in provided url to readTree. - const branch = ref === '' ? projectGitlabResponseJson.default_branch : ref; + const branch = ref || projectGitlabResponseJson.default_branch; // Fetch the latest commit in the provided or default branch to compare against // the provided sha. diff --git a/packages/integration/src/azure/core.ts b/packages/integration/src/azure/core.ts index 73ce4f83d7..b2878af89e 100644 --- a/packages/integration/src/azure/core.ts +++ b/packages/integration/src/azure/core.ts @@ -129,11 +129,11 @@ export function getAzureCommitsUrl(url: string): string { const ref = parsedUrl.searchParams.get('version')?.substr(2); if ( - empty !== '' || - userOrOrg === '' || - project === '' || + !!empty || + !userOrOrg || + !project || srcKeyword !== '_git' || - repoName === '' + !repoName ) { throw new Error('Wrong Azure Devops URL'); } From aca5158c7f03b3d7439aee9d162891fcb8fb5bed Mon Sep 17 00:00:00 2001 From: Jonah Back Date: Tue, 12 Jan 2021 14:50:04 -0800 Subject: [PATCH 22/75] feature: add auth backend for AWS ALB --- plugins/auth-backend/package.json | 8 +- .../src/providers/aws-alb/index.ts | 15 ++ .../src/providers/aws-alb/provider.test.ts | 225 ++++++++++++++++++ .../src/providers/aws-alb/provider.ts | 138 +++++++++++ yarn.lock | 27 ++- 5 files changed, 402 insertions(+), 11 deletions(-) create mode 100644 plugins/auth-backend/src/providers/aws-alb/index.ts create mode 100644 plugins/auth-backend/src/providers/aws-alb/provider.test.ts create mode 100644 plugins/auth-backend/src/providers/aws-alb/provider.ts diff --git a/plugins/auth-backend/package.json b/plugins/auth-backend/package.json index 8f88c69487..f8de2bc6f1 100644 --- a/plugins/auth-backend/package.json +++ b/plugins/auth-backend/package.json @@ -16,6 +16,11 @@ "url": "https://github.com/backstage/backstage", "directory": "plugins/auth-backend" }, + "jest": { + "moduleNameMapper": { + "jose/jwt/verify": "/../../../node_modules/jose/lib/jwt/verify" + } + }, "keywords": [ "backstage" ], @@ -44,7 +49,7 @@ "fs-extra": "^9.0.0", "got": "^11.5.2", "helmet": "^4.0.0", - "jose": "^1.27.1", + "jose": "^3.5.1", "jwt-decode": "^3.1.0", "knex": "^0.21.6", "moment": "^2.26.0", @@ -59,6 +64,7 @@ "passport-okta-oauth": "^0.0.1", "passport-onelogin-oauth": "^0.0.1", "passport-saml": "^2.0.0", + "r2": "^2.0.1", "uuid": "^8.0.0", "winston": "^3.2.1", "yn": "^4.0.0" diff --git a/plugins/auth-backend/src/providers/aws-alb/index.ts b/plugins/auth-backend/src/providers/aws-alb/index.ts new file mode 100644 index 0000000000..863d6e76e1 --- /dev/null +++ b/plugins/auth-backend/src/providers/aws-alb/index.ts @@ -0,0 +1,15 @@ +/* + * 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. + */ diff --git a/plugins/auth-backend/src/providers/aws-alb/provider.test.ts b/plugins/auth-backend/src/providers/aws-alb/provider.test.ts new file mode 100644 index 0000000000..55c27fd7ff --- /dev/null +++ b/plugins/auth-backend/src/providers/aws-alb/provider.test.ts @@ -0,0 +1,225 @@ +/* + * Copyright 2020 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 { getVoidLogger } from '@backstage/backend-common'; +import express from 'express'; +import * as jwtVerify from 'jose/jwt/verify'; + +import { AwsAlbAuthProvider } from './provider'; +import { UserEntityV1alpha1 } from '@backstage/catalog-model'; + +const mockKey = async () => { + return `-----BEGIN PUBLIC KEY----- +MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEnuN4LlaJhaUpx+qZFTzYCrSBLk0I +yOlxJ2VW88mLAQGJ7HPAvOdylxZsItMnzCuqNzZvie8m/NJsOjhDncVkrw== +-----END PUBLIC KEY----- +`; +}; + +jest.mock('r2', () => ({ + __esModule: true, + default: () => { + return { + text: mockKey(), + }; + }, +})); + +jest.mock('jose/jwt/verify', () => { + return { + __esModule: true, + default: jest.fn(), + }; +}); + +const identityResolutionCallbackMock = async (): Promise< + UserEntityV1alpha1 +> => { + return { + apiVersion: 'backstage.io/v1alpha1', + kind: 'User', + metadata: { + name: 'foo', + }, + spec: { + memberOf: [], + profile: { + displayName: 'Foo Bar', + }, + }, + }; +}; + +const identityResolutionCallbackRejectedMock = async (): Promise< + UserEntityV1alpha1 +> => { + throw new Error('failed'); +}; + +beforeEach(() => { + jest.clearAllMocks(); +}); + +describe('AwsALBAuthProvider', () => { + const catalogApi = { + /* eslint-disable-next-line @typescript-eslint/no-unused-vars */ + addLocation: jest.fn(), + getEntities: jest.fn(), + getLocationByEntity: jest.fn(), + getLocationById: jest.fn(), + removeEntityByUid: jest.fn(), + getEntityByName: jest.fn(), + }; + + const tokenIssuer = { + issueToken: async () => { + return ''; + }, + listPublicKeys: jest.fn(), + }; + + const mockResponseSend = jest.fn(); + const mockRequest = ({ + header: jest.fn(() => { + return 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCIsImtpZCI6ImZvbyIsImlzc3VlciI6ImZvbyJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.zUkMYAuMwC1T0tyHMpxXrkbFDa4aGhB8d9um_tI2hsI'; + }), + } as unknown) as express.Request; + const mockRequestWithoutJwt = ({ + header: jest.fn(() => { + return undefined; + }), + } as unknown) as express.Request; + const mockResponse = ({ + header: () => jest.fn(), + send: mockResponseSend, + } as unknown) as express.Response; + + describe('should transform to type OAuthResponse', () => { + it('when JWT is valid and identity is resolved successfully', async () => { + const provider = new AwsAlbAuthProvider( + getVoidLogger(), + catalogApi, + tokenIssuer, + { + region: 'us-west-2', + identityResolutionCallback: identityResolutionCallbackMock, + issuer: 'foo', + }, + ); + + jwtVerify.default.mockImplementationOnce(async () => { + return { + payload: { + sub: 'foo', + }, + }; + }); + + await provider.refresh(mockRequest, mockResponse); + + expect(mockResponseSend.mock.calls[0][0]).toEqual({ + backstageIdentity: { + id: 'foo', + idToken: '', + }, + profile: { + displayName: 'Foo Bar', + }, + providerInfo: {}, + }); + }); + }); + describe('should fail when', () => { + it('JWT is missing', async () => { + const provider = new AwsAlbAuthProvider( + getVoidLogger(), + catalogApi, + tokenIssuer, + { + region: 'us-west-2', + identityResolutionCallback: identityResolutionCallbackMock, + issuer: 'foo', + }, + ); + + await provider.refresh(mockRequestWithoutJwt, mockResponse); + + expect(mockResponseSend.mock.calls[0][0]).toEqual(401); + }); + + it('JWT is invalid', async () => { + const provider = new AwsAlbAuthProvider( + getVoidLogger(), + catalogApi, + tokenIssuer, + { + region: 'us-west-2', + identityResolutionCallback: identityResolutionCallbackMock, + issuer: 'foo', + }, + ); + + jwtVerify.default.mockImplementationOnce(async () => { + throw new Error('bad JWT'); + }); + + await provider.refresh(mockRequest, mockResponse); + + expect(mockResponseSend.mock.calls[0][0]).toEqual(401); + }); + + it('issuer is invalid', async () => { + const provider = new AwsAlbAuthProvider( + getVoidLogger(), + catalogApi, + tokenIssuer, + { + region: 'us-west-2', + identityResolutionCallback: identityResolutionCallbackMock, + issuer: 'foobar', + }, + ); + + jwtVerify.default.mockImplementationOnce(async () => { + return {}; + }); + + await provider.refresh(mockRequest, mockResponse); + + expect(mockResponseSend.mock.calls[0][0]).toEqual(401); + }); + + it('identity resolution callback rejects', async () => { + const provider = new AwsAlbAuthProvider( + getVoidLogger(), + catalogApi, + tokenIssuer, + { + region: 'us-west-2', + identityResolutionCallback: identityResolutionCallbackRejectedMock, + issuer: 'foo', + }, + ); + + jwtVerify.default.mockImplementationOnce(async () => { + return {}; + }); + + await provider.refresh(mockRequest, mockResponse); + + expect(mockResponseSend.mock.calls[0][0]).toEqual(401); + }); + }); +}); diff --git a/plugins/auth-backend/src/providers/aws-alb/provider.ts b/plugins/auth-backend/src/providers/aws-alb/provider.ts new file mode 100644 index 0000000000..f00543302b --- /dev/null +++ b/plugins/auth-backend/src/providers/aws-alb/provider.ts @@ -0,0 +1,138 @@ +/* + * 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 { + AuthProviderFactoryOptions, + AuthProviderRouteHandlers, +} from '../types'; +import { TokenIssuer } from '../../identity'; +import express from 'express'; +import r2 from 'r2'; +import * as crypto from 'crypto'; +import { KeyObject } from 'crypto'; +import { Logger } from 'winston'; +import jwtVerify from 'jose/jwt/verify'; +import { CatalogApi } from '@backstage/catalog-client'; +import { UserEntityV1alpha1 } from '@backstage/catalog-model'; + +const ALB_JWT_HEADER = 'x-amzn-oidc-data'; +/** + * A callback function that receives a verified JWT and returns a UserEntity + * @param {payload} The verified JWT payload + */ +type IdentityResolutionCallback = ( + payload: object, + catalogApi: CatalogApi, +) => Promise; +type AwsAlbAuthProviderOptions = { + region: string; + issuer: string; + identityResolutionCallback: IdentityResolutionCallback; +}; +export const getJWTHeaders = (input: string) => { + const encoded = input.split('.')[0]; + return JSON.parse(Buffer.from(encoded, 'base64').toString('utf8')); +}; + +export class AwsAlbAuthProvider implements AuthProviderRouteHandlers { + private logger: Logger; + private readonly catalogClient: CatalogApi; + private tokenIssuer: TokenIssuer; + private options: AwsAlbAuthProviderOptions; + private readonly keyCache: { [key: string]: KeyObject }; + + constructor( + logger: Logger, + catalogClient: CatalogApi, + tokenIssuer: TokenIssuer, + options: AwsAlbAuthProviderOptions, + ) { + this.logger = logger; + this.catalogClient = catalogClient; + this.tokenIssuer = tokenIssuer; + this.options = options; + this.keyCache = {}; + } + frameHandler(): Promise { + return Promise.resolve(undefined); + } + + async refresh(req: express.Request, res: express.Response): Promise { + const jwt = req.header(ALB_JWT_HEADER); + if (jwt !== undefined) { + try { + const headers = getJWTHeaders(jwt); + const key = await this.getKey(headers.kid); + const verifiedToken = await jwtVerify(jwt, key, {}); + + if ( + this.options.issuer !== '' && + headers.issuer !== this.options.issuer + ) { + throw new Error('issuer mismatch on JWT'); + } + + const resolvedEntity = await this.options.identityResolutionCallback( + verifiedToken.payload, + this.catalogClient, + ); + res.send({ + providerInfo: {}, + profile: resolvedEntity?.spec?.profile, + backstageIdentity: { + id: resolvedEntity?.metadata?.name, + idToken: await this.tokenIssuer.issueToken({ + claims: { sub: resolvedEntity?.metadata?.name }, + }), + }, + }); + } catch (e) { + this.logger.error('exception occurred during JWT processing', e); + res.send(401); + } + } else { + res.send(401); + } + } + + start(): Promise { + return Promise.resolve(undefined); + } + + async getKey(keyId: string): Promise { + if (this.keyCache[keyId]) { + return this.keyCache[keyId]; + } + const keyText: string = await r2( + `https://public-keys.auth.elb.${this.options.region}.amazonaws.com/${keyId}`, + ).text; + const keyValue = crypto.createPublicKey(keyText); + this.keyCache[keyId] = keyValue; + return keyValue; + } +} + +export const createAwsAlbProvider = ( + { logger, catalogApi, tokenIssuer, config }: AuthProviderFactoryOptions, + identityResolver: IdentityResolutionCallback, +) => { + const region = config.getString('region'); + const issuer = config.getString('iss'); + return new AwsAlbAuthProvider(logger, catalogApi, tokenIssuer, { + region, + issuer, + identityResolutionCallback: identityResolver, + }); +}; diff --git a/yarn.lock b/yarn.lock index 6710c7e8e7..0098954754 100644 --- a/yarn.lock +++ b/yarn.lock @@ -9781,7 +9781,7 @@ case-sensitive-paths-webpack-plugin@^2.2.0: resolved "https://registry.npmjs.org/case-sensitive-paths-webpack-plugin/-/case-sensitive-paths-webpack-plugin-2.3.0.tgz#23ac613cc9a856e4f88ff8bb73bbb5e989825cf7" integrity sha512-/4YgnZS8y1UXXmC02xD5rRrBEu6T5ub+mQHLNRj0fzTRbgdBYhsNo2V5EqwgqrExjxsjtF/OpAKAMkKsxbD5XQ== -caseless@~0.12.0: +caseless@^0.12.0, caseless@~0.12.0: version "0.12.0" resolved "https://registry.npmjs.org/caseless/-/caseless-0.12.0.tgz#1b681c21ff84033c826543090689420d187151dc" integrity sha1-G2gcIf+EAzyCZUMJBolCDRhxUdw= @@ -16549,13 +16549,6 @@ jest@^26.0.1: import-local "^3.0.2" jest-cli "^26.6.3" -jose@^1.27.1: - version "1.27.1" - resolved "https://registry.npmjs.org/jose/-/jose-1.27.1.tgz#a1de2ecb5b3ae1ae28f0d9d0cc536349ada27ec8" - integrity sha512-VyHM6IJPw0TTGqHVNlPWg16/ASDPAmcChcLqSb3WNBvwWFoWPeFqlmAUCm8/oIG1GjZwAlUDuRKFfycowarcVA== - dependencies: - "@panva/asn1.js" "^1.0.0" - jose@^2.0.2: version "2.0.2" resolved "https://registry.npmjs.org/jose/-/jose-2.0.2.tgz#fb22385b80c658cc7a0cae05b7086c04c6be49f4" @@ -16563,6 +16556,11 @@ jose@^2.0.2: dependencies: "@panva/asn1.js" "^1.0.0" +jose@^3.5.1: + version "3.5.1" + resolved "https://registry.npmjs.org/jose/-/jose-3.5.1.tgz#adc0d5000dbf64a7f4db171d449dbcf6b556b6f0" + integrity sha512-BQQJafCDvsmtc/LaK57cjfhU/1AKBhJSbJhKPq+uhrjMoV/sh3/dbk9cm08nAeSGS6j+sjhWoY+LZPUXiKzYzw== + joycon@^2.2.5: version "2.2.5" resolved "https://registry.npmjs.org/joycon/-/joycon-2.2.5.tgz#8d4cf4cbb2544d7b7583c216fcdfec19f6be1615" @@ -18824,7 +18822,7 @@ node-fetch-npm@^2.0.2: json-parse-better-errors "^1.0.0" safe-buffer "^5.1.1" -node-fetch@2.6.1, node-fetch@^2.1.2, node-fetch@^2.2.0, node-fetch@^2.3.0, node-fetch@^2.5.0, node-fetch@^2.6.0, node-fetch@^2.6.1: +node-fetch@2.6.1, node-fetch@^2.0.0-alpha.8, node-fetch@^2.1.2, node-fetch@^2.2.0, node-fetch@^2.3.0, node-fetch@^2.5.0, node-fetch@^2.6.0, node-fetch@^2.6.1: version "2.6.1" resolved "https://registry.npmjs.org/node-fetch/-/node-fetch-2.6.1.tgz#045bd323631f76ed2e2b55573394416b639a0052" integrity sha512-V4aYg89jEoVRxRb2fJdAg8FHvI7cEyYdVAh94HH0UIK8oJxUfkjlDQN9RbMx+bEjP7+ggMiFRprSti032Oipxw== @@ -21206,6 +21204,15 @@ quick-lru@^5.1.1: resolved "https://registry.npmjs.org/quick-lru/-/quick-lru-5.1.1.tgz#366493e6b3e42a3a6885e2e99d18f80fb7a8c932" integrity sha512-WuyALRjWPDGtt/wzJiadO5AXY+8hZ80hVpe6MyivgraREW751X3SbhRvG3eLKOYN+8VEvqLcf3wdnt44Z4S4SA== +r2@^2.0.1: + version "2.0.1" + resolved "https://registry.npmjs.org/r2/-/r2-2.0.1.tgz#94cd802ecfce9a622549c8182032d8e4a2b2e612" + integrity sha512-EEmxoxYCe3LHzAUhRIRxdCKERpeRNmlLj6KLUSORqnK6dWl/K5ShmDGZqM2lRZQeqJgF+wyqk0s1M7SWUveNOQ== + dependencies: + caseless "^0.12.0" + node-fetch "^2.0.0-alpha.8" + typedarray-to-buffer "^3.1.2" + raf-schd@^4.0.2: version "4.0.2" resolved "https://registry.npmjs.org/raf-schd/-/raf-schd-4.0.2.tgz#bd44c708188f2e84c810bf55fcea9231bcaed8a0" @@ -25002,7 +25009,7 @@ typed-rest-client@^1.8.0: tunnel "0.0.6" underscore "1.8.3" -typedarray-to-buffer@^3.1.5: +typedarray-to-buffer@^3.1.2, typedarray-to-buffer@^3.1.5: version "3.1.5" resolved "https://registry.npmjs.org/typedarray-to-buffer/-/typedarray-to-buffer-3.1.5.tgz#a97ee7a9ff42691b9f783ff1bc5112fe3fca9080" integrity sha512-zdu8XMNEDepKKR+XYOXAVPtWui0ly0NtohUscw+UmaHiAWT8hrV1rr//H6V+0DvJ3OQ19S979M0laLfX8rm82Q== From 6ef5428e73bd9b52818f2357422d433be697221f Mon Sep 17 00:00:00 2001 From: Jonah Back Date: Tue, 12 Jan 2021 14:53:15 -0800 Subject: [PATCH 23/75] fix: add new processor to index --- plugins/auth-backend/src/providers/aws-alb/index.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/auth-backend/src/providers/aws-alb/index.ts b/plugins/auth-backend/src/providers/aws-alb/index.ts index 863d6e76e1..f8b5c9e5d7 100644 --- a/plugins/auth-backend/src/providers/aws-alb/index.ts +++ b/plugins/auth-backend/src/providers/aws-alb/index.ts @@ -13,3 +13,4 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +export { createAwsAlbProvider } from './provider'; From 0d6ad133c75d0274695f59fd696718dbaa72f3b9 Mon Sep 17 00:00:00 2001 From: Jonah Back Date: Tue, 12 Jan 2021 15:17:28 -0800 Subject: [PATCH 24/75] fix: simplify identity resolver --- .../src/providers/aws-alb/provider.test.ts | 115 ++++++------------ .../src/providers/aws-alb/provider.ts | 49 +++----- .../auth-backend/src/providers/factories.ts | 2 + plugins/auth-backend/src/providers/types.ts | 6 + 4 files changed, 68 insertions(+), 104 deletions(-) diff --git a/plugins/auth-backend/src/providers/aws-alb/provider.test.ts b/plugins/auth-backend/src/providers/aws-alb/provider.test.ts index 55c27fd7ff..93649ce8d7 100644 --- a/plugins/auth-backend/src/providers/aws-alb/provider.test.ts +++ b/plugins/auth-backend/src/providers/aws-alb/provider.test.ts @@ -18,7 +18,9 @@ import express from 'express'; import * as jwtVerify from 'jose/jwt/verify'; import { AwsAlbAuthProvider } from './provider'; -import { UserEntityV1alpha1 } from '@backstage/catalog-model'; +import { AuthResponse } from '../types'; + +const mockedJwtVerify = jwtVerify as jest.Mocked; const mockKey = async () => { return `-----BEGIN PUBLIC KEY----- @@ -44,26 +46,21 @@ jest.mock('jose/jwt/verify', () => { }; }); -const identityResolutionCallbackMock = async (): Promise< - UserEntityV1alpha1 -> => { +const identityResolutionCallbackMock = async (): Promise> => { return { - apiVersion: 'backstage.io/v1alpha1', - kind: 'User', - metadata: { - name: 'foo', + backstageIdentity: { + id: 'foo', + idToken: '', }, - spec: { - memberOf: [], - profile: { - displayName: 'Foo Bar', - }, + profile: { + displayName: 'Foo Bar', }, + providerInfo: {}, }; }; const identityResolutionCallbackRejectedMock = async (): Promise< - UserEntityV1alpha1 + AuthResponse > => { throw new Error('failed'); }; @@ -83,13 +80,6 @@ describe('AwsALBAuthProvider', () => { getEntityByName: jest.fn(), }; - const tokenIssuer = { - issueToken: async () => { - return ''; - }, - listPublicKeys: jest.fn(), - }; - const mockResponseSend = jest.fn(); const mockRequest = ({ header: jest.fn(() => { @@ -108,18 +98,13 @@ describe('AwsALBAuthProvider', () => { describe('should transform to type OAuthResponse', () => { it('when JWT is valid and identity is resolved successfully', async () => { - const provider = new AwsAlbAuthProvider( - getVoidLogger(), - catalogApi, - tokenIssuer, - { - region: 'us-west-2', - identityResolutionCallback: identityResolutionCallbackMock, - issuer: 'foo', - }, - ); + const provider = new AwsAlbAuthProvider(getVoidLogger(), catalogApi, { + region: 'us-west-2', + identityResolutionCallback: identityResolutionCallbackMock, + issuer: 'foo', + }); - jwtVerify.default.mockImplementationOnce(async () => { + mockedJwtVerify.default.mockImplementationOnce(async () => { return { payload: { sub: 'foo', @@ -143,16 +128,11 @@ describe('AwsALBAuthProvider', () => { }); describe('should fail when', () => { it('JWT is missing', async () => { - const provider = new AwsAlbAuthProvider( - getVoidLogger(), - catalogApi, - tokenIssuer, - { - region: 'us-west-2', - identityResolutionCallback: identityResolutionCallbackMock, - issuer: 'foo', - }, - ); + const provider = new AwsAlbAuthProvider(getVoidLogger(), catalogApi, { + region: 'us-west-2', + identityResolutionCallback: identityResolutionCallbackMock, + issuer: 'foo', + }); await provider.refresh(mockRequestWithoutJwt, mockResponse); @@ -160,18 +140,13 @@ describe('AwsALBAuthProvider', () => { }); it('JWT is invalid', async () => { - const provider = new AwsAlbAuthProvider( - getVoidLogger(), - catalogApi, - tokenIssuer, - { - region: 'us-west-2', - identityResolutionCallback: identityResolutionCallbackMock, - issuer: 'foo', - }, - ); + const provider = new AwsAlbAuthProvider(getVoidLogger(), catalogApi, { + region: 'us-west-2', + identityResolutionCallback: identityResolutionCallbackMock, + issuer: 'foo', + }); - jwtVerify.default.mockImplementationOnce(async () => { + mockedJwtVerify.default.mockImplementationOnce(async () => { throw new Error('bad JWT'); }); @@ -181,18 +156,13 @@ describe('AwsALBAuthProvider', () => { }); it('issuer is invalid', async () => { - const provider = new AwsAlbAuthProvider( - getVoidLogger(), - catalogApi, - tokenIssuer, - { - region: 'us-west-2', - identityResolutionCallback: identityResolutionCallbackMock, - issuer: 'foobar', - }, - ); + const provider = new AwsAlbAuthProvider(getVoidLogger(), catalogApi, { + region: 'us-west-2', + identityResolutionCallback: identityResolutionCallbackMock, + issuer: 'foobar', + }); - jwtVerify.default.mockImplementationOnce(async () => { + mockedJwtVerify.default.mockImplementationOnce(async () => { return {}; }); @@ -202,18 +172,13 @@ describe('AwsALBAuthProvider', () => { }); it('identity resolution callback rejects', async () => { - const provider = new AwsAlbAuthProvider( - getVoidLogger(), - catalogApi, - tokenIssuer, - { - region: 'us-west-2', - identityResolutionCallback: identityResolutionCallbackRejectedMock, - issuer: 'foo', - }, - ); + const provider = new AwsAlbAuthProvider(getVoidLogger(), catalogApi, { + region: 'us-west-2', + identityResolutionCallback: identityResolutionCallbackRejectedMock, + issuer: 'foo', + }); - jwtVerify.default.mockImplementationOnce(async () => { + mockedJwtVerify.default.mockImplementationOnce(async () => { return {}; }); diff --git a/plugins/auth-backend/src/providers/aws-alb/provider.ts b/plugins/auth-backend/src/providers/aws-alb/provider.ts index f00543302b..568fc4dd46 100644 --- a/plugins/auth-backend/src/providers/aws-alb/provider.ts +++ b/plugins/auth-backend/src/providers/aws-alb/provider.ts @@ -16,30 +16,26 @@ import { AuthProviderFactoryOptions, AuthProviderRouteHandlers, + IdentityResolver, } from '../types'; -import { TokenIssuer } from '../../identity'; import express from 'express'; +// @ts-ignore no types available for R2 import r2 from 'r2'; import * as crypto from 'crypto'; import { KeyObject } from 'crypto'; import { Logger } from 'winston'; import jwtVerify from 'jose/jwt/verify'; import { CatalogApi } from '@backstage/catalog-client'; -import { UserEntityV1alpha1 } from '@backstage/catalog-model'; const ALB_JWT_HEADER = 'x-amzn-oidc-data'; /** * A callback function that receives a verified JWT and returns a UserEntity * @param {payload} The verified JWT payload */ -type IdentityResolutionCallback = ( - payload: object, - catalogApi: CatalogApi, -) => Promise; type AwsAlbAuthProviderOptions = { region: string; issuer: string; - identityResolutionCallback: IdentityResolutionCallback; + identityResolutionCallback: IdentityResolver; }; export const getJWTHeaders = (input: string) => { const encoded = input.split('.')[0]; @@ -49,19 +45,16 @@ export const getJWTHeaders = (input: string) => { export class AwsAlbAuthProvider implements AuthProviderRouteHandlers { private logger: Logger; private readonly catalogClient: CatalogApi; - private tokenIssuer: TokenIssuer; private options: AwsAlbAuthProviderOptions; private readonly keyCache: { [key: string]: KeyObject }; constructor( logger: Logger, catalogClient: CatalogApi, - tokenIssuer: TokenIssuer, options: AwsAlbAuthProviderOptions, ) { this.logger = logger; this.catalogClient = catalogClient; - this.tokenIssuer = tokenIssuer; this.options = options; this.keyCache = {}; } @@ -88,16 +81,7 @@ export class AwsAlbAuthProvider implements AuthProviderRouteHandlers { verifiedToken.payload, this.catalogClient, ); - res.send({ - providerInfo: {}, - profile: resolvedEntity?.spec?.profile, - backstageIdentity: { - id: resolvedEntity?.metadata?.name, - idToken: await this.tokenIssuer.issueToken({ - claims: { sub: resolvedEntity?.metadata?.name }, - }), - }, - }); + res.send(resolvedEntity); } catch (e) { this.logger.error('exception occurred during JWT processing', e); res.send(401); @@ -124,15 +108,22 @@ export class AwsAlbAuthProvider implements AuthProviderRouteHandlers { } } -export const createAwsAlbProvider = ( - { logger, catalogApi, tokenIssuer, config }: AuthProviderFactoryOptions, - identityResolver: IdentityResolutionCallback, -) => { +export const createAwsAlbProvider = ({ + logger, + catalogApi, + config, + identityResolver, +}: AuthProviderFactoryOptions) => { const region = config.getString('region'); const issuer = config.getString('iss'); - return new AwsAlbAuthProvider(logger, catalogApi, tokenIssuer, { - region, - issuer, - identityResolutionCallback: identityResolver, - }); + if (identityResolver !== undefined) { + return new AwsAlbAuthProvider(logger, catalogApi, { + region, + issuer, + identityResolutionCallback: identityResolver, + }); + } + throw new Error( + 'Identity resolver is required to use this authentication provider', + ); }; diff --git a/plugins/auth-backend/src/providers/factories.ts b/plugins/auth-backend/src/providers/factories.ts index 7bad4f4d81..619fb1c706 100644 --- a/plugins/auth-backend/src/providers/factories.ts +++ b/plugins/auth-backend/src/providers/factories.ts @@ -25,6 +25,7 @@ import { createAuth0Provider } from './auth0'; import { createMicrosoftProvider } from './microsoft'; import { createOneLoginProvider } from './onelogin'; import { AuthProviderFactory } from './types'; +import { createAwsAlbProvider } from './aws-alb'; export const factories: { [providerId: string]: AuthProviderFactory } = { google: createGoogleProvider, @@ -37,4 +38,5 @@ export const factories: { [providerId: string]: AuthProviderFactory } = { oauth2: createOAuth2Provider, oidc: createOidcProvider, onelogin: createOneLoginProvider, + awsalb: createAwsAlbProvider, }; diff --git a/plugins/auth-backend/src/providers/types.ts b/plugins/auth-backend/src/providers/types.ts index a40f2a96ee..51c1ee336e 100644 --- a/plugins/auth-backend/src/providers/types.ts +++ b/plugins/auth-backend/src/providers/types.ts @@ -112,6 +112,11 @@ export interface AuthProviderRouteHandlers { logout?(req: express.Request, res: express.Response): Promise; } +export type IdentityResolver = ( + payload: object, + catalogApi: CatalogApi, +) => Promise>; + export type AuthProviderFactoryOptions = { providerId: string; globalConfig: AuthProviderConfig; @@ -120,6 +125,7 @@ export type AuthProviderFactoryOptions = { tokenIssuer: TokenIssuer; discovery: PluginEndpointDiscovery; catalogApi: CatalogApi; + identityResolver?: IdentityResolver; }; export type AuthProviderFactory = ( From 0643a3336ab141cd2a87ea31d8bde6f01c32e276 Mon Sep 17 00:00:00 2001 From: Jonah Back Date: Tue, 12 Jan 2021 15:19:02 -0800 Subject: [PATCH 25/75] chore: add changeset --- .changeset/twelve-ants-sort.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/twelve-ants-sort.md diff --git a/.changeset/twelve-ants-sort.md b/.changeset/twelve-ants-sort.md new file mode 100644 index 0000000000..978047ab8e --- /dev/null +++ b/.changeset/twelve-ants-sort.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-auth-backend': patch +--- + +Add AWS ALB OIDC reverse proxy authentication provider From 2335fa9d174995c800920b76cdfcb117ef308c56 Mon Sep 17 00:00:00 2001 From: Jonah Back Date: Tue, 12 Jan 2021 15:21:44 -0800 Subject: [PATCH 26/75] chore: add comment around payload passed in identity resolver --- plugins/auth-backend/src/providers/types.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/plugins/auth-backend/src/providers/types.ts b/plugins/auth-backend/src/providers/types.ts index 51c1ee336e..a31ef85a7d 100644 --- a/plugins/auth-backend/src/providers/types.ts +++ b/plugins/auth-backend/src/providers/types.ts @@ -113,6 +113,9 @@ export interface AuthProviderRouteHandlers { } export type IdentityResolver = ( + /** + * An object containing information specific to the auth provider. + */ payload: object, catalogApi: CatalogApi, ) => Promise>; From 1f275fe31a4585bf6beb72e5cd4d3a9ef346cbe3 Mon Sep 17 00:00:00 2001 From: Jonah Back Date: Wed, 13 Jan 2021 20:18:07 -0800 Subject: [PATCH 27/75] Upgrade TokenFactory to latest jose library --- plugins/auth-backend/package.json | 8 +++- .../src/identity/TokenFactory.test.ts | 41 +++++++++++++------ .../auth-backend/src/identity/TokenFactory.ts | 40 ++++++++++-------- yarn.lock | 6 +-- 4 files changed, 61 insertions(+), 34 deletions(-) diff --git a/plugins/auth-backend/package.json b/plugins/auth-backend/package.json index f8de2bc6f1..f189f8078f 100644 --- a/plugins/auth-backend/package.json +++ b/plugins/auth-backend/package.json @@ -18,7 +18,13 @@ }, "jest": { "moduleNameMapper": { - "jose/jwt/verify": "/../../../node_modules/jose/lib/jwt/verify" + "jose/jwt/sign": "/../node_modules/jose/dist/node/cjs/jwt/sign", + "jose/jwt/verify": "/../node_modules/jose/dist/node/cjs/jwt/verify", + "jose/jwk/from_key_like": "/../node_modules/jose/dist/node/cjs/jwk/from_key_like", + "jose/jwk/parse": "/../node_modules/jose/dist/node/cjs/jwk/parse", + "jose/util/base64url": "/../node_modules/jose/dist/node/cjs/util/base64url", + "jose/util/decode_protected_header": "/../node_modules/jose/dist/node/cjs/util/decode_protected_header", + "jose/util/generate_secret": "/../node_modules/jose/dist/node/cjs/util/generate_secret" } }, "keywords": [ diff --git a/plugins/auth-backend/src/identity/TokenFactory.test.ts b/plugins/auth-backend/src/identity/TokenFactory.test.ts index 8b719799b1..7058dbb0f5 100644 --- a/plugins/auth-backend/src/identity/TokenFactory.test.ts +++ b/plugins/auth-backend/src/identity/TokenFactory.test.ts @@ -13,12 +13,23 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +import { TextDecoder, TextEncoder } from 'util'; + +// These two statements are structured like this because Jest doesn't include these in the default +// test environment, even though they exist in Node. +global.TextEncoder = TextEncoder; +// @ts-ignore +global.TextDecoder = TextDecoder; import { utc } from 'moment'; import { TokenFactory } from './TokenFactory'; import { getVoidLogger } from '@backstage/backend-common'; -import { KeyStore, AnyJWK, StoredKey } from './types'; -import { JWKS, JSONWebKey, JWT } from 'jose'; +import { AnyJWK, KeyStore, StoredKey } from './types'; +import jwtVerify from 'jose/jwt/verify'; +import parseJwk from 'jose/jwk/parse'; +import decodeProtectedHeader, { + ProtectedHeaderParameters, +} from 'jose/util/decode_protected_header'; const logger = getVoidLogger(); @@ -52,10 +63,8 @@ class MemoryKeyStore implements KeyStore { } function jwtKid(jwt: string): string { - const { header } = JWT.decode(jwt, { complete: true }) as { - header: { kid: string }; - }; - return header.kid; + const header = decodeProtectedHeader(jwt) as ProtectedHeaderParameters; + return header.kid as string; } describe('TokenFactory', () => { @@ -72,14 +81,20 @@ describe('TokenFactory', () => { const token = await factory.issueToken({ claims: { sub: 'foo' } }); const { keys } = await factory.listPublicKeys(); - const keyStore = JWKS.asKeyStore({ - keys: keys.map(key => key as JSONWebKey), + const keyMap: { + [key: string]: AnyJWK; + } = {}; + + keys.forEach(key => { + keyMap[key.kid] = key; }); - const payload = JWT.verify(token, keyStore) as object & { - iat: number; - exp: number; - }; + const payload = ( + await jwtVerify(token, async header => { + const kid = header.kid as string; + return await parseJwk(keyMap[kid]); + }) + ).payload; expect(payload).toEqual({ iss: 'my-issuer', aud: 'backstage', @@ -87,7 +102,7 @@ describe('TokenFactory', () => { iat: expect.any(Number), exp: expect.any(Number), }); - expect(payload.exp).toBe(payload.iat + keyDurationSeconds); + expect(payload.exp).toBe((payload.iat as number) + keyDurationSeconds); }); it('should generate new signing keys when the current one expires', async () => { diff --git a/plugins/auth-backend/src/identity/TokenFactory.ts b/plugins/auth-backend/src/identity/TokenFactory.ts index 36622332e0..8cf7c358d6 100644 --- a/plugins/auth-backend/src/identity/TokenFactory.ts +++ b/plugins/auth-backend/src/identity/TokenFactory.ts @@ -16,7 +16,10 @@ import moment from 'moment'; import { TokenIssuer, TokenParams, KeyStore, AnyJWK } from './types'; -import { JSONWebKey, JWK, JWS } from 'jose'; +import parseJwk, { JWK } from 'jose/jwk/parse'; +import SignJWT from 'jose/jwt/sign'; +import generateSecret from 'jose/util/generate_secret'; +import fromKeyLike from 'jose/jwk/from_key_like'; import { Logger } from 'winston'; import { v4 as uuid } from 'uuid'; @@ -53,7 +56,7 @@ export class TokenFactory implements TokenIssuer { private readonly keyDurationSeconds: number; private keyExpiry?: moment.Moment; - private privateKeyPromise?: Promise; + private privateKeyPromise?: Promise; constructor(options: Options) { this.issuer = options.issuer; @@ -64,7 +67,7 @@ export class TokenFactory implements TokenIssuer { async issueToken(params: TokenParams): Promise { const key = await this.getKey(); - + const keyLike = await parseJwk(key); const iss = this.issuer; const sub = params.claims.sub; const aud = 'backstage'; @@ -72,11 +75,9 @@ export class TokenFactory implements TokenIssuer { const exp = iat + this.keyDurationSeconds; this.logger.info(`Issuing token for ${sub}`); - - return JWS.sign({ iss, sub, aud, iat, exp }, key, { - alg: key.alg, - kid: key.kid, - }); + return new SignJWT({ iss, sub, aud, iat, exp }) + .setProtectedHeader({ alg: key.alg, typ: 'JWT', kid: key.kid }) + .sign(keyLike); } // This will be called by other services that want to verify ID tokens. @@ -114,7 +115,7 @@ export class TokenFactory implements TokenIssuer { return { keys: validKeys.map(({ key }) => key) }; } - private async getKey(): Promise { + private async getKey(): Promise { // Make sure that we only generate one key at a time if (this.privateKeyPromise) { if (this.keyExpiry?.isAfter()) { @@ -127,11 +128,12 @@ export class TokenFactory implements TokenIssuer { this.keyExpiry = moment().add(this.keyDurationSeconds, 'seconds'); const promise = (async () => { // This generates a new signing key to be used to sign tokens until the next key rotation - const key = await JWK.generate('EC', 'P-256', { - use: 'sig', - kid: uuid(), - alg: 'ES256', - }); + + const key = await generateSecret('HS256'); + const kid = uuid(); + const jwk = await fromKeyLike(key); + jwk.kid = kid; + jwk.alg = 'HS256'; // We're not allowed to use the key until it has been successfully stored // TODO: some token verification implementations aggressively cache the list of keys, and @@ -139,11 +141,15 @@ export class TokenFactory implements TokenIssuer { // may want to keep using the existing key for some period of time until we switch to // the new one. This also needs to be implemented cross-service though, meaning new services // that boot up need to be able to grab an existing key to use for signing. - this.logger.info(`Created new signing key ${key.kid}`); - await this.keyStore.addKey((key.toJWK(false) as unknown) as AnyJWK); + this.logger.info(`Created new signing key ${jwk.kid}`); + await this.keyStore.addKey({ + // @ts-ignore + use: 'sig', + ...jwk, + }); // At this point we are allowed to start using the new key - return key as JSONWebKey; + return jwk; })(); this.privateKeyPromise = promise; diff --git a/yarn.lock b/yarn.lock index 0098954754..8ce5543164 100644 --- a/yarn.lock +++ b/yarn.lock @@ -16550,9 +16550,9 @@ jest@^26.0.1: jest-cli "^26.6.3" jose@^2.0.2: - version "2.0.2" - resolved "https://registry.npmjs.org/jose/-/jose-2.0.2.tgz#fb22385b80c658cc7a0cae05b7086c04c6be49f4" - integrity sha512-yD93lsiMA1go/qxSY/vXWBodmIZJIxeB7QhFi8z1yQ3KUwKENqI9UA8VCHlQ5h3x1zWuWZjoY87ByQzkQbIrQg== + version "2.0.3" + resolved "https://registry.npmjs.org/jose/-/jose-2.0.3.tgz#9c931ab3e13e2d16a5b9e6183e60b2fc40a8e1b8" + integrity sha512-L+RlDgjO0Tk+Ki6/5IXCSEnmJCV8iMFZoBuEgu2vPQJJ4zfG/k3CAqZUMKDYNRHIDyy0QidJpOvX0NgpsAqFlw== dependencies: "@panva/asn1.js" "^1.0.0" From dfce32b3567e02d93241fdca623e0ec8193db3fb Mon Sep 17 00:00:00 2001 From: Jonah Back Date: Mon, 18 Jan 2021 14:09:23 -0800 Subject: [PATCH 28/75] Address review comments around algorithm being used, typescript ignore --- plugins/auth-backend/package.json | 8 +----- .../auth-backend/src/identity/TokenFactory.ts | 27 ++++++++++--------- 2 files changed, 16 insertions(+), 19 deletions(-) diff --git a/plugins/auth-backend/package.json b/plugins/auth-backend/package.json index f189f8078f..380b7da6b0 100644 --- a/plugins/auth-backend/package.json +++ b/plugins/auth-backend/package.json @@ -18,13 +18,7 @@ }, "jest": { "moduleNameMapper": { - "jose/jwt/sign": "/../node_modules/jose/dist/node/cjs/jwt/sign", - "jose/jwt/verify": "/../node_modules/jose/dist/node/cjs/jwt/verify", - "jose/jwk/from_key_like": "/../node_modules/jose/dist/node/cjs/jwk/from_key_like", - "jose/jwk/parse": "/../node_modules/jose/dist/node/cjs/jwk/parse", - "jose/util/base64url": "/../node_modules/jose/dist/node/cjs/util/base64url", - "jose/util/decode_protected_header": "/../node_modules/jose/dist/node/cjs/util/decode_protected_header", - "jose/util/generate_secret": "/../node_modules/jose/dist/node/cjs/util/generate_secret" + "^jose/(.*)$": "/../node_modules/jose/dist/node/cjs/$1" } }, "keywords": [ diff --git a/plugins/auth-backend/src/identity/TokenFactory.ts b/plugins/auth-backend/src/identity/TokenFactory.ts index 8cf7c358d6..65f9a07eb1 100644 --- a/plugins/auth-backend/src/identity/TokenFactory.ts +++ b/plugins/auth-backend/src/identity/TokenFactory.ts @@ -18,7 +18,7 @@ import moment from 'moment'; import { TokenIssuer, TokenParams, KeyStore, AnyJWK } from './types'; import parseJwk, { JWK } from 'jose/jwk/parse'; import SignJWT from 'jose/jwt/sign'; -import generateSecret from 'jose/util/generate_secret'; +import generateKeyPair from 'jose/util/generate_key_pair'; import fromKeyLike from 'jose/jwk/from_key_like'; import { Logger } from 'winston'; import { v4 as uuid } from 'uuid'; @@ -129,12 +129,20 @@ export class TokenFactory implements TokenIssuer { const promise = (async () => { // This generates a new signing key to be used to sign tokens until the next key rotation - const key = await generateSecret('HS256'); + const key = await generateKeyPair('ES256'); const kid = uuid(); - const jwk = await fromKeyLike(key); - jwk.kid = kid; - jwk.alg = 'HS256'; + const jwk = await fromKeyLike(key.privateKey); + // @ts-ignore https://github.com/microsoft/TypeScript/issues/13195 - + // JOSE Library provides optional for most fields - and TS does not distinguish between missing/undefined. + // Because AnyJWK requires keys to have type "string", this throws a TypeError - though in practice, if the field + // is undefined, JOSE will not send it back as key. + const storedJwk: AnyJWK = { + ...jwk, + alg: 'ES256', + kid: kid, + use: 'sig', + }; // We're not allowed to use the key until it has been successfully stored // TODO: some token verification implementations aggressively cache the list of keys, and // don't attempt to fetch new ones even if they encounter an unknown kid. Therefore we @@ -142,14 +150,9 @@ export class TokenFactory implements TokenIssuer { // the new one. This also needs to be implemented cross-service though, meaning new services // that boot up need to be able to grab an existing key to use for signing. this.logger.info(`Created new signing key ${jwk.kid}`); - await this.keyStore.addKey({ - // @ts-ignore - use: 'sig', - ...jwk, - }); - + await this.keyStore.addKey(storedJwk); // At this point we are allowed to start using the new key - return jwk; + return storedJwk; })(); this.privateKeyPromise = promise; From 147648a2d105a4d242861dcbe7129ab6ada3878b Mon Sep 17 00:00:00 2001 From: Jonah Back Date: Mon, 18 Jan 2021 14:12:12 -0800 Subject: [PATCH 29/75] Update type definition and docs to be clear about experimental status --- plugins/auth-backend/src/providers/types.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/plugins/auth-backend/src/providers/types.ts b/plugins/auth-backend/src/providers/types.ts index a31ef85a7d..1a4e7b118a 100644 --- a/plugins/auth-backend/src/providers/types.ts +++ b/plugins/auth-backend/src/providers/types.ts @@ -112,7 +112,12 @@ export interface AuthProviderRouteHandlers { logout?(req: express.Request, res: express.Response): Promise; } -export type IdentityResolver = ( +/** + * EXPERIMENTAL - this will almost certainly break in a future release. + * + * Used to resolve an identity from auth information in some auth providers. + */ +export type ExperimentalIdentityResolver = ( /** * An object containing information specific to the auth provider. */ @@ -128,7 +133,7 @@ export type AuthProviderFactoryOptions = { tokenIssuer: TokenIssuer; discovery: PluginEndpointDiscovery; catalogApi: CatalogApi; - identityResolver?: IdentityResolver; + identityResolver?: ExperimentalIdentityResolver; }; export type AuthProviderFactory = ( From 02140eb89ad056c52876352319991b5b95da738d Mon Sep 17 00:00:00 2001 From: Jonah Back Date: Mon, 18 Jan 2021 14:22:38 -0800 Subject: [PATCH 30/75] Add basic node-cache with 1 hour TTL for caching keys retrieved from AWS public key endpoint --- plugins/auth-backend/package.json | 1 + .../src/identity/TokenFactory.test.ts | 2 +- .../src/providers/aws-alb/provider.ts | 16 +++++++++------- yarn.lock | 12 ++++++++++++ 4 files changed, 23 insertions(+), 8 deletions(-) diff --git a/plugins/auth-backend/package.json b/plugins/auth-backend/package.json index 380b7da6b0..ad54e35b28 100644 --- a/plugins/auth-backend/package.json +++ b/plugins/auth-backend/package.json @@ -54,6 +54,7 @@ "knex": "^0.21.6", "moment": "^2.26.0", "morgan": "^1.10.0", + "node-cache": "^5.1.2", "openid-client": "^4.2.1", "passport": "^0.4.1", "passport-github2": "^0.1.12", diff --git a/plugins/auth-backend/src/identity/TokenFactory.test.ts b/plugins/auth-backend/src/identity/TokenFactory.test.ts index 7058dbb0f5..abbeecb40c 100644 --- a/plugins/auth-backend/src/identity/TokenFactory.test.ts +++ b/plugins/auth-backend/src/identity/TokenFactory.test.ts @@ -102,7 +102,7 @@ describe('TokenFactory', () => { iat: expect.any(Number), exp: expect.any(Number), }); - expect(payload.exp).toBe((payload.iat as number) + keyDurationSeconds); + expect(payload.exp).toBe(Number(payload.iat) + keyDurationSeconds); }); it('should generate new signing keys when the current one expires', async () => { diff --git a/plugins/auth-backend/src/providers/aws-alb/provider.ts b/plugins/auth-backend/src/providers/aws-alb/provider.ts index 568fc4dd46..6bb2e9fe7b 100644 --- a/plugins/auth-backend/src/providers/aws-alb/provider.ts +++ b/plugins/auth-backend/src/providers/aws-alb/provider.ts @@ -16,7 +16,7 @@ import { AuthProviderFactoryOptions, AuthProviderRouteHandlers, - IdentityResolver, + ExperimentalIdentityResolver, } from '../types'; import express from 'express'; // @ts-ignore no types available for R2 @@ -24,6 +24,7 @@ import r2 from 'r2'; import * as crypto from 'crypto'; import { KeyObject } from 'crypto'; import { Logger } from 'winston'; +import NodeCache from 'node-cache'; import jwtVerify from 'jose/jwt/verify'; import { CatalogApi } from '@backstage/catalog-client'; @@ -35,7 +36,7 @@ const ALB_JWT_HEADER = 'x-amzn-oidc-data'; type AwsAlbAuthProviderOptions = { region: string; issuer: string; - identityResolutionCallback: IdentityResolver; + identityResolutionCallback: ExperimentalIdentityResolver; }; export const getJWTHeaders = (input: string) => { const encoded = input.split('.')[0]; @@ -46,7 +47,7 @@ export class AwsAlbAuthProvider implements AuthProviderRouteHandlers { private logger: Logger; private readonly catalogClient: CatalogApi; private options: AwsAlbAuthProviderOptions; - private readonly keyCache: { [key: string]: KeyObject }; + private readonly keyCache: NodeCache; constructor( logger: Logger, @@ -56,7 +57,7 @@ export class AwsAlbAuthProvider implements AuthProviderRouteHandlers { this.logger = logger; this.catalogClient = catalogClient; this.options = options; - this.keyCache = {}; + this.keyCache = new NodeCache({ stdTTL: 3600 }); } frameHandler(): Promise { return Promise.resolve(undefined); @@ -96,14 +97,15 @@ export class AwsAlbAuthProvider implements AuthProviderRouteHandlers { } async getKey(keyId: string): Promise { - if (this.keyCache[keyId]) { - return this.keyCache[keyId]; + const optionalCacheKey = this.keyCache.get(keyId); + if (optionalCacheKey) { + return optionalCacheKey; } const keyText: string = await r2( `https://public-keys.auth.elb.${this.options.region}.amazonaws.com/${keyId}`, ).text; const keyValue = crypto.createPublicKey(keyText); - this.keyCache[keyId] = keyValue; + this.keyCache.set(keyId, keyValue); return keyValue; } } diff --git a/yarn.lock b/yarn.lock index 8ce5543164..b8531ef248 100644 --- a/yarn.lock +++ b/yarn.lock @@ -10126,6 +10126,11 @@ clone-response@^1.0.2: dependencies: mimic-response "^1.0.0" +clone@2.x: + version "2.1.2" + resolved "https://registry.npmjs.org/clone/-/clone-2.1.2.tgz#1b7f4b9f591f1e8f83670401600345a02887435f" + integrity sha1-G39Ln1kfHo+DZwQBYANFoCiHQ18= + clone@^1.0.2: version "1.0.4" resolved "https://registry.npmjs.org/clone/-/clone-1.0.4.tgz#da309cc263df15994c688ca902179ca3c7cd7c7e" @@ -18806,6 +18811,13 @@ node-addon-api@2.0.0: resolved "https://registry.npmjs.org/node-addon-api/-/node-addon-api-2.0.0.tgz#f9afb8d777a91525244b01775ea0ddbe1125483b" integrity sha512-ASCL5U13as7HhOExbT6OlWJJUV/lLzL2voOSP1UVehpRD8FbSrSDjfScK/KwAvVTI5AS6r4VwbOMlIqtvRidnA== +node-cache@^5.1.2: + version "5.1.2" + resolved "https://registry.npmjs.org/node-cache/-/node-cache-5.1.2.tgz#f264dc2ccad0a780e76253a694e9fd0ed19c398d" + integrity sha512-t1QzWwnk4sjLWaQAS8CHgOJ+RAfmHpxFWmc36IWTiWHQfs0w5JDMBS1b1ZxQteo0vVVuWJvIUKHDkkeK7vIGCg== + dependencies: + clone "2.x" + node-dir@^0.1.10: version "0.1.17" resolved "https://registry.npmjs.org/node-dir/-/node-dir-0.1.17.tgz#5f5665d93351335caabef8f1c554516cf5f1e4e5" From c43d1485560158fc54eb40acc42c8791531be27b Mon Sep 17 00:00:00 2001 From: Jonah Back Date: Mon, 18 Jan 2021 14:39:53 -0800 Subject: [PATCH 31/75] Fix usage of JOSE library in OIDC test as part of JOSE upgrade --- plugins/auth-backend/package.json | 2 +- plugins/auth-backend/src/providers/oidc/provider.test.ts | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/plugins/auth-backend/package.json b/plugins/auth-backend/package.json index ad54e35b28..a0c7cf1d84 100644 --- a/plugins/auth-backend/package.json +++ b/plugins/auth-backend/package.json @@ -18,7 +18,7 @@ }, "jest": { "moduleNameMapper": { - "^jose/(.*)$": "/../node_modules/jose/dist/node/cjs/$1" + "^jose/(.*)$": "/../../../node_modules/jose/dist/node/cjs/$1" } }, "keywords": [ diff --git a/plugins/auth-backend/src/providers/oidc/provider.test.ts b/plugins/auth-backend/src/providers/oidc/provider.test.ts index f2bd8dcb90..ae0ca2da2b 100644 --- a/plugins/auth-backend/src/providers/oidc/provider.test.ts +++ b/plugins/auth-backend/src/providers/oidc/provider.test.ts @@ -13,13 +13,17 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +import { TextDecoder, TextEncoder } from 'util'; +// @ts-ignore +global.TextDecoder = TextDecoder; +global.TextEncoder = TextEncoder; import express from 'express'; import { Session } from 'express-session'; import nock from 'nock'; import { ClientMetadata, IssuerMetadata } from 'openid-client'; import { createOidcProvider, OidcAuthProvider } from './provider'; -import { JWT, JWK } from 'jose'; +import UnsecuredJWT from 'jose/jwt/unsecured'; import { AuthProviderFactoryOptions } from '../types'; import { Config } from '@backstage/config'; import { OAuthAdapter } from '../../lib/oauth'; @@ -71,6 +75,7 @@ describe('OidcAuthProvider', () => { const jwt = { sub: 'alice', iss: 'https://oidc.test', + iat: Date.now(), aud: clientMetadata.clientId, exp: Date.now() + 10000, }; @@ -79,7 +84,7 @@ describe('OidcAuthProvider', () => { .reply(200, issuerMetadata) .post('/as/token.oauth2') .reply(200, { - id_token: JWT.sign(jwt, JWK.None), + id_token: new UnsecuredJWT(jwt).encode(), access_token: 'test', authorization_signed_response_alg: 'HS256', }) From d7726cdfebbada542ea5f8ac060e977a8595bed3 Mon Sep 17 00:00:00 2001 From: Jonah Back Date: Mon, 18 Jan 2021 17:37:00 -0800 Subject: [PATCH 32/75] Make keyMap generation more concise Co-authored-by: Patrik Oldsberg --- plugins/auth-backend/src/identity/TokenFactory.test.ts | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/plugins/auth-backend/src/identity/TokenFactory.test.ts b/plugins/auth-backend/src/identity/TokenFactory.test.ts index abbeecb40c..e28ded2bf2 100644 --- a/plugins/auth-backend/src/identity/TokenFactory.test.ts +++ b/plugins/auth-backend/src/identity/TokenFactory.test.ts @@ -81,13 +81,7 @@ describe('TokenFactory', () => { const token = await factory.issueToken({ claims: { sub: 'foo' } }); const { keys } = await factory.listPublicKeys(); - const keyMap: { - [key: string]: AnyJWK; - } = {}; - - keys.forEach(key => { - keyMap[key.kid] = key; - }); + const keyMap = Object.fromEntries(keys.map(key => [key.kid, key])); const payload = ( await jwtVerify(token, async header => { From 473cd6e1b8a4f2ce44535d6b5e394170aa52dab3 Mon Sep 17 00:00:00 2001 From: Adam Harvey Date: Mon, 18 Jan 2021 21:30:26 -0500 Subject: [PATCH 33/75] Fix architecture link --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index d2e01195d4..576ec6e563 100644 --- a/README.md +++ b/README.md @@ -39,7 +39,7 @@ Check out [the documentation](https://backstage.io/docs/getting-started) on how - [Main documentation](https://backstage.io/docs) - [Service Catalog](https://backstage.io/docs/features/software-catalog/software-catalog-overview) -- [Architecture](https://backstage.io/docs/overview/architecture-terminology) ([Decisions](https://backstage.io/docs/architecture-decisions/adrs-overview)) +- [Architecture](https://backstage.io/docs/overview/architecture-overview) ([Decisions](https://backstage.io/docs/architecture-decisions/adrs-overview)) - [Designing for Backstage](https://backstage.io/docs/dls/design) - [Storybook - UI components](https://backstage.io/storybook) From 533a665a69be3022b402aaddf84993405320c914 Mon Sep 17 00:00:00 2001 From: Adam Harvey Date: Mon, 18 Jan 2021 21:30:47 -0500 Subject: [PATCH 34/75] Update copyright --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 576ec6e563..7c04672998 100644 --- a/README.md +++ b/README.md @@ -57,6 +57,6 @@ Check out [the documentation](https://backstage.io/docs/getting-started) on how ## License -Copyright 2020 © Backstage Project Authors. All rights reserved. The Linux Foundation has registered trademarks and uses trademarks. For a list of trademarks of The Linux Foundation, please see our Trademark Usage page: https://www.linuxfoundation.org/trademark-usage +Copyright 2020-2021 © Backstage Project Authors. All rights reserved. The Linux Foundation has registered trademarks and uses trademarks. For a list of trademarks of The Linux Foundation, please see our Trademark Usage page: https://www.linuxfoundation.org/trademark-usage Licensed under the Apache License, Version 2.0: http://www.apache.org/licenses/LICENSE-2.0 From 4a88f5d5a58d262a27e3a315b782ad30bc3915ee Mon Sep 17 00:00:00 2001 From: Adam Harvey Date: Mon, 18 Jan 2021 23:40:48 -0500 Subject: [PATCH 35/75] Fix floating point math precision --- .../src/components/Cards/LastLighthouseAuditCard.test.tsx | 2 +- .../lighthouse/src/components/Cards/LastLighthouseAuditCard.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/lighthouse/src/components/Cards/LastLighthouseAuditCard.test.tsx b/plugins/lighthouse/src/components/Cards/LastLighthouseAuditCard.test.tsx index 20b3b9c072..42c213b42e 100644 --- a/plugins/lighthouse/src/components/Cards/LastLighthouseAuditCard.test.tsx +++ b/plugins/lighthouse/src/components/Cards/LastLighthouseAuditCard.test.tsx @@ -38,7 +38,7 @@ const websiteListResponse = data as WebsiteListResponse; let entityWebsite = websiteListResponse.items[2]; describe('', () => { - const asPercentage = (fraction: number) => `${fraction * 100}%`; + const asPercentage = (fraction: number) => `${Math.round(fraction * 100)}%`; beforeEach(() => { (useWebsiteForEntity as jest.Mock).mockReturnValue({ diff --git a/plugins/lighthouse/src/components/Cards/LastLighthouseAuditCard.tsx b/plugins/lighthouse/src/components/Cards/LastLighthouseAuditCard.tsx index 0bb83077ce..d30b9a3312 100644 --- a/plugins/lighthouse/src/components/Cards/LastLighthouseAuditCard.tsx +++ b/plugins/lighthouse/src/components/Cards/LastLighthouseAuditCard.tsx @@ -27,7 +27,7 @@ import { useWebsiteForEntity } from '../../hooks/useWebsiteForEntity'; import AuditStatusIcon from '../AuditStatusIcon'; const LighthouseCategoryScoreStatus = ({ score }: { score: number }) => { - const scoreAsPercentage = score * 100; + const scoreAsPercentage = Math.round(score * 100); switch (true) { case scoreAsPercentage >= 90: return ( From debf359b56f50293fd06aac3bbc5f409fed0257b Mon Sep 17 00:00:00 2001 From: Adam Harvey Date: Mon, 18 Jan 2021 23:42:21 -0500 Subject: [PATCH 36/75] Add changeset --- .changeset/warm-months-bake.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/warm-months-bake.md diff --git a/.changeset/warm-months-bake.md b/.changeset/warm-months-bake.md new file mode 100644 index 0000000000..c8f3723eb1 --- /dev/null +++ b/.changeset/warm-months-bake.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-lighthouse': patch +--- + +Fix display of floating point precision errors in card category scores From b473cf08b51fc37dbc36a8deddcf1d0ec9192fd0 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 19 Jan 2021 09:55:11 +0100 Subject: [PATCH 37/75] docs/adrs: remove status --- docs/architecture-decisions/adr000-template.md | 4 ---- .../architecture-decisions/adr001-add-adr-log.md | 16 ++++++++++------ .../adr002-default-catalog-file-format.md | 4 ---- .../adr003-avoid-default-exports.md | 4 ---- .../adr004-module-export-structure.md | 4 ---- .../adr005-catalog-core-entities.md | 4 ---- 6 files changed, 10 insertions(+), 26 deletions(-) diff --git a/docs/architecture-decisions/adr000-template.md b/docs/architecture-decisions/adr000-template.md index 783db8e4bc..7388b000d3 100644 --- a/docs/architecture-decisions/adr000-template.md +++ b/docs/architecture-decisions/adr000-template.md @@ -4,10 +4,6 @@ title: ADR000: [TITLE] description: Architecture Decision Record (ADR) for [TITLE] [DESCRIPTION] --- -| Created | Status | -| ---------- | ------ | -| YYYY-MM-DD | Open | - # ADR000: [title] diff --git a/docs/architecture-decisions/adr001-add-adr-log.md b/docs/architecture-decisions/adr001-add-adr-log.md index 16783367ab..872b3311a5 100644 --- a/docs/architecture-decisions/adr001-add-adr-log.md +++ b/docs/architecture-decisions/adr001-add-adr-log.md @@ -4,12 +4,16 @@ title: ADR001: Architecture Decision Record (ADR) log description: Architecture Decision Record (ADR) logs as a reference point for the team --- -| Created | Status | -| ---------- | ------ | -| 2020-04-26 | Open | +## Decision -## Decision: A decision was made to store ADRs in a log in the project repository +A decision was made to store ADRs in a log in the project repository -## Discussion: There is a need to store big decisions made in a log as a reference point for the team, help with onboarding new members and give context to others interested in the project. +## Discussion -## Risks: People stop adding ADRs to the log and context gets lost +There is a need to store big decisions made in a log as a reference point for +the team, help with onboarding new members and give context to others interested +in the project. + +## Risks + +People stop adding ADRs to the log and context gets lost diff --git a/docs/architecture-decisions/adr002-default-catalog-file-format.md b/docs/architecture-decisions/adr002-default-catalog-file-format.md index 699f7af8ed..8523b4a111 100644 --- a/docs/architecture-decisions/adr002-default-catalog-file-format.md +++ b/docs/architecture-decisions/adr002-default-catalog-file-format.md @@ -4,10 +4,6 @@ title: ADR002: Default Software Catalog File Format description: Architecture Decision Record (ADR) log on Default Software Catalog File Format --- -| Created | Status | -| ---------- | ------ | -| 2020-05-17 | Open | - ## Background Backstage comes with a software catalog functionality, that you can use to track diff --git a/docs/architecture-decisions/adr003-avoid-default-exports.md b/docs/architecture-decisions/adr003-avoid-default-exports.md index d08bc6a882..878d0d701c 100644 --- a/docs/architecture-decisions/adr003-avoid-default-exports.md +++ b/docs/architecture-decisions/adr003-avoid-default-exports.md @@ -4,10 +4,6 @@ title: ADR003: Avoid Default Exports and Prefer Named Exports description: Architecture Decision Record (ADR) log on Avoid Default Exports and Prefer Named Exports --- -| Created | Status | -| ---------- | ------ | -| 2020-05-19 | Open | - ## Context When CommonJS was the primary authoring format, the best practice was to export diff --git a/docs/architecture-decisions/adr004-module-export-structure.md b/docs/architecture-decisions/adr004-module-export-structure.md index 14c94c4ed1..846ca8feae 100644 --- a/docs/architecture-decisions/adr004-module-export-structure.md +++ b/docs/architecture-decisions/adr004-module-export-structure.md @@ -4,10 +4,6 @@ title: ADR004: Module Export Structure description: Architecture Decision Record (ADR) log on Module Export Structure --- -| Created | Status | -| ---------- | ------ | -| 2020-05-27 | Open | - ## Context With a growing number of exports of packages like `@backstage/core`, it is diff --git a/docs/architecture-decisions/adr005-catalog-core-entities.md b/docs/architecture-decisions/adr005-catalog-core-entities.md index 34d6449c6b..83196c12e2 100644 --- a/docs/architecture-decisions/adr005-catalog-core-entities.md +++ b/docs/architecture-decisions/adr005-catalog-core-entities.md @@ -4,10 +4,6 @@ title: ADR005: Catalog Core Entities description: Architecture Decision Record (ADR) log on Catalog Core Entities --- -| Created | Status | -| ---------- | ------ | -| 2020-05-29 | Open | - ## Context We want to standardize on a few core entities that we are tracking in the From 53c9c51f2161c847cb6459ef4dd10c0bdd865758 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Tue, 19 Jan 2021 10:17:12 +0100 Subject: [PATCH 38/75] Create techdocs-glasses-wonder.md --- .changeset/techdocs-glasses-wonder.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/techdocs-glasses-wonder.md diff --git a/.changeset/techdocs-glasses-wonder.md b/.changeset/techdocs-glasses-wonder.md new file mode 100644 index 0000000000..f3c9adb29f --- /dev/null +++ b/.changeset/techdocs-glasses-wonder.md @@ -0,0 +1,5 @@ +--- +"@backstage/techdocs-common": patch +--- + +TechDocs backend now streams files through from Google Cloud Storage to the browser, improving memory usage. From a65c4516ec854875e7d21139f9418bcc2bad1c11 Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Tue, 19 Jan 2021 10:41:40 +0100 Subject: [PATCH 39/75] Use GitHub API to download the archive --- .../src/reading/GithubUrlReader.test.ts | 33 +++++++++----- .../src/reading/GithubUrlReader.ts | 42 +++++++++--------- .../backstage-mock-etag123.tar.gz | Bin 0 -> 240 bytes 3 files changed, 42 insertions(+), 33 deletions(-) create mode 100644 packages/backend-common/src/reading/__fixtures__/backstage-mock-etag123.tar.gz diff --git a/packages/backend-common/src/reading/GithubUrlReader.test.ts b/packages/backend-common/src/reading/GithubUrlReader.test.ts index 26ba91bd29..0b7d56480f 100644 --- a/packages/backend-common/src/reading/GithubUrlReader.test.ts +++ b/packages/backend-common/src/reading/GithubUrlReader.test.ts @@ -108,7 +108,12 @@ describe('GithubUrlReader', () => { describe('readTree', () => { const repoBuffer = fs.readFileSync( - path.resolve('src', 'reading', '__fixtures__', 'mock-main.tar.gz'), + path.resolve( + 'src', + 'reading', + '__fixtures__', + 'backstage-mock-etag123.tar.gz', + ), ); const reposGithubApiResponse = { @@ -117,12 +122,16 @@ describe('GithubUrlReader', () => { default_branch: 'main', branches_url: 'https://api.github.com/repos/backstage/mock/branches{/branch}', + archive_url: + 'https://api.github.com/repos/backstage/mock/{archive_format}{/ref}', }; const reposGheApiResponse = { ...reposGithubApiResponse, branches_url: 'https://ghe.github.com/api/v3/repos/backstage/mock/branches{/branch}', + archive_url: + 'https://ghe.github.com/api/v3/repos/backstage/mock/{archive_format}{/ref}', }; const branchesApiResponse = { @@ -134,15 +143,6 @@ describe('GithubUrlReader', () => { beforeEach(() => { worker.use( - rest.get( - 'https://github.com/backstage/mock/archive/main.tar.gz', - (_, res, ctx) => - res( - ctx.status(200), - ctx.set('Content-Type', 'application/x-gzip'), - ctx.body(repoBuffer), - ), - ), rest.get('https://api.github.com/repos/backstage/mock', (_, res, ctx) => res( ctx.status(200), @@ -159,12 +159,21 @@ describe('GithubUrlReader', () => { ctx.json(branchesApiResponse), ), ), + rest.get( + 'https://api.github.com/repos/backstage/mock/tarball/etag123abc', + (_, res, ctx) => + res( + ctx.status(200), + ctx.set('Content-Type', 'application/x-gzip'), + ctx.body(repoBuffer), + ), + ), rest.get( 'https://api.github.com/repos/backstage/mock/branches/branchDoesNotExist', (_, res, ctx) => res(ctx.status(404)), ), rest.get( - 'https://ghe.github.com/backstage/mock/archive/main.tar.gz', + 'https://ghe.github.com/api/v3/repos/backstage/mock/tarball/etag123abc', (_, res, ctx) => res( ctx.status(200), @@ -224,7 +233,7 @@ describe('GithubUrlReader', () => { worker.use( rest.get( - 'https://ghe.github.com/backstage/mock/archive/main.tar.gz', + 'https://ghe.github.com/api/v3/repos/backstage/mock/tarball/etag123abc', (req, res, ctx) => { expect(req.headers.get('authorization')).toBe( mockHeaders.Authorization, diff --git a/packages/backend-common/src/reading/GithubUrlReader.ts b/packages/backend-common/src/reading/GithubUrlReader.ts index d6612a9da2..b5b4e9c1a7 100644 --- a/packages/backend-common/src/reading/GithubUrlReader.ts +++ b/packages/backend-common/src/reading/GithubUrlReader.ts @@ -98,14 +98,9 @@ export class GithubUrlReader implements UrlReader { url: string, options?: ReadTreeOptions, ): Promise { - const { - protocol, - resource, - name: repoName, - ref, - filepath, - full_name, - } = parseGitUrl(url); + const { ref, filepath, full_name } = parseGitUrl(url); + // Caveat: The ref will totally be incorrect if the branch name includes a / + // Thus, readTree can not work on url containing branch name that has a / const { headers } = await this.deps.credentialsProvider.getCredentials({ url, @@ -132,6 +127,7 @@ export class GithubUrlReader implements UrlReader { // Use GitHub API to get the default branch of the repository. const branch = ref || repoResponseJson.default_branch; const branchesApiUrl = repoResponseJson.branches_url; + const archiveApiUrl = repoResponseJson.archive_url; // Fetch the latest commit in the provided or default branch to compare against // the provided sha. @@ -155,19 +151,12 @@ export class GithubUrlReader implements UrlReader { throw new NotModifiedError(); } - // Note: the API way of downloading an archive URL does not return a real time archive. - // https://github.community/t/archive-downloaded-via-v3-rest-api-is-not-real-time/14827 - // It looks like this https://api.github.com/repos/owner/repo/{archive_format}{/ref} - // and can be used from `repoResponseJson.archive_url`. - // Continue using the "direct" way i.e. https://github.com/:owner/:repo/archive/branch.tar.gz - // until the bug? is fixed. const archive = await fetch( - new URL( - `${protocol}://${resource}/${full_name}/archive/${branch}.tar.gz`, - ).toString(), - { - headers, - }, + // archiveApiUrl looks like "https://api.github.com/repos/owner/repo/{archive_format}{/ref}" + archiveApiUrl + .replace('{archive_format}', 'tarball') + .replace('{/ref}', `/${commitSha}`), + { headers }, ); if (!archive.ok) { const message = `Failed to read tree (archive) from ${url}, ${archive.status} ${archive.statusText}`; @@ -177,7 +166,18 @@ export class GithubUrlReader implements UrlReader { throw new Error(message); } - const path = `${repoName}-${branch}/${filepath}`; + // Note that repoResponseJson.full_name must be used over full_name because the path + // is case sensitive and full_name may not be inq the correct case. + // TODO(OrkoHunter): The directory name inside the tarball should be retrieved from the tar + // instead of being constructed here. Same goes for GitLab, Bitbucket and Azure. + const extractedDirName = `${repoResponseJson.full_name.replace( + '/', + '-', + )}-${commitSha.substr(0, 7)}`; + + // The path includes the name of the directory inside the tarball and a sub path + // if requested in readTree. + const path = `${extractedDirName}/${filepath}`; return await this.deps.treeResponseFactory.fromTarArchive({ // TODO(Rugvip): Underlying implementation of fetch will be node-fetch, we probably want diff --git a/packages/backend-common/src/reading/__fixtures__/backstage-mock-etag123.tar.gz b/packages/backend-common/src/reading/__fixtures__/backstage-mock-etag123.tar.gz new file mode 100644 index 0000000000000000000000000000000000000000..e1ac2579de3999e2847c562c9d0882f486b81e22 GIT binary patch literal 240 zcmVS&>cxevkEi2meR ze~QeW5auDUP#vJO?yE@2E!s)ltbW~(V_pW{|Lyf#`vgAne+tI`LoC4g{~V0z qUsS0)2P*xx#-#s4_^Lkv0mAwJ9ITz~I~)$jBDn%0oP%uu5C8yN%yPE? literal 0 HcmV?d00001 From 18c97a8757d86326d35c0d8b13e43f4b771d7c99 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Tue, 19 Jan 2021 12:01:10 +0100 Subject: [PATCH 40/75] Prettier? --- .changeset/techdocs-glasses-wonder.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/techdocs-glasses-wonder.md b/.changeset/techdocs-glasses-wonder.md index f3c9adb29f..7610be2665 100644 --- a/.changeset/techdocs-glasses-wonder.md +++ b/.changeset/techdocs-glasses-wonder.md @@ -1,5 +1,5 @@ --- -"@backstage/techdocs-common": patch +'@backstage/techdocs-common': patch --- TechDocs backend now streams files through from Google Cloud Storage to the browser, improving memory usage. From d9de82ee6c24eb8e84f27bdd490dc456316743c3 Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Sat, 9 Jan 2021 23:04:22 +0100 Subject: [PATCH 41/75] docs: How to build TechDocs sites on CI/CD workflows --- docs/features/techdocs/architecture.md | 5 +- docs/features/techdocs/configuring-ci-cd.md | 87 +++++++++++++++++++++ microsite/sidebars.json | 1 + 3 files changed, 90 insertions(+), 3 deletions(-) create mode 100644 docs/features/techdocs/configuring-ci-cd.md diff --git a/docs/features/techdocs/architecture.md b/docs/features/techdocs/architecture.md index 7c35fbc943..53149ae4f3 100644 --- a/docs/features/techdocs/architecture.md +++ b/docs/features/techdocs/architecture.md @@ -142,12 +142,11 @@ Status of all the features mentioned above. - Basic setup with techdocs-backend file server as storage. - Basic setup with cloud storage solution. - -**Work in progress 🚧** - - `techdocs-cli` is able to generate docs in CI/CD environment. - `techdocs-cli` is able to publish docs site to any storage. +**Work in progress 🚧** + **Not implemented yet ❌** - `techdocs-backend` integration with Backstage access control management. diff --git a/docs/features/techdocs/configuring-ci-cd.md b/docs/features/techdocs/configuring-ci-cd.md new file mode 100644 index 0000000000..ec25684a94 --- /dev/null +++ b/docs/features/techdocs/configuring-ci-cd.md @@ -0,0 +1,87 @@ +--- +id: configuring-ci-cd +title: Configuring CI/CD to generate and publish TechDocs sites +description: + Configuring CI/CD to generate and publish TechDocs sites to cloud storage +--- + +In the [Recommended deployment setup](./architecture.md#recommended-deployment), +TechDocs reads the static generated documentation files from a cloud storage +bucket (GCS, AWS S3, etc.). The documentation site is generated on the CI/CD +workflow associated with the repository containing the documentation files. This +document explains the steps needed to generate docs on CI and publish to a cloud +storage using [`techdocs-cli`](https://github.com/backstage/techdocs-cli). + +The steps here target all kinds of CI providers (GitHub actions, Circle CI, +Jenkins, etc.) Specific tools for individual providers will also be made +available here for simplicity (e.g. A GitHub Actions runner, CircleCI orb, etc.) + +A summary of the instructions below looks like this - + +```sh +# Prepare +REPOSITORY_URL='https://github.com/org/repo' +git clone $REPOSITORY_URL + +# Generate +npx techdocs-cli generate --source-dir ./repo --output-dir ./site + +# Publish +npx techdocs-cli publish --directory ./site --publisher-type awsS3 --bucket-name --entity + +# That's it! +``` + +## 1. Setup a workflow + +The TechDocs workflow should trigger on CI when any changes are made in the +repository containing the documentation files. You can be specific and trigger +the workflow only on changes to files inside the `docs/` directory or +`mkdocs.yml`. + +## 2. Prepare step + +The first step on the CI is to clone the repository in a working directory. This +is almost always the first step in most CI workflows. + +On GitHub actions, you can add a step + +[`- uses: actions@checkout@v2`](https://github.com/actions/checkout). + +On CircleCI, you can add a special +[`checkout`](https://circleci.com/docs/2.0/configuration-reference/#checkout) +step. + +Eventually we are trying to do a `git clone `. + +## 3. Generate step + +Install [`npx`](https://www.npmjs.com/package/npx) to use it for running +`techdocs-cli`. We are going to use the `techdocs-cli generate` command here. + +Take a look at +[`techdocs-cli` README](https://github.com/backstage/techdocs-cli) for the +complete command reference, details, and options. + +``` +npx techdocs-cli generate --no-docker --source-dir PATH_TO_REPO --output-dir ./site +``` + +`PATH_TO_REPO` should be the location where the prepare step above clones the +repository. + +## 4. Publish step + +Take a look at +[`techdocs-cli` README](https://github.com/backstage/techdocs-cli) for the +complete command reference, details, and options. + +Depending on your cloud storage provider (AWS or Google Cloud), set the +necessary authentication environment variables. + +- [Google Cloud authentication](https://cloud.google.com/storage/docs/authentication#libauth) +- [AWS authentication](https://docs.aws.amazon.com/sdk-for-javascript/v3/developer-guide/loading-node-credentials-environment.html) + +``` +techdocs-cli publish --directory ./site --publisher-type --bucket-name --entity +``` diff --git a/microsite/sidebars.json b/microsite/sidebars.json index f5a433fc84..4822ffe397 100644 --- a/microsite/sidebars.json +++ b/microsite/sidebars.json @@ -91,6 +91,7 @@ "features/techdocs/creating-and-publishing", "features/techdocs/configuration", "features/techdocs/using-cloud-storage", + "features/techdocs/configuring-ci-cd", "features/techdocs/how-to-guides", "features/techdocs/troubleshooting", "features/techdocs/faqs" From 0dc2e8a97ebc0a26be25a261a0289e86981d62e1 Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Sun, 10 Jan 2021 10:19:45 +0100 Subject: [PATCH 42/75] docs: fix npx command for techdocs-cli --- docs/features/techdocs/configuring-ci-cd.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/features/techdocs/configuring-ci-cd.md b/docs/features/techdocs/configuring-ci-cd.md index ec25684a94..37d1b56692 100644 --- a/docs/features/techdocs/configuring-ci-cd.md +++ b/docs/features/techdocs/configuring-ci-cd.md @@ -24,10 +24,10 @@ REPOSITORY_URL='https://github.com/org/repo' git clone $REPOSITORY_URL # Generate -npx techdocs-cli generate --source-dir ./repo --output-dir ./site +npx @techdocs/cli generate --source-dir ./repo --output-dir ./site # Publish -npx techdocs-cli publish --directory ./site --publisher-type awsS3 --bucket-name --entity +npx @techdocs/cli publish --directory ./site --publisher-type awsS3 --bucket-name --entity # That's it! ``` @@ -64,7 +64,7 @@ Take a look at complete command reference, details, and options. ``` -npx techdocs-cli generate --no-docker --source-dir PATH_TO_REPO --output-dir ./site +npx @techdocs/cli generate --no-docker --source-dir PATH_TO_REPO --output-dir ./site ``` `PATH_TO_REPO` should be the location where the prepare step above clones the @@ -83,5 +83,5 @@ necessary authentication environment variables. - [AWS authentication](https://docs.aws.amazon.com/sdk-for-javascript/v3/developer-guide/loading-node-credentials-environment.html) ``` -techdocs-cli publish --directory ./site --publisher-type --bucket-name --entity +npx @techdocs/cli publish --directory ./site --publisher-type --bucket-name --entity ``` From 6b6bcb549a21ef59e4ba14e26aba31f4ebc2971a Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Tue, 19 Jan 2021 12:59:34 +0100 Subject: [PATCH 43/75] Apply suggestions from code review Co-authored-by: Adam Harvey --- docs/features/techdocs/configuring-ci-cd.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/features/techdocs/configuring-ci-cd.md b/docs/features/techdocs/configuring-ci-cd.md index 37d1b56692..5da7addb9a 100644 --- a/docs/features/techdocs/configuring-ci-cd.md +++ b/docs/features/techdocs/configuring-ci-cd.md @@ -12,7 +12,7 @@ workflow associated with the repository containing the documentation files. This document explains the steps needed to generate docs on CI and publish to a cloud storage using [`techdocs-cli`](https://github.com/backstage/techdocs-cli). -The steps here target all kinds of CI providers (GitHub actions, Circle CI, +The steps here target all kinds of CI providers (GitHub Actions, CircleCI, Jenkins, etc.) Specific tools for individual providers will also be made available here for simplicity (e.g. A GitHub Actions runner, CircleCI orb, etc.) @@ -44,7 +44,7 @@ the workflow only on changes to files inside the `docs/` directory or The first step on the CI is to clone the repository in a working directory. This is almost always the first step in most CI workflows. -On GitHub actions, you can add a step +On GitHub Actions, you can add a step [`- uses: actions@checkout@v2`](https://github.com/actions/checkout). @@ -67,7 +67,7 @@ complete command reference, details, and options. npx @techdocs/cli generate --no-docker --source-dir PATH_TO_REPO --output-dir ./site ``` -`PATH_TO_REPO` should be the location where the prepare step above clones the +`PATH_TO_REPO` should be the location in the file path where the prepare step above clones the repository. ## 4. Publish step @@ -76,7 +76,7 @@ Take a look at [`techdocs-cli` README](https://github.com/backstage/techdocs-cli) for the complete command reference, details, and options. -Depending on your cloud storage provider (AWS or Google Cloud), set the +Depending on your cloud storage provider (AWS, Google Cloud, or Azure), set the necessary authentication environment variables. - [Google Cloud authentication](https://cloud.google.com/storage/docs/authentication#libauth) From cc4df5eff43486dd1906554755170be3eb1779e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Tue, 19 Jan 2021 13:48:10 +0100 Subject: [PATCH 44/75] chore: fullcreen -> fullscreen --- plugins/scaffolder/src/components/JobStage/JobStage.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/scaffolder/src/components/JobStage/JobStage.tsx b/plugins/scaffolder/src/components/JobStage/JobStage.tsx index 86c2982dde..f285cb98c0 100644 --- a/plugins/scaffolder/src/components/JobStage/JobStage.tsx +++ b/plugins/scaffolder/src/components/JobStage/JobStage.tsx @@ -161,7 +161,7 @@ export const JobStage = ({ endedAt, startedAt, name, log, status }: Props) => { From 6c64b4b8b94a7c83acb3fb0cc1380c9c7ad466f0 Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Tue, 19 Jan 2021 13:56:18 +0100 Subject: [PATCH 45/75] docs: Search arch diagram catalogue -> catalog --- docs/assets/search/architecture.drawio.svg | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/assets/search/architecture.drawio.svg b/docs/assets/search/architecture.drawio.svg index 736d4545f0..af04ab63e8 100644 --- a/docs/assets/search/architecture.drawio.svg +++ b/docs/assets/search/architecture.drawio.svg @@ -92,7 +92,7 @@
Other Plugins
- (TechDocs, Catalogue, Etc) + (TechDocs, Catalog, Etc)
@@ -147,13 +147,13 @@
- Other Backend Plugin (TechDocs, Catalogue, Etc) + Other Backend Plugin (TechDocs, Catalog, Etc)
- Other Backend Plugin (TechDocs, Catalogue, Etc) + Other Backend Plugin (TechDocs, Catalog, Etc) From 3e24d89290ce9357963db5e423a449832705915b Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Tue, 19 Jan 2021 13:18:00 +0100 Subject: [PATCH 46/75] 1. Use --storage-name instead of bucket name 2. Prettier ignore description 3. Refer to CLI readme --- docs/features/techdocs/configuring-ci-cd.md | 58 ++++++++++++--------- 1 file changed, 34 insertions(+), 24 deletions(-) diff --git a/docs/features/techdocs/configuring-ci-cd.md b/docs/features/techdocs/configuring-ci-cd.md index 5da7addb9a..751785f880 100644 --- a/docs/features/techdocs/configuring-ci-cd.md +++ b/docs/features/techdocs/configuring-ci-cd.md @@ -1,8 +1,8 @@ --- id: configuring-ci-cd title: Configuring CI/CD to generate and publish TechDocs sites -description: - Configuring CI/CD to generate and publish TechDocs sites to cloud storage +# prettier-ignore +description: Configuring CI/CD to generate and publish TechDocs sites to cloud storage --- In the [Recommended deployment setup](./architecture.md#recommended-deployment), @@ -19,30 +19,37 @@ available here for simplicity (e.g. A GitHub Actions runner, CircleCI orb, etc.) A summary of the instructions below looks like this - ```sh +# This is an example script + # Prepare REPOSITORY_URL='https://github.com/org/repo' git clone $REPOSITORY_URL +cd repo # Generate -npx @techdocs/cli generate --source-dir ./repo --output-dir ./site +npx @techdocs/cli generate # Publish -npx @techdocs/cli publish --directory ./site --publisher-type awsS3 --bucket-name --entity - -# That's it! +npx @techdocs/cli publish --publisher-type awsS3 --storage-name --entity ``` +That's it! + +Take a look at +[`techdocs-cli` README](https://github.com/backstage/techdocs-cli) for the +complete command reference, details, and options. + ## 1. Setup a workflow The TechDocs workflow should trigger on CI when any changes are made in the -repository containing the documentation files. You can be specific and trigger -the workflow only on changes to files inside the `docs/` directory or -`mkdocs.yml`. +repository containing the documentation files. You can be specific and configure +the workflow to be triggered only when files inside the `docs/` directory or +`mkdocs.yml` are changed. ## 2. Prepare step -The first step on the CI is to clone the repository in a working directory. This -is almost always the first step in most CI workflows. +The first step on the CI is to clone your documentation source repository in a +working directory. This is almost always the first step in most CI workflows. On GitHub Actions, you can add a step @@ -57,31 +64,34 @@ Eventually we are trying to do a `git clone `. ## 3. Generate step Install [`npx`](https://www.npmjs.com/package/npx) to use it for running -`techdocs-cli`. We are going to use the `techdocs-cli generate` command here. +`techdocs-cli`. Or you can install using `npm install -g @techdocs/cli`. -Take a look at -[`techdocs-cli` README](https://github.com/backstage/techdocs-cli) for the -complete command reference, details, and options. +We are going to use the +[`techdocs-cli generate`](https://github.com/backstage/techdocs-cli#generate-techdocs-site-from-a-documentation-project) +command in this step. -``` +```sh npx @techdocs/cli generate --no-docker --source-dir PATH_TO_REPO --output-dir ./site ``` -`PATH_TO_REPO` should be the location in the file path where the prepare step above clones the -repository. +`PATH_TO_REPO` should be the location in the file path where the prepare step +above clones the repository. ## 4. Publish step -Take a look at -[`techdocs-cli` README](https://github.com/backstage/techdocs-cli) for the -complete command reference, details, and options. - Depending on your cloud storage provider (AWS, Google Cloud, or Azure), set the necessary authentication environment variables. - [Google Cloud authentication](https://cloud.google.com/storage/docs/authentication#libauth) - [AWS authentication](https://docs.aws.amazon.com/sdk-for-javascript/v3/developer-guide/loading-node-credentials-environment.html) +And then run the +[`techdocs-cli publish`](https://github.com/backstage/techdocs-cli#publish-generated-techdocs-sites) +command. + +```sh +npx @techdocs/cli publish --publisher-type --storage-name --entity --directory ./site ``` -npx @techdocs/cli publish --directory ./site --publisher-type --bucket-name --entity -``` + +The updated TechDocs site built in this workflow is now ready to be served by +the TechDocs plugin in your Backstage app. From cfc0b73951d3bf61a7326f07fad15263f04498cf Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Tue, 19 Jan 2021 15:05:52 +0100 Subject: [PATCH 47/75] Update docs/features/techdocs/configuring-ci-cd.md Co-authored-by: bodilb <66826349+bodilb@users.noreply.github.com> --- docs/features/techdocs/configuring-ci-cd.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/features/techdocs/configuring-ci-cd.md b/docs/features/techdocs/configuring-ci-cd.md index 751785f880..1f53c0fcb2 100644 --- a/docs/features/techdocs/configuring-ci-cd.md +++ b/docs/features/techdocs/configuring-ci-cd.md @@ -13,8 +13,8 @@ document explains the steps needed to generate docs on CI and publish to a cloud storage using [`techdocs-cli`](https://github.com/backstage/techdocs-cli). The steps here target all kinds of CI providers (GitHub Actions, CircleCI, -Jenkins, etc.) Specific tools for individual providers will also be made -available here for simplicity (e.g. A GitHub Actions runner, CircleCI orb, etc.) +Jenkins, etc.). Specific tools for individual providers will also be made +available here for simplicity (e.g. a GitHub Actions runner, CircleCI orb, etc.). A summary of the instructions below looks like this - From 82ca95e3757d37b386683e7e4a071fad0750638a Mon Sep 17 00:00:00 2001 From: Alan Crosswell Date: Tue, 19 Jan 2021 09:50:10 -0500 Subject: [PATCH 48/75] minor editorial suggestions from @freben and @adamdmharvey --- .../software-templates/extending/create-your-own-templater.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/features/software-templates/extending/create-your-own-templater.md b/docs/features/software-templates/extending/create-your-own-templater.md index 450892d914..19c54733b4 100644 --- a/docs/features/software-templates/extending/create-your-own-templater.md +++ b/docs/features/software-templates/extending/create-your-own-templater.md @@ -86,7 +86,7 @@ follows: _note_ Currently the templaters that we provide are basically Docker action containers that are run on top of the skeleton folder. This keeps dependencies -to a minimal for running backstage scaffolder, but you don't /have/ to use +to a minimum for running backstage scaffolder, but you don't _have_ to use Docker. You can `pip install cookiecutter` to run it locally in your backend. You could create your own templater that spins up an EC2 instance and downloads the folder and does everything using an AMI if you want. It's entirely up to From def2307f3367d2143b31ca45dd8bfc15729709b8 Mon Sep 17 00:00:00 2001 From: Dominik Henneke Date: Tue, 19 Jan 2021 14:05:52 +0100 Subject: [PATCH 49/75] Add a managed-by-origin-location annotation --- .changeset/angry-flowers-yawn.md | 29 +++++++++ .../well-known-annotations.md | 17 +++++ .../catalog-model/src/location/annotation.ts | 2 + packages/catalog-model/src/location/index.ts | 2 +- .../src/ingestion/LocationReaders.ts | 20 +++--- .../AnnotateLocationEntityProcessor.test.ts | 62 +++++++++++++++++++ .../AnnotateLocationEntityProcessor.ts | 14 ++++- .../src/ingestion/processors/types.ts | 4 ++ 8 files changed, 139 insertions(+), 11 deletions(-) create mode 100644 .changeset/angry-flowers-yawn.md create mode 100644 plugins/catalog-backend/src/ingestion/processors/AnnotateLocationEntityProcessor.test.ts diff --git a/.changeset/angry-flowers-yawn.md b/.changeset/angry-flowers-yawn.md new file mode 100644 index 0000000000..8f478b1eab --- /dev/null +++ b/.changeset/angry-flowers-yawn.md @@ -0,0 +1,29 @@ +--- +'@backstage/catalog-model': patch +'@backstage/plugin-catalog-backend': patch +--- + +Adds a `backstage.io/managed-by-origin-location` annotation to all entities. It links to the +location that was registered to the catalog and which emitted this entity. It has a different +semantic than the existing `backstage.io/managed-by-location` annotation, which tells the direct +parent location that created this entity. + +Consider this example: The Backstage operator adds a location of type `github-org` in the +`app-config.yaml`. This setting will be added to a `bootstrap:boostrap` location. The processor +discovers the entities in the following branch +`Location bootstrap:bootstrap -> Location github-org:… -> User xyz`. The user `xyz` will be: + +```yaml +apiVersion: backstage.io/v1alpha1 +kind: User +metadata: + name: xyz + annotations: + # This entity was added by the 'github-org:…' location + backstage.io/managed-by-location: github-org:… + # The entity was added because the 'bootstrap:boostrap' was added to the catalog + backstage.io/managed-by-origin-location: bootstrap:bootstrap + # ... +spec: + # ... +``` diff --git a/docs/features/software-catalog/well-known-annotations.md b/docs/features/software-catalog/well-known-annotations.md index 3627c74b9a..e3f9e86fae 100644 --- a/docs/features/software-catalog/well-known-annotations.md +++ b/docs/features/software-catalog/well-known-annotations.md @@ -40,6 +40,23 @@ expecting a two-item array out of it. The format of the target part is type-dependent and could conceivably even be an empty string, but the separator colon is always present. +### backstage.io/managed-by-origin-location + +```yaml +# Example: +metadata: + annotations: + backstage.io/managed-by-origin-location: github:http://github.com/backstage/backstage/catalog-info.yaml +``` + +The value of this annotation is a location reference string (see above). It +points to the location, which registration lead to the creation of the entity. +In most cases, the `backstage.io/managed-by-location` and +`backstage.io/managed-by-origin-location` will be equal. It will be different if +the original location delegates to another location. A common case is, that a +location is registered via the `bootstrap:boostrap` which means that is part of +the `app-config.yml` of a backstage installation. + ### backstage.io/techdocs-ref ```yaml diff --git a/packages/catalog-model/src/location/annotation.ts b/packages/catalog-model/src/location/annotation.ts index 371d095685..93f2fabea4 100644 --- a/packages/catalog-model/src/location/annotation.ts +++ b/packages/catalog-model/src/location/annotation.ts @@ -15,3 +15,5 @@ */ export const LOCATION_ANNOTATION = 'backstage.io/managed-by-location'; +export const ORIGIN_LOCATION_ANNOTATION = + 'backstage.io/managed-by-origin-location'; diff --git a/packages/catalog-model/src/location/index.ts b/packages/catalog-model/src/location/index.ts index ce64b988a6..8fd516120a 100644 --- a/packages/catalog-model/src/location/index.ts +++ b/packages/catalog-model/src/location/index.ts @@ -20,4 +20,4 @@ export { locationSpecSchema, analyzeLocationSchema, } from './validation'; -export { LOCATION_ANNOTATION } from './annotation'; +export { LOCATION_ANNOTATION, ORIGIN_LOCATION_ANNOTATION } from './annotation'; diff --git a/plugins/catalog-backend/src/ingestion/LocationReaders.ts b/plugins/catalog-backend/src/ingestion/LocationReaders.ts index a9248bb3fb..cb703b8e15 100644 --- a/plugins/catalog-backend/src/ingestion/LocationReaders.ts +++ b/plugins/catalog-backend/src/ingestion/LocationReaders.ts @@ -78,13 +78,17 @@ export class LocationReaders implements LocationReader { if (rulesEnforcer.isAllowed(item.entity, item.location)) { const relations = Array(); - const entity = await this.handleEntity(item, emitResult => { - if (emitResult.type === 'relation') { - relations.push(emitResult.relation); - return; - } - emit(emitResult); - }); + const entity = await this.handleEntity( + item, + emitResult => { + if (emitResult.type === 'relation') { + relations.push(emitResult.relation); + return; + } + emit(emitResult); + }, + location, + ); if (entity) { output.entities.push({ @@ -165,6 +169,7 @@ export class LocationReaders implements LocationReader { private async handleEntity( item: CatalogProcessorEntityResult, emit: CatalogProcessorEmit, + originLocation: LocationSpec, ): Promise { const { processors, logger } = this.options; @@ -185,6 +190,7 @@ export class LocationReaders implements LocationReader { current, item.location, emit, + originLocation, ); } catch (e) { const message = `Processor ${processor.constructor.name} threw an error while preprocessing entity ${kind}:${namespace}/${name} at ${item.location.type} ${item.location.target}, ${e}`; diff --git a/plugins/catalog-backend/src/ingestion/processors/AnnotateLocationEntityProcessor.test.ts b/plugins/catalog-backend/src/ingestion/processors/AnnotateLocationEntityProcessor.test.ts new file mode 100644 index 0000000000..cfd98e9e6a --- /dev/null +++ b/plugins/catalog-backend/src/ingestion/processors/AnnotateLocationEntityProcessor.test.ts @@ -0,0 +1,62 @@ +/* + * Copyright 2020 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 { Entity, LocationSpec } from '@backstage/catalog-model'; +import { AnnotateLocationEntityProcessor } from './AnnotateLocationEntityProcessor'; + +describe('AnnotateLocationEntityProcessor', () => { + describe('preProcessEntity', () => { + it('adds annotations', async () => { + const entity: Entity = { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', + metadata: { + name: 'my-component', + }, + }; + + const location: LocationSpec = { + type: 'url', + target: 'my-location', + }; + const originLocation: LocationSpec = { + type: 'url', + target: 'my-origin-location', + }; + + const processor = new AnnotateLocationEntityProcessor(); + + expect( + await processor.preProcessEntity( + entity, + location, + () => {}, + originLocation, + ), + ).toEqual({ + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', + metadata: { + name: 'my-component', + annotations: { + 'backstage.io/managed-by-location': 'url:my-location', + 'backstage.io/managed-by-origin-location': 'url:my-origin-location', + }, + }, + }); + }); + }); +}); diff --git a/plugins/catalog-backend/src/ingestion/processors/AnnotateLocationEntityProcessor.ts b/plugins/catalog-backend/src/ingestion/processors/AnnotateLocationEntityProcessor.ts index ea8afcddc5..b40378226a 100644 --- a/plugins/catalog-backend/src/ingestion/processors/AnnotateLocationEntityProcessor.ts +++ b/plugins/catalog-backend/src/ingestion/processors/AnnotateLocationEntityProcessor.ts @@ -14,20 +14,28 @@ * limitations under the License. */ -import { Entity, LocationSpec } from '@backstage/catalog-model'; +import { + Entity, + LOCATION_ANNOTATION, + LocationSpec, + ORIGIN_LOCATION_ANNOTATION, +} from '@backstage/catalog-model'; import lodash from 'lodash'; -import { CatalogProcessor } from './types'; +import { CatalogProcessor, CatalogProcessorEmit } from './types'; export class AnnotateLocationEntityProcessor implements CatalogProcessor { async preProcessEntity( entity: Entity, location: LocationSpec, + _: CatalogProcessorEmit, + originLocation: LocationSpec, ): Promise { return lodash.merge( { metadata: { annotations: { - 'backstage.io/managed-by-location': `${location.type}:${location.target}`, + [LOCATION_ANNOTATION]: `${location.type}:${location.target}`, + [ORIGIN_LOCATION_ANNOTATION]: `${originLocation.type}:${originLocation.target}`, }, }, }, diff --git a/plugins/catalog-backend/src/ingestion/processors/types.ts b/plugins/catalog-backend/src/ingestion/processors/types.ts index 5da3510f7a..1bf30567bc 100644 --- a/plugins/catalog-backend/src/ingestion/processors/types.ts +++ b/plugins/catalog-backend/src/ingestion/processors/types.ts @@ -46,12 +46,16 @@ export type CatalogProcessor = { * @param entity The (possibly partial) entity to process * @param location The location that the entity came from * @param emit A sink for auxiliary items resulting from the processing + * @param originLocation The location that the entity originally came from. + * While location resolves to the direct parent location, originLocation + * tells which location was used to start the ingestion loop. * @returns The same entity or a modified version of it */ preProcessEntity?( entity: Entity, location: LocationSpec, emit: CatalogProcessorEmit, + originLocation: LocationSpec, ): Promise; /** From 593632f0787c008d7ff8089bf7b05049a1cb44cc Mon Sep 17 00:00:00 2001 From: Dominik Henneke Date: Tue, 19 Jan 2021 15:51:40 +0100 Subject: [PATCH 50/75] Block deleting entities from the bootstrap location in the unregister dialog and list the correct delete preview --- .changeset/cool-horses-applaud.md | 6 ++ .../UnregisterEntityDialog.tsx | 56 +++++++++++++++---- 2 files changed, 51 insertions(+), 11 deletions(-) create mode 100644 .changeset/cool-horses-applaud.md diff --git a/.changeset/cool-horses-applaud.md b/.changeset/cool-horses-applaud.md new file mode 100644 index 0000000000..f934634225 --- /dev/null +++ b/.changeset/cool-horses-applaud.md @@ -0,0 +1,6 @@ +--- +'@backstage/plugin-catalog': patch +--- + +Derive the list of to-deleted entities in the `UnregisterEntityDialog` from the `backstage.io/managed-by-origin-location` annotation. +The dialog also rejects deleting entities that are created by the `bootstrap:bootstrap` location. diff --git a/plugins/catalog/src/components/UnregisterEntityDialog/UnregisterEntityDialog.tsx b/plugins/catalog/src/components/UnregisterEntityDialog/UnregisterEntityDialog.tsx index ce6e63da1b..0c52b24d90 100644 --- a/plugins/catalog/src/components/UnregisterEntityDialog/UnregisterEntityDialog.tsx +++ b/plugins/catalog/src/components/UnregisterEntityDialog/UnregisterEntityDialog.tsx @@ -14,7 +14,7 @@ * limitations under the License. */ -import { Entity, LOCATION_ANNOTATION } from '@backstage/catalog-model'; +import { Entity, ORIGIN_LOCATION_ANNOTATION } from '@backstage/catalog-model'; import { alertApiRef, Progress, useApi } from '@backstage/core'; import { Button, @@ -32,6 +32,7 @@ import React from 'react'; import { useAsync } from 'react-use'; import { AsyncState } from 'react-use/lib/useAsync'; import { catalogApiRef } from '../../plugin'; +import { formatEntityRefTitle } from '../EntityRefLink'; type Props = { open: boolean; @@ -40,15 +41,30 @@ type Props = { entity: Entity; }; +class DeniedLocationException extends Error { + constructor(public readonly locationName: string) { + super(`You may not remove the location ${locationName}`); + this.name = 'DeniedLocationException'; + } +} + function useColocatedEntities(entity: Entity): AsyncState { const catalogApi = useApi(catalogApiRef); return useAsync(async () => { - const myLocation = entity.metadata.annotations?.[LOCATION_ANNOTATION]; + const myLocation = + entity.metadata.annotations?.[ORIGIN_LOCATION_ANNOTATION]; if (!myLocation) { return []; } + + if (myLocation === 'bootstrap:bootstrap') { + throw new DeniedLocationException(myLocation); + } + const response = await catalogApi.getEntities({ - filter: { [LOCATION_ANNOTATION]: myLocation }, + filter: { + [`metadata.annotations.${ORIGIN_LOCATION_ANNOTATION}`]: myLocation, + }, }); return response.items; }, [catalogApi, entity]); @@ -82,13 +98,25 @@ export const UnregisterEntityDialog = ({ Are you sure you want to unregister this entity? + {loading ? : null} + {error ? ( - {error.toString()} + {error instanceof DeniedLocationException ? ( + <> + You cannot unregister this entity, since it originates from a + protected Backstage configuration (location + {`"${error.locationName}"`}). If you believe this is in error, + please contact your Backstage operator. + + ) : ( + error.toString() + )} ) : null} + {entities?.length ? ( <> @@ -96,9 +124,10 @@ export const UnregisterEntityDialog = ({
    - {entities.map(e => ( -
  • {e.metadata.name}
    • - ))} + {entities.map(e => { + const fullName = formatEntityRefTitle(e); + return
  • {fullName}
    • ; + })}
@@ -107,16 +136,21 @@ export const UnregisterEntityDialog = ({
  • - {entities[0]?.metadata.annotations?.[LOCATION_ANNOTATION]} + { + entities[0]?.metadata.annotations?.[ + ORIGIN_LOCATION_ANNOTATION + ] + }
+ + To undo, just re-register the entity in Backstage. + ) : null} - - To undo, just re-register the entity in Backstage. - +