address more review feedback; attempt to simplify cached workspace is

cstyan · cstyan · commit 55ee6d317816 · 2025-11-21T17:54:31.000Z
valid/present checks, plus ensure proper fallback (less noisy logging)
when a cached workspace is not found/valid in dbauthz

Signed-off-by: Callum Styan &lt;callumstyan@gmail.com&gt;
diff --git a/coderd/agentapi/cached_workspace.go b/coderd/agentapi/cached_workspace.go
@@ -73,9 +73,14 @@ func (cws *CachedWorkspaceFields) UpdateValues(ws database.Workspace) {
 	cws.AutostartSchedule = ws.AutostartSchedule
 }
 
-func (cws *CachedWorkspaceFields) AsDatabaseWorkspace() database.Workspace {
+// Returns the Workspace, true, unless the workspace has not been cached (nuked or was a prebuild).
+func (cws *CachedWorkspaceFields) AsDatabaseWorkspace() (database.Workspace, bool) {
 	cws.lock.RLock()
 	defer cws.lock.RUnlock()
+	// Should we be more explicit about all fields being set to be valid?
+	if cws.Equal(&CachedWorkspaceFields{}) {
+		return database.Workspace{}, false
+	}
 	return database.Workspace{
 		ID:                cws.ID,
 		OwnerID:           cws.OwnerID,
@@ -85,5 +90,5 @@ func (cws *CachedWorkspaceFields) AsDatabaseWorkspace() database.Workspace {
 		OwnerUsername:     cws.OwnerUsername,
 		TemplateName:      cws.TemplateName,
 		AutostartSchedule: cws.AutostartSchedule,
-	}
+	}, true
 }
diff --git a/coderd/agentapi/metadata.go b/coderd/agentapi/metadata.go
@@ -111,12 +111,11 @@ func (a *MetadataAPI) BatchUpdateMetadata(ctx context.Context, req *agentproto.B
 
 	// Inject RBAC object into context for dbauthz fast path, avoid having to
 	// call GetWorkspaceByAgentID on every metadata update.
-	rbacCtx, err := dbauthz.WithWorkspaceRBAC(ctx, a.Workspace.AsDatabaseWorkspace().RBACObject())
-	if err != nil {
-		// Don't use error level here; that will fail the call but in reality an invalid object here is okay
-		// since we will then just fallback to the actual GetWorkspaceByAgentID call in dbauthz.
-		a.Log.Warn(ctx, "cached RBAC Workspace context wrapping was invalid", slog.Error(err))
+	rbacCtx := ctx
+	if dbws, ok := a.Workspace.AsDatabaseWorkspace(); ok {
+		rbacCtx, err = dbauthz.WithWorkspaceRBAC(ctx, dbws.RBACObject())
 	}
+
 	err = a.Database.UpdateWorkspaceAgentMetadata(rbacCtx, dbUpdate)
 	if err != nil {
 		return nil, xerrors.Errorf("update workspace agent metadata in database: %w", err)
diff --git a/coderd/agentapi/stats.go b/coderd/agentapi/stats.go
@@ -4,7 +4,6 @@ import (
 	"context"
 	"time"
 
-	"github.com/google/uuid"
 	"golang.org/x/xerrors"
 	"google.golang.org/protobuf/types/known/durationpb"
 
@@ -50,8 +49,9 @@ func (a *StatsAPI) UpdateStats(ctx context.Context, req *agentproto.UpdateStatsR
 	}
 
 	// If cache is empty (prebuild or invalid), fall back to DB
-	ws := a.Workspace.AsDatabaseWorkspace()
-	if a.Workspace.ID == uuid.Nil {
+	var ws database.Workspace
+	var ok bool
+	if ws, ok = a.Workspace.AsDatabaseWorkspace(); !ok {
 		ws, err = a.Database.GetWorkspaceByAgentID(ctx, workspaceAgent.ID)
 		if err != nil {
 			return nil, xerrors.Errorf("get workspace by agent ID %q: %w", workspaceAgent.ID, err)
