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..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 @@ -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,144 @@ 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 IP literal, bracketed or not + * @throws ConfigurationException if the host cannot be advertised + */ + public static void validateAdvertisedHost(String key, String host) { + if (host == null || host.isEmpty()) { + return; + } + + // 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(literal)) { + return; + } + + 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 " + + "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) { + // 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); + } + } + } + + /** + * 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 lastColon = host.lastIndexOf(':'); + if (lastColon < 0) { + return; + } + + 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 " + + "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 +480,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 +506,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..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 @@ -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, @@ -126,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 8fdf6de7b50c..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 @@ -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,161 @@ 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", + "[::]", "[fe80::1]", "[fe80::1%eth0]"}) + 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); + } + + /** + * 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 8706f0f7f035..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 @@ -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,66 @@ 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()); + } + + @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/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..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 @@ -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 testGetSCMAddressForDatanodesRejectsWildcardSCMName() { + 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 testGetSCMAddressForDatanodesRejectsWildcardHASCMAddress() { + 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 testGetReconAddressForDatanodesRejectsWildcardAddress() { + 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 testGetReconAddressForDatanodesAcceptsHostname() { + 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);