Skip to content

HDDS-15773. Make HddsUtils host:port helpers IPv6-safe#10804

Open
chihsuan wants to merge 4 commits into
apache:masterfrom
chihsuan:HDDS-15773
Open

HDDS-15773. Make HddsUtils host:port helpers IPv6-safe#10804
chihsuan wants to merge 4 commits into
apache:masterfrom
chihsuan:HDDS-15773

Conversation

@chihsuan

@chihsuan chihsuan commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

This change makes HddsUtils host:port handling IPv6-safe.

getHostName() previously removed a trailing :port with a regex. This left brackets in [::1]:9862 and corrupted bare IPv6 literals such as ::1 or 2001:db8::1. getScmAddressForClients() then rebuilt addresses using host + ":" + port, which creates an ambiguous address for IPv6 hosts.

This patch:

  • Uses Guava HostAndPort in getHostName(), matching the adjacent getHostPort() helper and returning unbracketed IPv6 hosts.
  • Uses getHostPortString() to rebuild SCM client addresses, producing valid bracketed IPv6 addresses such as [2001:db8::1]:9860.
  • Removes the unused HddsUtils.format(List<String>) method. It has no repository callers and parses role strings with IPv6-unsafe fixed-colon splitting.

Malformed host:port inputs, including a:b, host:99999, and unbracketed IPv6-with-port values, now throw IllegalArgumentException. This matches getHostPort() and the documented contract of getHostNameFromConfigKeys().

The fixed-index role parsing in StorageContainerManager#getScmRatisRoles() (HDDS-15774) and the ServerNotLeaderException suggested-leader flow (HDDS-15895) remain out of scope and require separate follow-up changes.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-15773

How was this patch tested?

  • Added TestHddsUtils#testGetHostName coverage for bracketed and bare IPv6 literals, IPv6 without a port, and malformed input.
  • Added TestHddsClientUtils coverage for IPv6 SCM client addresses and the ozone.scm.names fallback path.
  • Full TestHddsUtils and TestHddsClientUtils runs pass; hdds-common module unit tests pass (404 tests, 0 failures).
  • checkstyle.sh, rat.sh, and author.sh pass locally; full mvn clean install -DskipTests succeeds.

Generated-by: Claude Code (Opus 4.8)

Copilot AI review requested due to automatic review settings July 18, 2026 06:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@chihsuan
chihsuan marked this pull request as draft July 18, 2026 06:53
Copilot AI review requested due to automatic review settings July 18, 2026 06:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@chihsuan
chihsuan marked this pull request as ready for review July 18, 2026 06:55

@adoroszlai adoroszlai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @chihsuan for the patch. Functionality LGTM. Please reduce duplication in test.

Comment on lines +182 to +187
final OzoneConfiguration conf = new OzoneConfiguration();
conf.set(OZONE_SCM_CLIENT_ADDRESS_KEY, "2001:db8::1");
InetSocketAddress addr =
HddsUtils.getScmAddressForClients(conf).iterator().next();
assertEquals("2001:db8:0:0:0:0:0:1", addr.getHostString());
assertEquals(OZONE_SCM_CLIENT_PORT_DEFAULT, addr.getPort());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please extract a helper method for single test case:

testClientAddress(String clientAddressConfig, String expectedHost, int expectedPort)

to avoid duplicated boilerplate.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the review! Addressed in 1fc02f7.

Copilot AI review requested due to automatic review settings July 18, 2026 12:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants