sentry-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From asur...@apache.org
Subject incubator-sentry git commit: SENTRY-560: Sentry HDFS Syncup applies duplicate ACLs for the same scope, group and type (Reviewed by Lenni Kuff)
Date Wed, 03 Dec 2014 06:42:26 GMT
Repository: incubator-sentry
Updated Branches:
  refs/heads/master 02041c56d -> 9da6a6733


SENTRY-560: Sentry HDFS Syncup applies duplicate ACLs for the same scope, group and type (Reviewed
by Lenni Kuff)


Project: http://git-wip-us.apache.org/repos/asf/incubator-sentry/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-sentry/commit/9da6a673
Tree: http://git-wip-us.apache.org/repos/asf/incubator-sentry/tree/9da6a673
Diff: http://git-wip-us.apache.org/repos/asf/incubator-sentry/diff/9da6a673

Branch: refs/heads/master
Commit: 9da6a67330f179636d20f5719cd514e6fd8fe892
Parents: 02041c5
Author: Arun Suresh <asuresh@cloudera.com>
Authored: Tue Dec 2 22:42:15 2014 -0800
Committer: Arun Suresh <asuresh@cloudera.com>
Committed: Tue Dec 2 22:42:15 2014 -0800

----------------------------------------------------------------------
 .../sentry/hdfs/SentryAuthorizationInfo.java    | 10 +++-
 .../hdfs/SentryAuthorizationProvider.java       | 48 ++++++++++++++------
 .../hdfs/TestSentryAuthorizationProvider.java   |  1 -
 .../tests/e2e/hdfs/TestHDFSIntegration.java     | 18 ++++++++
 4 files changed, 61 insertions(+), 16 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/9da6a673/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java
----------------------------------------------------------------------
diff --git a/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java
b/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java
index 08807ca..2415890 100644
--- a/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java
+++ b/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java
@@ -17,6 +17,7 @@
  */
 package org.apache.sentry.hdfs;
 
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.Executors;
@@ -37,6 +38,7 @@ import org.slf4j.LoggerFactory;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
+import com.google.common.collect.Lists;
 
 public class SentryAuthorizationInfo implements Runnable {
   private static Logger LOG =
@@ -264,8 +266,12 @@ public class SentryAuthorizationInfo implements Runnable {
     lock.readLock().lock();
     try {
       String authzObj = authzPaths.findAuthzObject(pathElements);
-      return (authzObj != null) ? authzPermissions.getAcls(authzObj) 
-          : Collections.EMPTY_LIST;
+      // Apparently setFAcl throws error if 'group::---' is not present
+      AclEntry noGroup = AclEntry.parseAclEntry("group::---", true);
+      ArrayList<AclEntry> retList = Lists.newArrayList(noGroup);
+      retList.addAll((authzObj != null) ? authzPermissions.getAcls(authzObj)
+          : Collections.EMPTY_LIST);
+      return retList;
     } finally {
       lock.readLock().unlock();
     }

http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/9da6a673/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationProvider.java
----------------------------------------------------------------------
diff --git a/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationProvider.java
b/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationProvider.java
index cd96cc4..001da65 100644
--- a/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationProvider.java
+++ b/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationProvider.java
@@ -20,6 +20,9 @@ package org.apache.sentry.hdfs;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -296,10 +299,6 @@ public class SentryAuthorizationProvider
     builder.setPermission(fsPerm.getGroupAction());
     list.add(builder.build());
     builder.setName(null);
-    builder.setType(AclEntryType.OTHER);
-    builder.setScope(AclEntryScope.ACCESS);
-    builder.setPermission(fsPerm.getOtherAction());
-    list.add(builder.build());
     return list;
   }
 
@@ -311,46 +310,69 @@ public class SentryAuthorizationProvider
     boolean isManaged = false;
     boolean isStale = false;
     boolean hasAuthzObj = false;
+    Map<String, AclEntry> aclMap = null;
     if (!authzInfo.isManaged(pathElements)) {
       isManaged = false;
       f = defaultAuthzProvider.getAclFeature(node, snapshotId);
     } else {
       isManaged = true;
-      List<AclEntry> list = new ArrayList<AclEntry>();
+      aclMap = new HashMap<String, AclEntry>();
       if (originalAuthzAsAcl) {
         String user = defaultAuthzProvider.getUser(node, snapshotId);
         String group = getDefaultProviderGroup(node, snapshotId);
         FsPermission perm = defaultAuthzProvider.getFsPermission(node, snapshotId);
-        list.addAll(createAclEntries(user, group, perm));
+        addToACLMap(aclMap, createAclEntries(user, group, perm));
       } else {
-        list.addAll(createAclEntries(this.user, this.group, this.permission));
+        addToACLMap(aclMap,
+            createAclEntries(this.user, this.group, this.permission));
       }
       if (!authzInfo.isStale()) { 
         isStale = false;
         if (authzInfo.doesBelongToAuthzObject(pathElements)) {
           hasAuthzObj = true;
-          list.addAll(authzInfo.getAclEntries(pathElements));
-          f = new SentryAclFeature(ImmutableList.copyOf(list));
+          addToACLMap(aclMap, authzInfo.getAclEntries(pathElements));
+          f = new SentryAclFeature(ImmutableList.copyOf(aclMap.values()));
         } else {
           hasAuthzObj = false;
           f = defaultAuthzProvider.getAclFeature(node, snapshotId);
         }
       } else {
         isStale = true;
-        f = new SentryAclFeature(ImmutableList.copyOf(list));
+        f = new SentryAclFeature(ImmutableList.copyOf(aclMap.values()));
       }
     }
     if (LOG.isDebugEnabled()) {
-      LOG.debug("### getAclEntry [" + (p == null ? "null" : p) + "] : ["
+      LOG.debug("### getAclEntry \n[" + (p == null ? "null" : p) + "] : ["
           + "isManaged=" + isManaged
           + ", isStale=" + isStale
           + ", hasAuthzObj=" + hasAuthzObj
-          + ", origAuthzAsAcl=" + originalAuthzAsAcl + "]"
-          + "[" + (f == null ? "null" : f.getEntries()) + "]");
+          + ", origAuthzAsAcl=" + originalAuthzAsAcl + "]\n"
+          + "[" + (aclMap == null ? "null" : aclMap) + "]\n"
+          + "[" + (f == null ? "null" : f.getEntries()) + "]\n");
     }
     return f;
   }
 
+  private void addToACLMap(Map<String, AclEntry> map,
+      Collection<AclEntry> entries) {
+    for (AclEntry ent : entries) {
+      String key = (ent.getName() == null ? "" : ent.getName())
+          + ent.getScope() + ent.getType();
+      AclEntry aclEntry = map.get(key);
+      if (aclEntry == null) {
+        map.put(key, ent);
+      } else {
+        map.put(key,
+            new AclEntry.Builder().
+            setName(ent.getName()).
+            setScope(ent.getScope()).
+            setType(ent.getType()).
+            setPermission(ent.getPermission().or(aclEntry.getPermission())).
+            build());
+      }
+    }
+  }
+
   private String getDefaultProviderGroup(INodeAuthorizationInfo node,
       int snapshotId) {
     String group = defaultAuthzProvider.getGroup(node, snapshotId);

http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/9da6a673/sentry-hdfs/sentry-hdfs-namenode-plugin/src/test/java/org/apache/sentry/hdfs/TestSentryAuthorizationProvider.java
----------------------------------------------------------------------
diff --git a/sentry-hdfs/sentry-hdfs-namenode-plugin/src/test/java/org/apache/sentry/hdfs/TestSentryAuthorizationProvider.java
b/sentry-hdfs/sentry-hdfs-namenode-plugin/src/test/java/org/apache/sentry/hdfs/TestSentryAuthorizationProvider.java
index b766a8f..767c8f6 100644
--- a/sentry-hdfs/sentry-hdfs-namenode-plugin/src/test/java/org/apache/sentry/hdfs/TestSentryAuthorizationProvider.java
+++ b/sentry-hdfs/sentry-hdfs-namenode-plugin/src/test/java/org/apache/sentry/hdfs/TestSentryAuthorizationProvider.java
@@ -136,7 +136,6 @@ public class TestSentryAuthorizationProvider {
         List<AclEntry> acls = new ArrayList<AclEntry>();
         acls.add(new AclEntry.Builder().setName(sysUser).setType(AclEntryType.USER).setScope(AclEntryScope.ACCESS).setPermission(FsAction.ALL).build());
         acls.add(new AclEntry.Builder().setName("supergroup").setType(AclEntryType.GROUP).setScope(AclEntryScope.ACCESS).setPermission(FsAction.READ_EXECUTE).build());
-        acls.add(new AclEntry.Builder().setName(null).setType(AclEntryType.OTHER).setScope(AclEntryScope.ACCESS).setPermission(FsAction.READ_EXECUTE).build());
         acls.add(new AclEntry.Builder().setName("user-authz").setType(AclEntryType.USER).setScope(AclEntryScope.ACCESS).setPermission(FsAction.ALL).build());
         Assert.assertEquals(new LinkedHashSet<AclEntry>(acls), new LinkedHashSet<AclEntry>(fs.getAclStatus(path).getEntries()));
 

http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/9da6a673/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java
----------------------------------------------------------------------
diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java
b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java
index c640db5..a33cc15 100644
--- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java
+++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java
@@ -500,6 +500,24 @@ public class TestHDFSIntegration {
     verifyOnPath("/user/hive/warehouse", null, "hbase", false);
     verifyOnAllSubDirs("/user/hive/warehouse/p1", FsAction.READ_EXECUTE, "hbase", true);
 
+    adminUgi.doAs(new PrivilegedExceptionAction<Void>() {
+      @Override
+      public Void run() throws Exception {
+        // Simulate hdfs dfs -setfacl -m <aclantry> <path>
+        AclStatus existing =
+            miniDFS.getFileSystem()
+            .getAclStatus(new Path("/user/hive/warehouse/p1"));
+        ArrayList<AclEntry> newEntries =
+            new ArrayList<AclEntry>(existing.getEntries());
+        newEntries.add(AclEntry.parseAclEntry("user::---", true));
+        newEntries.add(AclEntry.parseAclEntry("group:bla:rwx", true));
+        newEntries.add(AclEntry.parseAclEntry("other::---", true));
+        miniDFS.getFileSystem().setAcl(new Path("/user/hive/warehouse/p1"),
+            newEntries);
+        return null;
+      }
+    });
+
     stmt.execute("revoke select on table p1 from role p1_admin");
     verifyOnAllSubDirs("/user/hive/warehouse/p1", null, "hbase", false);
 


Mime
View raw message