strimzi / strimzi-kafka-operator

Apache Kafka® running on Kubernetes
https://strimzi.io/
Apache License 2.0
4.88k stars 1.31k forks source link

[Enhancement] Allow external config for KafkaConnector #2866

Closed mikekamornikov closed 2 years ago

mikekamornikov commented 4 years ago

Is your feature request related to a problem? Please describe. We plan to implement a custom operator for entities which besides other things cause creation of KafkaConnector resources. These connectors have 40+ config settings each and 3/4 of that is usually a copy paste and can be considered a reasonable default. Supporting all possible values is hard to both implement and maintain. Additionally it causes a basic copy/paste issue - you change the value in place A and forget to do the same in place B.

Describe the solution you'd like It'll be great to move common configuration to some ConfigMap and leave inline config for overrides.

Describe alternatives you've considered

Additional context

Only 7-8 values are important for me. But I have to specify them all: ```yaml spec: class: io.debezium.connector.mysql.MySqlConnector tasksMax: 1 config: database.hostname: cxp-perf-02 database.port: "3306" database.user: "${file:/opt/kafka/external-configuration/cxp-perf-02/dbcreds.properties:MYSQL_USER}" database.password: "${file:/opt/kafka/external-configuration/cxp-perf-02/dbcreds.properties:MYSQL_PASSWORD}" database.server.id: "6402" database.server.name: cxp-perf database.ssl.mode: preferred database.whitelist: "db_cxp_perf_\\w+" table.blacklist: database.history.kafka.bootstrap.servers: b-1:9094,b-2:9094,b-3:9094 database.history.kafka.topic: cxp-perf-02.stack.dbhistory database.history.producer.security.protocol: SSL database.history.consumer.security.protocol: SSL key.converter: org.apache.kafka.connect.storage.StringConverter key.converter.schemas.enable: "false" value.converter: org.apache.kafka.connect.json.JsonConverter value.converter.schemas.enable: "true" tombstones.on.delete: "false" decimal.handling.mode: double time.precision.mode: connect include.schema.changes: "false" snapshot.mode: initial snapshot.locking.mode: minimal snapshot.lock.timeout.ms: 28800000 snapshot.delay.ms: 5000 transforms: RerouteToDbTopic,AddDBNameToKey,ExtractDbName,RerouteToStackTopic transforms.RerouteToDbTopic.type: org.apache.kafka.connect.transforms.RegexRouter transforms.RerouteToDbTopic.regex: "([a-zA-Z0-9_\\-]+)\\.([a-zA-Z0-9_\\-]+)\\.([a-zA-Z0-9_\\-]+)" transforms.RerouteToDbTopic.replacement: $2 transforms.AddDBNameToKey.type: "org.apache.kafka.connect.transforms.InsertField$Key" transforms.AddDBNameToKey.topic.field: db transforms.ExtractDbName.type: "org.apache.kafka.connect.transforms.ExtractField$Key" transforms.ExtractDbName.field: db transforms.RerouteToStackTopic.type: org.apache.kafka.connect.transforms.RegexRouter transforms.RerouteToStackTopic.regex: (.*) transforms.RerouteToStackTopic.replacement: cxp-perf-02.stack.data max.queue.size: "16384" max.batch.size: "4096" producer.override.acks: "1" producer.override.compression.type: zstd producer.override.linger.ms: "10" producer.override.batch.size: "163840" producer.override.buffer.memory: "335544320" ```
avif commented 4 years ago

Have you tried FileConfigProvider with a ConfigMap mounted as a volume? https://strimzi.io/docs/master/#external_configuration_as_volumes

Also, you could probably implement some mechanism to read a ConfigMap from the k8s api, override some of it's values and apply that to whatever connector template you are applying with your operator.

scholzj commented 4 years ago

My understanding of the proposal, I think it is basically asking for something liek this:

spec:
  class: io.debezium.connector.mysql.MySqlConnector
  tasksMax: 1
  config:
    fromConfigMap: my-configuration

I think that is not a bad idea.

mikekamornikov commented 4 years ago

@scholzj I see it more like this:

spec:
  class: io.debezium.connector.mysql.MySqlConnector
  tasksMax: 1

  externalConfiguration:
  - name: defaults
    valueFrom:
      configMapRef: debezium-default-config
  - name: perf-tweaks
    valueFrom:
      configMapRef: debezium-optimized-config
  - name: common-db-creds
    valueFrom:
      secretRef: debezium-db-creds

  config:
    # overrides on top of defaults and other merged external configs
    transforms: RerouteToStackTopic
    transforms.RerouteToStackTopic.type: org.apache.kafka.connect.transforms.RegexRouter
    transforms.RerouteToStackTopic.regex: (.*)
    transforms.RerouteToStackTopic.replacement: cxp-dev.stack.data
mikekamornikov commented 4 years ago

I don't like FileConfigProvider as I have to mount everything in KafkaConnect. Imagine 10s of these mounts just to cover some common use cases. And it still leaves me with big hard to read KafkaConnector definition. This way it's even worse. Regarding some custom mechanism and k8s api. Yes, most likely that's what I'm going to do in short term. Long term I think the definition ^^^ is better.

scholzj commented 4 years ago

@scholzj I see it more like this

TBH I'm not sure I understand how it would work. How would we know how to combina 3 different config maps into one config? I also wonder if some GitOps tools can make it easier for you to combine the configurations into the KafkaConnector resources without adding the complexity.

Also, in the past, we intentionally didn't wanted to reference any config maps or secrets from KafkaConnector because it is not protected by Kubernetes RBAC. So the split right now works the way that the one who deployes Kafkaconnect can mount the secrets and config maps, the one who manages the connectors cannot.

mikekamornikov commented 4 years ago

We use kustomize tool to merge overlays. But patch / merge is done for specific resource name there. So it doesn't scale. Regarding how to combine. I'm not sure if yaml parser reorders items in array. If it's not then merging configs while parsing them 1 by 1 with inline config in the end should work. If reorder is possible having 1 source of external config is fine if it can be overriden by inline config.

mikekamornikov commented 4 years ago

If it's not possible from security / delegation of resposibilities POV then I'll try to implement it 1 level higher (in the code which creates these objects).

avif commented 4 years ago

My understanding of the proposal, I think it is basically asking for something liek this:

spec:
  class: io.debezium.connector.mysql.MySqlConnector
  tasksMax: 1
  config:
    fromConfigMap: my-configuration

I think that is not a bad idea.

I understand what the OP is asking, and I support his proposal - just offering a workaround :)

scholzj commented 2 years ago

Triaged on 17.3.2022: We do not see an actual use-case where this would make sense and provide some advantage. Both Config Map and KafkaConnector resources are YAML. So managing them is equally easy / hard. For things such as secrets or shared configurations (Database URL etc), the Config Providers can be used. If anyone has some convincing use-case, please let us know.