HDDS-15774. Make SCM Ratis roles string parsing IPv6-safe by AdyChechani · Pull Request #10831 · apache/ozone

AdyChechani · 2026-07-21T19:55:35Z

Generated-by: Claude Code (Opus 4.6)

What changes were proposed in this pull request?

SCMRatisServerImpl.getRatisRoles() encodes each peer as a colon-delimited string host:ratisPort:ROLE:uuid:hostIP. Five consumers split this string on : with hardcoded indices. When the host or hostIP is an IPv6 literal (e.g. 2001:db8::1), the extra colons shatter the split, parts[2] is no longer LEADER/FOLLOWER, and downstream code crashes or silently produces wrong results.

This PR fixes both the producer and all consumers:

Producer (SCMRatisServerImpl.getRatisRoles()):
- Normalizes peer.getAddress() through HddsUtils.getHostName() / getHostPort() / getHostPortString(), which brackets IPv6 hosts (e.g. [2001:db8::1]:9894).
- Brackets peerInetAddress.getHostAddress() when it contains colons.

The resulting string for IPv6 is now: [2001:db8::1]:9894:LEADER:peer1:[2001:db8:0:0:0:0:0:1]

Shared parser (HddsUtils.parseRatisRoleString()):
- New utility that parses the role string from right to left, handling bracketed IPv6 in both the host:port prefix and the hostIP suffix. Uses Guava HostAndPort for the host:port portion. Returns a 5-element String[]: [host, port, role, id, hostIP].
Consumers (all 4 sites):
- StorageContainerManager.getScmRatisRoles() — replaced role.split(":") with HddsUtils.parseRatisRoleString(role).
- GetScmRatisRolesSubcommand (table and JSON paths) — same replacement.
- StorageContainerServiceProviderImpl.getSCMDBSnapshot() (Recon) — same.
- SafeModeCheckSubcommand.findLeaderNode() — same, plus fixed node address comparison to use HddsUtils.getHostName() instead of split(":")[0].

No .proto schema change is needed: the wire field remains repeated string peerRoles in GetScmInfoResponseProto. Only the string content changes (IPv6 literals are now bracketed), which is transparent to protobuf.

What is the link to the Apache JIRA

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

How was this patch tested?

Added TestHddsUtils#testParseRatisRoleStringIPv4, testParseRatisRoleStringIPv6, testParseRatisRoleStringIPv6Follower, and testParseRatisRoleStringEmptyHostIp covering the parser with IPv4, IPv6 (leader/follower), and edge cases.
Added TestSCMRatisServerImpl#testGetRatisRolesWithIPv6, end-to-end test that creates a mock RaftPeer with a bracketed IPv6 address and verifies the produced string parses correctly.
Added TestGetScmRatisRolesSubcommand#testGetScmHARatisRolesIPv6, verifies the CLI --table output renders correctly for IPv6 role strings.
Existing testGetScmHARatisRoles (IPv4) continues to pass unchanged.
checkstyle.sh, rat.sh, and author.sh pass locally
mvn clean install -DskipTests succeeds

chihsuan

Thanks for the fix! @AdyChechani

I think structured representation would be the proper long-term fix (mentioned in Jira), but it requires broader API changes, so the shared-parser approach looks like a good scoped fix for now.

I found a few issues that might be worth addressing and left inline comments.

chihsuan · 2026-07-22T12:18:13Z

+
+    // Field 4: id (uuid or peer id, no colons)
+    int sep3 = roleString.lastIndexOf(':', idx - 1);
+    String id = roleString.substring(sep3 + 1, idx);


Could we validate the input before calling substring()? Inputs such as "" or "host:9894" currently produce StringIndexOutOfBoundsException but older SCMs can actually emit.

chihsuan · 2026-07-22T12:19:54Z

+        String leaderIp = parts[4];
        for (SCMNodeInfo node : nodes) {
-          String nodeHost = node.getScmClientAddress().split(":")[0];
+          String nodeHost = HddsUtils.getHostName(node.getScmClientAddress()).orElse("");


This correctly extracts the IPv6 host, but matchesAddress() still parses its inputs with split(":", 2). Equivalent forms such as 2001:db8::1 and 2001:db8:0:0:0:0:0:1 therefore do not match. Could we make matchesAddress() IPv6-aware as part of this fix?

https://github.com/AdyChechani/ozone/blob/0530be6310670bea08e54d711d929b7214cc25b0/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/SafeModeCheckSubcommand.java#L189-L190

chihsuan · 2026-07-22T12:22:23Z

-        roleDetails.put("InetAddress", roles[4]);
-      }
-      allRoles.put(roles[0], roleDetails);
+      String[] fields = HddsUtils.parseRatisRoleString(role);


Should we preserve the existing host:port handling here? The previous code explicitly supported this non-Ratis/two-field response, but the new parser requires all five fields and crashes on "host:9894". This looks like an unintended behavior change.

chihsuan · 2026-07-22T12:23:21Z

-        if (roleItems.length < 2) {
-          err.println("Invalid response received for ScmRatisRoles.");
-        }
+        String[] roleItems = HddsUtils.parseRatisRoleString(role);


The table path previously handled short or invalid responses without throwing. Since the new parser requires five fields, should we preserve the existing validation/skip behavior here?

chihsuan · 2026-07-22T12:24:09Z

-      }
-      allRoles.put(roles[0], roleDetails);
+      String[] fields = HddsUtils.parseRatisRoleString(role);
+      roleDetails.put("address", fields[0] + ":" + fields[1]);


For IPv6 this produces an ambiguous address such as 2001:db8::1:9894. Could we use HddsUtils.getHostPortString(...) here so the JSON output remains [2001:db8::1]:9894?

chihsuan · 2026-07-22T12:26:57Z

+        continue;
+      }
+      String host = HddsUtils.getHostName(peer.getAddress()).orElse("");
+      int port = HddsUtils.getHostPort(peer.getAddress()).orElse(0);


Is a missing Ratis port valid here? Defaulting to 0 makes the response look valid and consumers may display it as the actual port. If the port is required, failing with a clear message may be safer than silently using 0.

chihsuan · 2026-07-22T12:27:45Z

+      String hostIp = "";
+      if (peerInetAddress != null) {
+        String rawIp = peerInetAddress.getHostAddress();
+        hostIp = rawIp.contains(":") ? "[" + rawIp + "]" : rawIp;


Small suggestion: since we already have the InetAddress, InetAddresses.toUriString(peerInetAddress) handles this directly—IPv4 stays unchanged and IPv6 is bracketed. That may make the formatting intent a little clearer.

smengcl · 2026-07-23T15:56:07Z

Let's fix HostAndPort class as well:

diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/HostAndPort.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/HostAndPort.java index ... .. ... --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/HostAndPort.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/HostAndPort.java @@ -18,6 +18,7 @@ package org.apache.hadoop.hdds.scm.net; import java.net.InetSocketAddress; +import org.apache.hadoop.hdds.HddsUtils; import org.apache.hadoop.net.NetUtils; public class HostAndPort { @@ -35,7 +36,7 @@ public class HostAndPort { public HostAndPort(String host, int port) { this.host = host; this.port = port; - this.hostAndPortString = host + ":" + port; + this.hostAndPortString = HddsUtils.getHostPortString(host, port); this.hash = host.hashCode() ^ Integer.hashCode(port); // TODO: HDDS-15533 change the address resolution logic and make this.address threadsafe. this.address = NetUtils.createSocketAddr(hostAndPortString);

Otherwise SCMNodeInfo.buildNodeInfo() can still produce invalid unbracketed authority.

HDDS-15774. Make SCM Ratis roles string parsing IPv6-safe

0530be6

chihsuan reviewed Jul 22, 2026

View reviewed changes

smengcl reviewed Jul 23, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

HDDS-15774. Make SCM Ratis roles string parsing IPv6-safe#10831

HDDS-15774. Make SCM Ratis roles string parsing IPv6-safe#10831
AdyChechani wants to merge 1 commit into
apache:masterfrom
AdyChechani:HDDS-15774

AdyChechani commented Jul 21, 2026

Uh oh!

chihsuan left a comment

Uh oh!

chihsuan Jul 22, 2026

Uh oh!

chihsuan Jul 22, 2026

Uh oh!

chihsuan Jul 22, 2026

Uh oh!

chihsuan Jul 22, 2026

Uh oh!

chihsuan Jul 22, 2026

Uh oh!

chihsuan Jul 22, 2026

Uh oh!

chihsuan Jul 22, 2026

Uh oh!

smengcl Jul 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

AdyChechani commented Jul 21, 2026

What changes were proposed in this pull request?

This PR fixes both the producer and all consumers:

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

chihsuan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants