From d4bbaf821e4c12e31d91a05da559bdb8718727f1 Mon Sep 17 00:00:00 2001 From: Ben Moskovitz Date: Tue, 13 Feb 2024 09:25:19 +1100 Subject: [PATCH] Remove windows workarounds for statting sockets See: https://github.com/golang/go/issues/33357 Go 1.22 has made it so that os.Stat now follows all reparse points, of which IO_REPARSE_TAG_AF_UNIX (the reparse point that os.ModeSocket should have been triggering) is one --- internal/socket/client.go | 18 ++++++------------ internal/socket/server_test.go | 20 +++++++------------- internal/socket/utils.go | 7 +------ jobapi/server_test.go | 4 ---- 4 files changed, 14 insertions(+), 35 deletions(-) diff --git a/internal/socket/client.go b/internal/socket/client.go index e5557b184a..57f8aa39b6 100644 --- a/internal/socket/client.go +++ b/internal/socket/client.go @@ -9,7 +9,6 @@ import ( "net" "net/http" "os" - "runtime" ) // Error response is the response body for any errors that occur @@ -37,17 +36,12 @@ type Client struct { // NewClient creates a new Client. The context is used for an internal check // that the socket can be dialled. func NewClient(ctx context.Context, path, token string) (*Client, error) { - // Check the socket path exists and is a socket. - // Note that os.ModeSocket might not be set on Windows. - // (https://github.com/golang/go/issues/33357) - if runtime.GOOS != "windows" { - fi, err := os.Stat(path) - if err != nil { - return nil, fmt.Errorf("stat socket: %w", err) - } - if fi.Mode()&os.ModeSocket == 0 { - return nil, fmt.Errorf("%q is not a socket", path) - } + fi, err := os.Stat(path) + if err != nil { + return nil, fmt.Errorf("stat socket: %w", err) + } + if fi.Mode()&os.ModeSocket == 0 { + return nil, fmt.Errorf("%q is not a socket", path) } dialer := net.Dialer{} diff --git a/internal/socket/server_test.go b/internal/socket/server_test.go index 7c22015c41..3b3a55f0f4 100644 --- a/internal/socket/server_test.go +++ b/internal/socket/server_test.go @@ -9,7 +9,6 @@ import ( "net/http" "os" "path/filepath" - "runtime" "sync/atomic" "testing" "time" @@ -52,18 +51,13 @@ func TestServerStartStop(t *testing.T) { t.Fatalf("srv.Start() = %v", err) } - // Check the socket path exists and is a socket. - // Note that os.ModeSocket might not be set on Windows. - // (https://github.com/golang/go/issues/33357) - if runtime.GOOS != "windows" { - fi, err := os.Stat(sockPath) - if err != nil { - t.Fatalf("os.Stat(%q) = %v", sockPath, err) - } - - if fi.Mode()&os.ModeSocket == 0 { - t.Fatalf("%q is not a socket", sockPath) - } + fi, err := os.Stat(sockPath) + if err != nil { + t.Fatalf("os.Stat(%q) = %v", sockPath, err) + } + + if fi.Mode()&os.ModeSocket == 0 { + t.Fatalf("%q is not a socket", sockPath) } // Try to connect to the socket. diff --git a/internal/socket/utils.go b/internal/socket/utils.go index 90a525d056..a21cd4b5fe 100644 --- a/internal/socket/utils.go +++ b/internal/socket/utils.go @@ -12,13 +12,8 @@ import ( "strings" ) -// socketExists returns true if the socket path exists on linux and darwin -// on windows it always returns false, because of https://github.com/golang/go/issues/33357 (stat on sockets is broken on windows) +// socketExists returns true if the socket path exists func socketExists(path string) (bool, error) { - if runtime.GOOS == "windows" { - return false, nil - } - _, err := os.Stat(path) if err != nil { if errors.Is(err, os.ErrNotExist) { diff --git a/jobapi/server_test.go b/jobapi/server_test.go index 07fdb1a2b0..7bc226abc7 100644 --- a/jobapi/server_test.go +++ b/jobapi/server_test.go @@ -100,10 +100,6 @@ func TestServerStartStop(t *testing.T) { } func TestServerStartStop_WithPreExistingSocket(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("socket collision detection isn't support on windows. If the current go version is >1.23, it might be worth re-enabling this test, because hopefully the bug (https://github.com/golang/go/issues/33357) is fixed") - } - sockName := filepath.Join(os.TempDir(), "test-socket-collision.sock") srv1, _, err := jobapi.NewServer(shell.TestingLogger{T: t}, sockName, env.New()) if err != nil {