You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@accumulo.apache.org by jm...@apache.org on 2022/04/27 11:06:25 UTC

[accumulo] branch main updated: Update ChangeSecret to work as documentation states (#2651)

This is an automated email from the ASF dual-hosted git repository.

jmark99 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/accumulo.git


The following commit(s) were added to refs/heads/main by this push:
     new 1908f066fd Update ChangeSecret to work as documentation states (#2651)
1908f066fd is described below

commit 1908f066fd066096f9e26e8e4a4a8b7494014196
Author: Mark Owens <jm...@apache.org>
AuthorDate: Wed Apr 27 07:06:18 2022 -0400

    Update ChangeSecret to work as documentation states (#2651)
    
    * Update ChangeSecret to work as documentation states
    
    There are inconsistencies in how ChangeSecret should be run, as well as
    exceptions thrown when attempting to execute ChangeSecret.
    
    In two locations, instructions for executing ChangeSecret indicate
    
         ./bin/accumulo org.apache.accumulo.server.util.ChangeSecret
    
    But within accumulo-site.xml, instructions indicate
    
         ./bin/accumulo org.apache.accumulo.server.util.ChangeSecret [oldpasswd] [newpasswd]
    
    Neither one of the two methods work properly.
    
    Running with no parameters throws a NullPointerException. Running with
    the indicated parameters presents a usage message with an error
    message provided. Additionally, the usage message makes no mention of
    the old and new password parameters.
    
    This PR updates site.xml to match the instructions within the rest of
    the code, i.e., execute the command with no parameters.
    
    It also updates the method by which the old and new  parameters
    are collected. This allowed the `Opts` class to be removed along
    with other unneeded code.
    
    The use of `password` is replaced with the string `secret` to
    make the intent more clear.
    
    Co-authored-by: Christopher Tubbs <ct...@apache.org>
---
 .../apache/accumulo/server/util/ChangeSecret.java  | 30 ++++++----------------
 .../src/test/resources/conf/accumulo-site.xml      |  7 +++--
 2 files changed, 11 insertions(+), 26 deletions(-)

diff --git a/server/base/src/main/java/org/apache/accumulo/server/util/ChangeSecret.java b/server/base/src/main/java/org/apache/accumulo/server/util/ChangeSecret.java
index 560fe061e7..127720a644 100644
--- a/server/base/src/main/java/org/apache/accumulo/server/util/ChangeSecret.java
+++ b/server/base/src/main/java/org/apache/accumulo/server/util/ChangeSecret.java
@@ -38,6 +38,7 @@ import org.apache.accumulo.server.ServerContext;
 import org.apache.accumulo.server.ServerDirs;
 import org.apache.accumulo.server.cli.ServerUtilOpts;
 import org.apache.accumulo.server.fs.VolumeManager;
+import org.apache.commons.lang3.StringUtils;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.FileStatus;
 import org.apache.hadoop.fs.Path;
@@ -48,48 +49,33 @@ import org.apache.zookeeper.ZooDefs.Ids;
 import org.apache.zookeeper.data.ACL;
 import org.apache.zookeeper.data.Stat;
 
-import com.beust.jcommander.Parameter;
-
 import io.opentelemetry.api.trace.Span;
 import io.opentelemetry.context.Scope;
 
 public class ChangeSecret {
 
-  static class Opts extends ServerUtilOpts {
-    @Parameter(names = "--old", description = "old zookeeper password", password = true,
-        hidden = true)
-    String oldPass;
-    @Parameter(names = "--new", description = "new zookeeper password", password = true,
-        hidden = true)
-    String newPass;
-  }
-
   public static void main(String[] args) throws Exception {
     var siteConfig = SiteConfiguration.auto();
     var hadoopConf = new Configuration();
 
-    Opts opts = new Opts();
+    ServerUtilOpts opts = new ServerUtilOpts();
     ServerContext context = opts.getServerContext();
     try (var fs = context.getVolumeManager()) {
       ServerDirs serverDirs = new ServerDirs(siteConfig, hadoopConf);
       verifyHdfsWritePermission(serverDirs, fs);
 
-      List<String> argsList = new ArrayList<>(args.length + 2);
-      argsList.add("--old");
-      argsList.add("--new");
-      argsList.addAll(Arrays.asList(args));
-
-      opts.parseArgs(ChangeSecret.class.getName(), args);
+      String oldPass = String.valueOf(System.console().readPassword("Old secret: "));
+      String newPass = String.valueOf(System.console().readPassword("New secret: "));
       Span span = TraceUtil.startSpan(ChangeSecret.class, "main");
       try (Scope scope = span.makeCurrent()) {
 
-        verifyAccumuloIsDown(context, opts.oldPass);
+        verifyAccumuloIsDown(context, oldPass);
 
         final InstanceId newInstanceId = InstanceId.of(UUID.randomUUID());
         updateHdfs(serverDirs, fs, newInstanceId);
-        rewriteZooKeeperInstance(context, newInstanceId, opts.oldPass, opts.newPass);
-        if (opts.oldPass != null) {
-          deleteInstance(context, opts.oldPass);
+        rewriteZooKeeperInstance(context, newInstanceId, oldPass, newPass);
+        if (!StringUtils.isBlank(oldPass)) {
+          deleteInstance(context, oldPass);
         }
         System.out.println("New instance id is " + newInstanceId);
         System.out.println("Be sure to put your new secret in accumulo.properties");
diff --git a/server/manager/src/test/resources/conf/accumulo-site.xml b/server/manager/src/test/resources/conf/accumulo-site.xml
index da0ef3346c..265e51b0b2 100644
--- a/server/manager/src/test/resources/conf/accumulo-site.xml
+++ b/server/manager/src/test/resources/conf/accumulo-site.xml
@@ -41,10 +41,9 @@
   <property>
     <name>instance.secret</name>
     <value>DEFAULT</value>
-    <description>A secret unique to a given instance that all servers must know in order to communicate with one another.
-      Change it before initialization. To
-      change it later use ./bin/accumulo org.apache.accumulo.server.util.ChangeSecret [oldpasswd] [newpasswd],
-      and then update this file.
+    <description>A secret unique to a given instance that all servers must know in order to
+      communicate with one another. Change it before initialization. To change it later use
+      ./bin/accumulo org.apache.accumulo.server.util.ChangeSecret, and then update this file.
     </description>
   </property>