Uploaded image for project: 'Kafka'
  1. Kafka
  2. KAFKA-10600

Connect adds error to property in validation result if connector does not define the property

    XMLWordPrintableJSON

Details

    • Bug
    • Status: Resolved
    • Major
    • Resolution: Fixed
    • 0.10.0.0, 0.11.0.0, 1.0.0, 1.1.0, 2.0.0, 2.1.0, 2.2.0, 2.3.0, 2.4.0, 2.5.0, 2.6.0
    • 2.7.0, 2.5.2, 2.6.1
    • connect
    • None

    Description

      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) 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.

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

      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.

      Configuration is not defined: <additionalPropertyName>

      This code was in the 0.10.0.0 release of Kafka via the PR for KAFKA-3315, which is one of the tasks that implemented KIP-26 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 any additional 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.

      Attachments

        Issue Links

          Activity

            People

              rhauch Randall Hauch
              rhauch Randall Hauch
              Konstantine Karantasis Konstantine Karantasis
              Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: