kirktrue commented on a change in pull request #11465: URL: https://github.com/apache/kafka/pull/11465#discussion_r745852651
########## File path: tools/src/main/java/org/apache/kafka/tools/OAuthCompatibilityTool.java ########## @@ -208,71 +172,231 @@ public static void main(String[] args) { if (t instanceof ConfigException) { System.out.printf("%n"); - parser.printHelp(); + argsHandler.parser.printHelp(); } Exit.exit(1); } } - private static Map<String, ?> getConfigs(Namespace namespace) { - Map<String, Object> c = new HashMap<>(); - maybeAddInt(namespace, "connectTimeoutMs", c, SASL_LOGIN_CONNECT_TIMEOUT_MS); - maybeAddInt(namespace, "readTimeoutMs", c, SASL_LOGIN_READ_TIMEOUT_MS); - maybeAddLong(namespace, "loginRetryBackoffMs", c, SASL_LOGIN_RETRY_BACKOFF_MS); - maybeAddLong(namespace, "loginRetryBackoffMax", c, SASL_LOGIN_RETRY_BACKOFF_MAX_MS); - maybeAddString(namespace, "scopeClaimName", c, SASL_OAUTHBEARER_SCOPE_CLAIM_NAME); - maybeAddString(namespace, "subClaimName", c, SASL_OAUTHBEARER_SUB_CLAIM_NAME); - maybeAddString(namespace, "tokenEndpointUrl", c, SASL_OAUTHBEARER_TOKEN_ENDPOINT_URL); - maybeAddString(namespace, "jwksEndpointUrl", c, SASL_OAUTHBEARER_JWKS_ENDPOINT_URL); - maybeAddLong(namespace, "jwksEndpdointRefreshMs", c, SASL_OAUTHBEARER_JWKS_ENDPOINT_REFRESH_MS); - maybeAddLong(namespace, "jwksEndpdointRetryBackoffMaxMs", c, SASL_OAUTHBEARER_JWKS_ENDPOINT_RETRY_BACKOFF_MAX_MS); - maybeAddLong(namespace, "jwksEndpdointRetryBackoffMs", c, SASL_OAUTHBEARER_JWKS_ENDPOINT_RETRY_BACKOFF_MS); - maybeAddInt(namespace, "clockSkewSeconds", c, SASL_OAUTHBEARER_CLOCK_SKEW_SECONDS); - maybeAddStringList(namespace, "expectedAudience", c, SASL_OAUTHBEARER_EXPECTED_AUDIENCE); - maybeAddString(namespace, "expectedIssuer", c, SASL_OAUTHBEARER_EXPECTED_ISSUER); - - // This here is going to fill in all the defaults for the values we don't specify... - ConfigDef cd = new ConfigDef(); - SaslConfigs.addClientSaslSupport(cd); - AbstractConfig config = new AbstractConfig(cd, c); - return config.values(); - } - private static void maybeAddInt(Namespace namespace, String namespaceKey, Map<String, Object> configs, String configsKey) { - Integer value = namespace.getInt(namespaceKey); + private static class ArgsHandler { - if (value != null) - configs.put(configsKey, value); - } + private static final String DESCRIPTION = String.format( + "This tool is used to verify OAuth/OIDC provider compatibility.%n%n" + + "To use, first export KAFKA_OPTS with Java system properties that match%n" + + "your OAuth/OIDC configuration. Next, run the following script to%n" + + "execute the test:%n%n" + + " ./bin/kafka-run-class.sh %s" + + "%n%n" + + "Please refer to the following source files for OAuth/OIDC client and%n" + + "broker configuration options:" + + "%n%n" + + " %s%n" + + " %s%n" + + " %s", + OAuthCompatibilityTool.class.getName(), + SaslConfigs.class.getName(), + SslConfigs.class.getName(), + OAuthBearerLoginCallbackHandler.class.getName()); - private static void maybeAddLong(Namespace namespace, String namespaceKey, Map<String, Object> configs, String configsKey) { - Long value = namespace.getLong(namespaceKey); + private final ArgumentParser parser; - if (value != null) - configs.put(configsKey, value); - } + private ArgsHandler() { + this.parser = ArgumentParsers + .newArgumentParser("oauth-compatibility-tool") + .defaultHelp(true) + .description(DESCRIPTION); + } - private static void maybeAddString(Namespace namespace, String namespaceKey, Map<String, Object> configs, String configsKey) { - String value = namespace.getString(namespaceKey); + private Namespace parseArgs(String[] args) throws ArgumentParserException { + // SASL/OAuth + addArgument(SASL_LOGIN_CONNECT_TIMEOUT_MS, SASL_LOGIN_CONNECT_TIMEOUT_MS_DOC, Integer.class); + addArgument(SASL_LOGIN_READ_TIMEOUT_MS, SASL_LOGIN_READ_TIMEOUT_MS_DOC, Integer.class); + addArgument(SASL_LOGIN_RETRY_BACKOFF_MS, SASL_LOGIN_RETRY_BACKOFF_MS_DOC, Long.class); + addArgument(SASL_LOGIN_RETRY_BACKOFF_MAX_MS, SASL_LOGIN_RETRY_BACKOFF_MAX_MS_DOC, Long.class); + addArgument(SASL_OAUTHBEARER_SCOPE_CLAIM_NAME, SASL_OAUTHBEARER_SCOPE_CLAIM_NAME_DOC); + addArgument(SASL_OAUTHBEARER_SUB_CLAIM_NAME, SASL_OAUTHBEARER_SUB_CLAIM_NAME_DOC); + addArgument(SASL_OAUTHBEARER_TOKEN_ENDPOINT_URL, SASL_OAUTHBEARER_TOKEN_ENDPOINT_URL_DOC); + addArgument(SASL_OAUTHBEARER_JWKS_ENDPOINT_URL, SASL_OAUTHBEARER_JWKS_ENDPOINT_URL_DOC); + addArgument(SASL_OAUTHBEARER_JWKS_ENDPOINT_REFRESH_MS, SASL_OAUTHBEARER_JWKS_ENDPOINT_REFRESH_MS_DOC, Long.class); + addArgument(SASL_OAUTHBEARER_JWKS_ENDPOINT_RETRY_BACKOFF_MAX_MS, SASL_OAUTHBEARER_JWKS_ENDPOINT_RETRY_BACKOFF_MAX_MS_DOC, Long.class); + addArgument(SASL_OAUTHBEARER_JWKS_ENDPOINT_RETRY_BACKOFF_MS, SASL_OAUTHBEARER_JWKS_ENDPOINT_RETRY_BACKOFF_MS_DOC, Long.class); + addArgument(SASL_OAUTHBEARER_CLOCK_SKEW_SECONDS, SASL_OAUTHBEARER_CLOCK_SKEW_SECONDS_DOC, Integer.class); + addArgument(SASL_OAUTHBEARER_EXPECTED_AUDIENCE, SASL_OAUTHBEARER_EXPECTED_AUDIENCE_DOC) + .action(Arguments.append()); + addArgument(SASL_OAUTHBEARER_EXPECTED_ISSUER, SASL_OAUTHBEARER_EXPECTED_ISSUER_DOC); + + // SSL + addArgument(SSL_CIPHER_SUITES_CONFIG, SSL_CIPHER_SUITES_DOC) + .action(Arguments.append()); + addArgument(SSL_ENABLED_PROTOCOLS_CONFIG, SSL_ENABLED_PROTOCOLS_DOC) + .action(Arguments.append()); + addArgument(SSL_ENDPOINT_IDENTIFICATION_ALGORITHM_CONFIG, SSL_ENDPOINT_IDENTIFICATION_ALGORITHM_DOC); + addArgument(SSL_ENGINE_FACTORY_CLASS_CONFIG, SSL_ENGINE_FACTORY_CLASS_DOC); + addArgument(SSL_KEYMANAGER_ALGORITHM_CONFIG, SSL_KEYMANAGER_ALGORITHM_DOC); + addArgument(SSL_KEYSTORE_CERTIFICATE_CHAIN_CONFIG, SSL_KEYSTORE_CERTIFICATE_CHAIN_DOC); + addArgument(SSL_KEYSTORE_KEY_CONFIG, SSL_KEYSTORE_KEY_DOC); + addArgument(SSL_KEYSTORE_LOCATION_CONFIG, SSL_KEYSTORE_LOCATION_DOC); + addArgument(SSL_KEYSTORE_PASSWORD_CONFIG, SSL_KEYSTORE_PASSWORD_DOC); + addArgument(SSL_KEYSTORE_TYPE_CONFIG, SSL_KEYSTORE_TYPE_DOC); + addArgument(SSL_KEY_PASSWORD_CONFIG, SSL_KEY_PASSWORD_DOC); + addArgument(SSL_PROTOCOL_CONFIG, SSL_PROTOCOL_DOC); + addArgument(SSL_PROVIDER_CONFIG, SSL_PROVIDER_DOC); + addArgument(SSL_SECURE_RANDOM_IMPLEMENTATION_CONFIG, SSL_SECURE_RANDOM_IMPLEMENTATION_DOC); + addArgument(SSL_TRUSTMANAGER_ALGORITHM_CONFIG, SSL_TRUSTMANAGER_ALGORITHM_DOC); + addArgument(SSL_TRUSTSTORE_CERTIFICATES_CONFIG, SSL_TRUSTSTORE_CERTIFICATES_DOC); + addArgument(SSL_TRUSTSTORE_LOCATION_CONFIG, SSL_TRUSTSTORE_LOCATION_DOC); + addArgument(SSL_TRUSTSTORE_PASSWORD_CONFIG, SSL_TRUSTSTORE_PASSWORD_DOC); + addArgument(SSL_TRUSTSTORE_TYPE_CONFIG, SSL_TRUSTSTORE_TYPE_DOC); + + // JAAS options... + addArgument(CLIENT_ID_CONFIG, CLIENT_ID_DOC); + addArgument(CLIENT_SECRET_CONFIG, CLIENT_SECRET_DOC); + addArgument(SCOPE_CONFIG, SCOPE_DOC); + + // SSL configuration... + + try { + return parser.parseArgs(args); + } catch (ArgumentParserException e) { + parser.handleError(e); + throw e; + } + } - if (value != null) - configs.put(configsKey, value); - } + private Argument addArgument(String option, String help) { + return addArgument(option, help, String.class); + } - private static void maybeAddStringList(Namespace namespace, String namespaceKey, Map<String, Object> configs, String configsKey) { - String value = namespace.getString(namespaceKey); + private Argument addArgument(String option, String help, Class<?> clazz) { + // Change FOO_BAR into --foo-bar Review comment: > The code replaces . to -, not _. Yes, I'll fix the comment. > Actually, why do we want to change . to -? In the initial version of the tool there were no command line arguments at all; all configuration was done via setting `KAFKA_OPTS`. An early reviewer had requested proper command line arguments for all of the OAuth configuration. So this is an attempt to take the Kafka configuration names and make them more idiomatic for the command line. So instead of: ``` ./bin/kafka-run-class.sh ...OAuthCompatibilityTool sasl.oauthbearer.scope.claim.name foo ``` We have: ``` ./bin/kafka-run-class.sh ...OAuthCompatibilityTool --sasl-oauthbearer-scope-claim-name foo ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org