kafka-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ewe...@apache.org
Subject kafka git commit: KAFKA-4837: Fix class name comparison in connector-plugins REST endpoint
Date Tue, 04 Apr 2017 22:07:38 GMT
Repository: kafka
Updated Branches:
  refs/heads/trunk 54bf2fb5f -> 916081007


KAFKA-4837: Fix class name comparison in connector-plugins REST endpoint

Author: Konstantine Karantasis <konstantine@confluent.io>

Reviewers: Ewen Cheslack-Postava <ewen@confluent.io>

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 <konstantine@confluent.io>
Authored: Tue Apr 4 15:07:32 2017 -0700
Committer: Ewen Cheslack-Postava <me@ewencp.org>
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())));


Mime
View raw message