You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2021/11/03 22:47:48 UTC

[GitHub] [kafka] kirktrue opened a new pull request #11465: OAuth updates 1

kirktrue opened a new pull request #11465:
URL: https://github.com/apache/kafka/pull/11465


   *More detailed description of your change,
   if necessary. The PR title and PR message become
   the squashed commit message, so use a separate
   comment to ping reviewers.*
   
   *Summary of testing strategy (including rationale)
   for the feature or bug fix. Unit and/or integration
   tests are expected for any behaviour change and
   system tests should be considered for larger changes.*
   
   ### Committer Checklist (excluded from commit message)
   - [ ] Verify design and implementation 
   - [ ] Verify test coverage and CI build status
   - [ ] Verify documentation (including upgrade notes)
   


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] junrao commented on a change in pull request #11465: OAuth updates 1

Posted by GitBox <gi...@apache.org>.
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] junrao commented on a change in pull request #11465: OAuth updates 1

Posted by GitBox <gi...@apache.org>.
junrao commented on a change in pull request #11465:
URL: https://github.com/apache/kafka/pull/11465#discussion_r745864419



##########
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:
       So, we can't do ./bin/kafka-run-class.sh ...OAuthCompatibilityTool --sasl.oauthbearer.scope.claim.name foo?
   

##########
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" +

Review comment:
       Do we still need to set KAFKA_OPTS now that we have added more command line options?




-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] junrao commented on a change in pull request #11465: OAuth updates 1

Posted by GitBox <gi...@apache.org>.
junrao commented on a change in pull request #11465:
URL: https://github.com/apache/kafka/pull/11465#discussion_r744941462



##########
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" +

Review comment:
       Most users won't have access to our source files. Could we just print out the list of Sasl and SSL config options, etc?

##########
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 _. Actually, why do we want to change . to -?




-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] kirktrue commented on a change in pull request #11465: OAuth updates 1

Posted by GitBox <gi...@apache.org>.
kirktrue commented on a change in pull request #11465:
URL: https://github.com/apache/kafka/pull/11465#discussion_r746067198



##########
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:
       Not presently, but it was an arbitrary choice (by me) to make the arguments more idiomatic. If you'd prefer to leave the configuration options as is, I'm all for it 😄 




-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] kirktrue commented on a change in pull request #11465: OAuth updates 1

Posted by GitBox <gi...@apache.org>.
kirktrue commented on a change in pull request #11465:
URL: https://github.com/apache/kafka/pull/11465#discussion_r746067486



##########
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" +

Review comment:
       No. I've removed that from the help string.




-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] kirktrue commented on a change in pull request #11465: OAuth updates 1

Posted by GitBox <gi...@apache.org>.
kirktrue commented on a change in pull request #11465:
URL: https://github.com/apache/kafka/pull/11465#discussion_r745853515



##########
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" +

Review comment:
       Actually, this is a bit of a vestige of the old way of handing configuration via `KAFKA_OPTS`. All of the configuration is now handled via proper command line options and `--help` will list all the options.




-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] kirktrue commented on pull request #11465: OAuth updates 1

Posted by GitBox <gi...@apache.org>.
kirktrue commented on pull request #11465:
URL: https://github.com/apache/kafka/pull/11465#issuecomment-962191924


   @junrao - perhaps you might have a chance to look at this? It's basically some clean up from the main OAuth merge.
   
   Not sure if this should go directly to `trunk` or the 3.1 branch.


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] kirktrue commented on a change in pull request #11465: OAuth updates 1

Posted by GitBox <gi...@apache.org>.
kirktrue commented on a change in pull request #11465:
URL: https://github.com/apache/kafka/pull/11465#discussion_r747072916



##########
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:
       `OAuthCompatibilityTool` is a tool for testing/verifying compatibility with a given OAuth/OIDC provider. I can change it to "tool" if it makes more sense.




-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] kirktrue commented on pull request #11465: OAuth updates 1

Posted by GitBox <gi...@apache.org>.
kirktrue commented on pull request #11465:
URL: https://github.com/apache/kafka/pull/11465#issuecomment-965843124


   Replaced with discrete pull requests:
   
   - #11484
   - #11486 
   - #11487 
   - #11489 


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] kirktrue closed pull request #11465: OAuth updates 1

Posted by GitBox <gi...@apache.org>.
kirktrue closed pull request #11465:
URL: https://github.com/apache/kafka/pull/11465


   


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] kirktrue commented on a change in pull request #11465: OAuth updates 1

Posted by GitBox <gi...@apache.org>.
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org