Merge pull request #17126 from fearphage/familiarize-entity-names

Familiarize entity names before sending them to GitHub

.changeset/serious-items-walk.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-scaffolder-backend': patch
+---
+
+Stripped entity types and namespace before passing to GitHub API

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

@@ -28,6 +28,7 @@ import {
 import { when } from 'jest-when';
 import { PassThrough } from 'stream';
 import { createGithubRepoCreateAction } from './githubRepoCreate';
+import { entityRefToName } from '../helpers';
 
 const mockOctokit = {
   rest: {
@@ -83,15 +84,17 @@ describe('github:repo:create', () => {
   };
 
   beforeEach(() => {
-    jest.resetAllMocks();
     githubCredentialsProvider =
       DefaultGithubCredentialsProvider.fromIntegrations(integrations);
     action = createGithubRepoCreateAction({
       integrations,
       githubCredentialsProvider,
     });
+    (entityRefToName as jest.Mock).mockImplementation((s: string) => s);
   });
 
+  afterEach(jest.resetAllMocks);
+
   it('should call the githubApis with the correct values for createInOrg', async () => {
     mockOctokit.rest.users.getByUsername.mockResolvedValue({
       data: { type: 'Organization' },

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

@@ -29,6 +29,7 @@ import {
   initRepoAndPush,
 } from '../helpers';
 import { getRepoSourceDirectory, parseRepoUrl } from '../publish/util';
+import { entityRefToName } from '../../builtin/helpers';
 
 const DEFAULT_TIMEOUT_MS = 60_000;
 
@@ -218,13 +219,13 @@ export async function createGithubRepoWithCollaboratorsAndTopics(
           await client.rest.repos.addCollaborator({
             owner,
             repo,
-            username: collaborator.user,
+            username: entityRefToName(collaborator.user),
             permission: collaborator.access,
           });
         } else if ('team' in collaborator) {
           await client.rest.teams.addOrUpdateRepoPermissionsInOrg({
             org: owner,
-            team_slug: collaborator.team,
+            team_slug: entityRefToName(collaborator.team),
             owner,
             repo,
             permission: collaborator.access,

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

@@ -15,7 +15,7 @@
  */
 
 import { Git, getVoidLogger } from '@backstage/backend-common';
-import { commitAndPushRepo, initRepoAndPush } from './helpers';
+import { commitAndPushRepo, entityRefToName, initRepoAndPush } from './helpers';
 
 jest.mock('@backstage/backend-common', () => ({
   Git: {
@@ -301,3 +301,17 @@ describe('commitAndPushRepo', () => {
     });
   });
 });
+
+describe('entityRefToName', () => {
+  it.each([
+    'user:default/catpants',
+    'group:default/catpants',
+    'user:catpants',
+    'default/catpants',
+    'user:custom/catpants',
+    'group:custom/catpants',
+    'catpants',
+  ])('should parse: "%s"', (entityName: string) => {
+    expect(entityRefToName(entityName)).toEqual('catpants');
+  });
+});

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

@@ -296,3 +296,7 @@ export function getGitCommitMessage(
     ? gitCommitMessage
     : config.getOptionalString('scaffolder.defaultCommitMessage');
 }
+
+export function entityRefToName(name: string): string {
+  return name.replace(/^.*[:/]/g, '');
+}

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

@@ -29,6 +29,7 @@ import { when } from 'jest-when';
 import { PassThrough } from 'stream';
 import {
   enableBranchProtectionOnDefaultRepoBranch,
+  entityRefToName,
   initRepoAndPush,
 } from '../helpers';
 import { createPublishGithubAction } from './github';
@@ -68,6 +69,8 @@ describe('publish:github', () => {
     },
   });
 
+  const { entityRefToName: realFamiliarizeEntityName } =
+    jest.requireActual('../helpers');
   const integrations = ScmIntegrations.fromConfig(config);
   let githubCredentialsProvider: GithubCredentialsProvider;
   let action: TemplateAction<any>;
@@ -87,7 +90,6 @@ describe('publish:github', () => {
   };
 
   beforeEach(() => {
-    jest.resetAllMocks();
     githubCredentialsProvider =
       DefaultGithubCredentialsProvider.fromIntegrations(integrations);
     action = createPublishGithubAction({
@@ -95,8 +97,15 @@ describe('publish:github', () => {
       config,
       githubCredentialsProvider,
     });
+
+    // restore real implmentation
+    (entityRefToName as jest.Mock).mockImplementation(
+      realFamiliarizeEntityName,
+    );
   });
 
+  afterEach(jest.resetAllMocks);
+
   it('should fail to create if the team is not found in the org', async () => {
     mockOctokit.rest.users.getByUsername.mockResolvedValue({
       data: { type: 'Organization' },
@@ -651,6 +660,59 @@ describe('publish:github', () => {
     });
   });
 
+  it('should familiarize entity names while adding collaborators', async () => {
+    mockOctokit.rest.users.getByUsername.mockResolvedValue({
+      data: { type: 'User' },
+    });
+
+    mockOctokit.rest.repos.createForAuthenticatedUser.mockResolvedValue({
+      data: {
+        clone_url: 'https://github.com/clone/url.git',
+        html_url: 'https://github.com/html/url',
+      },
+    });
+
+    await action.handler({
+      ...mockContext,
+      input: {
+        ...mockContext.input,
+        collaborators: [
+          {
+            access: 'pull',
+            user: 'user:robot-1',
+          },
+          {
+            access: 'push',
+            team: 'group:default/robot-2',
+          },
+        ],
+      },
+    });
+
+    const commonProperties = {
+      owner: 'owner',
+      repo: 'repo',
+    };
+
+    expect(mockOctokit.rest.repos.addCollaborator).toHaveBeenCalledWith({
+      ...commonProperties,
+      username: 'robot-1',
+      permission: 'pull',
+    });
+
+    expect(
+      mockOctokit.rest.teams.addOrUpdateRepoPermissionsInOrg,
+    ).toHaveBeenCalledWith({
+      ...commonProperties,
+      org: 'owner',
+      team_slug: 'robot-2',
+      permission: 'push',
+    });
+
+    expect(entityRefToName).toHaveBeenCalledWith('user:robot-1');
+    expect(entityRefToName).toHaveBeenCalledWith('group:default/robot-2');
+  });
+
   it('should ignore failures when adding multiple collaborators', async () => {
     mockOctokit.rest.users.getByUsername.mockResolvedValue({
       data: { type: 'User' },