fix: fix tailnet resume using incorrect DB reference (#15522) (#15574)

sreya · web-flow · commit dbfadf2b73e5 · 2024-11-18T14:27:55.000-06:00
- We were instantiating a cryptokey cache with a vanilla reference to
the database instead of one wrapped by dbcrypt.
- Fixes an issue where failing to instantiate unrelated keycaches does
not fatally error out.
diff --git a/cli/server.go b/cli/server.go
@@ -61,7 +61,6 @@ import (
 	"github.com/coder/serpent"
 	"github.com/coder/wgtunnel/tunnelsdk"
 
-	"github.com/coder/coder/v2/coderd/cryptokeys"
 	"github.com/coder/coder/v2/coderd/entitlements"
 	"github.com/coder/coder/v2/coderd/notifications/reports"
 	"github.com/coder/coder/v2/coderd/runtimeconfig"
@@ -754,25 +753,6 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
 				return xerrors.Errorf("set deployment id: %w", err)
 			}
 
-			fetcher := &cryptokeys.DBFetcher{
-				DB: options.Database,
-			}
-
-			resumeKeycache, err := cryptokeys.NewSigningCache(ctx,
-				logger,
-				fetcher,
-				codersdk.CryptoKeyFeatureTailnetResume,
-			)
-			if err != nil {
-				logger.Critical(ctx, "failed to properly instantiate tailnet resume signing cache", slog.Error(err))
-			}
-
-			options.CoordinatorResumeTokenProvider = tailnet.NewResumeTokenKeyProvider(
-				resumeKeycache,
-				quartz.NewReal(),
-				tailnet.DefaultResumeTokenExpiry,
-			)
-
 			options.RuntimeConfig = runtimeconfig.NewManager()
 
 			// This should be output before the logs start streaming.
diff --git a/coderd/coderd.go b/coderd/coderd.go
@@ -467,7 +467,7 @@ func New(options *Options) *API {
 			codersdk.CryptoKeyFeatureOIDCConvert,
 		)
 		if err != nil {
-			options.Logger.Critical(ctx, "failed to properly instantiate oidc convert signing cache", slog.Error(err))
+			options.Logger.Fatal(ctx, "failed to properly instantiate oidc convert signing cache", slog.Error(err))
 		}
 	}
 
@@ -478,7 +478,7 @@ func New(options *Options) *API {
 			codersdk.CryptoKeyFeatureWorkspaceAppsToken,
 		)
 		if err != nil {
-			options.Logger.Critical(ctx, "failed to properly instantiate app signing key cache", slog.Error(err))
+			options.Logger.Fatal(ctx, "failed to properly instantiate app signing key cache", slog.Error(err))
 		}
 	}
 
@@ -489,10 +489,30 @@ func New(options *Options) *API {
 			codersdk.CryptoKeyFeatureWorkspaceAppsAPIKey,
 		)
 		if err != nil {
-			options.Logger.Critical(ctx, "failed to properly instantiate app encryption key cache", slog.Error(err))
+			options.Logger.Fatal(ctx, "failed to properly instantiate app encryption key cache", slog.Error(err))
 		}
 	}
 
+	if options.CoordinatorResumeTokenProvider == nil {
+		fetcher := &cryptokeys.DBFetcher{
+			DB: options.Database,
+		}
+
+		resumeKeycache, err := cryptokeys.NewSigningCache(ctx,
+			options.Logger,
+			fetcher,
+			codersdk.CryptoKeyFeatureTailnetResume,
+		)
+		if err != nil {
+			options.Logger.Fatal(ctx, "failed to properly instantiate tailnet resume signing cache", slog.Error(err))
+		}
+		options.CoordinatorResumeTokenProvider = tailnet.NewResumeTokenKeyProvider(
+			resumeKeycache,
+			options.Clock,
+			tailnet.DefaultResumeTokenExpiry,
+		)
+	}
+
 	// Start a background process that rotates keys. We intentionally start this after the caches
 	// are created to force initial requests for a key to populate the caches. This helps catch
 	// bugs that may only occur when a key isn't precached in tests and the latency cost is minimal.

Original file line number	Diff line number	Diff line change
`@@ -467,7 +467,7 @@ func New(options Options) API {`
`467`	`467`	`codersdk.CryptoKeyFeatureOIDCConvert,`
`468`	`468`	`)`
`469`	`469`	`if err != nil {`
`470`		`- options.Logger.Critical(ctx, "failed to properly instantiate oidc convert signing cache", slog.Error(err))`
	`470`	`+ options.Logger.Fatal(ctx, "failed to properly instantiate oidc convert signing cache", slog.Error(err))`
`471`	`471`	`}`
`472`	`472`	`}`
`473`	`473`
`@@ -478,7 +478,7 @@ func New(options Options) API {`
`478`	`478`	`codersdk.CryptoKeyFeatureWorkspaceAppsToken,`
`479`	`479`	`)`
`480`	`480`	`if err != nil {`
`481`		`- options.Logger.Critical(ctx, "failed to properly instantiate app signing key cache", slog.Error(err))`
	`481`	`+ options.Logger.Fatal(ctx, "failed to properly instantiate app signing key cache", slog.Error(err))`
`482`	`482`	`}`
`483`	`483`	`}`
`484`	`484`
`@@ -489,10 +489,30 @@ func New(options Options) API {`
`489`	`489`	`codersdk.CryptoKeyFeatureWorkspaceAppsAPIKey,`
`490`	`490`	`)`
`491`	`491`	`if err != nil {`
`492`		`- options.Logger.Critical(ctx, "failed to properly instantiate app encryption key cache", slog.Error(err))`
	`492`	`+ options.Logger.Fatal(ctx, "failed to properly instantiate app encryption key cache", slog.Error(err))`
`493`	`493`	`}`
`494`	`494`	`}`
`495`	`495`
	`496`	`+ if options.CoordinatorResumeTokenProvider == nil {`
	`497`	`+ fetcher := &cryptokeys.DBFetcher{`
	`498`	`+ DB: options.Database,`
	`499`	`+ }`
	`500`	`+`
	`501`	`+ resumeKeycache, err := cryptokeys.NewSigningCache(ctx,`
	`502`	`+ options.Logger,`
	`503`	`+ fetcher,`
	`504`	`+ codersdk.CryptoKeyFeatureTailnetResume,`
	`505`	`+ )`
	`506`	`+ if err != nil {`
	`507`	`+ options.Logger.Fatal(ctx, "failed to properly instantiate tailnet resume signing cache", slog.Error(err))`
	`508`	`+ }`
	`509`	`+ options.CoordinatorResumeTokenProvider = tailnet.NewResumeTokenKeyProvider(`
	`510`	`+ resumeKeycache,`
	`511`	`+ options.Clock,`
	`512`	`+ tailnet.DefaultResumeTokenExpiry,`
	`513`	`+ )`
	`514`	`+ }`
	`515`	`+`
`496`	`516`	`// Start a background process that rotates keys. We intentionally start this after the caches`
`497`	`517`	`// are created to force initial requests for a key to populate the caches. This helps catch`
`498`	`518`	`// bugs that may only occur when a key isn't precached in tests and the latency cost is minimal.`