Skip to content

Commit 33cdb96

Browse files
sreyasreya
committed
Refactor cryptokeys Fetcher to include feature param
Enhances flexibility by making the `Fetcher` interface receive a `CryptoKeyFeature` parameter. This change aligns various call sites that implement or utilize `Fetcher`, allowing for more granular queries.
1 parent d0d168b commit 33cdb96

File tree

9 files changed

+72
-38
lines changed

9 files changed

+72
-38
lines changed

‎cli/server.go

+11-2
Original file line numberDiff line numberDiff line change
@@ -746,9 +746,18 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
746746
return xerrors.Errorf("set deployment id: %w", err)
747747
}
748748

749-
resumeKeycache, err := cryptokeys.NewSigningCache(logger, options.Database, database.CryptoKeyFeatureTailnetResume)
749+
fetcher := &cryptokeys.DBFetcher{
750+
DB: options.Database,
751+
}
752+
753+
// TODO(JonA): The instantiation of this cache + coordinator seems like it should be done inside coderd so that it uses the correct context.
754+
resumeKeycache, err := cryptokeys.NewSigningCache(ctx,
755+
logger,
756+
fetcher,
757+
codersdk.CryptoKeyFeatureTailnetResume,
758+
)
750759
if err != nil {
751-
return xerrors.Errorf("create resume token key cache: %w", err)
760+
return xerrors.Errorf("create tailnet resume key cache: %w", err)
752761
}
753762

754763
options.CoordinatorResumeTokenProvider = tailnet.NewResumeTokenKeyProvider(resumeKeycache, quartz.NewReal(), tailnet.DefaultResumeTokenExpiry)

‎coderd/coderd.go

+45-7
Original file line numberDiff line numberDiff line change
@@ -448,6 +448,44 @@ func New(options *Options) *API {
448448
if err != nil {
449449
panic(xerrors.Errorf("get deployment ID: %w", err))
450450
}
451+
452+
// Start a background process that rotates keys.
453+
err = cryptokeys.StartRotator(ctx, options.Logger.Named("keyrotator"), options.Database)
454+
if err != nil {
455+
options.Logger.Fatal(ctx, "start key rotator", slog.Error(err))
456+
}
457+
458+
fetcher := &cryptokeys.DBFetcher{
459+
DB: options.Database,
460+
}
461+
462+
if options.OIDCConvertKeyCache == nil {
463+
options.OIDCConvertKeyCache, err = cryptokeys.NewSigningCache(ctx,
464+
options.Logger.Named("oidc_convert_keycache"),
465+
fetcher,
466+
codersdk.CryptoKeyFeatureOIDCConvert,
467+
)
468+
must(options.Logger, "start oidc convert key cache", err)
469+
}
470+
471+
if options.AppSigningKeyCache == nil {
472+
options.AppSigningKeyCache, err = cryptokeys.NewSigningCache(ctx,
473+
options.Logger.Named("app_signing_keycache"),
474+
fetcher,
475+
codersdk.CryptoKeyFeatureWorkspaceAppsToken,
476+
)
477+
must(options.Logger, "start app signing key cache", err)
478+
}
479+
480+
if options.AppEncryptionKeyCache == nil {
481+
options.AppEncryptionKeyCache, err = cryptokeys.NewEncryptionCache(ctx,
482+
options.Logger.Named("app_encryption_keycache"),
483+
fetcher,
484+
codersdk.CryptoKeyFeatureWorkspaceAppsAPIKey,
485+
)
486+
must(options.Logger, "start app encryption key cache", err)
487+
}
488+
451489
api := &API{
452490
ctx: ctx,
453491
cancel: cancel,
@@ -484,7 +522,7 @@ func New(options *Options) *API {
484522
options.Database,
485523
options.Pubsub,
486524
),
487-
dbRolluper: options.DatabaseRolluper,
525+
dbRolluper: options.DatabaseRolluper,
488526
}
489527

490528
f := appearance.NewDefaultFetcher(api.DeploymentValues.DocsURL.String())
@@ -613,12 +651,6 @@ func New(options *Options) *API {
613651
api.Logger.Fatal(api.ctx, "failed to initialize tailnet client service", slog.Error(err))
614652
}
615653

616-
// Start a background process that rotates keys.
617-
err = cryptokeys.StartRotator(api.ctx, api.Logger.Named("keyrotator"), api.Database)
618-
if err != nil {
619-
api.Logger.Fatal(api.ctx, "start key rotator", slog.Error(err))
620-
}
621-
622654
api.statsReporter = workspacestats.NewReporter(workspacestats.ReporterOptions{
623655
Database: options.Database,
624656
Logger: options.Logger.Named("workspacestats"),
@@ -1612,3 +1644,9 @@ func ReadExperiments(log slog.Logger, raw []string) codersdk.Experiments {
16121644
}
16131645
return exps
16141646
}
1647+
1648+
func must(logger slog.Logger, msg string, err error) {
1649+
if err != nil {
1650+
logger.Fatal(context.Background(), msg, slog.Error(err))
1651+
}
1652+
}

‎coderd/coderdtest/coderdtest.go

-11
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,6 @@ import (
5555
"github.com/coder/coder/v2/coderd/audit"
5656
"github.com/coder/coder/v2/coderd/autobuild"
5757
"github.com/coder/coder/v2/coderd/awsidentity"
58-
"github.com/coder/coder/v2/coderd/cryptokeys"
5958
"github.com/coder/coder/v2/coderd/database"
6059
"github.com/coder/coder/v2/coderd/database/db2sdk"
6160
"github.com/coder/coder/v2/coderd/database/dbauthz"
@@ -326,13 +325,6 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can
326325
}
327326
auditor.Store(&options.Auditor)
328327

329-
oidcConvertKeyCache, err := cryptokeys.NewSigningCache(options.Logger.Named("oidc_convert_keycache"), options.Database, database.CryptoKeyFeatureOIDCConvert)
330-
require.NoError(t, err)
331-
appSigningKeyCache, err := cryptokeys.NewSigningCache(options.Logger.Named("app_signing_keycache"), options.Database, database.CryptoKeyFeatureWorkspaceAppsToken)
332-
require.NoError(t, err)
333-
appEncryptionKeyCache, err := cryptokeys.NewEncryptionCache(options.Logger.Named("app_encryption_keycache"), options.Database, database.CryptoKeyFeatureWorkspaceAppsAPIKey)
334-
require.NoError(t, err)
335-
336328
ctx, cancelFunc := context.WithCancel(context.Background())
337329
lifecycleExecutor := autobuild.NewExecutor(
338330
ctx,
@@ -540,9 +532,6 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can
540532
WorkspaceUsageTracker: wuTracker,
541533
NotificationsEnqueuer: options.NotificationsEnqueuer,
542534
OneTimePasscodeValidityPeriod: options.OneTimePasscodeValidityPeriod,
543-
AppSigningKeyCache: appSigningKeyCache,
544-
AppEncryptionKeyCache: appEncryptionKeyCache,
545-
OIDCConvertKeyCache: oidcConvertKeyCache,
546535
}
547536
}
548537

‎coderd/cryptokeys/cache.go

+6-7
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ var (
2525
)
2626

2727
type Fetcher interface {
28-
Fetch(ctx context.Context) ([]codersdk.CryptoKey, error)
28+
Fetch(ctx context.Context, feature codersdk.CryptoKeyFeature) ([]codersdk.CryptoKey, error)
2929
}
3030

3131
type EncryptionKeycache interface {
@@ -62,12 +62,11 @@ const (
6262
)
6363

6464
type DBFetcher struct {
65-
DB database.Store
66-
Feature database.CryptoKeyFeature
65+
DB database.Store
6766
}
6867

69-
func (d *DBFetcher) Fetch(ctx context.Context) ([]codersdk.CryptoKey, error) {
70-
keys, err := d.DB.GetCryptoKeysByFeature(ctx, d.Feature)
68+
func (d *DBFetcher) Fetch(ctx context.Context, feature codersdk.CryptoKeyFeature) ([]codersdk.CryptoKey, error) {
69+
keys, err := d.DB.GetCryptoKeysByFeature(ctx, database.CryptoKeyFeature(feature))
7170
if err != nil {
7271
return nil, xerrors.Errorf("get crypto keys by feature: %w", err)
7372
}
@@ -198,7 +197,7 @@ func (c *cache) VerifyingKey(ctx context.Context, id string) (interface{}, error
198197
}
199198

200199
func isEncryptionKeyFeature(feature codersdk.CryptoKeyFeature) bool {
201-
return feature == codersdk.CryptoKeyFeatureWorkspaceApp
200+
return feature == codersdk.CryptoKeyFeatureWorkspaceAppsAPIKey
202201
}
203202

204203
func isSigningKeyFeature(feature codersdk.CryptoKeyFeature) bool {
@@ -332,7 +331,7 @@ func (c *cache) refresh() {
332331
// cryptoKeys queries the control plane for the crypto keys.
333332
// Outside of initialization, this should only be called by fetch.
334333
func (c *cache) cryptoKeys(ctx context.Context) (map[int32]codersdk.CryptoKey, error) {
335-
keys, err := c.fetcher.Fetch(ctx)
334+
keys, err := c.fetcher.Fetch(ctx, c.feature)
336335
if err != nil {
337336
return nil, xerrors.Errorf("crypto keys: %w", err)
338337
}

‎coderd/cryptokeys/cache_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -488,7 +488,7 @@ type fakeFetcher struct {
488488
called int
489489
}
490490

491-
func (f *fakeFetcher) Fetch(_ context.Context) ([]codersdk.CryptoKey, error) {
491+
func (f *fakeFetcher) Fetch(_ context.Context, _ codersdk.CryptoKeyFeature) ([]codersdk.CryptoKey, error) {
492492
f.called++
493493
return f.keys, nil
494494
}

‎coderd/jwtutils/jwt_test.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ func TestJWS(t *testing.T) {
240240
StartsAt: time.Now(),
241241
})
242242
log = slogtest.Make(t, nil)
243-
fetcher = &cryptokeys.DBFetcher{DB: db, Feature: database.CryptoKeyFeatureOidcConvert}
243+
fetcher = &cryptokeys.DBFetcher{DB: db}
244244
)
245245

246246
cache, err := cryptokeys.NewSigningCache(ctx, log, fetcher, codersdk.CryptoKeyFeatureOIDCConvert)
@@ -331,10 +331,10 @@ func TestJWE(t *testing.T) {
331331
})
332332
log = slogtest.Make(t, nil)
333333

334-
fetcher = &cryptokeys.DBFetcher{DB: db, Feature: database.CryptoKeyFeatureWorkspaceApps}
334+
fetcher = &cryptokeys.DBFetcher{DB: db}
335335
)
336336

337-
cache, err := cryptokeys.NewEncryptionCache(ctx, log, fetcher, codersdk.CryptoKeyFeatureWorkspaceApp)
337+
cache, err := cryptokeys.NewEncryptionCache(ctx, log, fetcher, codersdk.CryptoKeyFeatureWorkspaceAppsAPIKey)
338338
require.NoError(t, err)
339339

340340
claims := testClaims{

‎codersdk/deployment.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -3109,7 +3109,7 @@ func (c *Client) SSHConfiguration(ctx context.Context) (SSHConfigResponse, error
31093109
type CryptoKeyFeature string
31103110

31113111
const (
3112-
CryptoKeyFeatureWorkspaceAppAPIKey CryptoKeyFeature = "workspace_apps_api_key"
3112+
CryptoKeyFeatureWorkspaceAppsAPIKey CryptoKeyFeature = "workspace_apps_api_key"
31133113
//nolint:gosec // This denotes a type of key, not a literal.
31143114
CryptoKeyFeatureWorkspaceAppsToken CryptoKeyFeature = "workspace_apps_token"
31153115
CryptoKeyFeatureOIDCConvert CryptoKeyFeature = "oidc_convert"

‎enterprise/coderd/workspaceproxy_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -954,7 +954,7 @@ func TestGetCryptoKeys(t *testing.T) {
954954
Name: testutil.GetRandomName(t),
955955
})
956956

957-
keys, err := proxy.SDKClient.CryptoKeys(ctx, codersdk.CryptoKeyFeatureWorkspaceAppAPIKey)
957+
keys, err := proxy.SDKClient.CryptoKeys(ctx, codersdk.CryptoKeyFeatureWorkspaceAppsAPIKey)
958958
require.NoError(t, err)
959959
require.NotEmpty(t, keys)
960960
require.Equal(t, 2, len(keys.CryptoKeys))
@@ -986,7 +986,7 @@ func TestGetCryptoKeys(t *testing.T) {
986986
client := wsproxysdk.New(cclient.URL)
987987
client.SetSessionToken(cclient.SessionToken())
988988

989-
_, err := client.CryptoKeys(ctx, codersdk.CryptoKeyFeatureWorkspaceAppAPIKey)
989+
_, err := client.CryptoKeys(ctx, codersdk.CryptoKeyFeatureWorkspaceAppsAPIKey)
990990
require.Error(t, err)
991991
var sdkErr *codersdk.Error
992992
require.ErrorAs(t, err, &sdkErr)

‎enterprise/wsproxy/keyfetcher.go

+3-4
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,11 @@ import (
1212
var _ cryptokeys.Fetcher = &ProxyFetcher{}
1313

1414
type ProxyFetcher struct {
15-
Client *wsproxysdk.Client
16-
Feature codersdk.CryptoKeyFeature
15+
Client *wsproxysdk.Client
1716
}
1817

19-
func (p *ProxyFetcher) Fetch(ctx context.Context) ([]codersdk.CryptoKey, error) {
20-
keys, err := p.Client.CryptoKeys(ctx)
18+
func (p *ProxyFetcher) Fetch(ctx context.Context, feature codersdk.CryptoKeyFeature) ([]codersdk.CryptoKey, error) {
19+
keys, err := p.Client.CryptoKeys(ctx, feature)
2120
if err != nil {
2221
return nil, xerrors.Errorf("crypto keys: %w", err)
2322
}

0 commit comments

Comments
 (0)