This is an automated email from the ASF dual-hosted git repository.
abhi pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ranger.git
The following commit(s) were added to refs/heads/master by this push:
new 4d56c7fcf RANGER-5224: dedupTags removes the valid tags while
deduplicating tags (#592)
4d56c7fcf is described below
commit 4d56c7fcf14d3896606f38d303f20b4afa8c339f
Author: Vyom Mani Tiwari <[email protected]>
AuthorDate: Wed Jun 25 04:36:15 2025 +0530
RANGER-5224: dedupTags removes the valid tags while deduplicating tags
(#592)
---
.../org/apache/ranger/plugin/util/ServiceTags.java | 5 ++
.../apache/ranger/plugin/util/TestServiceTags.java | 93 ++++++++++++++++++++++
2 files changed, 98 insertions(+)
diff --git
a/agents-common/src/main/java/org/apache/ranger/plugin/util/ServiceTags.java
b/agents-common/src/main/java/org/apache/ranger/plugin/util/ServiceTags.java
index c5b0811fe..74b2464a8 100644
--- a/agents-common/src/main/java/org/apache/ranger/plugin/util/ServiceTags.java
+++ b/agents-common/src/main/java/org/apache/ranger/plugin/util/ServiceTags.java
@@ -247,6 +247,7 @@ public int dedupTags() {
final Map<Long, Long> replacedIds = new HashMap<>();
final int initialTagsCount = tags.size();
final List<Long> tagIdsToRemove = new ArrayList<>();
+ final Map<Long, RangerTag> tagsToAdd = new HashMap<>();
for (Iterator<Map.Entry<Long, RangerTag>> iter =
tags.entrySet().iterator(); iter.hasNext(); ) {
Map.Entry<Long, RangerTag> entry = iter.next();
@@ -263,6 +264,7 @@ public int dedupTags() {
cachedTag.left = tagId;
} else {
replacedIds.put(tagId, cachedTag.left);
+ tagsToAdd.put(cachedTag.left, tag);
iter.remove();
}
}
@@ -272,6 +274,9 @@ public int dedupTags() {
tags.remove(tagIdToRemove);
}
+ // Add all the tags whose tagIds are modified back to tags
+ tags.putAll(tagsToAdd);
+
final int finalTagsCount = tags.size();
for (Map.Entry<Long, List<Long>> resourceEntry :
resourceToTagIds.entrySet()) {
diff --git
a/agents-common/src/test/java/org/apache/ranger/plugin/util/TestServiceTags.java
b/agents-common/src/test/java/org/apache/ranger/plugin/util/TestServiceTags.java
index 1c8e41c5b..a2d52427f 100644
---
a/agents-common/src/test/java/org/apache/ranger/plugin/util/TestServiceTags.java
+++
b/agents-common/src/test/java/org/apache/ranger/plugin/util/TestServiceTags.java
@@ -25,8 +25,11 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
+import java.util.Map;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
public class TestServiceTags {
private static final RangerServiceResource[] RESOURCES = {
@@ -112,6 +115,96 @@ public void testDedup_DupTagsWithAttr_MultipleCalls() {
assertEquals(0, svcTags.dedupTags());
}
+ @Test
+ public void testDedupTags_DuplicateTagWithHigherId() {
+ // Create ServiceTags with duplicate tags
+ RangerTag[] tags = {
+ new RangerTag("PII", Collections.singletonMap("type",
"email")),
+ new RangerTag("PII", Collections.singletonMap("type",
"email")),
+ new RangerTag("PCI", Collections.emptyMap()),
+ new RangerTag("PCI", Collections.emptyMap()),
+ new RangerTag("PII", Collections.singletonMap("type", "email"))
+ };
+
+ ServiceTags svcTags = createServiceTags(tags, RESOURCES);
+ assertEquals(5, svcTags.getTags().size());
+ // Should remove 3 duplicates (2 PII, 1 PCI)
+ assertEquals(3, svcTags.dedupTags());
+
+ // Verify: 2 tags remain (one PII, one PCI)
+ assertEquals(2, svcTags.getTags().size());
+ // Find retained PII tag ID
+ Long piiTagId = svcTags.getTags().entrySet().stream()
+ .filter(e -> e.getValue().getType().equals("PII"))
+ .map(Map.Entry::getKey)
+ .findFirst()
+ .orElseThrow(() -> new AssertionError("PII tag not found"));
+ RangerTag cachedTag = svcTags.getTags().get(piiTagId);
+
+ // Verify resource mappings
+ assertTrue(svcTags.getResourceToTagIds().values().stream()
+ .allMatch(tagIds -> tagIds.contains(piiTagId)));
+
+ // Add a new PII tag with higher tag ID
+ ServiceTags svcTags1 = new ServiceTags(svcTags);
+ svcTags1.getTags().remove(piiTagId);
+ RangerTag newTag = new RangerTag("PII",
Collections.singletonMap("type", "email"));
+ long newTagId = 23L;
+ newTag.setId(newTagId);
+ svcTags1.getTags().put(newTagId, newTag);
+ svcTags1.getResourceToTagIds().get(1L).add(newTagId);
+
+ assertEquals(0, svcTags1.dedupTags());
+ assertEquals(2, svcTags1.getTags().size());
+ assertEquals(cachedTag, svcTags1.getTags().get(piiTagId));
+ assertFalse(svcTags1.getTags().containsKey(newTagId));
+
+ // Verify resource mappings still include piiTagId
+ assertTrue(svcTags1.getResourceToTagIds().values().stream()
+ .allMatch(tagIds -> tagIds.contains(piiTagId)));
+
+ // Simulate resource deletion
+ svcTags1.getResourceToTagIds().remove(0L);
+ assertEquals(0, svcTags1.dedupTags());
+ assertEquals(cachedTag, svcTags1.getTags().get(piiTagId));
+ assertTrue(svcTags1.getResourceToTagIds().get(1L).contains(piiTagId));
+ }
+
+ @Test
+ public void testDedupTags_HigherIdTagAfterLowerIdRemoval() {
+ // Create ServiceTags with one PII tag
+ RangerTag[] tags = {new RangerTag("PII",
Collections.singletonMap("type", "email"))};
+ ServiceTags svcTags = createServiceTags(tags, RESOURCES);
+ assertEquals(1, svcTags.getTags().size());
+
+ Long piiTagId = svcTags.getTags().entrySet().stream()
+ .filter(e -> e.getValue().getType().equals("PII"))
+ .map(Map.Entry::getKey)
+ .findFirst()
+ .orElseThrow(() -> new AssertionError("PII tag not found"));
+ RangerTag cachedTag = svcTags.getTags().get(piiTagId);
+
+ // calling dedupTags() make sure that cachedTags contains PII tag.
+ svcTags.dedupTags();
+ svcTags.getTags().remove(piiTagId);
+
+ ServiceTags svcTags1 = new ServiceTags(svcTags);
+ RangerTag newTag = new RangerTag("PII",
Collections.singletonMap("type", "email"));
+ long newTagId = 7L; // Higher tagID
+ newTag.setId(newTagId);
+ svcTags1.getTags().put(newTagId, newTag);
+ svcTags1.getResourceToTagIds().get(1L).add(newTagId);
+
+ // Call dedupTags (should fail with buggy code)
+ assertEquals(0, svcTags1.dedupTags());
+
+ // With buggy dedupTags(), newTagId (7) is removed, and no PII tag
remains
+ // With fixed dedupTags(), newTagId is replaced with a valid
ID(piiTagId)
+ assertEquals(1, svcTags1.getTags().size());
+ assertEquals(cachedTag, svcTags1.getTags().get(piiTagId));
+ assertFalse(svcTags1.getTags().containsKey(newTagId));
+ }
+
private ServiceTags createServiceTags(RangerTag[] tags,
RangerServiceResource[] resources) {
ServiceTags ret = new ServiceTags();
