junrao commented on a change in pull request #11465:
URL: https://github.com/apache/kafka/pull/11465#discussion_r746103012



##########
File path: 
tools/src/main/java/org/apache/kafka/tools/OAuthCompatibilityTool.java
##########
@@ -208,71 +171,219 @@ 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" +
+            "Run the following script to determine the configuration options 
for the test:%n%n" +

Review comment:
       for the test => for the tool?

##########
File path: 
tools/src/main/java/org/apache/kafka/tools/OAuthCompatibilityTool.java
##########
@@ -208,71 +171,221 @@ 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",
+            OAuthCompatibilityTool.class.getName());
+
+        private final ArgumentParser parser;
+
+        private ArgsHandler() {
+            this.parser = ArgumentParsers
+                .newArgumentParser("oauth-compatibility-tool")
+                .defaultHelp(true)
+                .description(DESCRIPTION);
+        }
 
-    private static void maybeAddLong(Namespace namespace, String namespaceKey, 
Map<String, Object> configs, String configsKey) {
-        Long value = namespace.getLong(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 maybeAddString(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
+            String name = "--" + option.toLowerCase(Locale.ROOT).replace('.', 
'-');

Review comment:
       In kafka.admin.ConfigCommand, we allow the user to specify the 
configuration using the dot separated names. To be consistent, it's better to 
do the same thing here.




-- 
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


Reply via email to