hadoop-yetus commented on a change in pull request #664: [HADOOP-14951] Make 
the KMSACLs implementation customizable, with an …
URL: https://github.com/apache/hadoop/pull/664#discussion_r280745665
 
 

 ##########
 File path: 
hadoop-common-project/hadoop-kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KMSACLs.java
 ##########
 @@ -65,269 +66,105 @@ public String getBlacklistConfigKey() {
     }
   }
 
-  public static final String ACL_DEFAULT = 
AccessControlList.WILDCARD_ACL_VALUE;
-
-  public static final int RELOADER_SLEEP_MILLIS = 1000;
-
-  private volatile Map<Type, AccessControlList> acls;
-  private volatile Map<Type, AccessControlList> blacklistedAcls;
-  @VisibleForTesting
-  volatile Map<String, HashMap<KeyOpType, AccessControlList>> keyAcls;
-  @VisibleForTesting
-  volatile Map<KeyOpType, AccessControlList> defaultKeyAcls = new HashMap<>();
-  @VisibleForTesting
-  volatile Map<KeyOpType, AccessControlList> whitelistKeyAcls = new 
HashMap<>();
-  private ScheduledExecutorService executorService;
-  private long lastReload;
-
-  KMSACLs(Configuration conf) {
-    if (conf == null) {
-      conf = loadACLs();
-    }
-    setKMSACLs(conf);
-    setKeyACLs(conf);
-  }
-
-  public KMSACLs() {
-    this(null);
-  }
-
-  private void setKMSACLs(Configuration conf) {
-    Map<Type, AccessControlList> tempAcls = new HashMap<Type, 
AccessControlList>();
-    Map<Type, AccessControlList> tempBlacklist = new HashMap<Type, 
AccessControlList>();
-    for (Type aclType : Type.values()) {
-      String aclStr = conf.get(aclType.getAclConfigKey(), ACL_DEFAULT);
-      tempAcls.put(aclType, new AccessControlList(aclStr));
-      String blacklistStr = conf.get(aclType.getBlacklistConfigKey());
-      if (blacklistStr != null) {
-        // Only add if blacklist is present
-        tempBlacklist.put(aclType, new AccessControlList(blacklistStr));
-        LOG.info("'{}' Blacklist '{}'", aclType, blacklistStr);
-      }
-      LOG.info("'{}' ACL '{}'", aclType, aclStr);
-    }
-    acls = tempAcls;
-    blacklistedAcls = tempBlacklist;
-  }
-
-  @VisibleForTesting
-  void setKeyACLs(Configuration conf) {
-    Map<String, HashMap<KeyOpType, AccessControlList>> tempKeyAcls =
-        new HashMap<String, HashMap<KeyOpType,AccessControlList>>();
-    Map<String, String> allKeyACLS =
-        conf.getValByRegex(KMSConfiguration.KEY_ACL_PREFIX_REGEX);
-    for (Map.Entry<String, String> keyAcl : allKeyACLS.entrySet()) {
-      String k = keyAcl.getKey();
-      // this should be of type "key.acl.<KEY_NAME>.<OP_TYPE>"
-      int keyNameStarts = KMSConfiguration.KEY_ACL_PREFIX.length();
-      int keyNameEnds = k.lastIndexOf(".");
-      if (keyNameStarts >= keyNameEnds) {
-        LOG.warn("Invalid key name '{}'", k);
-      } else {
-        String aclStr = keyAcl.getValue();
-        String keyName = k.substring(keyNameStarts, keyNameEnds);
-        String keyOp = k.substring(keyNameEnds + 1);
-        KeyOpType aclType = null;
-        try {
-          aclType = KeyOpType.valueOf(keyOp);
-        } catch (IllegalArgumentException e) {
-          LOG.warn("Invalid key Operation '{}'", keyOp);
-        }
-        if (aclType != null) {
-          // On the assumption this will be single threaded.. else we need to
-          // ConcurrentHashMap
-          HashMap<KeyOpType,AccessControlList> aclMap =
-              tempKeyAcls.get(keyName);
-          if (aclMap == null) {
-            aclMap = new HashMap<KeyOpType, AccessControlList>();
-            tempKeyAcls.put(keyName, aclMap);
-          }
-          aclMap.put(aclType, new AccessControlList(aclStr));
-          LOG.info("KEY_NAME '{}' KEY_OP '{}' ACL '{}'",
-              keyName, aclType, aclStr);
-        }
-      }
-    }
-    keyAcls = tempKeyAcls;
-
-    final Map<KeyOpType, AccessControlList> tempDefaults = new HashMap<>();
-    final Map<KeyOpType, AccessControlList> tempWhitelists = new HashMap<>();
-    for (KeyOpType keyOp : KeyOpType.values()) {
-      parseAclsWithPrefix(conf, KMSConfiguration.DEFAULT_KEY_ACL_PREFIX,
-          keyOp, tempDefaults);
-      parseAclsWithPrefix(conf, KMSConfiguration.WHITELIST_KEY_ACL_PREFIX,
-          keyOp, tempWhitelists);
-    }
-    defaultKeyAcls = tempDefaults;
-    whitelistKeyAcls = tempWhitelists;
-  }
+  /**
+   * First Check if user is in ACL for the KMS operation, if yes, then return
+   * true if user is not present in any configured blacklist for the operation.
+   * 
 
 Review comment:
   whitespace:end of line
   

----------------------------------------------------------------
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]


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to