This is an automated email from the ASF dual-hosted git repository.
dlmarion 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 be10fb3aba Always include legacy client options in error output (#6502)
be10fb3aba is described below
commit be10fb3aba96b48c00609f770b74311f62ca9659
Author: Laura Schanno <[email protected]>
AuthorDate: Mon Aug 31 09:51:18 2026 -0400
Always include legacy client options in error output (#6502)
The ClientOpts class does not always include the names of legacy options
in its error output when a user provides legacy options to the Accumulo
client.
Made the following changes:
- Break the legacy options in ClientOpts into individual properties that
can be set by JCommander.
- Add `ClientOpts.getPopulatedLegacyOptions()` which will return a set
of all user-provided legacy options.
- Update `ClientOpts.validateArgs()` to check for the presence of any
legacy options, and include them in the error message.
Fixes #6501
---
.../org/apache/accumulo/core/cli/ClientOpts.java | 132 ++++++++++++++++-----
.../apache/accumulo/core/cli/TestClientOpts.java | 95 +++++++++++++--
2 files changed, 186 insertions(+), 41 deletions(-)
diff --git a/core/src/main/java/org/apache/accumulo/core/cli/ClientOpts.java
b/core/src/main/java/org/apache/accumulo/core/cli/ClientOpts.java
index 1fa02a0bd2..0019fbabab 100644
--- a/core/src/main/java/org/apache/accumulo/core/cli/ClientOpts.java
+++ b/core/src/main/java/org/apache/accumulo/core/cli/ClientOpts.java
@@ -25,10 +25,12 @@ import java.net.URL;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Collections;
+import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Properties;
import java.util.Scanner;
+import java.util.Set;
import org.apache.accumulo.core.client.security.tokens.AuthenticationToken;
import org.apache.accumulo.core.clientImpl.ClientInfoImpl;
@@ -132,19 +134,51 @@ public class ClientOpts extends Help {
}
}
- /**
- * A catch all for older legacy options that have been dropped. Most of them
were replaced with
- * accumulo-client.properties in 2.0. Others have been dropped completely.
- */
- private String[] legacyClientOpts = {"-p", "-tc", "--tokenClass", "-i",
"--instance",
- "--site-file", "--keytab", "--debug", "-fake", "--mock", "--ssl",
"--sasl"};
+ public static final String OPT_CONFIG_FILE_SHORT = "-c";
+ public static final String OPT_CONFIG_FILE_LONG = "--config-file";
+
+ public static final String LEGACY_OPT_PASSWORD = "-p";
+ public static final String LEGACY_OPT_TOKEN_CLASS_SHORT = "-tc";
+ public static final String LEGACY_OPT_TOKEN_CLASS_LONG = "--tokenClass";
+ public static final String LEGACY_OPT_INSTANCE_SHORT = "-i";
+ public static final String LEGACY_OPT_INSTANCE_LONG = "--instance";
+ public static final String LEGACY_OPT_SITE_FILE = "--site-file";
+ public static final String LEGACY_OPT_KEYTAB = "--keytab";
+ public static final String LEGACY_OPT_DEBUG = "--debug";
+ public static final String LEGACY_OPT_FAKE = "--fake";
+ public static final String LEGACY_OPT_MOCK = "--mock";
+ public static final String LEGACY_OPT_SSL = "--ssl";
+ public static final String LEGACY_OPT_SASL = "--sasl";
+
+ @Parameter(names = LEGACY_OPT_PASSWORD, hidden = true)
+ private String legacyPassword;
+
+ @Parameter(names = {LEGACY_OPT_TOKEN_CLASS_SHORT,
LEGACY_OPT_TOKEN_CLASS_LONG}, hidden = true)
+ private String legacyTokenClass;
+
+ @Parameter(names = {LEGACY_OPT_INSTANCE_SHORT, LEGACY_OPT_INSTANCE_LONG},
hidden = true)
+ private String legacyInstance;
+
+ @Parameter(names = LEGACY_OPT_SITE_FILE, hidden = true)
+ private String legacySiteFile;
+
+ @Parameter(names = LEGACY_OPT_KEYTAB, hidden = true)
+ private String legacyKeytab;
- @Parameter(names = {"-p", "-tc", "--tokenClass", "-i", "--instance",
"--site-file", "--keytab"},
- hidden = true)
- private String legacyOpts = null;
+ @Parameter(names = LEGACY_OPT_DEBUG, hidden = true)
+ private boolean legacyDebug;
- @Parameter(names = {"--debug", "-fake", "--mock", "--ssl", "--sasl"}, hidden
= true)
- private boolean legacyOptsBoolean = false;
+ @Parameter(names = LEGACY_OPT_FAKE, hidden = true)
+ private boolean legacyFake;
+
+ @Parameter(names = LEGACY_OPT_MOCK, hidden = true)
+ private boolean legacyMock;
+
+ @Parameter(names = LEGACY_OPT_SSL, hidden = true)
+ private boolean legacySsl;
+
+ @Parameter(names = LEGACY_OPT_SASL, hidden = true)
+ private boolean legacySasl;
@Parameter(names = {"-u", "--user"}, description = "Connection user")
public String principal = null;
@@ -164,8 +198,9 @@ public class ClientOpts extends Help {
description = "the authorizations to use when reading or writing")
public Authorizations auths = Authorizations.EMPTY;
- @Parameter(names = {"-c", "--config-file"}, description = "Read the given
client config file. "
- + "If omitted, the classpath will be searched for file named
accumulo-client.properties")
+ @Parameter(names = {OPT_CONFIG_FILE_SHORT, OPT_CONFIG_FILE_LONG},
+ description = "Read the given client config file. "
+ + "If omitted, the classpath will be searched for file named
accumulo-client.properties")
private String clientConfigFile = null;
@Parameter(names = "-o", splitter = NullSplitter.class, description =
"Overrides property in "
@@ -178,28 +213,60 @@ public class ClientOpts extends Help {
@Override
public void validateArgs() {
- if (legacyOpts != null || legacyOptsBoolean) {
- if (legacyOpts != null) {
- // grab the bad options
- StringBuilder badOptions = new StringBuilder();
- for (String badArg : legacyClientOpts) {
- if (legacyOpts.contains(badArg)) {
- badOptions.append(badArg).append(" ");
- }
- }
- throw new IllegalArgumentException("The Client options: " + badOptions
- + "have been dropped. Use accumulo-client.properties for any
connection or token "
- + "options. See '-c, --config-file' option.");
- }
- if (legacyOptsBoolean) {
- throw new IllegalArgumentException(
- "The Client options: --debug, -fake, --mock, --ssl, --sasl"
- + "have been dropped. Use accumulo-client.properties for any
connection or token "
- + "options. See '-c, --config-file' option.");
- }
+ Set<String> options = getPopulatedLegacyOptions();
+ if (!options.isEmpty()) {
+ String optionsStr = String.join(" ", options);
+ throw new IllegalArgumentException("The Client options " + optionsStr
+ + " have been dropped. Use accumulo-client.properties for any
connection or"
+ + " token options. See '" + ClientOpts.OPT_CONFIG_FILE_SHORT + ", "
+ + ClientOpts.OPT_CONFIG_FILE_LONG + "' option.");
}
}
+ /**
+ * Get the list of legacy options provided to the Accumulo client.
+ *
+ * @return the legacy options
+ */
+ private Set<String> getPopulatedLegacyOptions() {
+ Set<String> options = new LinkedHashSet<>();
+ if (legacyPassword != null) {
+ options.add(LEGACY_OPT_PASSWORD);
+ }
+ if (legacyTokenClass != null) {
+ // Add both the short and long form.
+ options.add(LEGACY_OPT_TOKEN_CLASS_SHORT);
+ options.add(LEGACY_OPT_TOKEN_CLASS_LONG);
+ }
+ if (legacyInstance != null) {
+ // Add both the short and long form.
+ options.add(LEGACY_OPT_INSTANCE_SHORT);
+ options.add(LEGACY_OPT_INSTANCE_LONG);
+ }
+ if (legacySiteFile != null) {
+ options.add(LEGACY_OPT_SITE_FILE);
+ }
+ if (legacyKeytab != null) {
+ options.add(LEGACY_OPT_KEYTAB);
+ }
+ if (legacyDebug) {
+ options.add(LEGACY_OPT_DEBUG);
+ }
+ if (legacyFake) {
+ options.add(LEGACY_OPT_FAKE);
+ }
+ if (legacyMock) {
+ options.add(LEGACY_OPT_MOCK);
+ }
+ if (legacySsl) {
+ options.add(LEGACY_OPT_SSL);
+ }
+ if (legacySasl) {
+ options.add(LEGACY_OPT_SASL);
+ }
+ return options;
+ }
+
private Properties cachedProps = null;
public String getClientConfigFile() {
@@ -230,4 +297,5 @@ public class ClientOpts extends Help {
}
return cachedProps;
}
+
}
diff --git
a/core/src/test/java/org/apache/accumulo/core/cli/TestClientOpts.java
b/core/src/test/java/org/apache/accumulo/core/cli/TestClientOpts.java
index 3ff4f2e921..71fa400bc8 100644
--- a/core/src/test/java/org/apache/accumulo/core/cli/TestClientOpts.java
+++ b/core/src/test/java/org/apache/accumulo/core/cli/TestClientOpts.java
@@ -18,18 +18,104 @@
*/
package org.apache.accumulo.core.cli;
+import static org.apache.accumulo.core.cli.ClientOpts.LEGACY_OPT_DEBUG;
+import static org.apache.accumulo.core.cli.ClientOpts.LEGACY_OPT_FAKE;
+import static org.apache.accumulo.core.cli.ClientOpts.LEGACY_OPT_INSTANCE_LONG;
+import static
org.apache.accumulo.core.cli.ClientOpts.LEGACY_OPT_INSTANCE_SHORT;
+import static org.apache.accumulo.core.cli.ClientOpts.LEGACY_OPT_KEYTAB;
+import static org.apache.accumulo.core.cli.ClientOpts.LEGACY_OPT_MOCK;
+import static org.apache.accumulo.core.cli.ClientOpts.LEGACY_OPT_PASSWORD;
+import static org.apache.accumulo.core.cli.ClientOpts.LEGACY_OPT_SASL;
+import static org.apache.accumulo.core.cli.ClientOpts.LEGACY_OPT_SITE_FILE;
+import static org.apache.accumulo.core.cli.ClientOpts.LEGACY_OPT_SSL;
+import static
org.apache.accumulo.core.cli.ClientOpts.LEGACY_OPT_TOKEN_CLASS_LONG;
+import static
org.apache.accumulo.core.cli.ClientOpts.LEGACY_OPT_TOKEN_CLASS_SHORT;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.params.provider.Arguments.arguments;
import java.util.Properties;
+import java.util.stream.Stream;
import org.apache.accumulo.core.client.security.tokens.PasswordToken;
import org.apache.accumulo.core.conf.ClientProperty;
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.MethodOrderer;
+import org.junit.jupiter.api.Order;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestMethodOrder;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+@TestMethodOrder(MethodOrderer.OrderAnnotation.class)
public class TestClientOpts {
+ /**
+ * Provide a stream of arguments with the following parameters:
+ * <ol>
+ * <li>The option</li>
+ * <li>Whether the option is a boolean flag</li>
+ * <li>What is expected to be present in the error message</li>
+ * </ol>
+ *
+ * @return the arguments
+ */
+ private static Stream<Arguments> provideLegacyOptions() {
+ // @formatter:off
+ return Stream.of(
+ arguments(LEGACY_OPT_PASSWORD, false, LEGACY_OPT_PASSWORD),
+ arguments(LEGACY_OPT_TOKEN_CLASS_SHORT, false,
LEGACY_OPT_TOKEN_CLASS_SHORT + " " + LEGACY_OPT_TOKEN_CLASS_LONG),
+ arguments(LEGACY_OPT_TOKEN_CLASS_LONG, false,
LEGACY_OPT_TOKEN_CLASS_SHORT + " " + LEGACY_OPT_TOKEN_CLASS_LONG),
+ arguments(LEGACY_OPT_INSTANCE_SHORT, false,
LEGACY_OPT_INSTANCE_SHORT + " " + LEGACY_OPT_INSTANCE_LONG),
+ arguments(LEGACY_OPT_INSTANCE_LONG, false,
LEGACY_OPT_INSTANCE_SHORT + " " + LEGACY_OPT_INSTANCE_LONG),
+ arguments(LEGACY_OPT_SITE_FILE, false,
LEGACY_OPT_SITE_FILE),
+ arguments(LEGACY_OPT_KEYTAB, false, LEGACY_OPT_KEYTAB),
+ arguments(LEGACY_OPT_DEBUG, true, LEGACY_OPT_DEBUG),
+ arguments(LEGACY_OPT_FAKE, true, LEGACY_OPT_FAKE),
+ arguments(LEGACY_OPT_MOCK, true, LEGACY_OPT_MOCK),
+ arguments(LEGACY_OPT_SSL, true, LEGACY_OPT_SSL),
+ arguments(LEGACY_OPT_SASL, true, LEGACY_OPT_SASL)
+ );
+ // @formatter:on
+ }
+
+ /**
+ * Verify that if the given legacy option is provided to the Accumulo
client, an error is thrown.
+ *
+ * @param option the option name
+ * @param isFlag whether the option is a flag
+ * @param formattedOption what we expect to see in the error message
+ */
+ @DisplayName("Verify legacy options result in exception")
+ @ParameterizedTest(name = "Option: {0}, Flag: {1}, Expected in message:
''{2}''")
+ @Order(1)
+ @MethodSource("provideLegacyOptions")
+ void testLegacyOptions(String option, boolean isFlag, String
formattedOption) {
+ String[] args = isFlag ? new String[] {option} : new String[] {option,
"value"};
+ ClientOpts opts = new ClientOpts();
+ IllegalArgumentException exception =
+ assertThrows(IllegalArgumentException.class, () ->
opts.parseArgs("test", args));
+ assertTrue(exception.getMessage()
+ .contains("The Client options " + formattedOption + " have been
dropped."));
+ }
+
+ /**
+ * Verify that if multiple legacy options are provided to the Accumulo
client, they are all
+ * included in the error message.
+ */
+ @Test
+ void testMultipleLegacyOptionsAreAllListedInException() {
+ String[] args =
+ {LEGACY_OPT_PASSWORD, "1234", LEGACY_OPT_INSTANCE_SHORT, "myInstance",
LEGACY_OPT_DEBUG};
+ ClientOpts opts = new ClientOpts();
+ IllegalArgumentException exception =
+ assertThrows(IllegalArgumentException.class, () ->
opts.parseArgs("test", args));
+ assertTrue(exception.getMessage()
+ .contains("The Client options -p -i --instance --debug have been
dropped."));
+ }
+
@Test
public void testBasic() {
ClientOpts opts = new ClientOpts();
@@ -66,13 +152,4 @@ public class TestClientOpts {
assertEquals("myinst", props.getProperty("instance.name"));
}
- @Test
- public void testSasl() throws Exception {
- ClientOpts opts = new ClientOpts();
- String[] args =
- new String[] {"--password", "mypass", "-u", "userabc", "-o",
"instance.name=myinst", "-o",
- "instance.zookeepers=zoo1,zoo2", "-o", "auth.principal=user123",
"--sasl"};
- assertThrows(IllegalArgumentException.class, () -> opts.parseArgs("test",
args));
- }
-
}
