Merge pull request #16059 from ballpointcarrot/fix/github-teams-check

fix: better error around github teams not found
This commit is contained in:
Patrik Oldsberg
2023-02-13 22:10:56 +01:00
committed by GitHub
4 changed files with 92 additions and 1 deletions
+5
View File
@@ -0,0 +1,5 @@
---
'@backstage/plugin-scaffolder-backend': patch
---
Provide better error messaging when GitHub fails due to missing team definitions
@@ -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);
@@ -15,7 +15,7 @@
*/
import { Config } from '@backstage/config';
import { assertError, InputError } from '@backstage/errors';
import { assertError, InputError, NotFoundError } from '@backstage/errors';
import {
DefaultGithubCredentialsProvider,
GithubCredentialsProvider,
@@ -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({
@@ -351,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 NotFoundError(message);
}
}
}
@@ -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);
@@ -518,6 +549,30 @@ describe('publish:github', () => {
});
});
it('should provide an adequate failure message when adding access', async () => {
mockOctokit.rest.users.getByUsername.mockResolvedValue({
data: { type: 'User' },
});
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 () => {
mockOctokit.rest.users.getByUsername.mockResolvedValue({
data: { type: 'User' },