You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@accumulo.apache.org by ec...@apache.org on 2013/04/04 16:51:51 UTC

svn commit: r1464582 - in /accumulo/branches/1.5: server/src/main/java/org/apache/accumulo/server/master/tableOps/ server/src/main/java/org/apache/accumulo/server/security/handler/ test/src/main/java/org/apache/accumulo/test/randomwalk/concurrent/

Author: ecn
Date: Thu Apr  4 14:51:51 2013
New Revision: 1464582

URL: http://svn.apache.org/r1464582
Log:
ACCUMULO-1238 throw a better error than permission denied when a table has just been deleted

Modified:
    accumulo/branches/1.5/server/src/main/java/org/apache/accumulo/server/master/tableOps/CreateTable.java
    accumulo/branches/1.5/server/src/main/java/org/apache/accumulo/server/security/handler/ZKPermHandler.java
    accumulo/branches/1.5/test/src/main/java/org/apache/accumulo/test/randomwalk/concurrent/CloneTable.java

Modified: accumulo/branches/1.5/server/src/main/java/org/apache/accumulo/server/master/tableOps/CreateTable.java
URL: http://svn.apache.org/viewvc/accumulo/branches/1.5/server/src/main/java/org/apache/accumulo/server/master/tableOps/CreateTable.java?rev=1464582&r1=1464581&r2=1464582&view=diff
==============================================================================
--- accumulo/branches/1.5/server/src/main/java/org/apache/accumulo/server/master/tableOps/CreateTable.java (original)
+++ accumulo/branches/1.5/server/src/main/java/org/apache/accumulo/server/master/tableOps/CreateTable.java Thu Apr  4 14:51:51 2013
@@ -223,11 +223,6 @@ class SetupPermissions extends MasterRep
   }
   
   @Override
-  public long isReady(long tid, Master environment) throws Exception {
-    return 0;
-  }
-  
-  @Override
   public Repo<Master> call(long tid, Master env) throws Exception {
     // give all table permissions to the creator
     SecurityOperation security = AuditedSecurityOperation.getInstance();

Modified: accumulo/branches/1.5/server/src/main/java/org/apache/accumulo/server/security/handler/ZKPermHandler.java
URL: http://svn.apache.org/viewvc/accumulo/branches/1.5/server/src/main/java/org/apache/accumulo/server/security/handler/ZKPermHandler.java?rev=1464582&r1=1464581&r2=1464582&view=diff
==============================================================================
--- accumulo/branches/1.5/server/src/main/java/org/apache/accumulo/server/security/handler/ZKPermHandler.java (original)
+++ accumulo/branches/1.5/server/src/main/java/org/apache/accumulo/server/security/handler/ZKPermHandler.java Thu Apr  4 14:51:51 2013
@@ -47,6 +47,7 @@ public class ZKPermHandler implements Pe
   private static PermissionHandler zkPermHandlerInstance = null;
   
   private String ZKUserPath;
+  private String ZKTablePath;
   private final ZooCache zooCache;
   private final String ZKUserSysPerms = "/System";
   private final String ZKUserTablePerms = "/Tables";
@@ -59,6 +60,7 @@ public class ZKPermHandler implements Pe
   
   public void initialize(String instanceId, boolean initialize) {
     ZKUserPath = ZKSecurityTool.getInstancePath(instanceId) + "/users";
+    ZKTablePath = ZKSecurityTool.getInstancePath(instanceId) + "/tables";
   }
   
   public ZKPermHandler() {
@@ -66,7 +68,7 @@ public class ZKPermHandler implements Pe
   }
   
   @Override
-  public boolean hasTablePermission(String user, String table, TablePermission permission) {
+  public boolean hasTablePermission(String user, String table, TablePermission permission) throws TableNotFoundException {
     byte[] serializedPerms;
     try {
       String path = ZKUserPath + "/" + user + ZKUserTablePerms + "/" + table;
@@ -74,6 +76,22 @@ public class ZKPermHandler implements Pe
       serializedPerms = ZooReaderWriter.getRetryingInstance().getData(path, null);
     } catch (KeeperException e) {
       if (e.code() == Code.NONODE) {
+        // maybe the table was just deleted?
+        try {
+          // check for existence:
+          ZooReaderWriter.getRetryingInstance().getData(ZKTablePath + "/" + table, null);
+          // it's there, you don't have permission
+          return false;
+        } catch (InterruptedException ex) {
+          log.warn("Unhandled InterruptedException, failing closed for table permission check", e);
+          return false;
+        } catch (KeeperException ex) {
+          // not there, throw an informative exception
+          if (e.code() == Code.NONODE) {
+            throw new TableNotFoundException(null, table, "while checking permissions");
+          }
+          log.warn("Unhandled InterruptedException, failing closed for table permission check", e);
+        }
         return false;
       }
       log.warn("Unhandled KeeperException, failing closed for table permission check", e);

Modified: accumulo/branches/1.5/test/src/main/java/org/apache/accumulo/test/randomwalk/concurrent/CloneTable.java
URL: http://svn.apache.org/viewvc/accumulo/branches/1.5/test/src/main/java/org/apache/accumulo/test/randomwalk/concurrent/CloneTable.java?rev=1464582&r1=1464581&r2=1464582&view=diff
==============================================================================
--- accumulo/branches/1.5/test/src/main/java/org/apache/accumulo/test/randomwalk/concurrent/CloneTable.java (original)
+++ accumulo/branches/1.5/test/src/main/java/org/apache/accumulo/test/randomwalk/concurrent/CloneTable.java Thu Apr  4 14:51:51 2013
@@ -44,8 +44,8 @@ public class CloneTable extends Test {
     boolean flush = rand.nextBoolean();
     
     try {
+      log.debug("Cloning table " + srcTableName + " " + newTableName + " " + flush);
       conn.tableOperations().clone(srcTableName, newTableName, flush, new HashMap<String,String>(), new HashSet<String>());
-      log.debug("Cloned table " + srcTableName + " " + newTableName + " " + flush);
     } catch (TableExistsException e) {
       log.debug("Clone " + srcTableName + " failed, " + newTableName + " exist");
     } catch (TableNotFoundException e) {