HDDS-3360. Duplicate Ozone Client in renew/cancel token handlers

hadoop-ozone/dist/src/main/compose/ozone/test.sh

@@ -38,6 +38,8 @@ execute_robot_test scm gdpr
 
 execute_robot_test scm ozonefs/ozonefs.robot
 
+execute_robot_test scm security/ozone-secure-token.robot
+
 execute_robot_test scm s3
 
 execute_robot_test scm recon

hadoop-ozone/dist/src/main/smoketest/security/ozone-secure-token.robot

@@ -0,0 +1,76 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+*** Settings ***
+Documentation       Test token operations
+Library             OperatingSystem
+Library             String
+Library             BuiltIn
+Resource            ../commonlib.robot
+Test Timeout        5 minutes
+
+*** Keywords ***
+Get Token in Secure Cluster
+    Execute                      ozone sh token get > /tmp/token.txt
+    File Should Not Be Empty     /tmp/token.txt
+
+Get Token in Unsecure Cluster
+    ${output} =                  Execute             ozone sh token get
+    Should Contain               ${output}           ozone sh token get
+    Should Contain               ${output}           only when security is enabled
+
+# should be executed after Get Token
+Print Valid Token File
+    ${output} =                  Execute             ozone sh token print
+    Should Not Be Empty          ${output}
+
+Print Nonexistent Token File
+    ${output} =                  Execute             ozone sh token print -t /asdf
+    Should Contain               ${output}           operation failed as token file: /asdf
+
+Renew Token in Secure Cluster
+    ${output} =                  Execute             ozone sh token renew
+    Should contain               ${output}           Token renewed successfully
+
+Renew Token in Unsecure Cluster
+    ${output} =                  Execute             ozone sh token renew
+    Should Contain               ${output}           ozone sh token renew
+    Should Contain               ${output}           only when security is enabled
+
+Cancel Token in Secure Cluster
+    ${output} =                  Execute             ozone sh token cancel
+    Should contain               ${output}           Token canceled successfully
+
+Cancel Token in Unsecure Cluster
+    ${output} =                  Execute             ozone sh token cancel
+    Should Contain               ${output}           ozone sh token cancel
+    Should Contain               ${output}           only when security is enabled
+
+Token Test in Secure Cluster
+    Get Token in Secure Cluster
+    Print Valid Token File
+    Renew Token in Secure Cluster
+    Cancel Token in Secure Cluster
+
+Token Test in Unsecure Cluster
+    Get Token in Unsecure Cluster
+    Renew Token in Unsecure Cluster
+    Cancel Token in Unsecure Cluster
+
+*** Test Cases ***
+Token Test
+    Run Keyword if    '${SECURITY_ENABLED}' == 'false'   Token Test in Unsecure Cluster
+    Run Keyword if    '${SECURITY_ENABLED}' == 'true'    Token Test in Secure Cluster
+    Print Nonexistent Token File

hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/Handler.java

@@ -32,6 +32,7 @@
 import org.apache.hadoop.ozone.client.OzoneClientException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import picocli.CommandLine;
 import picocli.CommandLine.Command;
 import picocli.CommandLine.ParentCommand;
 
@@ -50,6 +51,9 @@ public abstract class Handler implements Callable<Void> {
   @ParentCommand
   private GenericParentCommand parent;
 
+  @CommandLine.Spec
+  private CommandLine.Model.CommandSpec spec;
+
   public boolean isVerbose() {
     return parent.isVerbose();
   }
@@ -58,15 +62,32 @@ public OzoneConfiguration createOzoneConfiguration() {
     return parent.createOzoneConfiguration();
   }
 
-  protected abstract OzoneAddress getAddress() throws OzoneClientException;
+  protected OzoneAddress getAddress() throws OzoneClientException {
+    return new OzoneAddress();
+  }
 
   protected abstract void execute(OzoneClient client, OzoneAddress address)
       throws IOException, OzoneClientException;
 
+  /**
+   * Checks whether the current command should be executed or not.
+   * If it is skipped, an informational message should be output.
+   * Eg. some commands only work in secure clusters.
+   *
+   * @return true if the command should be executed
+   */
+  protected boolean isApplicable() {
+    return true;
+  }
+
   @Override
   public Void call() throws Exception {
     conf = createOzoneConfiguration();
 
+    if (!isApplicable()) {
+      return null;
+    }
+
     OzoneAddress address = getAddress();
     try (OzoneClient client = createClient(address)) {
       if (isVerbose()) {
@@ -83,12 +104,12 @@ protected OzoneClient createClient(OzoneAddress address)
     return address.createClient(conf);
   }
 
-  protected boolean securityEnabled(String commandName) {
+  protected boolean securityEnabled() {
     boolean enabled = OzoneSecurityUtil.isSecurityEnabled(conf);
     if (!enabled) {
       err().printf("Error: '%s' operation works only when security is " +
           "enabled. To enable security set ozone.security.enabled to " +
-          "true.%n", commandName);
+          "true.%n", spec.qualifiedName());
     }
     return enabled;
   }

hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/s3/GetS3SecretHandler.java

@@ -31,13 +31,16 @@
     description = "Returns s3 secret for current user")
 public class GetS3SecretHandler extends S3Handler {
 
+  @Override
+  protected boolean isApplicable() {
+    return securityEnabled();
+  }
+
   @Override
   protected void execute(OzoneClient client, OzoneAddress address)
       throws IOException {
-    if (securityEnabled("s3 getsecret")) {
-      String userName = UserGroupInformation.getCurrentUser().getUserName();
-      out().println(client.getObjectStore().getS3Secret(userName));
-    }
+    String userName = UserGroupInformation.getCurrentUser().getUserName();
+    out().println(client.getObjectStore().getS3Secret(userName));
   }
 
 }

hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/token/CancelTokenHandler.java

@@ -20,12 +20,7 @@
 
 import org.apache.hadoop.ozone.client.OzoneClient;
 import org.apache.hadoop.ozone.client.OzoneClientException;
-import org.apache.hadoop.ozone.client.OzoneClientFactory;
-import org.apache.hadoop.ozone.security.OzoneTokenIdentifier;
 import org.apache.hadoop.ozone.shell.OzoneAddress;
-import org.apache.hadoop.ozone.shell.Handler;
-import org.apache.hadoop.security.token.Token;
-import picocli.CommandLine;
 import picocli.CommandLine.Command;
 
 import java.io.IOException;
@@ -35,27 +30,12 @@
  */
 @Command(name = "cancel",
     description = "cancel a delegation token.")
-public class CancelTokenHandler extends Handler {
-
-  @CommandLine.Mixin
-  private TokenOption tokenFile;
-
-  @Override
-  protected OzoneAddress getAddress() throws OzoneClientException {
-    return new OzoneAddress();
-  }
+public class CancelTokenHandler extends TokenHandler {
 
   @Override
   protected void execute(OzoneClient client, OzoneAddress address)
       throws IOException, OzoneClientException {
-
-    if (securityEnabled("token cancel") && tokenFile.exists()) {
-      Token<OzoneTokenIdentifier> token = tokenFile.decode();
-      try (OzoneClient ozoneClient = OzoneClientFactory.getOzoneClient(
-          getConf(), token)) {
-        ozoneClient.getObjectStore().cancelDelegationToken(token);
-        out().printf("Token canceled successfully.%n");
-      }
-    }
+    client.getObjectStore().cancelDelegationToken(getToken());
+    out().printf("Token canceled successfully.%n");
   }
 }

hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/token/GetTokenHandler.java

@@ -39,7 +39,7 @@
     description = "get a delegation token.")
 public class GetTokenHandler extends Handler {
 
-  @CommandLine.Parameters(arity = "1..1",
+  @CommandLine.Parameters(arity = "0..1",
       description = Shell.OZONE_URI_DESCRIPTION)
   private String uri;
 
@@ -51,19 +51,22 @@ protected OzoneAddress getAddress() throws OzoneClientException {
     return new OzoneAddress(uri);
   }
 
+  @Override
+  protected boolean isApplicable() {
+    return securityEnabled();
+  }
+
   @Override
   protected void execute(OzoneClient client, OzoneAddress address)
       throws IOException, OzoneClientException {
 
-    if (securityEnabled("token get")) {
-      Token<OzoneTokenIdentifier> token = client.getObjectStore()
-          .getDelegationToken(new Text(renewer.getValue()));
-      if (Objects.isNull(token)) {
-        err().println("Error: Get delegation token operation failed. " +
-            "Check OzoneManager logs for more details.");
-      } else {
-        printObjectAsJson(token.encodeToUrlString());
-      }
+    Token<OzoneTokenIdentifier> token = client.getObjectStore()
+        .getDelegationToken(new Text(renewer.getValue()));
+    if (Objects.isNull(token)) {
+      err().println("Error: Get delegation token operation failed. " +
+          "Check OzoneManager logs for more details.");
+    } else {
+      printObjectAsJson(token.encodeToUrlString());
     }
   }
 }

hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/token/RenewTokenHandler.java

@@ -20,12 +20,7 @@
 
 import org.apache.hadoop.ozone.client.OzoneClient;
 import org.apache.hadoop.ozone.client.OzoneClientException;
-import org.apache.hadoop.ozone.client.OzoneClientFactory;
-import org.apache.hadoop.ozone.security.OzoneTokenIdentifier;
-import org.apache.hadoop.ozone.shell.Handler;
 import org.apache.hadoop.ozone.shell.OzoneAddress;
-import org.apache.hadoop.security.token.Token;
-import picocli.CommandLine;
 import picocli.CommandLine.Command;
 
 import java.io.IOException;
@@ -35,29 +30,12 @@
  */
 @Command(name = "renew",
     description = "renew a delegation token.")
-public class RenewTokenHandler extends Handler {
-
-  @CommandLine.Mixin
-  private TokenOption tokenFile;
-
-  @Override
-  protected OzoneAddress getAddress() throws OzoneClientException {
-    return new OzoneAddress();
-  }
+public class RenewTokenHandler extends TokenHandler {
 
   @Override
   protected void execute(OzoneClient client, OzoneAddress address)
       throws IOException, OzoneClientException {
-
-    if (securityEnabled("token renew") && tokenFile.exists()) {
-      Token<OzoneTokenIdentifier> token = tokenFile.decode();
-      try (OzoneClient ozoneClient = OzoneClientFactory.getOzoneClient(
-          getConf(), token)) {
-        long expiryTime = ozoneClient.getObjectStore()
-            .renewDelegationToken(token);
-        out().printf("Token renewed successfully, expiry time: %s.%n",
-            expiryTime);
-      }
-    }
+    long expiryTime = client.getObjectStore().renewDelegationToken(getToken());
+    out().printf("Token renewed successfully, expiry time: %s.%n", expiryTime);
   }
 }

hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/token/TokenHandler.java

@@ -0,0 +1,54 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.ozone.shell.token;
+
+import org.apache.hadoop.ozone.client.OzoneClient;
+import org.apache.hadoop.ozone.client.OzoneClientFactory;
+import org.apache.hadoop.ozone.security.OzoneTokenIdentifier;
+import org.apache.hadoop.ozone.shell.Handler;
+import org.apache.hadoop.ozone.shell.OzoneAddress;
+import org.apache.hadoop.security.token.Token;
+import picocli.CommandLine;
+
+import java.io.IOException;
+
+/**
+ * Handler for requests with an existing token.
+ */
+public abstract class TokenHandler extends Handler {
+
+  @CommandLine.Mixin
+  private TokenOption tokenFile;
+  private Token<OzoneTokenIdentifier> token;
+
+  @Override
+  protected boolean isApplicable() {
+    return securityEnabled() && tokenFile.exists();
+  }
+
+  @Override
+  protected OzoneClient createClient(OzoneAddress address)
+      throws IOException {
+    token = tokenFile.decode();
+    return OzoneClientFactory.getOzoneClient(getConf(), token);
+  }
+
+  Token<OzoneTokenIdentifier> getToken() {
+    return token;
+  }
+}