ayushtkn commented on code in PR #524:
URL: https://github.com/apache/tez/pull/524#discussion_r3627489302
##########
tez-api/src/main/java/org/apache/tez/common/security/ACLManager.java:
##########
@@ -76,10 +76,11 @@ public ACLManager(String amUser, Configuration conf) {
public ACLManager(ACLManager amACLManager, String dagUser, ACLInfo aclInfo) {
this.amUser = amACLManager.amUser;
this.dagUser = dagUser;
- this.users = amACLManager.users;
- this.groups = amACLManager.groups;
+ // Copy the AM-level maps so per-DAG entries stay scoped to this DAG.
+ this.users = new HashMap<>(amACLManager.users);
+ this.groups = new HashMap<>(amACLManager.groups);
Review Comment:
I think this is a Shallow Copy not a Deep Copy? Is it intentional? Though we
are doing put here below. But if someone does in future `.get(key).add(...)` it
might lead to some problems, can you check once
something like in future
`this.users.get(ACLType.AM_VIEW_ACL).add("new_user")` inside this class, it
will silently modify the global Application Master's ACLs. They might think
they are only granting a user access to a specific DAG, but because of the
shallow copy, they would accidentally be granting that user global AM access.
--
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]
