nizhikov commented on code in PR #15873: URL: https://github.com/apache/kafka/pull/15873#discussion_r1592313103
########## core/src/test/java/kafka/admin/ConfigCommandUnitTest.java: ########## @@ -410,6 +448,430 @@ public void testOptionEntityTypeNames() { doTestOptionEntityTypeNames(false); } + @Test + public void shouldFailIfUnrecognisedEntityTypeUsingZookeeper() { + ConfigCommand.ConfigCommandOptions createOpts = new ConfigCommand.ConfigCommandOptions(new String[]{"--zookeeper", ZK_CONNECT, + "--entity-name", "client", "--entity-type", "not-recognised", "--alter", "--add-config", "a=b,c=d"}); + assertThrows(IllegalArgumentException.class, () -> ConfigCommand.alterConfigWithZk(null, createOpts, DUMMY_ADMIN_ZK_CLIENT)); + } + + @Test + public void shouldFailIfUnrecognisedEntityType() { + ConfigCommand.ConfigCommandOptions createOpts = new ConfigCommand.ConfigCommandOptions(new String[]{"--bootstrap-server", "localhost:9092", + "--entity-name", "client", "--entity-type", "not-recognised", "--alter", "--add-config", "a=b,c=d"}); + assertThrows(IllegalArgumentException.class, () -> ConfigCommand.alterConfig(new DummyAdminClient(new Node(1, "localhost", 9092)), createOpts)); + } + + @Test + public void shouldFailIfBrokerEntityTypeIsNotAnIntegerUsingZookeeper() { + ConfigCommand.ConfigCommandOptions createOpts = new ConfigCommand.ConfigCommandOptions(new String[]{"--zookeeper", ZK_CONNECT, + "--entity-name", "A", "--entity-type", "brokers", "--alter", "--add-config", "a=b,c=d"}); + assertThrows(IllegalArgumentException.class, () -> ConfigCommand.alterConfigWithZk(null, createOpts, DUMMY_ADMIN_ZK_CLIENT)); + } + + @Test + public void shouldFailIfBrokerEntityTypeIsNotAnInteger() { + ConfigCommand.ConfigCommandOptions createOpts = new ConfigCommand.ConfigCommandOptions(new String[]{"--bootstrap-server", "localhost:9092", + "--entity-name", "A", "--entity-type", "brokers", "--alter", "--add-config", "a=b,c=d"}); + assertThrows(IllegalArgumentException.class, () -> ConfigCommand.alterConfig(new DummyAdminClient(new Node(1, "localhost", 9092)), createOpts)); + } + + @Test + public void shouldFailIfShortBrokerEntityTypeIsNotAnIntegerUsingZookeeper() { + ConfigCommand.ConfigCommandOptions createOpts = new ConfigCommand.ConfigCommandOptions(new String[]{"--zookeeper", ZK_CONNECT, + "--broker", "A", "--alter", "--add-config", "a=b,c=d"}); + assertThrows(IllegalArgumentException.class, () -> ConfigCommand.alterConfigWithZk(null, createOpts, DUMMY_ADMIN_ZK_CLIENT)); + } + + @Test + public void shouldFailIfShortBrokerEntityTypeIsNotAnInteger() { + ConfigCommand.ConfigCommandOptions createOpts = new ConfigCommand.ConfigCommandOptions(new String[]{"--bootstrap-server", "localhost:9092", + "--broker", "A", "--alter", "--add-config", "a=b,c=d"}); + assertThrows(IllegalArgumentException.class, () -> ConfigCommand.alterConfig(new DummyAdminClient(new Node(1, "localhost", 9092)), createOpts)); + } + + @Test + public void shouldFailIfMixedEntityTypeFlagsUsingZookeeper() { + ConfigCommand.ConfigCommandOptions createOpts = new ConfigCommand.ConfigCommandOptions(new String[]{"--zookeeper", ZK_CONNECT, + "--entity-name", "A", "--entity-type", "users", "--client", "B", "--describe"}); + assertThrows(IllegalArgumentException.class, createOpts::checkArgs); + } + + @Test + public void shouldFailIfMixedEntityTypeFlags() { + ConfigCommand.ConfigCommandOptions createOpts = new ConfigCommand.ConfigCommandOptions(new String[]{"--bootstrap-server", "localhost:9092", + "--entity-name", "A", "--entity-type", "users", "--client", "B", "--describe"}); + assertThrows(IllegalArgumentException.class, createOpts::checkArgs); + } + + @Test + public void shouldFailIfInvalidHost() { + ConfigCommand.ConfigCommandOptions createOpts = new ConfigCommand.ConfigCommandOptions(new String[]{"--bootstrap-server", "localhost:9092", + "--entity-name", "A,B", "--entity-type", "ips", "--describe"}); + assertThrows(IllegalArgumentException.class, createOpts::checkArgs); + } + + @Test + public void shouldFailIfInvalidHostUsingZookeeper() { + ConfigCommand.ConfigCommandOptions createOpts = new ConfigCommand.ConfigCommandOptions(new String[]{"--zookeeper", ZK_CONNECT, + "--entity-name", "A,B", "--entity-type", "ips", "--describe"}); + assertThrows(IllegalArgumentException.class, createOpts::checkArgs); + } + + @Test + public void shouldFailIfUnresolvableHost() { + ConfigCommand.ConfigCommandOptions createOpts = new ConfigCommand.ConfigCommandOptions(new String[]{"--bootstrap-server", "localhost:9092", + "--entity-name", "RFC2606.invalid", "--entity-type", "ips", "--describe"}); + assertThrows(IllegalArgumentException.class, createOpts::checkArgs); + } + + @Test + public void shouldFailIfUnresolvableHostUsingZookeeper() { + ConfigCommand.ConfigCommandOptions createOpts = new ConfigCommand.ConfigCommandOptions(new String[]{"--zookeeper", ZK_CONNECT, + "--entity-name", "RFC2606.invalid", "--entity-type", "ips", "--describe"}); + assertThrows(IllegalArgumentException.class, createOpts::checkArgs); + } + + @Test + public void shouldAddClientConfigUsingZookeeper() { + ConfigCommand.ConfigCommandOptions createOpts = new ConfigCommand.ConfigCommandOptions(new String[]{"--zookeeper", ZK_CONNECT, + "--entity-name", "my-client-id", + "--entity-type", "clients", + "--alter", + "--add-config", "a=b,c=d"}); + + KafkaZkClient zkClient = mock(KafkaZkClient.class); + when(zkClient.getEntityConfigs(anyString(), anyString())).thenReturn(new Properties()); + + class TestAdminZkClient extends AdminZkClient { + public TestAdminZkClient(KafkaZkClient zkClient) { + super(zkClient, scala.None$.empty()); + } + + @Override + public void changeClientIdConfig(String clientId, Properties configChange) { + assertEquals("my-client-id", clientId); + assertEquals("b", configChange.get("a")); + assertEquals("d", configChange.get("c")); + } + } + + ConfigCommand.alterConfigWithZk(null, createOpts, new TestAdminZkClient(zkClient)); + } + + @Test + public void shouldAddIpConfigsUsingZookeeper() { + ConfigCommand.ConfigCommandOptions createOpts = new ConfigCommand.ConfigCommandOptions(new String[]{"--zookeeper", ZK_CONNECT, + "--entity-name", "1.2.3.4", + "--entity-type", "ips", + "--alter", + "--add-config", "a=b,c=d"}); + + KafkaZkClient zkClient = mock(KafkaZkClient.class); + when(zkClient.getEntityConfigs(anyString(), anyString())).thenReturn(new Properties()); + + class TestAdminZkClient extends AdminZkClient { + public TestAdminZkClient(KafkaZkClient zkClient) { + super(zkClient, scala.None$.empty()); + } + + @Override + public void changeIpConfig(String ip, Properties configChange) { + assertEquals("1.2.3.4", ip); + assertEquals("b", configChange.get("a")); + assertEquals("d", configChange.get("c")); + } + } + + ConfigCommand.alterConfigWithZk(null, createOpts, new TestAdminZkClient(zkClient)); Review Comment: Done ########## core/src/test/java/kafka/admin/ConfigCommandUnitTest.java: ########## @@ -410,6 +448,430 @@ public void testOptionEntityTypeNames() { doTestOptionEntityTypeNames(false); } + @Test + public void shouldFailIfUnrecognisedEntityTypeUsingZookeeper() { + ConfigCommand.ConfigCommandOptions createOpts = new ConfigCommand.ConfigCommandOptions(new String[]{"--zookeeper", ZK_CONNECT, + "--entity-name", "client", "--entity-type", "not-recognised", "--alter", "--add-config", "a=b,c=d"}); + assertThrows(IllegalArgumentException.class, () -> ConfigCommand.alterConfigWithZk(null, createOpts, DUMMY_ADMIN_ZK_CLIENT)); + } + + @Test + public void shouldFailIfUnrecognisedEntityType() { + ConfigCommand.ConfigCommandOptions createOpts = new ConfigCommand.ConfigCommandOptions(new String[]{"--bootstrap-server", "localhost:9092", + "--entity-name", "client", "--entity-type", "not-recognised", "--alter", "--add-config", "a=b,c=d"}); + assertThrows(IllegalArgumentException.class, () -> ConfigCommand.alterConfig(new DummyAdminClient(new Node(1, "localhost", 9092)), createOpts)); + } + + @Test + public void shouldFailIfBrokerEntityTypeIsNotAnIntegerUsingZookeeper() { + ConfigCommand.ConfigCommandOptions createOpts = new ConfigCommand.ConfigCommandOptions(new String[]{"--zookeeper", ZK_CONNECT, + "--entity-name", "A", "--entity-type", "brokers", "--alter", "--add-config", "a=b,c=d"}); + assertThrows(IllegalArgumentException.class, () -> ConfigCommand.alterConfigWithZk(null, createOpts, DUMMY_ADMIN_ZK_CLIENT)); + } + + @Test + public void shouldFailIfBrokerEntityTypeIsNotAnInteger() { + ConfigCommand.ConfigCommandOptions createOpts = new ConfigCommand.ConfigCommandOptions(new String[]{"--bootstrap-server", "localhost:9092", + "--entity-name", "A", "--entity-type", "brokers", "--alter", "--add-config", "a=b,c=d"}); + assertThrows(IllegalArgumentException.class, () -> ConfigCommand.alterConfig(new DummyAdminClient(new Node(1, "localhost", 9092)), createOpts)); + } + + @Test + public void shouldFailIfShortBrokerEntityTypeIsNotAnIntegerUsingZookeeper() { + ConfigCommand.ConfigCommandOptions createOpts = new ConfigCommand.ConfigCommandOptions(new String[]{"--zookeeper", ZK_CONNECT, + "--broker", "A", "--alter", "--add-config", "a=b,c=d"}); + assertThrows(IllegalArgumentException.class, () -> ConfigCommand.alterConfigWithZk(null, createOpts, DUMMY_ADMIN_ZK_CLIENT)); + } + + @Test + public void shouldFailIfShortBrokerEntityTypeIsNotAnInteger() { + ConfigCommand.ConfigCommandOptions createOpts = new ConfigCommand.ConfigCommandOptions(new String[]{"--bootstrap-server", "localhost:9092", + "--broker", "A", "--alter", "--add-config", "a=b,c=d"}); + assertThrows(IllegalArgumentException.class, () -> ConfigCommand.alterConfig(new DummyAdminClient(new Node(1, "localhost", 9092)), createOpts)); + } + + @Test + public void shouldFailIfMixedEntityTypeFlagsUsingZookeeper() { + ConfigCommand.ConfigCommandOptions createOpts = new ConfigCommand.ConfigCommandOptions(new String[]{"--zookeeper", ZK_CONNECT, + "--entity-name", "A", "--entity-type", "users", "--client", "B", "--describe"}); + assertThrows(IllegalArgumentException.class, createOpts::checkArgs); + } + + @Test + public void shouldFailIfMixedEntityTypeFlags() { + ConfigCommand.ConfigCommandOptions createOpts = new ConfigCommand.ConfigCommandOptions(new String[]{"--bootstrap-server", "localhost:9092", + "--entity-name", "A", "--entity-type", "users", "--client", "B", "--describe"}); + assertThrows(IllegalArgumentException.class, createOpts::checkArgs); + } + + @Test + public void shouldFailIfInvalidHost() { + ConfigCommand.ConfigCommandOptions createOpts = new ConfigCommand.ConfigCommandOptions(new String[]{"--bootstrap-server", "localhost:9092", + "--entity-name", "A,B", "--entity-type", "ips", "--describe"}); + assertThrows(IllegalArgumentException.class, createOpts::checkArgs); + } + + @Test + public void shouldFailIfInvalidHostUsingZookeeper() { + ConfigCommand.ConfigCommandOptions createOpts = new ConfigCommand.ConfigCommandOptions(new String[]{"--zookeeper", ZK_CONNECT, + "--entity-name", "A,B", "--entity-type", "ips", "--describe"}); + assertThrows(IllegalArgumentException.class, createOpts::checkArgs); + } + + @Test + public void shouldFailIfUnresolvableHost() { + ConfigCommand.ConfigCommandOptions createOpts = new ConfigCommand.ConfigCommandOptions(new String[]{"--bootstrap-server", "localhost:9092", + "--entity-name", "RFC2606.invalid", "--entity-type", "ips", "--describe"}); + assertThrows(IllegalArgumentException.class, createOpts::checkArgs); + } + + @Test + public void shouldFailIfUnresolvableHostUsingZookeeper() { + ConfigCommand.ConfigCommandOptions createOpts = new ConfigCommand.ConfigCommandOptions(new String[]{"--zookeeper", ZK_CONNECT, + "--entity-name", "RFC2606.invalid", "--entity-type", "ips", "--describe"}); + assertThrows(IllegalArgumentException.class, createOpts::checkArgs); + } + + @Test + public void shouldAddClientConfigUsingZookeeper() { + ConfigCommand.ConfigCommandOptions createOpts = new ConfigCommand.ConfigCommandOptions(new String[]{"--zookeeper", ZK_CONNECT, + "--entity-name", "my-client-id", + "--entity-type", "clients", + "--alter", + "--add-config", "a=b,c=d"}); + + KafkaZkClient zkClient = mock(KafkaZkClient.class); + when(zkClient.getEntityConfigs(anyString(), anyString())).thenReturn(new Properties()); + + class TestAdminZkClient extends AdminZkClient { + public TestAdminZkClient(KafkaZkClient zkClient) { + super(zkClient, scala.None$.empty()); + } + + @Override + public void changeClientIdConfig(String clientId, Properties configChange) { + assertEquals("my-client-id", clientId); + assertEquals("b", configChange.get("a")); + assertEquals("d", configChange.get("c")); + } + } + + ConfigCommand.alterConfigWithZk(null, createOpts, new TestAdminZkClient(zkClient)); Review Comment: Done -- 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