From 6ed66d7472cab08c61c8a8d39aa1bfc8f75680d9 Mon Sep 17 00:00:00 2001 From: Zach Kipp Date: Fri, 12 Dec 2025 11:04:12 -0700 Subject: [PATCH] fix: isolate keyring usage by parallel test processes This change ensures keyring tests that utilize the real OS keyring use credentials that are isolated by process ID so that parallel test processes do not access the same credentials. --- cli/keyring_test.go | 19 ++------------- cli/sessionstore/sessionstore_test.go | 24 +++++++------------ cli/sessionstore/sessionstore_windows_test.go | 3 ++- cli/sessionstore/testhelpers/testhelpers.go | 15 ++++++++++++ 4 files changed, 27 insertions(+), 34 deletions(-) create mode 100644 cli/sessionstore/testhelpers/testhelpers.go diff --git a/cli/keyring_test.go b/cli/keyring_test.go index 7cb190845a31b..08f5db7c8db2a 100644 --- a/cli/keyring_test.go +++ b/cli/keyring_test.go @@ -2,15 +2,11 @@ package cli_test import ( "bytes" - "crypto/rand" - "encoding/binary" - "fmt" "net/url" "os" "path" "runtime" "testing" - "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -19,23 +15,12 @@ import ( "github.com/coder/coder/v2/cli/clitest" "github.com/coder/coder/v2/cli/config" "github.com/coder/coder/v2/cli/sessionstore" + "github.com/coder/coder/v2/cli/sessionstore/testhelpers" "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/pty/ptytest" "github.com/coder/serpent" ) -// keyringTestServiceName generates a unique service name for keyring tests -// using the test name and a nanosecond timestamp to prevent collisions. -func keyringTestServiceName(t *testing.T) string { - t.Helper() - var n uint32 - err := binary.Read(rand.Reader, binary.BigEndian, &n) - if err != nil { - t.Fatal(err) - } - return fmt.Sprintf("%s_%v_%d", t.Name(), time.Now().UnixNano(), n) -} - type keyringTestEnv struct { serviceName string keyring sessionstore.Keyring @@ -52,7 +37,7 @@ func setupKeyringTestEnv(t *testing.T, clientURL string, args ...string) keyring cmd, err := root.Command(root.AGPL()) require.NoError(t, err) - serviceName := keyringTestServiceName(t) + serviceName := testhelpers.KeyringServiceName(t) root.WithKeyringServiceName(serviceName) root.UseKeyringWithGlobalConfig() diff --git a/cli/sessionstore/sessionstore_test.go b/cli/sessionstore/sessionstore_test.go index 1ecb0279918fd..7e8f0cb2fb3a3 100644 --- a/cli/sessionstore/sessionstore_test.go +++ b/cli/sessionstore/sessionstore_test.go @@ -3,18 +3,17 @@ package sessionstore_test import ( "encoding/json" "errors" - "fmt" "net/url" "os" "path" "runtime" "testing" - "time" "github.com/stretchr/testify/require" "github.com/coder/coder/v2/cli/config" "github.com/coder/coder/v2/cli/sessionstore" + "github.com/coder/coder/v2/cli/sessionstore/testhelpers" ) type storedCredentials map[string]struct { @@ -22,13 +21,6 @@ type storedCredentials map[string]struct { APIToken string `json:"api_token"` } -// Generate a test service name for use with the OS keyring. It uses a combination -// of the test name and a nanosecond timestamp to prevent collisions. -func keyringTestServiceName(t *testing.T) string { - t.Helper() - return t.Name() + "_" + fmt.Sprintf("%v", time.Now().UnixNano()) -} - func TestKeyring(t *testing.T) { t.Parallel() @@ -47,7 +39,7 @@ func TestKeyring(t *testing.T) { t.Run("ReadNonExistent", func(t *testing.T) { t.Parallel() - backend := sessionstore.NewKeyringWithService(keyringTestServiceName(t)) + backend := sessionstore.NewKeyringWithService(testhelpers.KeyringServiceName(t)) srvURL, err := url.Parse(testURL) require.NoError(t, err) t.Cleanup(func() { _ = backend.Delete(srvURL) }) @@ -60,7 +52,7 @@ func TestKeyring(t *testing.T) { t.Run("DeleteNonExistent", func(t *testing.T) { t.Parallel() - backend := sessionstore.NewKeyringWithService(keyringTestServiceName(t)) + backend := sessionstore.NewKeyringWithService(testhelpers.KeyringServiceName(t)) srvURL, err := url.Parse(testURL) require.NoError(t, err) t.Cleanup(func() { _ = backend.Delete(srvURL) }) @@ -73,7 +65,7 @@ func TestKeyring(t *testing.T) { t.Run("WriteAndRead", func(t *testing.T) { t.Parallel() - backend := sessionstore.NewKeyringWithService(keyringTestServiceName(t)) + backend := sessionstore.NewKeyringWithService(testhelpers.KeyringServiceName(t)) srvURL, err := url.Parse(testURL) require.NoError(t, err) t.Cleanup(func() { _ = backend.Delete(srvURL) }) @@ -101,7 +93,7 @@ func TestKeyring(t *testing.T) { t.Run("WriteAndDelete", func(t *testing.T) { t.Parallel() - backend := sessionstore.NewKeyringWithService(keyringTestServiceName(t)) + backend := sessionstore.NewKeyringWithService(testhelpers.KeyringServiceName(t)) srvURL, err := url.Parse(testURL) require.NoError(t, err) t.Cleanup(func() { _ = backend.Delete(srvURL) }) @@ -125,7 +117,7 @@ func TestKeyring(t *testing.T) { t.Run("OverwriteToken", func(t *testing.T) { t.Parallel() - backend := sessionstore.NewKeyringWithService(keyringTestServiceName(t)) + backend := sessionstore.NewKeyringWithService(testhelpers.KeyringServiceName(t)) srvURL, err := url.Parse(testURL) require.NoError(t, err) t.Cleanup(func() { _ = backend.Delete(srvURL) }) @@ -156,7 +148,7 @@ func TestKeyring(t *testing.T) { t.Run("MultipleServers", func(t *testing.T) { t.Parallel() - backend := sessionstore.NewKeyringWithService(keyringTestServiceName(t)) + backend := sessionstore.NewKeyringWithService(testhelpers.KeyringServiceName(t)) srvURL, err := url.Parse(testURL) require.NoError(t, err) srvURL2, err := url.Parse(testURL2) @@ -220,7 +212,7 @@ func TestKeyring(t *testing.T) { srv2URL, err := url.Parse(testURL2) require.NoError(t, err) - serviceName := keyringTestServiceName(t) + serviceName := testhelpers.KeyringServiceName(t) backend := sessionstore.NewKeyringWithService(serviceName) t.Cleanup(func() { _ = backend.Delete(srv1URL) diff --git a/cli/sessionstore/sessionstore_windows_test.go b/cli/sessionstore/sessionstore_windows_test.go index ef643d3033dba..e677d0988fe8c 100644 --- a/cli/sessionstore/sessionstore_windows_test.go +++ b/cli/sessionstore/sessionstore_windows_test.go @@ -12,6 +12,7 @@ import ( "github.com/stretchr/testify/require" "github.com/coder/coder/v2/cli/sessionstore" + "github.com/coder/coder/v2/cli/sessionstore/testhelpers" ) func readRawKeychainCredential(t *testing.T, serviceName string) []byte { @@ -31,7 +32,7 @@ func TestWindowsKeyring_WriteReadDelete(t *testing.T) { srvURL, err := url.Parse(testURL) require.NoError(t, err) - serviceName := keyringTestServiceName(t) + serviceName := testhelpers.KeyringServiceName(t) backend := sessionstore.NewKeyringWithService(serviceName) t.Cleanup(func() { _ = backend.Delete(srvURL) }) diff --git a/cli/sessionstore/testhelpers/testhelpers.go b/cli/sessionstore/testhelpers/testhelpers.go new file mode 100644 index 0000000000000..d07bdb809712e --- /dev/null +++ b/cli/sessionstore/testhelpers/testhelpers.go @@ -0,0 +1,15 @@ +package testhelpers + +import ( + "fmt" + "os" + "testing" +) + +// KeyringServiceName generates a test service name for use with the OS keyring. +// It intends to prevent keyring usage collisions between parallel tests within a +// process and parallel test processes (which may occur on CI). +func KeyringServiceName(t *testing.T) string { + t.Helper() + return t.Name() + "_" + fmt.Sprintf("%v", os.Getpid()) +}