From dbb952787885875a724c3c190b9a20753b2b4678 Mon Sep 17 00:00:00 2001 From: Dominik Henneke Date: Fri, 3 Sep 2021 10:41:10 +0200 Subject: [PATCH 01/14] Use `ScmIntegrationRegistry#resolveUrl` in the placeholder processors instead of a custom implementation Signed-off-by: Dominik Henneke --- .changeset/perfect-elephants-do.md | 21 +++++++++ .../processors/PlaceholderProcessor.test.ts | 16 +++++++ .../processors/PlaceholderProcessor.ts | 45 +++++++++++-------- .../src/next/NextCatalogBuilder.ts | 6 ++- .../src/service/CatalogBuilder.ts | 6 ++- 5 files changed, 73 insertions(+), 21 deletions(-) create mode 100644 .changeset/perfect-elephants-do.md diff --git a/.changeset/perfect-elephants-do.md b/.changeset/perfect-elephants-do.md new file mode 100644 index 0000000000..d5e4c864d7 --- /dev/null +++ b/.changeset/perfect-elephants-do.md @@ -0,0 +1,21 @@ +--- +'@backstage/plugin-catalog-backend': patch +--- + +Use `ScmIntegrationRegistry#resolveUrl` in the placeholder processors instead of a custom implementation. + +If you manually instantiate the `PlaceholderProcessor` (you most probably don't), add the new required constructor parameter: + +```diff ++ import { ScmIntegrations } from '@backstage/integration'; + // ... ++ const integrations = ScmIntegrations.fromConfig(config); + // ... + new PlaceholderProcessor({ + resolvers: placeholderResolvers, + reader, ++ integrations, + }); +``` + +All custom `PlaceholderResolver` can use the new `resolveUrl` parameter to resolve relative URLs. diff --git a/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.test.ts b/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.test.ts index 0254e854e5..3abf051e4e 100644 --- a/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.test.ts +++ b/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.test.ts @@ -15,6 +15,8 @@ */ import { UrlReader } from '@backstage/backend-common'; import { Entity } from '@backstage/catalog-model'; +import { ConfigReader } from '@backstage/config'; +import { ScmIntegrations } from '@backstage/integration'; import { jsonPlaceholderResolver, PlaceholderProcessor, @@ -25,6 +27,8 @@ import { yamlPlaceholderResolver, } from './PlaceholderProcessor'; +const integrations = ScmIntegrations.fromConfig(new ConfigReader({})); + describe('PlaceholderProcessor', () => { const read: jest.MockedFunction = jest.fn(); const reader: UrlReader = { read, readTree: jest.fn(), search: jest.fn() }; @@ -44,6 +48,7 @@ describe('PlaceholderProcessor', () => { foo: async () => 'replaced', }, reader, + integrations, }); await expect( processor.preProcessEntity(input, { type: 't', target: 'l' }), @@ -59,6 +64,7 @@ describe('PlaceholderProcessor', () => { upper: upperResolver, }, reader, + integrations, }); await expect( @@ -95,6 +101,7 @@ describe('PlaceholderProcessor', () => { bar: jest.fn(), }, reader, + integrations, }); const entity: Entity = { apiVersion: 'a', @@ -115,6 +122,7 @@ describe('PlaceholderProcessor', () => { bar: jest.fn(), }, reader, + integrations, }); const entity: Entity = { apiVersion: 'a', @@ -134,6 +142,7 @@ describe('PlaceholderProcessor', () => { const processor = new PlaceholderProcessor({ resolvers: { text: textPlaceholderResolver }, reader, + integrations, }); await expect( @@ -169,6 +178,7 @@ describe('PlaceholderProcessor', () => { const processor = new PlaceholderProcessor({ resolvers: { json: jsonPlaceholderResolver }, reader, + integrations, }); await expect( @@ -202,6 +212,7 @@ describe('PlaceholderProcessor', () => { const processor = new PlaceholderProcessor({ resolvers: { yaml: yamlPlaceholderResolver }, reader, + integrations, }); await expect( @@ -235,6 +246,7 @@ describe('PlaceholderProcessor', () => { const processor = new PlaceholderProcessor({ resolvers: { text: textPlaceholderResolver }, reader, + integrations, }); await expect( @@ -272,6 +284,7 @@ describe('PlaceholderProcessor', () => { const processor = new PlaceholderProcessor({ resolvers: { text: textPlaceholderResolver }, reader, + integrations, }); await expect( @@ -311,6 +324,7 @@ describe('PlaceholderProcessor', () => { const processor = new PlaceholderProcessor({ resolvers: { text: textPlaceholderResolver }, reader, + integrations, }); await expect( @@ -345,6 +359,7 @@ describe('yamlPlaceholderResolver', () => { value: './file.yaml', baseUrl: 'https://github.com/backstage/backstage/a/b/catalog-info.yaml', read, + resolveUrl: (url, base) => integrations.resolveUrl({ url, base }), }; beforeEach(() => { @@ -389,6 +404,7 @@ describe('jsonPlaceholderResolver', () => { value: './file.json', baseUrl: 'https://github.com/backstage/backstage/a/b/catalog-info.yaml', read, + resolveUrl: (url, base) => integrations.resolveUrl({ url, base }), }; beforeEach(() => { diff --git a/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.ts b/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.ts index 1b015d7d93..4456405857 100644 --- a/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.ts +++ b/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.ts @@ -17,16 +17,19 @@ import { UrlReader } from '@backstage/backend-common'; import { Entity, LocationSpec } from '@backstage/catalog-model'; import { JsonValue } from '@backstage/config'; +import { ScmIntegrationRegistry } from '@backstage/integration'; import yaml from 'yaml'; import { CatalogProcessor } from './types'; export type ResolverRead = (url: string) => Promise; +export type ResolverResolveUrl = (url: string, base: string) => string; export type ResolverParams = { key: string; value: JsonValue; baseUrl: string; read: ResolverRead; + resolveUrl: ResolverResolveUrl; }; export type PlaceholderResolver = ( @@ -36,6 +39,7 @@ export type PlaceholderResolver = ( type Options = { resolvers: Record; reader: UrlReader; + integrations: ScmIntegrationRegistry; }; /** @@ -103,12 +107,19 @@ export class PlaceholderProcessor implements CatalogProcessor { return this.options.reader.read(url); }; + const resolveUrl = (url: string, base: string): string => + this.options.integrations.resolveUrl({ + url, + base, + }); + return [ await resolver({ key: resolverKey, value: resolverValue, baseUrl: location.target, read, + resolveUrl, }), true, ]; @@ -191,31 +202,27 @@ async function readTextLocation(params: ResolverParams): Promise { } } -function relativeUrl({ key, value, baseUrl }: ResolverParams): string { +function relativeUrl({ + key, + value, + baseUrl, + resolveUrl, +}: ResolverParams): string { if (typeof value !== 'string') { throw new Error( `Placeholder \$${key} expected a string value parameter, in the form of an absolute URL or a relative path`, ); } - let url: URL; try { - // The two-value form of the URL constructor handles relative paths for us - url = new URL(value, baseUrl); - } catch { - try { - // Check whether value is a valid absolute URL on it's own, if not fail. - url = new URL(value); - } catch { - // The only remaining case that isn't support is a relative file path that should be - // resolved using a relative file location. Accessing local file paths can lead to - // path traversal attacks and access to any file on the host system. Implementing this - // would require additional security measures. - throw new Error( - `Placeholder \$${key} could not form a URL out of ${baseUrl} and ${value}`, - ); - } + return resolveUrl(value, baseUrl); + } catch (_) { + // The only remaining case that isn't support is a relative file path that should be + // resolved using a relative file location. Accessing local file paths can lead to + // path traversal attacks and access to any file on the host system. Implementing this + // would require additional security measures. + throw new Error( + `Placeholder \$${key} could not form a URL out of ${baseUrl} and ${value}`, + ); } - - return url.toString(); } diff --git a/plugins/catalog-backend/src/next/NextCatalogBuilder.ts b/plugins/catalog-backend/src/next/NextCatalogBuilder.ts index ed690d07b6..262df7525e 100644 --- a/plugins/catalog-backend/src/next/NextCatalogBuilder.ts +++ b/plugins/catalog-backend/src/next/NextCatalogBuilder.ts @@ -385,7 +385,11 @@ export class NextCatalogBuilder { // These are always there no matter what const processors: CatalogProcessor[] = [ - new PlaceholderProcessor({ resolvers: placeholderResolvers, reader }), + new PlaceholderProcessor({ + resolvers: placeholderResolvers, + reader, + integrations, + }), new BuiltinKindsEntityProcessor(), ]; diff --git a/plugins/catalog-backend/src/service/CatalogBuilder.ts b/plugins/catalog-backend/src/service/CatalogBuilder.ts index 70c1890fbf..e32e071ebc 100644 --- a/plugins/catalog-backend/src/service/CatalogBuilder.ts +++ b/plugins/catalog-backend/src/service/CatalogBuilder.ts @@ -306,7 +306,11 @@ export class CatalogBuilder { // These are always there no matter what const processors: CatalogProcessor[] = [ StaticLocationProcessor.fromConfig(config), - new PlaceholderProcessor({ resolvers: placeholderResolvers, reader }), + new PlaceholderProcessor({ + resolvers: placeholderResolvers, + reader, + integrations, + }), new BuiltinKindsEntityProcessor(), ]; From 49c3f2bbbbeacc798881141321f8e55fbe332286 Mon Sep 17 00:00:00 2001 From: hiba-aldalaty Date: Fri, 3 Sep 2021 09:57:03 +0100 Subject: [PATCH 02/14] Disable Create button in template after initial click Signed-off-by: hiba-aldalaty --- .changeset/sixty-carrots-marry.md | 5 +++++ .../MultistepJsonForm/MultistepJsonForm.tsx | 12 +++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) create mode 100644 .changeset/sixty-carrots-marry.md diff --git a/.changeset/sixty-carrots-marry.md b/.changeset/sixty-carrots-marry.md new file mode 100644 index 0000000000..414c41c645 --- /dev/null +++ b/.changeset/sixty-carrots-marry.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder': patch +--- + +Disable 'Create' button in template after initial click to avoid confusion when there is slowness in template execution diff --git a/plugins/scaffolder/src/components/MultistepJsonForm/MultistepJsonForm.tsx b/plugins/scaffolder/src/components/MultistepJsonForm/MultistepJsonForm.tsx index cdcc22da29..ef7e74f2f1 100644 --- a/plugins/scaffolder/src/components/MultistepJsonForm/MultistepJsonForm.tsx +++ b/plugins/scaffolder/src/components/MultistepJsonForm/MultistepJsonForm.tsx @@ -112,6 +112,7 @@ export const MultistepJsonForm = ({ widgets, }: Props) => { const [activeStep, setActiveStep] = useState(0); + const [disableCreateButton, setDisableCreateButton] = useState(false); const handleReset = () => { setActiveStep(0); @@ -121,6 +122,10 @@ export const MultistepJsonForm = ({ setActiveStep(Math.min(activeStep + 1, steps.length)); }; const handleBack = () => setActiveStep(Math.max(activeStep - 1, 0)); + const handleCreate = () => { + setDisableCreateButton(true); + onFinish(); + }; return ( <> @@ -174,7 +179,12 @@ export const MultistepJsonForm = ({ - From 0cb28c22e30eb6bce66520149a5b5fb86d98ad92 Mon Sep 17 00:00:00 2001 From: blam Date: Tue, 7 Sep 2021 11:55:14 +0200 Subject: [PATCH 03/14] feat: when reading from AzureDevOps strip the root directory as that is how the ZipArchive is returned Signed-off-by: blam --- .../writing-custom-field-extensions.md | 34 ++++++++++++++++ .../src/reading/AzureUrlReader.ts | 1 + .../reading/tree/ReadTreeResponseFactory.ts | 1 + packages/backend-common/src/reading/types.ts | 3 ++ .../src/test-integrations.ts | 40 +++++++++++++++++++ 5 files changed, 79 insertions(+) create mode 100644 docs/features/software-templates/writing-custom-field-extensions.md create mode 100644 plugins/scaffolder-backend/src/test-integrations.ts diff --git a/docs/features/software-templates/writing-custom-field-extensions.md b/docs/features/software-templates/writing-custom-field-extensions.md new file mode 100644 index 0000000000..e32aa4e78d --- /dev/null +++ b/docs/features/software-templates/writing-custom-field-extensions.md @@ -0,0 +1,34 @@ +--- +id: writing-custom-field-extensions +title: Writing Custom Field Extensions +description: How to write your own field extensions +--- + +Collecting input from the user is a very large part of the scaffolding process +and Software Templates as a whole. Sometimes the built in components and fields +just aren't good enough, and sometimes you want to enrich the form that the +users sees with better inputs that fit better. + +This is where `Custom Field Extensions` come in. + +With them you can show your own `React` Components and use them to control the +state of the JSON schema, as well as provide your own validation functions to +validate the data too. + +## Creating a Field Extension + +Field extensions are a way to combine an ID, a `React` Component and a +`validation` function together in a modular way that you can then use to pass to +the `Scaffolder` frontend plugin in your own `App.tsx`. + +You can create your own Field Extension by using the +`createScaffolderFieldExtension` `API` like below: + +```tsx +//packages/app/scaffolder/MyCustomExtension/MyCustomExtension.tsx +export const MyCustomExtension = () => {}; +``` + +```tsx +// packages/app/scaffolder/MyCustomExtension/validation.ts +``` diff --git a/packages/backend-common/src/reading/AzureUrlReader.ts b/packages/backend-common/src/reading/AzureUrlReader.ts index 910e8e04a5..5369306702 100644 --- a/packages/backend-common/src/reading/AzureUrlReader.ts +++ b/packages/backend-common/src/reading/AzureUrlReader.ts @@ -133,6 +133,7 @@ export class AzureUrlReader implements UrlReader { stream: archiveAzureResponse.body as unknown as Readable, etag: commitSha, filter: options?.filter, + stripFirstDirectory: true, }); } diff --git a/packages/backend-common/src/reading/tree/ReadTreeResponseFactory.ts b/packages/backend-common/src/reading/tree/ReadTreeResponseFactory.ts index 5fdd633102..70d8f63af8 100644 --- a/packages/backend-common/src/reading/tree/ReadTreeResponseFactory.ts +++ b/packages/backend-common/src/reading/tree/ReadTreeResponseFactory.ts @@ -55,6 +55,7 @@ export class DefaultReadTreeResponseFactory implements ReadTreeResponseFactory { this.workDir, options.etag, options.filter, + options.stripFirstDirectory, ); } } diff --git a/packages/backend-common/src/reading/types.ts b/packages/backend-common/src/reading/types.ts index b7e1ff823f..2d9c305b85 100644 --- a/packages/backend-common/src/reading/types.ts +++ b/packages/backend-common/src/reading/types.ts @@ -182,6 +182,9 @@ export type ReadTreeResponseFactoryOptions = { etag: string; // Filter passed on from the ReadTreeOptions filter?: (path: string, info?: { size: number }) => boolean; + + // Strip the first directory in the readTree response + stripFirstDirectory?: boolean; }; /** @public */ diff --git a/plugins/scaffolder-backend/src/test-integrations.ts b/plugins/scaffolder-backend/src/test-integrations.ts new file mode 100644 index 0000000000..65d84768aa --- /dev/null +++ b/plugins/scaffolder-backend/src/test-integrations.ts @@ -0,0 +1,40 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { + loadBackendConfig, + getRootLogger, + UrlReaders, +} from '@backstage/backend-common'; +// import { ScmIntegrations } from '@backstage/integration'; + +const run = async () => { + const root = getRootLogger(); + const config = await loadBackendConfig({ + argv: process.argv, + logger: root, + }); + + // const integrations = ScmIntegrations.fromConfig(config); + const reader = UrlReaders.default({ logger: root, config }); + + console.log( + await reader.readTree( + 'https://dev.azure.com/backstage-verification/test-template-fetch/_git/test-template-fetch', + ), + ); +}; + +run(); From e86adc689210c7cabdc73d90306797e71d2d680d Mon Sep 17 00:00:00 2001 From: blam Date: Tue, 7 Sep 2021 12:00:33 +0200 Subject: [PATCH 04/14] chore: remove unwanted files Signed-off-by: blam --- .../writing-custom-field-extensions.md | 34 ---------------- .../src/test-integrations.ts | 40 ------------------- 2 files changed, 74 deletions(-) delete mode 100644 docs/features/software-templates/writing-custom-field-extensions.md delete mode 100644 plugins/scaffolder-backend/src/test-integrations.ts diff --git a/docs/features/software-templates/writing-custom-field-extensions.md b/docs/features/software-templates/writing-custom-field-extensions.md deleted file mode 100644 index e32aa4e78d..0000000000 --- a/docs/features/software-templates/writing-custom-field-extensions.md +++ /dev/null @@ -1,34 +0,0 @@ ---- -id: writing-custom-field-extensions -title: Writing Custom Field Extensions -description: How to write your own field extensions ---- - -Collecting input from the user is a very large part of the scaffolding process -and Software Templates as a whole. Sometimes the built in components and fields -just aren't good enough, and sometimes you want to enrich the form that the -users sees with better inputs that fit better. - -This is where `Custom Field Extensions` come in. - -With them you can show your own `React` Components and use them to control the -state of the JSON schema, as well as provide your own validation functions to -validate the data too. - -## Creating a Field Extension - -Field extensions are a way to combine an ID, a `React` Component and a -`validation` function together in a modular way that you can then use to pass to -the `Scaffolder` frontend plugin in your own `App.tsx`. - -You can create your own Field Extension by using the -`createScaffolderFieldExtension` `API` like below: - -```tsx -//packages/app/scaffolder/MyCustomExtension/MyCustomExtension.tsx -export const MyCustomExtension = () => {}; -``` - -```tsx -// packages/app/scaffolder/MyCustomExtension/validation.ts -``` diff --git a/plugins/scaffolder-backend/src/test-integrations.ts b/plugins/scaffolder-backend/src/test-integrations.ts deleted file mode 100644 index 65d84768aa..0000000000 --- a/plugins/scaffolder-backend/src/test-integrations.ts +++ /dev/null @@ -1,40 +0,0 @@ -/* - * Copyright 2021 The Backstage Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -import { - loadBackendConfig, - getRootLogger, - UrlReaders, -} from '@backstage/backend-common'; -// import { ScmIntegrations } from '@backstage/integration'; - -const run = async () => { - const root = getRootLogger(); - const config = await loadBackendConfig({ - argv: process.argv, - logger: root, - }); - - // const integrations = ScmIntegrations.fromConfig(config); - const reader = UrlReaders.default({ logger: root, config }); - - console.log( - await reader.readTree( - 'https://dev.azure.com/backstage-verification/test-template-fetch/_git/test-template-fetch', - ), - ); -}; - -run(); From 6ad8fe1a0adf22eb7b64c82322518b61d6aacb0b Mon Sep 17 00:00:00 2001 From: Oliver Sand Date: Tue, 7 Sep 2021 15:08:31 +0200 Subject: [PATCH 05/14] Make ownership card style customizable via custom `theme.getPageTheme()` Signed-off-by: Oliver Sand --- .changeset/lucky-lies-count.md | 5 +++++ .../Cards/OwnershipCard/OwnershipCard.tsx | 14 ++------------ 2 files changed, 7 insertions(+), 12 deletions(-) create mode 100644 .changeset/lucky-lies-count.md diff --git a/.changeset/lucky-lies-count.md b/.changeset/lucky-lies-count.md new file mode 100644 index 0000000000..79e7c09391 --- /dev/null +++ b/.changeset/lucky-lies-count.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-org': patch +--- + +Make ownership card style customizable via custom `theme.getPageTheme()`. diff --git a/plugins/org/src/components/Cards/OwnershipCard/OwnershipCard.tsx b/plugins/org/src/components/Cards/OwnershipCard/OwnershipCard.tsx index da7f8381e3..cc0a379ff9 100644 --- a/plugins/org/src/components/Cards/OwnershipCard/OwnershipCard.tsx +++ b/plugins/org/src/components/Cards/OwnershipCard/OwnershipCard.tsx @@ -29,7 +29,7 @@ import { isOwnerOf, useEntity, } from '@backstage/plugin-catalog-react'; -import { BackstageTheme, genPageTheme } from '@backstage/theme'; +import { BackstageTheme } from '@backstage/theme'; import { Box, createStyles, @@ -49,16 +49,6 @@ type EntityTypeProps = { count: number; }; -const createPageTheme = ( - theme: BackstageTheme, - shapeKey: string, - colorsKey: string, -) => { - const { colors } = theme.getPageTheme({ themeId: colorsKey }); - const { shape } = theme.getPageTheme({ themeId: shapeKey }); - return genPageTheme(colors, shape).backgroundImage; -}; - const useStyles = makeStyles((theme: BackstageTheme) => createStyles({ card: { @@ -77,7 +67,7 @@ const useStyles = makeStyles((theme: BackstageTheme) => }, entityTypeBox: { background: (props: { type: string }) => - createPageTheme(theme, props.type, props.type), + theme.getPageTheme({ themeId: props.type }).backgroundImage, }, }), ); From 70718686f1a6336072bbd43c1a60cdf7fdf2cad2 Mon Sep 17 00:00:00 2001 From: Oliver Sand Date: Tue, 7 Sep 2021 16:17:52 +0200 Subject: [PATCH 06/14] Use correct `Link` in ownership card to avoid a full reload of the app while navigating Signed-off-by: Oliver Sand --- .changeset/sour-bees-pretend.md | 5 +++++ .../src/components/Cards/OwnershipCard/OwnershipCard.tsx | 7 +++---- 2 files changed, 8 insertions(+), 4 deletions(-) create mode 100644 .changeset/sour-bees-pretend.md diff --git a/.changeset/sour-bees-pretend.md b/.changeset/sour-bees-pretend.md new file mode 100644 index 0000000000..748266132c --- /dev/null +++ b/.changeset/sour-bees-pretend.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-org': patch +--- + +Use correct `Link` in ownership card to avoid a full reload of the app while navigating. diff --git a/plugins/org/src/components/Cards/OwnershipCard/OwnershipCard.tsx b/plugins/org/src/components/Cards/OwnershipCard/OwnershipCard.tsx index cc0a379ff9..96e7d08c31 100644 --- a/plugins/org/src/components/Cards/OwnershipCard/OwnershipCard.tsx +++ b/plugins/org/src/components/Cards/OwnershipCard/OwnershipCard.tsx @@ -18,6 +18,7 @@ import { Entity } from '@backstage/catalog-model'; import { InfoCard, InfoCardVariants, + Link, Progress, ResponseErrorPanel, } from '@backstage/core-components'; @@ -34,13 +35,11 @@ import { Box, createStyles, Grid, - Link, makeStyles, Typography, } from '@material-ui/core'; import qs from 'qs'; import React from 'react'; -import { generatePath } from 'react-router'; import { useAsync } from 'react-use'; type EntityTypeProps = { @@ -86,7 +85,7 @@ const EntityCountTile = ({ const classes = useStyles({ type }); return ( - + ))} From 93917b72c1558680593e320f186063df13c419a4 Mon Sep 17 00:00:00 2001 From: Dominik Henneke Date: Wed, 8 Sep 2021 09:58:57 +0200 Subject: [PATCH 07/14] Forward the error message Signed-off-by: Dominik Henneke --- .../src/ingestion/processors/PlaceholderProcessor.test.ts | 2 +- .../src/ingestion/processors/PlaceholderProcessor.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.test.ts b/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.test.ts index 3abf051e4e..c60b01d53d 100644 --- a/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.test.ts +++ b/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.test.ts @@ -345,7 +345,7 @@ describe('PlaceholderProcessor', () => { }, ), ).rejects.toThrow( - 'Placeholder $text could not form a URL out of ./a/b/catalog-info.yaml and ../c/catalog-info.yaml', + 'Placeholder $text could not form a URL out of ./a/b/catalog-info.yaml and ../c/catalog-info.yaml, TypeError: Invalid base URL: ./a/b/catalog-info.yaml', ); expect(read).not.toBeCalled(); diff --git a/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.ts b/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.ts index 4456405857..611e166605 100644 --- a/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.ts +++ b/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.ts @@ -216,13 +216,13 @@ function relativeUrl({ try { return resolveUrl(value, baseUrl); - } catch (_) { + } catch (e) { // The only remaining case that isn't support is a relative file path that should be // resolved using a relative file location. Accessing local file paths can lead to // path traversal attacks and access to any file on the host system. Implementing this // would require additional security measures. throw new Error( - `Placeholder \$${key} could not form a URL out of ${baseUrl} and ${value}`, + `Placeholder \$${key} could not form a URL out of ${baseUrl} and ${value}, ${e}`, ); } } From a316cd838da728b051deafc018dd2f444159114c Mon Sep 17 00:00:00 2001 From: blam Date: Wed, 8 Sep 2021 10:59:40 +0200 Subject: [PATCH 08/14] chore: reverting stripping as we don't need it Signed-off-by: blam --- packages/backend-common/src/reading/AzureUrlReader.ts | 6 +++--- packages/backend-common/src/reading/tree/util.ts | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/backend-common/src/reading/AzureUrlReader.ts b/packages/backend-common/src/reading/AzureUrlReader.ts index 5369306702..2269393cbf 100644 --- a/packages/backend-common/src/reading/AzureUrlReader.ts +++ b/packages/backend-common/src/reading/AzureUrlReader.ts @@ -94,8 +94,6 @@ export class AzureUrlReader implements UrlReader { url: string, options?: ReadTreeOptions, ): Promise { - // TODO: Support filepath based reading tree feature like other providers - // Get latest commit SHA const commitsAzureResponse = await fetch( @@ -129,11 +127,13 @@ export class AzureUrlReader implements UrlReader { throw new Error(message); } + const { filepath } = parseGitUrl(url); + return await this.deps.treeResponseFactory.fromZipArchive({ stream: archiveAzureResponse.body as unknown as Readable, etag: commitSha, filter: options?.filter, - stripFirstDirectory: true, + subpath: filepath, }); } diff --git a/packages/backend-common/src/reading/tree/util.ts b/packages/backend-common/src/reading/tree/util.ts index cfb986a0b0..5ddc5a3766 100644 --- a/packages/backend-common/src/reading/tree/util.ts +++ b/packages/backend-common/src/reading/tree/util.ts @@ -21,5 +21,6 @@ const directoryNameRegex = /^[^\/]+\//; // Removes the first segment of a forward-slash-separated path export function stripFirstDirectoryFromPath(path: string): string { + console.log('stripFirstDirectoryPath path:', path); return path.replace(directoryNameRegex, ''); } From e7b596fdae5922ca08d1104638bd4f769cd25ce1 Mon Sep 17 00:00:00 2001 From: blam Date: Wed, 8 Sep 2021 11:00:59 +0200 Subject: [PATCH 09/14] chore: removing stripFirstDirectory it's not needed Signed-off-by: blam --- .../backend-common/src/reading/tree/ReadTreeResponseFactory.ts | 1 - packages/backend-common/src/reading/types.ts | 3 --- 2 files changed, 4 deletions(-) diff --git a/packages/backend-common/src/reading/tree/ReadTreeResponseFactory.ts b/packages/backend-common/src/reading/tree/ReadTreeResponseFactory.ts index 70d8f63af8..5fdd633102 100644 --- a/packages/backend-common/src/reading/tree/ReadTreeResponseFactory.ts +++ b/packages/backend-common/src/reading/tree/ReadTreeResponseFactory.ts @@ -55,7 +55,6 @@ export class DefaultReadTreeResponseFactory implements ReadTreeResponseFactory { this.workDir, options.etag, options.filter, - options.stripFirstDirectory, ); } } diff --git a/packages/backend-common/src/reading/types.ts b/packages/backend-common/src/reading/types.ts index 2d9c305b85..b7e1ff823f 100644 --- a/packages/backend-common/src/reading/types.ts +++ b/packages/backend-common/src/reading/types.ts @@ -182,9 +182,6 @@ export type ReadTreeResponseFactoryOptions = { etag: string; // Filter passed on from the ReadTreeOptions filter?: (path: string, info?: { size: number }) => boolean; - - // Strip the first directory in the readTree response - stripFirstDirectory?: boolean; }; /** @public */ From bb7ce1c64c16623f11d047431b915a5538422a3e Mon Sep 17 00:00:00 2001 From: blam Date: Wed, 8 Sep 2021 11:06:24 +0200 Subject: [PATCH 10/14] chore: added test for subpath azure Signed-off-by: blam --- .../src/reading/AzureUrlReader.test.ts | 15 +++++++++++++++ packages/backend-common/src/reading/tree/util.ts | 1 - 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/packages/backend-common/src/reading/AzureUrlReader.test.ts b/packages/backend-common/src/reading/AzureUrlReader.test.ts index c7c8853929..fc1f6a03f3 100644 --- a/packages/backend-common/src/reading/AzureUrlReader.test.ts +++ b/packages/backend-common/src/reading/AzureUrlReader.test.ts @@ -220,6 +220,21 @@ describe('AzureUrlReader', () => { expect(indexMarkdownFile.toString()).toBe('# Test\n'); }); + it('returns the wanted files from an archive when a subpath is passed through', async () => { + const response = await processor.readTree( + 'https://dev.azure.com/organization/project/_git/repository?path=%2Fdocs', + ); + + expect(response.etag).toBe('123abc2'); + + const files = await response.files(); + + expect(files.length).toBe(1); + const indexMarkdownFile = await files[0].content(); + + expect(indexMarkdownFile.toString()).toBe('# Test\n'); + }); + it('creates a directory with the wanted files', async () => { const response = await processor.readTree( 'https://dev.azure.com/organization/project/_git/repository', diff --git a/packages/backend-common/src/reading/tree/util.ts b/packages/backend-common/src/reading/tree/util.ts index 5ddc5a3766..cfb986a0b0 100644 --- a/packages/backend-common/src/reading/tree/util.ts +++ b/packages/backend-common/src/reading/tree/util.ts @@ -21,6 +21,5 @@ const directoryNameRegex = /^[^\/]+\//; // Removes the first segment of a forward-slash-separated path export function stripFirstDirectoryFromPath(path: string): string { - console.log('stripFirstDirectoryPath path:', path); return path.replace(directoryNameRegex, ''); } From fdaa61a87b2d25f565476db13c7aa2c79779ed7f Mon Sep 17 00:00:00 2001 From: blam Date: Wed, 8 Sep 2021 11:08:01 +0200 Subject: [PATCH 11/14] chore: added changeset Signed-off-by: blam --- .changeset/early-seahorses-stare.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/early-seahorses-stare.md diff --git a/.changeset/early-seahorses-stare.md b/.changeset/early-seahorses-stare.md new file mode 100644 index 0000000000..963058a26f --- /dev/null +++ b/.changeset/early-seahorses-stare.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-common': patch +--- + +Fixing issue with `AzureUrlReader` that doesn't do `subpath` directories correctly From 8e6e95fc434fa5b0544774a7f5233e19da71162b Mon Sep 17 00:00:00 2001 From: blam Date: Wed, 8 Sep 2021 11:17:13 +0200 Subject: [PATCH 12/14] chore: remove the `stripFirstDirectory` option to the `ZipArchiveResponse` Signed-off-by: blam --- .../reading/tree/ZipArchiveResponse.test.ts | 30 ------------------- .../src/reading/tree/ZipArchiveResponse.ts | 20 ++----------- 2 files changed, 3 insertions(+), 47 deletions(-) diff --git a/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts b/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts index f6f9a7845a..a54bac0c4a 100644 --- a/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts +++ b/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts @@ -62,36 +62,6 @@ describe('ZipArchiveResponse', () => { ]); }); - it('should read files and strip root dir if requested', async () => { - const stream = fs.createReadStream('/test-archive-with-extra-root-dir.zip'); - - const res = new ZipArchiveResponse( - stream, - '', - '/tmp', - 'etag', - undefined, - true, - ); - const files = await res.files(); - - expect(files).toEqual([ - { - path: 'mkdocs.yml', - content: expect.any(Function), - }, - { - 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([ - 'site_name: Test', - '# Test', - ]); - }); - it('should read files with filter', async () => { const stream = fs.createReadStream('/test-archive.zip'); diff --git a/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts b/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts index fb22f24b94..e07cedde87 100644 --- a/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts +++ b/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts @@ -24,7 +24,6 @@ import { ReadTreeResponseDirOptions, ReadTreeResponseFile, } from '../types'; -import { stripFirstDirectoryFromPath } from './util'; /** * Wraps a zip archive stream into a tree response reader. @@ -38,7 +37,6 @@ export class ZipArchiveResponse implements ReadTreeResponse { private readonly workDir: string, public readonly etag: string, private readonly filter?: (path: string, info: { size: number }) => boolean, - private readonly stripFirstDirectory?: boolean, ) { if (subPath) { if (!subPath.endsWith('/')) { @@ -68,12 +66,8 @@ export class ZipArchiveResponse implements ReadTreeResponse { } private shouldBeIncluded(entry: Entry): boolean { - const strippedPath = this.stripFirstDirectory - ? stripFirstDirectoryFromPath(entry.path) - : entry.path; - if (this.subPath) { - if (!strippedPath.startsWith(this.subPath)) { + if (!entry.path.startsWith(this.subPath)) { return false; } } @@ -102,11 +96,7 @@ export class ZipArchiveResponse implements ReadTreeResponse { if (this.shouldBeIncluded(entry)) { files.push({ - path: this.getInnerPath( - this.stripFirstDirectory - ? stripFirstDirectoryFromPath(entry.path) - : entry.path, - ), + path: this.getInnerPath(entry.path), content: () => entry.buffer(), }); } else { @@ -154,11 +144,7 @@ export class ZipArchiveResponse implements ReadTreeResponse { // Ignore directory entries since we handle that with the file entries // as a zip can have files with directories without directory entries if (entry.type === 'File' && this.shouldBeIncluded(entry)) { - const entryPath = this.getInnerPath( - this.stripFirstDirectory - ? stripFirstDirectoryFromPath(entry.path) - : entry.path, - ); + const entryPath = this.getInnerPath(entry.path); const dirname = platformPath.dirname(entryPath); if (dirname) { await fs.mkdirp(platformPath.join(dir, dirname)); From c299e90a26276eacc90f5120c8826a502a021021 Mon Sep 17 00:00:00 2001 From: hiba-aldalaty Date: Wed, 8 Sep 2021 10:43:09 +0100 Subject: [PATCH 13/14] Disable all buttons in the final step in template when 'Create' button is clicked in template. Signed-off-by: hiba-aldalaty --- .changeset/lovely-windows-return.md | 5 +++++ .../MultistepJsonForm/MultistepJsonForm.tsx | 14 +++++++++----- 2 files changed, 14 insertions(+), 5 deletions(-) create mode 100644 .changeset/lovely-windows-return.md diff --git a/.changeset/lovely-windows-return.md b/.changeset/lovely-windows-return.md new file mode 100644 index 0000000000..55755af563 --- /dev/null +++ b/.changeset/lovely-windows-return.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder': patch +--- + +Disable all buttons in the final step when 'Create' button is clicked in template. diff --git a/plugins/scaffolder/src/components/MultistepJsonForm/MultistepJsonForm.tsx b/plugins/scaffolder/src/components/MultistepJsonForm/MultistepJsonForm.tsx index ef7e74f2f1..62c2881ab0 100644 --- a/plugins/scaffolder/src/components/MultistepJsonForm/MultistepJsonForm.tsx +++ b/plugins/scaffolder/src/components/MultistepJsonForm/MultistepJsonForm.tsx @@ -112,7 +112,7 @@ export const MultistepJsonForm = ({ widgets, }: Props) => { const [activeStep, setActiveStep] = useState(0); - const [disableCreateButton, setDisableCreateButton] = useState(false); + const [disableButtons, setDisableButtons] = useState(false); const handleReset = () => { setActiveStep(0); @@ -123,7 +123,7 @@ export const MultistepJsonForm = ({ }; const handleBack = () => setActiveStep(Math.max(activeStep - 1, 0)); const handleCreate = () => { - setDisableCreateButton(true); + setDisableButtons(true); onFinish(); }; @@ -177,13 +177,17 @@ export const MultistepJsonForm = ({ metadata={getReviewData(formData, steps)} /> - - + + From d16283a6c330c5c2240ca5b060b3edc41de15c0a Mon Sep 17 00:00:00 2001 From: hiba-aldalaty Date: Wed, 8 Sep 2021 14:21:17 +0100 Subject: [PATCH 14/14] Remove unnecessary extra changeset file Signed-off-by: hiba-aldalaty --- .changeset/sixty-carrots-marry.md | 5 ----- 1 file changed, 5 deletions(-) delete mode 100644 .changeset/sixty-carrots-marry.md diff --git a/.changeset/sixty-carrots-marry.md b/.changeset/sixty-carrots-marry.md deleted file mode 100644 index 414c41c645..0000000000 --- a/.changeset/sixty-carrots-marry.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@backstage/plugin-scaffolder': patch ---- - -Disable 'Create' button in template after initial click to avoid confusion when there is slowness in template execution