Skip to content

[improve][broker] Add tests for using absolute FQDN for advertisedAddress and remove extra dot from brokerId #24787

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

[improve][broker] Add tests for using absolute FQDN for advertisedAddress and remove extra dot from brokerId #24787

wants to merge 2 commits into from
+101 −3

Conversation

lhotari
Copy link
Member

@lhotari lhotari commented Sep 25, 2025

Motivation

When brokers are exposed directly to clients, there might be a need to make the advertisedAddress absolute so that clients with unoptimal ndots configuration won't do unnecessary DNS lookups on the client side when an advertisedAddress with less dots than ndots is returned to the client from topic lookup.

Modifications

  • remove extra dot from brokerId so that the brokerId format doesn't change when advertisedAddress is made absolute
  • add tests for brokerId
  • add integration tests to ensure that client can connect when TLS is enabled. This ensures that TLS hostname validation doesn't fail with the extra trailing dot.

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

@lhotari lhotari added this to the 4.2.0 milestone Sep 25, 2025
@lhotari lhotari requested review from nodece and dao-jun September 25, 2025 18:50
@lhotari lhotari self-assigned this Sep 25, 2025
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Sep 25, 2025
nodece
nodece approved these changes Sep 28, 2025
View reviewed changes
Copy link
Member

@nodece nodece left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nodece nodece nodece approved these changes

@dao-jun dao-jun dao-jun approved these changes

Assignees

@lhotari lhotari

Labels
doc-not-needed Your PR changes do not impact docs ready-to-test
Projects
None yet
Milestone
4.2.0
Development

Successfully merging this pull request may close these issues.
3 participants
@lhotari @nodece @dao-jun