diff --git a/.changeset/tasty-fishes-call.md b/.changeset/tasty-fishes-call.md new file mode 100644 index 0000000000..417f37078b --- /dev/null +++ b/.changeset/tasty-fishes-call.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-backend-module-ldap': patch +--- + +Changed relationship creation in LDAP data processor to leverage entityRefs instead of metadata.name diff --git a/plugins/catalog-backend-module-ldap/src/ldap/org.test.ts b/plugins/catalog-backend-module-ldap/src/ldap/org.test.ts index 254f511c4f..c675ee06b5 100644 --- a/plugins/catalog-backend-module-ldap/src/ldap/org.test.ts +++ b/plugins/catalog-backend-module-ldap/src/ldap/org.test.ts @@ -19,66 +19,80 @@ import { buildMemberOf, buildOrgHierarchy } from './org'; function g( name: string, + namespace: string, parent: string | undefined, children: string[], ): GroupEntity { return { apiVersion: 'backstage.io/v1alpha1', kind: 'Group', - metadata: { name }, + metadata: { name, namespace }, spec: { type: 'team', parent, children }, }; } describe('buildOrgHierarchy', () => { 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', []); + const a = g('a', 'a-namespace', undefined, []); + const b = g('b', 'b-namespace', 'group:a-namespace/a', []); + const c = g('c', 'c-namespace', 'group:b-namespace/b', []); + const d = g('d', 'd-namespace', 'group:a-namespace/a', []); buildOrgHierarchy([a, b, c, d]); - expect(a.spec.children).toEqual(expect.arrayContaining(['b', 'd'])); - expect(b.spec.children).toEqual(expect.arrayContaining(['c'])); + expect(a.spec.children).toEqual( + expect.arrayContaining(['group:b-namespace/b', 'group:d-namespace/d']), + ); + expect(b.spec.children).toEqual( + expect.arrayContaining(['group:c-namespace/c']), + ); expect(c.spec.children).toEqual([]); expect(d.spec.children).toEqual([]); }); 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, []); + const a = g('a', 'a-namespace', undefined, [ + 'group:b-namespace/b', + 'group:d-namespace/d', + ]); + const b = g('b', 'b-namespace', undefined, ['group:c-namespace/c']); + const c = g('c', 'c-namespace', undefined, []); + const d = g('d', 'd-namespace', undefined, []); buildOrgHierarchy([a, b, c, d]); expect(a.spec.parent).toBeUndefined(); - expect(b.spec.parent).toBe('a'); - expect(c.spec.parent).toBe('b'); - expect(d.spec.parent).toBe('a'); + expect(b.spec.parent).toBe('group:a-namespace/a'); + expect(c.spec.parent).toBe('group:b-namespace/b'); + expect(d.spec.parent).toBe('group:a-namespace/a'); }); }); describe('buildMemberOf', () => { it('fills indirect member of groups', () => { - const a = g('a', undefined, []); - const b = g('b', 'a', []); - const c = g('c', 'b', []); + const a = g('a', 'a-namespace', undefined, []); + const b = g('b', 'b-namespace', 'group:a-namespace/a', []); + const c = g('c', 'c-namespace', 'group:b-namespace/b', []); const u: UserEntity = { apiVersion: 'backstage.io/v1alpha1', kind: 'User', metadata: { name: 'n' }, - spec: { profile: {}, memberOf: ['c'] }, + spec: { profile: {}, memberOf: ['group:c-namespace/c'] }, }; const groups = [a, b, c]; buildOrgHierarchy(groups); buildMemberOf(groups, [u]); - expect(u.spec.memberOf).toEqual(expect.arrayContaining(['a', 'b', 'c'])); + expect(u.spec.memberOf).toEqual( + expect.arrayContaining([ + 'group:a-namespace/a', + 'group:b-namespace/b', + 'group:c-namespace/c', + ]), + ); }); it('takes group spec.members into account', () => { - const a = g('a', undefined, []); - const b = g('b', 'a', []); - const c = g('c', 'b', []); - c.spec.members = ['n']; + const a = g('a', 'a-namespace', undefined, []); + const b = g('b', 'b-namespace', 'group:a-namespace/a', []); + const c = g('c', 'c-namespace', 'group:b-namespace/b', []); + c.spec.members = ['user:default/n']; const u: UserEntity = { apiVersion: 'backstage.io/v1alpha1', kind: 'User', @@ -89,6 +103,36 @@ describe('buildMemberOf', () => { const groups = [a, b, c]; buildOrgHierarchy(groups); buildMemberOf(groups, [u]); - expect(u.spec.memberOf).toEqual(expect.arrayContaining(['a', 'b', 'c'])); + expect(u.spec.memberOf).toEqual( + expect.arrayContaining([ + 'group:a-namespace/a', + 'group:b-namespace/b', + 'group:c-namespace/c', + ]), + ); + }); + + it('is not affected by `:` or `/` in the group or user name', () => { + const a = g('a:a/a', 'a-namespace', undefined, []); + const b = g('b:b/b', 'b-namespace', 'group:a-namespace/a:a/a', []); + const c = g('c:c/c', 'c-namespace', 'group:b-namespace/b:b/b', []); + c.spec.members = ['user:default/n:n/n']; + const u: UserEntity = { + apiVersion: 'backstage.io/v1alpha1', + kind: 'User', + metadata: { name: 'n:n/n' }, + spec: { profile: {}, memberOf: [] }, + }; + + const groups = [a, b, c]; + buildOrgHierarchy(groups); + buildMemberOf(groups, [u]); + expect(u.spec.memberOf).toEqual( + expect.arrayContaining([ + 'group:a-namespace/a:a/a', + 'group:b-namespace/b:b/b', + 'group:c-namespace/c:c/c', + ]), + ); }); }); diff --git a/plugins/catalog-backend-module-ldap/src/ldap/org.ts b/plugins/catalog-backend-module-ldap/src/ldap/org.ts index 28ccbd1d3b..8f657b6625 100644 --- a/plugins/catalog-backend-module-ldap/src/ldap/org.ts +++ b/plugins/catalog-backend-module-ldap/src/ldap/org.ts @@ -14,25 +14,29 @@ * limitations under the License. */ -import { GroupEntity, UserEntity } from '@backstage/catalog-model'; +import { + GroupEntity, + stringifyEntityRef, + UserEntity, +} from '@backstage/catalog-model'; // TODO: Copied from plugin-catalog-backend, but we could also export them from // there. Or move them to catalog-model. export function buildOrgHierarchy(groups: GroupEntity[]) { - const groupsByName = new Map(groups.map(g => [g.metadata.name, g])); + const groupsByRef = new Map(groups.map(g => [stringifyEntityRef(g), g])); // // Make sure that g.parent.children contain g // for (const group of groups) { - const selfName = group.metadata.name; - const parentName = group.spec.parent; - if (parentName) { - const parent = groupsByName.get(parentName); - if (parent && !parent.spec.children.includes(selfName)) { - parent.spec.children.push(selfName); + const selfRef = stringifyEntityRef(group); + const parentRef = group.spec.parent; + if (parentRef) { + const parent = groupsByRef.get(parentRef); + if (parent && !parent.spec.children.includes(selfRef)) { + parent.spec.children.push(selfRef); } } } @@ -42,11 +46,11 @@ export function buildOrgHierarchy(groups: GroupEntity[]) { // for (const group of groups) { - const selfName = group.metadata.name; - for (const childName of group.spec.children) { - const child = groupsByName.get(childName); + const selfRef = stringifyEntityRef(group); + for (const childRef of group.spec.children) { + const child = groupsByRef.get(childRef); if (child && !child.spec.parent) { - child.spec.parent = selfName; + child.spec.parent = selfRef; } } } @@ -55,7 +59,7 @@ export function buildOrgHierarchy(groups: GroupEntity[]) { // Ensure that users have their transitive group memberships. Requires that // the groups were previously processed with buildOrgHierarchy() export function buildMemberOf(groups: GroupEntity[], users: UserEntity[]) { - const groupsByName = new Map(groups.map(g => [g.metadata.name, g])); + const groupsByRef = new Map(groups.map(g => [stringifyEntityRef(g), g])); users.forEach(user => { const transitiveMemberOf = new Set(); @@ -63,8 +67,8 @@ export function buildMemberOf(groups: GroupEntity[], users: UserEntity[]) { const todo = [ ...(user.spec.memberOf ?? []), ...groups - .filter(g => g.spec.members?.includes(user.metadata.name)) - .map(g => g.metadata.name), + .filter(g => g.spec.members?.includes(stringifyEntityRef(user))) + .map(g => stringifyEntityRef(g)), ]; for (;;) { @@ -75,7 +79,7 @@ export function buildMemberOf(groups: GroupEntity[], users: UserEntity[]) { if (!transitiveMemberOf.has(current)) { transitiveMemberOf.add(current); - const group = groupsByName.get(current); + const group = groupsByRef.get(current); if (group?.spec.parent) { todo.push(group.spec.parent); } diff --git a/plugins/catalog-backend-module-ldap/src/ldap/read.test.ts b/plugins/catalog-backend-module-ldap/src/ldap/read.test.ts index 3ded4635a0..410ac847b3 100644 --- a/plugins/catalog-backend-module-ldap/src/ldap/read.test.ts +++ b/plugins/catalog-backend-module-ldap/src/ldap/read.test.ts @@ -362,8 +362,8 @@ describe('resolveRelations', () => { ['pa', new Set(['ca'])], ]); resolveRelations([parent, child], [], new Map(), new Map(), groupMember); - expect(parent.spec.children).toEqual(['child']); - expect(child.spec.parent).toEqual('parent'); + expect(parent.spec.children).toEqual(['group:default/child']); + expect(child.spec.parent).toEqual('group:default/parent'); }); it('matches by UUID', () => { @@ -383,8 +383,8 @@ describe('resolveRelations', () => { ['pa', new Set(['ca'])], ]); resolveRelations([parent, child], [], new Map(), new Map(), groupMember); - expect(parent.spec.children).toEqual(['child']); - expect(child.spec.parent).toEqual('parent'); + expect(parent.spec.children).toEqual(['group:default/child']); + expect(child.spec.parent).toEqual('group:default/parent'); }); }); @@ -403,7 +403,7 @@ describe('resolveRelations', () => { ['ma', new Set(['ha'])], ]); resolveRelations([host], [member], userMemberOf, new Map(), new Map()); - expect(member.spec.memberOf).toEqual(['host']); + expect(member.spec.memberOf).toEqual(['group:default/host']); }); it('populates relations by uuid', () => { @@ -423,7 +423,7 @@ describe('resolveRelations', () => { ['ma', new Set(['ha'])], ]); resolveRelations([host], [member], userMemberOf, new Map(), new Map()); - expect(member.spec.memberOf).toEqual(['host']); + expect(member.spec.memberOf).toEqual(['group:default/host']); }); }); @@ -451,8 +451,8 @@ describe('resolveRelations', () => { groupMemberOf, new Map(), ); - expect(parent.spec.children).toEqual(['child']); - expect(child.spec.parent).toEqual('parent'); + expect(parent.spec.children).toEqual(['group:default/child']); + expect(child.spec.parent).toEqual('group:default/parent'); }); }); @@ -473,8 +473,8 @@ describe('resolveRelations', () => { ['ca', new Set(['pa'])], ]); resolveRelations([parent, child], [], new Map(), groupMemberOf, new Map()); - expect(parent.spec.children).toEqual(['child']); - expect(child.spec.parent).toEqual('parent'); + expect(parent.spec.children).toEqual(['group:default/child']); + expect(child.spec.parent).toEqual('group:default/parent'); }); describe('groupMember', () => { @@ -507,9 +507,9 @@ describe('resolveRelations', () => { new Map(), groupMember, ); - expect(parent.spec.children).toEqual(['child']); - expect(child.spec.parent).toEqual('parent'); - expect(member.spec.memberOf).toEqual(['parent']); + expect(parent.spec.children).toEqual(['group:default/child']); + expect(child.spec.parent).toEqual('group:default/parent'); + expect(member.spec.memberOf).toEqual(['group:default/parent']); }); it('populates relations by uuid', () => { @@ -541,9 +541,9 @@ describe('resolveRelations', () => { new Map(), groupMember, ); - expect(parent.spec.children).toEqual(['child']); - expect(child.spec.parent).toEqual('parent'); - expect(member.spec.memberOf).toEqual(['parent']); + expect(parent.spec.children).toEqual(['group:default/child']); + expect(child.spec.parent).toEqual('group:default/parent'); + expect(member.spec.memberOf).toEqual(['group:default/parent']); }); }); }); diff --git a/plugins/catalog-backend-module-ldap/src/ldap/read.ts b/plugins/catalog-backend-module-ldap/src/ldap/read.ts index e15ec85554..d0fe30e0e6 100644 --- a/plugins/catalog-backend-module-ldap/src/ldap/read.ts +++ b/plugins/catalog-backend-module-ldap/src/ldap/read.ts @@ -14,7 +14,11 @@ * limitations under the License. */ -import { GroupEntity, UserEntity } from '@backstage/catalog-model'; +import { + GroupEntity, + stringifyEntityRef, + UserEntity, +} from '@backstage/catalog-model'; import { SearchEntry } from 'ldapjs'; import lodashSet from 'lodash/set'; import cloneDeep from 'lodash/cloneDeep'; @@ -353,18 +357,18 @@ export function resolveRelations( ) { // Build reference lookup tables - all of the relations that are output from // the above calls can be expressed as either DNs or UUIDs so we need to be - // able to find by both, as well as the name. Note that we expect them to not + // able to find by both, as well as the entity reference. Note that we expect them to not // collide here - this is a reasonable assumption as long as the fields are // the supported forms. - const userMap: Map = new Map(); // by name, dn, uuid - const groupMap: Map = new Map(); // by name, dn, uuid + const userMap: Map = new Map(); // by entityRef, dn, uuid + const groupMap: Map = new Map(); // by entityRef, dn, uuid for (const user of users) { - userMap.set(user.metadata.name, user); + userMap.set(stringifyEntityRef(user), user); userMap.set(user.metadata.annotations![LDAP_DN_ANNOTATION], user); userMap.set(user.metadata.annotations![LDAP_UUID_ANNOTATION], user); } for (const group of groups) { - groupMap.set(group.metadata.name, group); + groupMap.set(stringifyEntityRef(group), group); groupMap.set(group.metadata.annotations![LDAP_DN_ANNOTATION], group); groupMap.set(group.metadata.annotations![LDAP_UUID_ANNOTATION], group); } @@ -375,7 +379,7 @@ export function resolveRelations( userMap.delete(undefined!); groupMap.delete(undefined!); - // Fill in all of the immediate relations, now keyed on metadata.name. We + // Fill in all of the immediate relations, now keyed on the entity reference. We // keep all parents at this point, whether the target model can support more // than one or not (it gets filtered farther down). And group children are // only groups in here. @@ -394,8 +398,8 @@ export function resolveRelations( for (const groupN of groupsN) { const group = groupMap.get(groupN); if (group) { - ensureItems(newUserMemberOf, user.metadata.name, [ - group.metadata.name, + ensureItems(newUserMemberOf, stringifyEntityRef(user), [ + stringifyEntityRef(group), ]); } } @@ -407,11 +411,11 @@ export function resolveRelations( for (const parentN of parentsN) { const parentGroup = groupMap.get(parentN); if (parentGroup) { - ensureItems(newGroupParents, group.metadata.name, [ - parentGroup.metadata.name, + ensureItems(newGroupParents, stringifyEntityRef(group), [ + stringifyEntityRef(parentGroup), ]); - ensureItems(newGroupChildren, parentGroup.metadata.name, [ - group.metadata.name, + ensureItems(newGroupChildren, stringifyEntityRef(parentGroup), [ + stringifyEntityRef(group), ]); } } @@ -425,17 +429,17 @@ export function resolveRelations( // try both const memberUser = userMap.get(memberN); if (memberUser) { - ensureItems(newUserMemberOf, memberUser.metadata.name, [ - group.metadata.name, + ensureItems(newUserMemberOf, stringifyEntityRef(memberUser), [ + stringifyEntityRef(group), ]); } else { const memberGroup = groupMap.get(memberN); if (memberGroup) { - ensureItems(newGroupChildren, group.metadata.name, [ - memberGroup.metadata.name, + ensureItems(newGroupChildren, stringifyEntityRef(group), [ + stringifyEntityRef(memberGroup), ]); - ensureItems(newGroupParents, memberGroup.metadata.name, [ - group.metadata.name, + ensureItems(newGroupParents, stringifyEntityRef(memberGroup), [ + stringifyEntityRef(group), ]); } }