From 053b38944d1d8c3916f17c9f972c1167fcb25fa4 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 10 Aug 2026 14:53:48 -0500 Subject: [PATCH] fix(coderd): render collected_at as UTC RFC3339 in the agent metadata aggregate (#27991) Follow-up to #27934; this fix was pushed to the branch after the squash-merge and missed it. `jsonb_build_object` renders timestamptz in the session `TimeZone`, which Coder never pins, and `collected_at` defaults to year 1 until the agent's first report. On a non-UTC Postgres session a registered-but-never-collected item renders with an LMT second-offset (even `BC`, e.g. `0001-12-31T19:03:58-04:56:02 BC`), which Go's RFC3339 parsing rejects - a 500 for the entire list page whenever `include_agent_metadata` is used. - `to_char(... AT TIME ZONE 'UTC', ...)` pins the rendering; never-collected items round-trip as Go's zero time. - The test now runs against a named-zone database (`dbtestutil.WithTimezone("America/Caracas")`) and requests a registered but never-collected key; it reproduces the 500 without the fix. Also contains the failure mode Go-side: an unparsable aggregate now degrades to missing metadata for that workspace (with a warning log) instead of failing the entire page. The SQL fix prevents the known cause; the containment covers any future one. The test still catches regressions because it asserts the metadata values, not just a 200. --- Authored by Coder Agents on behalf of @Emyrk. --- coderd/database/queries.sql.go | 6 +++- coderd/database/queries/workspaces.sql | 6 +++- coderd/workspaces.go | 23 ++++++------ coderd/workspaces_test.go | 49 +++++++++++++++++--------- 4 files changed, 54 insertions(+), 30 deletions(-) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 7a33169b35..f439f016a0 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -38914,7 +38914,11 @@ SELECT 'error', workspace_agent_metadata.error, 'timeout', workspace_agent_metadata.timeout, 'interval', workspace_agent_metadata.interval, - 'collected_at', workspace_agent_metadata.collected_at, + -- Rendered explicitly as UTC RFC3339: jsonb renders + -- timestamptz in the session TimeZone, and the year-1 + -- default of collected_at renders named zones with + -- LMT second-offsets (even BC), which Go rejects. + 'collected_at', to_char(workspace_agent_metadata.collected_at AT TIME ZONE 'UTC', 'YYYY-MM-DD"T"HH24:MI:SS.US"Z"'), 'display_order', workspace_agent_metadata.display_order )) FROM diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index 42cdec235a..15251b56ca 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -494,7 +494,11 @@ SELECT 'error', workspace_agent_metadata.error, 'timeout', workspace_agent_metadata.timeout, 'interval', workspace_agent_metadata.interval, - 'collected_at', workspace_agent_metadata.collected_at, + -- Rendered explicitly as UTC RFC3339: jsonb renders + -- timestamptz in the session TimeZone, and the year-1 + -- default of collected_at renders named zones with + -- LMT second-offsets (even BC), which Go rejects. + 'collected_at', to_char(workspace_agent_metadata.collected_at AT TIME ZONE 'UTC', 'YYYY-MM-DD"T"HH24:MI:SS.US"Z"'), 'display_order', workspace_agent_metadata.display_order )) FROM diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 8d9f1c4c1a..21d11d88b7 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -249,14 +249,7 @@ func (api *API) workspaces(rw http.ResponseWriter, r *http.Request) { } if len(filter.IncludeAgentMetadata) > 0 { - err = attachAgentMetadata(wss, workspaceRows) - if err != nil { - httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ - Message: "Internal error converting agent metadata.", - Detail: err.Error(), - }) - return - } + attachAgentMetadata(ctx, api.Logger, wss, workspaceRows) } httpapi.Write(ctx, rw, http.StatusOK, codersdk.WorkspacesResponse{ @@ -2771,8 +2764,11 @@ func (api *API) workspaceData(ctx context.Context, workspaces []database.Workspa // attachAgentMetadata maps the agent metadata the workspaces query // aggregated per workspace onto the agents in the converted response. -// Each aggregated datum carries its workspace_agent_id. -func attachAgentMetadata(workspaces []codersdk.Workspace, rows []database.GetWorkspacesRow) error { +// Each aggregated datum carries its workspace_agent_id. An unparsable +// aggregate degrades to missing metadata for that workspace rather +// than failing the page: the expansion is best-effort decoration on +// top of the list. +func attachAgentMetadata(ctx context.Context, logger slog.Logger, workspaces []codersdk.Workspace, rows []database.GetWorkspacesRow) { byAgent := map[uuid.UUID][]database.WorkspaceAgentMetadatum{} for _, row := range rows { if len(row.AgentMetadata) == 0 { @@ -2781,7 +2777,11 @@ func attachAgentMetadata(workspaces []codersdk.Workspace, rows []database.GetWor var metadata database.AgentMetadataAggregate err := metadata.Scan(row.AgentMetadata) if err != nil { - return xerrors.Errorf("scan agent metadata for workspace %q: %w", row.ID, err) + logger.Warn(ctx, "scan agent metadata, omitting it for the workspace", + slog.F("workspace_id", row.ID), + slog.Error(err), + ) + continue } for _, datum := range metadata { byAgent[datum.WorkspaceAgentID] = append(byAgent[datum.WorkspaceAgentID], datum) @@ -2798,7 +2798,6 @@ func attachAgentMetadata(workspaces []codersdk.Workspace, rows []database.GetWor } } } - return nil } func convertWorkspaces( diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index 1c75bd7de5..fdf33461af 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -2872,10 +2872,18 @@ func TestWorkspaceFilterManual(t *testing.T) { t.Run("IncludeAgentMetadata", func(t *testing.T) { t.Parallel() - client, db := coderdtest.NewWithDatabase(t, nil) + // A named non-UTC zone on purpose: jsonb renders timestamptz + // in the session TimeZone, and the year-1 collected_at + // default renders named zones with LMT second-offsets that Go + // refuses to parse unless the query pins UTC. + store, ps := dbtestutil.NewDB(t, dbtestutil.WithTimezone("America/Caracas")) + client := coderdtest.New(t, &coderdtest.Options{ + Database: store, + Pubsub: ps, + }) user := coderdtest.CreateFirstUser(t, client) - build := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ + build := dbfake.WorkspaceBuild(t, store, database.WorkspaceTable{ OrganizationID: user.OrganizationID, OwnerID: user.UserID, }).WithAgent().Do() @@ -2887,9 +2895,10 @@ func TestWorkspaceFilterManual(t *testing.T) { collectedAt := dbtime.Now() // Task_Status is mixed-case on purpose: requested keys are // lowercased by the search parser, and the query matches stored - // keys case-insensitively. - for i, key := range []string{"Task_Status", "cpu", "unrequested"} { - err := db.InsertWorkspaceAgentMetadata(ctx, database.InsertWorkspaceAgentMetadataParams{ + // keys case-insensitively. uncollected is registered but never + // reported, so it keeps the year-1 collected_at default. + for i, key := range []string{"Task_Status", "cpu", "unrequested", "uncollected"} { + err := store.InsertWorkspaceAgentMetadata(ctx, database.InsertWorkspaceAgentMetadataParams{ WorkspaceAgentID: agentID, DisplayName: key, Key: key, @@ -2898,10 +2907,13 @@ func TestWorkspaceFilterManual(t *testing.T) { Interval: int64(time.Second), // Reversed so the response order proves display_order // sorting rather than insertion order. - DisplayOrder: int32(3 - i), //nolint:gosec // Tiny test constant. + DisplayOrder: int32(4 - i), //nolint:gosec // Tiny test constant. }) require.NoError(t, err) - err = db.UpdateWorkspaceAgentMetadata(ctx, database.UpdateWorkspaceAgentMetadataParams{ + if key == "uncollected" { + continue + } + err = store.UpdateWorkspaceAgentMetadata(ctx, database.UpdateWorkspaceAgentMetadataParams{ WorkspaceAgentID: agentID, Key: []string{key}, Value: []string{"value-" + key}, @@ -2927,21 +2939,26 @@ func TestWorkspaceFilterManual(t *testing.T) { require.Empty(t, findAgent(res).Metadata) // Opting in returns exactly the requested keys, ordered by - // display_order, with their collected values. + // display_order, with their collected values. The uncollected + // item must round-trip as Go's zero time rather than breaking + // the page. res, err = client.Workspaces(reqCtx, codersdk.WorkspaceFilter{ - IncludeAgentMetadata: []string{"task_status", "cpu"}, + IncludeAgentMetadata: []string{"task_status", "cpu", "uncollected"}, }) require.NoError(t, err) metadata := findAgent(res).Metadata - require.Len(t, metadata, 2) - require.Equal(t, "cpu", metadata[0].Description.Key) - require.Equal(t, "value-cpu", metadata[0].Result.Value) + require.Len(t, metadata, 3) + require.Equal(t, "uncollected", metadata[0].Description.Key) + require.Empty(t, metadata[0].Result.Value) + require.True(t, metadata[0].Result.CollectedAt.IsZero()) + require.Equal(t, "cpu", metadata[1].Description.Key) + require.Equal(t, "value-cpu", metadata[1].Result.Value) // The collection script is deliberately not exposed on the list // endpoint; it can be long. - require.Empty(t, metadata[0].Description.Script) - require.Equal(t, "Task_Status", metadata[1].Description.Key) - require.Equal(t, "value-Task_Status", metadata[1].Result.Value) - require.WithinDuration(t, collectedAt, metadata[1].Result.CollectedAt, time.Second) + require.Empty(t, metadata[1].Description.Script) + require.Equal(t, "Task_Status", metadata[2].Description.Key) + require.Equal(t, "value-Task_Status", metadata[2].Result.Value) + require.WithinDuration(t, collectedAt, metadata[2].Result.CollectedAt, time.Second) // Unknown keys are not an error; the metadata is just absent. res, err = client.Workspaces(reqCtx, codersdk.WorkspaceFilter{