HDDS-15775. Support IPv6 literal authorities in OzoneFS URI parsing - #10933
Open
rich7420 wants to merge 3 commits into
Open
HDDS-15775. Support IPv6 literal authorities in OzoneFS URI parsing#10933rich7420 wants to merge 3 commits into
rich7420 wants to merge 3 commits into
Conversation
BasicRootedOzoneFileSystem parsed the ofs authority with authority.split(":"),
which breaks on bracketed IPv6 literals (for example ofs://[::1]:9862): the
address itself contains colons, so the split yields more than two parts and the
URI is rejected. Parse with Guava HostAndPort instead, which is bracket-aware,
matching HddsUtils. The IPv6 literal is kept bracketed so the downstream
host:port assembly that builds the OM address stays unambiguous.
o3fs is unaffected: java.net.URI rejects a bracketed IPv6 literal embedded in a
bucket.volume.host authority, so o3fs reaches an IPv6 OM through configuration
(covered by HDDS-15773), not the URI authority.
Part of the IPv6 support epic HDDS-15763.
The ofs authority fix hands the OM host to OzoneClientFactory.getRpcClient, which assembled the address with omHost + ":" + omRpcPort and validated it via OmUtils.resolveOmHost -> NetUtils.createSocketAddr(omHost, omPort). Both forms mishandle an IPv6 literal: a bare ::1 yields the ambiguous ::1:9862 and makes createSocketAddr reject it with "not a valid host:port authority". Assemble the address with HddsUtils.getHostPortString, which brackets IPv6 literals ([::1]:9862). Output is unchanged for hostnames and IPv4.
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Adds IPv6-literal-aware parsing for OzoneFS URI authorities (eg ofs://[::1]:9862/...) so bracketed IPv6 hosts are handled correctly and OM addresses remain unambiguous.
Changes:
- Switch authority parsing to an IPv6-aware parser (
HostAndPort) inBasicRootedOzoneFileSystem. - Ensure OM host resolution / OM address configuration correctly brackets IPv6 literals via
getHostPortString. - Add/extend unit tests validating IPv6 literal handling in filesystem init and OM host resolution.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java | Parse authority with HostAndPort and preserve bracketed IPv6 literals. |
| hadoop-ozone/ozonefs-common/src/test/java/org/apache/hadoop/fs/ozone/TestBasicOzoneFileSystems.java | Add parameterized test cases covering bracketed IPv6 authorities for OFS. |
| hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java | Build host:port via getHostPortString before createSocketAddr to support IPv6 literals. |
| hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/TestOmUtils.java | Add test ensuring IPv6 literals don’t fail parsing in resolveOmHost. |
| hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneClientFactory.java | Configure OM address using getHostPortString to avoid ambiguous IPv6 host:port strings. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+146
to
+156
| @ParameterizedTest | ||
| @CsvSource(value = { | ||
| // hostname / IPv4 authority (behaviour unchanged) | ||
| "ofs://host:9862/, host, 9862", | ||
| "ofs://omservice1/, omservice1, -1", | ||
| // service id with an underscore: HostAndPort tolerates it (URI.getHost does not) | ||
| "ofs://om_service/, om_service, -1", | ||
| // IPv6 literal authority, with and without a port; the literal stays bracketed | ||
| "ofs://[::1]:9862/, [::1], 9862", | ||
| "ofs://[2001:db8::1]/, [2001:db8::1], -1", | ||
| }) |
Comment on lines
+166
to
170
| try { | ||
| hostAndPort = HostAndPort.fromString(authority); | ||
| } catch (IllegalArgumentException e) { | ||
| throw new IllegalArgumentException(URI_EXCEPTION_TEXT); | ||
| } |
Comment on lines
+171
to
+175
| int omPort = hostAndPort.hasPort() ? hostAndPort.getPort() : -1; | ||
| String host = hostAndPort.getHost(); | ||
| // Keep IPv6 literals bracketed so the downstream host:port assembly that | ||
| // builds the OM address stays unambiguous. | ||
| String omHostOrServiceId = host.contains(":") ? "[" + host + "]" : host; |
Comment on lines
+165
to
+171
| final HostAndPort hostAndPort; | ||
| try { | ||
| hostAndPort = HostAndPort.fromString(authority); | ||
| } catch (IllegalArgumentException e) { | ||
| throw new IllegalArgumentException(URI_EXCEPTION_TEXT); | ||
| } | ||
| omHostOrServiceId = parts[0]; | ||
| if (parts.length == 2) { | ||
| try { | ||
| omPort = Integer.parseInt(parts[1]); | ||
| } catch (NumberFormatException e) { | ||
| throw new IllegalArgumentException(URI_EXCEPTION_TEXT); | ||
| } | ||
| } | ||
| int omPort = hostAndPort.hasPort() ? hostAndPort.getPort() : -1; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
ofs://[::1]:9862/pathfails because the authority is split on every colon.BasicRootedOzoneFileSystem.java: the authority parsing splits on:, so a bracketed IPv6 literal yields more than two parts and throwsIllegalArgumentException(URI_EXCEPTION_TEXT).This change parses the authority with Guava
HostAndPort, which understands bracketed IPv6 literals and rejects out-of-range ports. IPv6 hosts are kept bracketed when the OM host:port address is reassembled so the downstream parse stays unambiguous.Why ofs only (o3fs is out of scope)
o3fs encodes the OM host inside the authority as
o3fs://bucket.volume.host:port. For an IPv6 literal that would beo3fs://bucket.volume.[::1]:9862/, where the brackets sit in the middle of the authority.java.net.URIrejects that form (URISyntaxException: Illegal character in hostname), so an o3fs IPv6 authority can never reach the filesystem to begin with. ofs puts the authority at the start (ofs://[::1]:9862/), whichjava.net.URIaccepts, so ofs is the only scheme where this fix is reachable.BasicOzoneFileSystem(o3fs) is left unchanged.Parent epic: HDDS-15763.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-15775
How was this patch tested?
Added
TestBasicOzoneFileSystemscases covering ofs authority parsing (hostname, service id, underscore host,[::1]:9862,[2001:db8::1]) and out-of-range port rejection.https://github.com/rich7420/ozone/actions/runs/30821150742