From 43a4ef6442ee2eb8d05de5b2e88a78e0c862079c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Fri, 2 Jul 2021 16:18:42 +0200 Subject: [PATCH] More helpful error message when trying to import by folder from non-github MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .changeset/angry-hounds-bow.md | 5 ++ .changeset/eighty-deers-jam.md | 5 ++ .github/styles/vocab.txt | 1 + packages/integration/src/helpers.ts | 8 ++- .../src/api/CatalogImportClient.test.ts | 68 ++++++++----------- .../src/api/CatalogImportClient.ts | 6 ++ 6 files changed, 52 insertions(+), 41 deletions(-) create mode 100644 .changeset/angry-hounds-bow.md create mode 100644 .changeset/eighty-deers-jam.md diff --git a/.changeset/angry-hounds-bow.md b/.changeset/angry-hounds-bow.md new file mode 100644 index 0000000000..4bbfffd4f1 --- /dev/null +++ b/.changeset/angry-hounds-bow.md @@ -0,0 +1,5 @@ +--- +'@backstage/integration': patch +--- + +Do not throw in `ScmIntegration` `byUrl` for invalid URLs diff --git a/.changeset/eighty-deers-jam.md b/.changeset/eighty-deers-jam.md new file mode 100644 index 0000000000..df8b84aff1 --- /dev/null +++ b/.changeset/eighty-deers-jam.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-import': patch +--- + +More helpful error message when trying to import by folder from non-github diff --git a/.github/styles/vocab.txt b/.github/styles/vocab.txt index 846e050c49..80488056d8 100644 --- a/.github/styles/vocab.txt +++ b/.github/styles/vocab.txt @@ -281,6 +281,7 @@ unregistration untracked upvote url +URLs utils validator validators diff --git a/packages/integration/src/helpers.ts b/packages/integration/src/helpers.ts index 20c282c5a3..b1e74035f1 100644 --- a/packages/integration/src/helpers.ts +++ b/packages/integration/src/helpers.ts @@ -44,8 +44,12 @@ export function basicIntegrations( return integrations; }, byUrl(url: string | URL): T | undefined { - const parsed = typeof url === 'string' ? new URL(url) : url; - return integrations.find(i => getHost(i) === parsed.hostname); + try { + const parsed = typeof url === 'string' ? new URL(url) : url; + return integrations.find(i => getHost(i) === parsed.hostname); + } catch { + return undefined; + } }, byHost(host: string): T | undefined { return integrations.find(i => getHost(i) === host); diff --git a/plugins/catalog-import/src/api/CatalogImportClient.test.ts b/plugins/catalog-import/src/api/CatalogImportClient.test.ts index 8ab8f75db1..e2c3f2dc47 100644 --- a/plugins/catalog-import/src/api/CatalogImportClient.test.ts +++ b/plugins/catalog-import/src/api/CatalogImportClient.test.ts @@ -46,29 +46,19 @@ jest.doMock('@octokit/rest', () => { return { Octokit }; }); -// Mock the value to control which integrations are activated -jest.mock('./GitHub', () => ({ - getGithubIntegrationConfig: jest.fn(), -})); - -import { - GitHubIntegrationConfig, - ScmIntegrations, -} from '@backstage/integration'; +import { ScmIntegrations } from '@backstage/integration'; import { catalogApiRef } from '@backstage/plugin-catalog-react'; import { msw } from '@backstage/test-utils'; import { Octokit } from '@octokit/rest'; import { rest } from 'msw'; import { setupServer } from 'msw/node'; import { CatalogImportClient } from './CatalogImportClient'; -import { getGithubIntegrationConfig } from './GitHub'; import { ConfigReader, UrlPatternDiscovery } from '@backstage/core-app-api'; import { OAuthApi } from '@backstage/core-plugin-api'; -const server = setupServer(); - describe('CatalogImportClient', () => { + const server = setupServer(); msw.setupDefaultHandlers(server); const mockBaseUrl = 'http://backstage:9191/api/catalog'; @@ -92,7 +82,19 @@ describe('CatalogImportClient', () => { }, }; - const scmIntegrationsApi = ScmIntegrations.fromConfig(new ConfigReader({})); + const scmIntegrationsApi = ScmIntegrations.fromConfig( + new ConfigReader({ + integrations: { + github: [{ host: 'example.com' }], + gitlab: [ + { + host: 'registered-but-not-github.com', + apiBaseUrl: 'https://registered-but-not-github.com', + }, + ], + }, + }), + ); const catalogApi: jest.Mocked = { getEntities: jest.fn(), @@ -106,7 +108,6 @@ describe('CatalogImportClient', () => { }; let catalogImportClient: CatalogImportClient; - let getGithubIntegrationConfigFn: jest.Mock; beforeEach(() => { catalogImportClient = new CatalogImportClient({ @@ -116,9 +117,6 @@ describe('CatalogImportClient', () => { identityApi, catalogApi, }); - - getGithubIntegrationConfigFn = getGithubIntegrationConfig as jest.Mock; - getGithubIntegrationConfigFn.mockReset(); }); describe('analyzeUrl', () => { @@ -169,10 +167,22 @@ describe('CatalogImportClient', () => { }); }); - it('should ignore missing github integration', async () => { + it('should reject for integrations that are not github ones', async () => { await expect( catalogImportClient.analyzeUrl( - 'https://github.com/backstage/backstage', + 'https://registered-but-not-github.com/backstage/backstage', + ), + ).rejects.toThrow( + new Error( + 'The registered-but-not-github.com integration only supports full URLs to catalog-info.yaml files. Did you try to pass in the URL of a directory instead?', + ), + ); + }); + + it('should reject when unable to match with any integration', async () => { + await expect( + catalogImportClient.analyzeUrl( + 'https://not-registered-as-integration.com/foo/bar', ), ).rejects.toThrow( new Error( @@ -182,12 +192,6 @@ describe('CatalogImportClient', () => { }); it('should find locations from github', async () => { - getGithubIntegrationConfigFn.mockReturnValue({ - repo: 'backstage', - owner: 'backstage', - githubIntegrationConfig: {} as GitHubIntegrationConfig, - }); - ((new Octokit().search.code as any) as jest.Mock).mockResolvedValueOnce({ data: { total_count: 2, @@ -244,12 +248,6 @@ describe('CatalogImportClient', () => { }); it('should find repository from github', async () => { - getGithubIntegrationConfigFn.mockReturnValue({ - repo: 'backstage', - owner: 'backstage', - githubIntegrationConfig: {} as GitHubIntegrationConfig, - }); - ((new Octokit().search.code as any) as jest.Mock).mockResolvedValueOnce({ data: { total_count: 0, items: [] }, }); @@ -300,14 +298,6 @@ describe('CatalogImportClient', () => { describe('submitPullRequest', () => { it('should create GitHub pull request', async () => { - getGithubIntegrationConfigFn.mockReturnValue({ - repo: 'backstage', - owner: 'backstage', - githubIntegrationConfig: { - host: 'github.com', - } as GitHubIntegrationConfig, - }); - await expect( catalogImportClient.submitPullRequest({ repositoryUrl: 'https://github.com/backstage/backstage', diff --git a/plugins/catalog-import/src/api/CatalogImportClient.ts b/plugins/catalog-import/src/api/CatalogImportClient.ts index f53570617e..aa29c95247 100644 --- a/plugins/catalog-import/src/api/CatalogImportClient.ts +++ b/plugins/catalog-import/src/api/CatalogImportClient.ts @@ -77,6 +77,12 @@ export class CatalogImportClient implements CatalogImportApi { const ghConfig = getGithubIntegrationConfig(this.scmIntegrationsApi, url); if (!ghConfig) { + const other = this.scmIntegrationsApi.byUrl(url); + if (other) { + throw new Error( + `The ${other.title} integration only supports full URLs to catalog-info.yaml files. Did you try to pass in the URL of a directory instead?`, + ); + } throw new Error( 'This URL was not recognized as a valid GitHub URL because there was no configured integration that matched the given host name. You could try to paste the full URL to a catalog-info.yaml file instead.', );