You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "rjgoyln (via GitHub)" <gi...@apache.org> on 2026/08/27 11:26:39 UTC
[PR] HDDS-16307. Construct listeners from separate host and port values [ozone]
rjgoyln opened a new pull request, #11130:
URL: https://github.com/apache/ozone/pull/11130
## What changes were proposed in this pull request?
A bind-host property set to an IPv6 literal — including the wildcard `::` — stops the service from starting. Ozone joins host and port by string concatenation before parsing the result, so `::` and port 9860 become `:::9860`, which has no unambiguous host, and `NetUtils.createSocketAddr` rejects it with "Does not contain a valid host:port authority". The listener is never bound.
Every listener-construction site now combines through `HddsUtils.getHostPortString`, the bracket-aware helper added in HDDS-15768. That covers the SCM client, block, security and datanode endpoints and their SCM HA per-node equivalents, the datanode client RPC endpoint, the OM RPC and peer HTTP endpoints, and — through `BaseHttpServer.getBindAddress` — every HTTP and HTTPS listener in the project.
Three sites undid that fix further along the same code path by re-serializing an address that had already been built correctly: the Jetty endpoint URI, and the two places that write a server's real listen address back into configuration once it has bound. An unbracketed authority written there is read back as a bare host with the port silently dropped, so the endpoint cannot be recovered from configuration either. These used Hadoop's `NetUtils.getHostPortString(InetSocketAddress)`, which does not bracket; a bracket-aware overload of the Ozone helper replaces it.
IPv4 and DNS behaviour is unchanged, since a host is bracketed only when it contains a colon, and no configuration property or bind default changes. Advertised-address validation stays with the second sub-task of HDDS-15778, along with the four remaining call sites that render advertised peer identities.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-16307
## How was this patch tested?
New and extended unit tests in `TestHddsUtils`, `TestHddsServerUtil`, `TestScmUtils`, `TestBaseHttpServer`, `TestServerUtils` and `TestOmUtils`, each confirmed to fail without the production change. The existing IPv4 and DNS assertions in those classes are kept, so the unchanged defaults stay covered.
`updateConnectorAddress` has no dedicated test: asserting on it needs a server actually bound to `::`, whose accepted address families depend on the operating system's `IPV6_V6ONLY` handling. That qualification belongs with the IPv6 test environment in HDDS-15779.
Generated-by: Claude Code (Opus 5)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16307. Construct listeners from separate host and port values [ozone]
Posted by "rjgoyln (via GitHub)" <gi...@apache.org>.
rjgoyln commented on PR #11130:
URL: https://github.com/apache/ozone/pull/11130#issuecomment-5467378426
> o, the description still mentions the removed helper overload. Could you update PR description to reflect the changes? Thanks!
Thanks for the review! I've confirmed them against the code.
Regarding how to track them, I suggest splitting the two:
* **Unbracketed IPv6 (Silent Failure):** Let's track this in **HDDS-16308**. As you noted, Guava now brackets it into `[2001:db8::1:9862]:9862`, turning a loud failure into a silent one. Rejecting this ambiguous form outright fits perfectly into the scope of HDDS-16308's config validation.
* **SPNEGO `_HOST` resolving to `[::]`:** I suggest filing a **new JIRA** under HDDS-15763 for this. Since this is about deciding how `_HOST` should resolve when intentionally binding to a wildcard (rather than just rejecting a bad config), keeping it separate from HDDS-16308 makes sense.
Let me know if this split sounds right, and I'll file the new ticket!
*(Also, the PR description has been updated and no longer refers to the removed helper overload.)*
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16307. Construct listeners from separate host and port values [ozone]
Posted by "rjgoyln (via GitHub)" <gi...@apache.org>.
rjgoyln commented on PR #11130:
URL: https://github.com/apache/ozone/pull/11130#issuecomment-5460647606
> Thanks for the patch! @rjgoyln Overall, looks good.
>
> I left a few inline notes, mostly around test coverage for the URI path.
>
> Also a heads-up: #11108 makes the identical `OmUtils` change, so whichever lands first will make the other's diff vanish. cc @echonesis
Thanks for the heads-up on #11108! I just checked the diffs. The OmUtils.java changes are completely identical, and our test additions in TestOmUtils.java are in completely different locations.
I’ll keep this PR self-contained for now. If #11108 lands first, I’ll rebase and clean up the redundant changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16307. Construct listeners from separate host and port values [ozone]
Posted by "rjgoyln (via GitHub)" <gi...@apache.org>.
rjgoyln commented on code in PR #11130:
URL: https://github.com/apache/ozone/pull/11130#discussion_r3885752779
##########
hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/server/http/TestBaseHttpServer.java:
##########
@@ -137,6 +138,23 @@ protected String getHttpAuthConfigPrefix() {
assertEquals("/1.2.3.4:1234", baseHttpServer
.getBindAddress("bindhostkey", "addresskey",
"default", 65).toString());
+
+ // An IPv6 bind host, wildcard or literal, must survive being combined with
+ // the port. Assert on the address rather than toString(), whose bracketing
+ // of IPv6 literals is a JDK detail.
+ conf.set("bindhostkey", "::");
+
+ InetSocketAddress wildcard = baseHttpServer
+ .getBindAddress("bindhostkey", "addresskey", "default", 65);
+ assertEquals(InetAddress.getByName("::"), wildcard.getAddress());
Review Comment:
Great catch on the silent `null` host. I extracted a package-private `newEndpointUri` method to act as a test seam and added explicit test coverage for it. Verified that reverting the fix now correctly turns the suite red (`expected: <[...]> but was: <null>`).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16307. Construct listeners from separate host and port values [ozone]
Posted by "rjgoyln (via GitHub)" <gi...@apache.org>.
rjgoyln commented on code in PR #11130:
URL: https://github.com/apache/ozone/pull/11130#discussion_r3885752029
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/HddsUtils.java:
##########
@@ -243,6 +243,19 @@ public static String getHostPortString(String host, int port) {
return HostAndPort.fromParts(host, port).toString();
}
+ /**
+ * Render an address as a "host:port" string, wrapping the host in square
+ * brackets when it is an IPv6 literal. Hadoop's
+ * {@code NetUtils.getHostPortString(InetSocketAddress)} joins the two with a
+ * plain colon, which yields an ambiguous authority for IPv6.
+ *
+ * @param addr the address to render
+ * @return the combined address, bracketed for IPv6 literals
+ */
+ public static String getHostPortString(InetSocketAddress addr) {
+ return getHostPortString(addr.getHostName(), addr.getPort());
Review Comment:
Good point. I've removed the single-argument overload. The call sites now explicitly pass `getHostName()` and `getPort()`, making the reverse DNS lookup cost clearly visible where it occurs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16307. Construct listeners from separate host and port values [ozone]
Posted by "chihsuan (via GitHub)" <gi...@apache.org>.
chihsuan commented on code in PR #11130:
URL: https://github.com/apache/ozone/pull/11130#discussion_r3885487380
##########
hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/server/http/TestBaseHttpServer.java:
##########
@@ -137,6 +138,23 @@ protected String getHttpAuthConfigPrefix() {
assertEquals("/1.2.3.4:1234", baseHttpServer
.getBindAddress("bindhostkey", "addresskey",
"default", 65).toString());
+
+ // An IPv6 bind host, wildcard or literal, must survive being combined with
+ // the port. Assert on the address rather than toString(), whose bracketing
+ // of IPv6 literals is a JDK detail.
+ conf.set("bindhostkey", "::");
+
+ InetSocketAddress wildcard = baseHttpServer
+ .getBindAddress("bindhostkey", "addresskey", "default", 65);
+ assertEquals(InetAddress.getByName("::"), wildcard.getAddress());
Review Comment:
Could we also cover the endpoint URI path here? I found that an unbracketed IPv6 authority makes `URI.create` return a `null` host instead of throwing, so reverting that line keeps the suite green.
##########
hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/TestHddsUtils.java:
##########
@@ -102,6 +106,31 @@ void testGetHostPortString() {
assertEquals("[2001:db8::1]:9858", HddsUtils.getHostPortString("[2001:db8::1]", 9858));
}
+ @Test
+ void testGetHostPortStringFromAddress() {
+ assertEquals("1.2.3.4:9858",
+ HddsUtils.getHostPortString(new InetSocketAddress("1.2.3.4", 9858)));
Review Comment:
Worth using `192.0.2.1` here? I noticed this expectation holds only while the reverse lookup fails, and `1.2.3.4` is routable space that may have a PTR record on some resolvers.
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/HddsUtils.java:
##########
@@ -243,6 +243,19 @@ public static String getHostPortString(String host, int port) {
return HostAndPort.fromParts(host, port).toString();
}
+ /**
+ * Render an address as a "host:port" string, wrapping the host in square
+ * brackets when it is an IPv6 literal. Hadoop's
+ * {@code NetUtils.getHostPortString(InetSocketAddress)} joins the two with a
+ * plain colon, which yields an ambiguous authority for IPv6.
+ *
+ * @param addr the address to render
+ * @return the combined address, bracketed for IPv6 literals
+ */
+ public static String getHostPortString(InetSocketAddress addr) {
+ return getHostPortString(addr.getHostName(), addr.getPort());
Review Comment:
I wonder if we need this overload at all. The call sites could pass `addr.getHostName()` to the two-arg version, keeping parity with Hadoop. That avoids a new public helper and leaves `getHostName()`'s reverse lookup visible at each call site.
It would also avoid baking in `getHostName()`, which does a reverse lookup when the address carries no hostname.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16307. Construct listeners from separate host and port values [ozone]
Posted by "chihsuan (via GitHub)" <gi...@apache.org>.
chihsuan commented on PR #11130:
URL: https://github.com/apache/ozone/pull/11130#issuecomment-5467390039
> Let me know if this split sounds right, and I'll file the new ticket!
Sounds good to me.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16307. Construct listeners from separate host and port values [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on PR #11130:
URL: https://github.com/apache/ozone/pull/11130#issuecomment-5823543461
Thanks @rjgoyln for the patch. Thanks @chihsuan for the review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16307. Construct listeners from separate host and port values [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl merged PR #11130:
URL: https://github.com/apache/ozone/pull/11130
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16307. Construct listeners from separate host and port values [ozone]
Posted by "rjgoyln (via GitHub)" <gi...@apache.org>.
rjgoyln commented on code in PR #11130:
URL: https://github.com/apache/ozone/pull/11130#discussion_r3885740096
##########
hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/TestHddsUtils.java:
##########
@@ -102,6 +106,31 @@ void testGetHostPortString() {
assertEquals("[2001:db8::1]:9858", HddsUtils.getHostPortString("[2001:db8::1]", 9858));
}
+ @Test
+ void testGetHostPortStringFromAddress() {
+ assertEquals("1.2.3.4:9858",
+ HddsUtils.getHostPortString(new InetSocketAddress("1.2.3.4", 9858)));
Review Comment:
You're completely right about the PTR record risk. I've replaced `1.2.3.4` with the RFC 5737/3849 reserved TEST-NET addresses (`192.0.2.1` for IPv4 and `2001:db8::1` for IPv6) across the new URI tests to prevent any flaky lookups.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org