paulononato/backstage
1
0
Fork 0
Code Issues Pull Requests Actions 32 Packages Projects Releases Wiki Activity

chore: address comments

Browse Source
This commit is contained in:
Fredrik Adelöw
2020-09-30 13:43:25 +02:00
parent c6ba9cba49
commit 7ec6dc7f30
27 changed files with 608 additions and 413 deletions
Download Patch File Download Diff File Expand all files Collapse all files
plugins/catalog-backend/src/ingestion/LocationReaders.ts
+11 -7
View File
@@ -179,12 +179,14 @@ export class LocationReaders implements LocationReader {
} catch (e) {
const message = `Processor ${processor.constructor.name} threw an error while reading location ${item.location.type} ${item.location.target}, ${e}`;
emit(result.generalError(item.location, message));
this.logger.warn(message);
}
}
}
const message = `No processor was able to read location ${item.location.type} ${item.location.target}`;
emit(result.inputError(item.location, message));
this.logger.warn(message);
}
private async handleData(
@@ -204,6 +206,7 @@ export class LocationReaders implements LocationReader {
} catch (e) {
const message = `Processor ${processor.constructor.name} threw an error while parsing ${item.location.type} ${item.location.target}, ${e}`;
emit(result.generalError(item.location, message));
this.logger.warn(message);
}
}
}
@@ -232,13 +235,13 @@ export class LocationReaders implements LocationReader {
this.readLocation.bind(this),
);
} catch (e) {
const message = `Processor ${
processor.constructor.name
} threw an error while processing entity ${current.kind}:${
current.metadata.namespace ?? ENTITY_DEFAULT_NAMESPACE
}/${current.metadata.name} at ${item.location.type} ${
item.location.target
}, ${e}`;
// Construct the name carefully, if we got validation errors we do
// not want to crash here due to missing metadata or so
const namespace = !current.metadata
? ''
: current.metadata.namespace ?? ENTITY_DEFAULT_NAMESPACE;
const name = !current.metadata ? '' : current.metadata.name;
const message = `Processor ${processor.constructor.name} threw an error while processing entity ${current.kind}:${namespace}/${name} at ${item.location.type} ${item.location.target}, ${e}`;
emit(result.generalError(item.location, message));
this.logger.warn(message);
}
@@ -263,6 +266,7 @@ export class LocationReaders implements LocationReader {
} catch (e) {
const message = `Processor ${processor.constructor.name} threw an error while handling another error at ${item.location.type} ${item.location.target}, ${e}`;
emit(result.generalError(item.location, message));
this.logger.warn(message);
}
}
}
plugins/catalog-backend/src/ingestion/processors/EntityPolicyProcessor.ts
+1 -1
View File
@@ -27,7 +27,7 @@ export class EntityPolicyProcessor implements LocationProcessor {
async processEntity(entity: Entity): Promise<Entity> {
const output = await this.policy.enforce(entity);
if (!output) {
throw new Error(`No policy applied to entity`);
throw new Error(`Entity did not match any known schema`);
}
return output;
}
plugins/catalog-backend/src/ingestion/processors/GithubOrgReaderProcessor.ts
+3 -13
View File
@@ -152,19 +152,9 @@ export function parseUrl(urlString: string): { org: string } {
const path = new URL(urlString).pathname.substr(1).split('/');
// /spotify
if (path.length === 1) {
return { org: path[0] };
if (path.length === 1 && path[0].length) {
return { org: decodeURIComponent(path[0]) };
}
// /orgs/spotify[/<teams or people>]
if (
path.length >= 2 &&
path.length <= 3 &&
path[0] === 'orgs' &&
[undefined, 'teams', 'people'].includes(path[2])
) {
return { org: path[1] };
}
throw new Error(`Expected a URL pointing to /<org> or /orgs/<org>`);
throw new Error(`Expected a URL pointing to /<org>`);
}
plugins/catalog-backend/src/ingestion/processors/UrlReaderProcessor.test.ts
+1 -1
View File
@@ -27,7 +27,7 @@ describe('UrlReaderProcessor', () => {
const mockApiOrigin = 'http://localhost:23000';
const server = setupServer();
beforeAll(() => server.listen());
beforeAll(() => server.listen({ onUnhandledRequest: 'error' }));
afterEach(() => server.resetHandlers());
afterAll(() => server.close());
plugins/catalog-backend/src/ingestion/processors/util/github.test.ts
+147
View File
@@ -0,0 +1,147 @@
/*
* Copyright 2020 Spotify AB
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { graphql } from '@octokit/graphql';
import { graphql as graphqlMsw } from 'msw';
import { setupServer } from 'msw/node';
import {
getOrganizationTeams,
getOrganizationUsers,
getTeamMembers,
QueryResponse,
} from './github';
describe('github', () => {
const server = setupServer();
beforeAll(() => server.listen({ onUnhandledRequest: 'error' }));
afterEach(() => server.resetHandlers());
afterAll(() => server.close());
describe('getOrganizationUsers', () => {
it('reads members', async () => {
const input: QueryResponse = {
organization: {
membersWithRole: {
pageInfo: { hasNextPage: false },
nodes: [
{
login: 'a',
name: 'b',
bio: 'c',
email: 'd',
avatarUrl: 'e',
},
],
},
},
};
const output = {
users: [
expect.objectContaining({
metadata: expect.objectContaining({ name: 'a', description: 'c' }),
spec: {
profile: { displayName: 'b', email: 'd', picture: 'e' },
memberOf: [],
},
}),
],
};
server.use(
graphqlMsw.query('users', (_req, res, ctx) => res(ctx.data(input))),
);
await expect(getOrganizationUsers(graphql, 'a')).resolves.toEqual(output);
});
});
describe('getOrganizationTeams', () => {
it('reads teams', async () => {
const input: QueryResponse = {
organization: {
teams: {
pageInfo: { hasNextPage: false },
nodes: [
{
slug: 'team',
combinedSlug: 'blah/team',
parentTeam: {
slug: 'parent',
combinedSlug: '',
members: { pageInfo: { hasNextPage: false }, nodes: [] },
},
members: {
pageInfo: { hasNextPage: false },
nodes: [{ login: 'user' }],
},
},
],
},
},
};
const output = {
groups: [
expect.objectContaining({
metadata: expect.objectContaining({ name: 'team' }),
spec: {
type: 'team',
parent: 'parent',
ancestors: [],
children: [],
descendants: [],
},
}),
],
groupMemberUsers: new Map([['team', ['user']]]),
};
server.use(
graphqlMsw.query('teams', (_req, res, ctx) => res(ctx.data(input))),
);
await expect(getOrganizationTeams(graphql, 'a')).resolves.toEqual(output);
});
});
describe('getTeamMembers', () => {
it('reads team members', async () => {
const input: QueryResponse = {
organization: {
team: {
slug: '',
combinedSlug: '',
members: {
pageInfo: { hasNextPage: false },
nodes: [{ login: 'user' }],
},
},
},
};
const output = {
members: ['user'],
};
server.use(
graphqlMsw.query('members', (_req, res, ctx) => res(ctx.data(input))),
);
await expect(getTeamMembers(graphql, 'a', 'b')).resolves.toEqual(output);
});
});
});
plugins/catalog-backend/src/ingestion/processors/util/github.ts
+153 -141
View File
@@ -19,22 +19,22 @@ import { graphql } from '@octokit/graphql';
// Graphql types
type QueryResponse = {
export type QueryResponse = {
organization: Organization;
};
type Organization = {
membersWithRole: Connection<User>;
team: Team;
teams: Connection<Team>;
export type Organization = {
membersWithRole?: Connection<User>;
team?: Team;
teams?: Connection<Team>;
};
type PageInfo = {
export type PageInfo = {
hasNextPage: boolean;
endCursor?: string;
};
type User = {
export type User = {
login: string;
bio?: string;
avatarUrl?: string;
@@ -42,7 +42,7 @@ type User = {
name?: string;
};
type Team = {
export type Team = {
slug: string;
combinedSlug: string;
description?: string;
@@ -50,7 +50,7 @@ type Team = {
members: Connection<User>;
};
type Connection<T> = {
export type Connection<T> = {
pageInfo: PageInfo;
nodes: T[];
};
@@ -67,63 +67,49 @@ export async function getOrganizationUsers(
client: typeof graphql,
org: string,
): Promise<{ users: UserEntity[] }> {
const users: UserEntity[] = [];
const query = `
query users($org: String!, $cursor: String) {
organization(login: $org) {
membersWithRole(first: 100, after: $cursor) {
pageInfo { hasNextPage, endCursor }
nodes { avatarUrl, bio, email, login, name }
}
}
}`;
// There is no user -> teams edge, so we leave the memberships empty for
// now and let the team iteration handle it instead
let cursor: string | undefined = undefined;
const query = `
query users($org: String!, $cursor: String) {
organization(login: $org) {
membersWithRole(first: 100, after: $cursor) {
pageInfo { hasNextPage, endCursor }
nodes { avatarUrl, bio, email, login, name }
}
}
}`;
for (let i = 0; i < 100 /* just for sanity */; ++i) {
const response: QueryResponse = await client(query, {
org,
cursor,
});
const connection = response.organization?.membersWithRole;
if (!connection) {
throw new Error(`Found no organization named ${org}`);
}
for (const user of connection.nodes) {
const entity: UserEntity = {
apiVersion: 'backstage.io/v1alpha1',
kind: 'User',
metadata: {
name: user.login,
annotations: {
'github.com/user-login': user.login,
},
const mapper = (user: User) => {
const entity: UserEntity = {
apiVersion: 'backstage.io/v1alpha1',
kind: 'User',
metadata: {
name: user.login,
annotations: {
'github.com/user-login': user.login,
},
spec: {
profile: {},
memberOf: [],
},
};
},
spec: {
profile: {},
memberOf: [],
},
};
if (user.bio) entity.metadata.description = user.bio;
if (user.name) entity.spec.profile!.displayName = user.name;
if (user.email) entity.spec.profile!.email = user.email;
if (user.avatarUrl) entity.spec.profile!.picture = user.avatarUrl;
if (user.bio) entity.metadata.description = user.bio;
if (user.name) entity.spec.profile!.displayName = user.name;
if (user.email) entity.spec.profile!.email = user.email;
if (user.avatarUrl) entity.spec.profile!.picture = user.avatarUrl;
users.push(entity);
}
return entity;
};
const { hasNextPage, endCursor } = connection.pageInfo;
if (!hasNextPage) {
break;
} else {
cursor = endCursor;
}
}
const users = await queryWithPaging(
client,
query,
r => r.organization?.membersWithRole,
mapper,
{ org },
);
return { users };
}
@@ -143,10 +129,6 @@ export async function getOrganizationTeams(
groups: GroupEntity[];
groupMemberUsers: Map<string, string[]>;
}> {
const groups: GroupEntity[] = [];
const groupMemberUsers = new Map<string, string[]>();
let cursor: string | undefined = undefined;
const query = `
query teams($org: String!, $cursor: String) {
organization(login: $org) {
@@ -157,7 +139,7 @@ export async function getOrganizationTeams(
combinedSlug
parentTeam { slug }
members(first: 100, membership: IMMEDIATE) {
pageInfo { hasNextPage, endCursor }
pageInfo { hasNextPage }
nodes { login }
}
}
@@ -165,65 +147,57 @@ export async function getOrganizationTeams(
}
}`;
for (let i = 0; i < 100 /* just for sanity */; ++i) {
const response: QueryResponse = await client(query, {
org,
cursor,
});
// Gets populated inside the mapper below
const groupMemberUsers = new Map<string, string[]>();
const connection = response.organization?.teams;
if (!connection) {
throw new Error(`Found no organization named ${org}`);
}
for (const team of connection.nodes) {
const entity: GroupEntity = {
apiVersion: 'backstage.io/v1alpha1',
kind: 'Group',
metadata: {
name: team.slug,
annotations: {
'github.com/team-slug': team.combinedSlug,
},
const mapper = async (team: Team) => {
const entity: GroupEntity = {
apiVersion: 'backstage.io/v1alpha1',
kind: 'Group',
metadata: {
name: team.slug,
annotations: {
'github.com/team-slug': team.combinedSlug,
},
spec: {
type: 'team',
ancestors: [],
children: [],
descendants: [],
},
};
},
spec: {
type: 'team',
ancestors: [],
children: [],
descendants: [],
},
};
if (team.description) entity.metadata.description = team.description;
if (team.parentTeam) entity.spec.parent = team.parentTeam.slug;
if (team.description) entity.metadata.description = team.description;
if (team.parentTeam) entity.spec.parent = team.parentTeam.slug;
groups.push(entity);
const memberNames: string[] = [];
groupMemberUsers.set(team.slug, memberNames);
const memberNames: string[] = [];
groupMemberUsers.set(team.slug, memberNames);
if (!team.members.pageInfo.hasNextPage) {
// We got all the members in one go, run the fast path
for (const user of team.members.nodes) {
memberNames.push(user.login);
}
} else {
// There were more than a hundred immediate members - run the slow
// path of fetching them explicitly
const { members } = await getTeamMembers(client, org, team.slug);
for (const userLogin of members) {
memberNames.push(userLogin);
}
if (!team.members.pageInfo.hasNextPage) {
// We got all the members in one go, run the fast path
for (const user of team.members.nodes) {
memberNames.push(user.login);
}
} else {
// There were more than a hundred immediate members - run the slow
// path of fetching them explicitly
const { members } = await getTeamMembers(client, org, team.slug);
for (const userLogin of members) {
memberNames.push(userLogin);
}
}
const { hasNextPage, endCursor } = connection.pageInfo;
if (!hasNextPage) {
break;
} else {
cursor = endCursor;
}
}
return entity;
};
const groups = await queryWithPaging(
client,
query,
r => r.organization?.teams,
mapper,
{ org },
);
return { groups, groupMemberUsers };
}
@@ -242,9 +216,6 @@ export async function getTeamMembers(
org: string,
teamSlug: string,
): Promise<{ members: string[] }> {
const members: string[] = [];
let cursor: string | undefined = undefined;
const query = `
query members($org: String!, $teamSlug: String!, $cursor: String) {
organization(login: $org) {
@@ -257,29 +228,70 @@ export async function getTeamMembers(
}
}`;
for (let j = 0; j < 100 /* just for sanity */; ++j) {
const response: QueryResponse = await client(query, {
org,
teamSlug,
cursor,
});
const connection = response.organization?.team?.members;
if (!connection) {
throw new Error(`Found no team named ${teamSlug} in named ${org}`);
}
for (const user of connection.nodes) {
members.push(user.login);
}
const { hasNextPage, endCursor } = connection.pageInfo;
if (!hasNextPage) {
break;
} else {
cursor = endCursor;
}
}
const members = await queryWithPaging(
client,
query,
r => r.organization?.team?.members,
user => user.login,
{ org, teamSlug },
);
return { members };
}
//
// Helpers
//
/**
* Assists in repeatedly executing a query with a paged response.
*
* Requires that the query accepts a $cursor variable.
*
* @param client The octokit client
* @param query The query to execute
* @param connection A function that, given the response, picks out the actual
* Connection object that's being iterated
* @param mapper A function that, given one of the nodes in the Connection,
* returns the model mapped form of it
* @param variables The variable values that the query needs, minus the cursor
*/
export async function queryWithPaging<
GraphqlType,
OutputType,
Variables extends {},
Response = QueryResponse
>(
client: typeof graphql,
query: string,
connection: (response: Response) => Connection<GraphqlType> | undefined,
mapper: (item: GraphqlType) => Promise<OutputType> | OutputType,
variables: Variables,
): Promise<OutputType[]> {
const result: OutputType[] = [];
let cursor: string | undefined = undefined;
for (let j = 0; j < 1000 /* just for sanity */; ++j) {
const response: Response = await client(query, {
...variables,
cursor,
});
const conn = connection(response);
if (!conn) {
throw new Error(`Found no match for ${JSON.stringify(variables)}`);
}
for (const node of conn.nodes) {
result.push(await mapper(node));
}
if (!conn.pageInfo.hasNextPage) {
break;
} else {
cursor = conn.pageInfo.endCursor;
}
}
return result;
}
plugins/catalog-backend/src/ingestion/processors/util/org.test.ts
+92
View File
@@ -0,0 +1,92 @@
/*
* Copyright 2020 Spotify AB
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { GroupEntity, UserEntity } from '@backstage/catalog-model';
import { buildOrgHierarchy } from './org';
function u(name: string): UserEntity {
return {
apiVersion: 'backstage.io/v1alpha1',
kind: 'User',
metadata: { name },
spec: { memberOf: [] },
};
}
function g(
name: string,
parent: string | undefined,
children: string[],
): GroupEntity {
return {
apiVersion: 'backstage.io/v1alpha1',
kind: 'Group',
metadata: { name },
spec: { type: 'team', parent, children, ancestors: [], descendants: [] },
};
}
describe('buildOrgHierarchy', () => {
it('puts users in the respective groups', () => {
const a = g('a', undefined, []);
const b = g('b', undefined, []);
const x = u('x');
const y = u('y');
const groupMemberUsers: Map<string, string[]> = new Map([
['a', ['x', 'y']],
['b', ['y']],
]);
buildOrgHierarchy([a, b], [x, y], groupMemberUsers);
expect(x.spec.memberOf).toEqual(['a']);
expect(y.spec.memberOf).toEqual(['a', 'b']);
});
it('adds groups to their parent.children', () => {
const a = g('a', undefined, []);
const b = g('b', 'a', []);
const c = g('c', 'b', []);
const d = g('d', 'a', []);
buildOrgHierarchy([a, b, c, d], [], new Map());
expect(a.spec.children).toEqual(expect.arrayContaining(['b', 'd']));
expect(b.spec.children).toEqual(expect.arrayContaining(['c']));
expect(c.spec.children).toEqual([]);
expect(d.spec.children).toEqual([]);
});
it('fills out descendants', () => {
const a = g('a', undefined, []);
const b = g('b', 'a', []);
const c = g('c', 'b', []);
const d = g('d', 'a', []);
buildOrgHierarchy([a, b, c, d], [], new Map());
expect(a.spec.descendants).toEqual(expect.arrayContaining(['b', 'c', 'd']));
expect(b.spec.descendants).toEqual(expect.arrayContaining(['c']));
expect(c.spec.descendants).toEqual([]);
expect(d.spec.descendants).toEqual([]);
});
it('fills out ancestors', () => {
const a = g('a', undefined, []);
const b = g('b', 'a', []);
const c = g('c', 'b', []);
const d = g('d', 'a', []);
buildOrgHierarchy([a, b, c, d], [], new Map());
expect(a.spec.ancestors).toEqual([]);
expect(b.spec.ancestors).toEqual(expect.arrayContaining(['a']));
expect(c.spec.ancestors).toEqual(expect.arrayContaining(['a', 'b']));
expect(d.spec.ancestors).toEqual(expect.arrayContaining(['a']));
});
});
plugins/catalog-graphql/src/graphql/module.test.ts
+2 -2
View File
@@ -13,6 +13,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { createModule } from './module';
import { execute } from 'graphql';
import { rest } from 'msw';
@@ -36,9 +37,8 @@ describe('Catalog Module', () => {
},
]);
beforeAll(() => worker.listen());
beforeAll(() => worker.listen({ onUnhandledRequest: 'error' }));
afterAll(() => worker.close());
afterEach(() => worker.resetHandlers());
describe('Default Entity', () => {
plugins/catalog-graphql/src/service/client.test.ts
+1 -2
View File
@@ -20,9 +20,8 @@ import { setupServer } from 'msw/node';
describe('Catalog GraphQL Module', () => {
const worker = setupServer();
beforeAll(() => worker.listen());
beforeAll(() => worker.listen({ onUnhandledRequest: 'error' }));
afterAll(() => worker.close());
afterEach(() => worker.resetHandlers());
const baseUrl = 'http://localhost:1234';
plugins/catalog/src/api/CatalogClient.test.ts
+1 -1
View File
@@ -25,7 +25,7 @@ const mockBaseUrl = 'http://backstage:9191/i-am-a-mock-base';
const discoveryApi = UrlPatternDiscovery.compile(mockBaseUrl);
describe('CatalogClient', () => {
beforeAll(() => server.listen());
beforeAll(() => server.listen({ onUnhandledRequest: 'error' }));
afterEach(() => server.resetHandlers());
afterAll(() => server.close());