Refactor ListenerConfig and ListenerRole into Listener class with constants

[guancioul]

Close #212,

Summary

• Extract the ListenerConfig inner class and ListenerRole enum from StrimziKafkaContainer into a new top-level Listener class (Listener.java), reducing the coupling between listener configuration and the container class
• Define listener name constants (PLAINTEXT, CONTROLLER, CONTROLLER_EXTERNAL, INTER_BROKER_PREFIX) in Listener to eliminate magic strings scattered across StrimziKafkaContainer
• Update all call sites in StrimziKafkaContainer and StrimziKafkaContainerTest to use the new Listener class and its constants

[see-quick]

Thanks for this change @guancioul, please also update all test suites which uses exact strings of the listeners and change it to Listener.PLAINTEXT ... (i.e.,StrimziKafkaContainerMockTest, StrimziKafkaClusterTest, StrimziKafkaContainerIT).

[guancioul]

Thanks for the review. I have updated some missing references in those test suites in fb2948d. One thing I'd like to check — there are also usages where the listener name is embedded in a full URL string, for example:

assertThat(props.getProperty("advertised.listeners"), is("CONTROLLER://broker-1:9094"));
assertThat(networkBootstrapControllers, is("CONTROLLER://broker-1:9094"));

Should these also be updated to use the constants (e.g. Listener.CONTROLLER + "://broker-1:9094")? Personally I feel the current form is more readable since the full expected value is visible at a glance, but happy to update if you prefer consistency.

[see-quick]

Should these also be updated to use the constants (e.g. Listener.CONTROLLER + "://broker-1:9094")? Personally I feel the current form is more readable since the full expected value is visible at a glance, but happy to update if you prefer consistency.

While I partially agree, I believe it would be better to define listener names from a single source of truth. This would help avoid making the same change in multiple places. If the listener name needs to be updated in the future, it could be done in one location, and all related tests would be updated automatically through refactoring.

[guancioul]

Agree, I change the code to let listener names become a single source of truth. I also added SASL_PLAINTEXT and SASL_SSL constants to the Listener class, as those were also scattered as magic strings accross StrimziKafkaContainer and StrimziKafkaCluster.

[see-quick]

Would you mind rebasing after this change #227?

[guancioul]

Sure, I have rebased this branch!

[see-quick]

Sure, I have rebased this branch!

Thanks, I will have a look once I find some time.

[see-quick]

LGTM, thanks 👍

[see-quick]

Please take a look on this @strimzi/maintainers .