From c52b3c80b1e12ab093799eda71363ef73da56769 Mon Sep 17 00:00:00 2001 From: MT Lewis Date: Wed, 6 Mar 2024 20:49:17 +0000 Subject: [PATCH 1/2] integration: add some failing tests for defaultScmResolveUrl Discovered some cases with SCM URL resolution that don't appear to work as intended. This commit adds some new failing unit tests to demonstrate the problem, as well as a few new cases that pass to round out the area of the suite. Signed-off-by: MT Lewis --- packages/integration/src/helpers.test.ts | 94 ++++++++++++++++++++++++ 1 file changed, 94 insertions(+) diff --git a/packages/integration/src/helpers.test.ts b/packages/integration/src/helpers.test.ts index 7468c83bc6..72ccd53061 100644 --- a/packages/integration/src/helpers.test.ts +++ b/packages/integration/src/helpers.test.ts @@ -116,6 +116,82 @@ describe('defaultScmResolveUrl', () => { ); }); + it('works for the root path', () => { + expect( + defaultScmResolveUrl({ + url: '/', + base: 'https://gitlab.com/groupA/teams/teamA/subgroupA/repoA/-/blob/branch/folder', + }), + ).toBe( + 'https://gitlab.com/groupA/teams/teamA/subgroupA/repoA/-/blob/branch/', + ); + + expect( + defaultScmResolveUrl({ + url: '/', + base: 'https://gitlab.com/groupA/teams/teamA/subgroupA/repoA/-/blob/branch/folder/', + }), + ).toBe( + 'https://gitlab.com/groupA/teams/teamA/subgroupA/repoA/-/blob/branch/', + ); + + expect( + defaultScmResolveUrl({ + url: '/', + base: 'https://gitlab.com/groupA/teams/teamA/subgroupA/repoA/-/blob/branch/folder/a.yaml', + }), + ).toBe( + 'https://gitlab.com/groupA/teams/teamA/subgroupA/repoA/-/blob/branch/', + ); + + expect( + defaultScmResolveUrl({ + url: '/', + base: 'https://gitlab.com/groupA/teams/teamA/subgroupA/repoA/-/blob/branch/folder/a.yaml?at=master', + }), + ).toBe( + 'https://gitlab.com/groupA/teams/teamA/subgroupA/repoA/-/blob/branch/?at=master', + ); + }); + + it('works for files in the repo root', () => { + expect( + defaultScmResolveUrl({ + url: '/b.yaml', + base: 'https://gitlab.com/groupA/teams/teamA/subgroupA/repoA/-/blob/branch/folder', + }), + ).toBe( + 'https://gitlab.com/groupA/teams/teamA/subgroupA/repoA/-/blob/branch/b.yaml', + ); + + expect( + defaultScmResolveUrl({ + url: '/b.yaml', + base: 'https://gitlab.com/groupA/teams/teamA/subgroupA/repoA/-/blob/branch/folder/', + }), + ).toBe( + 'https://gitlab.com/groupA/teams/teamA/subgroupA/repoA/-/blob/branch/b.yaml', + ); + + expect( + defaultScmResolveUrl({ + url: '/b.yaml', + base: 'https://gitlab.com/groupA/teams/teamA/subgroupA/repoA/-/blob/branch/folder/a.yaml', + }), + ).toBe( + 'https://gitlab.com/groupA/teams/teamA/subgroupA/repoA/-/blob/branch/b.yaml', + ); + + expect( + defaultScmResolveUrl({ + url: '/b.yaml', + base: 'https://gitlab.com/groupA/teams/teamA/subgroupA/repoA/-/blob/branch/folder/a.yaml?at=master', + }), + ).toBe( + 'https://gitlab.com/groupA/teams/teamA/subgroupA/repoA/-/blob/branch/b.yaml?at=master', + ); + }); + it('works for absolute paths and retains query params', () => { expect( defaultScmResolveUrl({ @@ -126,6 +202,24 @@ describe('defaultScmResolveUrl', () => { 'https://gitlab.com/groupA/teams/teamA/subgroupA/repoA/-/blob/branch/other/b.yaml', ); + expect( + defaultScmResolveUrl({ + url: '/other/b.yaml', + base: 'https://gitlab.com/groupA/teams/teamA/subgroupA/repoA/-/blob/branch/folder', + }), + ).toBe( + 'https://gitlab.com/groupA/teams/teamA/subgroupA/repoA/-/blob/branch/other/b.yaml', + ); + + expect( + defaultScmResolveUrl({ + url: '/other/b.yaml', + base: 'https://gitlab.com/groupA/teams/teamA/subgroupA/repoA/-/blob/branch/folder/', + }), + ).toBe( + 'https://gitlab.com/groupA/teams/teamA/subgroupA/repoA/-/blob/branch/other/b.yaml', + ); + expect( defaultScmResolveUrl({ url: '/other/b.yaml', From 0386fa7fc21d1abbcc588b343dd306bcfc521dd9 Mon Sep 17 00:00:00 2001 From: MT Lewis Date: Wed, 6 Mar 2024 20:51:16 +0000 Subject: [PATCH 2/2] integration: use href from git-url-parse when resolving file urls When parsing a URL with a trailing slash, git-url-parse drops the trailing slash from the href and filepath properties on the result, which is inconsistent with the behavior of the native URL constructor. Since before we were using the filepath property from git-url-parse to determine how much to truncate the pathname from the URL constructor, we were off-by-one for URLs with a trailing slash. After this change, we pass the href property from git-url-parse to the URL constructor, such that the amount to truncate the pathname and the pathname itself are ultimately coming from the same source. Signed-off-by: MT Lewis --- .changeset/chatty-vans-cross.md | 5 +++++ packages/integration/src/helpers.ts | 6 ++++-- 2 files changed, 9 insertions(+), 2 deletions(-) create mode 100644 .changeset/chatty-vans-cross.md diff --git a/.changeset/chatty-vans-cross.md b/.changeset/chatty-vans-cross.md new file mode 100644 index 0000000000..fab7af5155 --- /dev/null +++ b/.changeset/chatty-vans-cross.md @@ -0,0 +1,5 @@ +--- +'@backstage/integration': patch +--- + +Fixed an issue with resolution of SCM URLs against a base URL with a trailing slash. diff --git a/packages/integration/src/helpers.ts b/packages/integration/src/helpers.ts index 94aafaa6c3..b403155cc5 100644 --- a/packages/integration/src/helpers.ts +++ b/packages/integration/src/helpers.ts @@ -84,8 +84,10 @@ export function defaultScmResolveUrl(options: { if (url.startsWith('/')) { // If it is an absolute path, move relative to the repo root - const { filepath } = parseGitUrl(base); - updated = new URL(base); + const { href, filepath } = parseGitUrl(base); + + updated = new URL(href); + const repoRootPath = trimEnd( updated.pathname.substring(0, updated.pathname.length - filepath.length), '/',