fix: check for teams before creating repository

Shifts the team check for the 'access' parameter to happen prior to the
repo creation, so that repositories are not created and have no
permissions set on them due to missing teams.

Signed-off-by: Christopher Kruse <ckruse@flexport.com>

plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubRepoCreate.test.ts

@@ -42,6 +42,7 @@ const mockOctokit = {
     },
     teams: {
       addOrUpdateRepoPermissionsInOrg: jest.fn(),
+      getByName: jest.fn(),
     },
   },
 };
@@ -96,6 +97,13 @@ describe('github:repo:create', () => {
       data: { type: 'Organization' },
     });
 
+    mockOctokit.rest.teams.getByName.mockResolvedValue({
+      data: {
+        name: 'blam',
+        id: 42,
+      },
+    });
+
     mockOctokit.rest.repos.createInOrg.mockResolvedValue({ data: {} });
 
     await action.handler(mockContext);

plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/helpers.ts

@@ -135,6 +135,10 @@ export async function createGithubRepoWithCollaboratorsAndTopics(
     username: owner,
   });
 
+  if (access?.startsWith(`${owner}/`)) {
+    await validateAccessTeam(client, access);
+  }
+
   const repoCreationPromise =
     user.data.type === 'Organization'
       ? client.rest.repos.createInOrg({
@@ -190,22 +194,13 @@ export async function createGithubRepoWithCollaboratorsAndTopics(
 
   if (access?.startsWith(`${owner}/`)) {
     const [, team] = access.split('/');
-    try {
-      await client.rest.teams.addOrUpdateRepoPermissionsInOrg({
-        org: owner,
-        team_slug: team,
-        owner,
-        repo,
-        permission: 'admin',
-      });
-    } catch (e) {
-      if (e.data.message === 'Not Found') {
-        const message = `Received 'Not Found' from the API; one of org:
-        ${owner}, team: ${team} or repo: ${repo} was not found within GitHub.`;
-        logger.warn(message);
-        throw new Error(message, { cause: e });
-      }
-    }
+    await client.rest.teams.addOrUpdateRepoPermissionsInOrg({
+      org: owner,
+      team_slug: team,
+      owner,
+      repo,
+      permission: 'admin',
+    });
     // No need to add access if it's the person who owns the personal account
   } else if (access && access !== owner) {
     await client.rest.repos.addCollaborator({
@@ -360,3 +355,22 @@ function extractCollaboratorName(
   if ('user' in collaborator) return collaborator.user;
   return collaborator.team;
 }
+
+async function validateAccessTeam(client: Octokit, access: string) {
+  const [org, team_slug] = access.split('/');
+  try {
+    // Below rule disabled because of a 'getByName' check for a different library
+    // incorrectly triggers here.
+    // eslint-disable-next-line testing-library/no-await-sync-query
+    await client.rest.teams.getByName({
+      org,
+      team_slug,
+    });
+  } catch (e) {
+    if (e.response.data.message === 'Not Found') {
+      const message = `Received 'Not Found' from the API; one of org:
+        ${org} or team: ${team_slug} was not found within GitHub.`;
+      throw new Error(message, { cause: e });
+    }
+  }
+}

plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.test.ts

@@ -45,6 +45,7 @@ const mockOctokit = {
       replaceAllTopics: jest.fn(),
     },
     teams: {
+      getByName: jest.fn(),
       addOrUpdateRepoPermissionsInOrg: jest.fn(),
     },
   },
@@ -96,11 +97,41 @@ describe('publish:github', () => {
     });
   });
 
+  it('should fail to create if the team is not found in the org', async () => {
+    mockOctokit.rest.users.getByUsername.mockResolvedValue({
+      data: { type: 'Organization' },
+    });
+
+    mockOctokit.rest.teams.getByName.mockRejectedValue({
+      response: {
+        status: 404,
+        data: {
+          message: 'Not Found',
+          documentation_url:
+            'https://docs.github.com/en/rest/teams/teams#add-or-update-team-repository-permissions',
+        },
+      },
+    });
+
+    await expect(action.handler(mockContext)).rejects.toThrow(
+      "Received 'Not Found' from the API;",
+    );
+
+    expect(mockOctokit.rest.repos.createInOrg).not.toHaveBeenCalled();
+  });
+
   it('should call the githubApis with the correct values for createInOrg', async () => {
     mockOctokit.rest.users.getByUsername.mockResolvedValue({
       data: { type: 'Organization' },
     });
 
+    mockOctokit.rest.teams.getByName.mockResolvedValue({
+      data: {
+        name: 'blam',
+        id: 42,
+      },
+    });
+
     mockOctokit.rest.repos.createInOrg.mockResolvedValue({ data: {} });
 
     await action.handler(mockContext);
@@ -523,24 +554,23 @@ describe('publish:github', () => {
       data: { type: 'User' },
     });
 
-    mockOctokit.rest.repos.createForAuthenticatedUser.mockResolvedValue({
-      data: {
-        clone_url: 'https://github.com/clone/url.git',
-        html_url: 'https://github.com/html/url',
-      },
-    });
-
-    mockOctokit.rest.teams.addOrUpdateRepoPermissionsInOrg.mockRejectedValue({
-      status: 404,
-      data: {
-        message: 'Not Found',
-        documentation_url:
-          'https://docs.github.com/en/rest/teams/teams#add-or-update-team-repository-permissions',
+    mockOctokit.rest.teams.getByName.mockRejectedValue({
+      response: {
+        status: 404,
+        data: {
+          message: 'Not Found',
+          documentation_url:
+            'https://docs.github.com/en/rest/teams/teams#add-or-update-team-repository-permissions',
+        },
       },
     });
     await expect(action.handler(mockContext)).rejects.toThrow(
       "Received 'Not Found' from the API;",
     );
+
+    expect(
+      mockOctokit.rest.repos.createForAuthenticatedUser,
+    ).not.toHaveBeenCalled();
   });
 
   it('should add outside collaborators when provided', async () => {