Conversation
…n, and publication A wildcard, link-local, or scoped address is valid to bind but names no endpoint a peer can reach, and a zone identifier cannot be encoded in an X.509 certificate. Nothing rejected such a value where it is read as an advertised address, so the misconfiguration surfaced later as a failure to connect, or as a certificate the peer cannot use. The textual form of a configured authority is a separate gap. Both readings of an unbracketed IPv6 literal are valid literals, so an operator who means a host and a port gets the whole literal as the host and the property's default port, with no error anywhere. Advertised properties are the ones a peer or client resolves and that never serve as a bind address. The unsuffixed ozone.om.address is excluded: it ships as 0.0.0.0:9862 and a non-HA OM binds to it.
A bracketed literal reaches the address with its brackets stripped, so [::] configured as an advertised host was taken for a hostname and passed every check while the bare form was rejected. A value that can never be advertised should also say so directly, rather than first be asked for brackets that leave it rejected. The SCM block-client, security-service and datanode address properties are read as peer identities on the non-HA path as well, so they are checked alongside ozone.scm.client.address and ozone.scm.names.
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Correct the ambiguous IPv6 peer-address serialization in OmUtils.java before approval.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
What changed in this PR
This PR validates advertised network addresses and improves IPv6 host/port formatting across SCM, OM, Recon, and related configuration paths.
Changes:
- Rejects invalid wildcard, link-local, scoped, and ambiguous IPv6 advertised addresses.
- Applies validation across SCM, OM, Recon, and datanode paths.
- Updates IPv6 formatting and expands unit coverage.
A critical issue remains in OmUtils.java: valid bracketed IPv6 OM peer addresses are serialized ambiguously.
| File | Summary |
|---|---|
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ha/OMHANodeDetails.java |
Formats IPv6 OM log addresses. |
hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/TestOmUtils.java |
Tests OM address validation. |
hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOMNodeDetails.java |
Tests IPv6 OM formatting. |
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java |
Validates OM advertised addresses and builds peer identities. |
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/TestHddsServerUtils.java |
Tests SCM and Recon validation. |
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMNodeDetails.java |
Uses shared RPC address formatting. |
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMHANodeDetails.java |
Formats SCM HA log addresses. |
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HddsServerUtil.java |
Validates SCM and Recon endpoints. |
hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/TestHddsUtils.java |
Tests address validation and parsing. |
hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/scm/ha/TestSCMNodeInfo.java |
Tests SCM validation. |
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMNodeInfo.java |
Validates SCM advertised addresses. |
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/NodeDetails.java |
Brackets IPv6 RPC addresses. |
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/HddsUtils.java |
Adds address validation and IPv6 authority checks. |
hadoop-hdds/client/src/test/java/org/apache/hadoop/hdds/scm/client/TestHddsClientUtils.java |
Tests IPv6 client address rejection. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
174
to
175
| return host.get() + ":" + getPortNumberFromConfigKeys(conf, confKey) | ||
| .orElse(OZONE_OM_PORT_DEFAULT); |
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?
A wildcard, link-local, or scoped address can bind successfully, but it does not identify an endpoint that a peer can reach. A zone identifier also cannot be used in an X.509 certificate. Previously, these values were not rejected when read as an advertised address, so the misconfiguration was only discovered later when a peer could not connect or a certificate could not be used.
MiniOzoneClusterImplalready pins SCM bind hosts to127.0.0.1to prevent0.0.0.0from leaking intoozone.scm.names. This patch applies the same validation at the point where an advertised address is read.The configured text has another ambiguity.
ozone.om.address = 2001:db8::1:9862can be interpreted either as host2001:db8::1with port9862, or as the complete IPv6 literal using the property's default port. Both interpretations are valid IPv6 literals, and since #11130 the latter is accepted silently instead of being rejected.This patch:
getPortNumberFromConfigKeys, the only helper where the property value may be followed by a port.HddsUtils.getHostPortString, as deferred from HDDS-16307. Construct listeners from separate host and port values #11130.The advertised addresses covered by this validation are
ozone.scm.names, the SCM and OM per-node address properties, the four SCM client-facing*.addressproperties,ozone.recon.address, andhdds.datanode.hostname. All*.bind.hostproperties are excluded. The unsuffixedozone.om.addressis also excluded because it defaults to0.0.0.0:9862and is used as the bind address for non-HA OM.Loopback remains accepted. This is why
isValidInetForCsris not reused: excluding loopback is appropriate for a certificate SAN, but would be incorrect for a single-host deployment.ServerUtils.updateListenAddresswrites a server's bind host back into its own address property. However, a non-HA SCM does not read those properties again after the servers bind, so this write-back does not trigger the new validation.OmUtilsstill joins the per-node OM address with a plain colon on this branch. As a result, a bracketed IPv6 value is emitted without brackets and is then rejected byNetUtils.createSocketAddrduring startup. #11130 converts all four such joins there.The identity sites converted in this patch pass
getHostName()andgetPort()toHddsUtils.getHostPortString, rather than introducing a newInetSocketAddressoverload, as requested during review.What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-16308
How was this patch tested?
Unit tests across the six affected helpers cover both accepted and rejected inputs.
Accepted inputs include:
Rejected inputs include:
Each rejection test was also confirmed to fail without the production change.
Additional guards cover cases where over-rejection would be particularly problematic:
::bind hostozone.om.addresswildcard defaultTestMiniOzoneClusteralso passes, exercising a real cluster startup through the validated paths.Behavior change
An IPv6 literal now needs brackets wherever a port may follow. For example:
ozone.scm.client.address = 2001:db8::1must now be configured as:
ozone.scm.client.address = [2001:db8::1]Wildcard HTTP listeners should be configured through
*.http-bind-host.This reverses the two bare-literal cases introduced by HDDS-15773; the corresponding tests now assert that those configurations are rejected.
Generated-by: Claude Code (Opus 5)