Copilot commented on code in PR #8744: URL: https://github.com/apache/hadoop/pull/8744#discussion_r4039581342
########## hadoop-common-project/hadoop-nfs/src/test/java/org/apache/hadoop/nfs/nfs3/TestNfsGatewayBindAddress.java: ########## @@ -0,0 +1,182 @@ +/** + * 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.nfs.nfs3; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; + +import java.io.IOException; +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.conf.Configuration; +import org.apache.hadoop.mount.MountdBase; +import org.apache.hadoop.oncrpc.RpcAcceptedReply; +import org.apache.hadoop.oncrpc.RpcCall; +import org.apache.hadoop.oncrpc.RpcInfo; +import org.apache.hadoop.oncrpc.RpcProgram; +import org.apache.hadoop.oncrpc.RpcResponse; +import org.apache.hadoop.oncrpc.RpcUtil; +import org.apache.hadoop.oncrpc.XDR; +import org.apache.hadoop.oncrpc.security.VerifierNone; +import org.junit.jupiter.api.Test; + +/** + * Verifies that the bind address configured via {@code nfs.server.bind.host} + * flows through {@link Nfs3Base} and {@link MountdBase} all the way to the + * actual listening sockets. + * + * These tests exercise the wiring inside the base classes — specifically that + * {@code rpcProgram.getBindHost()} is forwarded to {@link + * org.apache.hadoop.oncrpc.SimpleTcpServer} and {@link + * org.apache.hadoop.oncrpc.SimpleUdpServer} — so a regression that removes + * that call would be caught here even if the config-key unit tests still pass. + */ +public class TestNfsGatewayBindAddress { + + // ----------------------------------------------------------------------- + // Minimal stub infrastructure + // ----------------------------------------------------------------------- + + @ChannelHandler.Sharable + private static class StubRpcProgram extends RpcProgram { + StubRpcProgram(int port, String bindHost) { + super("stub", "localhost", port, 100001, 1, 1, null, true, 500, bindHost); + } + + @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; + } + } + + /** Concrete {@link Nfs3Base} subclass that does nothing beyond what the base class does. */ + private static class StubNfs3 extends Nfs3Base { + StubNfs3(RpcProgram program) { + super(program, new Configuration()); + } + } + + /** Concrete {@link MountdBase} subclass that does nothing beyond what the base class does. */ + private static class StubMountd extends MountdBase { + StubMountd(RpcProgram program) throws IOException { + super(program); + } + } + + private static int randomHighPort() { + return 20000 + new Random().nextInt(10000); Review Comment: These tests select a random fixed port, so another test or process can occupy the chosen port and make `start()` fail before the bind-host assertion. The existing NFS tests use port `0` for parallel safety (for example, `TestMountd.java:48-50`); use an ephemeral port here instead. ########## 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; Review Comment: This newly added import is unused because `randomPort()` always returns 0. Hadoop's checkstyle includes `UnusedImports` for test sources (`pom.xml:513-518`), so remove this import. This issue also appears on line 64 of the same file. ########## hadoop-hdfs-project/hadoop-hdfs-nfs/src/test/java/org/apache/hadoop/hdfs/nfs/nfs3/TestNfsBindConfiguration.java: ########## @@ -0,0 +1,57 @@ +/** + * 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); Review Comment: This statement is not indented to the method body, which violates the repository's configured `Indentation` check for test sources. Indent the local declaration to match the surrounding code. -- 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]
