Skip to content

Commit a7c63df

Browse files
sreyasreya
committed
Refactor keycache to improve error handling
- Use `cryptokeys.ErrKeyNotFound` for missing keys - Use `cryptokeys.ErrKeyInvalid` for invalid keys - Replace `xerrors` for streamlined error codes
1 parent a30b3c6 commit a7c63df

File tree

2 files changed

+51
-16
lines changed

2 files changed

+51
-16
lines changed

‎enterprise/wsproxy/keycache.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ func (k *CryptoKeyCache) Signing(ctx context.Context) (codersdk.CryptoKey, error
6464
latest := k.latest
6565
k.keysMu.RUnlock()
6666

67-
now := k.Clock.Now().UTC()
67+
now := k.Clock.Now()
6868
if latest.CanSign(now) {
6969
return latest, nil
7070
}
@@ -87,14 +87,14 @@ func (k *CryptoKeyCache) Signing(ctx context.Context) (codersdk.CryptoKey, error
8787
}
8888

8989
if !k.latest.CanSign(now) {
90-
return codersdk.CryptoKey{}, xerrors.Errorf("no active keys found")
90+
return codersdk.CryptoKey{}, cryptokeys.ErrKeyNotFound
9191
}
9292

9393
return k.latest, nil
9494
}
9595

9696
func (k *CryptoKeyCache) Verifying(ctx context.Context, sequence int32) (codersdk.CryptoKey, error) {
97-
now := k.Clock.Now().UTC()
97+
now := k.Clock.Now()
9898
k.keysMu.RLock()
9999
if k.closed {
100100
k.keysMu.RUnlock()
@@ -127,7 +127,7 @@ func (k *CryptoKeyCache) Verifying(ctx context.Context, sequence int32) (codersd
127127

128128
key, ok = k.keys[sequence]
129129
if !ok {
130-
return codersdk.CryptoKey{}, xerrors.Errorf("key %d not found", sequence)
130+
return codersdk.CryptoKey{}, cryptokeys.ErrKeyNotFound
131131
}
132132

133133
return validKey(key, now)
@@ -155,7 +155,7 @@ func (k *CryptoKeyCache) fetch(ctx context.Context) (map[int32]codersdk.CryptoKe
155155
return nil, codersdk.CryptoKey{}, xerrors.Errorf("get security keys: %w", err)
156156
}
157157

158-
kmap, latest := toKeyMap(keys.CryptoKeys, k.Clock.Now().UTC())
158+
kmap, latest := toKeyMap(keys.CryptoKeys, k.Clock.Now())
159159
return kmap, latest, nil
160160
}
161161

@@ -172,8 +172,8 @@ func toKeyMap(keys []codersdk.CryptoKey, now time.Time) (map[int32]codersdk.Cryp
172172
}
173173

174174
func validKey(key codersdk.CryptoKey, now time.Time) (codersdk.CryptoKey, error) {
175-
if !key.CanSign(now) {
176-
return codersdk.CryptoKey{}, xerrors.Errorf("key %d is invalid", key.Sequence)
175+
if !key.CanVerify(now) {
176+
return codersdk.CryptoKey{}, cryptokeys.ErrKeyInvalid
177177
}
178178

179179
return key, nil

‎enterprise/wsproxy/keycache_test.go

Lines changed: 44 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import (
2323
func TestCryptoKeyCache(t *testing.T) {
2424
t.Parallel()
2525

26-
t.Run("Latest", func(t *testing.T) {
26+
t.Run("Signing", func(t *testing.T) {
2727
t.Parallel()
2828

2929
t.Run("HitsCache", func(t *testing.T) {
@@ -138,9 +138,27 @@ func TestCryptoKeyCache(t *testing.T) {
138138
require.Equal(t, expected, got)
139139
require.Equal(t, 1, fc.called)
140140
})
141+
142+
t.Run("KeyNotFound", func(t *testing.T) {
143+
t.Parallel()
144+
145+
var (
146+
ctx = testutil.Context(t, testutil.WaitShort)
147+
logger = slogtest.Make(t, nil)
148+
clock = quartz.NewMock(t)
149+
)
150+
151+
fc := newFakeCoderd(t, []codersdk.CryptoKey{})
152+
153+
cache, err := wsproxy.NewCryptoKeyCache(ctx, logger, wsproxysdk.New(fc.url), withClock(clock))
154+
require.NoError(t, err)
155+
156+
_, err = cache.Verifying(ctx, 1)
157+
require.ErrorIs(t, err, cryptokeys.ErrKeyNotFound)
158+
})
141159
})
142160

143-
t.Run("Version", func(t *testing.T) {
161+
t.Run("Verifying", func(t *testing.T) {
144162
t.Parallel()
145163

146164
t.Run("HitsCache", func(t *testing.T) {
@@ -241,7 +259,7 @@ func TestCryptoKeyCache(t *testing.T) {
241259
require.Equal(t, 1, fc.called)
242260
})
243261

244-
t.Run("NoInvalid", func(t *testing.T) {
262+
t.Run("KeyInvalid", func(t *testing.T) {
245263
t.Parallel()
246264

247265
var (
@@ -267,9 +285,27 @@ func TestCryptoKeyCache(t *testing.T) {
267285
require.NoError(t, err)
268286

269287
_, err = cache.Verifying(ctx, expected.Sequence)
270-
require.Error(t, err)
288+
require.ErrorIs(t, err, cryptokeys.ErrKeyInvalid)
271289
require.Equal(t, 1, fc.called)
272290
})
291+
292+
t.Run("KeyNotFound", func(t *testing.T) {
293+
t.Parallel()
294+
295+
var (
296+
ctx = testutil.Context(t, testutil.WaitShort)
297+
logger = slogtest.Make(t, nil)
298+
clock = quartz.NewMock(t)
299+
)
300+
301+
fc := newFakeCoderd(t, []codersdk.CryptoKey{})
302+
303+
cache, err := wsproxy.NewCryptoKeyCache(ctx, logger, wsproxysdk.New(fc.url), withClock(clock))
304+
require.NoError(t, err)
305+
306+
_, err = cache.Verifying(ctx, 1)
307+
require.ErrorIs(t, err, cryptokeys.ErrKeyNotFound)
308+
})
273309
})
274310

275311
t.Run("CacheRefreshes", func(t *testing.T) {
@@ -342,11 +378,10 @@ func TestCryptoKeyCache(t *testing.T) {
342378

343379
now := clock.Now()
344380
expected := codersdk.CryptoKey{
345-
Feature: codersdk.CryptoKeyFeatureWorkspaceApp,
346-
Secret: "key1",
347-
Sequence: 12,
348-
StartsAt: now,
349-
DeletesAt: now.Add(time.Minute * 10),
381+
Feature: codersdk.CryptoKeyFeatureWorkspaceApp,
382+
Secret: "key1",
383+
Sequence: 12,
384+
StartsAt: now,
350385
}
351386
fc := newFakeCoderd(t, []codersdk.CryptoKey{
352387
expected,

0 commit comments

Comments
 (0)