Skip to content
This repository was archived by the owner on Jan 24, 2024. It is now read-only.

Implement placeholder 'advertisedAddress' in kafkaAdvertisedListeners#1011

Merged
BewareMyPower merged 2 commits intostreamnative:masterfrom
eolivelli:impl/advertisedListenerPlaceholder
Jan 14, 2022
Merged

Implement placeholder 'advertisedAddress' in kafkaAdvertisedListeners#1011
BewareMyPower merged 2 commits intostreamnative:masterfrom
eolivelli:impl/advertisedListenerPlaceholder

Conversation

@eolivelli
Copy link
Contributor

@eolivelli eolivelli commented Jan 10, 2022

With this patch when you configure kafkaAdvertisedListeners you can use the special placeholder advertisedAddress that picks up the value configured for advertisedAddress in broker.conf.

Something like:
kafkaAdvertisedListeners=PLAINTEXT://advertisedAddress:9092

Without this placeholder you have to inject the value using other external means, like adding stuff to the Helm Chart (status.podIP) and it is not trivial.

@eolivelli
Copy link
Contributor Author

I will add docs if the feature is accepted

@eolivelli
Copy link
Contributor Author

@Demogorgon314 @BewareMyPower PTAL

@eolivelli
Copy link
Contributor Author

@Demogorgon314 did you have time to take a look ?

Copy link
Member

@Demogorgon314 Demogorgon314 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@eolivelli
Copy link
Contributor Author

@BewareMyPower @Demogorgon314 could we commit this patch ?
or should I add docs together this patch ?
no problem from my side

@BewareMyPower
Copy link
Collaborator

What I concern is that the placeholder doesn't contain the special characters. Is there a possibility that advertisedAddress is a valid domain name.

@eolivelli
Copy link
Contributor Author

What I concern is that the placeholder doesn't contain the special characters. Is there a possibility that advertisedAddress is a valid domain name.

@BewareMyPower the fact is that special characters don't play well with Helm/Bash/YAML...it is always a pain with escaping (underscore, dot, dollar symbol...)

So I opted to use the same configuration entry that we have in broker.conf.
It is unlikely that there is a hostname named "advertisedAddress" so it is a safe placeholder.

We will document this very well

I would add that initially I thought to resolve to advertisedAddress the case of "empty hostname", but that would have changed the current behaviour, so "advertisedAddress" is safer and backward compatible

@BewareMyPower BewareMyPower merged commit 4269ed3 into streamnative:master Jan 14, 2022
@BewareMyPower BewareMyPower added the type/feature Indicates new functionality label Jan 14, 2022
@eolivelli eolivelli deleted the impl/advertisedListenerPlaceholder branch January 15, 2022 07:41
@eolivelli
Copy link
Contributor Author

Thanks!

I will follow up with docs next Monday

BewareMyPower pushed a commit that referenced this pull request Jan 23, 2022
…#1011)

With this patch when you configure kafkaAdvertisedListeners you can use the special placeholder `advertisedAddress` that picks up the value configured for `advertisedAddress` in broker.conf.

Something like:
`kafkaAdvertisedListeners=PLAINTEXT://advertisedAddress:9092`

Without this placeholder you have to inject the value using other external means, like adding stuff to the Helm Chart (status.podIP) and it is not trivial.
BewareMyPower pushed a commit that referenced this pull request Jan 23, 2022
…#1011)

With this patch when you configure kafkaAdvertisedListeners you can use the special placeholder `advertisedAddress` that picks up the value configured for `advertisedAddress` in broker.conf.

Something like:
`kafkaAdvertisedListeners=PLAINTEXT://advertisedAddress:9092`

Without this placeholder you have to inject the value using other external means, like adding stuff to the Helm Chart (status.podIP) and it is not trivial.
BewareMyPower pushed a commit that referenced this pull request Feb 9, 2022
…#1011)

With this patch when you configure kafkaAdvertisedListeners you can use the special placeholder `advertisedAddress` that picks up the value configured for `advertisedAddress` in broker.conf.

Something like:
`kafkaAdvertisedListeners=PLAINTEXT://advertisedAddress:9092`

Without this placeholder you have to inject the value using other external means, like adding stuff to the Helm Chart (status.podIP) and it is not trivial.

(cherry picked from commit 4269ed3)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants