Skip to content

Commit 08570b7

Browse files
sreyasreya
committed
Refactor key caching and logging behavior
- Improve code clarity by refactoring key caching logic. - Simplify logger initialization for signing and encryption caches. - Ensure consistent closing of caches in the API and server.
1 parent 33cdb96 commit 08570b7

File tree

6 files changed

+46
-14
lines changed

6 files changed

+46
-14
lines changed

‎coderd/activitybump_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ func TestWorkspaceActivityBump(t *testing.T) {
125125
}
126126

127127
// maxTimeDrift is how long we are willing wait for a deadline to
128-
// be increased. Since it could have been bumped at the intial
128+
// be increased. Since it could have been bumped at the initial
129129
maxTimeDrift := testutil.WaitMedium
130130

131131
updatedAfter := dbtime.Now()

‎coderd/coderd.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -461,7 +461,7 @@ func New(options *Options) *API {
461461

462462
if options.OIDCConvertKeyCache == nil {
463463
options.OIDCConvertKeyCache, err = cryptokeys.NewSigningCache(ctx,
464-
options.Logger.Named("oidc_convert_keycache"),
464+
options.Logger,
465465
fetcher,
466466
codersdk.CryptoKeyFeatureOIDCConvert,
467467
)
@@ -470,7 +470,7 @@ func New(options *Options) *API {
470470

471471
if options.AppSigningKeyCache == nil {
472472
options.AppSigningKeyCache, err = cryptokeys.NewSigningCache(ctx,
473-
options.Logger.Named("app_signing_keycache"),
473+
options.Logger,
474474
fetcher,
475475
codersdk.CryptoKeyFeatureWorkspaceAppsToken,
476476
)
@@ -479,7 +479,7 @@ func New(options *Options) *API {
479479

480480
if options.AppEncryptionKeyCache == nil {
481481
options.AppEncryptionKeyCache, err = cryptokeys.NewEncryptionCache(ctx,
482-
options.Logger.Named("app_encryption_keycache"),
482+
options.Logger,
483483
fetcher,
484484
codersdk.CryptoKeyFeatureWorkspaceAppsAPIKey,
485485
)
@@ -522,7 +522,7 @@ func New(options *Options) *API {
522522
options.Database,
523523
options.Pubsub,
524524
),
525-
dbRolluper: options.DatabaseRolluper,
525+
dbRolluper: options.DatabaseRolluper,
526526
}
527527

528528
f := appearance.NewDefaultFetcher(api.DeploymentValues.DocsURL.String())
@@ -1474,6 +1474,9 @@ func (api *API) Close() error {
14741474
_ = api.agentProvider.Close()
14751475
_ = api.statsReporter.Close()
14761476
_ = api.NetworkTelemetryBatcher.Close()
1477+
_ = api.OIDCConvertKeyCache.Close()
1478+
_ = api.AppSigningKeyCache.Close()
1479+
_ = api.AppEncryptionKeyCache.Close()
14771480
return nil
14781481
}
14791482

‎coderd/cryptokeys/cache.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package cryptokeys
33
import (
44
"context"
55
"encoding/hex"
6+
"fmt"
67
"io"
78
"strconv"
89
"sync"
@@ -108,6 +109,7 @@ func NewSigningCache(ctx context.Context, logger slog.Logger, fetcher Fetcher,
108109
if !isSigningKeyFeature(feature) {
109110
return nil, xerrors.Errorf("invalid feature: %s", feature)
110111
}
112+
logger = logger.Named(fmt.Sprintf("%s_signing_keycache", feature))
111113
return newCache(ctx, logger, fetcher, feature, opts...)
112114
}
113115

@@ -117,6 +119,7 @@ func NewEncryptionCache(ctx context.Context, logger slog.Logger, fetcher Fetcher
117119
if !isEncryptionKeyFeature(feature) {
118120
return nil, xerrors.Errorf("invalid feature: %s", feature)
119121
}
122+
logger = logger.Named(fmt.Sprintf("%s_encryption_keycache", feature))
120123
return newCache(ctx, logger, fetcher, feature, opts...)
121124
}
122125

‎enterprise/coderd/coderdenttest/proxytest.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,8 @@ type WorkspaceProxy struct {
6565
// owner client. If a token is provided, the proxy will become a replica of the
6666
// existing proxy region.
6767
func NewWorkspaceProxyReplica(t *testing.T, coderdAPI *coderd.API, owner *codersdk.Client, options *ProxyOptions) WorkspaceProxy {
68+
t.Helper()
69+
6870
ctx, cancelFunc := context.WithCancel(context.Background())
6971
t.Cleanup(cancelFunc)
7072

@@ -142,8 +144,10 @@ func NewWorkspaceProxyReplica(t *testing.T, coderdAPI *coderd.API, owner *coders
142144
statsCollectorOptions.Flush = options.FlushStats
143145
}
144146

147+
logger := slogtest.Make(t, nil).Leveled(slog.LevelDebug).With(slog.F("server_url", serverURL.String()))
148+
145149
wssrv, err := wsproxy.New(ctx, &wsproxy.Options{
146-
Logger: slogtest.Make(t, nil).Leveled(slog.LevelDebug).With(slog.F("server_url", serverURL.String())),
150+
Logger: logger,
147151
Experiments: options.Experiments,
148152
DashboardURL: coderdAPI.AccessURL,
149153
AccessURL: accessURL,

‎enterprise/wsproxy/keyfetcher.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,11 @@ package wsproxy
33
import (
44
"context"
55

6+
"golang.org/x/xerrors"
7+
68
"github.com/coder/coder/v2/coderd/cryptokeys"
79
"github.com/coder/coder/v2/codersdk"
810
"github.com/coder/coder/v2/enterprise/wsproxy/wsproxysdk"
9-
"golang.org/x/xerrors"
1011
)
1112

1213
var _ cryptokeys.Fetcher = &ProxyFetcher{}

‎enterprise/wsproxy/wsproxy.go

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,7 @@ type Options struct {
9393
// from the dashboardURL. This should only be used in development.
9494
AllowAllCors bool
9595

96-
StatsCollectorOptions workspaceapps.StatsCollectorOptions
97-
WorkspaceAppsEncryptionKeycache cryptokeys.EncryptionKeycache
98-
WorkspaceAppsSigningKeycache cryptokeys.SigningKeycache
96+
StatsCollectorOptions workspaceapps.StatsCollectorOptions
9997
}
10098

10199
func (o *Options) Validate() error {
@@ -133,6 +131,9 @@ type Server struct {
133131
// the moon's token.
134132
SDKClient *wsproxysdk.Client
135133

134+
WorkspaceAppsEncryptionKeycache cryptokeys.EncryptionKeycache
135+
WorkspaceAppsSigningKeycache cryptokeys.SigningKeycache
136+
136137
// DERP
137138
derpMesh *derpmesh.Mesh
138139
derpMeshTLSConfig *tls.Config
@@ -199,8 +200,28 @@ func New(ctx context.Context, opts *Options) (*Server, error) {
199200

200201
ctx, cancel := context.WithCancel(context.Background())
201202

203+
encryptionCache, err := cryptokeys.NewEncryptionCache(ctx,
204+
opts.Logger,
205+
&ProxyFetcher{Client: client},
206+
codersdk.CryptoKeyFeatureWorkspaceAppsAPIKey,
207+
)
208+
if err != nil {
209+
return nil, xerrors.Errorf("create api key encryption cache: %w", err)
210+
}
211+
signingCache, err := cryptokeys.NewSigningCache(ctx,
212+
opts.Logger,
213+
&ProxyFetcher{Client: client},
214+
codersdk.CryptoKeyFeatureWorkspaceAppsToken,
215+
)
216+
if err != nil {
217+
return nil, xerrors.Errorf("create api token signing cache: %w", err)
218+
}
219+
202220
r := chi.NewRouter()
203221
s := &Server{
222+
ctx: ctx,
223+
cancel: cancel,
224+
204225
Options: opts,
205226
Handler: r,
206227
DashboardURL: opts.DashboardURL,
@@ -210,8 +231,6 @@ func New(ctx context.Context, opts *Options) (*Server, error) {
210231
SDKClient: client,
211232
derpMesh: derpmesh.New(opts.Logger.Named("net.derpmesh"), derpServer, meshTLSConfig),
212233
derpMeshTLSConfig: meshTLSConfig,
213-
ctx: ctx,
214-
cancel: cancel,
215234
}
216235

217236
// Register the workspace proxy with the primary coderd instance and start a
@@ -280,8 +299,8 @@ func New(ctx context.Context, opts *Options) (*Server, error) {
280299
AccessURL: opts.AccessURL,
281300
AppHostname: opts.AppHostname,
282301
Client: client,
283-
SigningKey: opts.WorkspaceAppsSigningKeycache,
284-
EncryptingKey: opts.WorkspaceAppsEncryptionKeycache,
302+
SigningKey: signingCache,
303+
EncryptingKey: encryptionCache,
285304
Logger: s.Logger.Named("proxy_token_provider"),
286305
},
287306

@@ -432,6 +451,8 @@ func (s *Server) Close() error {
432451
err = multierror.Append(err, agentProviderErr)
433452
}
434453
s.SDKClient.SDKClient.HTTPClient.CloseIdleConnections()
454+
_ = s.WorkspaceAppsSigningKeycache.Close()
455+
_ = s.WorkspaceAppsEncryptionKeycache.Close()
435456
return err
436457
}
437458

0 commit comments

Comments
 (0)