Repository: kafka Updated Branches: refs/heads/trunk 54bf2fb5f -> 916081007
KAFKA-4837: Fix class name comparison in connector-plugins REST endpoint Author: Konstantine Karantasis <[email protected]> Reviewers: Ewen Cheslack-Postava <[email protected]> Closes #2798 from kkonstantine/KAFKA-4837-Config-validation-in-Connector-plugins-need-to-compare-against-both-canonical-and-simple-class-names Project: http://git-wip-us.apache.org/repos/asf/kafka/repo Commit: http://git-wip-us.apache.org/repos/asf/kafka/commit/91608100 Tree: http://git-wip-us.apache.org/repos/asf/kafka/tree/91608100 Diff: http://git-wip-us.apache.org/repos/asf/kafka/diff/91608100 Branch: refs/heads/trunk Commit: 9160810072e540a3a037481505aaaa23e3b50546 Parents: 54bf2fb Author: Konstantine Karantasis <[email protected]> Authored: Tue Apr 4 15:07:32 2017 -0700 Committer: Ewen Cheslack-Postava <[email protected]> Committed: Tue Apr 4 15:07:32 2017 -0700 ---------------------------------------------------------------------- .../resources/ConnectorPluginsResource.java | 23 +- .../resources/ConnectorPluginsResourceTest.java | 215 ++++++++++++++++++- 2 files changed, 224 insertions(+), 14 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kafka/blob/91608100/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorPluginsResource.java ---------------------------------------------------------------------- diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorPluginsResource.java b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorPluginsResource.java index 0d39e81..37e0f01 100644 --- a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorPluginsResource.java +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorPluginsResource.java @@ -39,6 +39,7 @@ import javax.ws.rs.core.MediaType; @Consumes(MediaType.APPLICATION_JSON) public class ConnectorPluginsResource { + private static final String ALIAS_SUFFIX = "Connector"; private final Herder herder; public ConnectorPluginsResource(Herder herder) { @@ -47,11 +48,18 @@ public class ConnectorPluginsResource { @PUT @Path("/{connectorType}/config/validate") - public ConfigInfos validateConfigs(final @PathParam("connectorType") String connType, - final Map<String, String> connectorConfig) throws Throwable { + public ConfigInfos validateConfigs( + final @PathParam("connectorType") String connType, + final Map<String, String> connectorConfig + ) throws Throwable { String includedConnType = connectorConfig.get(ConnectorConfig.CONNECTOR_CLASS_CONFIG); - if (includedConnType != null && !includedConnType.equals(connType)) - throw new BadRequestException("Included connector type " + includedConnType + " does not match request type " + connType); + if (includedConnType != null + && !normalizedPluginName(includedConnType).endsWith(normalizedPluginName(connType))) { + throw new BadRequestException( + "Included connector type " + includedConnType + " does not match request type " + + connType + ); + } return herder.validateConnectorConfig(connectorConfig); } @@ -61,4 +69,11 @@ public class ConnectorPluginsResource { public List<ConnectorPluginInfo> listConnectorPlugins() { return PluginDiscovery.connectorPlugins(); } + + private String normalizedPluginName(String pluginName) { + // Works for both full and simple class names. In the latter case, it generates the alias. + return pluginName.endsWith(ALIAS_SUFFIX) && pluginName.length() > ALIAS_SUFFIX.length() + ? pluginName.substring(0, pluginName.length() - ALIAS_SUFFIX.length()) + : pluginName; + } } http://git-wip-us.apache.org/repos/asf/kafka/blob/91608100/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorPluginsResourceTest.java ---------------------------------------------------------------------- diff --git a/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorPluginsResourceTest.java b/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorPluginsResourceTest.java index e033b6b..088b520 100644 --- a/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorPluginsResourceTest.java +++ b/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorPluginsResourceTest.java @@ -64,6 +64,8 @@ import java.util.List; import java.util.Map; import java.util.Set; +import javax.ws.rs.BadRequestException; + import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -73,46 +75,61 @@ import static org.junit.Assert.assertTrue; @PowerMockIgnore("javax.management.*") public class ConnectorPluginsResourceTest { - private static Map<String, String> props = new HashMap<>(); + private static Map<String, String> props; + private static Map<String, String> partialProps = new HashMap<>(); static { - props.put("name", "test"); - props.put("test.string.config", "testString"); - props.put("test.int.config", "1"); - props.put("test.list.config", "a,b"); + partialProps.put("name", "test"); + partialProps.put("test.string.config", "testString"); + partialProps.put("test.int.config", "1"); + partialProps.put("test.list.config", "a,b"); + + props = new HashMap<>(partialProps); + props.put("connector.class", ConnectorPluginsResourceTestConnector.class.getSimpleName()); } private static final ConfigInfos CONFIG_INFOS; - private static final int ERROR_COUNT = 1; + private static final ConfigInfos PARTIAL_CONFIG_INFOS; + private static final int ERROR_COUNT = 0; + private static final int PARTIAL_CONFIG_ERROR_COUNT = 1; static { List<ConfigInfo> configs = new LinkedList<>(); + List<ConfigInfo> partialConfigs = new LinkedList<>(); ConfigDef connectorConfigDef = ConnectorConfig.configDef(); List<ConfigValue> connectorConfigValues = connectorConfigDef.validate(props); + List<ConfigValue> partialConnectorConfigValues = connectorConfigDef.validate(partialProps); ConfigInfos result = AbstractHerder.generateResult(ConnectorPluginsResourceTestConnector.class.getName(), connectorConfigDef.configKeys(), connectorConfigValues, Collections.<String>emptyList()); + ConfigInfos partialResult = AbstractHerder.generateResult(ConnectorPluginsResourceTestConnector.class.getName(), connectorConfigDef.configKeys(), partialConnectorConfigValues, Collections.<String>emptyList()); configs.addAll(result.values()); + partialConfigs.addAll(partialResult.values()); ConfigKeyInfo configKeyInfo = new ConfigKeyInfo("test.string.config", "STRING", true, "", "HIGH", "Test configuration for string type.", null, -1, "NONE", "test.string.config", Collections.<String>emptyList()); ConfigValueInfo configValueInfo = new ConfigValueInfo("test.string.config", "testString", Collections.<String>emptyList(), Collections.<String>emptyList(), true); ConfigInfo configInfo = new ConfigInfo(configKeyInfo, configValueInfo); configs.add(configInfo); + partialConfigs.add(configInfo); configKeyInfo = new ConfigKeyInfo("test.int.config", "INT", true, "", "MEDIUM", "Test configuration for integer type.", "Test", 1, "MEDIUM", "test.int.config", Collections.<String>emptyList()); configValueInfo = new ConfigValueInfo("test.int.config", "1", Arrays.asList("1", "2", "3"), Collections.<String>emptyList(), true); configInfo = new ConfigInfo(configKeyInfo, configValueInfo); configs.add(configInfo); + partialConfigs.add(configInfo); configKeyInfo = new ConfigKeyInfo("test.string.config.default", "STRING", false, "", "LOW", "Test configuration with default value.", null, -1, "NONE", "test.string.config.default", Collections.<String>emptyList()); configValueInfo = new ConfigValueInfo("test.string.config.default", "", Collections.<String>emptyList(), Collections.<String>emptyList(), true); configInfo = new ConfigInfo(configKeyInfo, configValueInfo); configs.add(configInfo); + partialConfigs.add(configInfo); configKeyInfo = new ConfigKeyInfo("test.list.config", "LIST", true, "", "HIGH", "Test configuration for list type.", "Test", 2, "LONG", "test.list.config", Collections.<String>emptyList()); configValueInfo = new ConfigValueInfo("test.list.config", "a,b", Arrays.asList("a", "b", "c"), Collections.<String>emptyList(), true); configInfo = new ConfigInfo(configKeyInfo, configValueInfo); configs.add(configInfo); + partialConfigs.add(configInfo); CONFIG_INFOS = new ConfigInfos(ConnectorPluginsResourceTestConnector.class.getName(), ERROR_COUNT, Collections.singletonList("Test"), configs); + PARTIAL_CONFIG_INFOS = new ConfigInfos(ConnectorPluginsResourceTestConnector.class.getName(), PARTIAL_CONFIG_ERROR_COUNT, Collections.singletonList("Test"), partialConfigs); } @Mock @@ -127,7 +144,55 @@ public class ConnectorPluginsResourceTest { } @Test - public void testValidateConfig() throws Throwable { + public void testValidateConfigWithSingleErrorDueToMissingConnectorClassname() throws Throwable { + herder.validateConnectorConfig(EasyMock.eq(partialProps)); + + PowerMock.expectLastCall().andAnswer(new IAnswer<ConfigInfos>() { + @Override + public ConfigInfos answer() { + ConfigDef connectorConfigDef = ConnectorConfig.configDef(); + List<ConfigValue> connectorConfigValues = connectorConfigDef.validate(partialProps); + + Connector connector = new ConnectorPluginsResourceTestConnector(); + Config config = connector.validate(partialProps); + ConfigDef configDef = connector.config(); + Map<String, ConfigDef.ConfigKey> configKeys = configDef.configKeys(); + List<ConfigValue> configValues = config.configValues(); + + Map<String, ConfigDef.ConfigKey> resultConfigKeys = new HashMap<>(configKeys); + resultConfigKeys.putAll(connectorConfigDef.configKeys()); + configValues.addAll(connectorConfigValues); + + return AbstractHerder.generateResult( + ConnectorPluginsResourceTestConnector.class.getName(), + resultConfigKeys, + configValues, + Collections.singletonList("Test") + ); + } + }); + + PowerMock.replayAll(); + + // This call to validateConfigs does not throw a BadRequestException because we've mocked + // validateConnectorConfig. + ConfigInfos configInfos = connectorPluginsResource.validateConfigs( + ConnectorPluginsResourceTestConnector.class.getSimpleName(), + partialProps + ); + assertEquals(PARTIAL_CONFIG_INFOS.name(), configInfos.name()); + assertEquals(PARTIAL_CONFIG_INFOS.errorCount(), configInfos.errorCount()); + assertEquals(PARTIAL_CONFIG_INFOS.groups(), configInfos.groups()); + assertEquals( + new HashSet<>(PARTIAL_CONFIG_INFOS.values()), + new HashSet<>(configInfos.values()) + ); + + PowerMock.verifyAll(); + } + + @Test + public void testValidateConfigWithSimpleName() throws Throwable { herder.validateConnectorConfig(EasyMock.eq(props)); PowerMock.expectLastCall().andAnswer(new IAnswer<ConfigInfos>() { @@ -146,14 +211,24 @@ public class ConnectorPluginsResourceTest { resultConfigKeys.putAll(connectorConfigDef.configKeys()); configValues.addAll(connectorConfigValues); - return AbstractHerder.generateResult(ConnectorPluginsResourceTestConnector.class.getName(), resultConfigKeys, configValues, Collections.singletonList("Test")); + return AbstractHerder.generateResult( + ConnectorPluginsResourceTestConnector.class.getName(), + resultConfigKeys, + configValues, + Collections.singletonList("Test") + ); } }); + PowerMock.replayAll(); - ConfigInfos configInfos = connectorPluginsResource.validateConfigs(ConnectorPluginsResourceTestConnector.class.getName(), props); + // make a request to connector-plugins resource using just the simple class name. + ConfigInfos configInfos = connectorPluginsResource.validateConfigs( + ConnectorPluginsResourceTestConnector.class.getSimpleName(), + props + ); assertEquals(CONFIG_INFOS.name(), configInfos.name()); - assertEquals(CONFIG_INFOS.errorCount(), configInfos.errorCount()); + assertEquals(0, configInfos.errorCount()); assertEquals(CONFIG_INFOS.groups(), configInfos.groups()); assertEquals(new HashSet<>(CONFIG_INFOS.values()), new HashSet<>(configInfos.values())); @@ -161,6 +236,126 @@ public class ConnectorPluginsResourceTest { } @Test + public void testValidateConfigWithAlias() throws Throwable { + herder.validateConnectorConfig(EasyMock.eq(props)); + + PowerMock.expectLastCall().andAnswer(new IAnswer<ConfigInfos>() { + @Override + public ConfigInfos answer() { + ConfigDef connectorConfigDef = ConnectorConfig.configDef(); + List<ConfigValue> connectorConfigValues = connectorConfigDef.validate(props); + + Connector connector = new ConnectorPluginsResourceTestConnector(); + Config config = connector.validate(props); + ConfigDef configDef = connector.config(); + Map<String, ConfigDef.ConfigKey> configKeys = configDef.configKeys(); + List<ConfigValue> configValues = config.configValues(); + + Map<String, ConfigDef.ConfigKey> resultConfigKeys = new HashMap<>(configKeys); + resultConfigKeys.putAll(connectorConfigDef.configKeys()); + configValues.addAll(connectorConfigValues); + + return AbstractHerder.generateResult( + ConnectorPluginsResourceTestConnector.class.getName(), + resultConfigKeys, + configValues, + Collections.singletonList("Test") + ); + } + }); + + PowerMock.replayAll(); + + // make a request to connector-plugins resource using a valid alias. + ConfigInfos configInfos = connectorPluginsResource.validateConfigs( + "ConnectorPluginsResourceTest", + props + ); + assertEquals(CONFIG_INFOS.name(), configInfos.name()); + assertEquals(0, configInfos.errorCount()); + assertEquals(CONFIG_INFOS.groups(), configInfos.groups()); + assertEquals(new HashSet<>(CONFIG_INFOS.values()), new HashSet<>(configInfos.values())); + + PowerMock.verifyAll(); + } + + @Test(expected = BadRequestException.class) + public void testValidateConfigWithNonExistentName() throws Throwable { + herder.validateConnectorConfig(EasyMock.eq(props)); + + PowerMock.expectLastCall().andAnswer(new IAnswer<ConfigInfos>() { + @Override + public ConfigInfos answer() { + ConfigDef connectorConfigDef = ConnectorConfig.configDef(); + List<ConfigValue> connectorConfigValues = connectorConfigDef.validate(props); + + Connector connector = new ConnectorPluginsResourceTestConnector(); + Config config = connector.validate(props); + ConfigDef configDef = connector.config(); + Map<String, ConfigDef.ConfigKey> configKeys = configDef.configKeys(); + List<ConfigValue> configValues = config.configValues(); + + Map<String, ConfigDef.ConfigKey> resultConfigKeys = new HashMap<>(configKeys); + resultConfigKeys.putAll(connectorConfigDef.configKeys()); + configValues.addAll(connectorConfigValues); + + return AbstractHerder.generateResult( + ConnectorPluginsResourceTestConnector.class.getName(), + resultConfigKeys, + configValues, + Collections.singletonList("Test") + ); + } + }); + + PowerMock.replayAll(); + + // make a request to connector-plugins resource using a non-loaded connector with the same + // simple name but different package. + String customClassname = "com.custom.package." + + ConnectorPluginsResourceTestConnector.class.getSimpleName(); + connectorPluginsResource.validateConfigs(customClassname, props); + + PowerMock.verifyAll(); + } + + @Test(expected = BadRequestException.class) + public void testValidateConfigWithNonExistentAlias() throws Throwable { + herder.validateConnectorConfig(EasyMock.eq(props)); + + PowerMock.expectLastCall().andAnswer(new IAnswer<ConfigInfos>() { + @Override + public ConfigInfos answer() { + ConfigDef connectorConfigDef = ConnectorConfig.configDef(); + List<ConfigValue> connectorConfigValues = connectorConfigDef.validate(props); + + Connector connector = new ConnectorPluginsResourceTestConnector(); + Config config = connector.validate(props); + ConfigDef configDef = connector.config(); + Map<String, ConfigDef.ConfigKey> configKeys = configDef.configKeys(); + List<ConfigValue> configValues = config.configValues(); + + Map<String, ConfigDef.ConfigKey> resultConfigKeys = new HashMap<>(configKeys); + resultConfigKeys.putAll(connectorConfigDef.configKeys()); + configValues.addAll(connectorConfigValues); + + return AbstractHerder.generateResult( + ConnectorPluginsResourceTestConnector.class.getName(), + resultConfigKeys, + configValues, + Collections.singletonList("Test") + ); + } + }); + + PowerMock.replayAll(); + + connectorPluginsResource.validateConfigs("ConnectorPluginsTest", props); + + PowerMock.verifyAll(); + } + + @Test public void testListConnectorPlugins() { Set<ConnectorPluginInfo> connectorPlugins = new HashSet<>(connectorPluginsResource.listConnectorPlugins()); assertFalse(connectorPlugins.contains(new ConnectorPluginInfo(Connector.class.getCanonicalName())));
