> On Oct. 17, 2024, 5:17 p.m., Abhay Kulkarni wrote:
> > Added a minor comment to the test code. It looks good overall. However, it
> > is not clear how and where the dedupTags() is called multiple times during
> > the initialization of the plugin as described in the original description
> > of the JIRA.
I saw this issue in Ranger admin while retrieving policies for a given
resource, from RangerAdminTagEnricher.enrich(). Here is the stack from jstack.
RangerTagEnricher.setServiceTags() calls dedupTags() on a ServiceTags instance
that is already deduped!
"http-nio-6080-exec-8" #40 daemon prio=5 os_prio=0 tid=0x0000ffffa5212000
nid=0x15d1 runnable [0x0000ffff7b3f9000]
java.lang.Thread.State: RUNNABLE
at
org.apache.ranger.plugin.util.ServiceTags.dedupTags(ServiceTags.java:283)
at
org.apache.ranger.plugin.contextenricher.RangerTagEnricher.setServiceTags(RangerTagEnricher.java:322)
at
org.apache.ranger.plugin.contextenricher.RangerTagEnricher.setServiceTags(RangerTagEnricher.java:290)
at
org.apache.ranger.common.RangerAdminTagEnricher.refreshTagsIfNeeded(RangerAdminTagEnricher.java:164)
- locked <0x00000000f8a29400> (a
org.apache.ranger.common.RangerAdminTagEnricher)
at
org.apache.ranger.common.RangerAdminTagEnricher.enrich(RangerAdminTagEnricher.java:105)
at
org.apache.ranger.plugin.service.RangerDefaultRequestProcessor.enrich(RangerDefaultRequestProcessor.java:158)
at
org.apache.ranger.plugin.service.RangerDefaultRequestProcessor.preProcess(RangerDefaultRequestProcessor.java:137)
at
org.apache.ranger.biz.RangerPolicyAdminImpl.getMatchingPolicies(RangerPolicyAdminImpl.java:647)
> On Oct. 17, 2024, 5:17 p.m., Abhay Kulkarni wrote:
> > agents-common/src/test/java/org/apache/ranger/plugin/util/TestServiceTags.java
> > Lines 85 (patched)
> > <https://reviews.apache.org/r/75232/diff/1/?file=2293803#file2293803line85>
> >
> > Consider adding
> >
> > assertEquals(0, svcTags.dedupTags());
> >
> > after this line and after line 97.
Multiple calls to dedupTags() is covered in a separate test -
testDedup_DupTagsWithAttr_MultipleCalls(). Any reason to add it here as well?
May be testDedup_DupTagsWithAttr_MultipleCalls() can be removed then?
- Madhan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/75232/#review226978
-----------------------------------------------------------
On Oct. 17, 2024, 1:29 a.m., Madhan Neethiraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/75232/
> -----------------------------------------------------------
>
> (Updated Oct. 17, 2024, 1:29 a.m.)
>
>
> Review request for ranger, Abhishek Kumar, Anand Nadar, Asit Vadhavkar,
> Fateh Singh, Abhay Kulkarni, Pradeep Agrawal, Radhika Kundam, Ramesh Mani,
> and Sailaja Polavarapu.
>
>
> Bugs: RANGER-4956
> https://issues.apache.org/jira/browse/RANGER-4956
>
>
> Repository: ranger
>
>
> Description
> -------
>
> - dedupTags() on a ServiceTag instance can result in infinite loop when its
> cachedTags is already populated (by an earlier call or via a copy
> constructor), as it tries to replace a tag with itself. Updated to avoid such
> replacement.
>
>
> Diffs
> -----
>
> agents-common/src/main/java/org/apache/ranger/plugin/util/ServiceTags.java
> cc2ebe53a
>
> agents-common/src/test/java/org/apache/ranger/plugin/util/TestServiceTags.java
> PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/75232/diff/1/
>
>
> Testing
> -------
>
> - added unit tests to verify dedupTags()
> - verified that all unit tests pass
>
>
> Thanks,
>
> Madhan Neethiraj
>
>
