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 1fa02a0bd2b..2e378c94c9c 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.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,24 +134,62 @@ public String convert(String value) { } } - /** - * 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_USER_SHORT = "-u"; + public static final String OPT_USER_LONG = "--user"; + public static final String OPT_PASSWORD = "--password"; + public static final String OPT_AUTHS_SHORT = "-auths"; + public static final String OPT_AUTHS_LONG = "--auths"; + public static final String OPT_CONFIG_FILE_SHORT = "-c"; + public static final String OPT_CONFIG_FILE_LONG = "--config-file"; + public static final String OPT_OVERRIDE = "-o"; + + 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 = {"-u", "--user"}, description = "Connection user") + @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 = {OPT_USER_SHORT, OPT_USER_LONG}, description = "Connection user") public String principal = null; - @Parameter(names = "--password", converter = PasswordConverter.class, + @Parameter(names = OPT_PASSWORD, converter = PasswordConverter.class, description = "connection password (can be specified as '', 'pass:'," + " 'file:' or 'env:')", @@ -160,16 +200,18 @@ public AuthenticationToken getToken() { return ClientProperty.getAuthenticationToken(getClientProps()); } - @Parameter(names = {"-auths", "--auths"}, converter = AuthConverter.class, + @Parameter(names = {OPT_AUTHS_SHORT, OPT_AUTHS_LONG}, converter = AuthConverter.class, 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 " - + "accumulo-client.properties. Expected format: -o =") + @Parameter(names = OPT_OVERRIDE, splitter = NullSplitter.class, + description = "Overrides property in " + + "accumulo-client.properties. Expected format: -o =") private List overrides = new ArrayList<>(); public Map getOverrides() { @@ -178,28 +220,60 @@ public Map getOverrides() { @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 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 getPopulatedLegacyOptions() { + Set 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 +304,5 @@ public Properties getClientProps() { } 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 3ff4f2e921a..71fa400bc8a 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: + *
    + *
  1. The option
    2. + *
  2. Whether the option is a boolean flag
    3. + *
  3. What is expected to be present in the error message
    4. + *
+ * + * @return the arguments + */ + private static Stream 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 void testPassword() { 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)); - } - }