diff --git a/.changeset/proud-women-hammer.md b/.changeset/proud-women-hammer.md new file mode 100644 index 0000000000..030fc5bf7b --- /dev/null +++ b/.changeset/proud-women-hammer.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder-node': patch +--- + +trim leading and trailing slashes from parseRepoUrl query parameters diff --git a/plugins/scaffolder-node/package.json b/plugins/scaffolder-node/package.json index 011b09c91f..ba938c3843 100644 --- a/plugins/scaffolder-node/package.json +++ b/plugins/scaffolder-node/package.json @@ -66,6 +66,7 @@ "globby": "^11.0.0", "isomorphic-git": "^1.23.0", "jsonschema": "^1.5.0", + "lodash": "^4.17.21", "p-limit": "^3.1.0", "tar": "^6.1.12", "winston": "^3.2.1", @@ -76,6 +77,7 @@ "devDependencies": { "@backstage/backend-test-utils": "workspace:^", "@backstage/cli": "workspace:^", - "@backstage/config": "workspace:^" + "@backstage/config": "workspace:^", + "@types/lodash": "^4.14.151" } } diff --git a/plugins/scaffolder-node/src/actions/util.test.ts b/plugins/scaffolder-node/src/actions/util.test.ts new file mode 100644 index 0000000000..4b1e787bbc --- /dev/null +++ b/plugins/scaffolder-node/src/actions/util.test.ts @@ -0,0 +1,247 @@ +/* + * Copyright 2025 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 { ScmIntegrationRegistry } from '@backstage/integration'; +import { parseRepoUrl } from './util'; +import { mapValues } from 'lodash'; + +const queryString = ( + params: Partial< + Record<'owner' | 'organization' | 'workspace' | 'project' | 'repo', string> + > = {}, +): string => { + const pEntries = Object.entries(params); + if (pEntries.length) { + return `?${pEntries + .map(([k, v]) => `${k}=${encodeURIComponent(v)}`) + .join('&')}`; + } + return ''; +}; + +describe('scaffolder action utils', () => { + describe('parseRepoUrl', () => { + const byHost = jest.fn(); + const integrations = { + byHost, + } as unknown as ScmIntegrationRegistry; + + describe('rejects url when', () => { + it('empty', () => + expect(() => parseRepoUrl('', integrations)).toThrow( + /Invalid repo URL passed/, + )); + it('blank', () => + expect(() => parseRepoUrl(' ', integrations)).toThrow( + /Invalid repo URL passed/, + )); + }); + it('requires that host match an integration type', () => { + byHost.mockClear(); + expect(() => parseRepoUrl('foo', integrations)).toThrow( + /No matching integration configuration for host/, + ); + }); + describe('bitbucket', () => { + beforeEach(() => byHost.mockReturnValue({ type: 'bitbucket' })); + describe('cloud', () => { + const [host, workspace, project, repo] = [ + 'www.bitbucket.org', + 'foo', + 'bar', + 'baz', + ]; + it('requires workspace', () => + expect(() => + parseRepoUrl( + `${host}${queryString({ project, repo })}`, + integrations, + ), + ).toThrow(/missing workspace/)); + it('requires project', () => + expect(() => + parseRepoUrl( + `${host}${queryString({ workspace, repo })}`, + integrations, + ), + ).toThrow(/missing project/)); + it('requires repo', () => + expect(() => + parseRepoUrl( + `${host}${queryString({ workspace, project })}`, + integrations, + ), + ).toThrow(/missing repo/)); + it('happy path', () => + expect( + parseRepoUrl( + `${host}${queryString({ workspace, project, repo })}`, + integrations, + ), + ).toMatchObject({ + host, + workspace, + project, + repo, + })); + }); + describe('other', () => { + const [host, project, repo] = ['bitbucket.other', 'foo', 'bar']; + it('requires project', () => + expect(() => + parseRepoUrl(`${host}${queryString({ repo })}`, integrations), + ).toThrow(/missing project/)); + it('requires repo', () => + expect(() => + parseRepoUrl(`${host}${queryString({ project })}`, integrations), + ).toThrow(/missing repo/)); + it('happy path', () => + expect( + parseRepoUrl( + `${host}${queryString({ project, repo })}`, + integrations, + ), + ).toMatchObject({ + host, + project, + repo, + })); + }); + }); + describe('azure', () => { + beforeEach(() => byHost.mockReturnValue({ type: 'azure' })); + const [host, project, repo] = ['az.ure', 'foo', 'bar']; + it('requires project', () => + expect(() => + parseRepoUrl(`${host}${queryString({ repo })}`, integrations), + ).toThrow(/missing project/)); + it('requires repo', () => + expect(() => + parseRepoUrl(`${host}${queryString({ project })}`, integrations), + ).toThrow(/missing repo/)); + it('happy path', () => + expect( + parseRepoUrl( + `${host}${queryString({ project, repo })}`, + integrations, + ), + ).toMatchObject({ + host, + project, + repo, + })); + }); + describe('gitlab', () => { + beforeEach(() => byHost.mockReturnValue({ type: 'gitlab' })); + const [host, owner, repo, project] = ['gitl.ab', 'foo', 'bar', '123456']; + it('requires owner', () => + expect(() => + parseRepoUrl(`${host}${queryString({ repo })}`, integrations), + ).toThrow(/missing owner/)); + it('requires repo', () => + expect(() => + parseRepoUrl(`${host}${queryString({ owner })}`, integrations), + ).toThrow(/missing repo/)); + it('unless project specified', () => + expect( + parseRepoUrl(`${host}${queryString({ project })}`, integrations), + ).toMatchObject({ host, project })); + it('happy path', () => + expect( + parseRepoUrl(`${host}${queryString({ owner, repo })}`, integrations), + ).toMatchObject({ + host, + owner, + repo, + })); + }); + describe('gitea', () => { + beforeEach(() => byHost.mockReturnValue({ type: 'gitea' })); + const [host, repo] = ['git.ea', 'foo']; + it('requires repo', () => + expect(() => parseRepoUrl(host, integrations)).toThrow(/missing repo/)); + it('happy path', () => + expect( + parseRepoUrl(`${host}${queryString({ repo })}`, integrations), + ).toMatchObject({ host, repo })); + }); + describe('gerrit', () => { + beforeEach(() => byHost.mockReturnValue({ type: 'gerrit' })); + const [host, repo] = ['gerr.it', 'foo']; + it('requires repo', () => + expect(() => parseRepoUrl(host, integrations)).toThrow(/missing repo/)); + it('happy path', () => + expect( + parseRepoUrl(`${host}${queryString({ repo })}`, integrations), + ).toMatchObject({ + host, + repo, + })); + }); + describe('generic type', () => { + beforeEach(() => byHost.mockReturnValue({ type: 'generic' })); + const [host, owner, repo] = ['oth.er', 'foo', 'bar']; + it('requires owner', () => + expect(() => + parseRepoUrl(`${host}${queryString({ repo })}`, integrations), + ).toThrow(/missing owner/)); + it('requires repo', () => + expect(() => + parseRepoUrl(`${host}${queryString({ owner })}`, integrations), + ).toThrow(/missing repo/)); + it('happy path', () => + expect( + parseRepoUrl(`${host}${queryString({ owner, repo })}`, integrations), + ).toMatchObject({ + host, + owner, + repo, + })); + }); + describe('facilitates naive URL construction', () => { + beforeEach(() => byHost.mockReturnValue({ type: 'irrelevant' })); + it('decodes encoded params', () => { + const [host, owner, repo] = ['with_the_most', 'foo/bar/baz', 'blah']; + expect( + parseRepoUrl(`${host}${queryString({ owner, repo })}`, integrations), + ).toMatchObject({ host, owner, repo }); + }); + it('trims leading and trailing / from params', () => { + const [host, owner, organization, workspace, project, repo] = [ + 'anywhere', + 'anyone', + 'anything', + 'anyway', + 'anyhow', + 'any', + ]; + const junkedUp = mapValues( + { owner, organization, workspace, project, repo }, + v => `//${v}//`, + ); + return expect( + parseRepoUrl(`${host}${queryString(junkedUp)}`, integrations), + ).toMatchObject({ + host, + owner, + organization, + workspace, + project, + repo, + }); + }); + }); + }); +}); diff --git a/plugins/scaffolder-node/src/actions/util.ts b/plugins/scaffolder-node/src/actions/util.ts index f1775bb318..12f73b0c6c 100644 --- a/plugins/scaffolder-node/src/actions/util.ts +++ b/plugins/scaffolder-node/src/actions/util.ts @@ -22,6 +22,7 @@ import { TemplateActionOptions } from './createTemplateAction'; import zodToJsonSchema from 'zod-to-json-schema'; import { z } from 'zod'; import { Schema } from 'jsonschema'; +import { trim } from 'lodash'; /** * @public @@ -67,11 +68,6 @@ export const parseRepoUrl = ( ); } const host = parsed.host; - 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) { @@ -79,8 +75,14 @@ export const parseRepoUrl = ( `No matching integration configuration for host ${host}, please check your integrations config`, ); } - - const repo: string = parsed.searchParams.get('repo')!; + const { owner, organization, workspace, project, repo } = Object.fromEntries( + ['owner', 'organization', 'workspace', 'project', 'repo'].map(param => [ + param, + parsed.searchParams.has(param) + ? trim(parsed.searchParams.get(param)!, '/') + : undefined, + ]), + ); switch (type) { case 'bitbucket': { if (host === 'www.bitbucket.org') { @@ -113,8 +115,7 @@ export const parseRepoUrl = ( break; } } - - return { host, owner, repo, organization, workspace, project }; + return { host, owner, repo: repo!, organization, workspace, project }; }; function checkRequiredParams(repoUrl: URL, ...params: string[]) { diff --git a/yarn.lock b/yarn.lock index f700a3af06..1d55543152 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8020,11 +8020,13 @@ __metadata: "@backstage/plugin-scaffolder-common": "workspace:^" "@backstage/types": "workspace:^" "@isomorphic-git/pgp-plugin": "npm:^0.0.7" + "@types/lodash": "npm:^4.14.151" concat-stream: "npm:^2.0.0" fs-extra: "npm:^11.2.0" globby: "npm:^11.0.0" isomorphic-git: "npm:^1.23.0" jsonschema: "npm:^1.5.0" + lodash: "npm:^4.17.21" p-limit: "npm:^3.1.0" tar: "npm:^6.1.12" winston: "npm:^3.2.1"