Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhance KafkaBridge resource with consumer inactivity timeout and HTTP consumer/producer parts enablement #8732

Closed
ppatierno opened this issue Jun 26, 2023 · 6 comments · Fixed by #9820

Comments

@ppatierno
Copy link
Member

The current KafkaBridge custom resource has an http section for setting HTTP related configuration.
Right now it supports only host and port but the bridge also has:

  • http.timeoutSeconds : for deleting inactive consumers after a timeout (disabled by default)
  • http.consumer.enabled: to enable/disable the HTTP consumer part (enabled by default)
  • http.producer.enabled: to enable/disable the HTTP producer part (enabled by default)

We should allow the above parameters to be configured via the KafkaBridge spec.

@scholzj
Copy link
Member

scholzj commented Jun 29, 2023

Triaged on 29.6.2023: We should enable the configuration of these fields. But how should the API look like? Should have a proposal to clarify the API changes.

@steffen-karlsson
Copy link

steffen-karlsson commented Mar 12, 2024

If you'd like, I can try to look into this one @ppatierno and @scholzj :)

One comment, you @ppatierno write that host is currently supported, but when I look into the source code, it looks like its defaulting to HTTP_DEFAULT_HOST: https://github.com/strimzi/strimzi-kafka-operator/blob/main/cluster-operator/src/main/java/io/strimzi/operator/cluster/model/KafkaBridgeCluster.java#L408 and https://github.com/strimzi/strimzi-kafka-operator/blob/main/api/src/main/java/io/strimzi/api/kafka/model/bridge/KafkaBridgeHttpConfig.java#L31, also host is not defined in the CRD: https://github.com/strimzi/strimzi-kafka-operator/blob/main/install/cluster-operator/046-Crd-kafkabridge.yaml#L223

Just want to make sure that its intentional and no work is needed on the http.host property.

Thanks :)

@steffen-karlsson
Copy link

Hello @ppatierno

I've now added host and timeoutSeconds as options to the CRD.
Furthermore, I've decided to add consumer and producer object with an enabled boolean inside the http object.

Please see reference here for new CRD:

http:
  type: object
  properties:
    host:
      type: string
      description: The host which is the server listening on.
    port:
      type: integer
      minimum: 1023
      description: The port which is the server listening on.
    cors:
      type: object
      properties:
        allowedOrigins:
          type: array
          items:
            type: string
          description: List of allowed origins. Java regular expressions can be used.
        allowedMethods:
          type: array
          items:
            type: string
          description: List of allowed HTTP methods.
      required:
      - allowedOrigins
      - allowedMethods
      description: CORS configuration for the HTTP Bridge.
    timeoutSeconds:
      type: integer
      description: The timeout in seconds for deleting inactive consumers.
    producer:
      type: object
      properties:
        enabled:
          type: boolean
          description: Whether the HTTP producer should be enabled or disabled.
      description: Configurations for the HTTP Producer.
    consumer:
      type: object
      properties:
        enabled:
          type: boolean
          description: Whether the HTTP consumer should be enabled or disabled.
      description: Configurations for the HTTP Consumer.
  description: The HTTP related configuration.

Unsure wether you prefer it here or inside the dedicated consumer and producer objects, i.e. KafkaBridgeConsumerSpec and KafkaBridgeProducerSpec.

consumer:
  type: object
  properties:
    config:
      x-kubernetes-preserve-unknown-fields: true
      type: object
      description: "The Kafka consumer configuration used for consumer instances created by the bridge. Properties with the following prefixes cannot be set: ssl., bootstrap.servers, group.id, sasl., security. (with the exception of: ssl.endpoint.identification.algorithm, ssl.cipher.suites, ssl.protocol, ssl.enabled.protocols)."
  description: Kafka consumer related configuration.
producer:
  type: object
  properties:
    config:
      x-kubernetes-preserve-unknown-fields: true
      type: object
      description: "The Kafka producer configuration used for producer instances created by the bridge. Properties with the following prefixes cannot be set: ssl., bootstrap.servers, sasl., security. (with the exception of: ssl.endpoint.identification.algorithm, ssl.cipher.suites, ssl.protocol, ssl.enabled.protocols)."
  description: Kafka producer related configuration.

@ppatierno
Copy link
Member Author

@steffen-karlsson I mostly replied on the PR you opened (as answer to a Jakub's comment). I think as mentioned here, we would need a proposal to describe the API changes (as per the discussion during the triage).

@steffen-karlsson
Copy link

Raised discussion here for the two implementation details: strimzi/proposals#111

scholzj added a commit that referenced this issue May 15, 2024
…nd HTTP consumer/producer parts enablement (#9820)

Signed-off-by: Steffen Karlsson <[email protected]>
Signed-off-by: Steffen Wirenfeldt Karlsson <[email protected]>
Co-authored-by: Jakub Scholz <[email protected]>
@scholzj
Copy link
Member

scholzj commented May 15, 2024

Note: A new Bridge release will be needed to make the new options actually work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants