From bd3caab092ce64da02697bbab1b836c466b41b6f Mon Sep 17 00:00:00 2001 From: Magesh Nandakumar Date: Tue, 23 Apr 2019 14:32:09 -0700 Subject: [PATCH 1/8] Initial implementation for ConnectorClientConfigPOlicy to enable overrides KIP-458 - Connector Client config override policy KIP-458 - Connector Client config override policy Fix checkstyle --- checkstyle/import-control.xml | 8 ++ .../clients/admin/AdminClientConfig.java | 4 + .../clients/consumer/ConsumerConfig.java | 4 + .../clients/producer/ProducerConfig.java | 4 + .../ConnectorClientConfigOverridePolicy.java | 44 +++++++ .../policy/ConnectorClientConfigRequest.java | 105 +++++++++++++++ .../kafka/connect/cli/ConnectDistributed.java | 14 +- .../kafka/connect/cli/ConnectStandalone.java | 13 +- ...llConnectorClientConfigOverridePolicy.java | 40 ++++++ ...neConnectorClientConfigOverridePolicy.java | 42 ++++++ ...alConnectorClientConfigOverridePolicy.java | 49 +++++++ .../kafka/connect/runtime/AbstractHerder.java | 116 ++++++++++++++++- .../apache/kafka/connect/runtime/Worker.java | 117 ++++++++++++++--- .../kafka/connect/runtime/WorkerConfig.java | 11 +- .../distributed/DistributedHerder.java | 12 +- .../errors/DeadLetterQueueReporter.java | 5 +- .../isolation/DelegatingClassLoader.java | 13 +- .../runtime/isolation/PluginScanResult.java | 28 +++- .../connect/runtime/isolation/PluginType.java | 2 + .../runtime/isolation/PluginUtils.java | 1 + .../connect/runtime/isolation/Plugins.java | 7 +- .../runtime/standalone/StandaloneHerder.java | 12 +- ...policy.ConnectorClientConfigOverridePolicy | 18 +++ ...nnectorClientConfigOverridePolicyTest.java | 60 +++++++++ ...nnectorClientConfigOverridePolicyTest.java | 60 +++++++++ .../connect/runtime/AbstractHerderTest.java | 68 ++++++++-- .../kafka/connect/runtime/WorkerTest.java | 123 +++++++++++++++--- .../distributed/DistributedHerderTest.java | 2 +- .../standalone/StandaloneHerderTest.java | 2 +- 29 files changed, 905 insertions(+), 79 deletions(-) create mode 100644 connect/api/src/main/java/org/apache/kafka/connect/connector/policy/ConnectorClientConfigOverridePolicy.java create mode 100644 connect/api/src/main/java/org/apache/kafka/connect/connector/policy/ConnectorClientConfigRequest.java create mode 100644 connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/AllConnectorClientConfigOverridePolicy.java create mode 100644 connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/NoneConnectorClientConfigOverridePolicy.java create mode 100644 connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/PrincipalConnectorClientConfigOverridePolicy.java create mode 100644 connect/runtime/src/main/resources/META-INF/services/org.apache.kafka.connect.connector.policy.ConnectorClientConfigOverridePolicy create mode 100644 connect/runtime/src/test/java/org/apache/kafka/connect/connector/policy/NoneConnectorClientConfigOverridePolicyTest.java create mode 100644 connect/runtime/src/test/java/org/apache/kafka/connect/connector/policy/PrincipalConnectorClientConfigOverridePolicyTest.java diff --git a/checkstyle/import-control.xml b/checkstyle/import-control.xml index 67cbe5a15db37..a76fd1d68a621 100644 --- a/checkstyle/import-control.xml +++ b/checkstyle/import-control.xml @@ -322,6 +322,13 @@ + + + + + + + @@ -356,6 +363,7 @@ + diff --git a/clients/src/main/java/org/apache/kafka/clients/admin/AdminClientConfig.java b/clients/src/main/java/org/apache/kafka/clients/admin/AdminClientConfig.java index 47c76ac36af12..1fa23355a036f 100644 --- a/clients/src/main/java/org/apache/kafka/clients/admin/AdminClientConfig.java +++ b/clients/src/main/java/org/apache/kafka/clients/admin/AdminClientConfig.java @@ -200,6 +200,10 @@ public static Set configNames() { return CONFIG.names(); } + public static ConfigDef configDef() { + return CONFIG; + } + public static void main(String[] args) { System.out.println(CONFIG.toHtmlTable()); } diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/ConsumerConfig.java b/clients/src/main/java/org/apache/kafka/clients/consumer/ConsumerConfig.java index c9b500450891d..ba1928ed4800f 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/ConsumerConfig.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/ConsumerConfig.java @@ -531,6 +531,10 @@ public static Set configNames() { return CONFIG.names(); } + public static ConfigDef configDef() { + return CONFIG; + } + public static void main(String[] args) { System.out.println(CONFIG.toHtmlTable()); } diff --git a/clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java b/clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java index 758f858c0a5ad..396d91e3a2dd6 100644 --- a/clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java +++ b/clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java @@ -404,6 +404,10 @@ public static Set configNames() { return CONFIG.names(); } + public static ConfigDef configDef() { + return CONFIG; + } + public static void main(String[] args) { System.out.println(CONFIG.toHtmlTable()); } diff --git a/connect/api/src/main/java/org/apache/kafka/connect/connector/policy/ConnectorClientConfigOverridePolicy.java b/connect/api/src/main/java/org/apache/kafka/connect/connector/policy/ConnectorClientConfigOverridePolicy.java new file mode 100644 index 0000000000000..3c74282b9c202 --- /dev/null +++ b/connect/api/src/main/java/org/apache/kafka/connect/connector/policy/ConnectorClientConfigOverridePolicy.java @@ -0,0 +1,44 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.kafka.connect.connector.policy; + +import org.apache.kafka.common.Configurable; +import org.apache.kafka.common.errors.PolicyViolationException; + +/** + *

An interface for enforcing a policy on overriding of client configs via the connector configs. + * + *

Common use cases are ability to provide principal per connector, sasl.jaas.config + * and/or enforcing that the producer/consumer configurations for optimizations are within acceptable ranges. + */ +public interface ConnectorClientConfigOverridePolicy extends Configurable, AutoCloseable { + + + /** + * Worker will invoke this while constructing the producer for the SourceConnectors, DLQ for SinkConnectors and the consumer for the + * SinkConnectors to validate if all of the overridden client configurations are allowed per the + * policy implementation. This would also be invoked during the validate of connector configs via the Rest API. + * + * If there are any policy violations, the connector will not be started. + * + * @param connectorClientConfigRequest an instance of {@code ConnectorClientConfigRequest} that provides the configs to overridden and + * its context; never {@code null} + * @throws PolicyViolationException if any of the overridden property doesn't meet the defined policy + */ + void validate(ConnectorClientConfigRequest connectorClientConfigRequest) throws PolicyViolationException; +} \ No newline at end of file diff --git a/connect/api/src/main/java/org/apache/kafka/connect/connector/policy/ConnectorClientConfigRequest.java b/connect/api/src/main/java/org/apache/kafka/connect/connector/policy/ConnectorClientConfigRequest.java new file mode 100644 index 0000000000000..5cd9b6465a018 --- /dev/null +++ b/connect/api/src/main/java/org/apache/kafka/connect/connector/policy/ConnectorClientConfigRequest.java @@ -0,0 +1,105 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.kafka.connect.connector.policy; + +import org.apache.kafka.connect.connector.Connector; +import org.apache.kafka.connect.health.ConnectorType; + +import java.util.Map; + +public class ConnectorClientConfigRequest { + + private Map clientProps; + private ClientType clientType; + private String connectorName; + private ConnectorType connectorType; + private Class connectorClass; + + public ConnectorClientConfigRequest( + String connectorName, + ConnectorType connectorType, + Class connectorClass, + Map clientProps, + ClientType clientType) { + this.clientProps = clientProps; + this.clientType = clientType; + this.connectorName = connectorName; + this.connectorType = connectorType; + this.connectorClass = connectorClass; + } + + /** + * 

+     * Provides Config with prefix {@code producer.override.} for {@link ConnectorType#SOURCE}.
+     * Provides Config with prefix {@code consumer.override.} for {@link ConnectorType#SINK}.
+     * Provides Config with prefix {@code producer.override.} for {@link ConnectorType#SINK} for DLQ.
+     * Provides Config with prefix {@code admin.override.} for {@link ConnectorType#SINK} for DLQ.
+     *
+ * + * @return The client properties specified in the Connector Config with prefix {@code producer.override.} , + * {@code consumer.override.} and {@code admin.override.}. The configs returned don't include these prefixes. + */ + public Map clientProps() { + return clientProps; + } + + /** + * 
+     * {@link ClientType#PRODUCER} for {@link ConnectorType#SOURCE}
+     * {@link ClientType#CONSUMER} for {@link ConnectorType#SINK}
+     * {@link ClientType#PRODUCER} for DLQ in {@link ConnectorType#SINK}
+     * {@link ClientType#ADMIN} for DLQ  Topic Creation in {@link ConnectorType#SINK}
+     *
+ * + * @return enumeration specifying the client type that is being overriden by the worker; never null. + */ + public ClientType clientType() { + return clientType; + } + + /** + * Name of the connector specified in the connector config. + * + * @return name of the connector; never null. + */ + public String connectorName() { + return connectorName; + } + + /** + * Type of the Connector. + * + * @return enumeration specifying the type of the connector {@link ConnectorType#SINK} or {@link ConnectorType#SOURCE}. + */ + public ConnectorType connectorType() { + return connectorType; + } + + /** + * The class of the Connector. + * + * @return the class of the Connector being created; never null + */ + public Class connectorClass() { + return connectorClass; + } + + public enum ClientType { + PRODUCER, CONSUMER, ADMIN; + } +} \ No newline at end of file diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/cli/ConnectDistributed.java b/connect/runtime/src/main/java/org/apache/kafka/connect/cli/ConnectDistributed.java index 17d65ac678d45..27e62896ec453 100644 --- a/connect/runtime/src/main/java/org/apache/kafka/connect/cli/ConnectDistributed.java +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/cli/ConnectDistributed.java @@ -19,8 +19,10 @@ import org.apache.kafka.common.utils.Exit; import org.apache.kafka.common.utils.Time; import org.apache.kafka.common.utils.Utils; +import org.apache.kafka.connect.connector.policy.ConnectorClientConfigOverridePolicy; import org.apache.kafka.connect.runtime.Connect; import org.apache.kafka.connect.runtime.Worker; +import org.apache.kafka.connect.runtime.WorkerConfig; import org.apache.kafka.connect.runtime.WorkerConfigTransformer; import org.apache.kafka.connect.runtime.WorkerInfo; import org.apache.kafka.connect.runtime.distributed.DistributedConfig; @@ -102,7 +104,13 @@ public Connect startConnect(Map workerProps) { KafkaOffsetBackingStore offsetBackingStore = new KafkaOffsetBackingStore(); offsetBackingStore.configure(config); - Worker worker = new Worker(workerId, time, plugins, config, offsetBackingStore); + ConnectorClientConfigOverridePolicy connectorClientConfigOverridePolicy = null; + if (config.getString(WorkerConfig.CONNECTOR_CLIENT_POLICY_CLASS_CONFIG) != null) { + connectorClientConfigOverridePolicy = plugins.newPlugin( + config.getString(WorkerConfig.CONNECTOR_CLIENT_POLICY_CLASS_CONFIG), + config, ConnectorClientConfigOverridePolicy.class); + } + Worker worker = new Worker(workerId, time, plugins, config, offsetBackingStore, connectorClientConfigOverridePolicy); WorkerConfigTransformer configTransformer = worker.configTransformer(); Converter internalValueConverter = worker.getInternalValueConverter(); @@ -115,8 +123,8 @@ public Connect startConnect(Map workerProps) { configTransformer); DistributedHerder herder = new DistributedHerder(config, time, worker, - kafkaClusterId, statusBackingStore, configBackingStore, - advertisedUrl.toString()); + kafkaClusterId, statusBackingStore, configBackingStore, + advertisedUrl.toString(), connectorClientConfigOverridePolicy); final Connect connect = new Connect(herder, rest); log.info("Kafka Connect distributed worker initialization took {}ms", time.hiResClockMs() - initStart); diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/cli/ConnectStandalone.java b/connect/runtime/src/main/java/org/apache/kafka/connect/cli/ConnectStandalone.java index 499e6dfdfe93f..971637738bb82 100644 --- a/connect/runtime/src/main/java/org/apache/kafka/connect/cli/ConnectStandalone.java +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/cli/ConnectStandalone.java @@ -19,10 +19,12 @@ import org.apache.kafka.common.utils.Exit; import org.apache.kafka.common.utils.Time; import org.apache.kafka.common.utils.Utils; +import org.apache.kafka.connect.connector.policy.ConnectorClientConfigOverridePolicy; import org.apache.kafka.connect.runtime.Connect; import org.apache.kafka.connect.runtime.ConnectorConfig; import org.apache.kafka.connect.runtime.Herder; import org.apache.kafka.connect.runtime.Worker; +import org.apache.kafka.connect.runtime.WorkerConfig; import org.apache.kafka.connect.runtime.WorkerInfo; import org.apache.kafka.connect.runtime.isolation.Plugins; import org.apache.kafka.connect.runtime.rest.RestServer; @@ -87,9 +89,16 @@ public static void main(String[] args) { URI advertisedUrl = rest.advertisedUrl(); String workerId = advertisedUrl.getHost() + ":" + advertisedUrl.getPort(); - Worker worker = new Worker(workerId, time, plugins, config, new FileOffsetBackingStore()); + ConnectorClientConfigOverridePolicy connectorClientConfigOverridePolicy = null; + if (config.getString(WorkerConfig.CONNECTOR_CLIENT_POLICY_CLASS_CONFIG) != null) { + connectorClientConfigOverridePolicy = plugins.newPlugin( + config.getString(WorkerConfig.CONNECTOR_CLIENT_POLICY_CLASS_CONFIG), + config, ConnectorClientConfigOverridePolicy.class); + } + Worker worker = new Worker(workerId, time, plugins, config, new FileOffsetBackingStore(), + connectorClientConfigOverridePolicy); - Herder herder = new StandaloneHerder(worker, kafkaClusterId); + Herder herder = new StandaloneHerder(worker, kafkaClusterId, connectorClientConfigOverridePolicy); final Connect connect = new Connect(herder, rest); log.info("Kafka Connect standalone worker initialization took {}ms", time.hiResClockMs() - initStart); diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/AllConnectorClientConfigOverridePolicy.java b/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/AllConnectorClientConfigOverridePolicy.java new file mode 100644 index 0000000000000..ee3ef4b007789 --- /dev/null +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/AllConnectorClientConfigOverridePolicy.java @@ -0,0 +1,40 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.kafka.connect.connector.policy; + +import org.apache.kafka.common.errors.PolicyViolationException; + +import java.util.Map; + +public class AllConnectorClientConfigOverridePolicy implements ConnectorClientConfigOverridePolicy { + + @Override + public void validate(ConnectorClientConfigRequest connectorClientConfigRequest) throws PolicyViolationException { + //allow all no op + } + + @Override + public void close() throws Exception { + + } + + @Override + public void configure(Map configs) { + + } +} diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/NoneConnectorClientConfigOverridePolicy.java b/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/NoneConnectorClientConfigOverridePolicy.java new file mode 100644 index 0000000000000..e643b0551f4f7 --- /dev/null +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/NoneConnectorClientConfigOverridePolicy.java @@ -0,0 +1,42 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.kafka.connect.connector.policy; + +import org.apache.kafka.common.errors.PolicyViolationException; + +import java.util.Map; + +public class NoneConnectorClientConfigOverridePolicy implements ConnectorClientConfigOverridePolicy { + + @Override + public void validate(ConnectorClientConfigRequest connectorClientConfigRequest) throws PolicyViolationException { + if (connectorClientConfigRequest.clientProps().size() > 0) { + throw new PolicyViolationException("Client Config Overrides aren't allowed"); + } + } + + @Override + public void close() throws Exception { + + } + + @Override + public void configure(Map configs) { + + } +} diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/PrincipalConnectorClientConfigOverridePolicy.java b/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/PrincipalConnectorClientConfigOverridePolicy.java new file mode 100644 index 0000000000000..4ee118f10714b --- /dev/null +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/PrincipalConnectorClientConfigOverridePolicy.java @@ -0,0 +1,49 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.kafka.connect.connector.policy; + +import org.apache.kafka.common.config.SaslConfigs; +import org.apache.kafka.common.errors.PolicyViolationException; + +import java.util.Collections; +import java.util.Map; +import java.util.Set; + +public class PrincipalConnectorClientConfigOverridePolicy implements ConnectorClientConfigOverridePolicy { + + private static final Set ALLOWED_CONFIG = Collections.singleton(SaslConfigs.SASL_JAAS_CONFIG); + + + @Override + public void validate(ConnectorClientConfigRequest connectorClientConfigRequest) throws PolicyViolationException { + if (!ALLOWED_CONFIG.containsAll(connectorClientConfigRequest.clientProps().keySet())) { + throw new PolicyViolationException("Can override " + connectorClientConfigRequest.clientType() + " with only " + + ALLOWED_CONFIG); + } + } + + @Override + public void close() throws Exception { + + } + + @Override + public void configure(Map configs) { + + } +} diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractHerder.java b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractHerder.java index 8e7d01664126d..55806c4c67e63 100644 --- a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractHerder.java +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractHerder.java @@ -16,13 +16,18 @@ */ package org.apache.kafka.connect.runtime; +import org.apache.kafka.clients.producer.ProducerConfig; +import org.apache.kafka.common.config.AbstractConfig; import org.apache.kafka.common.config.Config; import org.apache.kafka.common.config.ConfigDef; import org.apache.kafka.common.config.ConfigDef.ConfigKey; import org.apache.kafka.common.config.ConfigDef.Type; import org.apache.kafka.common.config.ConfigTransformer; import org.apache.kafka.common.config.ConfigValue; +import org.apache.kafka.common.errors.PolicyViolationException; import org.apache.kafka.connect.connector.Connector; +import org.apache.kafka.connect.connector.policy.ConnectorClientConfigOverridePolicy; +import org.apache.kafka.connect.connector.policy.ConnectorClientConfigRequest; import org.apache.kafka.connect.errors.NotFoundException; import org.apache.kafka.connect.runtime.distributed.ClusterConfigState; import org.apache.kafka.connect.runtime.isolation.Plugins; @@ -87,6 +92,7 @@ public abstract class AbstractHerder implements Herder, TaskStatus.Listener, Con private final String kafkaClusterId; protected final StatusBackingStore statusBackingStore; protected final ConfigBackingStore configBackingStore; + private final ConnectorClientConfigOverridePolicy connectorClientConfigOverridePolicy; private Map tempConnectors = new ConcurrentHashMap<>(); @@ -94,13 +100,15 @@ public AbstractHerder(Worker worker, String workerId, String kafkaClusterId, StatusBackingStore statusBackingStore, - ConfigBackingStore configBackingStore) { + ConfigBackingStore configBackingStore, + ConnectorClientConfigOverridePolicy connectorClientConfigOverridePolicy) { this.worker = worker; this.worker.herder = this; this.workerId = workerId; this.kafkaClusterId = kafkaClusterId; this.statusBackingStore = statusBackingStore; this.configBackingStore = configBackingStore; + this.connectorClientConfigOverridePolicy = connectorClientConfigOverridePolicy; } @Override @@ -280,14 +288,17 @@ public ConfigInfos validateConnectorConfig(Map connectorProps) { throw new BadRequestException("Connector config " + connectorProps + " contains no connector type"); Connector connector = getConnector(connType); + org.apache.kafka.connect.health.ConnectorType connectorType; ClassLoader savedLoader = plugins().compareAndSwapLoaders(connector); try { ConfigDef baseConfigDef; if (connector instanceof SourceConnector) { baseConfigDef = SourceConnectorConfig.configDef(); + connectorType = org.apache.kafka.connect.health.ConnectorType.SOURCE; } else { baseConfigDef = SinkConnectorConfig.configDef(); SinkConnectorConfig.validate(connectorProps); + connectorType = org.apache.kafka.connect.health.ConnectorType.SINK; } ConfigDef enrichedConfigDef = ConnectorConfig.enrich(plugins(), baseConfigDef, connectorProps, false); Map validatedConnectorConfig = validateBasicConnectorConfig( @@ -321,12 +332,105 @@ public ConfigInfos validateConnectorConfig(Map connectorProps) { configKeys.putAll(configDef.configKeys()); allGroups.addAll(configDef.groups()); configValues.addAll(config.configValues()); - return generateResult(connType, configKeys, configValues, new ArrayList<>(allGroups)); + ConfigInfos configInfos = generateResult(connType, configKeys, configValues, new ArrayList<>(allGroups)); + + AbstractConfig connectorConfig = new AbstractConfig(new ConfigDef(), connectorProps); + String connName = connectorProps.get(ConnectorConfig.NAME_CONFIG); + ConfigInfos producerConfigInfos = null, consumerConfigInfos = null, adminConfigInfos = null; + if (connectorType.equals(org.apache.kafka.connect.health.ConnectorType.SOURCE)) { + producerConfigInfos = validateClientOverrides(connName, + "producer.", + connectorConfig, + ProducerConfig.configDef(), + connector.getClass(), + connectorType, + ConnectorClientConfigRequest.ClientType.PRODUCER, + connectorClientConfigOverridePolicy); + return mergeConfigInfos(connType, configInfos, producerConfigInfos); + } else { + consumerConfigInfos = validateClientOverrides(connName, + "consumer.", + connectorConfig, + ProducerConfig.configDef(), + connector.getClass(), + connectorType, + ConnectorClientConfigRequest.ClientType.CONSUMER, + connectorClientConfigOverridePolicy); + // check if topic for dead letter queue exists + String topic = connectorProps.get(SinkConnectorConfig.DLQ_TOPIC_NAME_CONFIG); + if (topic != null && !topic.isEmpty()) { + adminConfigInfos = validateClientOverrides(connName, + "admin.", + connectorConfig, + ProducerConfig.configDef(), + connector.getClass(), + connectorType, + ConnectorClientConfigRequest.ClientType.ADMIN, + connectorClientConfigOverridePolicy); + } + + } + return mergeConfigInfos(connType, configInfos, producerConfigInfos, consumerConfigInfos, adminConfigInfos); } finally { Plugins.compareAndSwapLoaders(savedLoader); } } + private static ConfigInfos mergeConfigInfos(String connType, ConfigInfos... configInfosList) { + int errorCount = 0; + List configInfoList = new LinkedList<>(); + Set groups = new LinkedHashSet<>(); + for (ConfigInfos configInfos : configInfosList) { + if (configInfos != null) { + errorCount += configInfos.errorCount(); + configInfoList.addAll(configInfos.values()); + groups.addAll(configInfos.groups()); + } + } + return new ConfigInfos(connType, errorCount, new ArrayList<>(groups), configInfoList); + } + + // public for testing + public static ConfigInfos validateClientOverrides(String connName, + String prefix, + AbstractConfig connectorConfig, + ConfigDef configDef, + Class connectorClass, + org.apache.kafka.connect.health.ConnectorType connectorType, + ConnectorClientConfigRequest.ClientType clientType, + ConnectorClientConfigOverridePolicy connectorClientConfigOverridePolicy) { + int errorCount = 0; + List configInfoList = new LinkedList<>(); + Map configKeys = configDef.configKeys(); + Set groups = new HashSet<>(); + Map clientConfigs = connectorConfig.originalsWithPrefix(prefix); + for (Map.Entry clientConfig : clientConfigs.entrySet()) { + ConfigKey configKey = configKeys.get(clientConfig.getKey()); + ConfigKeyInfo configKeyInfo = null; + if (configKey != null) { + if (configKey.group != null) { + groups.add(configKey.group); + } + configKeyInfo = convertConfigKey(configKey, prefix); + } + Map clientProps = Collections.singletonMap(clientConfig.getKey(), clientConfig.getValue()); + ConnectorClientConfigRequest connectorClientConfigRequest = new ConnectorClientConfigRequest( + connName, connectorType, connectorClass, clientProps, clientType); + + ConfigValue configValue = new ConfigValue(prefix + clientConfig.getKey(), clientConfig.getValue(), + new ArrayList(), new ArrayList()); + try { + connectorClientConfigOverridePolicy.validate(connectorClientConfigRequest); + } catch (PolicyViolationException e) { + errorCount++; + configValue.addErrorMessage(e.getMessage()); + } + ConfigValueInfo configValueInfo = convertConfigValue(configValue, configKey != null ? configKey.type : null); + configInfoList.add(new ConfigInfo(configKeyInfo, configValueInfo)); + } + return new ConfigInfos(connectorClass.toString(), errorCount, new ArrayList<>(groups), configInfoList); + } + // public for testing public static ConfigInfos generateResult(String connType, Map configKeys, List configValues, List groups) { int errorCount = 0; @@ -357,8 +461,8 @@ public static ConfigInfos generateResult(String connType, Map return new ConfigInfos(connType, errorCount, groups, configInfoList); } - private static ConfigKeyInfo convertConfigKey(ConfigKey configKey) { - String name = configKey.name; + private static ConfigKeyInfo convertConfigKey(ConfigKey configKey, String prefix) { + String name = prefix + configKey.name; Type type = configKey.type; String typeName = configKey.type.name(); @@ -380,6 +484,10 @@ private static ConfigKeyInfo convertConfigKey(ConfigKey configKey) { return new ConfigKeyInfo(name, typeName, required, defaultValue, importance, documentation, group, orderInGroup, width, displayName, dependents); } + private static ConfigKeyInfo convertConfigKey(ConfigKey configKey) { + return convertConfigKey(configKey, ""); + } + private static ConfigValueInfo convertConfigValue(ConfigValue configValue, Type type) { String value = ConfigDef.convertToString(configValue.value(), type); List recommendedValues = new LinkedList<>(); diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Worker.java b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Worker.java index f6f74523bc945..3f83fc602dd96 100644 --- a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Worker.java +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Worker.java @@ -22,6 +22,7 @@ import org.apache.kafka.clients.producer.ProducerConfig; import org.apache.kafka.common.MetricName; import org.apache.kafka.common.config.provider.ConfigProvider; +import org.apache.kafka.common.errors.PolicyViolationException; import org.apache.kafka.common.metrics.Sensor; import org.apache.kafka.common.metrics.stats.Frequencies; import org.apache.kafka.common.metrics.stats.Total; @@ -30,7 +31,10 @@ import org.apache.kafka.connect.connector.Connector; import org.apache.kafka.connect.connector.ConnectorContext; import org.apache.kafka.connect.connector.Task; +import org.apache.kafka.connect.connector.policy.ConnectorClientConfigOverridePolicy; +import org.apache.kafka.connect.connector.policy.ConnectorClientConfigRequest; import org.apache.kafka.connect.errors.ConnectException; +import org.apache.kafka.connect.health.ConnectorType; import org.apache.kafka.connect.runtime.ConnectMetrics.LiteralSupplier; import org.apache.kafka.connect.runtime.ConnectMetrics.MetricGroup; import org.apache.kafka.connect.runtime.distributed.ClusterConfigState; @@ -98,15 +102,16 @@ public class Worker { private final ConcurrentMap tasks = new ConcurrentHashMap<>(); private SourceTaskOffsetCommitter sourceTaskOffsetCommitter; private WorkerConfigTransformer workerConfigTransformer; + private ConnectorClientConfigOverridePolicy connectorClientConfigOverridePolicy; public Worker( - String workerId, - Time time, - Plugins plugins, - WorkerConfig config, - OffsetBackingStore offsetBackingStore - ) { - this(workerId, time, plugins, config, offsetBackingStore, Executors.newCachedThreadPool()); + String workerId, + Time time, + Plugins plugins, + WorkerConfig config, + OffsetBackingStore offsetBackingStore, + ConnectorClientConfigOverridePolicy connectorClientConfigOverridePolicy) { + this(workerId, time, plugins, config, offsetBackingStore, Executors.newCachedThreadPool(), connectorClientConfigOverridePolicy); } @SuppressWarnings("deprecation") @@ -116,7 +121,8 @@ public Worker( Plugins plugins, WorkerConfig config, OffsetBackingStore offsetBackingStore, - ExecutorService executorService + ExecutorService executorService, + ConnectorClientConfigOverridePolicy connectorClientConfigOverridePolicy ) { this.metrics = new ConnectMetrics(workerId, config, time); this.executor = executorService; @@ -124,6 +130,7 @@ public Worker( this.time = time; this.plugins = plugins; this.config = config; + this.connectorClientConfigOverridePolicy = connectorClientConfigOverridePolicy; this.workerMetricsGroup = new WorkerMetricsGroup(metrics); // Internal converters are required properties, thus getClass won't return null. @@ -486,7 +493,8 @@ private WorkerTask buildWorkerTask(ClusterConfigState configState, HeaderConverter headerConverter, ClassLoader loader) { ErrorHandlingMetrics errorHandlingMetrics = errorHandlingMetrics(id); - + final Class connectorClass = plugins.connectorClass( + connConfig.getString(ConnectorConfig.CONNECTOR_CLASS_CONFIG)); RetryWithToleranceOperator retryWithToleranceOperator = new RetryWithToleranceOperator(connConfig.errorRetryTimeout(), connConfig.errorMaxDelayInMillis(), connConfig.errorToleranceType(), Time.SYSTEM); retryWithToleranceOperator.metrics(errorHandlingMetrics); @@ -500,7 +508,7 @@ private WorkerTask buildWorkerTask(ClusterConfigState configState, internalKeyConverter, internalValueConverter); OffsetStorageWriter offsetWriter = new OffsetStorageWriter(offsetBackingStore, id.connector(), internalKeyConverter, internalValueConverter); - Map producerProps = producerConfigs("connector-producer-" + id, config); + Map producerProps = producerConfigs("connector-producer-" + id, config, connConfig, connectorClass, connectorClientConfigOverridePolicy); KafkaProducer producer = new KafkaProducer<>(producerProps); // Note we pass the configState as it performs dynamic transformations under the covers @@ -511,9 +519,9 @@ private WorkerTask buildWorkerTask(ClusterConfigState configState, TransformationChain transformationChain = new TransformationChain<>(connConfig.transformations(), retryWithToleranceOperator); log.info("Initializing: {}", transformationChain); SinkConnectorConfig sinkConfig = new SinkConnectorConfig(plugins, connConfig.originalsStrings()); - retryWithToleranceOperator.reporters(sinkTaskReporters(id, sinkConfig, errorHandlingMetrics)); + retryWithToleranceOperator.reporters(sinkTaskReporters(id, sinkConfig, errorHandlingMetrics, connectorClass)); - Map consumerProps = consumerConfigs(id, config); + Map consumerProps = consumerConfigs(id, config, connConfig, connectorClass, connectorClientConfigOverridePolicy); KafkaConsumer consumer = new KafkaConsumer<>(consumerProps); return new WorkerSinkTask(id, (SinkTask) task, statusListener, initialState, config, configState, metrics, keyConverter, @@ -525,7 +533,11 @@ private WorkerTask buildWorkerTask(ClusterConfigState configState, } } - static Map producerConfigs(String defaultClientId, WorkerConfig config) { + static Map producerConfigs(String defaultClientId, + WorkerConfig config, + ConnectorConfig connConfig, + Class connectorClass, + ConnectorClientConfigOverridePolicy connectorClientConfigOverridePolicy) { Map producerProps = new HashMap<>(); producerProps.put(ProducerConfig.BOOTSTRAP_SERVERS_CONFIG, Utils.join(config.getList(WorkerConfig.BOOTSTRAP_SERVERS_CONFIG), ",")); producerProps.put(ProducerConfig.KEY_SERIALIZER_CLASS_CONFIG, "org.apache.kafka.common.serialization.ByteArraySerializer"); @@ -540,11 +552,30 @@ static Map producerConfigs(String defaultClientId, WorkerConfig producerProps.put(ProducerConfig.CLIENT_ID_CONFIG, defaultClientId); // User-specified overrides producerProps.putAll(config.originalsWithPrefix("producer.")); + + Map producerOverrides = connConfig.originalsWithPrefix("producer."); + ConnectorClientConfigRequest connectorClientConfigRequest = new ConnectorClientConfigRequest( + id.connector(), + ConnectorType.SOURCE, + connectorClass, + producerOverrides, + ConnectorClientConfigRequest.ClientType.PRODUCER + ); + try { + connectorClientConfigOverridePolicy.validate(connectorClientConfigRequest); + } catch (PolicyViolationException e) { + throw new ConnectException("Error applying client config overrides", e); + } + producerProps.putAll(producerOverrides); + return producerProps; } - - static Map consumerConfigs(ConnectorTaskId id, WorkerConfig config) { + static Map consumerConfigs(ConnectorTaskId id, + WorkerConfig config, + ConnectorConfig connConfig, + Class connectorClass, + ConnectorClientConfigOverridePolicy connectorClientConfigOverridePolicy) { // Include any unknown worker configs so consumer configs can be set globally on the worker // and through to the task Map consumerProps = new HashMap<>(); @@ -552,22 +583,68 @@ static Map consumerConfigs(ConnectorTaskId id, WorkerConfig conf consumerProps.put(ConsumerConfig.GROUP_ID_CONFIG, SinkUtils.consumerGroupId(id.connector())); consumerProps.put(ConsumerConfig.CLIENT_ID_CONFIG, "connector-consumer-" + id); consumerProps.put(ConsumerConfig.BOOTSTRAP_SERVERS_CONFIG, - Utils.join(config.getList(WorkerConfig.BOOTSTRAP_SERVERS_CONFIG), ",")); + Utils.join(config.getList(WorkerConfig.BOOTSTRAP_SERVERS_CONFIG), ",")); consumerProps.put(ConsumerConfig.ENABLE_AUTO_COMMIT_CONFIG, "false"); consumerProps.put(ConsumerConfig.AUTO_OFFSET_RESET_CONFIG, "earliest"); consumerProps.put(ConsumerConfig.KEY_DESERIALIZER_CLASS_CONFIG, "org.apache.kafka.common.serialization.ByteArrayDeserializer"); consumerProps.put(ConsumerConfig.VALUE_DESERIALIZER_CLASS_CONFIG, "org.apache.kafka.common.serialization.ByteArrayDeserializer"); consumerProps.putAll(config.originalsWithPrefix("consumer.")); + + Map consumerOverrides = connConfig.originalsWithPrefix("consumer."); + ConnectorClientConfigRequest connectorClientConfigRequest = new ConnectorClientConfigRequest( + id.connector(), + ConnectorType.SINK, + connectorClass, + consumerOverrides, + ConnectorClientConfigRequest.ClientType.CONSUMER + ); + try { + connectorClientConfigOverridePolicy.validate(connectorClientConfigRequest); + } catch (PolicyViolationException e) { + throw new ConnectException("Error applying client config overrides", e); + } + consumerProps.putAll(consumerOverrides); + return consumerProps; } + static Map adminConfigs(ConnectorTaskId id, + WorkerConfig config, + ConnectorConfig connConfig, + Class connectorClass, + ConnectorClientConfigOverridePolicy connectorClientConfigOverridePolicy) { + Map adminProps = new HashMap<>(); + adminProps.put(ProducerConfig.BOOTSTRAP_SERVERS_CONFIG, Utils.join(config.getList(WorkerConfig.BOOTSTRAP_SERVERS_CONFIG), ",")); + // User-specified overrides + adminProps.putAll(config.originalsWithPrefix("admin.")); + + + Map adminOverrides = connConfig.originalsWithPrefix("admin."); + ConnectorClientConfigRequest connectorClientConfigRequest = new ConnectorClientConfigRequest( + id.connector(), + ConnectorType.SINK, + connectorClass, + adminOverrides, + ConnectorClientConfigRequest.ClientType.ADMIN + ); + try { + connectorClientConfigOverridePolicy.validate(connectorClientConfigRequest); + } catch (PolicyViolationException e) { + throw new ConnectException("Error applying client config overrides", e); + } + adminProps.putAll(adminOverrides); + + return adminProps; + } + ErrorHandlingMetrics errorHandlingMetrics(ConnectorTaskId id) { return new ErrorHandlingMetrics(id, metrics); } private List sinkTaskReporters(ConnectorTaskId id, SinkConnectorConfig connConfig, - ErrorHandlingMetrics errorHandlingMetrics) { + ErrorHandlingMetrics errorHandlingMetrics, + Class connectorClass) { ArrayList reporters = new ArrayList<>(); LogReporter logReporter = new LogReporter(id, connConfig, errorHandlingMetrics); reporters.add(logReporter); @@ -575,8 +652,10 @@ private List sinkTaskReporters(ConnectorTaskId id, SinkConnectorC // check if topic for dead letter queue exists String topic = connConfig.dlqTopicName(); if (topic != null && !topic.isEmpty()) { - Map producerProps = producerConfigs("connector-dlq-producer-" + id, config); - DeadLetterQueueReporter reporter = DeadLetterQueueReporter.createAndSetup(config, id, connConfig, producerProps, errorHandlingMetrics); + Map producerProps = producerConfigs("connector-dlq-producer-" + id, config, connConfig, connectorClass, connectorClientConfigOverridePolicy); + Map adminProps = adminConfigs(id, config, connConfig, connectorClass, connectorClientConfigOverridePolicy); + DeadLetterQueueReporter reporter = DeadLetterQueueReporter.createAndSetup(adminProps, id, connConfig, producerProps, + errorHandlingMetrics); reporters.add(reporter); } diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/WorkerConfig.java b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/WorkerConfig.java index efa92b5d732e2..f5e2572f319f4 100644 --- a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/WorkerConfig.java +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/WorkerConfig.java @@ -212,6 +212,13 @@ public class WorkerConfig extends AbstractConfig { + "ConnectRestExtension allows you to inject into Connect's REST API user defined resources like filters. " + "Typically used to add custom capability like logging, security, etc. "; + public static final String CONNECTOR_CLIENT_POLICY_CLASS_CONFIG = "client.config.policy"; + public static final String CONNECTOR_CLIENT_POLICY_CLASS_DOC = + "Class name or alias of implementation of ConnectorClientConfigOverridePolicy. Defines what client configurations can be " + + "overriden by the connector"; + public static final String CONNECTOR_CLIENT_POLICY_CLASS_DEFAULT = "None"; + + public static final String METRICS_SAMPLE_WINDOW_MS_CONFIG = CommonClientConfigs.METRICS_SAMPLE_WINDOW_MS_CONFIG; public static final String METRICS_NUM_SAMPLES_CONFIG = CommonClientConfigs.METRICS_NUM_SAMPLES_CONFIG; public static final String METRICS_RECORDING_LEVEL_CONFIG = CommonClientConfigs.METRICS_RECORDING_LEVEL_CONFIG; @@ -289,7 +296,9 @@ protected static ConfigDef baseConfigDef() { Collections.emptyList(), Importance.LOW, CONFIG_PROVIDERS_DOC) .define(REST_EXTENSION_CLASSES_CONFIG, Type.LIST, "", - Importance.LOW, REST_EXTENSION_CLASSES_DOC); + Importance.LOW, REST_EXTENSION_CLASSES_DOC) + .define(CONNECTOR_CLIENT_POLICY_CLASS_CONFIG, Type.STRING, CONNECTOR_CLIENT_POLICY_CLASS_DEFAULT, + Importance.MEDIUM, CONNECTOR_CLIENT_POLICY_CLASS_DOC); } private void logInternalConverterDeprecationWarnings(Map props) { diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/distributed/DistributedHerder.java b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/distributed/DistributedHerder.java index 94ca734753ff8..585836e557744 100644 --- a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/distributed/DistributedHerder.java +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/distributed/DistributedHerder.java @@ -30,6 +30,7 @@ import org.apache.kafka.common.utils.Utils; import org.apache.kafka.connect.connector.Connector; import org.apache.kafka.connect.connector.ConnectorContext; +import org.apache.kafka.connect.connector.policy.ConnectorClientConfigOverridePolicy; import org.apache.kafka.connect.errors.AlreadyExistsException; import org.apache.kafka.connect.errors.ConnectException; import org.apache.kafka.connect.errors.NotFoundException; @@ -165,8 +166,10 @@ public DistributedHerder(DistributedConfig config, String kafkaClusterId, StatusBackingStore statusBackingStore, ConfigBackingStore configBackingStore, - String restUrl) { - this(config, worker, worker.workerId(), kafkaClusterId, statusBackingStore, configBackingStore, null, restUrl, worker.metrics(), time); + String restUrl, + ConnectorClientConfigOverridePolicy connectorClientConfigOverridePolicy) { + this(config, worker, worker.workerId(), kafkaClusterId, statusBackingStore, configBackingStore, null, restUrl, worker.metrics(), + time, connectorClientConfigOverridePolicy); configBackingStore.setUpdateListener(new ConfigUpdateListener()); } @@ -180,8 +183,9 @@ public DistributedHerder(DistributedConfig config, WorkerGroupMember member, String restUrl, ConnectMetrics metrics, - Time time) { - super(worker, workerId, kafkaClusterId, statusBackingStore, configBackingStore); + Time time, + ConnectorClientConfigOverridePolicy connectorClientConfigOverridePolicy) { + super(worker, workerId, kafkaClusterId, statusBackingStore, configBackingStore, connectorClientConfigOverridePolicy); this.time = time; this.herderMetrics = new HerderMetrics(metrics); diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/errors/DeadLetterQueueReporter.java b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/errors/DeadLetterQueueReporter.java index 231226997833d..c78026f733d5a 100644 --- a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/errors/DeadLetterQueueReporter.java +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/errors/DeadLetterQueueReporter.java @@ -26,7 +26,6 @@ import org.apache.kafka.common.record.RecordBatch; import org.apache.kafka.connect.errors.ConnectException; import org.apache.kafka.connect.runtime.SinkConnectorConfig; -import org.apache.kafka.connect.runtime.WorkerConfig; import org.apache.kafka.connect.util.ConnectorTaskId; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -71,13 +70,13 @@ public class DeadLetterQueueReporter implements ErrorReporter { private KafkaProducer kafkaProducer; - public static DeadLetterQueueReporter createAndSetup(WorkerConfig workerConfig, + public static DeadLetterQueueReporter createAndSetup(Map adminProps, ConnectorTaskId id, SinkConnectorConfig sinkConfig, Map producerProps, ErrorHandlingMetrics errorHandlingMetrics) { String topic = sinkConfig.dlqTopicName(); - try (AdminClient admin = AdminClient.create(workerConfig.originals())) { + try (AdminClient admin = AdminClient.create(adminProps)) { if (!admin.listTopics().names().get().contains(topic)) { log.error("Topic {} doesn't exist. Will attempt to create topic.", topic); NewTopic schemaTopicRequest = new NewTopic(topic, DLQ_NUM_DESIRED_PARTITIONS, sinkConfig.dlqTopicReplicationFactor()); diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/DelegatingClassLoader.java b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/DelegatingClassLoader.java index 460df39db3dea..d8c4ccaca6b78 100644 --- a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/DelegatingClassLoader.java +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/DelegatingClassLoader.java @@ -19,6 +19,7 @@ import org.apache.kafka.common.config.provider.ConfigProvider; import org.apache.kafka.connect.components.Versioned; import org.apache.kafka.connect.connector.Connector; +import org.apache.kafka.connect.connector.policy.ConnectorClientConfigOverridePolicy; import org.apache.kafka.connect.rest.ConnectRestExtension; import org.apache.kafka.connect.storage.Converter; import org.apache.kafka.connect.storage.HeaderConverter; @@ -72,6 +73,7 @@ public class DelegatingClassLoader extends URLClassLoader { private final SortedSet> transformations; private final SortedSet> configProviders; private final SortedSet> restExtensions; + private final SortedSet> connectorClientConfigPolicies; private final List pluginPaths; private static final String MANIFEST_PREFIX = "META-INF/services/"; @@ -91,6 +93,7 @@ public DelegatingClassLoader(List pluginPaths, ClassLoader parent) { this.transformations = new TreeSet<>(); this.configProviders = new TreeSet<>(); this.restExtensions = new TreeSet<>(); + this.connectorClientConfigPolicies = new TreeSet<>(); } public DelegatingClassLoader(List pluginPaths) { @@ -125,6 +128,10 @@ public Set> restExtensions() { return restExtensions; } + public Set> connectorClientConfigPolicies() { + return connectorClientConfigPolicies; + } + public ClassLoader connectorLoader(Connector connector) { return connectorLoader(connector.getClass().getName()); } @@ -249,6 +256,8 @@ private void scanUrlsAndAddPlugins( configProviders.addAll(plugins.configProviders()); addPlugins(plugins.restExtensions(), loader); restExtensions.addAll(plugins.restExtensions()); + addPlugins(plugins.connectorClientConfigPolicies(), loader); + connectorClientConfigPolicies.addAll(plugins.connectorClientConfigPolicies()); } loadJdbcDrivers(loader); @@ -304,7 +313,8 @@ private PluginScanResult scanPluginPath( getPluginDesc(reflections, HeaderConverter.class, loader), getPluginDesc(reflections, Transformation.class, loader), getServiceLoaderPluginDesc(ConfigProvider.class, loader), - getServiceLoaderPluginDesc(ConnectRestExtension.class, loader) + getServiceLoaderPluginDesc(ConnectRestExtension.class, loader), + getServiceLoaderPluginDesc(ConnectorClientConfigOverridePolicy.class, loader) ); } @@ -371,6 +381,7 @@ private void addAllAliases() { addAliases(headerConverters); addAliases(transformations); addAliases(restExtensions); + addAliases(connectorClientConfigPolicies); } private void addAliases(Collection> plugins) { diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/PluginScanResult.java b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/PluginScanResult.java index ef077b3e7af19..e64a96c6f00a0 100644 --- a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/PluginScanResult.java +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/PluginScanResult.java @@ -18,12 +18,15 @@ import org.apache.kafka.common.config.provider.ConfigProvider; import org.apache.kafka.connect.connector.Connector; +import org.apache.kafka.connect.connector.policy.ConnectorClientConfigOverridePolicy; import org.apache.kafka.connect.rest.ConnectRestExtension; import org.apache.kafka.connect.storage.Converter; import org.apache.kafka.connect.storage.HeaderConverter; import org.apache.kafka.connect.transforms.Transformation; +import java.util.Arrays; import java.util.Collection; +import java.util.List; public class PluginScanResult { private final Collection> connectors; @@ -32,6 +35,9 @@ public class PluginScanResult { private final Collection> transformations; private final Collection> configProviders; private final Collection> restExtensions; + private final Collection> connectorClientConfigPolicies; + + private final List allPlugins; public PluginScanResult( Collection> connectors, @@ -39,7 +45,8 @@ public PluginScanResult( Collection> headerConverters, Collection> transformations, Collection> configProviders, - Collection> restExtensions + Collection> restExtensions, + Collection> connectorClientConfigPolicies ) { this.connectors = connectors; this.converters = converters; @@ -47,6 +54,10 @@ public PluginScanResult( this.transformations = transformations; this.configProviders = configProviders; this.restExtensions = restExtensions; + this.connectorClientConfigPolicies = connectorClientConfigPolicies; + this.allPlugins = + Arrays.asList(connectors, converters, headerConverters, transformations, configProviders, + connectorClientConfigPolicies); } public Collection> connectors() { @@ -73,12 +84,15 @@ public Collection> restExtensions() { return restExtensions; } + public Collection> connectorClientConfigPolicies() { + return connectorClientConfigPolicies; + } + public boolean isEmpty() { - return connectors().isEmpty() - && converters().isEmpty() - && headerConverters().isEmpty() - && transformations().isEmpty() - && configProviders().isEmpty() - && restExtensions().isEmpty(); + boolean isEmpty = true; + for (Collection plugins : allPlugins) { + isEmpty = isEmpty && plugins.isEmpty(); + } + return isEmpty; } } diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/PluginType.java b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/PluginType.java index 2833b4c4ba0bf..8b42f59ef9368 100644 --- a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/PluginType.java +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/PluginType.java @@ -18,6 +18,7 @@ import org.apache.kafka.common.config.provider.ConfigProvider; import org.apache.kafka.connect.connector.Connector; +import org.apache.kafka.connect.connector.policy.ConnectorClientConfigOverridePolicy; import org.apache.kafka.connect.rest.ConnectRestExtension; import org.apache.kafka.connect.sink.SinkConnector; import org.apache.kafka.connect.source.SourceConnector; @@ -34,6 +35,7 @@ public enum PluginType { TRANSFORMATION(Transformation.class), CONFIGPROVIDER(ConfigProvider.class), REST_EXTENSION(ConnectRestExtension.class), + CONNECTOR_CLIENT_CONFIG_OVERRIDE_POLICY(ConnectorClientConfigOverridePolicy.class), UNKNOWN(Object.class); private Class klass; diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/PluginUtils.java b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/PluginUtils.java index 8d2a3cedc1764..c02980210c21d 100644 --- a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/PluginUtils.java +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/PluginUtils.java @@ -131,6 +131,7 @@ public class PluginUtils { + "|storage\\.StringConverter" + "|storage\\.SimpleHeaderConverter" + "|rest\\.basic\\.auth\\.extension\\.BasicAuthSecurityRestExtension" + + "|connector\\.policy\\..*" + ")" + "|common\\.config\\.provider\\.(?!ConfigProvider$).*" + ")$"); diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/Plugins.java b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/Plugins.java index 148f8180894d3..e438db8147950 100644 --- a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/Plugins.java +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/Plugins.java @@ -149,6 +149,11 @@ public Set> configProviders() { } public Connector newConnector(String connectorClassOrAlias) { + Class klass = connectorClass(connectorClassOrAlias); + return newPlugin(klass); + } + + public Class connectorClass(String connectorClassOrAlias) { Class klass; try { klass = pluginClass( @@ -188,7 +193,7 @@ public Connector newConnector(String connectorClassOrAlias) { PluginDesc entry = matches.get(0); klass = entry.pluginClass(); } - return newPlugin(klass); + return klass; } public Task newTask(Class taskClass) { diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerder.java b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerder.java index 7b6d16a842647..00bda92574477 100644 --- a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerder.java +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerder.java @@ -16,6 +16,7 @@ */ package org.apache.kafka.connect.runtime.standalone; +import org.apache.kafka.connect.connector.policy.ConnectorClientConfigOverridePolicy; import org.apache.kafka.connect.errors.AlreadyExistsException; import org.apache.kafka.connect.errors.ConnectException; import org.apache.kafka.connect.errors.NotFoundException; @@ -62,12 +63,14 @@ public class StandaloneHerder extends AbstractHerder { private ClusterConfigState configState; - public StandaloneHerder(Worker worker, String kafkaClusterId) { + public StandaloneHerder(Worker worker, String kafkaClusterId, + ConnectorClientConfigOverridePolicy connectorClientConfigOverridePolicy) { this(worker, worker.workerId(), kafkaClusterId, new MemoryStatusBackingStore(), - new MemoryConfigBackingStore(worker.configTransformer())); + new MemoryConfigBackingStore(worker.configTransformer()), + connectorClientConfigOverridePolicy); } // visible for testing @@ -75,8 +78,9 @@ public StandaloneHerder(Worker worker, String kafkaClusterId) { String workerId, String kafkaClusterId, StatusBackingStore statusBackingStore, - MemoryConfigBackingStore configBackingStore) { - super(worker, workerId, kafkaClusterId, statusBackingStore, configBackingStore); + MemoryConfigBackingStore configBackingStore, + ConnectorClientConfigOverridePolicy connectorClientConfigOverridePolicy) { + super(worker, workerId, kafkaClusterId, statusBackingStore, configBackingStore, connectorClientConfigOverridePolicy); this.configState = ClusterConfigState.EMPTY; this.requestExecutorService = Executors.newSingleThreadScheduledExecutor(); configBackingStore.setUpdateListener(new ConfigUpdateListener()); diff --git a/connect/runtime/src/main/resources/META-INF/services/org.apache.kafka.connect.connector.policy.ConnectorClientConfigOverridePolicy b/connect/runtime/src/main/resources/META-INF/services/org.apache.kafka.connect.connector.policy.ConnectorClientConfigOverridePolicy new file mode 100644 index 0000000000000..8b76ce452b659 --- /dev/null +++ b/connect/runtime/src/main/resources/META-INF/services/org.apache.kafka.connect.connector.policy.ConnectorClientConfigOverridePolicy @@ -0,0 +1,18 @@ + # Licensed to the Apache Software Foundation (ASF) under one or more + # contributor license agreements. See the NOTICE file distributed with + # this work for additional information regarding copyright ownership. + # The ASF licenses this file to You under the Apache License, Version 2.0 + # (the "License"); you may not use this file except in compliance with + # the License. You may obtain a copy of the License at + # + # http://www.apache.org/licenses/LICENSE-2.0 + # + # Unless required by applicable law or agreed to in writing, software + # distributed under the License is distributed on an "AS IS" BASIS, + # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + # See the License for the specific language governing permissions and + # limitations under the License. + +org.apache.kafka.connect.connector.policy.AllConnectorClientConfigOverridePolicy +org.apache.kafka.connect.connector.policy.PrincipalConnectorClientConfigOverridePolicy +org.apache.kafka.connect.connector.policy.NoneConnectorClientConfigOverridePolicy \ No newline at end of file diff --git a/connect/runtime/src/test/java/org/apache/kafka/connect/connector/policy/NoneConnectorClientConfigOverridePolicyTest.java b/connect/runtime/src/test/java/org/apache/kafka/connect/connector/policy/NoneConnectorClientConfigOverridePolicyTest.java new file mode 100644 index 0000000000000..6eadf76f1b7ff --- /dev/null +++ b/connect/runtime/src/test/java/org/apache/kafka/connect/connector/policy/NoneConnectorClientConfigOverridePolicyTest.java @@ -0,0 +1,60 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.kafka.connect.connector.policy; + +import org.apache.kafka.clients.producer.ProducerConfig; +import org.apache.kafka.common.config.SaslConfigs; +import org.apache.kafka.common.errors.PolicyViolationException; +import org.apache.kafka.connect.health.ConnectorType; +import org.apache.kafka.connect.runtime.WorkerTest; +import org.junit.Test; + +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; + +public class NoneConnectorClientConfigOverridePolicyTest { + + ConnectorClientConfigOverridePolicy noneConnectorClientConfigOverridePolicy = new NoneConnectorClientConfigOverridePolicy(); + + @Test + public void testNoOverrides() { + Map clientConfig = Collections.emptyMap(); + ConnectorClientConfigRequest connectorClientConfigRequest = new ConnectorClientConfigRequest( + "test", + ConnectorType.SOURCE, + WorkerTest.WorkerTestConnector.class, + clientConfig, + ConnectorClientConfigRequest.ClientType.PRODUCER); + noneConnectorClientConfigOverridePolicy.validate(connectorClientConfigRequest); + } + + @Test(expected = PolicyViolationException.class) + public void testWithOverrides() { + Map clientConfig = new HashMap<>(); + clientConfig.put(SaslConfigs.SASL_JAAS_CONFIG, "test"); + clientConfig.put(ProducerConfig.ACKS_CONFIG, "none"); + ConnectorClientConfigRequest connectorClientConfigRequest = new ConnectorClientConfigRequest( + "test", + ConnectorType.SOURCE, + WorkerTest.WorkerTestConnector.class, + clientConfig, + ConnectorClientConfigRequest.ClientType.PRODUCER); + noneConnectorClientConfigOverridePolicy.validate(connectorClientConfigRequest); + } +} diff --git a/connect/runtime/src/test/java/org/apache/kafka/connect/connector/policy/PrincipalConnectorClientConfigOverridePolicyTest.java b/connect/runtime/src/test/java/org/apache/kafka/connect/connector/policy/PrincipalConnectorClientConfigOverridePolicyTest.java new file mode 100644 index 0000000000000..f251a8e3a6747 --- /dev/null +++ b/connect/runtime/src/test/java/org/apache/kafka/connect/connector/policy/PrincipalConnectorClientConfigOverridePolicyTest.java @@ -0,0 +1,60 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.kafka.connect.connector.policy; + +import org.apache.kafka.clients.producer.ProducerConfig; +import org.apache.kafka.common.config.SaslConfigs; +import org.apache.kafka.common.errors.PolicyViolationException; +import org.apache.kafka.connect.health.ConnectorType; +import org.apache.kafka.connect.runtime.WorkerTest; +import org.junit.Test; + +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; + +public class PrincipalConnectorClientConfigOverridePolicyTest { + + ConnectorClientConfigOverridePolicy principalConnectorClientConfigOverridePolicy = new PrincipalConnectorClientConfigOverridePolicy(); + + @Test + public void testPrincipalOnly() { + Map clientConfig = Collections.singletonMap(SaslConfigs.SASL_JAAS_CONFIG, "test"); + ConnectorClientConfigRequest connectorClientConfigRequest = new ConnectorClientConfigRequest( + "test", + ConnectorType.SOURCE, + WorkerTest.WorkerTestConnector.class, + clientConfig, + ConnectorClientConfigRequest.ClientType.PRODUCER); + principalConnectorClientConfigOverridePolicy.validate(connectorClientConfigRequest); + } + + @Test(expected = PolicyViolationException.class) + public void testPrincipalPlusOtherConfigs() { + Map clientConfig = new HashMap<>(); + clientConfig.put(SaslConfigs.SASL_JAAS_CONFIG, "test"); + clientConfig.put(ProducerConfig.ACKS_CONFIG, "none"); + ConnectorClientConfigRequest connectorClientConfigRequest = new ConnectorClientConfigRequest( + "test", + ConnectorType.SOURCE, + WorkerTest.WorkerTestConnector.class, + clientConfig, + ConnectorClientConfigRequest.ClientType.PRODUCER); + principalConnectorClientConfigOverridePolicy.validate(connectorClientConfigRequest); + } +} diff --git a/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/AbstractHerderTest.java b/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/AbstractHerderTest.java index f7ee8a68e311c..dcdbf868f9b53 100644 --- a/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/AbstractHerderTest.java +++ b/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/AbstractHerderTest.java @@ -16,10 +16,14 @@ */ package org.apache.kafka.connect.runtime; +import org.apache.kafka.clients.producer.ProducerConfig; import org.apache.kafka.common.config.ConfigDef; import org.apache.kafka.common.config.ConfigException; +import org.apache.kafka.common.config.SaslConfigs; import org.apache.kafka.connect.connector.ConnectRecord; import org.apache.kafka.connect.connector.Connector; +import org.apache.kafka.connect.connector.policy.ConnectorClientConfigOverridePolicy; +import org.apache.kafka.connect.connector.policy.PrincipalConnectorClientConfigOverridePolicy; import org.apache.kafka.connect.runtime.distributed.ClusterConfigState; import org.apache.kafka.connect.runtime.isolation.PluginDesc; import org.apache.kafka.connect.runtime.isolation.Plugins; @@ -116,6 +120,7 @@ public class AbstractHerderTest { private final String kafkaClusterId = "I4ZmrWqfT2e-upky_4fdPA"; private final int generation = 5; private final String connector = "connector"; + private final ConnectorClientConfigOverridePolicy ignoreConnectorClientConfigOverridePolicy = null; @MockStrict private Worker worker; @MockStrict private WorkerConfigTransformer transformer; @@ -179,8 +184,9 @@ public void connectorStatus() { ConnectorTaskId taskId = new ConnectorTaskId(connector, 0); AbstractHerder herder = partialMockBuilder(AbstractHerder.class) - .withConstructor(Worker.class, String.class, String.class, StatusBackingStore.class, ConfigBackingStore.class) - .withArgs(worker, workerId, kafkaClusterId, statusStore, configStore) + .withConstructor(Worker.class, String.class, String.class, StatusBackingStore.class, ConfigBackingStore.class, + ConnectorClientConfigOverridePolicy.class) + .withArgs(worker, workerId, kafkaClusterId, statusStore, configStore, ignoreConnectorClientConfigOverridePolicy) .addMockedMethod("generation") .createMock(); @@ -219,8 +225,9 @@ public void taskStatus() { String workerId = "workerId"; AbstractHerder herder = partialMockBuilder(AbstractHerder.class) - .withConstructor(Worker.class, String.class, String.class, StatusBackingStore.class, ConfigBackingStore.class) - .withArgs(worker, workerId, kafkaClusterId, statusStore, configStore) + .withConstructor(Worker.class, String.class, String.class, StatusBackingStore.class, ConfigBackingStore.class, + ConnectorClientConfigOverridePolicy.class) + .withArgs(worker, workerId, kafkaClusterId, statusStore, configStore, ignoreConnectorClientConfigOverridePolicy) .addMockedMethod("generation") .createMock(); @@ -253,7 +260,7 @@ public TaskStatus answer() throws Throwable { @Test(expected = BadRequestException.class) public void testConfigValidationEmptyConfig() { - AbstractHerder herder = createConfigValidationHerder(TestSourceConnector.class); + AbstractHerder herder = createConfigValidationHerder(TestSourceConnector.class, ignoreConnectorClientConfigOverridePolicy); replayAll(); herder.validateConnectorConfig(new HashMap()); @@ -263,7 +270,7 @@ public void testConfigValidationEmptyConfig() { @Test() public void testConfigValidationMissingName() { - AbstractHerder herder = createConfigValidationHerder(TestSourceConnector.class); + AbstractHerder herder = createConfigValidationHerder(TestSourceConnector.class, ignoreConnectorClientConfigOverridePolicy); replayAll(); Map config = Collections.singletonMap(ConnectorConfig.CONNECTOR_CLASS_CONFIG, TestSourceConnector.class.getName()); @@ -288,7 +295,7 @@ public void testConfigValidationMissingName() { @Test(expected = ConfigException.class) public void testConfigValidationInvalidTopics() { - AbstractHerder herder = createConfigValidationHerder(TestSinkConnector.class); + AbstractHerder herder = createConfigValidationHerder(TestSinkConnector.class, ignoreConnectorClientConfigOverridePolicy); replayAll(); Map config = new HashMap<>(); @@ -303,7 +310,7 @@ public void testConfigValidationInvalidTopics() { @Test() public void testConfigValidationTransformsExtendResults() { - AbstractHerder herder = createConfigValidationHerder(TestSourceConnector.class); + AbstractHerder herder = createConfigValidationHerder(TestSourceConnector.class, ignoreConnectorClientConfigOverridePolicy); // 2 transform aliases defined -> 2 plugin lookups Set> transformations = new HashSet<>(); @@ -349,6 +356,43 @@ public void testConfigValidationTransformsExtendResults() { verifyAll(); } + @Test() + public void testConfigValidationPrincipalOnlyOverride() { + AbstractHerder herder = createConfigValidationHerder(TestSourceConnector.class, new PrincipalConnectorClientConfigOverridePolicy()); + replayAll(); + + // Define 2 transformations. One has a class defined and so can get embedded configs, the other is missing + // class info that should generate an error. + Map config = new HashMap<>(); + config.put(ConnectorConfig.CONNECTOR_CLASS_CONFIG, TestSourceConnector.class.getName()); + config.put(ConnectorConfig.NAME_CONFIG, "connector-name"); + config.put("required", "value"); // connector required config + config.put("producer." + ProducerConfig.ACKS_CONFIG, "none"); + config.put("producer." + SaslConfigs.SASL_JAAS_CONFIG, "jaas_config"); + + ConfigInfos result = herder.validateConnectorConfig(config); + assertEquals(herder.connectorTypeForClass(config.get(ConnectorConfig.CONNECTOR_CLASS_CONFIG)), ConnectorType.SOURCE); + + // We expect there to be errors due to now allowed override policy for ACKS.... Note that these assertions depend heavily on + // the config fields for SourceConnectorConfig, but we expect these to change rarely. + assertEquals(TestSourceConnector.class.getName(), result.name()); + // Each transform also gets its own group + List expectedGroups = Arrays.asList( + ConnectorConfig.COMMON_GROUP, + ConnectorConfig.TRANSFORMS_GROUP, + ConnectorConfig.ERROR_GROUP + ); + assertEquals(expectedGroups, result.groups()); + assertEquals(1, result.errorCount()); + // Base connector config has 13 fields, connector's configs add 2, and 2 producer overrides + assertEquals(17, result.values().size()); + assertEquals("producer." + ProducerConfig.ACKS_CONFIG, result.values().get(15).configValue().name()); + assertFalse(result.values().get(15).configValue().errors().isEmpty()); + assertEquals("producer." + SaslConfigs.SASL_JAAS_CONFIG, result.values().get(16).configValue().name()); + assertTrue(result.values().get(16).configValue().errors().isEmpty()); + verifyAll(); + } + @Test public void testReverseTransformConfigs() { // Construct a task config with constant values for TEST_KEY and TEST_KEY2 @@ -372,15 +416,17 @@ public void testReverseTransformConfigs() { assertFalse(reverseTransformed.get(0).containsKey(TEST_KEY3)); } - private AbstractHerder createConfigValidationHerder(Class connectorClass) { + private AbstractHerder createConfigValidationHerder(Class connectorClass, + ConnectorClientConfigOverridePolicy connectorClientConfigOverridePolicy) { ConfigBackingStore configStore = strictMock(ConfigBackingStore.class); StatusBackingStore statusStore = strictMock(StatusBackingStore.class); AbstractHerder herder = partialMockBuilder(AbstractHerder.class) - .withConstructor(Worker.class, String.class, String.class, StatusBackingStore.class, ConfigBackingStore.class) - .withArgs(worker, workerId, kafkaClusterId, statusStore, configStore) + .withConstructor(Worker.class, String.class, String.class, StatusBackingStore.class, ConfigBackingStore.class, + ConnectorClientConfigOverridePolicy.class) + .withArgs(worker, workerId, kafkaClusterId, statusStore, configStore, connectorClientConfigOverridePolicy) .addMockedMethod("generation") .createMock(); EasyMock.expect(herder.generation()).andStubReturn(generation); diff --git a/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/WorkerTest.java b/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/WorkerTest.java index 586587bcb210a..c0f11a323a6f7 100644 --- a/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/WorkerTest.java +++ b/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/WorkerTest.java @@ -29,6 +29,9 @@ import org.apache.kafka.connect.connector.Connector; import org.apache.kafka.connect.connector.ConnectorContext; import org.apache.kafka.connect.connector.Task; +import org.apache.kafka.connect.connector.policy.AllConnectorClientConfigOverridePolicy; +import org.apache.kafka.connect.connector.policy.ConnectorClientConfigOverridePolicy; +import org.apache.kafka.connect.connector.policy.NoneConnectorClientConfigOverridePolicy; import org.apache.kafka.connect.data.Schema; import org.apache.kafka.connect.data.SchemaAndValue; import org.apache.kafka.connect.errors.ConnectException; @@ -60,6 +63,7 @@ import org.junit.runner.RunWith; import org.powermock.api.easymock.PowerMock; import org.powermock.api.easymock.annotation.Mock; +import org.powermock.api.easymock.annotation.MockNice; import org.powermock.api.easymock.annotation.MockStrict; import org.powermock.core.classloader.annotations.PowerMockIgnore; import org.powermock.core.classloader.annotations.PrepareForTest; @@ -89,6 +93,8 @@ public class WorkerTest extends ThreadedTest { private static final String CONNECTOR_ID = "test-connector"; private static final ConnectorTaskId TASK_ID = new ConnectorTaskId("job", 0); private static final String WORKER_ID = "localhost:8083"; + private final ConnectorClientConfigOverridePolicy noneConnectorClientConfigOverridePolicy = new NoneConnectorClientConfigOverridePolicy(); + private final ConnectorClientConfigOverridePolicy allConnectorClientConfigOverridePolicy = new AllConnectorClientConfigOverridePolicy(); private Map workerProps = new HashMap<>(); private WorkerConfig config; @@ -120,6 +126,7 @@ public class WorkerTest extends ThreadedTest { @Mock private Converter taskValueConverter; @Mock private HeaderConverter taskHeaderConverter; @Mock private ExecutorService executorService; + @MockNice private ConnectorConfig connectorConfig; @Before public void setup() { @@ -200,7 +207,7 @@ public void testStartAndStopConnector() { PowerMock.replayAll(); - worker = new Worker(WORKER_ID, new MockTime(), plugins, config, offsetBackingStore); + worker = new Worker(WORKER_ID, new MockTime(), plugins, config, offsetBackingStore, noneConnectorClientConfigOverridePolicy); worker.start(); assertEquals(Collections.emptySet(), worker.connectorNames()); @@ -251,7 +258,7 @@ public void testStartConnectorFailure() { PowerMock.replayAll(); - worker = new Worker(WORKER_ID, new MockTime(), plugins, config, offsetBackingStore); + worker = new Worker(WORKER_ID, new MockTime(), plugins, config, offsetBackingStore, noneConnectorClientConfigOverridePolicy); worker.start(); assertStatistics(worker, 0, 0); @@ -311,7 +318,7 @@ public void testAddConnectorByAlias() { PowerMock.replayAll(); - worker = new Worker(WORKER_ID, new MockTime(), plugins, config, offsetBackingStore); + worker = new Worker(WORKER_ID, new MockTime(), plugins, config, offsetBackingStore, noneConnectorClientConfigOverridePolicy); worker.start(); assertStatistics(worker, 0, 0); @@ -375,7 +382,7 @@ public void testAddConnectorByShortAlias() { PowerMock.replayAll(); - worker = new Worker(WORKER_ID, new MockTime(), plugins, config, offsetBackingStore); + worker = new Worker(WORKER_ID, new MockTime(), plugins, config, offsetBackingStore, noneConnectorClientConfigOverridePolicy); worker.start(); assertStatistics(worker, 0, 0); @@ -401,7 +408,7 @@ public void testStopInvalidConnector() { PowerMock.replayAll(); - worker = new Worker(WORKER_ID, new MockTime(), plugins, config, offsetBackingStore); + worker = new Worker(WORKER_ID, new MockTime(), plugins, config, offsetBackingStore, noneConnectorClientConfigOverridePolicy); worker.start(); worker.stopConnector(CONNECTOR_ID); @@ -458,7 +465,7 @@ public void testReconfigureConnectorTasks() { PowerMock.replayAll(); - worker = new Worker(WORKER_ID, new MockTime(), plugins, config, offsetBackingStore); + worker = new Worker(WORKER_ID, new MockTime(), plugins, config, offsetBackingStore, noneConnectorClientConfigOverridePolicy); worker.start(); assertStatistics(worker, 0, 0); @@ -550,7 +557,6 @@ public void testAddRemoveTask() throws Exception { EasyMock.expect(plugins.delegatingLoader()).andReturn(delegatingLoader); EasyMock.expect(delegatingLoader.connectorLoader(WorkerTestConnector.class.getName())) .andReturn(pluginLoader); - EasyMock.expect(Plugins.compareAndSwapLoaders(pluginLoader)).andReturn(delegatingLoader) .times(2); @@ -558,7 +564,8 @@ public void testAddRemoveTask() throws Exception { EasyMock.expect(Plugins.compareAndSwapLoaders(delegatingLoader)).andReturn(pluginLoader) .times(2); - + plugins.connectorClass(WorkerTestConnector.class.getName()); + EasyMock.expectLastCall().andReturn(WorkerTestConnector.class); // Remove workerTask.stop(); EasyMock.expectLastCall(); @@ -569,7 +576,8 @@ public void testAddRemoveTask() throws Exception { PowerMock.replayAll(); - worker = new Worker(WORKER_ID, new MockTime(), plugins, config, offsetBackingStore, executorService); + worker = new Worker(WORKER_ID, new MockTime(), plugins, config, offsetBackingStore, executorService, + noneConnectorClientConfigOverridePolicy); worker.start(); assertStatistics(worker, 0, 0); assertStartupStatistics(worker, 0, 0, 0, 0); @@ -620,7 +628,7 @@ public void testStartTaskFailure() { PowerMock.replayAll(); - worker = new Worker(WORKER_ID, new MockTime(), plugins, config, offsetBackingStore); + worker = new Worker(WORKER_ID, new MockTime(), plugins, config, offsetBackingStore, noneConnectorClientConfigOverridePolicy); worker.start(); assertStatistics(worker, 0, 0); assertStartupStatistics(worker, 0, 0, 0, 0); @@ -699,7 +707,8 @@ public void testCleanupTasksOnStop() throws Exception { EasyMock.expect(Plugins.compareAndSwapLoaders(delegatingLoader)).andReturn(pluginLoader) .times(2); - + plugins.connectorClass(WorkerTestConnector.class.getName()); + EasyMock.expectLastCall().andReturn(WorkerTestConnector.class); // Remove on Worker.stop() workerTask.stop(); EasyMock.expectLastCall(); @@ -712,7 +721,8 @@ public void testCleanupTasksOnStop() throws Exception { PowerMock.replayAll(); - worker = new Worker(WORKER_ID, new MockTime(), plugins, config, offsetBackingStore, executorService); + worker = new Worker(WORKER_ID, new MockTime(), plugins, config, offsetBackingStore, executorService, + noneConnectorClientConfigOverridePolicy); worker.start(); assertStatistics(worker, 0, 0); worker.startTask(TASK_ID, ClusterConfigState.EMPTY, anyConnectorConfigMap(), origProps, taskStatusListener, TargetState.STARTED); @@ -791,6 +801,8 @@ public void testConverterOverrides() throws Exception { EasyMock.expect(Plugins.compareAndSwapLoaders(delegatingLoader)).andReturn(pluginLoader) .times(2); + plugins.connectorClass(WorkerTestConnector.class.getName()); + EasyMock.expectLastCall().andReturn(WorkerTestConnector.class); // Remove workerTask.stop(); @@ -802,7 +814,8 @@ public void testConverterOverrides() throws Exception { PowerMock.replayAll(); - worker = new Worker(WORKER_ID, new MockTime(), plugins, config, offsetBackingStore, executorService); + worker = new Worker(WORKER_ID, new MockTime(), plugins, config, offsetBackingStore, executorService, + noneConnectorClientConfigOverridePolicy); worker.start(); assertStatistics(worker, 0, 0); assertEquals(Collections.emptySet(), worker.taskIds()); @@ -828,7 +841,9 @@ public void testConverterOverrides() throws Exception { @Test public void testProducerConfigsWithoutOverrides() { - Map expectedConfigs = new HashMap<>(defaultProducerConfigs); + EasyMock.expect(connectorConfig.originalsWithPrefix("producer.")).andReturn( + new HashMap()); + PowerMock.replayAll(); expectedConfigs.put("client.id", "connector-producer-job-0"); assertEquals(expectedConfigs, Worker.producerConfigs("connector-producer-" + TASK_ID, config)); } @@ -845,7 +860,33 @@ public void testProducerConfigsWithOverrides() { expectedConfigs.put("acks", "-1"); expectedConfigs.put("linger.ms", "1000"); expectedConfigs.put("client.id", "producer-test-id"); - assertEquals(expectedConfigs, Worker.producerConfigs("connector-producer-" + TASK_ID, configWithOverrides)); + EasyMock.expect(connectorConfig.originalsWithPrefix("producer.")).andReturn( + new HashMap()); + PowerMock.replayAll(); + assertEquals(expectedConfigs, Worker.producerConfigs(new ConnectorTaskId("test", 1), configWithOverrides, connectorConfig, + null, allConnectorClientConfigOverridePolicy)); + } + + @Test + public void testProducerConfigsWithClientOverrides() { + Map props = new HashMap<>(workerProps); + props.put("producer.acks", "-1"); + props.put("producer.linger.ms", "1000"); + props.put("producer.client.id", "producer-test-id"); + WorkerConfig configWithOverrides = new StandaloneConfig(props); + + Map expectedConfigs = new HashMap<>(defaultProducerConfigs); + expectedConfigs.put("acks", "-1"); + expectedConfigs.put("linger.ms", "5000"); + expectedConfigs.put("batch.size", "1000"); + expectedConfigs.put("client.id", "producer-test-id"); + Map connConfig = new HashMap(); + connConfig.put("linger.ms", "5000"); + connConfig.put("batch.size", "1000"); + EasyMock.expect(connectorConfig.originalsWithPrefix("producer.")).andReturn(connConfig); + PowerMock.replayAll(); + assertEquals(expectedConfigs, Worker.producerConfigs(new ConnectorTaskId("test", 1), configWithOverrides, connectorConfig, + null, allConnectorClientConfigOverridePolicy)); } @Test @@ -853,7 +894,11 @@ public void testConsumerConfigsWithoutOverrides() { Map expectedConfigs = new HashMap<>(defaultConsumerConfigs); expectedConfigs.put("group.id", "connect-test"); expectedConfigs.put("client.id", "connector-consumer-test-1"); - assertEquals(expectedConfigs, Worker.consumerConfigs(new ConnectorTaskId("test", 1), config)); + EasyMock.expect(connectorConfig.originalsWithPrefix("consumer.")).andReturn( + new HashMap()); + PowerMock.replayAll(); + assertEquals(expectedConfigs, Worker.consumerConfigs(new ConnectorTaskId("test", 1), config, connectorConfig, + null, noneConnectorClientConfigOverridePolicy)); } @Test @@ -869,9 +914,53 @@ public void testConsumerConfigsWithOverrides() { expectedConfigs.put("auto.offset.reset", "latest"); expectedConfigs.put("max.poll.records", "1000"); expectedConfigs.put("client.id", "consumer-test-id"); - assertEquals(expectedConfigs, Worker.consumerConfigs(new ConnectorTaskId("test", 1), configWithOverrides)); + EasyMock.expect(connectorConfig.originalsWithPrefix("consumer.")).andReturn( + new HashMap()); + PowerMock.replayAll(); + assertEquals(expectedConfigs, Worker.consumerConfigs(new ConnectorTaskId("test", 1), configWithOverrides, connectorConfig, + null, noneConnectorClientConfigOverridePolicy)); + } + @Test + public void testConsumerConfigsWithClientOverrides() { + Map props = new HashMap<>(workerProps); + props.put("consumer.auto.offset.reset", "latest"); + props.put("consumer.max.poll.records", "5000"); + WorkerConfig configWithOverrides = new StandaloneConfig(props); + + Map expectedConfigs = new HashMap<>(defaultConsumerConfigs); + expectedConfigs.put("group.id", "connect-test"); + expectedConfigs.put("auto.offset.reset", "latest"); + expectedConfigs.put("max.poll.records", "5000"); + expectedConfigs.put("max.poll.interval.ms", "1000"); + + Map connConfig = new HashMap(); + connConfig.put("max.poll.records", "5000"); + connConfig.put("max.poll.interval.ms", "1000"); + EasyMock.expect(connectorConfig.originalsWithPrefix("consumer.")).andReturn(connConfig); + PowerMock.replayAll(); + assertEquals(expectedConfigs, Worker.consumerConfigs(new ConnectorTaskId("test", 1), configWithOverrides, connectorConfig, + null, allConnectorClientConfigOverridePolicy)); + } + + @Test(expected = ConnectException.class) + public void testConsumerConfigsClientOverridesWithNonePolicy() { + Map props = new HashMap<>(workerProps); + props.put("consumer.auto.offset.reset", "latest"); + props.put("consumer.max.poll.records", "5000"); + WorkerConfig configWithOverrides = new StandaloneConfig(props); + + Map connConfig = new HashMap(); + connConfig.put("max.poll.records", "5000"); + connConfig.put("max.poll.interval.ms", "1000"); + EasyMock.expect(connectorConfig.originalsWithPrefix("consumer.")).andReturn(connConfig); + PowerMock.replayAll(); + Worker.consumerConfigs(new ConnectorTaskId("test", 1), configWithOverrides, connectorConfig, + null, noneConnectorClientConfigOverridePolicy); + } + + private void assertStatistics(Worker worker, int connectors, int tasks) { MetricGroup workerMetrics = worker.workerMetricsGroup().metricGroup(); assertEquals(connectors, MockConnectMetrics.currentMetricValueAsDouble(worker.metrics(), workerMetrics, "connector-count"), 0.0001d); diff --git a/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/distributed/DistributedHerderTest.java b/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/distributed/DistributedHerderTest.java index b381263718256..c5035dc9b1fb2 100644 --- a/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/distributed/DistributedHerderTest.java +++ b/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/distributed/DistributedHerderTest.java @@ -191,7 +191,7 @@ public void setUp() throws Exception { herder = PowerMock.createPartialMock(DistributedHerder.class, new String[]{"backoff", "connectorTypeForClass", "updateDeletedConnectorStatus"}, new DistributedConfig(HERDER_CONFIG), worker, WORKER_ID, KAFKA_CLUSTER_ID, - statusBackingStore, configBackingStore, member, MEMBER_URL, metrics, time); + statusBackingStore, configBackingStore, member, MEMBER_URL, metrics, time, null); configUpdateListener = herder.new ConfigUpdateListener(); rebalanceListener = herder.new RebalanceListener(time); diff --git a/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java b/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java index 8aa1c706f7af1..7f77430566bee 100644 --- a/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java +++ b/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java @@ -115,7 +115,7 @@ private enum SourceSink { public void setup() { worker = PowerMock.createMock(Worker.class); herder = PowerMock.createPartialMock(StandaloneHerder.class, new String[]{"connectorTypeForClass"}, - worker, WORKER_ID, KAFKA_CLUSTER_ID, statusBackingStore, new MemoryConfigBackingStore(transformer)); + worker, WORKER_ID, KAFKA_CLUSTER_ID, statusBackingStore, new MemoryConfigBackingStore(transformer), null); plugins = PowerMock.createMock(Plugins.class); pluginLoader = PowerMock.createMock(PluginClassLoader.class); delegatingLoader = PowerMock.createMock(DelegatingClassLoader.class); From cbcf3953a4d99a0cc037c5d3064426493b700286 Mon Sep 17 00:00:00 2001 From: Magesh Nandakumar Date: Tue, 14 May 2019 22:35:57 -0700 Subject: [PATCH 2/8] Fix review comments Fix review comments Fix review comments --- checkstyle/suppressions.xml | 3 +- .../policy/ConnectorClientConfigRequest.java | 6 +- .../kafka/connect/cli/ConnectDistributed.java | 10 ++- .../kafka/connect/cli/ConnectStandalone.java | 9 +-- ...llConnectorClientConfigOverridePolicy.java | 10 ++- ...neConnectorClientConfigOverridePolicy.java | 11 +++- ...alConnectorClientConfigOverridePolicy.java | 22 +++++-- .../kafka/connect/runtime/AbstractHerder.java | 21 +++--- .../connect/runtime/ConnectorConfig.java | 5 ++ .../apache/kafka/connect/runtime/Worker.java | 66 +++++++++---------- .../kafka/connect/runtime/WorkerConfig.java | 2 +- .../connect/runtime/AbstractHerderTest.java | 15 +++-- .../kafka/connect/runtime/WorkerTest.java | 23 ++++--- 13 files changed, 113 insertions(+), 90 deletions(-) diff --git a/checkstyle/suppressions.xml b/checkstyle/suppressions.xml index 0c65edca68599..977a7ac286ac5 100644 --- a/checkstyle/suppressions.xml +++ b/checkstyle/suppressions.xml @@ -81,7 +81,8 @@ - + diff --git a/connect/api/src/main/java/org/apache/kafka/connect/connector/policy/ConnectorClientConfigRequest.java b/connect/api/src/main/java/org/apache/kafka/connect/connector/policy/ConnectorClientConfigRequest.java index 5cd9b6465a018..11f756bcb41e3 100644 --- a/connect/api/src/main/java/org/apache/kafka/connect/connector/policy/ConnectorClientConfigRequest.java +++ b/connect/api/src/main/java/org/apache/kafka/connect/connector/policy/ConnectorClientConfigRequest.java @@ -44,27 +44,23 @@ public ConnectorClientConfigRequest( } /** - * 
      * Provides Config with prefix {@code producer.override.} for {@link ConnectorType#SOURCE}.
      * Provides Config with prefix {@code consumer.override.} for {@link ConnectorType#SINK}.
      * Provides Config with prefix {@code producer.override.} for {@link ConnectorType#SINK} for DLQ.
      * Provides Config with prefix {@code admin.override.} for {@link ConnectorType#SINK} for DLQ.
-     *
* * @return The client properties specified in the Connector Config with prefix {@code producer.override.} , - * {@code consumer.override.} and {@code admin.override.}. The configs returned don't include these prefixes. + * {@code consumer.override.} and {@code admin.override.}. The configs don't include the prefixes. */ public Map clientProps() { return clientProps; } /** - * 
      * {@link ClientType#PRODUCER} for {@link ConnectorType#SOURCE}
      * {@link ClientType#CONSUMER} for {@link ConnectorType#SINK}
      * {@link ClientType#PRODUCER} for DLQ in {@link ConnectorType#SINK}
      * {@link ClientType#ADMIN} for DLQ  Topic Creation in {@link ConnectorType#SINK}
-     *
* * @return enumeration specifying the client type that is being overriden by the worker; never null. */ diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/cli/ConnectDistributed.java b/connect/runtime/src/main/java/org/apache/kafka/connect/cli/ConnectDistributed.java index 27e62896ec453..22c1ad82d6138 100644 --- a/connect/runtime/src/main/java/org/apache/kafka/connect/cli/ConnectDistributed.java +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/cli/ConnectDistributed.java @@ -104,12 +104,10 @@ public Connect startConnect(Map workerProps) { KafkaOffsetBackingStore offsetBackingStore = new KafkaOffsetBackingStore(); offsetBackingStore.configure(config); - ConnectorClientConfigOverridePolicy connectorClientConfigOverridePolicy = null; - if (config.getString(WorkerConfig.CONNECTOR_CLIENT_POLICY_CLASS_CONFIG) != null) { - connectorClientConfigOverridePolicy = plugins.newPlugin( + ConnectorClientConfigOverridePolicy connectorClientConfigOverridePolicy = plugins.newPlugin( config.getString(WorkerConfig.CONNECTOR_CLIENT_POLICY_CLASS_CONFIG), config, ConnectorClientConfigOverridePolicy.class); - } + Worker worker = new Worker(workerId, time, plugins, config, offsetBackingStore, connectorClientConfigOverridePolicy); WorkerConfigTransformer configTransformer = worker.configTransformer(); @@ -123,8 +121,8 @@ public Connect startConnect(Map workerProps) { configTransformer); DistributedHerder herder = new DistributedHerder(config, time, worker, - kafkaClusterId, statusBackingStore, configBackingStore, - advertisedUrl.toString(), connectorClientConfigOverridePolicy); + kafkaClusterId, statusBackingStore, configBackingStore, + advertisedUrl.toString(), connectorClientConfigOverridePolicy); final Connect connect = new Connect(herder, rest); log.info("Kafka Connect distributed worker initialization took {}ms", time.hiResClockMs() - initStart); diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/cli/ConnectStandalone.java b/connect/runtime/src/main/java/org/apache/kafka/connect/cli/ConnectStandalone.java index 971637738bb82..cf7b93bd7c838 100644 --- a/connect/runtime/src/main/java/org/apache/kafka/connect/cli/ConnectStandalone.java +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/cli/ConnectStandalone.java @@ -89,12 +89,9 @@ public static void main(String[] args) { URI advertisedUrl = rest.advertisedUrl(); String workerId = advertisedUrl.getHost() + ":" + advertisedUrl.getPort(); - ConnectorClientConfigOverridePolicy connectorClientConfigOverridePolicy = null; - if (config.getString(WorkerConfig.CONNECTOR_CLIENT_POLICY_CLASS_CONFIG) != null) { - connectorClientConfigOverridePolicy = plugins.newPlugin( - config.getString(WorkerConfig.CONNECTOR_CLIENT_POLICY_CLASS_CONFIG), - config, ConnectorClientConfigOverridePolicy.class); - } + ConnectorClientConfigOverridePolicy connectorClientConfigOverridePolicy = plugins.newPlugin( + config.getString(WorkerConfig.CONNECTOR_CLIENT_POLICY_CLASS_CONFIG), + config, ConnectorClientConfigOverridePolicy.class); Worker worker = new Worker(workerId, time, plugins, config, new FileOffsetBackingStore(), connectorClientConfigOverridePolicy); diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/AllConnectorClientConfigOverridePolicy.java b/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/AllConnectorClientConfigOverridePolicy.java index ee3ef4b007789..07ee104ba691e 100644 --- a/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/AllConnectorClientConfigOverridePolicy.java +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/AllConnectorClientConfigOverridePolicy.java @@ -18,10 +18,16 @@ package org.apache.kafka.connect.connector.policy; import org.apache.kafka.common.errors.PolicyViolationException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.util.Map; +/** + * Allows all client configurations to be overridden via the connector configs by setting {@code client.config.policy} to {@code All} + */ public class AllConnectorClientConfigOverridePolicy implements ConnectorClientConfigOverridePolicy { + private static final Logger log = LoggerFactory.getLogger(AllConnectorClientConfigOverridePolicy.class); @Override public void validate(ConnectorClientConfigRequest connectorClientConfigRequest) throws PolicyViolationException { @@ -29,12 +35,12 @@ public void validate(ConnectorClientConfigRequest connectorClientConfigRequest) } @Override - public void close() throws Exception { + public void close() { } @Override public void configure(Map configs) { - + log.info("Setting up All Policy for ConnectorClientConfigOverride. This will allow all client configurations to be overridden"); } } diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/NoneConnectorClientConfigOverridePolicy.java b/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/NoneConnectorClientConfigOverridePolicy.java index e643b0551f4f7..fe411fa6c86b4 100644 --- a/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/NoneConnectorClientConfigOverridePolicy.java +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/NoneConnectorClientConfigOverridePolicy.java @@ -18,10 +18,17 @@ package org.apache.kafka.connect.connector.policy; import org.apache.kafka.common.errors.PolicyViolationException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.util.Map; +/** + * Disallow any client configuration to be overridden via the connector configs by setting {@code client.config.policy} to {@code None}. + * This is the default behavior. + */ public class NoneConnectorClientConfigOverridePolicy implements ConnectorClientConfigOverridePolicy { + private static final Logger log = LoggerFactory.getLogger(NoneConnectorClientConfigOverridePolicy.class); @Override public void validate(ConnectorClientConfigRequest connectorClientConfigRequest) throws PolicyViolationException { @@ -31,12 +38,12 @@ public void validate(ConnectorClientConfigRequest connectorClientConfigRequest) } @Override - public void close() throws Exception { + public void close() { } @Override public void configure(Map configs) { - + log.info("Setting up None Policy for ConnectorClientConfigOverride. This will disallow any client configuration to be overridden"); } } diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/PrincipalConnectorClientConfigOverridePolicy.java b/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/PrincipalConnectorClientConfigOverridePolicy.java index 4ee118f10714b..1cf60897405ac 100644 --- a/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/PrincipalConnectorClientConfigOverridePolicy.java +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/PrincipalConnectorClientConfigOverridePolicy.java @@ -19,31 +19,41 @@ import org.apache.kafka.common.config.SaslConfigs; import org.apache.kafka.common.errors.PolicyViolationException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; -import java.util.Collections; import java.util.Map; import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.Stream; +/** + * Allows all {@code sasl} configurations to be overridden via the connector configs by setting {@code client.config.policy} to + * {@code Principal}. This allows to set a principal per connector. + */ public class PrincipalConnectorClientConfigOverridePolicy implements ConnectorClientConfigOverridePolicy { + private static final Logger log = LoggerFactory.getLogger(PrincipalConnectorClientConfigOverridePolicy.class); - private static final Set ALLOWED_CONFIG = Collections.singleton(SaslConfigs.SASL_JAAS_CONFIG); + private static final Set ALLOWED_CONFIG = + Stream.of(SaslConfigs.SASL_JAAS_CONFIG, SaslConfigs.SASL_MECHANISM).collect(Collectors.toSet()); @Override public void validate(ConnectorClientConfigRequest connectorClientConfigRequest) throws PolicyViolationException { if (!ALLOWED_CONFIG.containsAll(connectorClientConfigRequest.clientProps().keySet())) { - throw new PolicyViolationException("Can override " + connectorClientConfigRequest.clientType() + " with only " + - ALLOWED_CONFIG); + throw new PolicyViolationException( + "Can override " + connectorClientConfigRequest.clientType() + " with only " + ALLOWED_CONFIG); } } @Override - public void close() throws Exception { + public void close() { } @Override public void configure(Map configs) { - + log.info("Setting up Principal policy for ConnectorClientConfigOverride. This will allow `sasl` client configuration to be " + + "overridden."); } } diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractHerder.java b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractHerder.java index 55806c4c67e63..5201bbca54995 100644 --- a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractHerder.java +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractHerder.java @@ -336,10 +336,12 @@ public ConfigInfos validateConnectorConfig(Map connectorProps) { AbstractConfig connectorConfig = new AbstractConfig(new ConfigDef(), connectorProps); String connName = connectorProps.get(ConnectorConfig.NAME_CONFIG); - ConfigInfos producerConfigInfos = null, consumerConfigInfos = null, adminConfigInfos = null; + ConfigInfos producerConfigInfos = null; + ConfigInfos consumerConfigInfos = null; + ConfigInfos adminConfigInfos = null; if (connectorType.equals(org.apache.kafka.connect.health.ConnectorType.SOURCE)) { producerConfigInfos = validateClientOverrides(connName, - "producer.", + ConnectorConfig.CONNECTOR_CLIENT_PRODUCER_OVERRIDES_PREFIX, connectorConfig, ProducerConfig.configDef(), connector.getClass(), @@ -349,7 +351,7 @@ public ConfigInfos validateConnectorConfig(Map connectorProps) { return mergeConfigInfos(connType, configInfos, producerConfigInfos); } else { consumerConfigInfos = validateClientOverrides(connName, - "consumer.", + ConnectorConfig.CONNECTOR_CLIENT_CONSUMER_OVERRIDES_PREFIX, connectorConfig, ProducerConfig.configDef(), connector.getClass(), @@ -360,7 +362,7 @@ public ConfigInfos validateConnectorConfig(Map connectorProps) { String topic = connectorProps.get(SinkConnectorConfig.DLQ_TOPIC_NAME_CONFIG); if (topic != null && !topic.isEmpty()) { adminConfigInfos = validateClientOverrides(connName, - "admin.", + ConnectorConfig.CONNECTOR_CLIENT_ADMIN_OVERRIDES_PREFIX, connectorConfig, ProducerConfig.configDef(), connector.getClass(), @@ -390,8 +392,7 @@ private static ConfigInfos mergeConfigInfos(String connType, ConfigInfos... conf return new ConfigInfos(connType, errorCount, new ArrayList<>(groups), configInfoList); } - // public for testing - public static ConfigInfos validateClientOverrides(String connName, + private static ConfigInfos validateClientOverrides(String connName, String prefix, AbstractConfig connectorConfig, ConfigDef configDef, @@ -461,6 +462,10 @@ public static ConfigInfos generateResult(String connType, Map return new ConfigInfos(connType, errorCount, groups, configInfoList); } + private static ConfigKeyInfo convertConfigKey(ConfigKey configKey) { + return convertConfigKey(configKey, ""); + } + private static ConfigKeyInfo convertConfigKey(ConfigKey configKey, String prefix) { String name = prefix + configKey.name; Type type = configKey.type; @@ -484,10 +489,6 @@ private static ConfigKeyInfo convertConfigKey(ConfigKey configKey, String prefix return new ConfigKeyInfo(name, typeName, required, defaultValue, importance, documentation, group, orderInGroup, width, displayName, dependents); } - private static ConfigKeyInfo convertConfigKey(ConfigKey configKey) { - return convertConfigKey(configKey, ""); - } - private static ConfigValueInfo convertConfigValue(ConfigValue configValue, Type type) { String value = ConfigDef.convertToString(configValue.value(), type); List recommendedValues = new LinkedList<>(); diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/ConnectorConfig.java b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/ConnectorConfig.java index 8889aadbae18a..1cebc94222fc9 100644 --- a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/ConnectorConfig.java +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/ConnectorConfig.java @@ -140,6 +140,11 @@ public class ConnectorConfig extends AbstractConfig { "a failure. This is 'false' by default, which will prevent record keys, values, and headers from being written to log files, " + "although some information such as topic and partition number will still be logged."; + + public static final String CONNECTOR_CLIENT_PRODUCER_OVERRIDES_PREFIX = "producer.overrides."; + public static final String CONNECTOR_CLIENT_CONSUMER_OVERRIDES_PREFIX = "consumer.overrides."; + public static final String CONNECTOR_CLIENT_ADMIN_OVERRIDES_PREFIX = "admin.overrides."; + private final EnrichedConnectorConfig enrichedConfig; private static class EnrichedConnectorConfig extends AbstractConfig { EnrichedConnectorConfig(ConfigDef configDef, Map props) { diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Worker.java b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Worker.java index 3f83fc602dd96..7aa96b51ed409 100644 --- a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Worker.java +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Worker.java @@ -553,19 +553,10 @@ static Map producerConfigs(String defaultClientId, // User-specified overrides producerProps.putAll(config.originalsWithPrefix("producer.")); - Map producerOverrides = connConfig.originalsWithPrefix("producer."); - ConnectorClientConfigRequest connectorClientConfigRequest = new ConnectorClientConfigRequest( - id.connector(), - ConnectorType.SOURCE, - connectorClass, - producerOverrides, - ConnectorClientConfigRequest.ClientType.PRODUCER - ); - try { - connectorClientConfigOverridePolicy.validate(connectorClientConfigRequest); - } catch (PolicyViolationException e) { - throw new ConnectException("Error applying client config overrides", e); - } + Map producerOverrides = + connectorClientConfigOverrides(id, connConfig, connectorClass, ConnectorConfig.CONNECTOR_CLIENT_PRODUCER_OVERRIDES_PREFIX, + ConnectorType.SOURCE, ConnectorClientConfigRequest.ClientType.PRODUCER, + connectorClientConfigOverridePolicy); producerProps.putAll(producerOverrides); return producerProps; @@ -583,7 +574,7 @@ static Map consumerConfigs(ConnectorTaskId id, consumerProps.put(ConsumerConfig.GROUP_ID_CONFIG, SinkUtils.consumerGroupId(id.connector())); consumerProps.put(ConsumerConfig.CLIENT_ID_CONFIG, "connector-consumer-" + id); consumerProps.put(ConsumerConfig.BOOTSTRAP_SERVERS_CONFIG, - Utils.join(config.getList(WorkerConfig.BOOTSTRAP_SERVERS_CONFIG), ",")); + Utils.join(config.getList(WorkerConfig.BOOTSTRAP_SERVERS_CONFIG), ",")); consumerProps.put(ConsumerConfig.ENABLE_AUTO_COMMIT_CONFIG, "false"); consumerProps.put(ConsumerConfig.AUTO_OFFSET_RESET_CONFIG, "earliest"); consumerProps.put(ConsumerConfig.KEY_DESERIALIZER_CLASS_CONFIG, "org.apache.kafka.common.serialization.ByteArrayDeserializer"); @@ -591,19 +582,10 @@ static Map consumerConfigs(ConnectorTaskId id, consumerProps.putAll(config.originalsWithPrefix("consumer.")); - Map consumerOverrides = connConfig.originalsWithPrefix("consumer."); - ConnectorClientConfigRequest connectorClientConfigRequest = new ConnectorClientConfigRequest( - id.connector(), - ConnectorType.SINK, - connectorClass, - consumerOverrides, - ConnectorClientConfigRequest.ClientType.CONSUMER - ); - try { - connectorClientConfigOverridePolicy.validate(connectorClientConfigRequest); - } catch (PolicyViolationException e) { - throw new ConnectException("Error applying client config overrides", e); - } + Map consumerOverrides = + connectorClientConfigOverrides(id, connConfig, connectorClass, ConnectorConfig.CONNECTOR_CLIENT_CONSUMER_OVERRIDES_PREFIX, + ConnectorType.SINK, ConnectorClientConfigRequest.ClientType.CONSUMER, + connectorClientConfigOverridePolicy); consumerProps.putAll(consumerOverrides); return consumerProps; @@ -619,23 +601,36 @@ static Map adminConfigs(ConnectorTaskId id, // User-specified overrides adminProps.putAll(config.originalsWithPrefix("admin.")); + Map adminOverrides = + connectorClientConfigOverrides(id, connConfig, connectorClass, ConnectorConfig.CONNECTOR_CLIENT_ADMIN_OVERRIDES_PREFIX, + ConnectorType.SINK, ConnectorClientConfigRequest.ClientType.ADMIN, + connectorClientConfigOverridePolicy); + adminProps.putAll(adminOverrides); + + return adminProps; + } - Map adminOverrides = connConfig.originalsWithPrefix("admin."); + private static Map connectorClientConfigOverrides(ConnectorTaskId id, + ConnectorConfig connConfig, + Class connectorClass, + String clientConfigPrefix, + ConnectorType connectorType, + ConnectorClientConfigRequest.ClientType clientType, + ConnectorClientConfigOverridePolicy connectorClientConfigOverridePolicy) { + Map clientOverrides = connConfig.originalsWithPrefix(clientConfigPrefix); ConnectorClientConfigRequest connectorClientConfigRequest = new ConnectorClientConfigRequest( id.connector(), - ConnectorType.SINK, + connectorType, connectorClass, - adminOverrides, - ConnectorClientConfigRequest.ClientType.ADMIN + clientOverrides, + clientType ); try { connectorClientConfigOverridePolicy.validate(connectorClientConfigRequest); } catch (PolicyViolationException e) { throw new ConnectException("Error applying client config overrides", e); } - adminProps.putAll(adminOverrides); - - return adminProps; + return clientOverrides; } ErrorHandlingMetrics errorHandlingMetrics(ConnectorTaskId id) { @@ -654,8 +649,7 @@ private List sinkTaskReporters(ConnectorTaskId id, SinkConnectorC if (topic != null && !topic.isEmpty()) { Map producerProps = producerConfigs("connector-dlq-producer-" + id, config, connConfig, connectorClass, connectorClientConfigOverridePolicy); Map adminProps = adminConfigs(id, config, connConfig, connectorClass, connectorClientConfigOverridePolicy); - DeadLetterQueueReporter reporter = DeadLetterQueueReporter.createAndSetup(adminProps, id, connConfig, producerProps, - errorHandlingMetrics); + DeadLetterQueueReporter reporter = DeadLetterQueueReporter.createAndSetup(adminProps, id, connConfig, producerProps, errorHandlingMetrics); reporters.add(reporter); } diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/WorkerConfig.java b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/WorkerConfig.java index f5e2572f319f4..2652440701274 100644 --- a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/WorkerConfig.java +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/WorkerConfig.java @@ -215,7 +215,7 @@ public class WorkerConfig extends AbstractConfig { public static final String CONNECTOR_CLIENT_POLICY_CLASS_CONFIG = "client.config.policy"; public static final String CONNECTOR_CLIENT_POLICY_CLASS_DOC = "Class name or alias of implementation of ConnectorClientConfigOverridePolicy. Defines what client configurations can be " - + "overriden by the connector"; + + "overriden by the connector> The default implementation is `None` and other possible values include `All` and `Principal`."; public static final String CONNECTOR_CLIENT_POLICY_CLASS_DEFAULT = "None"; diff --git a/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/AbstractHerderTest.java b/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/AbstractHerderTest.java index dcdbf868f9b53..ef7ec14039f8e 100644 --- a/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/AbstractHerderTest.java +++ b/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/AbstractHerderTest.java @@ -367,8 +367,10 @@ public void testConfigValidationPrincipalOnlyOverride() { config.put(ConnectorConfig.CONNECTOR_CLASS_CONFIG, TestSourceConnector.class.getName()); config.put(ConnectorConfig.NAME_CONFIG, "connector-name"); config.put("required", "value"); // connector required config - config.put("producer." + ProducerConfig.ACKS_CONFIG, "none"); - config.put("producer." + SaslConfigs.SASL_JAAS_CONFIG, "jaas_config"); + String ackConfigKey = ConnectorConfig.CONNECTOR_CLIENT_PRODUCER_OVERRIDES_PREFIX + ProducerConfig.ACKS_CONFIG; + String saslConfigKey = ConnectorConfig.CONNECTOR_CLIENT_PRODUCER_OVERRIDES_PREFIX + SaslConfigs.SASL_JAAS_CONFIG; + config.put(ackConfigKey, "none"); + config.put(saslConfigKey, "jaas_config"); ConfigInfos result = herder.validateConnectorConfig(config); assertEquals(herder.connectorTypeForClass(config.get(ConnectorConfig.CONNECTOR_CLASS_CONFIG)), ConnectorType.SOURCE); @@ -386,10 +388,11 @@ public void testConfigValidationPrincipalOnlyOverride() { assertEquals(1, result.errorCount()); // Base connector config has 13 fields, connector's configs add 2, and 2 producer overrides assertEquals(17, result.values().size()); - assertEquals("producer." + ProducerConfig.ACKS_CONFIG, result.values().get(15).configValue().name()); - assertFalse(result.values().get(15).configValue().errors().isEmpty()); - assertEquals("producer." + SaslConfigs.SASL_JAAS_CONFIG, result.values().get(16).configValue().name()); - assertTrue(result.values().get(16).configValue().errors().isEmpty()); + assertTrue(result.values().stream().anyMatch( + configInfo -> ackConfigKey.equals(configInfo.configValue().name()) && !configInfo.configValue().errors().isEmpty())); + assertTrue(result.values().stream().anyMatch( + configInfo -> saslConfigKey.equals(configInfo.configValue().name()) && configInfo.configValue().errors().isEmpty())); + verifyAll(); } diff --git a/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/WorkerTest.java b/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/WorkerTest.java index c0f11a323a6f7..f369428fb7fe3 100644 --- a/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/WorkerTest.java +++ b/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/WorkerTest.java @@ -841,11 +841,13 @@ public void testConverterOverrides() throws Exception { @Test public void testProducerConfigsWithoutOverrides() { - EasyMock.expect(connectorConfig.originalsWithPrefix("producer.")).andReturn( + EasyMock.expect(connectorConfig.originalsWithPrefix(ConnectorConfig.CONNECTOR_CLIENT_PRODUCER_OVERRIDES_PREFIX)).andReturn( new HashMap()); PowerMock.replayAll(); + Map expectedConfigs = new HashMap<>(defaultProducerConfigs); expectedConfigs.put("client.id", "connector-producer-job-0"); - assertEquals(expectedConfigs, Worker.producerConfigs("connector-producer-" + TASK_ID, config)); + assertEquals(defaultProducerConfigs, Worker.producerConfigs("connector-producer-" + TASK_ID, config, connectorConfig, + null, noneConnectorClientConfigOverridePolicy)); } @Test @@ -860,7 +862,7 @@ public void testProducerConfigsWithOverrides() { expectedConfigs.put("acks", "-1"); expectedConfigs.put("linger.ms", "1000"); expectedConfigs.put("client.id", "producer-test-id"); - EasyMock.expect(connectorConfig.originalsWithPrefix("producer.")).andReturn( + EasyMock.expect(connectorConfig.originalsWithPrefix(ConnectorConfig.CONNECTOR_CLIENT_PRODUCER_OVERRIDES_PREFIX)).andReturn( new HashMap()); PowerMock.replayAll(); assertEquals(expectedConfigs, Worker.producerConfigs(new ConnectorTaskId("test", 1), configWithOverrides, connectorConfig, @@ -883,7 +885,8 @@ public void testProducerConfigsWithClientOverrides() { Map connConfig = new HashMap(); connConfig.put("linger.ms", "5000"); connConfig.put("batch.size", "1000"); - EasyMock.expect(connectorConfig.originalsWithPrefix("producer.")).andReturn(connConfig); + EasyMock.expect(connectorConfig.originalsWithPrefix(ConnectorConfig.CONNECTOR_CLIENT_PRODUCER_OVERRIDES_PREFIX)) + .andReturn(connConfig); PowerMock.replayAll(); assertEquals(expectedConfigs, Worker.producerConfigs(new ConnectorTaskId("test", 1), configWithOverrides, connectorConfig, null, allConnectorClientConfigOverridePolicy)); @@ -894,7 +897,7 @@ public void testConsumerConfigsWithoutOverrides() { Map expectedConfigs = new HashMap<>(defaultConsumerConfigs); expectedConfigs.put("group.id", "connect-test"); expectedConfigs.put("client.id", "connector-consumer-test-1"); - EasyMock.expect(connectorConfig.originalsWithPrefix("consumer.")).andReturn( + EasyMock.expect(connectorConfig.originalsWithPrefix(ConnectorConfig.CONNECTOR_CLIENT_CONSUMER_OVERRIDES_PREFIX)).andReturn( new HashMap()); PowerMock.replayAll(); assertEquals(expectedConfigs, Worker.consumerConfigs(new ConnectorTaskId("test", 1), config, connectorConfig, @@ -914,8 +917,8 @@ public void testConsumerConfigsWithOverrides() { expectedConfigs.put("auto.offset.reset", "latest"); expectedConfigs.put("max.poll.records", "1000"); expectedConfigs.put("client.id", "consumer-test-id"); - EasyMock.expect(connectorConfig.originalsWithPrefix("consumer.")).andReturn( - new HashMap()); + EasyMock.expect(connectorConfig.originalsWithPrefix(ConnectorConfig.CONNECTOR_CLIENT_CONSUMER_OVERRIDES_PREFIX)) + .andReturn(new HashMap()); PowerMock.replayAll(); assertEquals(expectedConfigs, Worker.consumerConfigs(new ConnectorTaskId("test", 1), configWithOverrides, connectorConfig, null, noneConnectorClientConfigOverridePolicy)); @@ -938,7 +941,8 @@ public void testConsumerConfigsWithClientOverrides() { Map connConfig = new HashMap(); connConfig.put("max.poll.records", "5000"); connConfig.put("max.poll.interval.ms", "1000"); - EasyMock.expect(connectorConfig.originalsWithPrefix("consumer.")).andReturn(connConfig); + EasyMock.expect(connectorConfig.originalsWithPrefix(ConnectorConfig.CONNECTOR_CLIENT_CONSUMER_OVERRIDES_PREFIX)) + .andReturn(connConfig); PowerMock.replayAll(); assertEquals(expectedConfigs, Worker.consumerConfigs(new ConnectorTaskId("test", 1), configWithOverrides, connectorConfig, null, allConnectorClientConfigOverridePolicy)); @@ -954,7 +958,8 @@ public void testConsumerConfigsClientOverridesWithNonePolicy() { Map connConfig = new HashMap(); connConfig.put("max.poll.records", "5000"); connConfig.put("max.poll.interval.ms", "1000"); - EasyMock.expect(connectorConfig.originalsWithPrefix("consumer.")).andReturn(connConfig); + EasyMock.expect(connectorConfig.originalsWithPrefix(ConnectorConfig.CONNECTOR_CLIENT_CONSUMER_OVERRIDES_PREFIX)) + .andReturn(connConfig); PowerMock.replayAll(); Worker.consumerConfigs(new ConnectorTaskId("test", 1), configWithOverrides, connectorConfig, null, noneConnectorClientConfigOverridePolicy); From f04cc546b66684e0d917e79708628b9944fc37ee Mon Sep 17 00:00:00 2001 From: Magesh Nandakumar Date: Thu, 16 May 2019 00:51:21 -0700 Subject: [PATCH 3/8] use List instead of PolicyViolationException --- .../ConnectorClientConfigOverridePolicy.java | 8 ++- ...llConnectorClientConfigOverridePolicy.java | 7 ++- ...neConnectorClientConfigOverridePolicy.java | 19 ++++-- ...alConnectorClientConfigOverridePolicy.java | 23 ++++++-- .../kafka/connect/runtime/AbstractHerder.java | 42 +++++++------ .../apache/kafka/connect/runtime/Worker.java | 12 ++-- ...nnectorClientConfigOverridePolicyTest.java | 59 +++++++++++++++++++ ...nnectorClientConfigOverridePolicyTest.java | 29 +++------ ...nnectorClientConfigOverridePolicyTest.java | 28 +++------ .../connect/runtime/AbstractHerderTest.java | 25 ++++---- .../distributed/DistributedHerderTest.java | 6 +- .../standalone/StandaloneHerderTest.java | 8 ++- .../util/clusters/EmbeddedConnectCluster.java | 22 +++++-- 13 files changed, 187 insertions(+), 101 deletions(-) create mode 100644 connect/runtime/src/test/java/org/apache/kafka/connect/connector/policy/BaseConnectorClientConfigOverridePolicyTest.java diff --git a/connect/api/src/main/java/org/apache/kafka/connect/connector/policy/ConnectorClientConfigOverridePolicy.java b/connect/api/src/main/java/org/apache/kafka/connect/connector/policy/ConnectorClientConfigOverridePolicy.java index 3c74282b9c202..31eab1e1024ae 100644 --- a/connect/api/src/main/java/org/apache/kafka/connect/connector/policy/ConnectorClientConfigOverridePolicy.java +++ b/connect/api/src/main/java/org/apache/kafka/connect/connector/policy/ConnectorClientConfigOverridePolicy.java @@ -18,7 +18,9 @@ package org.apache.kafka.connect.connector.policy; import org.apache.kafka.common.Configurable; -import org.apache.kafka.common.errors.PolicyViolationException; +import org.apache.kafka.common.config.ConfigValue; + +import java.util.List; /** *

An interface for enforcing a policy on overriding of client configs via the connector configs. @@ -38,7 +40,7 @@ public interface ConnectorClientConfigOverridePolicy extends Configurable, AutoC * * @param connectorClientConfigRequest an instance of {@code ConnectorClientConfigRequest} that provides the configs to overridden and * its context; never {@code null} - * @throws PolicyViolationException if any of the overridden property doesn't meet the defined policy + * @return List of Config, each Config should indicate if they are allowed via {@link ConfigValue#errorMessages} */ - void validate(ConnectorClientConfigRequest connectorClientConfigRequest) throws PolicyViolationException; + List validate(ConnectorClientConfigRequest connectorClientConfigRequest); } \ No newline at end of file diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/AllConnectorClientConfigOverridePolicy.java b/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/AllConnectorClientConfigOverridePolicy.java index 07ee104ba691e..d72da569e00d3 100644 --- a/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/AllConnectorClientConfigOverridePolicy.java +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/AllConnectorClientConfigOverridePolicy.java @@ -17,10 +17,12 @@ package org.apache.kafka.connect.connector.policy; -import org.apache.kafka.common.errors.PolicyViolationException; +import org.apache.kafka.common.config.ConfigValue; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.util.Collections; +import java.util.List; import java.util.Map; /** @@ -30,8 +32,9 @@ public class AllConnectorClientConfigOverridePolicy implements ConnectorClientCo private static final Logger log = LoggerFactory.getLogger(AllConnectorClientConfigOverridePolicy.class); @Override - public void validate(ConnectorClientConfigRequest connectorClientConfigRequest) throws PolicyViolationException { + public List validate(ConnectorClientConfigRequest connectorClientConfigRequest) { //allow all no op + return Collections.emptyList(); } @Override diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/NoneConnectorClientConfigOverridePolicy.java b/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/NoneConnectorClientConfigOverridePolicy.java index fe411fa6c86b4..2954a5ba88965 100644 --- a/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/NoneConnectorClientConfigOverridePolicy.java +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/NoneConnectorClientConfigOverridePolicy.java @@ -17,11 +17,14 @@ package org.apache.kafka.connect.connector.policy; -import org.apache.kafka.common.errors.PolicyViolationException; +import org.apache.kafka.common.config.ConfigValue; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.util.ArrayList; +import java.util.List; import java.util.Map; +import java.util.stream.Collectors; /** * Disallow any client configuration to be overridden via the connector configs by setting {@code client.config.policy} to {@code None}. @@ -31,10 +34,16 @@ public class NoneConnectorClientConfigOverridePolicy implements ConnectorClientC private static final Logger log = LoggerFactory.getLogger(NoneConnectorClientConfigOverridePolicy.class); @Override - public void validate(ConnectorClientConfigRequest connectorClientConfigRequest) throws PolicyViolationException { - if (connectorClientConfigRequest.clientProps().size() > 0) { - throw new PolicyViolationException("Client Config Overrides aren't allowed"); - } + public List validate(ConnectorClientConfigRequest connectorClientConfigRequest) { + Map inputConfig = connectorClientConfigRequest.clientProps(); + return inputConfig.entrySet().stream().map(configEntry -> configValue(configEntry)).collect(Collectors.toList()); + } + + private static ConfigValue configValue(Map.Entry configEntry) { + ConfigValue configValue = + new ConfigValue(configEntry.getKey(), configEntry.getValue(), new ArrayList(), new ArrayList()); + configValue.addErrorMessage("None policy doesn't allow any client overrides"); + return configValue; } @Override diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/PrincipalConnectorClientConfigOverridePolicy.java b/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/PrincipalConnectorClientConfigOverridePolicy.java index 1cf60897405ac..e39164a5c59ef 100644 --- a/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/PrincipalConnectorClientConfigOverridePolicy.java +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/PrincipalConnectorClientConfigOverridePolicy.java @@ -17,11 +17,14 @@ package org.apache.kafka.connect.connector.policy; +import org.apache.kafka.clients.CommonClientConfigs; +import org.apache.kafka.common.config.ConfigValue; import org.apache.kafka.common.config.SaslConfigs; -import org.apache.kafka.common.errors.PolicyViolationException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.util.ArrayList; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.stream.Collectors; @@ -35,15 +38,23 @@ public class PrincipalConnectorClientConfigOverridePolicy implements ConnectorCl private static final Logger log = LoggerFactory.getLogger(PrincipalConnectorClientConfigOverridePolicy.class); private static final Set ALLOWED_CONFIG = - Stream.of(SaslConfigs.SASL_JAAS_CONFIG, SaslConfigs.SASL_MECHANISM).collect(Collectors.toSet()); + Stream.of(SaslConfigs.SASL_JAAS_CONFIG, SaslConfigs.SASL_MECHANISM, CommonClientConfigs.SECURITY_PROTOCOL_CONFIG). + collect(Collectors.toSet()); @Override - public void validate(ConnectorClientConfigRequest connectorClientConfigRequest) throws PolicyViolationException { - if (!ALLOWED_CONFIG.containsAll(connectorClientConfigRequest.clientProps().keySet())) { - throw new PolicyViolationException( - "Can override " + connectorClientConfigRequest.clientType() + " with only " + ALLOWED_CONFIG); + public List validate(ConnectorClientConfigRequest connectorClientConfigRequest) { + Map inputConfig = connectorClientConfigRequest.clientProps(); + return inputConfig.entrySet().stream().map(configEntry -> configValue(configEntry)).collect(Collectors.toList()); + } + + private static ConfigValue configValue(Map.Entry configEntry) { + ConfigValue configValue = + new ConfigValue(configEntry.getKey(), configEntry.getValue(), new ArrayList(), new ArrayList()); + if (!ALLOWED_CONFIG.contains(configEntry.getKey())) { + configValue.addErrorMessage("Principal policy allows only " + ALLOWED_CONFIG + "to be overriden"); } + return configValue; } @Override diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractHerder.java b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractHerder.java index 5201bbca54995..33fb2f8afcf25 100644 --- a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractHerder.java +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractHerder.java @@ -24,7 +24,6 @@ import org.apache.kafka.common.config.ConfigDef.Type; import org.apache.kafka.common.config.ConfigTransformer; import org.apache.kafka.common.config.ConfigValue; -import org.apache.kafka.common.errors.PolicyViolationException; import org.apache.kafka.connect.connector.Connector; import org.apache.kafka.connect.connector.policy.ConnectorClientConfigOverridePolicy; import org.apache.kafka.connect.connector.policy.ConnectorClientConfigRequest; @@ -405,29 +404,28 @@ private static ConfigInfos validateClientOverrides(String connName, Map configKeys = configDef.configKeys(); Set groups = new HashSet<>(); Map clientConfigs = connectorConfig.originalsWithPrefix(prefix); - for (Map.Entry clientConfig : clientConfigs.entrySet()) { - ConfigKey configKey = configKeys.get(clientConfig.getKey()); - ConfigKeyInfo configKeyInfo = null; - if (configKey != null) { - if (configKey.group != null) { - groups.add(configKey.group); + ConnectorClientConfigRequest connectorClientConfigRequest = new ConnectorClientConfigRequest( + connName, connectorType, connectorClass, clientConfigs, clientType); + List configValues = connectorClientConfigOverridePolicy.validate(connectorClientConfigRequest); + if (configValues != null) { + for (ConfigValue validatedConfigValue : configValues) { + ConfigKey configKey = configKeys.get(validatedConfigValue.name()); + ConfigKeyInfo configKeyInfo = null; + if (configKey != null) { + if (configKey.group != null) { + groups.add(configKey.group); + } + configKeyInfo = convertConfigKey(configKey, prefix); } - configKeyInfo = convertConfigKey(configKey, prefix); - } - Map clientProps = Collections.singletonMap(clientConfig.getKey(), clientConfig.getValue()); - ConnectorClientConfigRequest connectorClientConfigRequest = new ConnectorClientConfigRequest( - connName, connectorType, connectorClass, clientProps, clientType); - - ConfigValue configValue = new ConfigValue(prefix + clientConfig.getKey(), clientConfig.getValue(), - new ArrayList(), new ArrayList()); - try { - connectorClientConfigOverridePolicy.validate(connectorClientConfigRequest); - } catch (PolicyViolationException e) { - errorCount++; - configValue.addErrorMessage(e.getMessage()); + + ConfigValue configValue = new ConfigValue(prefix + validatedConfigValue.name(), validatedConfigValue.value(), + validatedConfigValue.recommendedValues(), validatedConfigValue.errorMessages()); + if (configValue.errorMessages().size() > 0) { + errorCount++; + } + ConfigValueInfo configValueInfo = convertConfigValue(configValue, configKey != null ? configKey.type : null); + configInfoList.add(new ConfigInfo(configKeyInfo, configValueInfo)); } - ConfigValueInfo configValueInfo = convertConfigValue(configValue, configKey != null ? configKey.type : null); - configInfoList.add(new ConfigInfo(configKeyInfo, configValueInfo)); } return new ConfigInfos(connectorClass.toString(), errorCount, new ArrayList<>(groups), configInfoList); } diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Worker.java b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Worker.java index 7aa96b51ed409..69a74269af293 100644 --- a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Worker.java +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Worker.java @@ -21,8 +21,8 @@ import org.apache.kafka.clients.producer.KafkaProducer; import org.apache.kafka.clients.producer.ProducerConfig; import org.apache.kafka.common.MetricName; +import org.apache.kafka.common.config.ConfigValue; import org.apache.kafka.common.config.provider.ConfigProvider; -import org.apache.kafka.common.errors.PolicyViolationException; import org.apache.kafka.common.metrics.Sensor; import org.apache.kafka.common.metrics.stats.Frequencies; import org.apache.kafka.common.metrics.stats.Total; @@ -72,6 +72,7 @@ import java.util.concurrent.ConcurrentMap; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.stream.Collectors; /** @@ -625,10 +626,11 @@ private static Map connectorClientConfigOverrides(ConnectorTaskI clientOverrides, clientType ); - try { - connectorClientConfigOverridePolicy.validate(connectorClientConfigRequest); - } catch (PolicyViolationException e) { - throw new ConnectException("Error applying client config overrides", e); + List configValues = connectorClientConfigOverridePolicy.validate(connectorClientConfigRequest); + List errorConfigs = configValues.stream(). + filter(configValue -> configValue.errorMessages().size() > 0).collect(Collectors.toList()); + if (errorConfigs.size() > 0) { + throw new ConnectException("Client Config Overrides not allowed " + errorConfigs); } return clientOverrides; } diff --git a/connect/runtime/src/test/java/org/apache/kafka/connect/connector/policy/BaseConnectorClientConfigOverridePolicyTest.java b/connect/runtime/src/test/java/org/apache/kafka/connect/connector/policy/BaseConnectorClientConfigOverridePolicyTest.java new file mode 100644 index 0000000000000..28fee73a93966 --- /dev/null +++ b/connect/runtime/src/test/java/org/apache/kafka/connect/connector/policy/BaseConnectorClientConfigOverridePolicyTest.java @@ -0,0 +1,59 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.kafka.connect.connector.policy; + +import org.apache.kafka.common.config.ConfigValue; +import org.apache.kafka.connect.health.ConnectorType; +import org.apache.kafka.connect.runtime.WorkerTest; +import org.junit.Assert; + +import java.util.List; +import java.util.Map; + +public abstract class BaseConnectorClientConfigOverridePolicyTest { + + protected abstract ConnectorClientConfigOverridePolicy policyToTest(); + + protected void testValidOverride(Map clientConfig) { + List configValues = configValues(clientConfig); + assertNoError(configValues); + } + + protected void testInvalidOverride(Map clientConfig) { + List configValues = configValues(clientConfig); + assertError(configValues); + } + + private List configValues(Map clientConfig) { + ConnectorClientConfigRequest connectorClientConfigRequest = new ConnectorClientConfigRequest( + "test", + ConnectorType.SOURCE, + WorkerTest.WorkerTestConnector.class, + clientConfig, + ConnectorClientConfigRequest.ClientType.PRODUCER); + return policyToTest().validate(connectorClientConfigRequest); + } + + protected void assertNoError(List configValues) { + Assert.assertTrue(configValues.stream().allMatch(configValue -> configValue.errorMessages().size() == 0)); + } + + protected void assertError(List configValues) { + Assert.assertTrue(configValues.stream().anyMatch(configValue -> configValue.errorMessages().size() > 0)); + } +} diff --git a/connect/runtime/src/test/java/org/apache/kafka/connect/connector/policy/NoneConnectorClientConfigOverridePolicyTest.java b/connect/runtime/src/test/java/org/apache/kafka/connect/connector/policy/NoneConnectorClientConfigOverridePolicyTest.java index 6eadf76f1b7ff..2c7b0789d8022 100644 --- a/connect/runtime/src/test/java/org/apache/kafka/connect/connector/policy/NoneConnectorClientConfigOverridePolicyTest.java +++ b/connect/runtime/src/test/java/org/apache/kafka/connect/connector/policy/NoneConnectorClientConfigOverridePolicyTest.java @@ -19,42 +19,31 @@ import org.apache.kafka.clients.producer.ProducerConfig; import org.apache.kafka.common.config.SaslConfigs; -import org.apache.kafka.common.errors.PolicyViolationException; -import org.apache.kafka.connect.health.ConnectorType; -import org.apache.kafka.connect.runtime.WorkerTest; import org.junit.Test; import java.util.Collections; import java.util.HashMap; import java.util.Map; -public class NoneConnectorClientConfigOverridePolicyTest { +public class NoneConnectorClientConfigOverridePolicyTest extends BaseConnectorClientConfigOverridePolicyTest { ConnectorClientConfigOverridePolicy noneConnectorClientConfigOverridePolicy = new NoneConnectorClientConfigOverridePolicy(); @Test public void testNoOverrides() { - Map clientConfig = Collections.emptyMap(); - ConnectorClientConfigRequest connectorClientConfigRequest = new ConnectorClientConfigRequest( - "test", - ConnectorType.SOURCE, - WorkerTest.WorkerTestConnector.class, - clientConfig, - ConnectorClientConfigRequest.ClientType.PRODUCER); - noneConnectorClientConfigOverridePolicy.validate(connectorClientConfigRequest); + testValidOverride(Collections.emptyMap()); } - @Test(expected = PolicyViolationException.class) + @Test public void testWithOverrides() { Map clientConfig = new HashMap<>(); clientConfig.put(SaslConfigs.SASL_JAAS_CONFIG, "test"); clientConfig.put(ProducerConfig.ACKS_CONFIG, "none"); - ConnectorClientConfigRequest connectorClientConfigRequest = new ConnectorClientConfigRequest( - "test", - ConnectorType.SOURCE, - WorkerTest.WorkerTestConnector.class, - clientConfig, - ConnectorClientConfigRequest.ClientType.PRODUCER); - noneConnectorClientConfigOverridePolicy.validate(connectorClientConfigRequest); + testInvalidOverride(clientConfig); + } + + @Override + protected ConnectorClientConfigOverridePolicy policyToTest() { + return noneConnectorClientConfigOverridePolicy; } } diff --git a/connect/runtime/src/test/java/org/apache/kafka/connect/connector/policy/PrincipalConnectorClientConfigOverridePolicyTest.java b/connect/runtime/src/test/java/org/apache/kafka/connect/connector/policy/PrincipalConnectorClientConfigOverridePolicyTest.java index f251a8e3a6747..0e79c8a6ca990 100644 --- a/connect/runtime/src/test/java/org/apache/kafka/connect/connector/policy/PrincipalConnectorClientConfigOverridePolicyTest.java +++ b/connect/runtime/src/test/java/org/apache/kafka/connect/connector/policy/PrincipalConnectorClientConfigOverridePolicyTest.java @@ -19,42 +19,32 @@ import org.apache.kafka.clients.producer.ProducerConfig; import org.apache.kafka.common.config.SaslConfigs; -import org.apache.kafka.common.errors.PolicyViolationException; -import org.apache.kafka.connect.health.ConnectorType; -import org.apache.kafka.connect.runtime.WorkerTest; import org.junit.Test; import java.util.Collections; import java.util.HashMap; import java.util.Map; -public class PrincipalConnectorClientConfigOverridePolicyTest { +public class PrincipalConnectorClientConfigOverridePolicyTest extends BaseConnectorClientConfigOverridePolicyTest { ConnectorClientConfigOverridePolicy principalConnectorClientConfigOverridePolicy = new PrincipalConnectorClientConfigOverridePolicy(); @Test public void testPrincipalOnly() { Map clientConfig = Collections.singletonMap(SaslConfigs.SASL_JAAS_CONFIG, "test"); - ConnectorClientConfigRequest connectorClientConfigRequest = new ConnectorClientConfigRequest( - "test", - ConnectorType.SOURCE, - WorkerTest.WorkerTestConnector.class, - clientConfig, - ConnectorClientConfigRequest.ClientType.PRODUCER); - principalConnectorClientConfigOverridePolicy.validate(connectorClientConfigRequest); + testValidOverride(clientConfig); } - @Test(expected = PolicyViolationException.class) + @Test public void testPrincipalPlusOtherConfigs() { Map clientConfig = new HashMap<>(); clientConfig.put(SaslConfigs.SASL_JAAS_CONFIG, "test"); clientConfig.put(ProducerConfig.ACKS_CONFIG, "none"); - ConnectorClientConfigRequest connectorClientConfigRequest = new ConnectorClientConfigRequest( - "test", - ConnectorType.SOURCE, - WorkerTest.WorkerTestConnector.class, - clientConfig, - ConnectorClientConfigRequest.ClientType.PRODUCER); - principalConnectorClientConfigOverridePolicy.validate(connectorClientConfigRequest); + testInvalidOverride(clientConfig); + } + + @Override + protected ConnectorClientConfigOverridePolicy policyToTest() { + return principalConnectorClientConfigOverridePolicy; } } diff --git a/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/AbstractHerderTest.java b/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/AbstractHerderTest.java index ef7ec14039f8e..35c0dd2e131e1 100644 --- a/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/AbstractHerderTest.java +++ b/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/AbstractHerderTest.java @@ -23,6 +23,7 @@ import org.apache.kafka.connect.connector.ConnectRecord; import org.apache.kafka.connect.connector.Connector; import org.apache.kafka.connect.connector.policy.ConnectorClientConfigOverridePolicy; +import org.apache.kafka.connect.connector.policy.NoneConnectorClientConfigOverridePolicy; import org.apache.kafka.connect.connector.policy.PrincipalConnectorClientConfigOverridePolicy; import org.apache.kafka.connect.runtime.distributed.ClusterConfigState; import org.apache.kafka.connect.runtime.isolation.PluginDesc; @@ -120,7 +121,7 @@ public class AbstractHerderTest { private final String kafkaClusterId = "I4ZmrWqfT2e-upky_4fdPA"; private final int generation = 5; private final String connector = "connector"; - private final ConnectorClientConfigOverridePolicy ignoreConnectorClientConfigOverridePolicy = null; + private final ConnectorClientConfigOverridePolicy noneConnectorClientConfigOverridePolicy = new NoneConnectorClientConfigOverridePolicy(); @MockStrict private Worker worker; @MockStrict private WorkerConfigTransformer transformer; @@ -137,9 +138,10 @@ public void testConnectors() { String.class, String.class, StatusBackingStore.class, - ConfigBackingStore.class + ConfigBackingStore.class, + ConnectorClientConfigOverridePolicy.class ) - .withArgs(worker, workerId, kafkaClusterId, statusStore, configStore) + .withArgs(worker, workerId, kafkaClusterId, statusStore, configStore, noneConnectorClientConfigOverridePolicy) .addMockedMethod("generation") .createMock(); @@ -160,9 +162,10 @@ public void testConnectorStatus() { String.class, String.class, StatusBackingStore.class, - ConfigBackingStore.class + ConfigBackingStore.class, + ConnectorClientConfigOverridePolicy.class ) - .withArgs(worker, workerId, kafkaClusterId, statusStore, configStore) + .withArgs(worker, workerId, kafkaClusterId, statusStore, configStore, noneConnectorClientConfigOverridePolicy) .addMockedMethod("generation") .createMock(); @@ -186,7 +189,7 @@ public void connectorStatus() { AbstractHerder herder = partialMockBuilder(AbstractHerder.class) .withConstructor(Worker.class, String.class, String.class, StatusBackingStore.class, ConfigBackingStore.class, ConnectorClientConfigOverridePolicy.class) - .withArgs(worker, workerId, kafkaClusterId, statusStore, configStore, ignoreConnectorClientConfigOverridePolicy) + .withArgs(worker, workerId, kafkaClusterId, statusStore, configStore, noneConnectorClientConfigOverridePolicy) .addMockedMethod("generation") .createMock(); @@ -227,7 +230,7 @@ public void taskStatus() { AbstractHerder herder = partialMockBuilder(AbstractHerder.class) .withConstructor(Worker.class, String.class, String.class, StatusBackingStore.class, ConfigBackingStore.class, ConnectorClientConfigOverridePolicy.class) - .withArgs(worker, workerId, kafkaClusterId, statusStore, configStore, ignoreConnectorClientConfigOverridePolicy) + .withArgs(worker, workerId, kafkaClusterId, statusStore, configStore, noneConnectorClientConfigOverridePolicy) .addMockedMethod("generation") .createMock(); @@ -260,7 +263,7 @@ public TaskStatus answer() throws Throwable { @Test(expected = BadRequestException.class) public void testConfigValidationEmptyConfig() { - AbstractHerder herder = createConfigValidationHerder(TestSourceConnector.class, ignoreConnectorClientConfigOverridePolicy); + AbstractHerder herder = createConfigValidationHerder(TestSourceConnector.class, noneConnectorClientConfigOverridePolicy); replayAll(); herder.validateConnectorConfig(new HashMap()); @@ -270,7 +273,7 @@ public void testConfigValidationEmptyConfig() { @Test() public void testConfigValidationMissingName() { - AbstractHerder herder = createConfigValidationHerder(TestSourceConnector.class, ignoreConnectorClientConfigOverridePolicy); + AbstractHerder herder = createConfigValidationHerder(TestSourceConnector.class, noneConnectorClientConfigOverridePolicy); replayAll(); Map config = Collections.singletonMap(ConnectorConfig.CONNECTOR_CLASS_CONFIG, TestSourceConnector.class.getName()); @@ -295,7 +298,7 @@ public void testConfigValidationMissingName() { @Test(expected = ConfigException.class) public void testConfigValidationInvalidTopics() { - AbstractHerder herder = createConfigValidationHerder(TestSinkConnector.class, ignoreConnectorClientConfigOverridePolicy); + AbstractHerder herder = createConfigValidationHerder(TestSinkConnector.class, noneConnectorClientConfigOverridePolicy); replayAll(); Map config = new HashMap<>(); @@ -310,7 +313,7 @@ public void testConfigValidationInvalidTopics() { @Test() public void testConfigValidationTransformsExtendResults() { - AbstractHerder herder = createConfigValidationHerder(TestSourceConnector.class, ignoreConnectorClientConfigOverridePolicy); + AbstractHerder herder = createConfigValidationHerder(TestSourceConnector.class, noneConnectorClientConfigOverridePolicy); // 2 transform aliases defined -> 2 plugin lookups Set> transformations = new HashSet<>(); diff --git a/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/distributed/DistributedHerderTest.java b/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/distributed/DistributedHerderTest.java index c5035dc9b1fb2..b03ddf341ed2a 100644 --- a/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/distributed/DistributedHerderTest.java +++ b/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/distributed/DistributedHerderTest.java @@ -23,6 +23,8 @@ import org.apache.kafka.common.utils.MockTime; import org.apache.kafka.connect.connector.Connector; import org.apache.kafka.connect.connector.ConnectorContext; +import org.apache.kafka.connect.connector.policy.ConnectorClientConfigOverridePolicy; +import org.apache.kafka.connect.connector.policy.NoneConnectorClientConfigOverridePolicy; import org.apache.kafka.connect.errors.AlreadyExistsException; import org.apache.kafka.connect.errors.NotFoundException; import org.apache.kafka.connect.runtime.ConnectMetrics.MetricGroup; @@ -177,6 +179,8 @@ public class DistributedHerderTest { private SinkConnectorConfig conn1SinkConfig; private SinkConnectorConfig conn1SinkConfigUpdated; private short connectProtocolVersion; + private final ConnectorClientConfigOverridePolicy + noneConnectorClientConfigOverridePolicy = new NoneConnectorClientConfigOverridePolicy(); @Before public void setUp() throws Exception { @@ -191,7 +195,7 @@ public void setUp() throws Exception { herder = PowerMock.createPartialMock(DistributedHerder.class, new String[]{"backoff", "connectorTypeForClass", "updateDeletedConnectorStatus"}, new DistributedConfig(HERDER_CONFIG), worker, WORKER_ID, KAFKA_CLUSTER_ID, - statusBackingStore, configBackingStore, member, MEMBER_URL, metrics, time, null); + statusBackingStore, configBackingStore, member, MEMBER_URL, metrics, time, noneConnectorClientConfigOverridePolicy); configUpdateListener = herder.new ConfigUpdateListener(); rebalanceListener = herder.new RebalanceListener(time); diff --git a/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java b/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java index 7f77430566bee..e020cee3016fd 100644 --- a/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java +++ b/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java @@ -22,6 +22,8 @@ import org.apache.kafka.connect.connector.Connector; import org.apache.kafka.connect.connector.ConnectorContext; import org.apache.kafka.connect.connector.Task; +import org.apache.kafka.connect.connector.policy.ConnectorClientConfigOverridePolicy; +import org.apache.kafka.connect.connector.policy.NoneConnectorClientConfigOverridePolicy; import org.apache.kafka.connect.errors.AlreadyExistsException; import org.apache.kafka.connect.errors.ConnectException; import org.apache.kafka.connect.errors.NotFoundException; @@ -111,11 +113,15 @@ private enum SourceSink { @Mock protected Callback> createCallback; @Mock protected StatusBackingStore statusBackingStore; + private final ConnectorClientConfigOverridePolicy + noneConnectorClientConfigOverridePolicy = new NoneConnectorClientConfigOverridePolicy(); + + @Before public void setup() { worker = PowerMock.createMock(Worker.class); herder = PowerMock.createPartialMock(StandaloneHerder.class, new String[]{"connectorTypeForClass"}, - worker, WORKER_ID, KAFKA_CLUSTER_ID, statusBackingStore, new MemoryConfigBackingStore(transformer), null); + worker, WORKER_ID, KAFKA_CLUSTER_ID, statusBackingStore, new MemoryConfigBackingStore(transformer), noneConnectorClientConfigOverridePolicy); plugins = PowerMock.createMock(Plugins.class); pluginLoader = PowerMock.createMock(PluginClassLoader.class); delegatingLoader = PowerMock.createMock(DelegatingClassLoader.class); diff --git a/connect/runtime/src/test/java/org/apache/kafka/connect/util/clusters/EmbeddedConnectCluster.java b/connect/runtime/src/test/java/org/apache/kafka/connect/util/clusters/EmbeddedConnectCluster.java index e610812040658..beba027f4b0e8 100644 --- a/connect/runtime/src/test/java/org/apache/kafka/connect/util/clusters/EmbeddedConnectCluster.java +++ b/connect/runtime/src/test/java/org/apache/kafka/connect/util/clusters/EmbeddedConnectCluster.java @@ -362,13 +362,14 @@ public int executePut(String url, String body) throws IOException { try (OutputStreamWriter out = new OutputStreamWriter(httpCon.getOutputStream())) { out.write(body); } - try (InputStream is = httpCon.getInputStream()) { - int c; - StringBuilder response = new StringBuilder(); - while ((c = is.read()) != -1) { - response.append((char) c); + if (httpCon.getResponseCode() < HttpURLConnection.HTTP_BAD_REQUEST) { + try (InputStream is = httpCon.getInputStream()) { + log.info("Put response for URL={} is {}", url, responseToString(is)); + } + } else { + try (InputStream is = httpCon.getErrorStream()) { + log.info("Put error response for URL={} is {}", url, responseToString(is)); } - log.info("Put response for URL={} is {}", url, response); } return httpCon.getResponseCode(); } @@ -413,6 +414,15 @@ public int executeDelete(String url) throws IOException { return httpCon.getResponseCode(); } + private String responseToString(InputStream stream) throws IOException { + int c; + StringBuilder response = new StringBuilder(); + while ((c = stream.read()) != -1) { + response.append((char) c); + } + return response.toString(); + } + public static class Builder { private String name = UUID.randomUUID().toString(); private Map workerProps = new HashMap<>(); From 4dbe3fc29dd6468ea26dd73d517ddd0fa39abb94 Mon Sep 17 00:00:00 2001 From: Magesh Nandakumar Date: Thu, 16 May 2019 01:22:34 -0700 Subject: [PATCH 4/8] Fix rebase issue --- .../java/org/apache/kafka/connect/runtime/Worker.java | 9 ++++++--- .../org/apache/kafka/connect/runtime/WorkerTest.java | 8 ++++---- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Worker.java b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Worker.java index 69a74269af293..76da0e5070ca5 100644 --- a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Worker.java +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Worker.java @@ -509,7 +509,8 @@ private WorkerTask buildWorkerTask(ClusterConfigState configState, internalKeyConverter, internalValueConverter); OffsetStorageWriter offsetWriter = new OffsetStorageWriter(offsetBackingStore, id.connector(), internalKeyConverter, internalValueConverter); - Map producerProps = producerConfigs("connector-producer-" + id, config, connConfig, connectorClass, connectorClientConfigOverridePolicy); + Map producerProps = producerConfigs(id, "connector-producer-" + id, config, connConfig, connectorClass, + connectorClientConfigOverridePolicy); KafkaProducer producer = new KafkaProducer<>(producerProps); // Note we pass the configState as it performs dynamic transformations under the covers @@ -534,7 +535,8 @@ private WorkerTask buildWorkerTask(ClusterConfigState configState, } } - static Map producerConfigs(String defaultClientId, + static Map producerConfigs(ConnectorTaskId id, + String defaultClientId, WorkerConfig config, ConnectorConfig connConfig, Class connectorClass, @@ -649,7 +651,8 @@ private List sinkTaskReporters(ConnectorTaskId id, SinkConnectorC // check if topic for dead letter queue exists String topic = connConfig.dlqTopicName(); if (topic != null && !topic.isEmpty()) { - Map producerProps = producerConfigs("connector-dlq-producer-" + id, config, connConfig, connectorClass, connectorClientConfigOverridePolicy); + Map producerProps = producerConfigs(id,"connector-dlq-producer-" + id, config, connConfig, connectorClass, + connectorClientConfigOverridePolicy); Map adminProps = adminConfigs(id, config, connConfig, connectorClass, connectorClientConfigOverridePolicy); DeadLetterQueueReporter reporter = DeadLetterQueueReporter.createAndSetup(adminProps, id, connConfig, producerProps, errorHandlingMetrics); reporters.add(reporter); diff --git a/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/WorkerTest.java b/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/WorkerTest.java index f369428fb7fe3..2fa5835a13831 100644 --- a/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/WorkerTest.java +++ b/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/WorkerTest.java @@ -846,7 +846,7 @@ public void testProducerConfigsWithoutOverrides() { PowerMock.replayAll(); Map expectedConfigs = new HashMap<>(defaultProducerConfigs); expectedConfigs.put("client.id", "connector-producer-job-0"); - assertEquals(defaultProducerConfigs, Worker.producerConfigs("connector-producer-" + TASK_ID, config, connectorConfig, + assertEquals(expectedConfigs, Worker.producerConfigs(TASK_ID,"connector-producer-" + TASK_ID, config, connectorConfig, null, noneConnectorClientConfigOverridePolicy)); } @@ -865,7 +865,7 @@ public void testProducerConfigsWithOverrides() { EasyMock.expect(connectorConfig.originalsWithPrefix(ConnectorConfig.CONNECTOR_CLIENT_PRODUCER_OVERRIDES_PREFIX)).andReturn( new HashMap()); PowerMock.replayAll(); - assertEquals(expectedConfigs, Worker.producerConfigs(new ConnectorTaskId("test", 1), configWithOverrides, connectorConfig, + assertEquals(expectedConfigs, Worker.producerConfigs(TASK_ID,"connector-producer-" + TASK_ID, configWithOverrides, connectorConfig, null, allConnectorClientConfigOverridePolicy)); } @@ -888,7 +888,7 @@ public void testProducerConfigsWithClientOverrides() { EasyMock.expect(connectorConfig.originalsWithPrefix(ConnectorConfig.CONNECTOR_CLIENT_PRODUCER_OVERRIDES_PREFIX)) .andReturn(connConfig); PowerMock.replayAll(); - assertEquals(expectedConfigs, Worker.producerConfigs(new ConnectorTaskId("test", 1), configWithOverrides, connectorConfig, + assertEquals(expectedConfigs, Worker.producerConfigs(TASK_ID,"connector-producer-" + TASK_ID, configWithOverrides, connectorConfig, null, allConnectorClientConfigOverridePolicy)); } @@ -937,7 +937,7 @@ public void testConsumerConfigsWithClientOverrides() { expectedConfigs.put("auto.offset.reset", "latest"); expectedConfigs.put("max.poll.records", "5000"); expectedConfigs.put("max.poll.interval.ms", "1000"); - + expectedConfigs.put("client.id", "connector-consumer-test-1"); Map connConfig = new HashMap(); connConfig.put("max.poll.records", "5000"); connConfig.put("max.poll.interval.ms", "1000"); From c59db5a1c61654a95478aa725e15a97317a2c754 Mon Sep 17 00:00:00 2001 From: Magesh Nandakumar Date: Thu, 16 May 2019 10:18:53 -0700 Subject: [PATCH 5/8] Fix checkstyle issue --- .../kafka/connect/runtime/WorkerTest.java | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/WorkerTest.java b/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/WorkerTest.java index 2fa5835a13831..28700b74fe6d9 100644 --- a/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/WorkerTest.java +++ b/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/WorkerTest.java @@ -846,8 +846,8 @@ public void testProducerConfigsWithoutOverrides() { PowerMock.replayAll(); Map expectedConfigs = new HashMap<>(defaultProducerConfigs); expectedConfigs.put("client.id", "connector-producer-job-0"); - assertEquals(expectedConfigs, Worker.producerConfigs(TASK_ID,"connector-producer-" + TASK_ID, config, connectorConfig, - null, noneConnectorClientConfigOverridePolicy)); + assertEquals(expectedConfigs, + Worker.producerConfigs(TASK_ID, "connector-producer-" + TASK_ID, config, connectorConfig, null, noneConnectorClientConfigOverridePolicy)); } @Test @@ -865,8 +865,8 @@ public void testProducerConfigsWithOverrides() { EasyMock.expect(connectorConfig.originalsWithPrefix(ConnectorConfig.CONNECTOR_CLIENT_PRODUCER_OVERRIDES_PREFIX)).andReturn( new HashMap()); PowerMock.replayAll(); - assertEquals(expectedConfigs, Worker.producerConfigs(TASK_ID,"connector-producer-" + TASK_ID, configWithOverrides, connectorConfig, - null, allConnectorClientConfigOverridePolicy)); + assertEquals(expectedConfigs, + Worker.producerConfigs(TASK_ID, "connector-producer-" + TASK_ID, configWithOverrides, connectorConfig, null, allConnectorClientConfigOverridePolicy)); } @Test @@ -888,8 +888,8 @@ public void testProducerConfigsWithClientOverrides() { EasyMock.expect(connectorConfig.originalsWithPrefix(ConnectorConfig.CONNECTOR_CLIENT_PRODUCER_OVERRIDES_PREFIX)) .andReturn(connConfig); PowerMock.replayAll(); - assertEquals(expectedConfigs, Worker.producerConfigs(TASK_ID,"connector-producer-" + TASK_ID, configWithOverrides, connectorConfig, - null, allConnectorClientConfigOverridePolicy)); + assertEquals(expectedConfigs, + Worker.producerConfigs(TASK_ID, "connector-producer-" + TASK_ID, configWithOverrides, connectorConfig, null, allConnectorClientConfigOverridePolicy)); } @Test @@ -897,8 +897,7 @@ public void testConsumerConfigsWithoutOverrides() { Map expectedConfigs = new HashMap<>(defaultConsumerConfigs); expectedConfigs.put("group.id", "connect-test"); expectedConfigs.put("client.id", "connector-consumer-test-1"); - EasyMock.expect(connectorConfig.originalsWithPrefix(ConnectorConfig.CONNECTOR_CLIENT_CONSUMER_OVERRIDES_PREFIX)).andReturn( - new HashMap()); + EasyMock.expect(connectorConfig.originalsWithPrefix(ConnectorConfig.CONNECTOR_CLIENT_CONSUMER_OVERRIDES_PREFIX)).andReturn(new HashMap<>()); PowerMock.replayAll(); assertEquals(expectedConfigs, Worker.consumerConfigs(new ConnectorTaskId("test", 1), config, connectorConfig, null, noneConnectorClientConfigOverridePolicy)); @@ -917,8 +916,7 @@ public void testConsumerConfigsWithOverrides() { expectedConfigs.put("auto.offset.reset", "latest"); expectedConfigs.put("max.poll.records", "1000"); expectedConfigs.put("client.id", "consumer-test-id"); - EasyMock.expect(connectorConfig.originalsWithPrefix(ConnectorConfig.CONNECTOR_CLIENT_CONSUMER_OVERRIDES_PREFIX)) - .andReturn(new HashMap()); + EasyMock.expect(connectorConfig.originalsWithPrefix(ConnectorConfig.CONNECTOR_CLIENT_CONSUMER_OVERRIDES_PREFIX)).andReturn(new HashMap<>()); PowerMock.replayAll(); assertEquals(expectedConfigs, Worker.consumerConfigs(new ConnectorTaskId("test", 1), configWithOverrides, connectorConfig, null, noneConnectorClientConfigOverridePolicy)); From ededad76468db5ae4e3194931a14c695fdba4aac Mon Sep 17 00:00:00 2001 From: Magesh Nandakumar Date: Thu, 16 May 2019 15:48:25 -0700 Subject: [PATCH 6/8] Fix review comments, add more unit tests and include integration test --- .../ConnectorClientConfigOverridePolicy.java | 2 +- ...ctConnectorClientConfigOverridePolicy.java | 57 +++++++ ...llConnectorClientConfigOverridePolicy.java | 13 +- ...neConnectorClientConfigOverridePolicy.java | 21 +-- ...alConnectorClientConfigOverridePolicy.java | 23 +-- .../kafka/connect/runtime/AbstractHerder.java | 2 +- .../apache/kafka/connect/runtime/Worker.java | 7 +- .../ConnectorCientPolicyIntegrationTest.java | 146 ++++++++++++++++++ .../kafka/connect/runtime/WorkerTest.java | 41 +++++ 9 files changed, 266 insertions(+), 46 deletions(-) create mode 100644 connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/AbstractConnectorClientConfigOverridePolicy.java create mode 100644 connect/runtime/src/test/java/org/apache/kafka/connect/integration/ConnectorCientPolicyIntegrationTest.java diff --git a/connect/api/src/main/java/org/apache/kafka/connect/connector/policy/ConnectorClientConfigOverridePolicy.java b/connect/api/src/main/java/org/apache/kafka/connect/connector/policy/ConnectorClientConfigOverridePolicy.java index 31eab1e1024ae..66691f725c078 100644 --- a/connect/api/src/main/java/org/apache/kafka/connect/connector/policy/ConnectorClientConfigOverridePolicy.java +++ b/connect/api/src/main/java/org/apache/kafka/connect/connector/policy/ConnectorClientConfigOverridePolicy.java @@ -40,7 +40,7 @@ public interface ConnectorClientConfigOverridePolicy extends Configurable, AutoC * * @param connectorClientConfigRequest an instance of {@code ConnectorClientConfigRequest} that provides the configs to overridden and * its context; never {@code null} - * @return List of Config, each Config should indicate if they are allowed via {@link ConfigValue#errorMessages} + * @return List of Config, each Config should indicate if they are allowed via {@link ConfigValue#errorMessages}; never null */ List validate(ConnectorClientConfigRequest connectorClientConfigRequest); } \ No newline at end of file diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/AbstractConnectorClientConfigOverridePolicy.java b/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/AbstractConnectorClientConfigOverridePolicy.java new file mode 100644 index 0000000000000..3c310db966c04 --- /dev/null +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/AbstractConnectorClientConfigOverridePolicy.java @@ -0,0 +1,57 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.kafka.connect.connector.policy; + +import org.apache.kafka.common.config.ConfigValue; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; + +public abstract class AbstractConnectorClientConfigOverridePolicy implements ConnectorClientConfigOverridePolicy { + + @Override + public void close() throws Exception { + + } + + @Override + public final List validate(ConnectorClientConfigRequest connectorClientConfigRequest) { + Map inputConfig = connectorClientConfigRequest.clientProps(); + return inputConfig.entrySet().stream().map(this::configValue).collect(Collectors.toList()); + } + + protected ConfigValue configValue(Map.Entry configEntry) { + ConfigValue configValue = + new ConfigValue(configEntry.getKey(), configEntry.getValue(), new ArrayList<>(), new ArrayList()); + validate(configValue); + return configValue; + } + + protected void validate(ConfigValue configValue) { + if (!isAllowed(configValue)) { + configValue.addErrorMessage("The '" + policyName() + "' policy does not allow '" + configValue.name() + + "' to be overridden in the connector configuration."); + } + } + + protected abstract String policyName(); + + protected abstract boolean isAllowed(ConfigValue configValue); +} diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/AllConnectorClientConfigOverridePolicy.java b/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/AllConnectorClientConfigOverridePolicy.java index d72da569e00d3..ecd76ba06c756 100644 --- a/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/AllConnectorClientConfigOverridePolicy.java +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/AllConnectorClientConfigOverridePolicy.java @@ -21,25 +21,22 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.util.Collections; -import java.util.List; import java.util.Map; /** * Allows all client configurations to be overridden via the connector configs by setting {@code client.config.policy} to {@code All} */ -public class AllConnectorClientConfigOverridePolicy implements ConnectorClientConfigOverridePolicy { +public class AllConnectorClientConfigOverridePolicy extends AbstractConnectorClientConfigOverridePolicy { private static final Logger log = LoggerFactory.getLogger(AllConnectorClientConfigOverridePolicy.class); @Override - public List validate(ConnectorClientConfigRequest connectorClientConfigRequest) { - //allow all no op - return Collections.emptyList(); + protected String policyName() { + return "All"; } @Override - public void close() { - + protected boolean isAllowed(ConfigValue configValue) { + return true; } @Override diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/NoneConnectorClientConfigOverridePolicy.java b/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/NoneConnectorClientConfigOverridePolicy.java index 2954a5ba88965..8236c89acdafd 100644 --- a/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/NoneConnectorClientConfigOverridePolicy.java +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/NoneConnectorClientConfigOverridePolicy.java @@ -21,34 +21,23 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.util.ArrayList; -import java.util.List; import java.util.Map; -import java.util.stream.Collectors; /** * Disallow any client configuration to be overridden via the connector configs by setting {@code client.config.policy} to {@code None}. * This is the default behavior. */ -public class NoneConnectorClientConfigOverridePolicy implements ConnectorClientConfigOverridePolicy { +public class NoneConnectorClientConfigOverridePolicy extends AbstractConnectorClientConfigOverridePolicy { private static final Logger log = LoggerFactory.getLogger(NoneConnectorClientConfigOverridePolicy.class); @Override - public List validate(ConnectorClientConfigRequest connectorClientConfigRequest) { - Map inputConfig = connectorClientConfigRequest.clientProps(); - return inputConfig.entrySet().stream().map(configEntry -> configValue(configEntry)).collect(Collectors.toList()); - } - - private static ConfigValue configValue(Map.Entry configEntry) { - ConfigValue configValue = - new ConfigValue(configEntry.getKey(), configEntry.getValue(), new ArrayList(), new ArrayList()); - configValue.addErrorMessage("None policy doesn't allow any client overrides"); - return configValue; + protected String policyName() { + return "None"; } @Override - public void close() { - + protected boolean isAllowed(ConfigValue configValue) { + return false; } @Override diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/PrincipalConnectorClientConfigOverridePolicy.java b/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/PrincipalConnectorClientConfigOverridePolicy.java index e39164a5c59ef..7d80d80ef1c87 100644 --- a/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/PrincipalConnectorClientConfigOverridePolicy.java +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/connector/policy/PrincipalConnectorClientConfigOverridePolicy.java @@ -23,8 +23,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.util.ArrayList; -import java.util.List; import java.util.Map; import java.util.Set; import java.util.stream.Collectors; @@ -34,32 +32,21 @@ * Allows all {@code sasl} configurations to be overridden via the connector configs by setting {@code client.config.policy} to * {@code Principal}. This allows to set a principal per connector. */ -public class PrincipalConnectorClientConfigOverridePolicy implements ConnectorClientConfigOverridePolicy { +public class PrincipalConnectorClientConfigOverridePolicy extends AbstractConnectorClientConfigOverridePolicy { private static final Logger log = LoggerFactory.getLogger(PrincipalConnectorClientConfigOverridePolicy.class); private static final Set ALLOWED_CONFIG = Stream.of(SaslConfigs.SASL_JAAS_CONFIG, SaslConfigs.SASL_MECHANISM, CommonClientConfigs.SECURITY_PROTOCOL_CONFIG). collect(Collectors.toSet()); - @Override - public List validate(ConnectorClientConfigRequest connectorClientConfigRequest) { - Map inputConfig = connectorClientConfigRequest.clientProps(); - return inputConfig.entrySet().stream().map(configEntry -> configValue(configEntry)).collect(Collectors.toList()); - } - - private static ConfigValue configValue(Map.Entry configEntry) { - ConfigValue configValue = - new ConfigValue(configEntry.getKey(), configEntry.getValue(), new ArrayList(), new ArrayList()); - if (!ALLOWED_CONFIG.contains(configEntry.getKey())) { - configValue.addErrorMessage("Principal policy allows only " + ALLOWED_CONFIG + "to be overriden"); - } - return configValue; + protected String policyName() { + return "Principal"; } @Override - public void close() { - + protected boolean isAllowed(ConfigValue configValue) { + return ALLOWED_CONFIG.contains(configValue.name()); } @Override diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractHerder.java b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractHerder.java index 33fb2f8afcf25..e92f55e758a27 100644 --- a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractHerder.java +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractHerder.java @@ -402,7 +402,7 @@ private static ConfigInfos validateClientOverrides(String connName, int errorCount = 0; List configInfoList = new LinkedList<>(); Map configKeys = configDef.configKeys(); - Set groups = new HashSet<>(); + Set groups = new LinkedHashSet<>(); Map clientConfigs = connectorConfig.originalsWithPrefix(prefix); ConnectorClientConfigRequest connectorClientConfigRequest = new ConnectorClientConfigRequest( connName, connectorType, connectorClass, clientConfigs, clientType); diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Worker.java b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Worker.java index 76da0e5070ca5..f848a18f06f9e 100644 --- a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Worker.java +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Worker.java @@ -556,6 +556,7 @@ static Map producerConfigs(ConnectorTaskId id, // User-specified overrides producerProps.putAll(config.originalsWithPrefix("producer.")); + // Connector-specified overrides Map producerOverrides = connectorClientConfigOverrides(id, connConfig, connectorClass, ConnectorConfig.CONNECTOR_CLIENT_PRODUCER_OVERRIDES_PREFIX, ConnectorType.SOURCE, ConnectorClientConfigRequest.ClientType.PRODUCER, @@ -584,7 +585,7 @@ static Map consumerConfigs(ConnectorTaskId id, consumerProps.put(ConsumerConfig.VALUE_DESERIALIZER_CLASS_CONFIG, "org.apache.kafka.common.serialization.ByteArrayDeserializer"); consumerProps.putAll(config.originalsWithPrefix("consumer.")); - + // Connector-specified overrides Map consumerOverrides = connectorClientConfigOverrides(id, connConfig, connectorClass, ConnectorConfig.CONNECTOR_CLIENT_CONSUMER_OVERRIDES_PREFIX, ConnectorType.SINK, ConnectorClientConfigRequest.ClientType.CONSUMER, @@ -604,6 +605,7 @@ static Map adminConfigs(ConnectorTaskId id, // User-specified overrides adminProps.putAll(config.originalsWithPrefix("admin.")); + // Connector-specified overrides Map adminOverrides = connectorClientConfigOverrides(id, connConfig, connectorClass, ConnectorConfig.CONNECTOR_CLIENT_ADMIN_OVERRIDES_PREFIX, ConnectorType.SINK, ConnectorClientConfigRequest.ClientType.ADMIN, @@ -631,6 +633,7 @@ private static Map connectorClientConfigOverrides(ConnectorTaskI List configValues = connectorClientConfigOverridePolicy.validate(connectorClientConfigRequest); List errorConfigs = configValues.stream(). filter(configValue -> configValue.errorMessages().size() > 0).collect(Collectors.toList()); + // These should be caught when the herder validates the connector configuration, but just in case if (errorConfigs.size() > 0) { throw new ConnectException("Client Config Overrides not allowed " + errorConfigs); } @@ -651,7 +654,7 @@ private List sinkTaskReporters(ConnectorTaskId id, SinkConnectorC // check if topic for dead letter queue exists String topic = connConfig.dlqTopicName(); if (topic != null && !topic.isEmpty()) { - Map producerProps = producerConfigs(id,"connector-dlq-producer-" + id, config, connConfig, connectorClass, + Map producerProps = producerConfigs(id, "connector-dlq-producer-" + id, config, connConfig, connectorClass, connectorClientConfigOverridePolicy); Map adminProps = adminConfigs(id, config, connConfig, connectorClass, connectorClientConfigOverridePolicy); DeadLetterQueueReporter reporter = DeadLetterQueueReporter.createAndSetup(adminProps, id, connConfig, producerProps, errorHandlingMetrics); diff --git a/connect/runtime/src/test/java/org/apache/kafka/connect/integration/ConnectorCientPolicyIntegrationTest.java b/connect/runtime/src/test/java/org/apache/kafka/connect/integration/ConnectorCientPolicyIntegrationTest.java new file mode 100644 index 0000000000000..499916bf69299 --- /dev/null +++ b/connect/runtime/src/test/java/org/apache/kafka/connect/integration/ConnectorCientPolicyIntegrationTest.java @@ -0,0 +1,146 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.kafka.connect.integration; + +import org.apache.kafka.clients.CommonClientConfigs; +import org.apache.kafka.clients.consumer.ConsumerConfig; +import org.apache.kafka.common.config.SaslConfigs; +import org.apache.kafka.connect.runtime.ConnectorConfig; +import org.apache.kafka.connect.runtime.WorkerConfig; +import org.apache.kafka.connect.runtime.rest.errors.ConnectRestException; +import org.apache.kafka.connect.storage.StringConverter; +import org.apache.kafka.connect.util.clusters.EmbeddedConnectCluster; +import org.apache.kafka.test.IntegrationTest; +import org.junit.After; +import org.junit.Test; +import org.junit.experimental.categories.Category; + +import java.io.IOException; +import java.util.HashMap; +import java.util.Map; +import java.util.Properties; + +import static org.apache.kafka.connect.runtime.ConnectorConfig.CONNECTOR_CLASS_CONFIG; +import static org.apache.kafka.connect.runtime.ConnectorConfig.KEY_CONVERTER_CLASS_CONFIG; +import static org.apache.kafka.connect.runtime.ConnectorConfig.TASKS_MAX_CONFIG; +import static org.apache.kafka.connect.runtime.ConnectorConfig.VALUE_CONVERTER_CLASS_CONFIG; +import static org.apache.kafka.connect.runtime.SinkConnectorConfig.TOPICS_CONFIG; +import static org.apache.kafka.connect.runtime.WorkerConfig.OFFSET_COMMIT_INTERVAL_MS_CONFIG; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; + +@Category(IntegrationTest.class) +public class ConnectorCientPolicyIntegrationTest { + + private static final int NUM_TASKS = 1; + private static final int NUM_WORKERS = 1; + private static final String CONNECTOR_NAME = "simple-conn"; + + + @After + public void close() { + } + + @Test + public void testCreateWithOverridesForNonePolicy() throws Exception { + Map props = basicConnectorConfig(); + props.put(ConnectorConfig.CONNECTOR_CLIENT_CONSUMER_OVERRIDES_PREFIX + SaslConfigs.SASL_JAAS_CONFIG, "sasl"); + assertFailCreateConnector("None", props); + } + + @Test + public void testCreateWithNotAllowedOverridesForPrincipalPolicy() throws Exception { + Map props = basicConnectorConfig(); + props.put(ConnectorConfig.CONNECTOR_CLIENT_CONSUMER_OVERRIDES_PREFIX + SaslConfigs.SASL_JAAS_CONFIG, "sasl"); + props.put(ConnectorConfig.CONNECTOR_CLIENT_CONSUMER_OVERRIDES_PREFIX + ConsumerConfig.AUTO_OFFSET_RESET_CONFIG, "latest"); + assertFailCreateConnector("Principal", props); + } + + @Test + public void testCreateWithAllowedOverridesForPrincipalPolicy() throws Exception { + Map props = basicConnectorConfig(); + props.put(ConnectorConfig.CONNECTOR_CLIENT_CONSUMER_OVERRIDES_PREFIX + CommonClientConfigs.SECURITY_PROTOCOL_CONFIG, "PLAIN"); + assertPassCreateConnector("Principal", props); + } + + @Test + public void testCreateWithAllowedOverridesForAllPolicy() throws Exception { + // setup up props for the sink connector + Map props = basicConnectorConfig(); + props.put(ConnectorConfig.CONNECTOR_CLIENT_CONSUMER_OVERRIDES_PREFIX + CommonClientConfigs.CLIENT_ID_CONFIG, "test"); + assertPassCreateConnector("All", props); + } + + private EmbeddedConnectCluster connectClusterWithPolicy(String policy) throws IOException { + // setup Connect worker properties + Map workerProps = new HashMap<>(); + workerProps.put(OFFSET_COMMIT_INTERVAL_MS_CONFIG, String.valueOf(5_000)); + workerProps.put(WorkerConfig.CONNECTOR_CLIENT_POLICY_CLASS_CONFIG, policy); + + // setup Kafka broker properties + Properties exampleBrokerProps = new Properties(); + exampleBrokerProps.put("auto.create.topics.enable", "false"); + + // build a Connect cluster backed by Kafka and Zk + EmbeddedConnectCluster connect = new EmbeddedConnectCluster.Builder() + .name("connect-cluster") + .numWorkers(NUM_WORKERS) + .numBrokers(1) + .workerProps(workerProps) + .brokerProps(exampleBrokerProps) + .build(); + + // start the clusters + connect.start(); + return connect; + } + + private void assertFailCreateConnector(String policy, Map props) throws IOException { + EmbeddedConnectCluster connect = connectClusterWithPolicy(policy); + try { + connect.configureConnector(CONNECTOR_NAME, props); + fail("Shouldn't be able to create connector"); + } catch (ConnectRestException e) { + assertEquals(e.statusCode(), 400); + } finally { + connect.stop(); + } + } + + private void assertPassCreateConnector(String policy, Map props) throws IOException { + EmbeddedConnectCluster connect = connectClusterWithPolicy(policy); + try { + connect.configureConnector(CONNECTOR_NAME, props); + } catch (ConnectRestException e) { + fail("Should be able to create connector"); + } finally { + connect.stop(); + } + } + + + public Map basicConnectorConfig() { + Map props = new HashMap<>(); + props.put(CONNECTOR_CLASS_CONFIG, MonitorableSinkConnector.class.getSimpleName()); + props.put(TASKS_MAX_CONFIG, String.valueOf(NUM_TASKS)); + props.put(TOPICS_CONFIG, "test-topic"); + props.put(KEY_CONVERTER_CLASS_CONFIG, StringConverter.class.getName()); + props.put(VALUE_CONVERTER_CLASS_CONFIG, StringConverter.class.getName()); + return props; + } + +} diff --git a/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/WorkerTest.java b/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/WorkerTest.java index 28700b74fe6d9..9cb83eb5e8771 100644 --- a/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/WorkerTest.java +++ b/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/WorkerTest.java @@ -963,6 +963,47 @@ public void testConsumerConfigsClientOverridesWithNonePolicy() { null, noneConnectorClientConfigOverridePolicy); } + @Test + public void testAdminConfigsClientOverridesWithAllPolicy() { + Map props = new HashMap<>(workerProps); + props.put("admin.client.id", "testid"); + props.put("admin.metadata.max.age.ms", "5000"); + WorkerConfig configWithOverrides = new StandaloneConfig(props); + + Map connConfig = new HashMap(); + connConfig.put("metadata.max.age.ms", "10000"); + + Map expectedConfigs = new HashMap<>(); + expectedConfigs.put("bootstrap.servers", "localhost:9092"); + expectedConfigs.put("client.id", "testid"); + expectedConfigs.put("metadata.max.age.ms", "10000"); + + EasyMock.expect(connectorConfig.originalsWithPrefix(ConnectorConfig.CONNECTOR_CLIENT_ADMIN_OVERRIDES_PREFIX)) + .andReturn(connConfig); + PowerMock.replayAll(); + assertEquals(expectedConfigs, Worker.adminConfigs(new ConnectorTaskId("test", 1), configWithOverrides, connectorConfig, + null, allConnectorClientConfigOverridePolicy)); + + } + + @Test(expected = ConnectException.class) + public void testAdminConfigsClientOverridesWithNonePolicy() { + Map props = new HashMap<>(workerProps); + props.put("admin.client.id", "testid"); + props.put("admin.metadata.max.age.ms", "5000"); + WorkerConfig configWithOverrides = new StandaloneConfig(props); + + Map connConfig = new HashMap(); + connConfig.put("metadata.max.age.ms", "10000"); + + EasyMock.expect(connectorConfig.originalsWithPrefix(ConnectorConfig.CONNECTOR_CLIENT_ADMIN_OVERRIDES_PREFIX)) + .andReturn(connConfig); + PowerMock.replayAll(); + Worker.adminConfigs(new ConnectorTaskId("test", 1), configWithOverrides, connectorConfig, + null, noneConnectorClientConfigOverridePolicy); + + } + private void assertStatistics(Worker worker, int connectors, int tasks) { MetricGroup workerMetrics = worker.workerMetricsGroup().metricGroup(); From bff32a637698f8f61dd3f79b27a6fdf9c94fd29e Mon Sep 17 00:00:00 2001 From: Magesh Nandakumar Date: Thu, 16 May 2019 15:58:59 -0700 Subject: [PATCH 7/8] Fix config doc --- .../java/org/apache/kafka/connect/runtime/WorkerConfig.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/WorkerConfig.java b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/WorkerConfig.java index 2652440701274..773358fdff466 100644 --- a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/WorkerConfig.java +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/WorkerConfig.java @@ -215,7 +215,8 @@ public class WorkerConfig extends AbstractConfig { public static final String CONNECTOR_CLIENT_POLICY_CLASS_CONFIG = "client.config.policy"; public static final String CONNECTOR_CLIENT_POLICY_CLASS_DOC = "Class name or alias of implementation of ConnectorClientConfigOverridePolicy. Defines what client configurations can be " - + "overriden by the connector> The default implementation is `None` and other possible values include `All` and `Principal`."; + + "overriden by the connector. The default implementation is `None`. The other possible policies in the framework include `All` " + + "and `Principal`. "; public static final String CONNECTOR_CLIENT_POLICY_CLASS_DEFAULT = "None"; From 59f2916a69e71722532c6c60203df35231f56224 Mon Sep 17 00:00:00 2001 From: Magesh Nandakumar Date: Thu, 16 May 2019 18:14:40 -0700 Subject: [PATCH 8/8] Update connect/api/src/main/java/org/apache/kafka/connect/connector/policy/ConnectorClientConfigOverridePolicy.java Co-Authored-By: Randall Hauch --- .../policy/ConnectorClientConfigOverridePolicy.java | 5 +++-- .../kafka/connect/util/clusters/EmbeddedConnectCluster.java | 6 +++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/connect/api/src/main/java/org/apache/kafka/connect/connector/policy/ConnectorClientConfigOverridePolicy.java b/connect/api/src/main/java/org/apache/kafka/connect/connector/policy/ConnectorClientConfigOverridePolicy.java index 66691f725c078..94e5fd6f5b0cf 100644 --- a/connect/api/src/main/java/org/apache/kafka/connect/connector/policy/ConnectorClientConfigOverridePolicy.java +++ b/connect/api/src/main/java/org/apache/kafka/connect/connector/policy/ConnectorClientConfigOverridePolicy.java @@ -40,7 +40,8 @@ public interface ConnectorClientConfigOverridePolicy extends Configurable, AutoC * * @param connectorClientConfigRequest an instance of {@code ConnectorClientConfigRequest} that provides the configs to overridden and * its context; never {@code null} - * @return List of Config, each Config should indicate if they are allowed via {@link ConfigValue#errorMessages}; never null + * @return list of {@link ConfigValue} instances that describe each client configuration in the request and includes an + {@link ConfigValue#errorMessages error} if the configuration is not allowed by the policy; never null */ List validate(ConnectorClientConfigRequest connectorClientConfigRequest); -} \ No newline at end of file +} diff --git a/connect/runtime/src/test/java/org/apache/kafka/connect/util/clusters/EmbeddedConnectCluster.java b/connect/runtime/src/test/java/org/apache/kafka/connect/util/clusters/EmbeddedConnectCluster.java index beba027f4b0e8..07c27553fc622 100644 --- a/connect/runtime/src/test/java/org/apache/kafka/connect/util/clusters/EmbeddedConnectCluster.java +++ b/connect/runtime/src/test/java/org/apache/kafka/connect/util/clusters/EmbeddedConnectCluster.java @@ -364,11 +364,11 @@ public int executePut(String url, String body) throws IOException { } if (httpCon.getResponseCode() < HttpURLConnection.HTTP_BAD_REQUEST) { try (InputStream is = httpCon.getInputStream()) { - log.info("Put response for URL={} is {}", url, responseToString(is)); + log.info("PUT response for URL={} is {}", url, responseToString(is)); } } else { try (InputStream is = httpCon.getErrorStream()) { - log.info("Put error response for URL={} is {}", url, responseToString(is)); + log.info("PUT error response for URL={} is {}", url, responseToString(is)); } } return httpCon.getResponseCode(); @@ -393,7 +393,7 @@ public String executeGet(String url) throws IOException { while ((c = is.read()) != -1) { response.append((char) c); } - log.debug("Get response for URL={} is {}", url, response); + log.debug("GET response for URL={} is {}", url, response); return response.toString(); } catch (IOException e) { Response.Status status = Response.Status.fromStatusCode(httpCon.getResponseCode());