Skip to content

Always include legacy client options in error output #6502

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lbschanno wants to merge 2 commits into
base: main
Choose a base branch
from
+198 −46
Open

Always include legacy client options in error output #6502

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7a30b0e
Always include legacy client options in error output
lbschanno Aug 14, 2026
8bf41c1
Rename constant OPT_PASSWORD_LONG to OPT_PASSWORD
lbschanno Aug 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
149 changes: 112 additions & 37 deletions core/src/main/java/org/apache/accumulo/core/cli/ClientOpts.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 '<password>', 'pass:<password>',"
+ " 'file:<local file containing the password>' or 'env:<variable containing"
+ " the pass>')",
Expand All @@ -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 <key>=<value>")
@Parameter(names = OPT_OVERRIDE, splitter = NullSplitter.class,
description = "Overrides property in "
+ "accumulo-client.properties. Expected format: -o <key>=<value>")
private List<String> overrides = new ArrayList<>();

public Map<String,String> getOverrides() {
Expand All @@ -178,28 +220,60 @@ public Map<String,String> 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<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() {
Expand Down Expand Up @@ -230,4 +304,5 @@ public Properties getClientProps() {
}
return cachedProps;
}

}
95 changes: 86 additions & 9 deletions core/src/test/java/org/apache/accumulo/core/cli/TestClientOpts.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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));
}

}