timmylicheng commented on a change in pull request #918:
URL: https://github.com/apache/hadoop-ozone/pull/918#discussion_r424916337
##########
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/BucketManagerImpl.java
##########
@@ -555,6 +564,14 @@ public boolean checkAccess(OzoneObj ozObject,
RequestContext context)
throws OMException {
Objects.requireNonNull(ozObject);
Objects.requireNonNull(context);
+ String whiteList = conf.get(OZONE_WRITE_LIST);
+ Set<String> whiteListSet = new HashSet<>();
+ if (whiteList != null) {
+ whiteListSet.addAll(Arrays.asList(whiteList.trim().split(",")));
+ }
+ if(whiteListSet.contains(context.getClientUgi().getUserName())) {
Review comment:
space
##########
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
##########
@@ -1589,6 +1593,15 @@ public boolean checkAccess(OzoneObj ozObject,
RequestContext context)
Objects.requireNonNull(ozObject);
Objects.requireNonNull(context);
Objects.requireNonNull(context.getClientUgi());
+ String whiteList = conf.get(OZONE_WRITE_LIST);
+ Set<String> whiteListSet = new HashSet<>();
+ LOG.warn("=====keymanagerImpl=======checkAccess=====");
+ if (whiteList != null) {
+ whiteListSet.addAll(Arrays.asList(whiteList.trim().split(",")));
+ }
+ if(whiteListSet.contains(context.getClientUgi().getUserName())) {
Review comment:
space
##########
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
##########
@@ -1589,6 +1593,15 @@ public boolean checkAccess(OzoneObj ozObject,
RequestContext context)
Objects.requireNonNull(ozObject);
Objects.requireNonNull(context);
Objects.requireNonNull(context.getClientUgi());
+ String whiteList = conf.get(OZONE_WRITE_LIST);
+ Set<String> whiteListSet = new HashSet<>();
+ LOG.warn("=====keymanagerImpl=======checkAccess=====");
+ if (whiteList != null) {
+ whiteListSet.addAll(Arrays.asList(whiteList.trim().split(",")));
+ }
+ if(whiteListSet.contains(context.getClientUgi().getUserName())) {
+ return true;
Review comment:
Consider to add comment for how the white list work here
##########
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
##########
@@ -1589,6 +1593,15 @@ public boolean checkAccess(OzoneObj ozObject,
RequestContext context)
Objects.requireNonNull(ozObject);
Objects.requireNonNull(context);
Objects.requireNonNull(context.getClientUgi());
+ String whiteList = conf.get(OZONE_WRITE_LIST);
+ Set<String> whiteListSet = new HashSet<>();
+ LOG.warn("=====keymanagerImpl=======checkAccess=====");
+ if (whiteList != null) {
+ whiteListSet.addAll(Arrays.asList(whiteList.trim().split(",")));
Review comment:
This is going to rely on config to set up white list users, which ends
up being static and requires reboot when changes to the lists happen.
We want some CLI tool to dynamic modify the user list while the cluster is
running and the list should be persisted somewhere so that it could load the
list after reboot. So then concurrentHashSet may be a better option. Any
updates to the list will have to go the concurrent hash set as well as to
persistence.
Also think about OM HA. Would this change be replicate to OM followers?
Change to the white list is effectively a state change for OM.
##########
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
##########
@@ -1589,6 +1593,15 @@ public boolean checkAccess(OzoneObj ozObject,
RequestContext context)
Objects.requireNonNull(ozObject);
Objects.requireNonNull(context);
Objects.requireNonNull(context.getClientUgi());
+ String whiteList = conf.get(OZONE_WRITE_LIST);
+ Set<String> whiteListSet = new HashSet<>();
+ LOG.warn("=====keymanagerImpl=======checkAccess=====");
Review comment:
Is this warn log necessary?
##########
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##########
@@ -1631,8 +1634,15 @@ public void checkAcls(ResourceType resType, StoreType
storeType,
ACLType aclType, String vol, String bucket, String key,
UserGroupInformation ugi, InetAddress remoteAddress, String hostName)
throws OMException {
- checkAcls(resType, storeType, aclType, vol, bucket, key,
- ugi, remoteAddress, hostName, true);
+ String whiteList = configuration.get(OZONE_WRITE_LIST);
+ Set<String> whiteListSet = new HashSet<>();
+ if (whiteList != null) {
+ whiteListSet.addAll(Arrays.asList(whiteList.trim().split(",")));
Review comment:
Same here and the whitelist should be member OM. We cannot afford
reading config every time the stack reaches here.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
