Skip to content

Commit 7ecfd1a

Browse files
zedkippzedkipp
authored
fix: isolate keyring usage by parallel test processes (#21256)
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. coder/internal#1192
1 parent 13fbbcd commit 7ecfd1a

File tree

4 files changed

+27
-34
lines changed

4 files changed

+27
-34
lines changed

‎cli/keyring_test.go‎

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,11 @@ package cli_test
22

33
import (
44
"bytes"
5-
"crypto/rand"
6-
"encoding/binary"
7-
"fmt"
85
"net/url"
96
"os"
107
"path"
118
"runtime"
129
"testing"
13-
"time"
1410

1511
"github.com/stretchr/testify/assert"
1612
"github.com/stretchr/testify/require"
@@ -19,23 +15,12 @@ import (
1915
"github.com/coder/coder/v2/cli/clitest"
2016
"github.com/coder/coder/v2/cli/config"
2117
"github.com/coder/coder/v2/cli/sessionstore"
18+
"github.com/coder/coder/v2/cli/sessionstore/testhelpers"
2219
"github.com/coder/coder/v2/coderd/coderdtest"
2320
"github.com/coder/coder/v2/pty/ptytest"
2421
"github.com/coder/serpent"
2522
)
2623

27-
// keyringTestServiceName generates a unique service name for keyring tests
28-
// using the test name and a nanosecond timestamp to prevent collisions.
29-
func keyringTestServiceName(t *testing.T) string {
30-
t.Helper()
31-
var n uint32
32-
err := binary.Read(rand.Reader, binary.BigEndian, &n)
33-
if err != nil {
34-
t.Fatal(err)
35-
}
36-
return fmt.Sprintf("%s_%v_%d", t.Name(), time.Now().UnixNano(), n)
37-
}
38-
3924
type keyringTestEnv struct {
4025
serviceName string
4126
keyring sessionstore.Keyring
@@ -52,7 +37,7 @@ func setupKeyringTestEnv(t *testing.T, clientURL string, args ...string) keyring
5237
cmd, err := root.Command(root.AGPL())
5338
require.NoError(t, err)
5439

55-
serviceName := keyringTestServiceName(t)
40+
serviceName := testhelpers.KeyringServiceName(t)
5641
root.WithKeyringServiceName(serviceName)
5742
root.UseKeyringWithGlobalConfig()
5843

‎cli/sessionstore/sessionstore_test.go‎

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,32 +3,24 @@ package sessionstore_test
33
import (
44
"encoding/json"
55
"errors"
6-
"fmt"
76
"net/url"
87
"os"
98
"path"
109
"runtime"
1110
"testing"
12-
"time"
1311

1412
"github.com/stretchr/testify/require"
1513

1614
"github.com/coder/coder/v2/cli/config"
1715
"github.com/coder/coder/v2/cli/sessionstore"
16+
"github.com/coder/coder/v2/cli/sessionstore/testhelpers"
1817
)
1918

2019
type storedCredentials map[string]struct {
2120
CoderURL string `json:"coder_url"`
2221
APIToken string `json:"api_token"`
2322
}
2423

25-
// Generate a test service name for use with the OS keyring. It uses a combination
26-
// of the test name and a nanosecond timestamp to prevent collisions.
27-
func keyringTestServiceName(t *testing.T) string {
28-
t.Helper()
29-
return t.Name() + "_" + fmt.Sprintf("%v", time.Now().UnixNano())
30-
}
31-
3224
func TestKeyring(t *testing.T) {
3325
t.Parallel()
3426

@@ -47,7 +39,7 @@ func TestKeyring(t *testing.T) {
4739
t.Run("ReadNonExistent", func(t *testing.T) {
4840
t.Parallel()
4941

50-
backend := sessionstore.NewKeyringWithService(keyringTestServiceName(t))
42+
backend := sessionstore.NewKeyringWithService(testhelpers.KeyringServiceName(t))
5143
srvURL, err := url.Parse(testURL)
5244
require.NoError(t, err)
5345
t.Cleanup(func() { _ = backend.Delete(srvURL) })
@@ -60,7 +52,7 @@ func TestKeyring(t *testing.T) {
6052
t.Run("DeleteNonExistent", func(t *testing.T) {
6153
t.Parallel()
6254

63-
backend := sessionstore.NewKeyringWithService(keyringTestServiceName(t))
55+
backend := sessionstore.NewKeyringWithService(testhelpers.KeyringServiceName(t))
6456
srvURL, err := url.Parse(testURL)
6557
require.NoError(t, err)
6658
t.Cleanup(func() { _ = backend.Delete(srvURL) })
@@ -73,7 +65,7 @@ func TestKeyring(t *testing.T) {
7365
t.Run("WriteAndRead", func(t *testing.T) {
7466
t.Parallel()
7567

76-
backend := sessionstore.NewKeyringWithService(keyringTestServiceName(t))
68+
backend := sessionstore.NewKeyringWithService(testhelpers.KeyringServiceName(t))
7769
srvURL, err := url.Parse(testURL)
7870
require.NoError(t, err)
7971
t.Cleanup(func() { _ = backend.Delete(srvURL) })
@@ -101,7 +93,7 @@ func TestKeyring(t *testing.T) {
10193
t.Run("WriteAndDelete", func(t *testing.T) {
10294
t.Parallel()
10395

104-
backend := sessionstore.NewKeyringWithService(keyringTestServiceName(t))
96+
backend := sessionstore.NewKeyringWithService(testhelpers.KeyringServiceName(t))
10597
srvURL, err := url.Parse(testURL)
10698
require.NoError(t, err)
10799
t.Cleanup(func() { _ = backend.Delete(srvURL) })
@@ -125,7 +117,7 @@ func TestKeyring(t *testing.T) {
125117
t.Run("OverwriteToken", func(t *testing.T) {
126118
t.Parallel()
127119

128-
backend := sessionstore.NewKeyringWithService(keyringTestServiceName(t))
120+
backend := sessionstore.NewKeyringWithService(testhelpers.KeyringServiceName(t))
129121
srvURL, err := url.Parse(testURL)
130122
require.NoError(t, err)
131123
t.Cleanup(func() { _ = backend.Delete(srvURL) })
@@ -156,7 +148,7 @@ func TestKeyring(t *testing.T) {
156148
t.Run("MultipleServers", func(t *testing.T) {
157149
t.Parallel()
158150

159-
backend := sessionstore.NewKeyringWithService(keyringTestServiceName(t))
151+
backend := sessionstore.NewKeyringWithService(testhelpers.KeyringServiceName(t))
160152
srvURL, err := url.Parse(testURL)
161153
require.NoError(t, err)
162154
srvURL2, err := url.Parse(testURL2)
@@ -220,7 +212,7 @@ func TestKeyring(t *testing.T) {
220212
srv2URL, err := url.Parse(testURL2)
221213
require.NoError(t, err)
222214

223-
serviceName := keyringTestServiceName(t)
215+
serviceName := testhelpers.KeyringServiceName(t)
224216
backend := sessionstore.NewKeyringWithService(serviceName)
225217
t.Cleanup(func() {
226218
_ = backend.Delete(srv1URL)

‎cli/sessionstore/sessionstore_windows_test.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"github.com/stretchr/testify/require"
1313

1414
"github.com/coder/coder/v2/cli/sessionstore"
15+
"github.com/coder/coder/v2/cli/sessionstore/testhelpers"
1516
)
1617

1718
func readRawKeychainCredential(t *testing.T, serviceName string) []byte {
@@ -31,7 +32,7 @@ func TestWindowsKeyring_WriteReadDelete(t *testing.T) {
3132
srvURL, err := url.Parse(testURL)
3233
require.NoError(t, err)
3334

34-
serviceName := keyringTestServiceName(t)
35+
serviceName := testhelpers.KeyringServiceName(t)
3536
backend := sessionstore.NewKeyringWithService(serviceName)
3637
t.Cleanup(func() { _ = backend.Delete(srvURL) })
3738

‎cli/sessionstore/testhelpers/testhelpers.go‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
package testhelpers
2+
3+
import (
4+
"fmt"
5+
"os"
6+
"testing"
7+
)
8+
9+
// KeyringServiceName generates a test service name for use with the OS keyring.
10+
// It intends to prevent keyring usage collisions between parallel tests within a
11+
// process and parallel test processes (which may occur on CI).
12+
func KeyringServiceName(t *testing.T) string {
13+
t.Helper()
14+
return t.Name() + "_" + fmt.Sprintf("%v", os.Getpid())
15+
}

0 commit comments

Comments
 (0)