chihsuan commented on code in PR #11108:
URL: https://github.com/apache/ozone/pull/11108#discussion_r3862799239
##########
hadoop-ozone/ozonefs-common/src/test/java/org/apache/hadoop/fs/ozone/TestBasicOzoneFileSystems.java:
##########
@@ -178,6 +184,98 @@ public void testRootedAuthorityParsing(String uri, String
expectedHost,
assertEquals(new URI(uri).getAuthority(), ofs.getUri().getAuthority());
}
+ @Test
+ public void testRootedIpv6UriConstructionAndRoundTrip() throws Exception {
+ URI uri = new URI("ofs", null, "2001:db8::10", 9862, "/", null, null);
+ assertEquals("ofs://[2001:db8::10]:9862/", uri.toString());
+
+ BasicRootedOzoneFileSystem ofs = spy(new BasicRootedOzoneFileSystem());
+ BasicRootedOzoneClientAdapterImpl adapter =
mock(BasicRootedOzoneClientAdapterImpl.class);
+ doReturn(adapter).when(ofs).createAdapter(any(), anyString(), anyInt());
+ ofs.initialize(uri, new OzoneConfiguration());
+
+ Path qualified = ofs.makeQualified(new Path("/volume/bucket/key"));
+ assertEquals("ofs://[2001:db8::10]:9862/volume/bucket/key",
qualified.toString());
+ assertEquals(qualified, new Path(qualified.toUri()));
+ }
+
+ @ParameterizedTest
+ @CsvSource({
+ "ofs://2001:db8::10/",
+ "ofs://2001:db8::10:9862/"
+ })
+ public void testRootedAuthorityRejectsUnbracketedIpv6(String uri) throws
Exception {
+ BasicRootedOzoneFileSystem ofs = new BasicRootedOzoneFileSystem();
+ IllegalArgumentException exception =
assertThrows(IllegalArgumentException.class,
+ () -> ofs.initialize(new URI(uri), new OzoneConfiguration()));
+ assertTrue(exception.getMessage().contains("must be enclosed in
brackets"));
+ }
+
+ @ParameterizedTest
+ @CsvSource(value = {
+ "o3fs://bucket.volume/, NULL, -1",
+ "o3fs://bucket.volume.omservice1/, omservice1, -1",
+ "o3fs://bucket.volume.om.example.com:9862/, om.example.com, 9862",
+ "o3fs://bucket.volume.192.0.2.1:9862/, 192.0.2.1, 9862"
+ }, nullValues = "NULL")
+ public void testO3fsAuthorityParsing(String uri, String expectedHost, int
expectedPort) throws Exception {
+ BasicOzoneFileSystem o3fs = spy(new BasicOzoneFileSystem());
+ BasicOzoneClientAdapterImpl adapter =
mock(BasicOzoneClientAdapterImpl.class);
+ doReturn(adapter).when(o3fs).createAdapter(any(), anyString(),
anyString(), nullable(String.class), anyInt());
+
+ o3fs.initialize(new URI(uri), new OzoneConfiguration());
+
+ ArgumentCaptor<String> hostCaptor = ArgumentCaptor.forClass(String.class);
+ ArgumentCaptor<Integer> portCaptor =
ArgumentCaptor.forClass(Integer.class);
+ verify(o3fs).createAdapter(any(), anyString(), anyString(),
hostCaptor.capture(), portCaptor.capture());
+ assertEquals(expectedHost, hostCaptor.getValue());
+ assertEquals(expectedPort, portCaptor.getValue().intValue());
+ assertEquals(new URI(uri).getAuthority(), o3fs.getUri().getAuthority());
+ }
+
+ @Test
+ public void testO3fsConfiguredIpv6Endpoint() throws Exception {
+ OzoneConfiguration conf = new OzoneConfiguration();
+ conf.set(OMConfigKeys.OZONE_OM_ADDRESS_KEY,
+ "[2001:db8::10]:9862");
Review Comment:
nit: these lines fit within the 120-character limit. Could remain on one
line, consistent with the test above?
##########
hadoop-ozone/ozonefs-common/src/test/java/org/apache/hadoop/fs/ozone/TestBasicOzoneFileSystems.java:
##########
@@ -178,6 +184,98 @@ public void testRootedAuthorityParsing(String uri, String
expectedHost,
assertEquals(new URI(uri).getAuthority(), ofs.getUri().getAuthority());
}
+ @Test
+ public void testRootedIpv6UriConstructionAndRoundTrip() throws Exception {
+ URI uri = new URI("ofs", null, "2001:db8::10", 9862, "/", null, null);
+ assertEquals("ofs://[2001:db8::10]:9862/", uri.toString());
+
+ BasicRootedOzoneFileSystem ofs = spy(new BasicRootedOzoneFileSystem());
+ BasicRootedOzoneClientAdapterImpl adapter =
mock(BasicRootedOzoneClientAdapterImpl.class);
+ doReturn(adapter).when(ofs).createAdapter(any(), anyString(), anyInt());
+ ofs.initialize(uri, new OzoneConfiguration());
+
+ Path qualified = ofs.makeQualified(new Path("/volume/bucket/key"));
+ assertEquals("ofs://[2001:db8::10]:9862/volume/bucket/key",
qualified.toString());
+ assertEquals(qualified, new Path(qualified.toUri()));
+ }
+
+ @ParameterizedTest
+ @CsvSource({
+ "ofs://2001:db8::10/",
+ "ofs://2001:db8::10:9862/"
+ })
+ public void testRootedAuthorityRejectsUnbracketedIpv6(String uri) throws
Exception {
+ BasicRootedOzoneFileSystem ofs = new BasicRootedOzoneFileSystem();
+ IllegalArgumentException exception =
assertThrows(IllegalArgumentException.class,
+ () -> ofs.initialize(new URI(uri), new OzoneConfiguration()));
+ assertTrue(exception.getMessage().contains("must be enclosed in
brackets"));
+ }
+
+ @ParameterizedTest
+ @CsvSource(value = {
+ "o3fs://bucket.volume/, NULL, -1",
+ "o3fs://bucket.volume.omservice1/, omservice1, -1",
+ "o3fs://bucket.volume.om.example.com:9862/, om.example.com, 9862",
+ "o3fs://bucket.volume.192.0.2.1:9862/, 192.0.2.1, 9862"
+ }, nullValues = "NULL")
+ public void testO3fsAuthorityParsing(String uri, String expectedHost, int
expectedPort) throws Exception {
+ BasicOzoneFileSystem o3fs = spy(new BasicOzoneFileSystem());
+ BasicOzoneClientAdapterImpl adapter =
mock(BasicOzoneClientAdapterImpl.class);
+ doReturn(adapter).when(o3fs).createAdapter(any(), anyString(),
anyString(), nullable(String.class), anyInt());
+
+ o3fs.initialize(new URI(uri), new OzoneConfiguration());
+
+ ArgumentCaptor<String> hostCaptor = ArgumentCaptor.forClass(String.class);
+ ArgumentCaptor<Integer> portCaptor =
ArgumentCaptor.forClass(Integer.class);
+ verify(o3fs).createAdapter(any(), anyString(), anyString(),
hostCaptor.capture(), portCaptor.capture());
+ assertEquals(expectedHost, hostCaptor.getValue());
+ assertEquals(expectedPort, portCaptor.getValue().intValue());
+ assertEquals(new URI(uri).getAuthority(), o3fs.getUri().getAuthority());
+ }
+
+ @Test
+ public void testO3fsConfiguredIpv6Endpoint() throws Exception {
+ OzoneConfiguration conf = new OzoneConfiguration();
+ conf.set(OMConfigKeys.OZONE_OM_ADDRESS_KEY,
+ "[2001:db8::10]:9862");
+ BasicOzoneFileSystem o3fs = spy(new BasicOzoneFileSystem());
+ BasicOzoneClientAdapterImpl adapter =
+ mock(BasicOzoneClientAdapterImpl.class);
+ doReturn(adapter).when(o3fs).createAdapter(any(), anyString(), anyString(),
+ nullable(String.class), anyInt());
+
+ o3fs.initialize(new URI("o3fs://bucket.volume/"), conf);
+
+ ArgumentCaptor<ConfigurationSource> confCaptor =
+ ArgumentCaptor.forClass(ConfigurationSource.class);
+ ArgumentCaptor<String> hostCaptor = ArgumentCaptor.forClass(String.class);
+ verify(o3fs).createAdapter(confCaptor.capture(), anyString(), anyString(),
+ hostCaptor.capture(), anyInt());
+ assertNull(hostCaptor.getValue());
+ assertEquals("[2001:db8::10]:9862",
+ OmUtils.getOmRpcAddress(confCaptor.getValue()));
Review Comment:
This assertion tests `OmUtils`, which `TestOmUtils` already covers. I wonder
if we should instead assert what `createAdapter` receives to keep this test
focused on O3FS behavior?
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicOzoneFileSystem.java:
##########
@@ -153,6 +156,10 @@ public void initialize(URI name, Configuration conf)
throws IOException {
throw new IllegalArgumentException(URI_EXCEPTION_TEXT);
}
+ if (authority.indexOf(':') != authority.lastIndexOf(':')) {
Review Comment:
This treats any authority with multiple colons as IPv6, so malformed
non-IPv6 authorities now get the IPv6-specific error. Could we classify it as
IPv6 only after validating the authority shape?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
