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

[Enhancement]: additional listener should inherit the configured authentication method #7531

Closed
lburgazzoli opened this issue Sep 15, 2023 · 6 comments · Fixed by #7594
Closed

Comments

@lburgazzoli
Copy link
Contributor

Module

Redpanda

Proposal

With the current redpanda.yaml template, additional listener are added as follow:

<#list kafkaApi.listeners as listener>
    - address: 0.0.0.0
      name: ${listener.address}
      port: ${listener.port}
</#list>

However if authentication is turned on, then using such listeners would fail because no authentication is configured. Ideally any new additional listener should inherit the defined authentication_method or - but that would be a little bit more convoluted - allow the user to also define the authentication method one adding a listener, i.e.

RedpandaContainer redpanda = new RedpandaContainer("docker.redpanda.com/redpandadata/redpanda:v23.1.7")
    .withListener(() -> "redpanda:19092?auth=sasl")
    .withNetwork(network);

I can provide a PR if this enhancement is accepted.

@eddumelendez
Copy link
Member

The RedpandaContainer was inspired by Testcontainers for Go, plus a few bits for additional listeners. I would like to see feedback from @weeco, who initially submitted the Redpanda module in Testcontainers for Go, related to how this can be achieved in both languages.

/cc @mdelapenya

@weeco
Copy link

weeco commented Sep 15, 2023

The Go testcontainers moduile does not have this "problem" because it does not allow adding more than one listener. I'm not sure what the usecase for doing this is in a testcontainers environment, but I think @lburgazzoli suggestion makes sense to allow adding the authentication method as a parameter when adding listeners.

@eddumelendez
Copy link
Member

I'm not sure what the usecase for doing this is in a testcontainers environment,

Nowadays, frameworks in Java ecosystem provides OOTB support with Testcontainers for testing and development mode. In development, there could be cases where running redpanda broker and console can be required, here the listener is added in order to be available through the docker network, so, the redpanda console can be configured correctly.

In the long term, this can happen in Testcontainers for Go as @mdelapenya already explored and blogged about it here.

@lburgazzoli can you share how the redpanda.yml file would looks like with auth, please? Just want to make sure got the full picture before going to implementation.

@lburgazzoli
Copy link
Contributor Author

The main reason of my enhancement request is to make redpanda available to other containers in the same network as in my e2e test I want to test the containers running my application themself hence I start both the app container an any additional container that is needed.

According to the doc, connecting to the redpanda broker from another container should be possible by adding additional listeners but those listeners do not inherit the authentication method which is eventually configured.

I've created a small commit to illustrate a possible implementation.

@eddumelendez given the following code:

RedpandaContainer redpanda = new RedpandaContainer("docker.redpanda.com/redpandadata/redpanda:v23.1.7")
    .withListener(() -> "tc-kafka:19092")
    .withNetwork(network);

Then the redpanda.kafka_api section of the redpanda.yml would become:

redpanda:
  kafka_api:
    - address: 0.0.0.0
      name: external
      port: 9092
      authentication_method: sasl

    # This listener is required for the schema registry client. The schema
    # registry client connects via an advertised listener like a normal Kafka
    # client would do. It can't use the other listener because the mapped
    # port is not accessible from within the Redpanda container.
    - address: 0.0.0.0
      name: internal
      port: 9093
      authentication_method: sasl

    - address: 0.0.0.0
      name: tc-kafka
      port: 19092
      authentication_method: sasl

@lburgazzoli
Copy link
Contributor Author

@eddumelendez @weeco do you think that this enhancement makes sense ?

@eddumelendez
Copy link
Member

Sorry for the delay, I was on holidays. Looking at RedpandaContainerTest there is no scenario about adding a listener and enabling Authorization. For that reason, didn't caught it. @lburgazzoli perfectly fine, your PR would be very welcome!

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