From 3c83ccab233a8c8b0adfefc5a5b49af21224f5a0 Mon Sep 17 00:00:00 2001
From: Ayush Ranjan <ayushranjan@google.com>
Date: Mon, 14 Nov 2022 11:59:45 -0800
Subject: [PATCH] Get connect external tests to work natively.

- Use the correct paths for sockets based on environment variables.
- Doing a double-bind(2) before listen(2) causes subsequent double-bind(2) after
  listen(2) to fail with EADDRINUSE on Linux somehow. Avoid tripping that
  behavior in the test.
- Do not run BindListenAccept test with TEST_UDS_ATTACH_TREE parameter.

PiperOrigin-RevId: 488430850
---
 test/runner/main.go                     | 46 +++++++++----------------
 test/syscalls/linux/connect_external.cc | 16 ++++-----
 2 files changed, 24 insertions(+), 38 deletions(-)

diff --git a/test/runner/main.go b/test/runner/main.go
index 5e917461e..0c47a158d 100644
--- a/test/runner/main.go
+++ b/test/runner/main.go
@@ -345,41 +345,27 @@ func setupHostCommTree(spec *specs.Spec) (cleanup func(), err error) {
 
 	// Individial attach points for each socket to test mounts that attach
 	// directly to the sockets.
-	spec.Mounts = append(spec.Mounts, specs.Mount{
-		Destination: "/tmp/sockets-attach/stream/echo",
-		Source:      filepath.Join(socketDir, "stream/echo"),
-		Type:        "bind",
-	})
-	spec.Mounts = append(spec.Mounts, specs.Mount{
-		Destination: "/tmp/sockets-attach/stream/nonlistening",
-		Source:      filepath.Join(socketDir, "stream/nonlistening"),
-		Type:        "bind",
-	})
-	spec.Mounts = append(spec.Mounts, specs.Mount{
-		Destination: "/tmp/sockets-attach/seqpacket/echo",
-		Source:      filepath.Join(socketDir, "seqpacket/echo"),
-		Type:        "bind",
-	})
-	spec.Mounts = append(spec.Mounts, specs.Mount{
-		Destination: "/tmp/sockets-attach/seqpacket/nonlistening",
-		Source:      filepath.Join(socketDir, "seqpacket/nonlistening"),
-		Type:        "bind",
-	})
+	for _, protocol := range []string{"stream", "seqpacket"} {
+		for _, name := range []string{"echo", "nonlistening"} {
+			spec.Mounts = append(spec.Mounts, specs.Mount{
+				Destination: filepath.Join("/tmp/sockets-attach", protocol, name),
+				Source:      filepath.Join(socketDir, protocol, name),
+				Type:        "bind",
+			})
+		}
+	}
 	spec.Mounts = append(spec.Mounts, specs.Mount{
 		Destination: "/tmp/sockets-attach/dgram/null",
 		Source:      filepath.Join(socketDir, "dgram/null"),
 		Type:        "bind",
 	})
-	spec.Mounts = append(spec.Mounts, specs.Mount{
-		Destination: "/tmp/sockets-attach/pipe/in",
-		Source:      filepath.Join(socketDir, "pipe/in"),
-		Type:        "bind",
-	})
-	spec.Mounts = append(spec.Mounts, specs.Mount{
-		Destination: "/tmp/sockets-attach/pipe/out",
-		Source:      filepath.Join(socketDir, "pipe/out"),
-		Type:        "bind",
-	})
+	for _, name := range []string{"in", "out"} {
+		spec.Mounts = append(spec.Mounts, specs.Mount{
+			Destination: filepath.Join("/tmp/sockets-attach/pipe", name),
+			Source:      filepath.Join(socketDir, "pipe", name),
+			Type:        "bind",
+		})
+	}
 
 	spec.Process.Env = append(spec.Process.Env, "TEST_UDS_TREE=/tmp/sockets")
 	spec.Process.Env = append(spec.Process.Env, "TEST_UDS_ATTACH_TREE=/tmp/sockets-attach")
diff --git a/test/syscalls/linux/connect_external.cc b/test/syscalls/linux/connect_external.cc
index d97b29ab2..fa043969f 100644
--- a/test/syscalls/linux/connect_external.cc
+++ b/test/syscalls/linux/connect_external.cc
@@ -19,6 +19,7 @@
 #include <sys/types.h>
 #include <sys/un.h>
 
+#include <cstring>
 #include <string>
 #include <tuple>
 
@@ -114,12 +115,16 @@ TEST_P(GoferStreamSeqpacketTest, NonListening) {
 // Bind to a socket, then Listen and Accept.
 TEST_P(GoferStreamSeqpacketTest, BindListenAccept) {
   // Binding to host socket requires LisaFS.
-  SKIP_IF(!IsLisafsEnabled());
+  SKIP_IF(IsRunningOnGvisor() && !IsLisafsEnabled());
 
   std::string env;
   ProtocolSocket proto;
   std::tie(env, proto) = GetParam();
 
+  // Do not parametrize this test with attach tree variant. This test creates a
+  // new UDS via bind(2). It is not possible to bind mount a non-existing file.
+  SKIP_IF(!strcmp("TEST_UDS_ATTACH_TREE", env.c_str()));
+
   char* val = getenv(env.c_str());
   ASSERT_NE(val, nullptr);
   std::string root(val);
@@ -127,8 +132,7 @@ TEST_P(GoferStreamSeqpacketTest, BindListenAccept) {
   FileDescriptor sock =
       ASSERT_NO_ERRNO_AND_VALUE(Socket(AF_UNIX, proto.protocol, 0));
 
-  std::string socket_path =
-      JoinPath("/tmp/sockets", proto.name, "created-in-sandbox");
+  std::string socket_path = JoinPath(root, proto.name, "created-in-sandbox");
 
   struct sockaddr_un addr = {};
   addr.sun_family = AF_UNIX;
@@ -137,6 +141,7 @@ TEST_P(GoferStreamSeqpacketTest, BindListenAccept) {
   ASSERT_THAT(
       bind(sock.get(), reinterpret_cast<struct sockaddr*>(&addr), sizeof(addr)),
       SyscallSucceeds());
+  ASSERT_THAT(listen(sock.get(), 1), SyscallSucceeds());
 
   // Bind again on that socket with a diff address should fail.
   std::string socket_path2 = socket_path + "-fail";
@@ -147,11 +152,6 @@ TEST_P(GoferStreamSeqpacketTest, BindListenAccept) {
                    sizeof(addr2)),
               SyscallFailsWithErrno(EINVAL));
 
-  ASSERT_THAT(listen(sock.get(), 1), SyscallSucceeds());
-  ASSERT_THAT(bind(sock.get(), reinterpret_cast<struct sockaddr*>(&addr2),
-                   sizeof(addr2)),
-              SyscallFailsWithErrno(EINVAL));
-
   FileDescriptor accSock =
       ASSERT_NO_ERRNO_AND_VALUE(Accept(sock.get(), NULL, NULL));