HDFS-986. Delegation token renewing and cancelling should provide

meaningful exceptions when there are failures instead of returning 
false. (omalley)


git-svn-id: https://svn.apache.org/repos/asf/hadoop/hdfs/trunk@911744 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -66,6 +66,10 @@ Trunk (unreleased changes)
     HDFS-930. Better error message for DATA_TRANSFER_VERSION mismatched.
     (Kay Kay via szetszwo)
 
+    HDFS-986. Delegation token renewing and cancelling should provide
+    meaningful exceptions when there are failures instead of returning 
+    false. (omalley)
+
   OPTIMIZATIONS
 
   BUG FIXES

src/java/org/apache/hadoop/hdfs/DFSClient.java

@@ -347,18 +347,24 @@ public class DFSClient implements FSConstants, java.io.Closeable {
     return namenode.getDelegationToken(renewer);
   }
 
-  public Boolean renewDelegationToken(Token<DelegationTokenIdentifier> token)
+  public long renewDelegationToken(Token<DelegationTokenIdentifier> token)
       throws InvalidToken, IOException {
     try {
       return namenode.renewDelegationToken(token);
     } catch (RemoteException re) {
-      throw re.unwrapRemoteException(InvalidToken.class);
+      throw re.unwrapRemoteException(InvalidToken.class,
+                                     AccessControlException.class);
     }
   }
 
-  public Boolean cancelDelegationToken(Token<DelegationTokenIdentifier> token)
-      throws IOException {
-    return namenode.cancelDelegationToken(token);
+  public void cancelDelegationToken(Token<DelegationTokenIdentifier> token)
+      throws InvalidToken, IOException {
+    try {
+      namenode.cancelDelegationToken(token);
+    } catch (RemoteException re) {
+      throw re.unwrapRemoteException(InvalidToken.class,
+                                     AccessControlException.class);
+    }
   }
   
   /**

src/java/org/apache/hadoop/hdfs/DistributedFileSystem.java

@@ -623,10 +623,10 @@ public class DistributedFileSystem extends FileSystem {
    * Renew an existing delegation token.
    * 
    * @param token delegation token obtained earlier
-   * @return True if renewed successfully else false
+   * @return the new expiration time
    * @throws IOException
    */
-  public Boolean renewDelegationToken(Token<DelegationTokenIdentifier> token)
+  public long renewDelegationToken(Token<DelegationTokenIdentifier> token)
       throws InvalidToken, IOException {
     return dfs.renewDelegationToken(token);
   }
@@ -635,11 +635,10 @@ public class DistributedFileSystem extends FileSystem {
    * Cancel an existing delegation token.
    * 
    * @param token delegation token
-   * @return True if canceled successfully else false
    * @throws IOException
    */
-  public Boolean cancelDelegationToken(Token<DelegationTokenIdentifier> token)
+  public void cancelDelegationToken(Token<DelegationTokenIdentifier> token)
       throws IOException {
-    return dfs.cancelDelegationToken(token);
+    dfs.cancelDelegationToken(token);
   }
 }

src/java/org/apache/hadoop/hdfs/protocol/ClientProtocol.java

@@ -602,19 +602,18 @@ public interface ClientProtocol extends VersionedProtocol {
    * Renew an existing delegation token.
    * 
    * @param token delegation token obtained earlier
-   * @return True if renewed successfully else false
+   * @return the new expiration time
    * @throws IOException
    */
-  public Boolean renewDelegationToken(Token<DelegationTokenIdentifier> token)
+  public long renewDelegationToken(Token<DelegationTokenIdentifier> token)
       throws IOException;
   
   /**
    * Cancel an existing delegation token.
    * 
    * @param token delegation token
-   * @return True if canceled successfully else false
    * @throws IOException
    */
-  public Boolean cancelDelegationToken(Token<DelegationTokenIdentifier> token)
+  public void cancelDelegationToken(Token<DelegationTokenIdentifier> token)
       throws IOException;
 }

src/java/org/apache/hadoop/hdfs/security/token/delegation/DelegationTokenIdentifier.java

@@ -28,7 +28,7 @@ import org.apache.hadoop.security.token.delegation.AbstractDelegationTokenIdenti
 @InterfaceAudience.Private
 public class DelegationTokenIdentifier 
     extends AbstractDelegationTokenIdentifier {
-  static final Text HDFS_DELEGATION_KIND = new Text("HDFS_DELEGATION_TOKEN");
+  public static final Text HDFS_DELEGATION_KIND = new Text("HDFS_DELEGATION_TOKEN");
 
   /**
    * Create an empty delegation token identifier for reading into.

src/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java

@@ -4354,15 +4354,15 @@ public class FSNamesystem implements FSConstants, FSNamesystemMBean, FSClusterSt
     return new Token<DelegationTokenIdentifier>(dtId, dtSecretManager);
   }
 
-  public Boolean renewDelegationToken(Token<DelegationTokenIdentifier> token)
+  public long renewDelegationToken(Token<DelegationTokenIdentifier> token)
       throws InvalidToken, IOException {
     String renewer = UserGroupInformation.getCurrentUser().getShortUserName();
     return dtSecretManager.renewToken(token, renewer);
   }
 
-  public Boolean cancelDelegationToken(Token<DelegationTokenIdentifier> token)
+  public void cancelDelegationToken(Token<DelegationTokenIdentifier> token)
       throws IOException {
     String canceller = UserGroupInformation.getCurrentUser().getShortUserName();
-    return dtSecretManager.cancelToken(token, canceller);
+    dtSecretManager.cancelToken(token, canceller);
   }
 }

src/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java

@@ -556,14 +556,16 @@ public class NameNode implements ClientProtocol, DatanodeProtocol,
     return namesystem.getDelegationToken(renewer);
   }
 
-  public Boolean renewDelegationToken(Token<DelegationTokenIdentifier> token)
+  @Override
+  public long renewDelegationToken(Token<DelegationTokenIdentifier> token)
       throws InvalidToken, IOException {
     return namesystem.renewDelegationToken(token);
   }
 
-  public Boolean cancelDelegationToken(Token<DelegationTokenIdentifier> token)
+  @Override
+  public void cancelDelegationToken(Token<DelegationTokenIdentifier> token)
       throws IOException {
-    return namesystem.cancelDelegationToken(token);
+    namesystem.cancelDelegationToken(token);
   }
   
   /** {@inheritDoc} */

src/test/hdfs/org/apache/hadoop/hdfs/TestDFSClientRetries.java

@@ -201,14 +201,13 @@ public class TestDFSClientRetries extends TestCase {
       return null;
     }
     
-    public Boolean renewDelegationToken(Token<DelegationTokenIdentifier> token)
+    public long renewDelegationToken(Token<DelegationTokenIdentifier> token)
         throws InvalidToken, IOException {
-      return false;
+      return 0;
     }
 
-    public Boolean cancelDelegationToken(Token<DelegationTokenIdentifier> token)
+    public void cancelDelegationToken(Token<DelegationTokenIdentifier> token)
         throws IOException {
-      return false;
     }
     
     

src/test/hdfs/org/apache/hadoop/hdfs/security/TestDelegationToken.java

@@ -34,6 +34,8 @@ import org.apache.hadoop.hdfs.DistributedFileSystem;
 import org.apache.hadoop.hdfs.HdfsConfiguration;
 import org.apache.hadoop.hdfs.MiniDFSCluster;
 import org.apache.hadoop.io.Text;
+import org.apache.hadoop.security.AccessControlException;
+import org.apache.hadoop.security.UserGroupInformation;
 import org.apache.hadoop.security.token.Token;
 import org.apache.hadoop.security.token.SecretManager.InvalidToken;
 import org.apache.hadoop.hdfs.security.token.delegation.DelegationTokenIdentifier;
@@ -80,8 +82,13 @@ public class TestDelegationToken {
     Token<DelegationTokenIdentifier> token = generateDelegationToken(
         "SomeUser", "JobTracker");
     // Fake renewer should not be able to renew
-	  Assert.assertFalse(dtSecretManager.renewToken(token, "FakeRenewer"));
-	  Assert.assertTrue(dtSecretManager.renewToken(token, "JobTracker"));
+    try {
+  	  dtSecretManager.renewToken(token, "FakeRenewer");
+  	  Assert.fail("should have failed");
+    } catch (AccessControlException ace) {
+      // PASS
+    }
+	  dtSecretManager.renewToken(token, "JobTracker");
     DelegationTokenIdentifier identifier = new DelegationTokenIdentifier();
     byte[] tokenId = token.getIdentifier();
     identifier.readFields(new DataInputStream(
@@ -97,10 +104,15 @@ public class TestDelegationToken {
 	  } catch (InvalidToken e) {
 	    //Success
 	  }
-	  Assert.assertTrue(dtSecretManager.renewToken(token, "JobTracker"));
+	  dtSecretManager.renewToken(token, "JobTracker");
 	  Log.info("Sleep beyond the max lifetime");
 	  Thread.sleep(5000);
-	  Assert.assertFalse(dtSecretManager.renewToken(token, "JobTracker"));
+	  try {
+  	  dtSecretManager.renewToken(token, "JobTracker");
+  	  Assert.fail("should have been expired");
+	  } catch (InvalidToken it) {
+	    // PASS
+	  }
   }
   
   @Test 
@@ -110,9 +122,19 @@ public class TestDelegationToken {
     Token<DelegationTokenIdentifier> token = generateDelegationToken(
         "SomeUser", "JobTracker");
     //Fake renewer should not be able to renew
-    Assert.assertFalse(dtSecretManager.cancelToken(token, "FakeCanceller"));
-    Assert.assertTrue(dtSecretManager.cancelToken(token, "JobTracker"));
-    Assert.assertFalse(dtSecretManager.renewToken(token, "JobTracker"));
+    try {
+      dtSecretManager.cancelToken(token, "FakeCanceller");
+      Assert.fail("should have failed");
+    } catch (AccessControlException ace) {
+      // PASS
+    }
+    dtSecretManager.cancelToken(token, "JobTracker");
+    try {
+      dtSecretManager.renewToken(token, "JobTracker");
+      Assert.fail("should have failed");
+    } catch (InvalidToken it) {
+      // PASS
+    }
   }
   
   @Test
@@ -126,6 +148,7 @@ public class TestDelegationToken {
              new ByteArrayInputStream(tokenId)));
     Log.info("A valid token should have non-null password, and should be renewed successfully");
     Assert.assertTrue(null != dtSecretManager.retrievePassword(identifier));
-    Assert.assertTrue(dtSecretManager.renewToken(token, "JobTracker"));
-  } 
+    dtSecretManager.renewToken(token, "JobTracker");
+  }
+ 
 }