close
Skip to content

RANGER-5110: Prevent entity soft deletion when LDAP DN is updated - #1162

Open
krishnamuttevi wants to merge 2 commits into
apache:masterfrom
krishnamuttevi:RANGER-5110
Open

RANGER-5110: Prevent entity soft deletion when LDAP DN is updated#1162
krishnamuttevi wants to merge 2 commits into
apache:masterfrom
krishnamuttevi:RANGER-5110

Conversation

@krishnamuttevi

Copy link
Copy Markdown
Contributor

Fixes an issue in LDAP/AD sync where users or groups are incorrectly skipped or marked for deletion when their DN changes (e.g., due to OU moves). This PR correctly detects, maps, and updates DN changes efficiently using the existing cache.

Key Changes:
Allow DN Updates: Added an isLdapAdSync check in delta computation (computeUserDelta/computeGroupDelta). If a DN changes, the update is now explicitly allowed rather than skipped, and old DN mappings are cleaned up.

Smart Deletion Check:
Before marking an entity as deleted due to a missing DN (computeDeletedUsers/computeDeletedGroups), we now verify if the base name still exists in the source. If it does, deletion is bypassed so the delta sync can naturally correct the DN.

Why this approach (vs. PR #516)
Better Performance: Avoids the heavy REST reloads and cache wiping introduced in #516. We utilize the existing cache.

Complete Fix: Actually updates the entity with the new DN across cycles (PR #516 only attempts a soft-delete and fails to map the new DN).

Cleaner: Highly efficient fix with a significantly smaller LOC footprint.

Testing:

Verified LDAP/AD users and groups update correctly during OU moves.

Confirmed no extra cache-clearing loads are triggered.

Verified non-LDAP/AD sync behavior remains unaffected.


if (StringUtils.equalsIgnoreCase(curUserAttrsStr, newUserAttrsStr)) {
userNameMap.put(userDN, userName);
boolean isLdapAdSync = StringUtils.equalsIgnoreCase(newSyncSource, "LDAP/AD")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comments as above apply here as well

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have updated in the following commit.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check the below the below doc for the testcaes which was verified by runtime

Test cases for the RANGER-5110_.pdf

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants