Merge pull request #12774 from jamieklassen/gitlab-unscoped

support unscoped routes in location targets on gitlab

.changeset/rotten-moles-give.md

@@ -0,0 +1,7 @@
+---
+'@backstage/integration': patch
+---
+
+Fixed bug in getGitLabFileFetchUrl where a target whose path did not contain the
+`/-/` scope would result in a fetch URL that did not support
+private-token-based authentication.

packages/backend-common/src/reading/GitlabUrlReader.test.ts

@@ -106,7 +106,7 @@ describe('GitlabUrlReader', () => {
       );
 
     it.each([
-      // Project URLs
+      // Scoped routes
       {
         url: 'https://gitlab.com/groupA/teams/teamA/subgroupA/repoA/-/blob/branch/my/path/to/file.yaml',
         config: createConfig(),
@@ -135,12 +135,12 @@ describe('GitlabUrlReader', () => {
         }),
       },
 
-      // Raw URLs
+      // Unscoped route
       {
         url: 'https://gitlab.example.com/a/b/blob/master/c.yaml',
         config: createConfig(),
         response: expect.objectContaining({
-          url: 'https://gitlab.example.com/a/b/raw/master/c.yaml',
+          url: 'https://gitlab.example.com/api/v4/projects/12345/repository/files/c.yaml/raw?ref=master',
         }),
       },
     ])('should handle happy path %#', async ({ url, config, response }) => {

packages/integration/src/gitlab/core.test.ts

@@ -28,19 +28,15 @@ describe('gitlab core', () => {
 
   beforeEach(() => {
     worker.use(
-      rest.get('*/api/v4/projects/:name', (_, res, ctx) =>
+      rest.get('*/api/v4/projects/group%2Fproject', (_, res, ctx) =>
+        res(ctx.status(200), ctx.json({ id: 12345 })),
+      ),
+      rest.get('*/api/v4/projects/group%2Fsubgroup%2Fproject', (_, res, ctx) =>
         res(ctx.status(200), ctx.json({ id: 12345 })),
       ),
     );
   });
 
-  const configWithToken: GitLabIntegrationConfig = {
-    host: 'gitlab.com',
-    token: '0123456789',
-    apiBaseUrl: '<ignored>',
-    baseUrl: '<ignored>',
-  };
-
   const configWithNoToken: GitLabIntegrationConfig = {
     host: 'gitlab.com',
     apiBaseUrl: '<ignored>',
@@ -61,163 +57,133 @@ describe('gitlab core', () => {
     baseUrl: 'https://gitlab.mycompany.com',
   };
 
-  describe('getGitLabFileFetchUrl with .yaml extension', () => {
-    it.each([
-      // Project URLs
-      {
-        config: configWithNoToken,
-        url: 'https://gitlab.com/groupA/teams/teamA/subgroupA/repoA/-/blob/branch/my/path/to/file.yaml',
-        result:
-          'https://gitlab.com/api/v4/projects/12345/repository/files/my%2Fpath%2Fto%2Ffile.yaml/raw?ref=branch',
-      },
-      {
-        config: configSelfHosteWithRelativePath,
-        url: 'https://gitlab.mycompany.com/gitlab/groupA/teams/teamA/subgroupA/repoA/-/blob/branch/my/path/to/file.yaml',
-        result:
-          'https://gitlab.mycompany.com/gitlab/api/v4/projects/12345/repository/files/my%2Fpath%2Fto%2Ffile.yaml/raw?ref=branch',
-      },
-      {
-        config: configSelfHostedWithoutRelativePath,
-        url: 'https://gitlab.mycompany.com/groupA/teams/teamA/subgroupA/repoA/-/blob/branch/my/path/to/file.yaml',
-        result:
-          'https://gitlab.mycompany.com/api/v4/projects/12345/repository/files/my%2Fpath%2Fto%2Ffile.yaml/raw?ref=branch',
-      },
-      {
-        config: configWithNoToken,
-        // Works with non URI encoded link
-        url: 'https://gitlab.com/groupA/teams/teamA/subgroupA/repoA/-/blob/branch/my/path/to/file with spaces.yaml',
-        result:
-          'https://gitlab.com/api/v4/projects/12345/repository/files/my%2Fpath%2Fto%2Ffile%20with%20spaces.yaml/raw?ref=branch',
-      },
-      {
-        config: configSelfHosteWithRelativePath,
-        url: 'https://gitlab.mycompany.com/gitlab/groupA/teams/teamA/subgroupA/repoA/-/blob/branch/my/path/to/file with spaces.yaml',
-        result:
-          'https://gitlab.mycompany.com/gitlab/api/v4/projects/12345/repository/files/my%2Fpath%2Fto%2Ffile%20with%20spaces.yaml/raw?ref=branch',
-      },
-      {
-        config: configSelfHostedWithoutRelativePath,
-        url: 'https://gitlab.mycompany.com/groupA/teams/teamA/subgroupA/repoA/-/blob/branch/my/path/to/file with spaces.yaml',
-        result:
-          'https://gitlab.mycompany.com/api/v4/projects/12345/repository/files/my%2Fpath%2Fto%2Ffile%20with%20spaces.yaml/raw?ref=branch',
-      },
-      {
-        config: configWithNoToken,
-        url: 'https://gitlab.com/groupA/teams/teamA/subgroupA/repoA/-/blob/branch/my/path%20with%20spaces/to/file.yaml',
-        result:
-          'https://gitlab.com/api/v4/projects/12345/repository/files/my%2Fpath%20with%20spaces%2Fto%2Ffile.yaml/raw?ref=branch',
-      },
-      {
-        config: configWithToken,
-        url: 'https://gitlab.example.com/groupA/teams/teamA/subgroupA/repoA/-/blob/branch/my/path%20with%20spaces/to/file.yaml',
-        result:
-          'https://gitlab.example.com/api/v4/projects/12345/repository/files/my%2Fpath%20with%20spaces%2Fto%2Ffile.yaml/raw?ref=branch',
-      },
-      {
-        config: configWithNoToken,
-        url: 'https://gitlab.com/groupA/teams/teamA/repoA/-/blob/branch/my/path%20with%20spaces/to/file.yaml', // Repo not in subgroup
-        result:
-          'https://gitlab.com/api/v4/projects/12345/repository/files/my%2Fpath%20with%20spaces%2Fto%2Ffile.yaml/raw?ref=branch',
-      },
-      // Raw URLs
-      {
-        config: configWithNoToken,
-        url: 'https://gitlab.example.com/a/b/blob/master/c.yaml',
-        result: 'https://gitlab.example.com/a/b/raw/master/c.yaml',
-      },
-      {
-        config: configWithNoToken,
-        url: 'https://gitlab.example.com/a/b/repo/blob/master/c.yaml',
-        result: 'https://gitlab.example.com/a/b/repo/raw/master/c.yaml',
-      },
-      {
-        config: configWithNoToken,
-        url: 'https://gitlab.example.com/a/blob/blob/master/c.yaml',
-        result: 'https://gitlab.example.com/a/blob/raw/master/c.yaml',
-      },
-      {
-        config: configWithNoToken,
-        url: 'https://gitlab.example.com/a/b/blob/blob/c.yaml',
-        result: 'https://gitlab.example.com/a/b/raw/blob/c.yaml',
-      },
-      {
-        config: configWithNoToken,
-        url: 'https://gitlab.example.com/a//blob/blob/c.yaml',
-        result: 'https://gitlab.example.com/a/blob/raw/c.yaml',
-      },
-    ])('should handle happy path %#', async ({ config, url, result }) => {
-      await expect(getGitLabFileFetchUrl(url, config)).resolves.toBe(result);
-    });
-  });
+  describe('getGitLabFileFetchUrl', () => {
+    describe('when target has a scoped route', () => {
+      it('returns a projects API URL', async () => {
+        const target =
+          'https://gitlab.com/group/project/-/blob/branch/folder/file.yaml';
+        const fetchUrl =
+          'https://gitlab.com/api/v4/projects/12345/repository/files/folder%2Ffile.yaml/raw?ref=branch';
+        await expect(
+          getGitLabFileFetchUrl(target, configWithNoToken),
+        ).resolves.toBe(fetchUrl);
+      });
 
-  describe('getGitLabFileFetchUrl with .yml extension', () => {
-    it.each([
-      // Project URLs
-      {
-        config: configWithNoToken,
-        url: 'https://gitlab.com/groupA/teams/teamA/subgroupA/repoA/-/blob/branch/my/path/to/file.yml',
-        result:
-          'https://gitlab.com/api/v4/projects/12345/repository/files/my%2Fpath%2Fto%2Ffile.yml/raw?ref=branch',
-      },
-      {
-        config: configSelfHosteWithRelativePath,
-        url: 'https://gitlab.mycompany.com/gitlab/groupA/teams/teamA/subgroupA/repoA/-/blob/branch/my/path/to/file.yml',
-        result:
-          'https://gitlab.mycompany.com/gitlab/api/v4/projects/12345/repository/files/my%2Fpath%2Fto%2Ffile.yml/raw?ref=branch',
-      },
-      {
-        config: configSelfHostedWithoutRelativePath,
-        url: 'https://gitlab.mycompany.com/groupA/teams/teamA/subgroupA/repoA/-/blob/branch/my/path/to/file.yml',
-        result:
-          'https://gitlab.mycompany.com/api/v4/projects/12345/repository/files/my%2Fpath%2Fto%2Ffile.yml/raw?ref=branch',
-      },
-      {
-        config: configWithNoToken,
-        // Works with non URI encoded link
-        url: 'https://gitlab.com/groupA/teams/teamA/subgroupA/repoA/-/blob/branch/my/path/to/file with spaces.yml',
-        result:
-          'https://gitlab.com/api/v4/projects/12345/repository/files/my%2Fpath%2Fto%2Ffile%20with%20spaces.yml/raw?ref=branch',
-      },
-      {
-        config: configSelfHosteWithRelativePath,
-        // Works with non URI encoded link
-        url: 'https://gitlab.mycompany.com/gitlab/groupA/teams/teamA/subgroupA/repoA/-/blob/branch/my/path/to/file with spaces.yml',
-        result:
-          'https://gitlab.mycompany.com/gitlab/api/v4/projects/12345/repository/files/my%2Fpath%2Fto%2Ffile%20with%20spaces.yml/raw?ref=branch',
-      },
-      {
-        config: configSelfHostedWithoutRelativePath,
-        // Works with non URI encoded link
-        url: 'https://gitlab.mycompany.com/groupA/teams/teamA/subgroupA/repoA/-/blob/branch/my/path/to/file with spaces.yml',
-        result:
-          'https://gitlab.mycompany.com/api/v4/projects/12345/repository/files/my%2Fpath%2Fto%2Ffile%20with%20spaces.yml/raw?ref=branch',
-      },
-      {
-        config: configWithNoToken,
-        url: 'https://gitlab.com/groupA/teams/teamA/subgroupA/repoA/-/blob/branch/my/path%20with%20spaces/to/file.yml',
-        result:
-          'https://gitlab.com/api/v4/projects/12345/repository/files/my%2Fpath%20with%20spaces%2Fto%2Ffile.yml/raw?ref=branch',
-      },
-      {
-        config: configWithToken,
-        url: 'https://gitlab.example.com/groupA/teams/teamA/subgroupA/repoA/-/blob/branch/my/path%20with%20spaces/to/file.yml',
-        result:
-          'https://gitlab.example.com/api/v4/projects/12345/repository/files/my%2Fpath%20with%20spaces%2Fto%2Ffile.yml/raw?ref=branch',
-      },
-      {
-        config: configWithNoToken,
-        url: 'https://gitlab.com/groupA/teams/teamA/repoA/-/blob/branch/my/path%20with%20spaces/to/file.yml', // Repo not in subgroup
-        result:
-          'https://gitlab.com/api/v4/projects/12345/repository/files/my%2Fpath%20with%20spaces%2Fto%2Ffile.yml/raw?ref=branch',
-      },
-      // Raw URLs
-      {
-        config: configWithNoToken,
-        url: 'https://gitlab.example.com/a/b/blob/master/c.yml',
-        result: 'https://gitlab.example.com/a/b/raw/master/c.yml',
-      },
-    ])('should handle happy path %#', async ({ config, url, result }) => {
-      await expect(getGitLabFileFetchUrl(url, config)).resolves.toBe(result);
+      it('supports folder named "blob"', async () => {
+        const target =
+          'https://gitlab.com/group/project/-/blob/branch/blob/file.yaml';
+        const fetchUrl =
+          'https://gitlab.com/api/v4/projects/12345/repository/files/blob%2Ffile.yaml/raw?ref=branch';
+        await expect(
+          getGitLabFileFetchUrl(target, configWithNoToken),
+        ).resolves.toBe(fetchUrl);
+      });
+
+      it('locates projects in subgroups', async () => {
+        const target =
+          'https://gitlab.com/group/subgroup/project/-/blob/branch/folder/file.yaml';
+        const fetchUrl =
+          'https://gitlab.com/api/v4/projects/12345/repository/files/folder%2Ffile.yaml/raw?ref=branch';
+        await expect(
+          getGitLabFileFetchUrl(target, configWithNoToken),
+        ).resolves.toBe(fetchUrl);
+      });
+
+      it('supports filename with .yml extension', async () => {
+        const target =
+          'https://gitlab.com/group/project/-/blob/branch/folder/file.yml';
+        const fetchUrl =
+          'https://gitlab.com/api/v4/projects/12345/repository/files/folder%2Ffile.yml/raw?ref=branch';
+        await expect(
+          getGitLabFileFetchUrl(target, configWithNoToken),
+        ).resolves.toBe(fetchUrl);
+      });
+
+      it('supports non-URI-encoded target', async () => {
+        const target =
+          'https://gitlab.com/group/project/-/blob/branch/folder/file with spaces.yaml';
+        const fetchUrl =
+          'https://gitlab.com/api/v4/projects/12345/repository/files/folder%2Ffile%20with%20spaces.yaml/raw?ref=branch';
+        await expect(
+          getGitLabFileFetchUrl(target, configWithNoToken),
+        ).resolves.toBe(fetchUrl);
+      });
+
+      describe('when gitlab is self-hosted', () => {
+        it('returns projects API URL', async () => {
+          const target =
+            'https://gitlab.mycompany.com/group/project/-/blob/branch/folder/file.yaml';
+          const fetchUrl =
+            'https://gitlab.mycompany.com/api/v4/projects/12345/repository/files/folder%2Ffile.yaml/raw?ref=branch';
+          await expect(
+            getGitLabFileFetchUrl(target, configSelfHostedWithoutRelativePath),
+          ).resolves.toBe(fetchUrl);
+        });
+
+        it('handles non-URI-encoded target', async () => {
+          const target =
+            'https://gitlab.mycompany.com/group/project/-/blob/branch/folder/file with spaces.yaml';
+          const fetchUrl =
+            'https://gitlab.mycompany.com/api/v4/projects/12345/repository/files/folder%2Ffile%20with%20spaces.yaml/raw?ref=branch';
+          await expect(
+            getGitLabFileFetchUrl(target, configSelfHostedWithoutRelativePath),
+          ).resolves.toBe(fetchUrl);
+        });
+
+        describe('with a relative path', () => {
+          it('returns projects API URL', async () => {
+            const target =
+              'https://gitlab.mycompany.com/gitlab/group/project/-/blob/branch/folder/file.yaml';
+            const fetchUrl =
+              'https://gitlab.mycompany.com/gitlab/api/v4/projects/12345/repository/files/folder%2Ffile.yaml/raw?ref=branch';
+            await expect(
+              getGitLabFileFetchUrl(target, configSelfHosteWithRelativePath),
+            ).resolves.toBe(fetchUrl);
+          });
+
+          it('handles non-URI-encoded target', async () => {
+            const target =
+              'https://gitlab.mycompany.com/gitlab/group/project/-/blob/branch/folder/file with spaces.yaml';
+            const fetchUrl =
+              'https://gitlab.mycompany.com/gitlab/api/v4/projects/12345/repository/files/folder%2Ffile%20with%20spaces.yaml/raw?ref=branch';
+            await expect(
+              getGitLabFileFetchUrl(target, configSelfHosteWithRelativePath),
+            ).resolves.toBe(fetchUrl);
+          });
+        });
+      });
+    });
+
+    describe('when target has an unscoped route', () => {
+      it('returns projects API URL', async () => {
+        const target =
+          'https://gitlab.com/group/project/blob/branch/folder/file.yaml';
+        const fetchUrl =
+          'https://gitlab.com/api/v4/projects/12345/repository/files/folder%2Ffile.yaml/raw?ref=branch';
+        await expect(
+          getGitLabFileFetchUrl(target, configWithNoToken),
+        ).resolves.toBe(fetchUrl);
+      });
+
+      it('supports project in subgroup', async () => {
+        const target =
+          'https://gitlab.com/group/subgroup/project/blob/branch/folder/file.yaml';
+        const fetchUrl =
+          'https://gitlab.com/api/v4/projects/12345/repository/files/folder%2Ffile.yaml/raw?ref=branch';
+        await expect(
+          getGitLabFileFetchUrl(target, configWithNoToken),
+        ).resolves.toBe(fetchUrl);
+      });
+
+      it('supports repo with branch named "blob"', async () => {
+        const target =
+          'https://gitlab.com/group/project/blob/blob/folder/file.yaml';
+        const fetchUrl =
+          'https://gitlab.com/api/v4/projects/12345/repository/files/folder%2Ffile.yaml/raw?ref=blob';
+        await expect(
+          getGitLabFileFetchUrl(target, configWithNoToken),
+        ).resolves.toBe(fetchUrl);
+      });
     });
   });
 });

packages/integration/src/gitlab/core.ts

@@ -19,7 +19,6 @@ import {
   GitLabIntegrationConfig,
 } from './config';
 import fetch from 'cross-fetch';
-import { InputError } from '@backstage/errors';
 
 /**
  * Given a URL pointing to a file on a provider, returns a URL that is suitable
@@ -29,7 +28,7 @@ import { InputError } from '@backstage/errors';
  *
  * Converts
  * from: https://gitlab.example.com/a/b/blob/master/c.yaml
- * to:   https://gitlab.example.com/a/b/raw/master/c.yaml
+ * to:   https://gitlab.com/api/v4/projects/projectId/repository/c.yaml?ref=master
  * -or-
  * from: https://gitlab.com/groupA/teams/teamA/subgroupA/repoA/-/blob/branch/filepath
  * to:   https://gitlab.com/api/v4/projects/projectId/repository/files/filepath?ref=branch
@@ -42,15 +41,8 @@ export async function getGitLabFileFetchUrl(
   url: string,
   config: GitLabIntegrationConfig,
 ): Promise<string> {
-  // TODO(Rugvip): From the old GitlabReaderProcessor; used
-  // the existence of /-/blob/ to switch the logic. Don't know if this
-  // makes sense and it might require some more work.
-
-  if (url.includes('/-/blob/')) {
-    const projectID = await getProjectId(url, config);
-    return buildProjectUrl(url, projectID, config).toString();
-  }
-  return buildRawUrl(url).toString();
+  const projectID = await getProjectId(url, config);
+  return buildProjectUrl(url, projectID, config).toString();
 }
 
 /**
@@ -70,38 +62,6 @@ export function getGitLabRequestOptions(config: GitLabIntegrationConfig): {
   };
 }
 
-// Converts
-// from: https://gitlab.example.com/groupA/teams/repoA/blob/master/c.yaml
-// to:   https://gitlab.example.com/groupA/teams/repoA/raw/master/c.yaml
-export function buildRawUrl(target: string): URL {
-  try {
-    const url = new URL(target);
-
-    const splitPath = url.pathname.split('/').filter(Boolean);
-
-    // Check blob existence
-    const blobIndex = splitPath.indexOf('blob', 2);
-    if (blobIndex < 2 || blobIndex === splitPath.length - 1) {
-      throw new InputError('Wrong GitLab URL');
-    }
-
-    // Take repo path
-    const repoPath = splitPath.slice(0, blobIndex);
-    const restOfPath = splitPath.slice(blobIndex + 1);
-
-    if (!restOfPath.join('/').match(/\.(yaml|yml)$/)) {
-      throw new InputError('Wrong GitLab URL');
-    }
-
-    // Replace 'blob' with 'raw'
-    url.pathname = [...repoPath, 'raw', ...restOfPath].join('/');
-
-    return url;
-  } catch (e) {
-    throw new InputError(`Incorrect url: ${target}, ${e}`);
-  }
-}
-
 // Converts
 // from: https://gitlab.com/groupA/teams/teamA/subgroupA/repoA/-/blob/branch/filepath
 // to:   https://gitlab.com/api/v4/projects/projectId/repository/files/filepath?ref=branch
@@ -113,7 +73,10 @@ export function buildProjectUrl(
   try {
     const url = new URL(target);
 
-    const branchAndFilePath = url.pathname.split('/-/blob/')[1];
+    const branchAndFilePath = url.pathname
+      .split('/blob/')
+      .slice(1)
+      .join('/blob/');
     const [branch, ...filePath] = branchAndFilePath.split('/');
     const relativePath = getGitLabIntegrationRelativePath(config);
 
@@ -143,12 +106,12 @@ export async function getProjectId(
 ): Promise<number> {
   const url = new URL(target);
 
-  if (!url.pathname.includes('/-/blob/')) {
+  if (!url.pathname.includes('/blob/')) {
     throw new Error('Please provide full path to yaml file from GitLab');
   }
 
   try {
-    let repo = url.pathname.split('/-/blob/')[0];
+    let repo = url.pathname.split('/-/blob/')[0].split('/blob/')[0];
 
     // Get gitlab relative path
     const relativePath = getGitLabIntegrationRelativePath(config);