Merge pull request #31217 from backstage/clairep/gitlab_module_to_prefer_projectId_1

GitlabDiscoverEntityProvider: update shouldProcessProject to use project.id for hasFile check

.changeset/twelve-oranges-grin.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-catalog-backend-module-gitlab': patch
+---
+
+Fixed an issue in `GitlabDiscoveryEntityProvider` where entity fetching could fail for projects with special characters or that had been renamed or moved.

plugins/catalog-backend-module-gitlab/src/GitLabDiscoveryProcessor.test.ts

@@ -33,6 +33,7 @@ const API_URL = `${SERVER_URL}/api/v4`;
 const PROJECTS_URL = `${API_URL}/projects`;
 const GROUP_PROJECTS_URL = `${API_URL}/groups/group%2Fsubgroup/projects`;
 const EXISTING_PROJECT_PATH = 'exist';
+const EXISTING_PROJECT_ID = 2;
 
 const PROJECT_LOCATION: LocationSpec = {
   type: 'gitlab-discovery',
@@ -105,7 +106,7 @@ function setupFakeServer(
       );
     }),
     rest.head(
-      `${API_URL}/projects/:project_path/repository/files/:file_path`,
+      `${API_URL}/projects/:projectIdentifier/repository/files/:file_path`,
       (req, res, ctx) => {
         if (req.headers.get('authorization') !== 'Bearer test-token') {
           return res(
@@ -119,7 +120,11 @@ function setupFakeServer(
           return res(ctx.status(200));
         }
 
-        if (EXISTING_PROJECT_PATH === req.params.project_path) {
+        if (
+          // Gitlab GET project endpoint can use either numeric project.id or string project.path_with_namespace
+          EXISTING_PROJECT_ID.toString() === req.params.projectIdentifier ||
+          EXISTING_PROJECT_PATH === req.params.projectIdentifier
+        ) {
           return res(ctx.status(200));
         }
 
@@ -385,7 +390,7 @@ describe('GitlabDiscoveryProcessor', () => {
                   path_with_namespace: '1',
                 },
                 {
-                  id: 1,
+                  id: EXISTING_PROJECT_ID,
                   archived: false,
                   default_branch: 'main',
                   last_activity_at: '2021-08-05T11:03:05.774Z',

plugins/catalog-backend-module-gitlab/src/GitLabDiscoveryProcessor.ts

@@ -139,7 +139,7 @@ export class GitLabDiscoveryProcessor implements CatalogProcessor {
         const project_branch = branch === '*' ? project.default_branch : branch;
 
         const projectHasFile: boolean = await client.hasFile(
-          project.path_with_namespace,
+          project.id,
           project_branch,
           catalogPath,
         );

plugins/catalog-backend-module-gitlab/src/__testUtils__/handlers.ts

@@ -266,13 +266,13 @@ const httpProjectFindByIdDynamic = all_projects_response.map(project => {
     return res(ctx.json(all_projects_response.find(p => p.id === project.id)));
   });
 });
-const httpProjectCatalogDynamic = all_projects_response.map(project => {
-  const path: string = project.path_with_namespace
-    ? project.path_with_namespace!.replace(/\//g, '%2F')
-    : `${project.path_with_namespace}%2F${project.name}`;
 
+/**
+ * See https://docs.gitlab.com/api/repository_files/#get-file-from-repository
+ */
+const httpProjectCatalogDynamic = all_projects_response.flatMap(project => {
   return rest.head(
-    `${apiBaseUrl}/projects/${path}/repository/files/catalog-info.yaml`,
+    `${apiBaseUrl}/projects/${project.id.toString()}/repository/files/catalog-info.yaml`,
     (req, res, ctx) => {
       const branch = req.url.searchParams.get('ref');
       if (

plugins/catalog-backend-module-gitlab/src/lib/client.test.ts

@@ -581,20 +581,12 @@ describe('hasFile', () => {
   });
 
   it('should find catalog file', async () => {
-    const hasFile = await client.hasFile(
-      'group1/test-repo1',
-      'main',
-      'catalog-info.yaml',
-    );
+    const hasFile = await client.hasFile(1, 'main', 'catalog-info.yaml');
     expect(hasFile).toBe(true);
   });
 
   it('should not find catalog file', async () => {
-    const hasFile = await client.hasFile(
-      'group1/test-repo1',
-      'unknown',
-      'catalog-info.yaml',
-    );
+    const hasFile = await client.hasFile(1, 'unknown', 'catalog-info.yaml');
     expect(hasFile).toBe(false);
   });
 });

plugins/catalog-backend-module-gitlab/src/lib/client.ts

@@ -342,18 +342,19 @@ export class GitLabClient {
 
   /**
    * General existence check.
+   * @see {@link https://docs.gitlab.com/api/repository_files/#get-file-from-repository | GitLab Repository Files API}
    *
-   * @param projectPath - The path to the project
+   * @param projectIdentifier - The identifier of the project, either the numeric ID or the namespaced path.
    * @param branch - The branch used to search
    * @param filePath - The path to the file
    */
   async hasFile(
-    projectPath: string,
+    projectIdentifier: string | number,
     branch: string,
     filePath: string,
   ): Promise<boolean> {
     const endpoint: string = `/projects/${encodeURIComponent(
-      projectPath,
+      projectIdentifier,
     )}/repository/files/${encodeURIComponent(filePath)}`;
     const request = new URL(`${this.config.apiBaseUrl}${endpoint}`);
     request.searchParams.append('ref', branch);

plugins/catalog-backend-module-gitlab/src/providers/GitlabDiscoveryEntityProvider.ts

@@ -590,7 +590,7 @@ export class GitlabDiscoveryEntityProvider implements EntityProvider {
       this.config.fallbackBranch;
 
     const hasFile = await client.hasFile(
-      project.path_with_namespace ?? '',
+      project.id,
       project_branch,
       this.config.catalogFile,
     );