From adc0414cf79ddfa3338d0872db8397e7b466f537 Mon Sep 17 00:00:00 2001 From: npiyush97 Date: Mon, 12 Feb 2024 20:44:58 +0530 Subject: [PATCH 1/6] preetier Signed-off-by: npiyush97 --- .changeset/spotty-carrots-refuse.md | 5 +++++ .../StepPrepareCreatePullRequest.tsx | 14 ++++++++++++++ 2 files changed, 19 insertions(+) create mode 100644 .changeset/spotty-carrots-refuse.md diff --git a/.changeset/spotty-carrots-refuse.md b/.changeset/spotty-carrots-refuse.md new file mode 100644 index 0000000000..b615c7a302 --- /dev/null +++ b/.changeset/spotty-carrots-refuse.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-import': patch +--- + +fix related to component name not adhering to kubernetes validobjectname. diff --git a/plugins/catalog-import/src/components/StepPrepareCreatePullRequest/StepPrepareCreatePullRequest.tsx b/plugins/catalog-import/src/components/StepPrepareCreatePullRequest/StepPrepareCreatePullRequest.tsx index 1cbdfce355..1614488922 100644 --- a/plugins/catalog-import/src/components/StepPrepareCreatePullRequest/StepPrepareCreatePullRequest.tsx +++ b/plugins/catalog-import/src/components/StepPrepareCreatePullRequest/StepPrepareCreatePullRequest.tsx @@ -143,11 +143,25 @@ export const StepPrepareCreatePullRequest = ( .sort(); }); + const isValidObjectName = (value: unknown): boolean => { + return ( + typeof value === 'string' && + value.length >= 1 && + value.length <= 63 && + /^([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]$/.test(value) + ); + }; + const handleResult = useCallback( async (data: FormData) => { setSubmitted(true); try { + if (!isValidObjectName(data.componentName)) { + throw new Error( + 'Component name: Must start and end with an alphanumeric character, and contain only alphanumeric characters, hyphens, underscores, and periods. Maximum length is 63 characters.', + ); + } const pr = await catalogImportApi.submitPullRequest({ repositoryUrl: analyzeResult.url, title: data.title, From 71e0812f613398fd8d40d66b0f1979d295a02f31 Mon Sep 17 00:00:00 2001 From: npiyush97 Date: Tue, 13 Feb 2024 19:55:33 +0530 Subject: [PATCH 2/6] fix related to catalog import accepting invalid name Signed-off-by: npiyush97 --- .changeset/spotty-carrots-refuse.md | 2 +- .../src/api/CatalogImportClient.ts | 23 +++++++++++++++++-- .../StepPrepareCreatePullRequest.tsx | 14 ----------- 3 files changed, 22 insertions(+), 17 deletions(-) diff --git a/.changeset/spotty-carrots-refuse.md b/.changeset/spotty-carrots-refuse.md index b615c7a302..47dcee7239 100644 --- a/.changeset/spotty-carrots-refuse.md +++ b/.changeset/spotty-carrots-refuse.md @@ -2,4 +2,4 @@ '@backstage/plugin-catalog-import': patch --- -fix related to component name not adhering to kubernetes validobjectname. +fix related to component name not adhering to kubernetes valid object name. diff --git a/plugins/catalog-import/src/api/CatalogImportClient.ts b/plugins/catalog-import/src/api/CatalogImportClient.ts index 7b582c90fa..52582bfe4d 100644 --- a/plugins/catalog-import/src/api/CatalogImportClient.ts +++ b/plugins/catalog-import/src/api/CatalogImportClient.ts @@ -28,6 +28,7 @@ import { ScmAuthApi } from '@backstage/integration-react'; import { Octokit } from '@octokit/rest'; import { Base64 } from 'js-base64'; import { AnalyzeResult, CatalogImportApi } from './CatalogImportApi'; +import YAML from 'yaml'; import { getGithubIntegrationConfig } from './GitHub'; import { getBranchName, getCatalogFilename } from '../components/helpers'; import { AnalyzeLocationResponse } from '@backstage/plugin-catalog-common'; @@ -175,12 +176,21 @@ the component will become available.\n\nFor more information, read an \ body: string; }): Promise<{ link: string; location: string }> { const { repositoryUrl, fileContent, title, body } = options; - + const parseData = YAML.parse(fileContent); + const { + metadata: { name }, + } = parseData; + if (!isValidComponentName(name)) { + throw new Error( + 'Component name: Must start and end with an alphanumeric character, and contain only alphanumeric characters, hyphens, underscores, and periods. Maximum length is 63 characters.', + ); + } const ghConfig = getGithubIntegrationConfig( this.scmIntegrationsApi, repositoryUrl, ); - + // eslint-disable-next-line no-console + console.log({ ghConfig }); if (ghConfig) { return await this.submitGitHubPrToRepo({ ...ghConfig, @@ -352,3 +362,12 @@ function formatHttpErrorMessage( ) { return `${message}, received http response status code ${error.status}: ${error.message}`; } + +function isValidComponentName(componentName: string) { + return ( + typeof componentName === 'string' && + componentName.length >= 1 && + componentName.length <= 63 && + /^([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]$/.test(componentName) + ); +} diff --git a/plugins/catalog-import/src/components/StepPrepareCreatePullRequest/StepPrepareCreatePullRequest.tsx b/plugins/catalog-import/src/components/StepPrepareCreatePullRequest/StepPrepareCreatePullRequest.tsx index 1614488922..1cbdfce355 100644 --- a/plugins/catalog-import/src/components/StepPrepareCreatePullRequest/StepPrepareCreatePullRequest.tsx +++ b/plugins/catalog-import/src/components/StepPrepareCreatePullRequest/StepPrepareCreatePullRequest.tsx @@ -143,25 +143,11 @@ export const StepPrepareCreatePullRequest = ( .sort(); }); - const isValidObjectName = (value: unknown): boolean => { - return ( - typeof value === 'string' && - value.length >= 1 && - value.length <= 63 && - /^([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]$/.test(value) - ); - }; - const handleResult = useCallback( async (data: FormData) => { setSubmitted(true); try { - if (!isValidObjectName(data.componentName)) { - throw new Error( - 'Component name: Must start and end with an alphanumeric character, and contain only alphanumeric characters, hyphens, underscores, and periods. Maximum length is 63 characters.', - ); - } const pr = await catalogImportApi.submitPullRequest({ repositoryUrl: analyzeResult.url, title: data.title, From ba8d96f6cf4437fb3cc81499deec0755270d7633 Mon Sep 17 00:00:00 2001 From: npiyush97 Date: Tue, 13 Feb 2024 20:10:54 +0530 Subject: [PATCH 3/6] cleanup Signed-off-by: npiyush97 --- plugins/catalog-import/src/api/CatalogImportClient.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/plugins/catalog-import/src/api/CatalogImportClient.ts b/plugins/catalog-import/src/api/CatalogImportClient.ts index 52582bfe4d..faddd615d4 100644 --- a/plugins/catalog-import/src/api/CatalogImportClient.ts +++ b/plugins/catalog-import/src/api/CatalogImportClient.ts @@ -189,8 +189,7 @@ the component will become available.\n\nFor more information, read an \ this.scmIntegrationsApi, repositoryUrl, ); - // eslint-disable-next-line no-console - console.log({ ghConfig }); + if (ghConfig) { return await this.submitGitHubPrToRepo({ ...ghConfig, From 8882dda58ef0295e13be60431738b871eded243f Mon Sep 17 00:00:00 2001 From: npiyush97 Date: Tue, 13 Feb 2024 20:52:58 +0530 Subject: [PATCH 4/6] added related tests Signed-off-by: npiyush97 --- .../src/api/CatalogImportClient.test.ts | 68 +++++++++++++++++-- 1 file changed, 64 insertions(+), 4 deletions(-) diff --git a/plugins/catalog-import/src/api/CatalogImportClient.test.ts b/plugins/catalog-import/src/api/CatalogImportClient.test.ts index 646a3b7bf9..3c277bdf2a 100644 --- a/plugins/catalog-import/src/api/CatalogImportClient.test.ts +++ b/plugins/catalog-import/src/api/CatalogImportClient.test.ts @@ -559,7 +559,23 @@ describe('CatalogImportClient', () => { await expect( catalogImportClient.submitPullRequest({ repositoryUrl: 'https://github.com/backstage/backstage', - fileContent: 'some content 🤖', + fileContent: ` + { + "apiVersion": "backstage.io/v1alpha1", + "kind": "Component", + "metadata": { + "name": "valid-name", + "annotations": { + "github.com/project-slug": "backstage/example-repo" + } + }, + "spec": { + "type": "other", + "lifecycle": "unknown", + "owner": "backstage" + } + } + `, title: 'A title/message', body: 'A body', }), @@ -585,7 +601,8 @@ describe('CatalogImportClient', () => { repo: 'backstage', path: 'catalog-info.yaml', message: 'A title/message', - content: 'c29tZSBjb250ZW50IPCfpJY=', + content: + 'CiAgICAgICAgICAgIHsKICAgICAgICAgICAgICAgICJhcGlWZXJzaW9uIjogImJhY2tzdGFnZS5pby92MWFscGhhMSIsCiAgICAgICAgICAgICAgICAia2luZCI6ICJDb21wb25lbnQiLAogICAgICAgICAgICAgICAgIm1ldGFkYXRhIjogewogICAgICAgICAgICAgICAgICAibmFtZSI6ICJ2YWxpZC1uYW1lIiwKICAgICAgICAgICAgICAgICAgImFubm90YXRpb25zIjogewogICAgICAgICAgICAgICAgICAgICAgImdpdGh1Yi5jb20vcHJvamVjdC1zbHVnIjogImJhY2tzdGFnZS9leGFtcGxlLXJlcG8iCiAgICAgICAgICAgICAgICB9CiAgICAgICAgICAgICAgfSwKICAgICAgICAgICAgICAic3BlYyI6IHsKICAgICAgICAgICAgICAgICAgInR5cGUiOiAib3RoZXIiLAogICAgICAgICAgICAgICAgICAibGlmZWN5Y2xlIjogInVua25vd24iLAogICAgICAgICAgICAgICAgICAib3duZXIiOiAiYmFja3N0YWdlIgogICAgICAgICAgICAgIH0KICAgICAgICAgICAgfQogICAgICAgICAg', branch: 'backstage-integration', }); expect( @@ -599,7 +616,34 @@ describe('CatalogImportClient', () => { base: 'main', }); }); - + it('Submit Pull Request with invalid component name', async () => { + await expect( + catalogImportClient.submitPullRequest({ + repositoryUrl: 'https://github.com/acme-corp/our-awesome-api', + fileContent: ` + { + "apiVersion": "backstage.io/v1alpha1", + "kind": "Component", + "metadata": { + "name": "invalid name", + "annotations": { + "github.com/project-slug": "backstage/example-repo" + } + }, + "spec": { + "type": "other", + "lifecycle": "unknown", + "owner": "backstage" + } + } + `, + title: 'A title/message', + body: 'A body', + }), + ).rejects.toThrow( + 'Component name: Must start and end with an alphanumeric character, and contain only alphanumeric characters, hyphens, underscores, and periods. Maximum length is 63 characters.', + ); + }); it('should create GitHub pull request with custom filename and branch name', async () => { const entityFilename = 'anvil.yaml'; const pullRequestBranchName = 'anvil-integration'; @@ -623,7 +667,23 @@ describe('CatalogImportClient', () => { await expect( catalogImportClient.submitPullRequest({ repositoryUrl: 'https://github.com/acme-corp/our-awesome-api', - fileContent: '', + fileContent: ` + { + "apiVersion": "backstage.io/v1alpha1", + "kind": "Component", + "metadata": { + "name": "valid-name", + "annotations": { + "github.com/project-slug": "backstage/example-repo" + } + }, + "spec": { + "type": "other", + "lifecycle": "unknown", + "owner": "backstage" + } + } + `, title: `Add ${entityFilename} config file`, body: `Add ${entityFilename} config file`, }), From 2b632c44337a8b39cf9dd8ecc64b04f800ea3c2a Mon Sep 17 00:00:00 2001 From: npiyush97 Date: Thu, 22 Feb 2024 17:17:19 +0530 Subject: [PATCH 5/6] using validate Entity to check catalog import instance Signed-off-by: npiyush97 --- .../src/api/CatalogImportClient.test.ts | 15 ++++-- .../src/api/CatalogImportClient.ts | 53 ++++++++----------- 2 files changed, 34 insertions(+), 34 deletions(-) diff --git a/plugins/catalog-import/src/api/CatalogImportClient.test.ts b/plugins/catalog-import/src/api/CatalogImportClient.test.ts index 3c277bdf2a..df44ec94c4 100644 --- a/plugins/catalog-import/src/api/CatalogImportClient.test.ts +++ b/plugins/catalog-import/src/api/CatalogImportClient.test.ts @@ -556,6 +556,9 @@ describe('CatalogImportClient', () => { describe('submitPullRequest', () => { it('should create GitHub pull request', async () => { + catalogApi.validateEntity.mockResolvedValueOnce({ + valid: true, + }); await expect( catalogImportClient.submitPullRequest({ repositoryUrl: 'https://github.com/backstage/backstage', @@ -584,7 +587,7 @@ describe('CatalogImportClient', () => { location: 'https://github.com/backstage/backstage/blob/main/catalog-info.yaml', }); - + expect(catalogApi.validateEntity).toHaveBeenCalledTimes(1); expect( (new Octokit().git.createRef as any as jest.Mock).mock.calls[0][0], ).toEqual({ @@ -617,6 +620,9 @@ describe('CatalogImportClient', () => { }); }); it('Submit Pull Request with invalid component name', async () => { + const ErrorMessage = + 'Policy check failed for component:default/invalid name; caused by Error: "metadata.name" is not valid; expected a string that is sequences of [a-zA-Z0-9] separated by any of [-_.], at most 63 characters in total but found "invalid name". To learn more about catalog file format, visit: https://github.com/backstage/backstage/blob/master/docs/architecture-decisions/adr002-default-catalog-file-format.md'; + catalogApi.validateEntity.mockRejectedValueOnce(new Error(ErrorMessage)); await expect( catalogImportClient.submitPullRequest({ repositoryUrl: 'https://github.com/acme-corp/our-awesome-api', @@ -640,11 +646,12 @@ describe('CatalogImportClient', () => { title: 'A title/message', body: 'A body', }), - ).rejects.toThrow( - 'Component name: Must start and end with an alphanumeric character, and contain only alphanumeric characters, hyphens, underscores, and periods. Maximum length is 63 characters.', - ); + ).rejects.toThrow(ErrorMessage); }); it('should create GitHub pull request with custom filename and branch name', async () => { + catalogApi.validateEntity.mockResolvedValueOnce({ + valid: true, + }); const entityFilename = 'anvil.yaml'; const pullRequestBranchName = 'anvil-integration'; diff --git a/plugins/catalog-import/src/api/CatalogImportClient.ts b/plugins/catalog-import/src/api/CatalogImportClient.ts index faddd615d4..b76454eb17 100644 --- a/plugins/catalog-import/src/api/CatalogImportClient.ts +++ b/plugins/catalog-import/src/api/CatalogImportClient.ts @@ -177,30 +177,32 @@ the component will become available.\n\nFor more information, read an \ }): Promise<{ link: string; location: string }> { const { repositoryUrl, fileContent, title, body } = options; const parseData = YAML.parse(fileContent); - const { - metadata: { name }, - } = parseData; - if (!isValidComponentName(name)) { - throw new Error( - 'Component name: Must start and end with an alphanumeric character, and contain only alphanumeric characters, hyphens, underscores, and periods. Maximum length is 63 characters.', + try { + const validationResponse = await this.catalogApi.validateEntity( + parseData, + `url:${repositoryUrl}`, ); - } - const ghConfig = getGithubIntegrationConfig( - this.scmIntegrationsApi, - repositoryUrl, - ); - - if (ghConfig) { - return await this.submitGitHubPrToRepo({ - ...ghConfig, + if (!validationResponse.valid) { + throw new Error(validationResponse.errors[0].message); + } + const ghConfig = getGithubIntegrationConfig( + this.scmIntegrationsApi, repositoryUrl, - fileContent, - title, - body, - }); - } + ); - throw new Error('unimplemented!'); + if (ghConfig) { + return await this.submitGitHubPrToRepo({ + ...ghConfig, + repositoryUrl, + fileContent, + title, + body, + }); + } + throw new Error('unimplemented!'); + } catch (err) { + throw err; + } } // TODO: this could be part of the catalog api @@ -361,12 +363,3 @@ function formatHttpErrorMessage( ) { return `${message}, received http response status code ${error.status}: ${error.message}`; } - -function isValidComponentName(componentName: string) { - return ( - typeof componentName === 'string' && - componentName.length >= 1 && - componentName.length <= 63 && - /^([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]$/.test(componentName) - ); -} From af927f1d47b421c5156cd477834cad620a80909a Mon Sep 17 00:00:00 2001 From: npiyush97 Date: Fri, 23 Feb 2024 11:06:18 +0530 Subject: [PATCH 6/6] removed unnecessary try catch and minor ui cleanup Signed-off-by: npiyush97 --- .../src/api/CatalogImportClient.ts | 45 +++++++++---------- .../StepFinishImportLocation.tsx | 1 - 2 files changed, 21 insertions(+), 25 deletions(-) diff --git a/plugins/catalog-import/src/api/CatalogImportClient.ts b/plugins/catalog-import/src/api/CatalogImportClient.ts index b76454eb17..cecb85f9a3 100644 --- a/plugins/catalog-import/src/api/CatalogImportClient.ts +++ b/plugins/catalog-import/src/api/CatalogImportClient.ts @@ -177,32 +177,29 @@ the component will become available.\n\nFor more information, read an \ }): Promise<{ link: string; location: string }> { const { repositoryUrl, fileContent, title, body } = options; const parseData = YAML.parse(fileContent); - try { - const validationResponse = await this.catalogApi.validateEntity( - parseData, - `url:${repositoryUrl}`, - ); - if (!validationResponse.valid) { - throw new Error(validationResponse.errors[0].message); - } - const ghConfig = getGithubIntegrationConfig( - this.scmIntegrationsApi, - repositoryUrl, - ); - if (ghConfig) { - return await this.submitGitHubPrToRepo({ - ...ghConfig, - repositoryUrl, - fileContent, - title, - body, - }); - } - throw new Error('unimplemented!'); - } catch (err) { - throw err; + const validationResponse = await this.catalogApi.validateEntity( + parseData, + `url:${repositoryUrl}`, + ); + if (!validationResponse.valid) { + throw new Error(validationResponse.errors[0].message); } + const ghConfig = getGithubIntegrationConfig( + this.scmIntegrationsApi, + repositoryUrl, + ); + + if (ghConfig) { + return await this.submitGitHubPrToRepo({ + ...ghConfig, + repositoryUrl, + fileContent, + title, + body, + }); + } + throw new Error('unimplemented!'); } // TODO: this could be part of the catalog api diff --git a/plugins/catalog-import/src/components/StepFinishImportLocation/StepFinishImportLocation.tsx b/plugins/catalog-import/src/components/StepFinishImportLocation/StepFinishImportLocation.tsx index 38372e59ba..31bff5c697 100644 --- a/plugins/catalog-import/src/components/StepFinishImportLocation/StepFinishImportLocation.tsx +++ b/plugins/catalog-import/src/components/StepFinishImportLocation/StepFinishImportLocation.tsx @@ -76,7 +76,6 @@ export const StepFinishImportLocation = ({ prepareResult, onReset }: Props) => { Register another - ; ); }