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

catalog: remove group ancestors and descendants

Browse Source
This commit is contained in:
Fredrik Adelöw
2020-11-20 16:30:57 +01:00
parent f76626c1c4
commit 83b6e0c1fc
22 changed files with 45 additions and 286 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.changeset/silent-boxes-exercise.md
+8
View File
@@ -0,0 +1,8 @@
---
'@backstage/catalog-model': minor
'@backstage/plugin-catalog-backend': minor
---
Remove the deprecated fields `ancestors` and `descendants` from the `Group` entity.
See https://github.com/backstage/backstage/issues/3049 and the PRs linked from it for details.
docs/features/software-catalog/descriptor-format.md
-40
View File
@@ -723,9 +723,7 @@ metadata:
spec:
type: business-unit
parent: ops
ancestors: [ops, global-synergies, acme-corp]
children: [backstage, other]
descendants: [backstage, other, team-a, team-b, team-c, team-d]
```
In addition to the [common envelope metadata](#common-to-all-kinds-the-metadata)
@@ -761,25 +759,6 @@ namespace as the user. Only `Group` entities may be referenced. Most commonly,
this field points to a group in the same namespace, so in those cases it is
sufficient to enter only the `metadata.name` field of that group.
### `spec.ancestors` [required]
**NOTE**: This field was marked for deprecation on Nov 22nd, 2020. It will be
removed entirely from the model on Dec 6th, 2020 in the repository and will not
be present in released packages following the next release after that. Please
update your code to not consume this field before the removal date.
The recursive list of parents up the hierarchy, by stepping through parents one
by one. The list must be present, but may be empty if `parent` is not present.
The first entry in the list is equal to `parent`, and then the following ones
are progressively farther up the hierarchy.
The entries of this array are
[entity references](https://backstage.io/docs/features/software-catalog/references),
with the default kind `Group` and the default namespace equal to the same
namespace as the user. Only `Group` entities may be referenced. Most commonly,
these entries point to groups in the same namespace, so in those cases it is
sufficient to enter only the `metadata.name` field of those groups.
### `spec.children` [required]
The immediate child groups of this group in the hierarchy (whose `parent` field
@@ -794,25 +773,6 @@ namespace as the user. Only `Group` entities may be referenced. Most commonly,
these entries point to groups in the same namespace, so in those cases it is
sufficient to enter only the `metadata.name` field of those groups.
### `spec.descendants` [required]
**NOTE**: This field was marked for deprecation on Nov 22nd, 2020. It will be
removed entirely from the model on Dec 6th, 2020 in the repository and will not
be present in released packages following the next release after that. Please
update your code to not consume this field before the removal date.
The immediate and recursive child groups of this group in the hierarchy
(children, and children's children, etc.). The list must be present, but may be
empty if there are no child groups. The items are not guaranteed to be ordered
in any particular way.
The entries of this array are
[entity references](https://backstage.io/docs/features/software-catalog/references),
with the default kind `Group` and the default namespace equal to the same
namespace as the user. Only `Group` entities may be referenced. Most commonly,
these entries point to groups in the same namespace, so in those cases it is
sufficient to enter only the `metadata.name` field of those groups.
## Kind: User
Describes the following entity kind:
packages/catalog-model/examples/acme/backstage-group.yaml
-2
View File
@@ -6,6 +6,4 @@ metadata:
spec:
type: sub-department
parent: infrastructure
ancestors: [infrastructure, acme-corp]
children: [team-a, team-b]
descendants: [team-a, team-b]
packages/catalog-model/examples/acme/boxoffice-group.yaml
-2
View File
@@ -6,6 +6,4 @@ metadata:
spec:
type: sub-department
parent: infrastructure
ancestors: [infrastructure, acme-corp]
children: [team-c, team-d]
descendants: [team-c, team-d]
packages/catalog-model/examples/acme/infrastructure-group.yaml
-2
View File
@@ -6,6 +6,4 @@ metadata:
spec:
type: department
parent: acme-corp
ancestors: [acme-corp]
children: [backstage, boxoffice]
descendants: [backstage, boxoffice, team-a, team-b, team-c, team-d]
packages/catalog-model/examples/acme/org.yaml
-3
View File
@@ -5,10 +5,7 @@ metadata:
description: The acme-corp organization
spec:
type: organization
ancestors: []
children: [infrastructure]
descendants:
[infrastructure, backstage, boxoffice, team-a, team-b, team-c, team-d]
---
apiVersion: backstage.io/v1alpha1
kind: Location
packages/catalog-model/examples/acme/team-a-group.yaml
-2
View File
@@ -6,9 +6,7 @@ metadata:
spec:
type: team
parent: backstage
ancestors: [backstage, infrastructure, acme-corp]
children: []
descendants: []
---
apiVersion: backstage.io/v1alpha1
kind: User
packages/catalog-model/examples/acme/team-b-group.yaml
-2
View File
@@ -6,9 +6,7 @@ metadata:
spec:
type: team
parent: backstage
ancestors: [backstage, infrastructure, acme-corp]
children: []
descendants: []
---
apiVersion: backstage.io/v1alpha1
kind: User
packages/catalog-model/examples/acme/team-c-group.yaml
-2
View File
@@ -6,9 +6,7 @@ metadata:
spec:
type: team
parent: boxoffice
ancestors: [boxoffice, infrastructure, acme-corp]
children: []
descendants: []
---
apiVersion: backstage.io/v1alpha1
kind: User
packages/catalog-model/examples/acme/team-d-group.yaml
-2
View File
@@ -6,9 +6,7 @@ metadata:
spec:
type: team
parent: boxoffice
ancestors: [boxoffice, infrastructure, acme-corp]
children: []
descendants: []
---
apiVersion: backstage.io/v1alpha1
kind: User
packages/catalog-model/src/kinds/GroupEntityV1alpha1.test.ts
-42
View File
@@ -34,9 +34,7 @@ describe('GroupV1alpha1Validator', () => {
spec: {
type: 'squad',
parent: 'group-a',
ancestors: ['group-a', 'global-synergies', 'acme-corp'],
children: ['child-a', 'child-b'],
descendants: ['desc-a', 'desc-b'],
},
};
});
@@ -85,26 +83,6 @@ describe('GroupV1alpha1Validator', () => {
await expect(validator.check(entity)).rejects.toThrow(/parent/);
});
it('rejects missing ancestors', async () => {
delete (entity as any).spec.ancestors;
await expect(validator.check(entity)).rejects.toThrow(/ancestor/);
});
it('rejects empty ancestors', async () => {
(entity as any).spec.ancestors = [''];
await expect(validator.check(entity)).rejects.toThrow(/ancestor/);
});
it('rejects undefined ancestors', async () => {
(entity as any).spec.ancestors = [undefined];
await expect(validator.check(entity)).rejects.toThrow(/ancestor/);
});
it('accepts no ancestors', async () => {
(entity as any).spec.ancestors = [];
await expect(validator.check(entity)).resolves.toBe(true);
});
it('rejects missing children', async () => {
delete (entity as any).spec.children;
await expect(validator.check(entity)).rejects.toThrow(/children/);
@@ -124,24 +102,4 @@ describe('GroupV1alpha1Validator', () => {
(entity as any).spec.children = [];
await expect(validator.check(entity)).resolves.toBe(true);
});
it('rejects missing descendants', async () => {
delete (entity as any).spec.descendants;
await expect(validator.check(entity)).rejects.toThrow(/descendants/);
});
it('rejects empty descendants', async () => {
(entity as any).spec.descendants = [''];
await expect(validator.check(entity)).rejects.toThrow(/descendants/);
});
it('rejects undefined descendants', async () => {
(entity as any).spec.descendants = [undefined];
await expect(validator.check(entity)).rejects.toThrow(/descendants/);
});
it('accepts no descendants', async () => {
(entity as any).spec.descendants = [];
await expect(validator.check(entity)).resolves.toBe(true);
});
});
packages/catalog-model/src/kinds/GroupEntityV1alpha1.ts
-26
View File
@@ -32,21 +32,11 @@ const schema = yup.object<Partial<GroupEntityV1alpha1>>({
// one element and there is no simple workaround -_-
// the cast is there to convince typescript that the array itself is
// required without using .required()
ancestors: yup.array(yup.string().required()).test({
name: 'isDefined',
message: 'ancestors must be defined',
test: v => Boolean(v),
}) as yup.ArraySchema<string, object>,
children: yup.array(yup.string().required()).test({
name: 'isDefined',
message: 'children must be defined',
test: v => Boolean(v),
}) as yup.ArraySchema<string, object>,
descendants: yup.array(yup.string().required()).test({
name: 'isDefined',
message: 'descendants must be defined',
test: v => Boolean(v),
}) as yup.ArraySchema<string, object>,
})
.required(),
});
@@ -57,23 +47,7 @@ export interface GroupEntityV1alpha1 extends Entity {
spec: {
type: string;
parent?: string;
/**
* @deprecated This field will disappear on Dec 6th, 2020. Please remove
* any consuming code. Producers can stop producing this field
* before that date, as long as the catalog backend uses the
* BuiltinKindsEntityProcessor which inserts the fields in the
* mean time.
*/
ancestors: string[];
children: string[];
/**
* @deprecated This field will disappear on Dec 6th, 2020. Please remove
* any consuming code. Producers can stop producing this field
* before that date, as long as the catalog backend uses the
* BuiltinKindsEntityProcessor which inserts the fields in the
* mean time.
*/
descendants: string[];
};
}
plugins/catalog-backend/src/ingestion/processors/BuiltinKindsEntityProcessor.test.ts
-30
View File
@@ -23,34 +23,6 @@ import {
import { BuiltinKindsEntityProcessor } from './BuiltinKindsEntityProcessor';
describe('BuiltinKindsEntityProcessor', () => {
it('fills in fields for #3049', async () => {
const p = new BuiltinKindsEntityProcessor();
const result = await p.preProcessEntity({
apiVersion: 'backstage.io/v1alpha1',
kind: 'Group',
metadata: {
name: 'n',
},
spec: {
type: 't',
children: [],
} as any,
});
expect(result).toEqual({
apiVersion: 'backstage.io/v1alpha1',
kind: 'Group',
metadata: {
name: 'n',
},
spec: {
type: 't',
children: [],
ancestors: [],
descendants: [],
},
});
});
describe('postProcessEntity', () => {
const processor = new BuiltinKindsEntityProcessor();
const location = { type: 'a', target: 'b' };
@@ -215,9 +187,7 @@ describe('BuiltinKindsEntityProcessor', () => {
spec: {
type: 't',
parent: 'p',
ancestors: [],
children: ['c'],
descendants: [],
},
};
plugins/catalog-backend/src/ingestion/processors/BuiltinKindsEntityProcessor.ts
-19
View File
@@ -53,25 +53,6 @@ export class BuiltinKindsEntityProcessor implements CatalogProcessor {
userEntityV1alpha1Validator,
];
async preProcessEntity(entity: Entity): Promise<Entity> {
// NOTE(freben): Part of Group field deprecation on Nov 22nd, 2020. Fields
// scheduled for removal Dec 6th, 2020. This code can be deleted after that
// point. See https://github.com/backstage/backstage/issues/3049
if (
entity.apiVersion === 'backstage.io/v1alpha1' &&
entity.kind === 'Group' &&
entity.spec
) {
if (!entity.spec.ancestors) {
entity.spec.ancestors = [];
}
if (!entity.spec.descendants) {
entity.spec.descendants = [];
}
}
return entity;
}
async validateEntityKind(entity: Entity): Promise<boolean> {
for (const validator of this.validators) {
const result = await validator.check(entity);
plugins/catalog-backend/src/ingestion/processors/ldap/read.test.ts
+1 -3
View File
@@ -47,7 +47,7 @@ function group(data: RecursivePartial<GroupEntity>): GroupEntity {
apiVersion: 'backstage.io/v1alpha1',
kind: 'Group',
metadata: { name: 'name' },
spec: { type: 'type', ancestors: [], children: [], descendants: [] },
spec: { type: 'type', children: [] },
} as GroupEntity,
data,
);
@@ -173,9 +173,7 @@ describe('readLdapGroups', () => {
},
spec: {
type: 'type-value',
ancestors: [],
children: [],
descendants: [],
},
}),
]);
plugins/catalog-backend/src/ingestion/processors/ldap/read.ts
-2
View File
@@ -150,9 +150,7 @@ export async function readLdapGroups(
},
spec: {
type: 'unknown',
ancestors: [],
children: [],
descendants: [],
},
};
plugins/catalog-backend/src/ingestion/processors/microsoftGraph/read.test.ts
-15
View File
@@ -49,9 +49,7 @@ function group(data: RecursivePartial<GroupEntity>): GroupEntity {
name: 'name',
},
spec: {
ancestors: [],
children: [],
descendants: [],
type: 'team',
},
} as GroupEntity,
@@ -306,33 +304,20 @@ describe('read microsoft graph', () => {
resolveRelations(rootGroup, groups, users, groupMember, groupMemberOf);
expect(rootGroup.spec.parent).toBeUndefined();
expect(rootGroup.spec.ancestors).toEqual(expect.arrayContaining([]));
expect(rootGroup.spec.children).toEqual(
expect.arrayContaining(['a', 'b']),
);
expect(rootGroup.spec.descendants).toEqual(
expect.arrayContaining(['a', 'b', 'c']),
);
expect(groupA.spec.parent).toEqual('root');
expect(groupA.spec.ancestors).toEqual(expect.arrayContaining(['root']));
expect(groupA.spec.children).toEqual(expect.arrayContaining([]));
expect(groupA.spec.descendants).toEqual(expect.arrayContaining([]));
expect(groupB.spec.parent).toEqual('root');
expect(groupB.spec.ancestors).toEqual(expect.arrayContaining(['root']));
expect(groupB.spec.children).toEqual(expect.arrayContaining(['c']));
expect(groupB.spec.descendants).toEqual(expect.arrayContaining(['c']));
expect(groupC.spec.parent).toEqual('b');
expect(groupC.spec.ancestors).toEqual(
expect.arrayContaining(['root', 'b']),
);
expect(groupC.spec.children).toEqual(expect.arrayContaining([]));
expect(groupC.spec.descendants).toEqual(expect.arrayContaining([]));
expect(user1.spec.memberOf).toEqual(expect.arrayContaining(['a']));
expect(user2.spec.memberOf).toEqual(expect.arrayContaining(['b', 'c']));
});
});
plugins/catalog-backend/src/ingestion/processors/microsoftGraph/read.ts
+3 -7
View File
@@ -14,6 +14,7 @@
* limitations under the License.
*/
import { GroupEntity, UserEntity } from '@backstage/catalog-model';
import limiterFactory from 'p-limit';
import { buildMemberOf, buildOrgHierarchy } from '../util/org';
import { MicrosoftGraphClient } from './client';
import {
@@ -21,7 +22,6 @@ import {
MICROSOFT_GRAPH_TENANT_ID_ANNOTATION,
MICROSOFT_GRAPH_USER_ID_ANNOTATION,
} from './constants';
import limiterFactory from 'p-limit';
export function normalizeEntityName(name: string): string {
return name
@@ -98,7 +98,7 @@ export async function readMicrosoftGraphOrganization(
): Promise<{
rootGroup: GroupEntity; // With all relations empty
}> {
// For now we expect a single root orgranization
// For now we expect a single root organization
const organization = await client.getOrganization(tenantId);
const name = normalizeEntityName(organization.displayName!);
const rootGroup: GroupEntity = {
@@ -113,9 +113,7 @@ export async function readMicrosoftGraphOrganization(
},
spec: {
type: 'root',
ancestors: [],
children: [],
descendants: [],
},
};
@@ -165,9 +163,7 @@ export async function readMicrosoftGraphGroups(
spec: {
type: 'team',
// TODO: We could include a group email and picture
ancestors: [],
children: [],
descendants: [],
},
};
@@ -278,7 +274,7 @@ export function resolveRelations(
});
});
// Make sure that all groups have proper ancestors and descendants
// Make sure that all groups have proper parents and children
buildOrgHierarchy(groups);
// Set relations for all users
plugins/catalog-backend/src/ingestion/processors/util/github.test.ts
-2
View File
@@ -100,9 +100,7 @@ describe('github', () => {
spec: {
type: 'team',
parent: 'parent',
ancestors: [],
children: [],
descendants: [],
},
}),
],
plugins/catalog-backend/src/ingestion/processors/util/github.ts
-2
View File
@@ -162,9 +162,7 @@ export async function getOrganizationTeams(
},
spec: {
type: 'team',
ancestors: [],
children: [],
descendants: [],
},
};
plugins/catalog-backend/src/ingestion/processors/util/org.test.ts
+11 -23
View File
@@ -26,7 +26,7 @@ function g(
apiVersion: 'backstage.io/v1alpha1',
kind: 'Group',
metadata: { name },
spec: { type: 'team', parent, children, ancestors: [], descendants: [] },
spec: { type: 'team', parent, children },
};
}
@@ -43,28 +43,16 @@ describe('buildOrgHierarchy', () => {
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', []);
it('sets parent of groups children', () => {
const a = g('a', undefined, ['b', 'd']);
const b = g('b', undefined, ['c']);
const c = g('c', undefined, []);
const d = g('d', undefined, []);
buildOrgHierarchy([a, b, c, d]);
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]);
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']));
expect(a.spec.parent).toBeUndefined();
expect(b.spec.parent).toBe('a');
expect(c.spec.parent).toBe('b');
expect(d.spec.parent).toBe('a');
});
});
@@ -76,7 +64,7 @@ describe('buildMemberOf', () => {
const u: UserEntity = {
apiVersion: 'backstage.io/v1alpha1',
kind: 'User',
metadata: { name },
metadata: { name: 'n' },
spec: { profile: {}, memberOf: ['c'] },
};
plugins/catalog-backend/src/ingestion/processors/util/org.ts
+22 -58
View File
@@ -35,62 +35,17 @@ export function buildOrgHierarchy(groups: GroupEntity[]) {
}
//
// Make sure that g.descendants is complete
// Make sure that g.children.parent is g
//
function visitDescendants(current: GroupEntity): string[] {
if (current.spec.descendants.length) {
return current.spec.descendants;
}
const accumulator = new Set<string>();
for (const childName of current.spec.children) {
accumulator.add(childName);
for (const group of groups) {
const selfName = group.metadata.name;
for (const childName of group.spec.children) {
const child = groupsByName.get(childName);
if (child) {
for (const d of visitDescendants(child)) {
accumulator.add(d);
}
if (child && !child.spec.parent) {
child.spec.parent = selfName;
}
}
const descendants = Array.from(accumulator);
current.spec.descendants = descendants;
return descendants;
}
for (const group of groups) {
visitDescendants(group);
}
//
// Make sure that g.ancestors is complete
//
function visitAncestors(current: GroupEntity): string[] {
if (current.spec.ancestors.length) {
return current.spec.ancestors;
}
let ancestors: string[];
const parentName = current.spec.parent;
if (!parentName) {
ancestors = [];
} else {
const parent = groupsByName.get(parentName);
if (parent) {
ancestors = [parentName, ...visitAncestors(parent)];
} else {
ancestors = [parentName];
}
}
current.spec.ancestors = ancestors;
return ancestors;
}
for (const group of groups) {
visitAncestors(group);
}
}
@@ -100,15 +55,24 @@ export function buildMemberOf(groups: GroupEntity[], users: UserEntity[]) {
const groupsByName = new Map(groups.map(g => [g.metadata.name, g]));
users.forEach(user => {
const transitiveMemberOf = new Set([...user.spec.memberOf]);
const transitiveMemberOf = new Set<string>();
user.spec.memberOf.forEach(groupName => {
const group = groupsByName.get(groupName);
if (group) {
group.spec.ancestors.forEach(g => transitiveMemberOf.add(g));
const todo = [...user.spec.memberOf];
for (;;) {
const current = todo.pop();
if (!current) {
break;
}
});
if (!transitiveMemberOf.has(current)) {
transitiveMemberOf.add(current);
const group = groupsByName.get(current);
if (group?.spec.parent) {
todo.push(group.spec.parent);
}
}
}
user.spec.memberOf = [...transitiveMemberOf];
});
}