You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@accumulo.apache.org by el...@apache.org on 2013/12/26 22:48:17 UTC

[7/8] git commit: ACCUMULO-2096 Clean up the security RW tests.

ACCUMULO-2096 Clean up the security RW tests.

Ensure that tableID is used where necessary and ensure that the correct SecurityErrorCode is used when using the
internal method calls (that throw ThriftSecurityException and not AccumuloSecurityException) so the assertions work
correctly.


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

Branch: refs/heads/master
Commit: 9f59c0022777fc86d0b269d064bcc898b27c9b23
Parents: 06cbd91
Author: Josh Elser <el...@apache.org>
Authored: Thu Dec 26 16:36:25 2013 -0500
Committer: Josh Elser <el...@apache.org>
Committed: Thu Dec 26 16:36:25 2013 -0500

----------------------------------------------------------------------
 .../test/randomwalk/security/AlterTable.java    | 26 ++++++++++-
 .../randomwalk/security/AlterTablePerm.java     | 45 ++++++++++++++------
 .../test/randomwalk/security/ChangePass.java    |  2 +-
 .../test/randomwalk/security/DropTable.java     | 21 ++++++++-
 .../test/randomwalk/security/TableOp.java       |  3 +-
 .../test/randomwalk/security/Validate.java      |  4 +-
 6 files changed, 81 insertions(+), 20 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/accumulo/blob/9f59c002/test/src/main/java/org/apache/accumulo/test/randomwalk/security/AlterTable.java
----------------------------------------------------------------------
diff --git a/test/src/main/java/org/apache/accumulo/test/randomwalk/security/AlterTable.java b/test/src/main/java/org/apache/accumulo/test/randomwalk/security/AlterTable.java
index b2c4c87..91f5ba8 100644
--- a/test/src/main/java/org/apache/accumulo/test/randomwalk/security/AlterTable.java
+++ b/test/src/main/java/org/apache/accumulo/test/randomwalk/security/AlterTable.java
@@ -17,6 +17,7 @@
 package org.apache.accumulo.test.randomwalk.security;
 
 import java.net.InetAddress;
+import java.util.Map;
 import java.util.Properties;
 
 import org.apache.accumulo.core.client.AccumuloException;
@@ -27,8 +28,10 @@ import org.apache.accumulo.core.client.TableNotFoundException;
 import org.apache.accumulo.core.client.security.SecurityErrorCode;
 import org.apache.accumulo.test.randomwalk.State;
 import org.apache.accumulo.test.randomwalk.Test;
+import org.apache.log4j.Logger;
 
 public class AlterTable extends Test {
+  private static final Logger log = Logger.getLogger(AlterTable.class);
   
   @Override
   public void visit(State state, Properties props) throws Exception {
@@ -36,12 +39,33 @@ public class AlterTable extends Test {
     
     String tableName = WalkingSecurity.get(state).getTableName();
     
+    Map<String,String> nameToId = conn.tableOperations().tableIdMap();
+    String tableId = nameToId.get(tableName);
+    
     boolean exists = WalkingSecurity.get(state).getTableExists();
-    boolean hasPermission = WalkingSecurity.get(state).canAlterTable(WalkingSecurity.get(state).getSysCredentials(), tableName);
+    
+    if ((null == tableId && exists) || (null != tableId && !exists)) {
+      log.error("For table " + tableName + ": found table ID " + tableId + " and " + (exists ? "expect" : "did not expect") + " it to exist");
+      throw new TableNotFoundException(null, tableName, "Could not find table ID when it should exist");
+    }
+    
+    boolean hasPermission;
+    try {
+      hasPermission = WalkingSecurity.get(state).canAlterTable(WalkingSecurity.get(state).getSysCredentials(), tableId);
+    } catch (Exception e) {
+      if (!exists) {
+        log.debug("Ignoring exception when trying to alter non-existent table", e);
+        return;
+      }
+      
+      throw e;
+    }
+    
     String newTableName = String.format("security_%s_%s_%d", InetAddress.getLocalHost().getHostName().replaceAll("[-.]", "_"), state.getPid(),
         System.currentTimeMillis());
     
     renameTable(conn, state, tableName, newTableName, hasPermission, exists);
+
   }
   
   public static void renameTable(Connector conn, State state, String oldName, String newName, boolean hasPermission, boolean tableExists)

http://git-wip-us.apache.org/repos/asf/accumulo/blob/9f59c002/test/src/main/java/org/apache/accumulo/test/randomwalk/security/AlterTablePerm.java
----------------------------------------------------------------------
diff --git a/test/src/main/java/org/apache/accumulo/test/randomwalk/security/AlterTablePerm.java b/test/src/main/java/org/apache/accumulo/test/randomwalk/security/AlterTablePerm.java
index bad7b28..4bd0d64 100644
--- a/test/src/main/java/org/apache/accumulo/test/randomwalk/security/AlterTablePerm.java
+++ b/test/src/main/java/org/apache/accumulo/test/randomwalk/security/AlterTablePerm.java
@@ -27,30 +27,32 @@ import org.apache.accumulo.core.security.Credentials;
 import org.apache.accumulo.core.security.TablePermission;
 import org.apache.accumulo.test.randomwalk.State;
 import org.apache.accumulo.test.randomwalk.Test;
+import org.apache.log4j.Logger;
 
 public class AlterTablePerm extends Test {
-  
+  private static final Logger log = Logger.getLogger(AlterTablePerm.class);
+
   @Override
   public void visit(State state, Properties props) throws Exception {
     alter(state, props);
   }
-  
+
   public static void alter(State state, Properties props) throws Exception {
     String action = props.getProperty("task", "toggle");
     String perm = props.getProperty("perm", "random");
     String sourceUserProp = props.getProperty("source", "system");
     String targetUser = props.getProperty("target", "table");
     boolean tabExists = WalkingSecurity.get(state).getTableExists();
-    
+
     String target;
     if ("table".equals(targetUser))
       target = WalkingSecurity.get(state).getTabUserName();
     else
       target = WalkingSecurity.get(state).getSysUserName();
-    
+
     boolean exists = WalkingSecurity.get(state).userExists(target);
     boolean tableExists = WalkingSecurity.get(state).getTableExists();
-    
+
     TablePermission tabPerm;
     if (perm.equals("random")) {
       Random r = new Random();
@@ -74,10 +76,25 @@ public class AlterTablePerm extends Test {
       sourceToken = state.getToken();
     }
     Connector conn = state.getInstance().getConnector(sourceUser, sourceToken);
-    
-    canGive = WalkingSecurity.get(state).canGrantTable(new Credentials(sourceUser, sourceToken).toThrift(state.getInstance()), target,
-        WalkingSecurity.get(state).getTableName());
-    
+    String tableId = conn.tableOperations().tableIdMap().get(tableName);
+
+    // Make sure we get an ID when we can
+    if ((null == tableId && tableExists) || (null != tableId && !tableExists)) {
+      log.error("For table " + tableName + ": found table ID " + tableId + " and we " + (exists ? "expect" : "did not expect") + " it to exist");
+      throw new AccumuloException("Could not find table ID for " + tableName + " but it should have existed");
+    }
+
+    try {
+      canGive = WalkingSecurity.get(state).canGrantTable(new Credentials(sourceUser, sourceToken).toThrift(state.getInstance()), target, tableId);
+    } catch (Exception e) {
+      if (!tableExists) {
+        log.debug("Ignoring exception checking permission on non-existent table", e);
+        return;
+      }
+
+      throw e;
+    }
+
     // toggle
     if (!"take".equals(action) && !"give".equals(action)) {
       try {
@@ -85,7 +102,7 @@ public class AlterTablePerm extends Test {
         if (hasPerm != (res = state.getConnector().securityOperations().hasTablePermission(target, tableName, tabPerm)))
           throw new AccumuloException("Test framework and accumulo are out of sync for user " + conn.whoami() + " for perm " + tabPerm.name()
               + " with local vs. accumulo being " + hasPerm + " " + res);
-        
+
         if (hasPerm)
           action = "take";
         else
@@ -107,7 +124,7 @@ public class AlterTablePerm extends Test {
         }
       }
     }
-    
+
     boolean trans = WalkingSecurity.get(state).userPassTransient(conn.whoami());
     if ("take".equals(action)) {
       try {
@@ -166,14 +183,14 @@ public class AlterTablePerm extends Test {
       }
       WalkingSecurity.get(state).grantTablePermission(target, tableName, tabPerm);
     }
-    
+
     if (!exists)
       throw new AccumuloException("User shouldn't have existed, but apparantly does");
     if (!tableExists)
       throw new AccumuloException("Table shouldn't have existed, but apparantly does");
     if (!canGive)
       throw new AccumuloException(conn.whoami() + " shouldn't have been able to grant privilege");
-    
+
   }
-  
+
 }

http://git-wip-us.apache.org/repos/asf/accumulo/blob/9f59c002/test/src/main/java/org/apache/accumulo/test/randomwalk/security/ChangePass.java
----------------------------------------------------------------------
diff --git a/test/src/main/java/org/apache/accumulo/test/randomwalk/security/ChangePass.java b/test/src/main/java/org/apache/accumulo/test/randomwalk/security/ChangePass.java
index 439e724..6287d6a 100644
--- a/test/src/main/java/org/apache/accumulo/test/randomwalk/security/ChangePass.java
+++ b/test/src/main/java/org/apache/accumulo/test/randomwalk/security/ChangePass.java
@@ -85,7 +85,7 @@ public class ChangePass extends Test {
       }
     }
     WalkingSecurity.get(state).changePassword(target, newPass);
-    // Waiting 1 second for password to propogate through Zk
+    // Waiting 1 second for password to propagate through Zk
     Thread.sleep(1000);
     if (!hasPerm)
       throw new AccumuloException("Password change succeeded when it should have failed for " + source + " changing the password for " + target + ".");

http://git-wip-us.apache.org/repos/asf/accumulo/blob/9f59c002/test/src/main/java/org/apache/accumulo/test/randomwalk/security/DropTable.java
----------------------------------------------------------------------
diff --git a/test/src/main/java/org/apache/accumulo/test/randomwalk/security/DropTable.java b/test/src/main/java/org/apache/accumulo/test/randomwalk/security/DropTable.java
index 52b6e25..4dc25ea 100644
--- a/test/src/main/java/org/apache/accumulo/test/randomwalk/security/DropTable.java
+++ b/test/src/main/java/org/apache/accumulo/test/randomwalk/security/DropTable.java
@@ -28,8 +28,10 @@ import org.apache.accumulo.core.client.security.tokens.AuthenticationToken;
 import org.apache.accumulo.core.security.Credentials;
 import org.apache.accumulo.test.randomwalk.State;
 import org.apache.accumulo.test.randomwalk.Test;
+import org.apache.log4j.Logger;
 
 public class DropTable extends Test {
+  private static final Logger log = Logger.getLogger(DropTable.class);
   
   @Override
   public void visit(State state, Properties props) throws Exception {
@@ -50,9 +52,26 @@ public class DropTable extends Test {
     Connector conn = state.getInstance().getConnector(principal, token);
     
     String tableName = WalkingSecurity.get(state).getTableName();
+    String tableId = conn.tableOperations().tableIdMap().get(tableName);
     
     boolean exists = WalkingSecurity.get(state).getTableExists();
-    boolean hasPermission = WalkingSecurity.get(state).canDeleteTable(new Credentials(principal, token).toThrift(state.getInstance()), tableName);
+    
+    if ((null == tableId && exists) || (null != tableId && !exists)) {
+      log.error("For table " + tableName + ": found table ID " + tableId + " and " + (exists ? "expect" : "did not expect") + " it to exist");
+      throw new AccumuloException("Test and Accumulo state differ on " + tableName + " existence.");
+    }
+    
+    boolean hasPermission;
+    try {
+      hasPermission = WalkingSecurity.get(state).canDeleteTable(new Credentials(principal, token).toThrift(state.getInstance()), tableId);
+    } catch (Exception e) {
+      if (!exists) {
+        log.error("Ignoring exception checking permissions on non-existent table", e);
+        return;
+      }
+      
+      throw e;
+    }
     
     try {
       conn.tableOperations().delete(tableName);

http://git-wip-us.apache.org/repos/asf/accumulo/blob/9f59c002/test/src/main/java/org/apache/accumulo/test/randomwalk/security/TableOp.java
----------------------------------------------------------------------
diff --git a/test/src/main/java/org/apache/accumulo/test/randomwalk/security/TableOp.java b/test/src/main/java/org/apache/accumulo/test/randomwalk/security/TableOp.java
index 347be89..7199322 100644
--- a/test/src/main/java/org/apache/accumulo/test/randomwalk/security/TableOp.java
+++ b/test/src/main/java/org/apache/accumulo/test/randomwalk/security/TableOp.java
@@ -75,13 +75,14 @@ public class TableOp extends Test {
 
     switch (tp) {
       case READ: {
-        boolean canRead = WalkingSecurity.get(state).canScan(WalkingSecurity.get(state).getTabCredentials(), tableId);
         Authorizations auths = WalkingSecurity.get(state).getUserAuthorizations(WalkingSecurity.get(state).getTabCredentials());
         boolean ambiguousZone = WalkingSecurity.get(state).inAmbiguousZone(conn.whoami(), tp);
         boolean ambiguousAuths = WalkingSecurity.get(state).ambiguousAuthorizations(conn.whoami());
 
         Scanner scan = null;
+        boolean canRead = false;
         try {
+          canRead = WalkingSecurity.get(state).canScan(WalkingSecurity.get(state).getTabCredentials(), tableId);
           scan = conn.createScanner(tableName, conn.securityOperations().getUserAuthorizations(conn.whoami()));
           int seen = 0;
           Iterator<Entry<Key,Value>> iter = scan.iterator();

http://git-wip-us.apache.org/repos/asf/accumulo/blob/9f59c002/test/src/main/java/org/apache/accumulo/test/randomwalk/security/Validate.java
----------------------------------------------------------------------
diff --git a/test/src/main/java/org/apache/accumulo/test/randomwalk/security/Validate.java b/test/src/main/java/org/apache/accumulo/test/randomwalk/security/Validate.java
index 2a3e445..48586c3 100644
--- a/test/src/main/java/org/apache/accumulo/test/randomwalk/security/Validate.java
+++ b/test/src/main/java/org/apache/accumulo/test/randomwalk/security/Validate.java
@@ -21,8 +21,8 @@ import java.util.Properties;
 import org.apache.accumulo.core.client.AccumuloException;
 import org.apache.accumulo.core.client.AccumuloSecurityException;
 import org.apache.accumulo.core.client.Connector;
-import org.apache.accumulo.core.client.impl.thrift.ThriftSecurityException;
 import org.apache.accumulo.core.client.security.SecurityErrorCode;
+import org.apache.accumulo.core.client.impl.thrift.ThriftSecurityException;
 import org.apache.accumulo.core.security.Authorizations;
 import org.apache.accumulo.core.security.SystemPermission;
 import org.apache.accumulo.core.security.TablePermission;
@@ -108,7 +108,7 @@ public class Validate extends Test {
       auths = WalkingSecurity.get(state).getUserAuthorizations(WalkingSecurity.get(state).getTabCredentials());
       accuAuths = conn.securityOperations().getUserAuthorizations(WalkingSecurity.get(state).getTabUserName());
     } catch (ThriftSecurityException ae) {
-      if (ae.getCode().equals(SecurityErrorCode.USER_DOESNT_EXIST)) {
+      if (ae.getCode().equals(org.apache.accumulo.core.client.impl.thrift.SecurityErrorCode.USER_DOESNT_EXIST)) {
         if (tableUserExists)
           throw new AccumuloException("Table user didn't exist when they should.", ae);
         else