TPortManager shouldn't crash even if it doesn't have enough permissions to operate on lock files created by other users

Create the `testing_port_locks` directory with RWX permission for all, allowing other users to create lock files within it.
Don't abort if current user doesn't have enough permission to delete a lock file created by another user.

KIKIMR-23792
commit_hash:d887ce073d42d93c997a0f9a0c16416860395377

2 files changed, 79 insertions, 22 deletions

diff --git a/library/cpp/testing/common/network.cpp b/library/cpp/testing/common/network.cpp
index 9f018f69b72..dbc80715fde 100644
--- a/library/cpp/testing/common/network.cpp
+++ b/library/cpp/testing/common/network.cpp
@@ -2,6 +2,7 @@
 
 #include <util/folder/dirut.h>
 #include <util/folder/path.h>
+#include <util/generic/noncopyable.h>
 #include <util/generic/singleton.h>
 #include <util/generic/utility.h>
 #include <util/generic/vector.h>
@@ -15,6 +16,7 @@
 #include <util/system/error.h>
 #include <util/system/file_lock.h>
 #include <util/system/fs.h>
+#include <util/system/sysstat.h>
 
 #ifdef _darwin_
 #include <sys/types.h>
@@ -29,7 +31,7 @@ namespace {
         }                                                                 \
     } while (false)
 
-    class TPortGuard : public NTesting::IPort {
+    class TPortGuard : public NTesting::IPort, TNonCopyable {
     public:
         TPortGuard(ui16 port, THolder<TFileLock> lock)
             : Lock_(std::move(lock))
@@ -38,7 +40,16 @@ namespace {
         }
 
         ~TPortGuard() override {
-            Y_VERIFY_SYSERROR(NFs::Remove(Lock_->GetName()));
+            try {
+                if (!NFs::Remove(Lock_->GetName())) {
+                    // Deletion may fail if the current user does not have
+                    // write permission to the PORT_SYNC_PATH directory.
+                    // At least let's release the lock file.
+                    Lock_->Release();
+                }
+            } catch (const TSystemError& e) {
+                Y_ABORT("Failed to unlock port %d, error=%d", Port_, e.Status());
+            }
         }
 
         ui16 Get() override {
@@ -107,6 +118,7 @@ namespace {
             }
             Y_ABORT_UNLESS(SyncDir_.IsDefined());
             NFs::MakeDirectoryRecursive(SyncDir_);
+            Chmod(SyncDir_.c_str(), NFs::FP_COMMON_FILE); // override umask
 
             Ranges_ = GetPortRanges();
             TotalCount_ = 0;
diff --git a/library/cpp/testing/common/ut/network_ut.cpp b/library/cpp/testing/common/ut/network_ut.cpp
index 2016e26b092..22cedf87ae9 100644
--- a/library/cpp/testing/common/ut/network_ut.cpp
+++ b/library/cpp/testing/common/ut/network_ut.cpp
@@ -1,20 +1,38 @@
+#include <library/cpp/testing/common/env.h>
 #include <library/cpp/testing/common/network.h>
 #include <library/cpp/testing/common/scope.h>
+#include <library/cpp/testing/gtest/gtest.h>
 
 #include <util/generic/hash_set.h>
 
 #include <util/folder/dirut.h>
 #include <util/folder/path.h>
 #include <util/folder/tempdir.h>
+#include <util/generic/scope.h>
 #include <util/network/sock.h>
+#include <util/system/file.h>
 #include <util/system/fs.h>
+#include <util/system/sysstat.h>
 
-#include <library/cpp/testing/gtest/gtest.h>
+class NetworkTestBase: public ::testing::Test {
+protected:
+    void SetUp() override {
+        TmpDir.ConstructInPlace();
+    }
+
+    void TearDown() override {
+        TmpDir.Clear();
+    }
+
+public:
+    TMaybe<TTempDir> TmpDir;
+};
 
-static TTempDir TmpDir;
+class NetworkTest: public NetworkTestBase {};
+class FreePortTest: public NetworkTestBase {};
 
-TEST(NetworkTest, FreePort) {
-    NTesting::TScopedEnvironment envGuard("PORT_SYNC_PATH", TmpDir.Name());
+TEST_F(NetworkTest, FreePort) {
+    NTesting::TScopedEnvironment envGuard("PORT_SYNC_PATH", TmpDir->Name());
     NTesting::InitPortManagerFromEnv();
     TVector<NTesting::TPortHolder> ports(Reserve(100));
 
@@ -24,7 +42,7 @@ TEST(NetworkTest, FreePort) {
 
     THashSet<ui16> uniqPorts;
     for (auto& port : ports) {
-        const TString guardPath = TmpDir.Path() / ToString(static_cast<ui16>(port));
+        const TString guardPath = TmpDir->Path() / ToString(static_cast<ui16>(port));
         EXPECT_TRUE(NFs::Exists(guardPath));
         EXPECT_TRUE(uniqPorts.emplace(port).second);
 
@@ -35,16 +53,16 @@ TEST(NetworkTest, FreePort) {
     }
     ports.clear();
     for (ui16 port : uniqPorts) {
-        const TString guardPath = TmpDir.Path() / ToString(port);
+        const TString guardPath = TmpDir->Path() / ToString(port);
         EXPECT_FALSE(NFs::Exists(guardPath));
     }
 }
 
-TEST(NetworkTest, FreePortWithinRanges) {
+TEST_F(NetworkTest, FreePortWithinRanges) {
     NTesting::TScopedEnvironment envGuard{{
-            {"PORT_SYNC_PATH", TmpDir.Name()},
-            {"VALID_PORT_RANGE", "3456:7654"},
-        }};
+        {"PORT_SYNC_PATH", TmpDir->Name()},
+        {"VALID_PORT_RANGE", "3456:7654"},
+    }};
     NTesting::InitPortManagerFromEnv();
 
     for (size_t i = 0; i < 100; ++i) {
@@ -55,11 +73,11 @@ TEST(NetworkTest, FreePortWithinRanges) {
     }
 }
 
-TEST(NetworkTest, GetPortRandom) {
+TEST_F(NetworkTest, GetPortRandom) {
     NTesting::TScopedEnvironment envGuard{{
-            {"PORT_SYNC_PATH", TmpDir.Name()},
-            {"NO_RANDOM_PORTS", ""},
-        }};
+        {"PORT_SYNC_PATH", TmpDir->Name()},
+        {"NO_RANDOM_PORTS", ""},
+    }};
     NTesting::InitPortManagerFromEnv();
 
     ui16 testPort = 80; // value just must be outside the assignable range
@@ -70,11 +88,11 @@ TEST(NetworkTest, GetPortRandom) {
     }
 }
 
-TEST(NetworkTest, GetPortNonRandom) {
+TEST_F(NetworkTest, GetPortNonRandom) {
     NTesting::TScopedEnvironment envGuard{{
-            {"PORT_SYNC_PATH", TmpDir.Name()},
-            {"NO_RANDOM_PORTS", "1"},
-        }};
+        {"PORT_SYNC_PATH", TmpDir->Name()},
+        {"NO_RANDOM_PORTS", "1"},
+    }};
     NTesting::InitPortManagerFromEnv();
 
     TVector<ui16> ports(Reserve(100)); // keep integers, we don't need the ports to remain allocated
@@ -91,9 +109,36 @@ TEST(NetworkTest, GetPortNonRandom) {
     }
 }
 
+TEST_F(NetworkTest, Permissions) {
+    constexpr ui16 loPort = 3456;
+    constexpr ui16 hiPort = 7654;
+    NTesting::TScopedEnvironment envGuard{{
+        {"PORT_SYNC_PATH", TmpDir->Name()},
+        {"VALID_PORT_RANGE", ToString(loPort) + ":" + ToString(hiPort)},
+    }};
+    NTesting::InitPortManagerFromEnv();
+    TVector<NTesting::TPortHolder> ports(Reserve(100));
+    for (ui64 port = loPort; port <= hiPort; ++port) {
+        const TString guardPath = TmpDir->Path() / ToString(static_cast<ui16>(port));
+        TFile f{guardPath, OpenAlways | RdOnly};
+        ASSERT_TRUE(f.IsOpen());
+        ASSERT_TRUE(Chmod(f.GetName().c_str(), 0444) != -1);
+    }
+    Y_DEFER {
+        Chmod(TmpDir->Path().c_str(), NFs::FP_COMMON_FILE);
+    };
+    ASSERT_TRUE(Chmod(TmpDir->Path().c_str(), 0555) != -1) << errno << " " << strerror(errno); // Lock dir
+    for (size_t i = 0; i < 100; ++i) {
+        NTesting::TPortHolder p;
+        EXPECT_NO_THROW(p = NTesting::GetFreePort());
+        ui16 port = p;
+        ASSERT_GE(port, 3456u);
+        ASSERT_LE(port, 7654u);
+    }
+}
 
-TEST(FreePortTest, FreePortsRange) {
-    NTesting::TScopedEnvironment envGuard("PORT_SYNC_PATH", TmpDir.Name());
+TEST_F(FreePortTest, FreePortsRange) {
+    NTesting::TScopedEnvironment envGuard("PORT_SYNC_PATH", TmpDir->Name());
     NTesting::InitPortManagerFromEnv();
 
     for (ui16 i = 2; i < 10; ++i) {