lsst-sqre / strimzi-registry-operator

A Kubernetes Operator for running the Confluent Schema Registry with a Strimzi-based Kafka cluster
MIT License
81 stars 17 forks source link

Need proper generic support for the strimzi v1beta2 listener API #51

Closed jonathansick closed 2 years ago

jonathansick commented 2 years ago

I'm not sure we successfully enabled generic support for strimzi's v1beta1 API and different listener types after all in #11.

This is what a Kafka listener spec looked like in a v1beta1 resource that we based the original operator implementation on:

apiVersion: kafka.strimzi.io/v1beta1
kind: Kafka
spec:
  kafka:
    authorization:
      type: simple
    listeners:
      tls:
        authentication:
          type: tls

Then in the strimzi v1beta2 API the listener schema turned into an array:

apiVersion: kafka.strimzi.io/v1beta2
kind: Kafka
spec:
  kafka:
    authorization:
      superUsers:
        - kafka-admin
      type: simple
    listeners:
      - authentication:
          type: scram-sha-512
        name: plain
        port: 9092
        tls: false
        type: internal
      - authentication:
          type: tls
        name: tls
        port: 9093
        tls: true
        type: internal

So our original approach at getting the listener, which was slightly updated in #11, is actually just leaning on the get fallback:

    listener_name = spec.get("listener", "plain")
    kafka = k8s_cr_api.get_namespaced_custom_object(
        group="kafka.strimzi.io",
        version=strimzi_version,
        namespace=namespace,
        plural="kafkas",
        name=cluster_name,
    )

This is probably the reason #50 seemed like a fix there because the user had their Kafka set up with a listener using a listener named tls — and hence the default's tug of war between Rubin using using plain listeners, tls listeners, and our external open source users!

I think the solution we'll implement is to update how the listener name is obtained/retrieved and add an extra code path for v1beta1 that we're still using for one cluster internally.

jonathansick commented 2 years ago

This is now resolved in the 0.5.0 release.