Data 4950 - Atlan backend addition to Usernaut - #162
Conversation
Summary of ChangesHello @Pujathacker2210, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a comprehensive integration of Atlan as a new backend for Usernaut. The primary goal is to automate the synchronization of user and group lifecycle events between Usernaut and Atlan, thereby reducing the need for manual intervention and enhancing the overall reliability of data governance across platforms. The changes encompass configuration, dependency management, core controller logic, and a new client implementation tailored for Atlan's API, including considerations for SSO and LDAP synchronization. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Data flows, a silent stream, Atlan joins, a vibrant dream. Users synced, groups align, Code's new path, a clear design. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a new backend integration for Atlan, which is a significant and well-structured addition. The code is organized logically into a new atlan package, covering user management, team management, and team memberships. The implementation correctly handles SSO and LDAP sync scenarios by conditionally skipping certain operations. I've included several suggestions to improve efficiency, reduce code redundancy, and enhance configuration clarity. Overall, this is a solid contribution.
51f76d2 to
a0a2a5f
Compare
07028be to
7fb012b
Compare
7fb012b to
83c7d30
Compare
175e0bd to
9058006
Compare
|
|
||
| r.backendLogger.Info("successfully migrated team details from TeamStore to GroupStore") | ||
| return id, nil | ||
| return id, transformedGroupName, nil |
There was a problem hiding this comment.
I do not see a reason to return the transformedGroupName because it can be formed from Util more over the signature change to this method can prompt us to make changes to other backends as well.
| controllerConfig: | ||
| maxConcurrentReconciles: 1 | ||
|
No newline at end of file |
||
|
|
8e9ffac to
4f29e6b
Compare
|
Important Review skippedToo many files! This PR contains 379 files, which is 279 over the limit of 100. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. Usage-priced reviews support at most 300 files. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (18)
📒 Files selected for processing (443)
You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Signed-off-by: Pujathacker2210 <pthacker@redhat.com>
…p params via the ReconcileGroupParams function Signed-off-by: Pujathacker2210 <pthacker@redhat.com>
Signed-off-by: Pujathacker2210 <pthacker@redhat.com>
Signed-off-by: Pujathacker2210 <pthacker@redhat.com>
Signed-off-by: Ruchi Agrawal <ruagrawa@redhat.com>
Signed-off-by: Ruchi Agrawal <ruagrawa@redhat.com>
Signed-off-by: Ruchi Agrawal <ruagrawa@redhat.com>
Signed-off-by: Ruchi Agrawal <ruagrawa@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
- Call ReconcileGroupParams for Atlan backend even without explicit group_params to enable default persona assignment - Use ToAtlanInternalName() to convert group names to Atlan's internal format (lowercase with underscores) for consistent persona assignment - Add pagination support to FetchAllTeams - Add copyright header to teams.go Signed-off-by: Ruchi Agrawal <ruagrawa@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
- Add TeamName field to TeamParams struct instead of adding a parameter to the interface method - Revert interface signature to original: ReconcileGroupParams(ctx, teamID, groupParams) - Other backends (snowflake, fivetran, gitlab, rover) remain unchanged as they don't use the TeamName field - Controller sets backendGroupParams.TeamName for backends that need it - Atlan reads team name from groupParams.TeamName for persona assignment Signed-off-by: Ruchi Agrawal <ruagrawa@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: Ruchi Agrawal <ruagrawa@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
| # Set to true if users are auto-created via SSO login (skip manual user creation) | ||
| sso_sync: true | ||
| # Default persona to assign all groups | ||
| default_persona: "prod" |
There was a problem hiding this comment.
I don't think we should have default as prod, it should be literally a default one which has no permission, only add users, and not prod, because we are always giving prod persona even if not needed.
| type TeamParams struct { | ||
| Property string `json:"property"` | ||
| Value []string `json:"value"` | ||
| TeamName string `json:"team_name,omitempty"` // Set by controller for backends that need it |
There was a problem hiding this comment.
I guess TeamName was added for Atlan persona but if personas should be moving to operator, then I dont think we need this field.
| // Returns the list of users present under a team | ||
| FetchTeamMembersByTeamID(ctx context.Context, teamID string) (map[string]*structs.User, error) | ||
| // ReconcileGroupParams reconciles backend-specific parameters for a group/team. | ||
| // Backends that need the team name can read it from groupParams.TeamName. |
There was a problem hiding this comment.
I dont think we will be using TeamName so this comment is not valid and could be misleading..
| url: "https://yourcompany.atlan.com" | ||
| api_token: file|path/to/atlan_api_token | ||
| # Alias given during setting up identity_provider in the application | ||
| identity_provider_alias: "saml|azure" |
There was a problem hiding this comment.
Is "saml|azure" a place holder? What is thwe actual value this is should have.
| defaultPersona string | ||
| paginationLimit int | ||
| ssoSync bool | ||
| ldapSync bool |
There was a problem hiding this comment.
Can we add a short comment on ssoSync vs ldapSync? Easy to confuse which one is for what..
| defer wg.Done() | ||
| defer func() { <-sem }() | ||
|
|
||
| url := fmt.Sprintf("%s/api/service/users/%s/groups", ac.url, uid) |
There was a problem hiding this comment.
Linked to https://github.com/redhat-data-and-ai/usernaut/pull/162/changes#r3768422706
I guess it will be /users//groups rather than it should be /users//groups
| "qualifiedName": persona.QualifiedName, | ||
| "name": persona.Name, | ||
| "personaGroups": groups, | ||
| "isAccessControlEnabled": true, |
There was a problem hiding this comment.
What is this for and why we setting this always when updating the Persona?
|
|
||
| case "": | ||
| // No explicit group params specified, but still assign default persona if configured | ||
| if ac.defaultPersona != "" { |
There was a problem hiding this comment.
https://github.com/redhat-data-and-ai/usernaut/pull/162/changes#r3767981930
This is based on what we set as default.
|
|
||
| if groupParams.TeamName == "" { | ||
| log.Warn("team name is empty, cannot reconcile group params") | ||
| return nil |
There was a problem hiding this comment.
I believe we should return the error or else we might skip the persona assign..
| // Reconcile group params for backends that need it | ||
| // - Call when Property is explicitly specified in group_params, OR | ||
| // - Call for Atlan backend (which may assign default persona even without explicit params) | ||
| if backendGroupParams.Property != "" || backend.Type == "atlan" { |
There was a problem hiding this comment.
does it have to be hardcoded?
Changes
📝 Description
Adding Atlan backend to Usernaut
What changed?
This PR adds Atlan as a backend for Usernaut, enabling end-to-end user and group lifecycle management.
Key Features
Atlan backend support: Introduces Atlan as a managed backend for provisioning, de-provisioning, and group management.
depends_onsupport: Allows Atlan groups to sync membership from an upstream directory (LDAP / Active Directory) instead of directly managing users.identity_provider_aliasfor group sync: Uses the identity provider alias configured in Atlan to enable LDAP/AD-based group synchronization.Flexible dependency configuration:
depends_onandidentity_provider_aliasare only required when LDAP/AD group sync is enabled.Atlan connection configuration: Requires
urlandapi_tokenfor standard API operations.asset_transfer_username: Mandates a service account username to transfer asset ownership when users are deleted during offboarding.sso_syncsupport: sso_sync=true skips manual user creation when users are expected to be created via SSO login.default_personaassignment: Allows assigning a default persona to all groups created via Usernaut.OAuth credentials for user deletion: Supports
oauth_client_idandoauth_client_secretto generate runtime OAuth tokens required for user deletion (API tokens are insufficient).Group-level persona overrides: Additional personas can be specified per group using groupParams, handled during group reconciliation.
✅ Developer Checklist