From e7add3679ac8cbe300e7ab2358f23505a916a8a1 Mon Sep 17 00:00:00 2001 From: rjgoyln Date: Sat, 19 Sep 2026 21:34:50 +0800 Subject: [PATCH 1/7] HDDS-16308. Validate advertised addresses before startup, registration, 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. --- .../hdds/scm/client/TestHddsClientUtils.java | 40 +++-- .../org/apache/hadoop/hdds/HddsUtils.java | 121 ++++++++++++++ .../org/apache/hadoop/hdds/NodeDetails.java | 3 +- .../hadoop/hdds/scm/ha/SCMNodeInfo.java | 1 + .../org/apache/hadoop/hdds/TestHddsUtils.java | 148 ++++++++++++++++++ .../hadoop/hdds/scm/ha/TestSCMNodeInfo.java | 34 ++++ .../hadoop/hdds/utils/HddsServerUtil.java | 5 + .../hadoop/hdds/scm/ha/SCMHANodeDetails.java | 3 +- .../hadoop/hdds/scm/ha/SCMNodeDetails.java | 6 - .../hadoop/hdds/scm/TestHddsServerUtils.java | 50 ++++++ .../java/org/apache/hadoop/ozone/OmUtils.java | 4 + .../org/apache/hadoop/ozone/TestOmUtils.java | 37 +++++ .../ozone/om/helpers/TestOMNodeDetails.java | 19 +++ .../hadoop/ozone/om/ha/OMHANodeDetails.java | 4 +- 14 files changed, 457 insertions(+), 18 deletions(-) diff --git a/hadoop-hdds/client/src/test/java/org/apache/hadoop/hdds/scm/client/TestHddsClientUtils.java b/hadoop-hdds/client/src/test/java/org/apache/hadoop/hdds/scm/client/TestHddsClientUtils.java index 647ae727465a..98013d891c13 100644 --- a/hadoop-hdds/client/src/test/java/org/apache/hadoop/hdds/scm/client/TestHddsClientUtils.java +++ b/hadoop-hdds/client/src/test/java/org/apache/hadoop/hdds/scm/client/TestHddsClientUtils.java @@ -184,11 +184,6 @@ public void testClientFallbackToScmNamesWithPort() { @Test public void testClientAddressIPv6() { - // Bare IPv6 literal without port: port falls back to the default and the - // host must be re-bracketed before the address string is parsed. - checkScmClientAddr(OZONE_SCM_CLIENT_ADDRESS_KEY, "2001:db8::1", - "2001:db8:0:0:0:0:0:1", OZONE_SCM_CLIENT_PORT_DEFAULT); - // Bracketed IPv6 literal with explicit port. checkScmClientAddr(OZONE_SCM_CLIENT_ADDRESS_KEY, "[2001:db8::1]:9876", "2001:db8:0:0:0:0:0:1", 9876); @@ -199,12 +194,26 @@ public void testClientAddressIPv6() { "2001:db8:0:0:0:0:0:1", OZONE_SCM_CLIENT_PORT_DEFAULT); } + /** + * A bare literal used to resolve to the default port (HDDS-15773). It is + * rejected since HDDS-16308, because the same text also reads as a shorter + * host with the trailing group for a port. + */ @Test - public void testClientFallbackToScmNamesIPv6() { - // Bare IPv6 literal in ozone.scm.names. - checkScmClientAddr(OZONE_SCM_NAMES, "2001:db8::1", - "2001:db8:0:0:0:0:0:1", OZONE_SCM_CLIENT_PORT_DEFAULT); + public void testClientAddressRejectsBareIPv6Literal() { + final OzoneConfiguration conf = new OzoneConfiguration(); + conf.set(OZONE_SCM_CLIENT_ADDRESS_KEY, "2001:db8::1"); + + ConfigurationException e = assertThrows(ConfigurationException.class, + () -> HddsUtils.getScmAddressForClients(conf)); + + assertThat(e.getMessage()) + .contains(OZONE_SCM_CLIENT_ADDRESS_KEY) + .contains("[2001:db8::1]"); + } + @Test + public void testClientFallbackToScmNamesIPv6() { // On the ozone.scm.names fallback path an inline port is ignored and the // default client port is used instead (same semantics as // testClientFallbackToScmNamesWithPort). @@ -212,6 +221,19 @@ public void testClientFallbackToScmNamesIPv6() { "2001:db8:0:0:0:0:0:1", OZONE_SCM_CLIENT_PORT_DEFAULT); } + @Test + public void testClientFallbackToScmNamesRejectsBareIPv6Literal() { + final OzoneConfiguration conf = new OzoneConfiguration(); + conf.set(OZONE_SCM_NAMES, "2001:db8::1"); + + ConfigurationException e = assertThrows(ConfigurationException.class, + () -> HddsUtils.getScmAddressForClients(conf)); + + assertThat(e.getMessage()) + .contains(OZONE_SCM_NAMES) + .contains("[2001:db8::1]"); + } + @Test @SuppressWarnings("StringSplitter") public void testBlockClientFallbackToClientWithPort() { diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/HddsUtils.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/HddsUtils.java index b4bebeb8b27f..d4e09d618529 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/HddsUtils.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/HddsUtils.java @@ -34,6 +34,7 @@ import com.google.common.base.Preconditions; import com.google.common.net.HostAndPort; +import com.google.common.net.InetAddresses; import com.google.protobuf.InvalidProtocolBufferException; import com.google.protobuf.ServiceException; import jakarta.annotation.Nonnull; @@ -42,6 +43,7 @@ import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.lang.reflect.UndeclaredThrowableException; +import java.net.InetAddress; import java.net.InetSocketAddress; import java.net.UnknownHostException; import java.nio.file.Path; @@ -138,6 +140,8 @@ public static Collection getScmAddressForClients( String address = conf.getTrimmed(OZONE_SCM_CLIENT_ADDRESS_KEY); int port = -1; + validateAdvertisedAddress(OZONE_SCM_CLIENT_ADDRESS_KEY, address); + if (address == null) { // fall back to ozone.scm.names for non-ha Collection scmAddresses = @@ -155,6 +159,7 @@ public static Collection getScmAddressForClients( } address = scmAddresses.iterator().next(); + validateAdvertisedAddress(OZONE_SCM_NAMES, address); port = conf.getInt(OZONE_SCM_CLIENT_PORT_KEY, OZONE_SCM_CLIENT_PORT_DEFAULT); @@ -243,6 +248,116 @@ public static String getHostPortString(String host, int port) { return HostAndPort.fromParts(host, port).toString(); } + /** + * Rejects a host configured as an advertised endpoint that cannot identify + * this node to a peer. A wildcard ({@code 0.0.0.0}, {@code ::}) names every + * local interface instead of one reachable endpoint, a link-local literal is + * only meaningful on a single link, and the zone identifier of a scoped + * literal ({@code fe80::1%eth0}) names an interface on the host that wrote + * it, so it cannot be resolved by a peer nor encoded in the SAN extension of + * an X.509 certificate (see RFC-5280). A DNS name is accepted without being + * resolved, and so is loopback, which a single-host deployment legitimately + * advertises. + * + * @param key the property the host was configured under + * @param host a hostname or an unbracketed IP literal + * @throws ConfigurationException if the host cannot be advertised + */ + public static void validateAdvertisedHost(String key, String host) { + if (host == null || host.isEmpty()) { + return; + } + + // Judged on the text, because the scope makes the literal unresolvable + // here: InetAddresses.forString("fe80::1%eth0") throws unless this host + // happens to own an interface by that name. + if (host.indexOf('%') >= 0 || host.indexOf('/') >= 0) { + throw new ConfigurationException(String.format( + "%s = %s carries a zone identifier or prefix length, which cannot be advertised: it names an interface " + + "on this host, so a peer cannot resolve it and it cannot be encoded in an X.509 certificate.", + key, host)); + } + + if (!InetAddresses.isInetAddress(host)) { + return; + } + + final InetAddress address = InetAddresses.forString(host); + if (address.isAnyLocalAddress()) { + throw new ConfigurationException(String.format( + "%s = %s is a wildcard address, which cannot be advertised. Configure the address this node is reachable " + + "at, and listen on every interface through the matching bind host property.", + key, host)); + } + if (address.isLinkLocalAddress()) { + throw new ConfigurationException(String.format( + "%s = %s is a link-local address, which is only reachable on one link and cannot be advertised.", + key, host)); + } + } + + /** + * Rejects a configured advertised address, both for the textual form of its + * authority and for the host it names. + * + * @param key the property the address was configured under + * @param value host or host:port + * @throws ConfigurationException if the address cannot be advertised + * @see #validateAdvertisedHost(String, String) + */ + public static void validateAdvertisedAddress(String key, String value) { + validateHostPortAuthority(key, value); + getHostName(value).ifPresent(host -> validateAdvertisedHost(key, host)); + } + + /** + * Rejects an unbracketed IPv6 literal configured under a property that a port + * may follow. Both readings of {@code 2001:db8::1:9862} are valid IPv6 + * literals - the host {@code 2001:db8::1} on port 9862, or the whole literal + * on the property's default port - and nothing in the text tells them apart, + * so one of them is used silently. Brackets remove the ambiguity, and are + * needed for the single-colon form too, since a bare literal there is read as + * a host and a port. + * + * @param key the property the value was configured under + * @param value host or host:port + * @throws ConfigurationException if the host is an unbracketed IPv6 literal + */ + private static void validateHostPortAuthority(String key, String value) { + if (value == null || value.isEmpty() || value.startsWith("[")) { + return; + } + + final String host = HostAndPort.fromString(value).getHost(); + final int lastGroup = host.lastIndexOf(':'); + if (lastGroup < 0) { + return; + } + + final String shorterHost = host.substring(0, lastGroup); + final String trailingGroup = host.substring(lastGroup + 1); + if (isPortNumber(trailingGroup) && InetAddresses.isInetAddress(shorterHost)) { + throw new ConfigurationException(String.format( + "%s = %s is an unbracketed IPv6 literal. Write %s for host %s with port %s, or %s to use the whole " + + "literal as the host.", + key, value, getHostPortString(shorterHost, Integer.parseInt(trailingGroup)), shorterHost, trailingGroup, + HostAndPort.fromHost(host))); + } + throw new ConfigurationException(String.format( + "%s = %s is an unbracketed IPv6 literal. Write %s; a port may follow this property, so the host has to be " + + "bracketed.", + key, value, HostAndPort.fromHost(host))); + } + + private static boolean isPortNumber(String value) { + try { + final int port = Integer.parseInt(value); + return port > 0 && port <= 65535; + } catch (NumberFormatException e) { + return false; + } + } + /** * Parse a Ratis role string produced by * {@code SCMRatisServerImpl.getRatisRoles()} into its constituent fields. @@ -337,11 +452,14 @@ public static OptionalInt getNumberFromConfigKeys( * @return first port number component found from the given keys, or absent. * @throws IllegalArgumentException if any values are not in the 'host' * or host:port format. + * @throws ConfigurationException if any value holds an unbracketed IPv6 + * literal, which cannot be told apart from a host and a port. */ public static OptionalInt getPortNumberFromConfigKeys( ConfigurationSource conf, String... keys) { for (final String key : keys) { final String value = conf.getTrimmed(key); + validateHostPortAuthority(key, value); final OptionalInt hostPort = getHostPort(value); if (hostPort.isPresent()) { return hostPort; @@ -360,10 +478,13 @@ public static OptionalInt getPortNumberFromConfigKeys( * @return the hostname (NB: may not be a FQDN) * @throws UnknownHostException if the hdds.datanode.dns.interface * option is used and the hostname can not be determined + * @throws ConfigurationException if the configured hostname cannot be + * advertised to SCM */ public static String getHostName(ConfigurationSource conf) throws UnknownHostException { String name = conf.get(HDDS_DATANODE_HOST_NAME_KEY); + validateAdvertisedHost(HDDS_DATANODE_HOST_NAME_KEY, name); if (name == null) { String dnsInterface = conf.get( CommonConfigurationKeysPublic.HADOOP_SECURITY_DNS_INTERFACE_KEY); diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/NodeDetails.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/NodeDetails.java index ae871a8a86fd..93f9d62ca565 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/NodeDetails.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/NodeDetails.java @@ -111,7 +111,8 @@ public int getRatisPort() { } public String getRpcAddressString() { - return NetUtils.getHostPortString(getRpcAddress()); + final InetSocketAddress addr = getRpcAddress(); + return HddsUtils.getHostPortString(addr.getHostName(), addr.getPort()); } public String getHttpAddress() { diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMNodeInfo.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMNodeInfo.java index 98b138de1ce7..161103c5c245 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMNodeInfo.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMNodeInfo.java @@ -96,6 +96,7 @@ public static List buildNodeInfo(ConfigurationSource conf) { if (scmAddress == null) { throw new ConfigurationException(addressKey + "is not defined"); } + HddsUtils.validateAdvertisedHost(addressKey, scmAddress); // Get port from Address Key if defined, else fall back to port key. int scmClientPort = getPort(conf, scmServiceId, scmNodeId, diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/TestHddsUtils.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/TestHddsUtils.java index 8fdf6de7b50c..d4f484bdbc0a 100644 --- a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/TestHddsUtils.java +++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/TestHddsUtils.java @@ -17,9 +17,17 @@ package org.apache.hadoop.hdds; +import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_DATANODE_HOST_NAME_KEY; import static org.apache.hadoop.hdds.HddsUtils.processForLogging; +import static org.apache.hadoop.hdds.HddsUtils.validateAdvertisedAddress; +import static org.apache.hadoop.hdds.HddsUtils.validateAdvertisedHost; +import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_CLIENT_ADDRESS_KEY; +import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_CLIENT_BIND_HOST_KEY; import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_DATANODE_PORT_KEY; +import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_NAMES; import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_PIPELINE_OWNER_CONTAINER_COUNT; +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -30,6 +38,7 @@ import java.util.Optional; import java.util.OptionalInt; import org.apache.hadoop.fs.CommonConfigurationKeysPublic; +import org.apache.hadoop.hdds.conf.ConfigurationException; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.scm.ScmConfigKeys; import org.apache.hadoop.ozone.ha.ConfUtils; @@ -37,6 +46,7 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; +import org.junit.jupiter.params.provider.ValueSource; /** * Testing HddsUtils. @@ -249,4 +259,142 @@ void testRedactSensitivePropsForLogging() { /* Verify that non-sensitive properties retain their value */ assertEquals(processedConf.get("ozone.normal.config"), ORIGINAL_VALUE); } + + @ParameterizedTest + @MethodSource("ambiguousIPv6Authorities") + void getPortNumberFromConfigKeysRejectsAmbiguousIPv6Authority(String value, String host, String port) { + OzoneConfiguration conf = new OzoneConfiguration(); + conf.set(OZONE_SCM_CLIENT_ADDRESS_KEY, value); + + ConfigurationException e = assertThrows(ConfigurationException.class, + () -> HddsUtils.getPortNumberFromConfigKeys(conf, OZONE_SCM_CLIENT_ADDRESS_KEY)); + + // Both readings have to be spelled out, since the configured text does not + // say which one was meant. + assertThat(e.getMessage()) + .contains(OZONE_SCM_CLIENT_ADDRESS_KEY) + .contains(value) + .contains("[" + host + "]:" + port) + .contains("[" + value + "]"); + } + + static List ambiguousIPv6Authorities() { + return Arrays.asList( + Arguments.of("2001:db8::1:9862", "2001:db8::1", "9862"), + Arguments.of("fd00::1:2", "fd00::1", "2") + ); + } + + @ParameterizedTest + @ValueSource(strings = {"::1", "2001:db8::1", "2001:db8::1:abcd", "fe80::1%eth0"}) + void getPortNumberFromConfigKeysRejectsBareIPv6Literal(String value) { + OzoneConfiguration conf = new OzoneConfiguration(); + conf.set(OZONE_SCM_CLIENT_ADDRESS_KEY, value); + + ConfigurationException e = assertThrows(ConfigurationException.class, + () -> HddsUtils.getPortNumberFromConfigKeys(conf, OZONE_SCM_CLIENT_ADDRESS_KEY)); + + assertThat(e.getMessage()) + .contains(OZONE_SCM_CLIENT_ADDRESS_KEY) + .contains("[" + value + "]"); + } + + @ParameterizedTest + @MethodSource("unambiguousAuthorities") + void getPortNumberFromConfigKeysAcceptsUnambiguousAuthority(String value, OptionalInt expectedPort) { + OzoneConfiguration conf = new OzoneConfiguration(); + conf.set(OZONE_SCM_CLIENT_ADDRESS_KEY, value); + + assertEquals(expectedPort, + HddsUtils.getPortNumberFromConfigKeys(conf, OZONE_SCM_CLIENT_ADDRESS_KEY)); + } + + static List unambiguousAuthorities() { + return Arrays.asList( + Arguments.of("[2001:db8::1]:9862", OptionalInt.of(9862)), + Arguments.of("[2001:db8::1:9862]:9862", OptionalInt.of(9862)), + Arguments.of("[2001:db8::1]", OptionalInt.empty()), + Arguments.of("192.0.2.1:9862", OptionalInt.of(9862)), + Arguments.of("scm1.example.com:9862", OptionalInt.of(9862)), + Arguments.of("scm1.example.com", OptionalInt.empty()) + ); + } + + /** + * A bind host names no port, so the bracket rule must leave it alone: an + * explicit IPv6 listener is configured as a bare {@code ::}. + */ + @Test + void getHostNameFromConfigKeysAcceptsBareIPv6BindHost() { + OzoneConfiguration conf = new OzoneConfiguration(); + conf.set(OZONE_SCM_CLIENT_BIND_HOST_KEY, "::"); + + assertEquals(Optional.of("::"), + HddsUtils.getHostNameFromConfigKeys(conf, OZONE_SCM_CLIENT_BIND_HOST_KEY)); + } + + @ParameterizedTest + @ValueSource(strings = {"scm1.example.com", "192.0.2.1", "2001:db8::1", "localhost", "127.0.0.1", "::1"}) + void validateAdvertisedHostAcceptsReachableHost(String host) { + assertDoesNotThrow(() -> validateAdvertisedHost(OZONE_SCM_CLIENT_ADDRESS_KEY, host)); + } + + @ParameterizedTest + @ValueSource(strings = {"0.0.0.0", "::", "169.254.1.1", "fe80::1", "fe80::1%eth0", "2001:db8::1/64"}) + void validateAdvertisedHostRejectsUnadvertisableHost(String host) { + ConfigurationException e = assertThrows(ConfigurationException.class, + () -> validateAdvertisedHost(OZONE_SCM_CLIENT_ADDRESS_KEY, host)); + + assertThat(e.getMessage()) + .contains(OZONE_SCM_CLIENT_ADDRESS_KEY) + .contains(host); + } + + @ParameterizedTest + @ValueSource(strings = {"0.0.0.0:9860", "[::]:9860", "[fe80::1%eth0]:9860"}) + void validateAdvertisedAddressRejectsUnadvertisableHostWithPort(String value) { + assertThrows(ConfigurationException.class, + () -> validateAdvertisedAddress(OZONE_SCM_CLIENT_ADDRESS_KEY, value)); + } + + @Test + void getHostNameRejectsUnadvertisableDatanodeHostname() { + OzoneConfiguration conf = new OzoneConfiguration(); + conf.set(HDDS_DATANODE_HOST_NAME_KEY, "0.0.0.0"); + + ConfigurationException e = assertThrows(ConfigurationException.class, + () -> HddsUtils.getHostName(conf)); + + assertThat(e.getMessage()).contains(HDDS_DATANODE_HOST_NAME_KEY); + } + + @Test + void getHostNameAcceptsConfiguredDatanodeHostname() throws Exception { + OzoneConfiguration conf = new OzoneConfiguration(); + conf.set(HDDS_DATANODE_HOST_NAME_KEY, "dn1.example.com"); + + assertEquals("dn1.example.com", HddsUtils.getHostName(conf)); + } + + @Test + void getScmAddressForClientsRejectsUnadvertisableClientAddress() { + OzoneConfiguration conf = new OzoneConfiguration(); + conf.set(OZONE_SCM_CLIENT_ADDRESS_KEY, "0.0.0.0:9860"); + + ConfigurationException e = assertThrows(ConfigurationException.class, + () -> HddsUtils.getScmAddressForClients(conf)); + + assertThat(e.getMessage()).contains(OZONE_SCM_CLIENT_ADDRESS_KEY); + } + + @Test + void getScmAddressForClientsRejectsUnadvertisableScmName() { + OzoneConfiguration conf = new OzoneConfiguration(); + conf.set(OZONE_SCM_NAMES, "0.0.0.0"); + + ConfigurationException e = assertThrows(ConfigurationException.class, + () -> HddsUtils.getScmAddressForClients(conf)); + + assertThat(e.getMessage()).contains(OZONE_SCM_NAMES); + } } diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/scm/ha/TestSCMNodeInfo.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/scm/ha/TestSCMNodeInfo.java index 8706f0f7f035..69d3edd1518c 100644 --- a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/scm/ha/TestSCMNodeInfo.java +++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/scm/ha/TestSCMNodeInfo.java @@ -30,6 +30,7 @@ import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_SECURITY_SERVICE_ADDRESS_KEY; import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_SECURITY_SERVICE_PORT_DEFAULT; import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_SECURITY_SERVICE_PORT_KEY; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -162,4 +163,37 @@ public void testNonHAWithRestDefaults() { assertEquals("localhost:" + OZONE_SCM_DATANODE_PORT_DEFAULT, scmNodeInfos.get(0).getScmDatanodeAddress()); } + + @Test + public void testSCMHANodeInfoRejectsWildcardSCMAddress() { + for (String nodeId : nodes) { + conf.set(ConfUtils.addKeySuffixes(OZONE_SCM_ADDRESS_KEY, + scmServiceId, nodeId), "localhost"); + } + String addressKey = ConfUtils.addKeySuffixes(OZONE_SCM_ADDRESS_KEY, + scmServiceId, "scm1"); + conf.set(addressKey, "0.0.0.0"); + + ConfigurationException e = assertThrows(ConfigurationException.class, + () -> SCMNodeInfo.buildNodeInfo(conf)); + + assertThat(e.getMessage()).contains(addressKey).contains("0.0.0.0"); + } + + /** + * The SCM address property names a host and takes its ports from separate + * properties, so a bare IPv6 literal is unambiguous there. + */ + @Test + public void testSCMHANodeInfoAcceptsBareIPv6SCMAddress() { + for (String nodeId : nodes) { + conf.set(ConfUtils.addKeySuffixes(OZONE_SCM_ADDRESS_KEY, + scmServiceId, nodeId), "2001:db8::1"); + } + + List scmNodeInfos = SCMNodeInfo.buildNodeInfo(conf); + + assertEquals("[2001:db8::1]:" + OZONE_SCM_CLIENT_PORT_DEFAULT, + scmNodeInfos.get(0).getScmClientAddress()); + } } diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HddsServerUtil.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HddsServerUtil.java index 49a71025c76a..8019e7ca5259 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HddsServerUtil.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HddsServerUtil.java @@ -30,6 +30,8 @@ import static org.apache.hadoop.hdds.HddsUtils.getHostPort; import static org.apache.hadoop.hdds.HddsUtils.getPortNumberFromConfigKeys; import static org.apache.hadoop.hdds.HddsUtils.getScmServiceId; +import static org.apache.hadoop.hdds.HddsUtils.validateAdvertisedAddress; +import static org.apache.hadoop.hdds.HddsUtils.validateAdvertisedHost; import static org.apache.hadoop.hdds.recon.ReconConfigKeys.OZONE_RECON_ADDRESS_KEY; import static org.apache.hadoop.hdds.recon.ReconConfigKeys.OZONE_RECON_DATANODE_PORT_DEFAULT; import static org.apache.hadoop.hdds.scm.ScmConfigKeys.HDDS_DATANODE_DIR_KEY; @@ -897,6 +899,7 @@ public static Collection getSCMAddressForDatanodes(ConfigurationSou final Collection addresses = new HashSet<>(names.size()); for (String address : names) { + validateAdvertisedAddress(OZONE_SCM_NAMES, address); Optional hostname = getHostName(address); if (!hostname.isPresent()) { throw new IllegalArgumentException("Invalid hostname for SCM: " @@ -937,6 +940,7 @@ public static Collection> getSCMAddressForDatanodes( LOG.warn("The SCM address configuration {} is not defined, return nothing", addressKey); return null; } + validateAdvertisedHost(addressKey, scmAddress); int scmDatanodePort = SCMNodeInfo.getPort(conf, scmServiceId, scmNodeId, OZONE_SCM_DATANODE_ADDRESS_KEY, OZONE_SCM_DATANODE_PORT_KEY, @@ -959,6 +963,7 @@ public static HostAndPort getReconAddressForDatanodes( if (StringUtils.isEmpty(name)) { return null; } + validateAdvertisedAddress(OZONE_RECON_ADDRESS_KEY, name); Optional hostname = getHostName(name); if (!hostname.isPresent()) { throw new IllegalArgumentException("Invalid hostname for Recon: " diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMHANodeDetails.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMHANodeDetails.java index 60a4ca7325f1..7bb13ffc71df 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMHANodeDetails.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMHANodeDetails.java @@ -266,7 +266,8 @@ public static SCMHANodeDetails loadSCMHAConfig(OzoneConfiguration conf, LOG.info("Found matching SCM address with SCMServiceId: {}, " + "SCMNodeId: {}, RPC Address: {} and Ratis port: {}", localScmServiceId, localScmNodeId, - NetUtils.getHostPortString(localRpcAddress), localRatisPort); + HddsUtils.getHostPortString(localRpcAddress.getHostName(), localRpcAddress.getPort()), + localRatisPort); // Set SCM node specific config keys. ConfUtils.setNodeSpecificConfigs(nodeSpecificConfigKeys, conf, diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMNodeDetails.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMNodeDetails.java index 39fa7e97d039..d819b19432be 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMNodeDetails.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMNodeDetails.java @@ -19,7 +19,6 @@ import java.net.InetSocketAddress; import org.apache.hadoop.hdds.NodeDetails; -import org.apache.hadoop.net.NetUtils; /** * Construct SCM node details. @@ -147,11 +146,6 @@ public SCMNodeDetails build() { } } - @Override - public String getRpcAddressString() { - return NetUtils.getHostPortString(getRpcAddress()); - } - public InetSocketAddress getClientProtocolServerAddress() { return clientProtocolServerAddress; } diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/TestHddsServerUtils.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/TestHddsServerUtils.java index b19b59320c72..b01d2f7b4d8f 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/TestHddsServerUtils.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/TestHddsServerUtils.java @@ -17,6 +17,8 @@ package org.apache.hadoop.hdds.scm; +import static org.apache.hadoop.hdds.recon.ReconConfigKeys.OZONE_RECON_ADDRESS_KEY; +import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_ADDRESS_KEY; import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_CLIENT_ADDRESS_KEY; import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_DATANODE_ADDRESS_KEY; import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_DATANODE_ID_DIR; @@ -33,14 +35,17 @@ import java.net.InetAddress; import java.net.InetSocketAddress; import java.net.UnknownHostException; +import java.util.Collections; import java.util.concurrent.TimeUnit; import org.apache.commons.io.FileUtils; import org.apache.hadoop.hdds.HddsConfigKeys; +import org.apache.hadoop.hdds.conf.ConfigurationException; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.scm.ha.SCMNodeInfo; import org.apache.hadoop.hdds.server.ServerUtils; import org.apache.hadoop.hdds.utils.HddsServerUtil; import org.apache.hadoop.net.NetUtils; +import org.apache.hadoop.ozone.ha.ConfUtils; import org.apache.hadoop.test.PathUtils; import org.junit.jupiter.api.Test; @@ -265,4 +270,49 @@ public void testGetDatanodeIdFilePath() { FileUtils.deleteQuietly(metaDir); } } + + @Test + public void getSCMAddressForDatanodesRejectsWildcardSCMName() { + final OzoneConfiguration conf = new OzoneConfiguration(); + conf.set(OZONE_SCM_NAMES, "0.0.0.0"); + + ConfigurationException e = assertThrows(ConfigurationException.class, + () -> HddsServerUtil.getSCMAddressForDatanodes(conf)); + + assertThat(e.getMessage()).contains(OZONE_SCM_NAMES).contains("0.0.0.0"); + } + + @Test + public void getSCMAddressForDatanodesRejectsWildcardHASCMAddress() { + final OzoneConfiguration conf = new OzoneConfiguration(); + final String addressKey = ConfUtils.addKeySuffixes(OZONE_SCM_ADDRESS_KEY, + "scmservice", "scm1"); + conf.set(addressKey, "::"); + + ConfigurationException e = assertThrows(ConfigurationException.class, + () -> HddsServerUtil.getSCMAddressForDatanodes(conf, "scmservice", + Collections.singleton("scm1"))); + + assertThat(e.getMessage()).contains(addressKey).contains("::"); + } + + @Test + public void getReconAddressForDatanodesRejectsWildcardAddress() { + final OzoneConfiguration conf = new OzoneConfiguration(); + conf.set(OZONE_RECON_ADDRESS_KEY, "0.0.0.0:9891"); + + ConfigurationException e = assertThrows(ConfigurationException.class, + () -> HddsServerUtil.getReconAddressForDatanodes(conf)); + + assertThat(e.getMessage()).contains(OZONE_RECON_ADDRESS_KEY); + } + + @Test + public void getReconAddressForDatanodesAcceptsHostname() { + final OzoneConfiguration conf = new OzoneConfiguration(); + conf.set(OZONE_RECON_ADDRESS_KEY, "recon.example.com:9891"); + + assertEquals("recon.example.com:9891", + HddsServerUtil.getReconAddressForDatanodes(conf).getHostAndPortString()); + } } diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java index dc676300b423..7a4a5d30f703 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java @@ -21,6 +21,7 @@ import static org.apache.hadoop.hdds.HddsUtils.getHostNameFromConfigKeys; import static org.apache.hadoop.hdds.HddsUtils.getHostPortString; import static org.apache.hadoop.hdds.HddsUtils.getPortNumberFromConfigKeys; +import static org.apache.hadoop.hdds.HddsUtils.validateAdvertisedHost; import static org.apache.hadoop.ozone.OzoneConsts.DOUBLE_SLASH_OM_KEY_PREFIX; import static org.apache.hadoop.ozone.OzoneConsts.OM_KEY_PREFIX; import static org.apache.hadoop.ozone.OzoneConsts.OM_SNAPSHOT_INDICATOR; @@ -161,12 +162,15 @@ public static String getOmRpcAddress(ConfigurationSource conf) { * @param conf configuration * @param confKey configuration key to lookup address from * @return Target InetSocketAddress for the OM RPC server. + * @throws ConfigurationException if the configured host cannot be advertised + * to the other OMs of the service */ public static String getOmRpcAddress(ConfigurationSource conf, String confKey) { final Optional host = getHostNameFromConfigKeys(conf, confKey); if (host.isPresent()) { + validateAdvertisedHost(confKey, host.get()); return host.get() + ":" + getPortNumberFromConfigKeys(conf, confKey) .orElse(OZONE_OM_PORT_DEFAULT); } else { diff --git a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/TestOmUtils.java b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/TestOmUtils.java index 1521cdd24ee8..311857307af0 100644 --- a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/TestOmUtils.java +++ b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/TestOmUtils.java @@ -24,6 +24,7 @@ import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_ADDRESS_KEY; import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_INTERNAL_SERVICE_ID; import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_NODES_KEY; +import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_PORT_DEFAULT; import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_SERVICE_IDS_KEY; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -42,7 +43,9 @@ import java.util.Set; import java.util.TreeSet; import java.util.UUID; +import org.apache.hadoop.hdds.conf.ConfigurationException; import org.apache.hadoop.hdds.conf.OzoneConfiguration; +import org.apache.hadoop.ozone.ha.ConfUtils; import org.apache.hadoop.ozone.om.OMConfigKeys; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; @@ -403,4 +406,38 @@ public void testResolveOmHostAcceptsIpv6Literal() { } } } + + @Test + void getOmRpcAddressRejectsWildcardPeerAddress() { + OzoneConfiguration conf = new OzoneConfiguration(); + String rpcAddrKey = ConfUtils.addKeySuffixes(OZONE_OM_ADDRESS_KEY, + "omservice", "om1"); + conf.set(rpcAddrKey, "0.0.0.0"); + + ConfigurationException e = assertThrows(ConfigurationException.class, + () -> OmUtils.getOmRpcAddress(conf, rpcAddrKey)); + + assertThat(e.getMessage()).contains(rpcAddrKey).contains("0.0.0.0"); + } + + @Test + void getOmRpcAddressAcceptsPeerHostname() { + OzoneConfiguration conf = new OzoneConfiguration(); + String rpcAddrKey = ConfUtils.addKeySuffixes(OZONE_OM_ADDRESS_KEY, + "omservice", "om1"); + conf.set(rpcAddrKey, "om1.example.com"); + + assertEquals("om1.example.com:" + OZONE_OM_PORT_DEFAULT, + OmUtils.getOmRpcAddress(conf, rpcAddrKey)); + } + + /** + * The unsuffixed property ships as a wildcard and doubles as the non-HA bind + * address, so it is not an advertised-only property and stays accepted. + */ + @Test + void getOmRpcAddressKeepsWildcardDefault() { + assertEquals("0.0.0.0:" + OZONE_OM_PORT_DEFAULT, + OmUtils.getOmRpcAddress(new OzoneConfiguration())); + } } diff --git a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOMNodeDetails.java b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOMNodeDetails.java index 4b3eaef00183..46125e761ca3 100644 --- a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOMNodeDetails.java +++ b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOMNodeDetails.java @@ -450,4 +450,23 @@ public void testSetRatisAddress() { assertEquals("192.168.1.100", nodeDetails.getHostAddress()); assertEquals(9873, nodeDetails.getRatisPort()); } + + /** + * An advertised peer identity has to bracket an IPv6 literal, or a reader + * takes the last group of the address for the port. + */ + @Test + public void testRpcAddressStringBracketsIPv6Literal() { + OMNodeDetails nodeDetails = new OMNodeDetails.Builder() + .setOMServiceId(OM_SERVICE_ID) + .setOMNodeId(OM_NODE_ID) + .setRpcAddress(InetSocketAddress.createUnresolved("2001:db8::1", RPC_PORT)) + .setRatisPort(RATIS_PORT) + .setHttpAddress(HTTP_ADDRESS) + .setHttpsAddress(HTTPS_ADDRESS) + .setIsListener(false) + .build(); + + assertEquals("[2001:db8::1]:" + RPC_PORT, nodeDetails.getRpcAddressString()); + } } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ha/OMHANodeDetails.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ha/OMHANodeDetails.java index da0ac79be26f..7c68a51cc570 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ha/OMHANodeDetails.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ha/OMHANodeDetails.java @@ -35,6 +35,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import org.apache.hadoop.hdds.HddsUtils; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.net.NetUtils; import org.apache.hadoop.ozone.OmUtils; @@ -203,7 +204,8 @@ public static OMHANodeDetails loadOMHAConfig(OzoneConfiguration conf) { LOG.info("Found matching OM address with OMServiceId: {}, " + "OMNodeId: {}, RPC Address: {} ,Ratis port: {} and isListener: {}", localOMServiceId, localOMNodeId, - NetUtils.getHostPortString(localRpcAddress), localRatisPort, localIsListener); + HddsUtils.getHostPortString(localRpcAddress.getHostName(), localRpcAddress.getPort()), + localRatisPort, localIsListener); ConfUtils.setNodeSpecificConfigs(genericConfigKeys, conf, localOMServiceId, localOMNodeId, LOG); From 6d203d43e9ec3363e0a9dda6d64169e1dbe67f99 Mon Sep 17 00:00:00 2001 From: rjgoyln Date: Sat, 19 Sep 2026 21:55:55 +0800 Subject: [PATCH 2/7] HDDS-16308. Close the bracketed-wildcard bypass in advertised validation 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. --- .../org/apache/hadoop/hdds/HddsUtils.java | 52 ++++++++++++++----- .../hadoop/hdds/scm/ha/SCMNodeInfo.java | 5 ++ .../org/apache/hadoop/hdds/TestHddsUtils.java | 21 +++++++- .../hadoop/hdds/scm/ha/TestSCMNodeInfo.java | 29 +++++++++++ .../hadoop/hdds/scm/TestHddsServerUtils.java | 8 +-- 5 files changed, 98 insertions(+), 17 deletions(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/HddsUtils.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/HddsUtils.java index d4e09d618529..aeb090b7094f 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/HddsUtils.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/HddsUtils.java @@ -260,7 +260,7 @@ public static String getHostPortString(String host, int port) { * advertises. * * @param key the property the host was configured under - * @param host a hostname or an unbracketed IP literal + * @param host a hostname or an IP literal, bracketed or not * @throws ConfigurationException if the host cannot be advertised */ public static void validateAdvertisedHost(String key, String host) { @@ -268,21 +268,29 @@ public static void validateAdvertisedHost(String key, String host) { return; } - // Judged on the text, because the scope makes the literal unresolvable - // here: InetAddresses.forString("fe80::1%eth0") throws unless this host - // happens to own an interface by that name. - if (host.indexOf('%') >= 0 || host.indexOf('/') >= 0) { + // A host-only property can still be written with brackets, and the brackets + // are stripped on the way to the address, so unwrap them here or the + // literal inside escapes every check below. + final String literal = host.startsWith("[") && host.endsWith("]") + ? host.substring(1, host.length() - 1) + : host; + + // Judged on the text, like HddsServerUtil.isScopedOrMaskingIPv6Address, + // because the scope makes the literal unresolvable here: + // InetAddresses.forString("fe80::1%eth0") throws unless this host happens + // to own an interface by that name. + if (literal.indexOf('%') >= 0 || literal.indexOf('/') >= 0) { throw new ConfigurationException(String.format( "%s = %s carries a zone identifier or prefix length, which cannot be advertised: it names an interface " + "on this host, so a peer cannot resolve it and it cannot be encoded in an X.509 certificate.", key, host)); } - if (!InetAddresses.isInetAddress(host)) { + if (!InetAddresses.isInetAddress(literal)) { return; } - final InetAddress address = InetAddresses.forString(host); + final InetAddress address = InetAddresses.forString(literal); if (address.isAnyLocalAddress()) { throw new ConfigurationException(String.format( "%s = %s is a wildcard address, which cannot be advertised. Configure the address this node is reachable " @@ -306,8 +314,28 @@ public static void validateAdvertisedHost(String key, String host) { * @see #validateAdvertisedHost(String, String) */ public static void validateAdvertisedAddress(String key, String value) { - validateHostPortAuthority(key, value); + // The host first: a wildcard is rejected outright, so it must not be told + // to add brackets that leave it rejected anyway. getHostName(value).ifPresent(host -> validateAdvertisedHost(key, host)); + validateHostPortAuthority(key, value); + } + + /** + * Rejects an advertised address configured under any of the given properties. + * A property holding a comma-separated list is checked entry by entry, and a + * property that is not set is skipped. + * + * @param conf the configuration to read + * @param keys the properties to check + * @throws ConfigurationException if any configured address cannot be advertised + * @see #validateAdvertisedAddress(String, String) + */ + public static void validateAdvertisedAddressConfig(ConfigurationSource conf, String... keys) { + for (final String key : keys) { + for (final String value : conf.getTrimmedStringCollection(key)) { + validateAdvertisedAddress(key, value); + } + } } /** @@ -329,13 +357,13 @@ private static void validateHostPortAuthority(String key, String value) { } final String host = HostAndPort.fromString(value).getHost(); - final int lastGroup = host.lastIndexOf(':'); - if (lastGroup < 0) { + final int lastColon = host.lastIndexOf(':'); + if (lastColon < 0) { return; } - final String shorterHost = host.substring(0, lastGroup); - final String trailingGroup = host.substring(lastGroup + 1); + final String shorterHost = host.substring(0, lastColon); + final String trailingGroup = host.substring(lastColon + 1); if (isPortNumber(trailingGroup) && InetAddresses.isInetAddress(shorterHost)) { throw new ConfigurationException(String.format( "%s = %s is an unbracketed IPv6 literal. Write %s for host %s with port %s, or %s to use the whole " diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMNodeInfo.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMNodeInfo.java index 161103c5c245..a8403b5df720 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMNodeInfo.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMNodeInfo.java @@ -127,6 +127,11 @@ public static List buildNodeInfo(ConfigurationSource conf) { } else { scmServiceId = SCM_DUMMY_SERVICE_ID; + HddsUtils.validateAdvertisedAddressConfig(conf, + OZONE_SCM_CLIENT_ADDRESS_KEY, OZONE_SCM_BLOCK_CLIENT_ADDRESS_KEY, + OZONE_SCM_SECURITY_SERVICE_ADDRESS_KEY, OZONE_SCM_DATANODE_ADDRESS_KEY, + OZONE_SCM_NAMES); + // Following current approach of fall back to // OZONE_SCM_CLIENT_ADDRESS_KEY to figure out hostname. diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/TestHddsUtils.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/TestHddsUtils.java index d4f484bdbc0a..d9423a963dbf 100644 --- a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/TestHddsUtils.java +++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/TestHddsUtils.java @@ -340,7 +340,8 @@ void validateAdvertisedHostAcceptsReachableHost(String host) { } @ParameterizedTest - @ValueSource(strings = {"0.0.0.0", "::", "169.254.1.1", "fe80::1", "fe80::1%eth0", "2001:db8::1/64"}) + @ValueSource(strings = {"0.0.0.0", "::", "169.254.1.1", "fe80::1", "fe80::1%eth0", "2001:db8::1/64", + "[::]", "[fe80::1]", "[fe80::1%eth0]"}) void validateAdvertisedHostRejectsUnadvertisableHost(String host) { ConfigurationException e = assertThrows(ConfigurationException.class, () -> validateAdvertisedHost(OZONE_SCM_CLIENT_ADDRESS_KEY, host)); @@ -397,4 +398,22 @@ void getScmAddressForClientsRejectsUnadvertisableScmName() { assertThat(e.getMessage()).contains(OZONE_SCM_NAMES); } + + /** + * A host that can never be advertised has to be named as such, rather than + * be asked for brackets that leave it rejected anyway. + */ + @Test + void validateAdvertisedAddressReportsTheHostBeforeTheBrackets() { + ConfigurationException e = assertThrows(ConfigurationException.class, + () -> validateAdvertisedAddress(OZONE_SCM_CLIENT_ADDRESS_KEY, "::")); + + assertThat(e.getMessage()).contains("wildcard").doesNotContain("bracket"); + } + + @ParameterizedTest + @ValueSource(strings = {"scm1.example.com", "192.0.2.1", "[2001:db8::1]:9860", "127.0.0.1:9860"}) + void validateAdvertisedAddressAcceptsReachableAddress(String value) { + assertDoesNotThrow(() -> validateAdvertisedAddress(OZONE_SCM_CLIENT_ADDRESS_KEY, value)); + } } diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/scm/ha/TestSCMNodeInfo.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/scm/ha/TestSCMNodeInfo.java index 69d3edd1518c..6db5e7edfda4 100644 --- a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/scm/ha/TestSCMNodeInfo.java +++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/scm/ha/TestSCMNodeInfo.java @@ -196,4 +196,33 @@ public void testSCMHANodeInfoAcceptsBareIPv6SCMAddress() { assertEquals("[2001:db8::1]:" + OZONE_SCM_CLIENT_PORT_DEFAULT, scmNodeInfos.get(0).getScmClientAddress()); } + + @Test + public void testSCMHANodeInfoRejectsBracketedWildcardSCMAddress() { + for (String nodeId : nodes) { + conf.set(ConfUtils.addKeySuffixes(OZONE_SCM_ADDRESS_KEY, + scmServiceId, nodeId), "localhost"); + } + String addressKey = ConfUtils.addKeySuffixes(OZONE_SCM_ADDRESS_KEY, + scmServiceId, "scm1"); + conf.set(addressKey, "[::]"); + + ConfigurationException e = assertThrows(ConfigurationException.class, + () -> SCMNodeInfo.buildNodeInfo(conf)); + + assertThat(e.getMessage()).contains(addressKey).contains("[::]"); + } + + @Test + public void testNonHARejectsWildcardDatanodeAddress() { + OzoneConfiguration config = new OzoneConfiguration(); + config.set(OZONE_SCM_CLIENT_ADDRESS_KEY, "localhost"); + config.set(OZONE_SCM_DATANODE_ADDRESS_KEY, "0.0.0.0:9861"); + + ConfigurationException e = assertThrows(ConfigurationException.class, + () -> SCMNodeInfo.buildNodeInfo(config)); + + assertThat(e.getMessage()).contains(OZONE_SCM_DATANODE_ADDRESS_KEY) + .contains("0.0.0.0"); + } } diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/TestHddsServerUtils.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/TestHddsServerUtils.java index b01d2f7b4d8f..e452ef51dbb1 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/TestHddsServerUtils.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/TestHddsServerUtils.java @@ -272,7 +272,7 @@ public void testGetDatanodeIdFilePath() { } @Test - public void getSCMAddressForDatanodesRejectsWildcardSCMName() { + public void testGetSCMAddressForDatanodesRejectsWildcardSCMName() { final OzoneConfiguration conf = new OzoneConfiguration(); conf.set(OZONE_SCM_NAMES, "0.0.0.0"); @@ -283,7 +283,7 @@ public void getSCMAddressForDatanodesRejectsWildcardSCMName() { } @Test - public void getSCMAddressForDatanodesRejectsWildcardHASCMAddress() { + public void testGetSCMAddressForDatanodesRejectsWildcardHASCMAddress() { final OzoneConfiguration conf = new OzoneConfiguration(); final String addressKey = ConfUtils.addKeySuffixes(OZONE_SCM_ADDRESS_KEY, "scmservice", "scm1"); @@ -297,7 +297,7 @@ public void getSCMAddressForDatanodesRejectsWildcardHASCMAddress() { } @Test - public void getReconAddressForDatanodesRejectsWildcardAddress() { + public void testGetReconAddressForDatanodesRejectsWildcardAddress() { final OzoneConfiguration conf = new OzoneConfiguration(); conf.set(OZONE_RECON_ADDRESS_KEY, "0.0.0.0:9891"); @@ -308,7 +308,7 @@ public void getReconAddressForDatanodesRejectsWildcardAddress() { } @Test - public void getReconAddressForDatanodesAcceptsHostname() { + public void testGetReconAddressForDatanodesAcceptsHostname() { final OzoneConfiguration conf = new OzoneConfiguration(); conf.set(OZONE_RECON_ADDRESS_KEY, "recon.example.com:9891"); From 889dcb10e5ee1517928efd6d369e3740cfa5c415 Mon Sep 17 00:00:00 2001 From: rjgoyln Date: Sun, 4 Oct 2026 00:58:32 +0800 Subject: [PATCH 3/7] HDDS-16308. Keep server-owned RPC addresses out of the wildcard check A non-HA SCM rewrites its four *.address properties with its bound host once its RPC servers start, and the unsuffixed ozone.om.address is the non-HA OM's own RPC address with a wildcard default. A process that shares such a configuration, like `ozone local`, and a non-HA client relying on the default read the wildcard back and were rejected, so those properties keep only the bracket rule. A per-node OM address is advertised to the other OMs, so the tests that used 0.0.0.0 there to mean "this host" now use localhost. --- .../org/apache/hadoop/hdds/HddsUtils.java | 5 +++- .../hadoop/hdds/scm/ha/SCMNodeInfo.java | 8 +++---- .../org/apache/hadoop/hdds/TestHddsUtils.java | 12 ++++++---- .../hadoop/hdds/scm/ha/TestSCMNodeInfo.java | 23 +++++++++++++++---- .../java/org/apache/hadoop/ozone/OmUtils.java | 6 ++++- .../org/apache/hadoop/ozone/TestOmUtils.java | 6 ++--- ...pcOMFollowerReadFailoverProxyProvider.java | 2 +- .../om/ha/TestOMFailoverProxyProvider.java | 2 +- .../om/TestOzoneManagerConfiguration.java | 14 +++++------ 9 files changed, 52 insertions(+), 26 deletions(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/HddsUtils.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/HddsUtils.java index 1c27e954a1df..e9079b8b8bbe 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/HddsUtils.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/HddsUtils.java @@ -140,7 +140,10 @@ public static Collection getScmAddressForClients( String address = conf.getTrimmed(OZONE_SCM_CLIENT_ADDRESS_KEY); int port = -1; - validateAdvertisedAddress(OZONE_SCM_CLIENT_ADDRESS_KEY, address); + // Only the bracket rule applies: SCM rewrites this property with its + // bound host, a wildcard by default, and a process sharing its + // configuration reads that back here. + validateHostPortAuthority(OZONE_SCM_CLIENT_ADDRESS_KEY, address); if (address == null) { // fall back to ozone.scm.names for non-ha diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMNodeInfo.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMNodeInfo.java index a8403b5df720..f3a7b59a0e4b 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMNodeInfo.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMNodeInfo.java @@ -127,10 +127,10 @@ public static List buildNodeInfo(ConfigurationSource conf) { } else { scmServiceId = SCM_DUMMY_SERVICE_ID; - HddsUtils.validateAdvertisedAddressConfig(conf, - OZONE_SCM_CLIENT_ADDRESS_KEY, OZONE_SCM_BLOCK_CLIENT_ADDRESS_KEY, - OZONE_SCM_SECURITY_SERVICE_ADDRESS_KEY, OZONE_SCM_DATANODE_ADDRESS_KEY, - OZONE_SCM_NAMES); + // The *.address properties double as the non-HA SCM's listen addresses, + // which it rewrites with its bound host (a wildcard by default) once its + // RPC servers start, so only ozone.scm.names is checked for the host. + HddsUtils.validateAdvertisedAddressConfig(conf, OZONE_SCM_NAMES); // Following current approach of fall back to // OZONE_SCM_CLIENT_ADDRESS_KEY to figure out hostname. diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/TestHddsUtils.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/TestHddsUtils.java index 33d08b1b85e7..3abc8bc8aa89 100644 --- a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/TestHddsUtils.java +++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/TestHddsUtils.java @@ -393,15 +393,19 @@ void getHostNameAcceptsConfiguredDatanodeHostname() throws Exception { assertEquals("dn1.example.com", HddsUtils.getHostName(conf)); } + /** + * SCM rewrites its client address with the bound host, so a process that + * shares its configuration reads the wildcard back. + */ @Test - void getScmAddressForClientsRejectsUnadvertisableClientAddress() { + void getScmAddressForClientsAcceptsWildcardListenAddress() { OzoneConfiguration conf = new OzoneConfiguration(); conf.set(OZONE_SCM_CLIENT_ADDRESS_KEY, "0.0.0.0:9860"); - ConfigurationException e = assertThrows(ConfigurationException.class, - () -> HddsUtils.getScmAddressForClients(conf)); + InetSocketAddress addr = HddsUtils.getScmAddressForClients(conf).iterator().next(); - assertThat(e.getMessage()).contains(OZONE_SCM_CLIENT_ADDRESS_KEY); + assertEquals("0.0.0.0", addr.getHostString()); + assertEquals(9860, addr.getPort()); } @Test diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/scm/ha/TestSCMNodeInfo.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/scm/ha/TestSCMNodeInfo.java index 6db5e7edfda4..60111c2f3332 100644 --- a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/scm/ha/TestSCMNodeInfo.java +++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/scm/ha/TestSCMNodeInfo.java @@ -27,6 +27,7 @@ import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_DATANODE_ADDRESS_KEY; import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_DATANODE_PORT_DEFAULT; import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_DATANODE_PORT_KEY; +import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_NAMES; import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_SECURITY_SERVICE_ADDRESS_KEY; import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_SECURITY_SERVICE_PORT_DEFAULT; import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_SECURITY_SERVICE_PORT_KEY; @@ -213,16 +214,30 @@ public void testSCMHANodeInfoRejectsBracketedWildcardSCMAddress() { assertThat(e.getMessage()).contains(addressKey).contains("[::]"); } + /** + * A non-HA SCM rewrites its address properties with the bound host, so a + * process that shares its configuration reads the wildcard back. + */ @Test - public void testNonHARejectsWildcardDatanodeAddress() { + public void testNonHAAcceptsWildcardListenAddress() { OzoneConfiguration config = new OzoneConfiguration(); - config.set(OZONE_SCM_CLIENT_ADDRESS_KEY, "localhost"); + config.set(OZONE_SCM_CLIENT_ADDRESS_KEY, "0.0.0.0:9860"); config.set(OZONE_SCM_DATANODE_ADDRESS_KEY, "0.0.0.0:9861"); + List scmNodeInfos = SCMNodeInfo.buildNodeInfo(config); + + assertEquals("0.0.0.0:9860", scmNodeInfos.get(0).getScmClientAddress()); + assertEquals("0.0.0.0:9861", scmNodeInfos.get(0).getScmDatanodeAddress()); + } + + @Test + public void testNonHARejectsWildcardScmNames() { + OzoneConfiguration config = new OzoneConfiguration(); + config.set(OZONE_SCM_NAMES, "0.0.0.0"); + ConfigurationException e = assertThrows(ConfigurationException.class, () -> SCMNodeInfo.buildNodeInfo(config)); - assertThat(e.getMessage()).contains(OZONE_SCM_DATANODE_ADDRESS_KEY) - .contains("0.0.0.0"); + assertThat(e.getMessage()).contains(OZONE_SCM_NAMES).contains("0.0.0.0"); } } diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java index 53a6866ab92a..c4e9fe38b7ab 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java @@ -170,7 +170,11 @@ public static String getOmRpcAddress(ConfigurationSource conf, final Optional host = getHostNameFromConfigKeys(conf, confKey); if (host.isPresent()) { - validateAdvertisedHost(confKey, host.get()); + // Without a service and node suffix this is the non-HA OM's own RPC + // address, which defaults to a wildcard. + if (!OZONE_OM_ADDRESS_KEY.equals(confKey)) { + validateAdvertisedHost(confKey, host.get()); + } return getHostPortString(host.get(), getPortNumberFromConfigKeys(conf, confKey) .orElse(OZONE_OM_PORT_DEFAULT)); diff --git a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/TestOmUtils.java b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/TestOmUtils.java index a39142d98238..57ddf6414ef3 100644 --- a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/TestOmUtils.java +++ b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/TestOmUtils.java @@ -511,12 +511,12 @@ void getOmRpcAddressAcceptsPeerHostname() { } /** - * The unsuffixed property ships as a wildcard and doubles as the non-HA bind - * address, so it is not an advertised-only property and stays accepted. + * The non-HA client failover proxy reads the unsuffixed property, which is + * the non-HA OM's own RPC address and ships as a wildcard. */ @Test void getOmRpcAddressKeepsWildcardDefault() { assertEquals("0.0.0.0:" + OZONE_OM_PORT_DEFAULT, - OmUtils.getOmRpcAddress(new OzoneConfiguration())); + OmUtils.getOmRpcAddress(new OzoneConfiguration(), OZONE_OM_ADDRESS_KEY)); } } diff --git a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/ha/TestHadoopRpcOMFollowerReadFailoverProxyProvider.java b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/ha/TestHadoopRpcOMFollowerReadFailoverProxyProvider.java index 9c9f573eea55..b958da792eb9 100644 --- a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/ha/TestHadoopRpcOMFollowerReadFailoverProxyProvider.java +++ b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/ha/TestHadoopRpcOMFollowerReadFailoverProxyProvider.java @@ -456,7 +456,7 @@ private void setupProxyProvider(int omNodeCount, OzoneConfiguration config) thro for (int i = 0; i < omNodeCount; i++) { String nodeId = NODE_ID_BASE_STR + (i + 1); // 1-th indexed config.set(ConfUtils.addKeySuffixes(OZONE_OM_ADDRESS_KEY, OM_SERVICE_ID, - nodeId), "0.0.0.0:" + i); + nodeId), "localhost:" + i); allNodeIds.add(nodeId); omNodeIds[i] = nodeId; omNodeAnswers[i] = new OMAnswer(); diff --git a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/ha/TestOMFailoverProxyProvider.java b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/ha/TestOMFailoverProxyProvider.java index 4a91b44ee341..08085743270b 100644 --- a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/ha/TestOMFailoverProxyProvider.java +++ b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/ha/TestOMFailoverProxyProvider.java @@ -49,7 +49,7 @@ public class TestOMFailoverProxyProvider { private static final String OM_SERVICE_ID = "om-service-test1"; private static final String NODE_ID_BASE_STR = "omNode-"; - private static final String DUMMY_NODE_ADDR = "0.0.0.0:8080"; + private static final String DUMMY_NODE_ADDR = "localhost:8080"; private HadoopRpcOMFailoverProxyProvider provider; private long waitBetweenRetries; private int numNodes = 3; diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerConfiguration.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerConfiguration.java index d6b9d51e9219..17ffa2fef404 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerConfiguration.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerConfiguration.java @@ -112,7 +112,7 @@ public void testDefaultPortIfNotSpecified() throws Exception { String omNode1RpcAddrKey = getOMAddrKeyWithSuffix(serviceID, omNode1Id); String omNode2RpcAddrKey = getOMAddrKeyWithSuffix(serviceID, omNode2Id); - conf.set(omNode1RpcAddrKey, "0.0.0.0"); + conf.set(omNode1RpcAddrKey, "localhost"); conf.set(omNode2RpcAddrKey, "122.0.0.122"); // Set omNode1 as the current node. omNode1 address does not have a port @@ -121,7 +121,7 @@ public void testDefaultPortIfNotSpecified() throws Exception { startCluster(); OzoneManager om = cluster.getOzoneManager(); - assertEquals("0.0.0.0", + assertEquals("localhost", om.getOmRpcServerAddr().getHostName()); assertEquals(OMConfigKeys.OZONE_OM_PORT_DEFAULT, om.getOmRpcServerAddr().getPort()); @@ -188,7 +188,7 @@ public void testThreeNodeOMservice() throws Exception { // Set node2 to localhost and the other two nodes to dummy addresses conf.set(omNode1RpcAddrKey, "123.0.0.123:9862"); - conf.set(omNode2RpcAddrKey, "0.0.0.0:9862"); + conf.set(omNode2RpcAddrKey, "localhost:9862"); conf.set(omNode3RpcAddrKey, "124.0.0.124:9862"); conf.setInt(omNode3RatisPortKey, 9898); @@ -217,7 +217,7 @@ public void testThreeNodeOMservice() throws Exception { OMConfigKeys.OZONE_OM_RATIS_PORT_DEFAULT; break; case omNode2Id : - expectedPeerAddress = "0.0.0.0:" + + expectedPeerAddress = "localhost:" + OMConfigKeys.OZONE_OM_RATIS_PORT_DEFAULT; break; case omNode3Id : @@ -263,7 +263,7 @@ public void testOMHAWithUnresolvedAddresses() throws Exception { // Set node2 to localhost and the other two nodes to dummy addresses conf.set(omNode1RpcAddrKey, node1Hostname + ":9862"); - conf.set(omNode2RpcAddrKey, "0.0.0.0:9862"); + conf.set(omNode2RpcAddrKey, "localhost:9862"); conf.set(omNode3RpcAddrKey, node3Hostname + ":9804"); conf.setInt(omNode3RatisPortKey, 9898); @@ -300,7 +300,7 @@ public void testOMHAWithUnresolvedAddresses() throws Exception { OMConfigKeys.OZONE_OM_RATIS_PORT_DEFAULT; break; case omNode2Id : - expectedPeerAddress = "0.0.0.0:" + + expectedPeerAddress = "localhost:" + OMConfigKeys.OZONE_OM_RATIS_PORT_DEFAULT; break; case omNode3Id : @@ -425,7 +425,7 @@ public void testMultipleOMServiceIds() throws Exception { conf.set(getOMAddrKeyWithSuffix(om2ServiceId, omNode1Id), "125.0.0.126:9862"); conf.set(getOMAddrKeyWithSuffix(om2ServiceId, omNode2Id), - "0.0.0.0:9862"); + "localhost:9862"); conf.set(getOMAddrKeyWithSuffix(om2ServiceId, omNode3Id), "126.0.0.127:9862"); From 1b678d9923aaa63fb74f4e4bfbfb653c68fe6707 Mon Sep 17 00:00:00 2001 From: rjgoyln Date: Sun, 4 Oct 2026 01:05:02 +0800 Subject: [PATCH 4/7] HDDS-16308. Bracket the IPv6 peer address in the HDDS-16307 test HDDS-16307 added this case with a bare literal under a property a port may follow, which this change rejects as ambiguous. The test is about the emitted address being bracketed, which the bracketed input still covers. --- .../src/test/java/org/apache/hadoop/ozone/TestOmUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/TestOmUtils.java b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/TestOmUtils.java index 57ddf6414ef3..889d2d3b9500 100644 --- a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/TestOmUtils.java +++ b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/TestOmUtils.java @@ -457,7 +457,7 @@ void testOmAddressesBracketIPv6Literals() { assertEquals("[2001:db8::1]:9999", OmUtils.getOmRpcAddress(conf)); String peerAddressKey = OZONE_OM_ADDRESS_KEY + ".omservice.om1"; - conf.set(peerAddressKey, "2001:db8::2"); + conf.set(peerAddressKey, "[2001:db8::2]"); assertEquals("[2001:db8::2]:" + OMConfigKeys.OZONE_OM_PORT_DEFAULT, OmUtils.getOmRpcAddress(conf, peerAddressKey)); From e037cba6c947a0d3a361e388bc7a759092a66694 Mon Sep 17 00:00:00 2001 From: rjgoyln Date: Fri, 9 Oct 2026 19:20:27 +0800 Subject: [PATCH 5/7] HDDS-16308. Shut the datanode down on an unadvertisable SCM or Recon address InitDatanodeState only caught IllegalArgumentException, so the ConfigurationException the new validation raises escaped through the future and the datanode retried INIT on every heartbeat instead of shutting down the way it does for any other invalid SCM address. The Recon address was read outside that guard, with the same effect. --- .../common/states/datanode/InitDatanodeState.java | 8 +++++--- .../container/common/TestDatanodeStateMachine.java | 12 ++++++++++++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/states/datanode/InitDatanodeState.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/states/datanode/InitDatanodeState.java index d0c0a60b03ee..c8801ecc5c5c 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/states/datanode/InitDatanodeState.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/states/datanode/InitDatanodeState.java @@ -29,6 +29,7 @@ import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; +import org.apache.hadoop.hdds.conf.ConfigurationException; import org.apache.hadoop.hdds.conf.ConfigurationSource; import org.apache.hadoop.hdds.protocol.DatanodeDetails; import org.apache.hadoop.hdds.scm.net.HostAndPort; @@ -76,11 +77,13 @@ public InitDatanodeState(ConfigurationSource conf, @Override public DatanodeStateMachine.DatanodeStates call() throws Exception { final Collection addresses; + final HostAndPort reconAddress; try { addresses = HddsServerUtil.getSCMAddressForDatanodes(conf); - } catch (IllegalArgumentException e) { + reconAddress = getReconAddressForDatanodes(conf); + } catch (IllegalArgumentException | ConfigurationException e) { if (!Strings.isNullOrEmpty(e.getMessage())) { - LOG.error("Failed to get SCM addresses: {}", e.getMessage()); + LOG.error("Failed to get SCM or Recon addresses: {}", e.getMessage()); } return DatanodeStateMachine.DatanodeStates.SHUTDOWN; } @@ -103,7 +106,6 @@ public DatanodeStateMachine.DatanodeStates call() throws Exception { connectionManager.addSCMServer(addr, context.getThreadNamePrefix()); this.context.addEndpoint(addr); } - final HostAndPort reconAddress = getReconAddressForDatanodes(conf); if (reconAddress != null) { connectionManager.addReconServer(reconAddress, context.getThreadNamePrefix()); diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/TestDatanodeStateMachine.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/TestDatanodeStateMachine.java index 3db702ef3b51..2e568b2a7d28 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/TestDatanodeStateMachine.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/TestDatanodeStateMachine.java @@ -50,6 +50,7 @@ import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.conf.ReconfigurationHandler; import org.apache.hadoop.hdds.protocol.DatanodeDetails; +import org.apache.hadoop.hdds.recon.ReconConfigKeys; import org.apache.hadoop.hdds.scm.ScmConfigKeys; import org.apache.hadoop.hdds.upgrade.HDDSLayoutFeature; import org.apache.hadoop.ipc_.RPC; @@ -511,6 +512,17 @@ public void testDatanodeStateMachineWithInvalidConfiguration() /** Port out of range **/ confList.add(Maps.immutableEntry( ScmConfigKeys.OZONE_SCM_NAMES, "scm:123456")); + /** Unbracketed IPv6 literal **/ + confList.add(Maps.immutableEntry( + ScmConfigKeys.OZONE_SCM_NAMES, "::1")); + /** Wildcard **/ + confList.add(Maps.immutableEntry( + ScmConfigKeys.OZONE_SCM_NAMES, "0.0.0.0")); + + // Invalid ozone.recon.address + /** Wildcard **/ + confList.add(Maps.immutableEntry( + ReconConfigKeys.OZONE_RECON_ADDRESS_KEY, "0.0.0.0:9891")); confList.forEach((entry) -> { OzoneConfiguration perTestConf = new OzoneConfiguration(conf); From a52223dde8a96b247dcc9e50428d2c4fc075f609 Mon Sep 17 00:00:00 2001 From: rjgoyln Date: Fri, 9 Oct 2026 19:20:27 +0800 Subject: [PATCH 6/7] HDDS-16308. Validate SCM HA addresses in the startup loader SCM reads its per-node addresses through SCMHANodeDetails.loadSCMHAConfig, which skipped the check clients and datanodes apply through SCMNodeInfo.buildNodeInfo. A non-secure SCM therefore started with a wildcard or link-local identity that every peer then refused, leaving the misconfiguration to surface on the clients again. --- .../hadoop/hdds/scm/ha/SCMHANodeDetails.java | 1 + .../hdds/scm/ha/TestSCMConfiguration.java | 29 +++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMHANodeDetails.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMHANodeDetails.java index 7bb13ffc71df..50aea32c8b89 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMHANodeDetails.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMHANodeDetails.java @@ -215,6 +215,7 @@ public static SCMHANodeDetails loadSCMHAConfig(OzoneConfiguration conf, "%s. SCM RPC Address should be set for all nodes in a SCM " + "service.", rpcAddrKey); } + HddsUtils.validateAdvertisedHost(rpcAddrKey, rpcAddrStr); isSCMddressSet = true; String ratisPortKey = ConfUtils.addKeySuffixes(OZONE_SCM_RATIS_PORT_KEY, diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/ha/TestSCMConfiguration.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/ha/TestSCMConfiguration.java index 15715d72b618..17c743beeab5 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/ha/TestSCMConfiguration.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/ha/TestSCMConfiguration.java @@ -36,13 +36,16 @@ import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_SECURITY_SERVICE_BIND_HOST_KEY; import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_SECURITY_SERVICE_PORT_KEY; import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_METADATA_DIRS; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import java.io.File; import java.net.InetSocketAddress; import org.apache.hadoop.hdds.HddsConfigKeys; +import org.apache.hadoop.hdds.conf.ConfigurationException; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.scm.ScmConfigKeys; import org.apache.hadoop.hdds.scm.ScmRatisServerConfig; @@ -57,6 +60,8 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; /** * Test for SCM HA-related configuration. @@ -211,6 +216,30 @@ public void testSCMConfig() throws Exception { RaftServerConfigKeys.Log.Appender.WAIT_TIME_MIN_KEY); } + @ParameterizedTest + @CsvSource({"scm1, 0.0.0.0", "scm2, 169.254.1.1"}) + void testSCMConfigRejectsUnadvertisableAddress(String nodeId, String address) { + String scmServiceId = "scmserviceId"; + conf.set(ScmConfigKeys.OZONE_SCM_SERVICE_IDS_KEY, scmServiceId); + conf.set(ScmConfigKeys.OZONE_SCM_NODES_KEY + "." + scmServiceId, + "scm1,scm2,scm3"); + conf.set(ScmConfigKeys.OZONE_SCM_NODE_ID_KEY, "scm1"); + for (String node : new String[] {"scm1", "scm2", "scm3"}) { + conf.set(ConfUtils.addKeySuffixes(OZONE_SCM_ADDRESS_KEY, scmServiceId, + node), "localhost"); + } + String addressKey = ConfUtils.addKeySuffixes(OZONE_SCM_ADDRESS_KEY, + scmServiceId, nodeId); + conf.set(addressKey, address); + SCMStorageConfig scmStorageConfig = mock(SCMStorageConfig.class); + when(scmStorageConfig.getState()).thenReturn(Storage.StorageState.NOT_INITIALIZED); + + ConfigurationException e = assertThrows(ConfigurationException.class, + () -> SCMHANodeDetails.loadSCMHAConfig(conf, scmStorageConfig)); + + assertThat(e.getMessage()).contains(addressKey).contains(address); + } + @Test public void testSamePortConfig() throws Exception { String scmServiceId = "scmserviceId"; From 58b4085d890935295fc617ffe0910671f522fb83 Mon Sep 17 00:00:00 2001 From: rjgoyln Date: Fri, 9 Oct 2026 19:20:27 +0800 Subject: [PATCH 7/7] HDDS-16308. Validate the Recon advertised address at startup Recon only read ozone.recon.address during Kerberos login, so a non-secure Recon started with a wildcard address while every datanode shut down on it. This is the same asymmetry as the SCM HA loader. --- .../org/apache/hadoop/hdds/TestHddsUtils.java | 19 +++++++++++++++++++ .../hadoop/ozone/recon/ReconServer.java | 2 ++ 2 files changed, 21 insertions(+) diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/TestHddsUtils.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/TestHddsUtils.java index 3abc8bc8aa89..fc2da4832a65 100644 --- a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/TestHddsUtils.java +++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/TestHddsUtils.java @@ -22,6 +22,7 @@ import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_DATANODE_HOST_NAME_KEY; import static org.apache.hadoop.hdds.HddsUtils.processForLogging; import static org.apache.hadoop.hdds.HddsUtils.validateAdvertisedAddress; +import static org.apache.hadoop.hdds.HddsUtils.validateAdvertisedAddressConfig; import static org.apache.hadoop.hdds.HddsUtils.validateAdvertisedHost; import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_CLIENT_ADDRESS_KEY; import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_CLIENT_BIND_HOST_KEY; @@ -436,4 +437,22 @@ void validateAdvertisedAddressReportsTheHostBeforeTheBrackets() { void validateAdvertisedAddressAcceptsReachableAddress(String value) { assertDoesNotThrow(() -> validateAdvertisedAddress(OZONE_SCM_CLIENT_ADDRESS_KEY, value)); } + + @Test + void validateAdvertisedAddressConfigSkipsUnsetProperty() { + OzoneConfiguration conf = new OzoneConfiguration(); + + assertDoesNotThrow(() -> validateAdvertisedAddressConfig(conf, OZONE_SCM_CLIENT_ADDRESS_KEY)); + } + + @Test + void validateAdvertisedAddressConfigChecksEveryListEntry() { + OzoneConfiguration conf = new OzoneConfiguration(); + conf.set(OZONE_SCM_NAMES, "scm1.example.com,0.0.0.0"); + + ConfigurationException e = assertThrows(ConfigurationException.class, + () -> validateAdvertisedAddressConfig(conf, OZONE_SCM_CLIENT_ADDRESS_KEY, OZONE_SCM_NAMES)); + + assertThat(e.getMessage()).contains(OZONE_SCM_NAMES).contains("0.0.0.0"); + } } diff --git a/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/ReconServer.java b/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/ReconServer.java index 07956814d4f6..d3719adfe080 100644 --- a/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/ReconServer.java +++ b/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/ReconServer.java @@ -34,6 +34,7 @@ import java.util.concurrent.Callable; import java.util.concurrent.atomic.AtomicBoolean; import javax.sql.DataSource; +import org.apache.hadoop.hdds.HddsUtils; import org.apache.hadoop.hdds.cli.GenericCli; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.protocolPB.SCMSecurityProtocolClientSideTranslatorPB; @@ -113,6 +114,7 @@ public Void call() throws Exception { HddsServerUtil.startupShutdownMessage(OzoneVersionInfo.OZONE_VERSION_INFO, ReconServer.class, originalArgs, LOG, configuration); ConfigurationProvider.setConfiguration(configuration); + HddsUtils.validateAdvertisedAddressConfig(configuration, ReconConfigKeys.OZONE_RECON_ADDRESS_KEY); try { reconAdmins = createReconAdmins(configuration);