Copilot commented on code in PR #8744: URL: https://github.com/apache/hadoop/pull/8744#discussion_r4039112665
########## hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/oncrpc/TestSimpleServerBind.java: ########## @@ -0,0 +1,151 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.oncrpc; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.net.InetSocketAddress; +import java.util.Random; + +import io.netty.buffer.ByteBuf; +import io.netty.buffer.Unpooled; +import io.netty.channel.ChannelHandler; +import io.netty.channel.ChannelHandlerContext; +import org.apache.hadoop.oncrpc.security.VerifierNone; +import org.junit.jupiter.api.Test; + +/** + * Tests that {@link SimpleTcpServer} and {@link SimpleUdpServer} bind to the + * address specified at construction time. + */ +public class TestSimpleServerBind { + + @ChannelHandler.Sharable + static class NoopRpcProgram extends RpcProgram { + NoopRpcProgram(int port) { + super("noop", "localhost", port, 100001, 1, 1, null, true); + } + + @Override + protected void handleInternal(ChannelHandlerContext ctx, RpcInfo info) { + RpcCall rpcCall = (RpcCall) info.header(); + RpcAcceptedReply reply = + RpcAcceptedReply.getAcceptInstance(rpcCall.getXid(), new VerifierNone()); + XDR out = new XDR(); + reply.write(out); + ByteBuf b = Unpooled.wrappedBuffer(out.asReadOnlyWrap().buffer()); + RpcUtil.sendRpcResponse(ctx, new RpcResponse(b, info.remoteAddress())); + } + + @Override + protected boolean isIdempotent(RpcCall call) { + return false; + } + } + + private static int randomPort() { + return 20000 + new Random().nextInt(10000); Review Comment: This helper chooses a random port from a fixed range without checking whether it is already occupied or retrying. A concurrent test or another process can therefore make this new test class fail intermittently; use port 0 so the OS selects a free port, or add the retry pattern used by TestFrameDecoder. ########## hadoop-hdfs-project/hadoop-hdfs-nfs/src/test/java/org/apache/hadoop/hdfs/nfs/nfs3/TestNfsBindConfiguration.java: ########## @@ -0,0 +1,58 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hdfs.nfs.nfs3; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import org.apache.hadoop.hdfs.nfs.conf.NfsConfigKeys; +import org.apache.hadoop.hdfs.nfs.conf.NfsConfiguration; +import org.junit.jupiter.api.Test; + +/** + * Tests for the NFS gateway bind host configuration option + * ({@value NfsConfigKeys#DFS_NFS_SERVER_BIND_HOST_KEY}). + */ +public class TestNfsBindConfiguration { + + @Test + public void testDefaultBindHostIsAllInterfaces() { + assertEquals("0.0.0.0", NfsConfigKeys.DFS_NFS_SERVER_BIND_HOST_DEFAULT); + } + + @Test + public void testConfigKeyName() { + assertEquals("nfs.server.bind.host", NfsConfigKeys.DFS_NFS_SERVER_BIND_HOST_KEY); + } + + @Test + public void testNfsConfigurationReturnsDefault() { + NfsConfiguration conf = new NfsConfiguration(); + String bindHost = conf.get(NfsConfigKeys.DFS_NFS_SERVER_BIND_HOST_KEY, + NfsConfigKeys.DFS_NFS_SERVER_BIND_HOST_DEFAULT); Review Comment: Because this call supplies `DFS_NFS_SERVER_BIND_HOST_DEFAULT` explicitly, the test still passes if `hdfs-default.xml` omits the new property or `NfsConfiguration` does not load it. Read the key without a fallback so this test verifies the shipped resource default. ########## hadoop-hdfs-project/hadoop-hdfs-nfs/src/test/java/org/apache/hadoop/hdfs/nfs/nfs3/TestNfsBindConfiguration.java: ########## @@ -0,0 +1,58 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hdfs.nfs.nfs3; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import org.apache.hadoop.hdfs.nfs.conf.NfsConfigKeys; +import org.apache.hadoop.hdfs.nfs.conf.NfsConfiguration; +import org.junit.jupiter.api.Test; + +/** + * Tests for the NFS gateway bind host configuration option + * ({@value NfsConfigKeys#DFS_NFS_SERVER_BIND_HOST_KEY}). + */ +public class TestNfsBindConfiguration { + + @Test + public void testDefaultBindHostIsAllInterfaces() { + assertEquals("0.0.0.0", NfsConfigKeys.DFS_NFS_SERVER_BIND_HOST_DEFAULT); + } + + @Test + public void testConfigKeyName() { + assertEquals("nfs.server.bind.host", NfsConfigKeys.DFS_NFS_SERVER_BIND_HOST_KEY); + } + + @Test + public void testNfsConfigurationReturnsDefault() { + NfsConfiguration conf = new NfsConfiguration(); + String bindHost = conf.get(NfsConfigKeys.DFS_NFS_SERVER_BIND_HOST_KEY, + NfsConfigKeys.DFS_NFS_SERVER_BIND_HOST_DEFAULT); + assertEquals("0.0.0.0", bindHost); + } + + @Test + public void testNfsConfigurationReturnsConfiguredValue() { + NfsConfiguration conf = new NfsConfiguration(); + conf.set(NfsConfigKeys.DFS_NFS_SERVER_BIND_HOST_KEY, "127.0.0.1"); + String bindHost = conf.get(NfsConfigKeys.DFS_NFS_SERVER_BIND_HOST_KEY, + NfsConfigKeys.DFS_NFS_SERVER_BIND_HOST_DEFAULT); Review Comment: These assertions only round-trip the value through Configuration; they do not exercise the new propagation through RpcProgramNfs3/RpcProgramMountd and Nfs3Base/MountdBase into the actual listeners. A regression in either constructor wiring could leave all new tests green while the gateway still binds to 0.0.0.0, so add coverage that starts the NFS and mountd services with a non-default host and verifies their resulting bindings. -- 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]
