vyommani commented on code in PR #950:
URL: https://github.com/apache/ranger/pull/950#discussion_r3333821042
##########
agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceDefHelper.java:
##########
@@ -507,14 +507,23 @@ public Delegate(RangerServiceDef serviceDef, boolean
checkForCycles) {
this.hierarchies.put(policyType,
Collections.unmodifiableSet(hierarchies));
hierarchyKeys.put(policyType,
Collections.unmodifiableSet(hierachyKeys));
+ Map<String, RangerResourceDef> wildcardMap = new
HashMap<>(resources.size());
+ for (RangerResourceDef res : resources) {
+ if (res != null) {
+ wildcardMap.put(res.getName(),
createWildcardEnabledResourceDef(res));
Review Comment:
Good catch. You are correct that if resources contains duplicate names, the
current eager init code uses HashMap.put() which would silently overwrite
earlier entries with later ones — meaning the last match wins.
However the original lazy code had a different behavior — it iterated the
list and broke on the first match
So the eager init introduces a subtle behavioral difference on duplicate
resource names.
That said, in practice duplicate resource names in a RangerServiceDef should
never occur — the service definition is validated upstream before being stored
and a duplicate resource name would indicate a malformed service definition.
However to be defensively correct and to exactly preserve the original
behavior (first match wins), we should change put to putIfAbsent
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
