Randall Hauch created KAFKA-10600:
-------------------------------------

             Summary: Connect adds error to property in validation result if 
connector does not define the property
                 Key: KAFKA-10600
                 URL: https://issues.apache.org/jira/browse/KAFKA-10600
             Project: Kafka
          Issue Type: Bug
          Components: KafkaConnect
    Affects Versions: 0.10.0.0
            Reporter: Randall Hauch


Kafka Connect's {{AbstractHerder.generateResult(...)}} method is responsible 
for taking the result of a {{Connector.validate(...)}} call and constructing 
the {{ConfigInfos}} object that is then mapped to the JSON representation.

As this method (see 
[code|https://github.com/apache/kafka/blob/1f8ac6e6fee3aa404fc1a4c01ac2e0c48429a306/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractHerder.java#L504-L507])
 iterates over the {{ConfigKey}} objects in the connector's {{ConfigDef}} and 
the {{ConfigValue}} objects returned by the {{Connector.validate(...)}} method, 
this method adds an error message to any {{ConfigValue}} whose 
{{configValue.name()}} does not correspond to a {{ConfigKey}} in the 
connector's {{ConfigDef}}. 

{code}
            if (!configKeys.containsKey(configName)) {
                configValue.addErrorMessage("Configuration is not defined: " + 
configName);
                configInfoList.add(new ConfigInfo(null, 
convertConfigValue(configValue, null)));
            }
{code}

Interestingly, these errors are not included in the total error count of the 
response. Is that intentional??

This behavior does not allow connectors to report validation errors against 
extra properties not defined in the connector's {{ConfigDef}}. 

Consider a connector that allows arbitrary properties with some prefix (e.g., 
{{connection.*}}) to be included and used in the connector properties. One 
example is to supply additional properties to a JDBC connection, where the 
connector may not be able to know these "additional properties" in advance 
because the connector either works with multiple JDBC drivers or the connection 
properties allowed by a JDBC driver are many and/or vary over different JDBC 
driver versions or server versions.

Such "additional properties" are not prohibited by Connect API, yet if a 
connector implementation chooses to include any such additional properties in 
the {{Connector.validate(...)}} result (whether or not the corresponding 
{{ConfigValue}} has an error) then Connect will always add the following error 
to that property. 

{quote}
Configuration is not defined: <additionalPropertyName>
{quote}

This code was in the 0.10.0.0 release of Kafka via the 
[PR|https://github.com/apache/kafka/pull/964] for KAFKA-3315, which is one of 
the tasks that implemented 
[KIP-26|https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=58851767]
 for Kafka Connect (approved and partially added in 0.9.0.0). There is no 
mention of "validation" in KIP-26 nor any followup KIP (that I can find).

I can kind of imagine the original thought process: any user-supplied property 
that is not defined by a {{ConfigDef}} is inherently an error. However, this 
assumption is not matched by any mention in the Connect API, documentation, or 
one of Connect's KIP.
IMO, this is a bug in the {{AbstractHerder}} that over-constrains the connector 
properties to be only those defined in the connector's {{ConfigDef}}.

Quite a few connectors already support additional properties, and it's perhaps 
only by chance that this happens to work: 
* If a connector does not override {{Connector.validate(...)}}, extra 
properties are not validated and therefore are not included in the resulting 
{{Config}} response with one {{ConfigValue}} per property defined in the 
connector's {{ConfigDef}}.
* If a connector does override {{Connector.validate(...)}} and includes in the 
{{Config}} response a {{ConfigValue}} for the any additional properties, the 
{{AbstractHerder.generateResults(...)}} method does add the error but does not 
include this error in the error count, which is actually used to determine if 
there are any validation problems before starting/updating the connector.

I propose that the {{AbstractHerder.generateResult(...)}} method be changed to 
not add it's error message to the validation result, and to properly handle all 
{{ConfigValue}} objects regardless of whether there is a corresponding 
{{ConfigKey}} in the connector's {{ConfigDef}}.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to