From 0f101a87d9b5982790aa5f6ead9ef898dc75b278 Mon Sep 17 00:00:00 2001 From: Mustansar Anwar ul Samad Date: Wed, 23 Jun 2021 10:57:15 +1200 Subject: [PATCH] Fix RepoUrlPicker to use integration api directly Signed-off-by: Mustansar Anwar ul Samad --- app-config.yaml | 5 +++ .../bitbucket-demo/template.yaml | 2 -- .../actions/builtin/publish/bitbucket.ts | 5 ++- .../actions/builtin/publish/github.ts | 8 ++++- .../actions/builtin/publish/gitlab.ts | 8 ++++- .../actions/builtin/publish/util.ts | 24 +++++++++---- .../src/scaffolder/tasks/TaskWorker.test.ts | 29 +++++++++++++-- .../src/scaffolder/tasks/TaskWorker.ts | 3 +- .../scaffolder-backend/src/service/router.ts | 1 + .../fields/RepoUrlPicker/RepoUrlPicker.tsx | 36 +++++++------------ .../fields/RepoUrlPicker/validation.ts | 2 +- 11 files changed, 85 insertions(+), 38 deletions(-) diff --git a/app-config.yaml b/app-config.yaml index f2df97b7b2..c1d0121dff 100644 --- a/app-config.yaml +++ b/app-config.yaml @@ -152,6 +152,11 @@ integrations: - host: bitbucket.org username: ${BITBUCKET_USERNAME} appPassword: ${BITBUCKET_APP_PASSWORD} + ### Example for how to add your bitbucket server instance using the API: + # - host: server.bitbucket.com + # apiBaseUrl: server.bitbucket.com + # username: ${BITBUCKET_SERVER_USERNAME} + # appPassword: ${BITBUCKET_SERVER_APP_PASSWORD} azure: - host: dev.azure.com token: ${AZURE_TOKEN} diff --git a/plugins/scaffolder-backend/sample-templates/bitbucket-demo/template.yaml b/plugins/scaffolder-backend/sample-templates/bitbucket-demo/template.yaml index fda1324e42..87f4d449f5 100644 --- a/plugins/scaffolder-backend/sample-templates/bitbucket-demo/template.yaml +++ b/plugins/scaffolder-backend/sample-templates/bitbucket-demo/template.yaml @@ -19,7 +19,6 @@ spec: ui:field: RepoUrlPicker ui:options: allowedHosts: - - github.com - bitbucket.org - server.bitbucket.com - title: Fill in some steps @@ -64,7 +63,6 @@ spec: name: Publish action: publish:bitbucket input: - allowedHosts: ['bitbucket.org'] description: 'This is {{ parameters.name }}' repoUrl: '{{ parameters.repoUrl }}' diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/bitbucket.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/bitbucket.ts index 092390d08e..1291684719 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/bitbucket.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/bitbucket.ts @@ -260,7 +260,10 @@ export function createPublishBitbucketAction(options: { enableLFS = false, } = ctx.input; - const { workspace, project, repo, host } = parseRepoUrl(repoUrl); + const { workspace, project, repo, host } = parseRepoUrl( + repoUrl, + integrations, + ); // Workspace is only required for bitbucket cloud if (host === 'bitbucket.org') { diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.ts index 640ae5dfe5..f767d6d38c 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.ts @@ -144,7 +144,13 @@ export function createPublishGithubAction(options: { topics, } = ctx.input; - const { owner, repo, host } = parseRepoUrl(repoUrl); + const { owner, repo, host } = parseRepoUrl(repoUrl, integrations); + + if (!owner) { + throw new InputError( + `No owner provided for host: ${host}, and repo ${repo}`, + ); + } const credentialsProvider = credentialsProviders.get(host); const integrationConfig = integrations.github.byHost(host); diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/gitlab.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/gitlab.ts index fbd2dcaaae..eead310777 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/gitlab.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/gitlab.ts @@ -84,7 +84,13 @@ export function createPublishGitlabAction(options: { defaultBranch = 'master', } = ctx.input; - const { owner, repo, host } = parseRepoUrl(repoUrl); + const { owner, repo, host } = parseRepoUrl(repoUrl, integrations); + + if (!owner) { + throw new InputError( + `No owner provided for host: ${host}, and repo ${repo}`, + ); + } const integrationConfig = integrations.gitlab.byHost(host); diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/util.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/util.ts index c86c4d8f38..0cc0830fd7 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/util.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/util.ts @@ -16,6 +16,7 @@ import { InputError } from '@backstage/errors'; import { join as joinPath, normalize as normalizePath } from 'path'; +import { ScmIntegrationRegistry } from '@backstage/integration'; export const getRepoSourceDirectory = ( workspacePath: string, @@ -33,11 +34,16 @@ export const getRepoSourceDirectory = ( export type RepoSpec = { repo: string; host: string; - owner: string; + owner?: string; organization?: string; + workspace?: string; + project?: string; }; -export const parseRepoUrl = (repoUrl: string): RepoSpec => { +export const parseRepoUrl = ( + repoUrl: string, + integrations: ScmIntegrationRegistry, +): RepoSpec => { let parsed; try { parsed = new URL(`https://${repoUrl}`); @@ -47,10 +53,16 @@ export const parseRepoUrl = (repoUrl: string): RepoSpec => { ); } const host = parsed.host; - const type = parsed.searchParams.get('type'); - const owner = parsed.searchParams.get('owner'); - const workspace = parsed.searchParams.get('workspace'); - const project = parsed.searchParams.get('project'); + const owner = parsed.searchParams.get('owner') ?? undefined; + const organization = parsed.searchParams.get('organization') ?? undefined; + const workspace = parsed.searchParams.get('workspace') ?? undefined; + const project = parsed.searchParams.get('project') ?? undefined; + + const type = integrations.byHost(host)?.type; + + if (!type) { + throw new InputError(`Unable to find host ${host} in integrations`); + } if (type === 'bitbucket') { diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.test.ts index b5e8d19e62..6b18d0dac3 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.test.ts @@ -41,6 +41,14 @@ describe('TaskWorker', () => { let storage: DatabaseTaskStore; let actionRegistry = new TemplateActionRegistry(); + const integrations = ScmIntegrations.fromConfig( + new ConfigReader({ + integrations: { + github: [{ host: 'github.com', token: 'token' }], + }, + }), + ); + beforeAll(async () => { storage = await createStore(); }); @@ -65,6 +73,7 @@ describe('TaskWorker', () => { workingDirectory: os.tmpdir(), actionRegistry, taskBroker: broker, + integrations, }); const { taskId } = await broker.dispatch({ steps: [{ id: 'test', name: 'test', action: 'not-found-action' }], @@ -90,6 +99,7 @@ describe('TaskWorker', () => { workingDirectory: os.tmpdir(), actionRegistry, taskBroker: broker, + integrations, }); const { taskId } = await broker.dispatch({ @@ -142,6 +152,7 @@ describe('TaskWorker', () => { workingDirectory: os.tmpdir(), actionRegistry, taskBroker: broker, + integrations, }); const { taskId } = await broker.dispatch({ @@ -331,6 +342,7 @@ describe('TaskWorker', () => { workingDirectory: os.tmpdir(), actionRegistry, taskBroker: broker, + integrations, }); const { taskId } = await broker.dispatch({ @@ -388,6 +400,12 @@ describe('TaskWorker', () => { organization: { type: 'string', }, + workspace: { + type: 'string', + }, + project: { + type: 'string', + }, }, }, }, @@ -396,7 +414,10 @@ describe('TaskWorker', () => { async handler(ctx) { ctx.output('host', ctx.input.destination.host); ctx.output('repo', ctx.input.destination.repo); - ctx.output('owner', ctx.input.destination.owner); + + if (ctx.input.destination.owner) { + ctx.output('owner', ctx.input.destination.owner); + } if (ctx.input.destination.host !== 'github.com') { throw new Error( @@ -410,7 +431,10 @@ describe('TaskWorker', () => { ); } - if (ctx.input.destination.owner !== 'owner') { + if ( + ctx.input.destination.owner && + ctx.input.destination.owner !== 'owner' + ) { throw new Error( `expected repo to be "owner" got ${ctx.input.destination.owner}`, ); @@ -425,6 +449,7 @@ describe('TaskWorker', () => { workingDirectory: os.tmpdir(), actionRegistry, taskBroker: broker, + integrations, }); const { taskId } = await broker.dispatch({ diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts index b4ba936719..d34979599d 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts @@ -33,6 +33,7 @@ type Options = { taskBroker: TaskBroker; workingDirectory: string; actionRegistry: TemplateActionRegistry; + integrations: ScmIntegrations; }; export class TaskWorker { @@ -45,7 +46,7 @@ export class TaskWorker { // scary right now, so we're going to lock it off like the component API is // in the frontend until we can work out a nice way to do it. this.handlebars.registerHelper('parseRepoUrl', repoUrl => { - return JSON.stringify(parseRepoUrl(repoUrl)); + return JSON.stringify(parseRepoUrl(repoUrl, options.integrations)); }); this.handlebars.registerHelper('projectSlug', repoUrl => { diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index cee9d2cfb4..f2a9c5b708 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -90,6 +90,7 @@ export async function createRouter( taskBroker, actionRegistry, workingDirectory, + integrations, }); workers.push(worker); } diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx index 8d936ebb46..278f1c3ae8 100644 --- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx +++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx @@ -16,6 +16,7 @@ import React, { useCallback, useEffect } from 'react'; import { FieldProps } from '@rjsf/core'; import { scaffolderApiRef } from '../../../api'; +import { scmIntegrationsApiRef } from '@backstage/integration-react'; import { useAsync } from 'react-use'; import Select from '@material-ui/core/Select'; import InputLabel from '@material-ui/core/InputLabel'; @@ -28,7 +29,6 @@ import { Progress } from '@backstage/core-components'; function splitFormData(url: string | undefined) { let host = undefined; - let type = undefined; let owner = undefined; let repo = undefined; let organization = undefined; @@ -39,7 +39,6 @@ function splitFormData(url: string | undefined) { if (url) { const parsed = new URL(`https://${url}`); host = parsed.host; - type = parsed.searchParams.get('type') || undefined; owner = parsed.searchParams.get('owner') || undefined; repo = parsed.searchParams.get('repo') || undefined; // This is azure dev ops specific. not used for any other provider. @@ -52,12 +51,11 @@ function splitFormData(url: string | undefined) { /* ok */ } - return { host, type, owner, repo, organization, workspace, project }; + return { host, owner, repo, organization, workspace, project }; } function serializeFormData(data: { host?: string; - type?: string; owner?: string; repo?: string; organization?: string; @@ -69,9 +67,6 @@ function serializeFormData(data: { } const params = new URLSearchParams(); - if (data.type) { - params.set('type', data.type); - } if (data.owner) { params.set('owner', data.owner); } @@ -97,11 +92,12 @@ export const RepoUrlPicker = ({ rawErrors, formData, }: FieldProps) => { - const api = useApi(scaffolderApiRef); + const scaffolderApi = useApi(scaffolderApiRef); + const integrationApi = useApi(scmIntegrationsApiRef); const allowedHosts = uiSchema['ui:options']?.allowedHosts as string[]; const { value: integrations, loading } = useAsync(async () => { - return await api.getIntegrationsList({ allowedHosts }); + return await scaffolderApi.getIntegrationsList({ allowedHosts }); }); const { host, type, owner, repo, organization, workspace, project } = splitFormData(formData); @@ -119,7 +115,7 @@ export const RepoUrlPicker = ({ }), ) }, - [onChange, integrations, owner, repo, organization, workspace, project], + [onChange, owner, repo, organization, workspace, project], ); const updateOwner = useCallback( @@ -127,7 +123,6 @@ export const RepoUrlPicker = ({ onChange( serializeFormData({ host, - type, owner: evt.target.value as string, repo, organization, @@ -135,7 +130,7 @@ export const RepoUrlPicker = ({ project, }), ), - [onChange, host, type, repo, organization, workspace, project], + [onChange, host, repo, organization, workspace, project], ); const updateRepo = useCallback( @@ -143,7 +138,6 @@ export const RepoUrlPicker = ({ onChange( serializeFormData({ host, - type, owner, repo: evt.target.value as string, organization, @@ -151,7 +145,7 @@ export const RepoUrlPicker = ({ project, }), ), - [onChange, host, type, owner, organization, workspace, project], + [onChange, host, owner, organization, workspace, project], ); const updateOrganization = useCallback( @@ -159,7 +153,6 @@ export const RepoUrlPicker = ({ onChange( serializeFormData({ host, - type, owner, repo, organization: evt.target.value as string, @@ -175,7 +168,6 @@ export const RepoUrlPicker = ({ onChange( serializeFormData({ host, - type, owner, repo, organization, @@ -183,7 +175,7 @@ export const RepoUrlPicker = ({ project, }), ), - [onChange, host, type, owner, repo, organization, project], + [onChange, host, owner, repo, organization, project], ); const updateProject = useCallback( @@ -191,7 +183,6 @@ export const RepoUrlPicker = ({ onChange( serializeFormData({ host, - type, owner, repo, organization, @@ -199,7 +190,7 @@ export const RepoUrlPicker = ({ project: evt.target.value as string, }), ), - [onChange, host, type, owner, repo, organization, workspace], + [onChange, host, owner, repo, organization, workspace], ); useEffect(() => { @@ -207,7 +198,6 @@ export const RepoUrlPicker = ({ onChange( serializeFormData({ host: integrations[0].host, - type: integrations[0].type, owner, repo, organization, @@ -263,9 +253,9 @@ export const RepoUrlPicker = ({ The name of the organization )} - {/* Show this for bitbucket.org only */} - {type === 'bitbucket' && ( + {host && integrationApi.byHost(host)?.type === 'bitbucket' && ( <> + {/* Show this for bitbucket.org only */} {host === 'bitbucket.org' && ( )} {/* Show this for all hosts except bitbucket */} - {type !== 'bitbucket' && ( + {host && integrationApi.byHost(host)?.type !== 'bitbucket' && ( <> { try { const { host, searchParams } = new URL(`https://${value}`); - if (!host || !searchParams.get('owner') || !searchParams.get('repo')) { + if (!host || !searchParams.get('repo')) { validation.addError('Incomplete repository location provided'); } } catch {