Allow to use any ldap attribute as user id for batch role sync

[krasilnikov-dmitriy]

Currently we're have two options for batch sync users roles:

1. Sync roles for each user by making request to ldap with userSearchFilter. That way allow us to use any ldap attribute as user id but it very slow ~ 16min for 1000 users.

2. Sync users roles by making 1 request to ldap using ** groupSearchBase** perf(auth/ldap): Use group to user mapping to role sync #573. For that case all user related information gets from member attribute which consists of only DN(distinguished name) because ldap doesn't support join queries. That way dramatically faster ~ 14sec for 1000 users, but we're couldn't use sAMAccountName or any other ldap attributes which we're used to use in userSearchFilter and have to use only CN as user id.

Usually ldap contains a lot of users and few roles and in that case we could change our approach for sync users roles.
First of all we can get all users with role by one ldap call like that (&(objectCategory=user)(memberOf={ROLE_DN})). That query return all objects which takes part of the group, but we're allowed to use only strong equality by DN for a memberOf filter.
As a result we'll make N+1 requests to ldap where N is number of groups and it seems to be fast also we could use the same ldap attribute as user id which we using in userSearchFilter for syncing single user. In our case it takes 7sec for syncing 250 users in 2 roles.

I understand that request contains breaking changes but maintains 3 different ways to sync roles doesn't sounds like good idea and such approach looks like best compromise: it still fast for large ldap instalation and not so slow for small instalations.

Linked issue: spinnaker/spinnaker#6150

[spinnakerbot]

The following commits need their title changed:

• b2a31b3: Allow to set any ldap attribute as user id

Please format your commit title into the form:

<type>(<scope>): <subject>, e.g. fix(kubernetes): address NPE in status check

This allows us to easily generate changelogs & determine semantic version numbers when cutting releases. You can read more about commit conventions here.

[fieldju]

I haven't actually peer reviewed the code, but I wanted to add a general comment on breaking changes.

We can't just abruptly introduce breaking changes to core functionality, as this produces a very unstable user experience for Spinnakers enterprise customers.

The general approach i'd like to see for something like this is that it is opt in for x releases, then it becomes the default for y releases but you can opt into the legacy provider and announce after z releases you will be deleting the legacy impl.

So i'd like to see the UX for enabling this be like so

fiat.ldap.rolesProvider: v2 but it defaults to v1 if that is not present.

this might involve making a new module in this project fiat-ldap-v2 and doing a slight tweak on the v1 config so that it only loads if its set to v1 and in the base config set fiat.ldap.rolesProvider: v1

☝️ something like this allows us to responsibly deprecate and replace functionality.

EDIT

I pulled those configuration paths out of a hat, and would need to be tweaked to something that made sense.

Then we would also update this: https://spinnaker.io/community/releases/next-release-preview/ to talk about the new V2 provider and any plans to make it the default and remove the other one, etc.

[krasilnikov-dmitriy]

@fieldju Hi, i've moved all changes to different module could you have a look to it.

Also i've changed the way how to get users from ldap by one query. Now i've construct one query joining the userSearchFilter for all users by | (or operator). As i known ldap support query size up to 10Mb and sounds like it should works for any condition.

[jvz]

You can create a new OrFilter and run filter.or(Filter) on each filter to or together.