diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 6019bafde4..188928905f 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -12902,6 +12902,9 @@ const docTemplate = `{ }, "count": { "type": "integer" + }, + "count_cap": { + "type": "integer" } } }, @@ -13209,6 +13212,9 @@ const docTemplate = `{ }, "count": { "type": "integer" + }, + "count_cap": { + "type": "integer" } } }, diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 8709d6ec7f..88e1fafca4 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -11550,6 +11550,9 @@ }, "count": { "type": "integer" + }, + "count_cap": { + "type": "integer" } } }, @@ -11836,6 +11839,9 @@ }, "count": { "type": "integer" + }, + "count_cap": { + "type": "integer" } } }, diff --git a/coderd/audit.go b/coderd/audit.go index 3a3237a9fe..6f2e66d323 100644 --- a/coderd/audit.go +++ b/coderd/audit.go @@ -26,6 +26,11 @@ import ( "github.com/coder/coder/v2/codersdk" ) +// Limit the count query to avoid a slow sequential scan due to joins +// on a large table. Set to 0 to disable capping (but also see the note +// in the SQL query). +const auditLogCountCap = 2000 + // @Summary Get audit logs // @ID get-audit-logs // @Security CoderSessionToken @@ -66,7 +71,7 @@ func (api *API) auditLogs(rw http.ResponseWriter, r *http.Request) { countFilter.Username = "" } - // Use the same filters to count the number of audit logs + countFilter.CountCap = auditLogCountCap count, err := api.Database.CountAuditLogs(ctx, countFilter) if dbauthz.IsNotAuthorizedError(err) { httpapi.Forbidden(rw) @@ -81,6 +86,7 @@ func (api *API) auditLogs(rw http.ResponseWriter, r *http.Request) { httpapi.Write(ctx, rw, http.StatusOK, codersdk.AuditLogResponse{ AuditLogs: []codersdk.AuditLog{}, Count: 0, + CountCap: auditLogCountCap, }) return } @@ -98,6 +104,7 @@ func (api *API) auditLogs(rw http.ResponseWriter, r *http.Request) { httpapi.Write(ctx, rw, http.StatusOK, codersdk.AuditLogResponse{ AuditLogs: api.convertAuditLogs(ctx, dblogs), Count: count, + CountCap: auditLogCountCap, }) } diff --git a/coderd/database/modelqueries.go b/coderd/database/modelqueries.go index fae0f3eca4..d770ef4547 100644 --- a/coderd/database/modelqueries.go +++ b/coderd/database/modelqueries.go @@ -607,6 +607,7 @@ func (q *sqlQuerier) CountAuthorizedAuditLogs(ctx context.Context, arg CountAudi arg.DateTo, arg.BuildReason, arg.RequestID, + arg.CountCap, ) if err != nil { return 0, err @@ -743,6 +744,7 @@ func (q *sqlQuerier) CountAuthorizedConnectionLogs(ctx context.Context, arg Coun arg.WorkspaceID, arg.ConnectionID, arg.Status, + arg.CountCap, ) if err != nil { return 0, err diff --git a/coderd/database/modelqueries_internal_test.go b/coderd/database/modelqueries_internal_test.go index 9e84324b72..3f425b4347 100644 --- a/coderd/database/modelqueries_internal_test.go +++ b/coderd/database/modelqueries_internal_test.go @@ -145,5 +145,13 @@ func extractWhereClause(query string) string { // Remove SQL comments whereClause = regexp.MustCompile(`(?m)--.*$`).ReplaceAllString(whereClause, "") + // Normalize indentation so subquery wrapping doesn't cause + // mismatches. + lines := strings.Split(whereClause, "\n") + for i, line := range lines { + lines[i] = strings.TrimLeft(line, " \t") + } + whereClause = strings.Join(lines, "\n") + return strings.TrimSpace(whereClause) } diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 909e79cd7f..2643b75158 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -1486,93 +1486,105 @@ func (q *sqlQuerier) UpdateAPIKeyByID(ctx context.Context, arg UpdateAPIKeyByIDP } const countAuditLogs = `-- name: CountAuditLogs :one -SELECT COUNT(*) -FROM audit_logs - LEFT JOIN users ON audit_logs.user_id = users.id - LEFT JOIN organizations ON audit_logs.organization_id = organizations.id - -- First join on workspaces to get the initial workspace create - -- to workspace build 1 id. This is because the first create is - -- is a different audit log than subsequent starts. - LEFT JOIN workspaces ON audit_logs.resource_type = 'workspace' - AND audit_logs.resource_id = workspaces.id - -- Get the reason from the build if the resource type - -- is a workspace_build - LEFT JOIN workspace_builds wb_build ON audit_logs.resource_type = 'workspace_build' - AND audit_logs.resource_id = wb_build.id - -- Get the reason from the build #1 if this is the first - -- workspace create. - LEFT JOIN workspace_builds wb_workspace ON audit_logs.resource_type = 'workspace' - AND audit_logs.action = 'create' - AND workspaces.id = wb_workspace.workspace_id - AND wb_workspace.build_number = 1 -WHERE - -- Filter resource_type - CASE - WHEN $1::text != '' THEN resource_type = $1::resource_type - ELSE true - END - -- Filter resource_id - AND CASE - WHEN $2::uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN resource_id = $2 - ELSE true - END - -- Filter organization_id - AND CASE - WHEN $3::uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN audit_logs.organization_id = $3 - ELSE true - END - -- Filter by resource_target - AND CASE - WHEN $4::text != '' THEN resource_target = $4 - ELSE true - END - -- Filter action - AND CASE - WHEN $5::text != '' THEN action = $5::audit_action - ELSE true - END - -- Filter by user_id - AND CASE - WHEN $6::uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN user_id = $6 - ELSE true - END - -- Filter by username - AND CASE - WHEN $7::text != '' THEN user_id = ( - SELECT id - FROM users - WHERE lower(username) = lower($7) - AND deleted = false - ) - ELSE true - END - -- Filter by user_email - AND CASE - WHEN $8::text != '' THEN users.email = $8 - ELSE true - END - -- Filter by date_from - AND CASE - WHEN $9::timestamp with time zone != '0001-01-01 00:00:00Z' THEN "time" >= $9 - ELSE true - END - -- Filter by date_to - AND CASE - WHEN $10::timestamp with time zone != '0001-01-01 00:00:00Z' THEN "time" <= $10 - ELSE true - END - -- Filter by build_reason - AND CASE - WHEN $11::text != '' THEN COALESCE(wb_build.reason::text, wb_workspace.reason::text) = $11 - ELSE true - END - -- Filter request_id - AND CASE - WHEN $12::uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN audit_logs.request_id = $12 - ELSE true - END - -- Authorize Filter clause will be injected below in CountAuthorizedAuditLogs - -- @authorize_filter +SELECT COUNT(*) FROM ( + SELECT 1 + FROM audit_logs + LEFT JOIN users ON audit_logs.user_id = users.id + LEFT JOIN organizations ON audit_logs.organization_id = organizations.id + -- First join on workspaces to get the initial workspace create + -- to workspace build 1 id. This is because the first create is + -- is a different audit log than subsequent starts. + LEFT JOIN workspaces ON audit_logs.resource_type = 'workspace' + AND audit_logs.resource_id = workspaces.id + -- Get the reason from the build if the resource type + -- is a workspace_build + LEFT JOIN workspace_builds wb_build ON audit_logs.resource_type = 'workspace_build' + AND audit_logs.resource_id = wb_build.id + -- Get the reason from the build #1 if this is the first + -- workspace create. + LEFT JOIN workspace_builds wb_workspace ON audit_logs.resource_type = 'workspace' + AND audit_logs.action = 'create' + AND workspaces.id = wb_workspace.workspace_id + AND wb_workspace.build_number = 1 + WHERE + -- Filter resource_type + CASE + WHEN $1::text != '' THEN resource_type = $1::resource_type + ELSE true + END + -- Filter resource_id + AND CASE + WHEN $2::uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN resource_id = $2 + ELSE true + END + -- Filter organization_id + AND CASE + WHEN $3::uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN audit_logs.organization_id = $3 + ELSE true + END + -- Filter by resource_target + AND CASE + WHEN $4::text != '' THEN resource_target = $4 + ELSE true + END + -- Filter action + AND CASE + WHEN $5::text != '' THEN action = $5::audit_action + ELSE true + END + -- Filter by user_id + AND CASE + WHEN $6::uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN user_id = $6 + ELSE true + END + -- Filter by username + AND CASE + WHEN $7::text != '' THEN user_id = ( + SELECT id + FROM users + WHERE lower(username) = lower($7) + AND deleted = false + ) + ELSE true + END + -- Filter by user_email + AND CASE + WHEN $8::text != '' THEN users.email = $8 + ELSE true + END + -- Filter by date_from + AND CASE + WHEN $9::timestamp with time zone != '0001-01-01 00:00:00Z' THEN "time" >= $9 + ELSE true + END + -- Filter by date_to + AND CASE + WHEN $10::timestamp with time zone != '0001-01-01 00:00:00Z' THEN "time" <= $10 + ELSE true + END + -- Filter by build_reason + AND CASE + WHEN $11::text != '' THEN COALESCE(wb_build.reason::text, wb_workspace.reason::text) = $11 + ELSE true + END + -- Filter request_id + AND CASE + WHEN $12::uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN audit_logs.request_id = $12 + ELSE true + END + -- Authorize Filter clause will be injected below in CountAuthorizedAuditLogs + -- @authorize_filter + -- Avoid a slow scan on a large table with joins. The caller + -- passes the count cap and we add 1 so the frontend can detect + -- capping and show "... of N+". A cap of 0 means no limit (NULLIF + -- -> NULL + 1 = NULL). + -- NOTE: Parameterizing this so that we can easily change from, + -- e.g., 2000 to 5000. However, use literal NULL (or no LIMIT) + -- here if disabling the capping on a large table permanently. + -- This way the PG planner can plan parallel execution for + -- potential large wins. + LIMIT NULLIF($13::int, 0) + 1 +) AS limited_count ` type CountAuditLogsParams struct { @@ -1588,6 +1600,7 @@ type CountAuditLogsParams struct { DateTo time.Time `db:"date_to" json:"date_to"` BuildReason string `db:"build_reason" json:"build_reason"` RequestID uuid.UUID `db:"request_id" json:"request_id"` + CountCap int32 `db:"count_cap" json:"count_cap"` } func (q *sqlQuerier) CountAuditLogs(ctx context.Context, arg CountAuditLogsParams) (int64, error) { @@ -1604,6 +1617,7 @@ func (q *sqlQuerier) CountAuditLogs(ctx context.Context, arg CountAuditLogsParam arg.DateTo, arg.BuildReason, arg.RequestID, + arg.CountCap, ) var count int64 err := row.Scan(&count) @@ -1952,110 +1966,113 @@ func (q *sqlQuerier) InsertAuditLog(ctx context.Context, arg InsertAuditLogParam } const countConnectionLogs = `-- name: CountConnectionLogs :one -SELECT - COUNT(*) AS count -FROM - connection_logs -JOIN users AS workspace_owner ON - connection_logs.workspace_owner_id = workspace_owner.id -LEFT JOIN users ON - connection_logs.user_id = users.id -JOIN organizations ON - connection_logs.organization_id = organizations.id -WHERE - -- Filter organization_id - CASE - WHEN $1 :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN - connection_logs.organization_id = $1 - ELSE true - END - -- Filter by workspace owner username - AND CASE - WHEN $2 :: text != '' THEN - workspace_owner_id = ( - SELECT id FROM users - WHERE lower(username) = lower($2) AND deleted = false - ) - ELSE true - END - -- Filter by workspace_owner_id - AND CASE - WHEN $3 :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN - workspace_owner_id = $3 - ELSE true - END - -- Filter by workspace_owner_email - AND CASE - WHEN $4 :: text != '' THEN - workspace_owner_id = ( - SELECT id FROM users - WHERE email = $4 AND deleted = false - ) - ELSE true - END - -- Filter by type - AND CASE - WHEN $5 :: text != '' THEN - type = $5 :: connection_type - ELSE true - END - -- Filter by user_id - AND CASE - WHEN $6 :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN - user_id = $6 - ELSE true - END - -- Filter by username - AND CASE - WHEN $7 :: text != '' THEN - user_id = ( - SELECT id FROM users - WHERE lower(username) = lower($7) AND deleted = false - ) - ELSE true - END - -- Filter by user_email - AND CASE - WHEN $8 :: text != '' THEN - users.email = $8 - ELSE true - END - -- Filter by connected_after - AND CASE - WHEN $9 :: timestamp with time zone != '0001-01-01 00:00:00Z' THEN - connect_time >= $9 - ELSE true - END - -- Filter by connected_before - AND CASE - WHEN $10 :: timestamp with time zone != '0001-01-01 00:00:00Z' THEN - connect_time <= $10 - ELSE true - END - -- Filter by workspace_id - AND CASE - WHEN $11 :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN - connection_logs.workspace_id = $11 - ELSE true - END - -- Filter by connection_id - AND CASE - WHEN $12 :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN - connection_logs.connection_id = $12 - ELSE true - END - -- Filter by whether the session has a disconnect_time - AND CASE - WHEN $13 :: text != '' THEN - (($13 = 'ongoing' AND disconnect_time IS NULL) OR - ($13 = 'completed' AND disconnect_time IS NOT NULL)) AND - -- Exclude web events, since we don't know their close time. - "type" NOT IN ('workspace_app', 'port_forwarding') - ELSE true - END - -- Authorize Filter clause will be injected below in - -- CountAuthorizedConnectionLogs - -- @authorize_filter +SELECT COUNT(*) AS count FROM ( + SELECT 1 + FROM + connection_logs + JOIN users AS workspace_owner ON + connection_logs.workspace_owner_id = workspace_owner.id + LEFT JOIN users ON + connection_logs.user_id = users.id + JOIN organizations ON + connection_logs.organization_id = organizations.id + WHERE + -- Filter organization_id + CASE + WHEN $1 :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN + connection_logs.organization_id = $1 + ELSE true + END + -- Filter by workspace owner username + AND CASE + WHEN $2 :: text != '' THEN + workspace_owner_id = ( + SELECT id FROM users + WHERE lower(username) = lower($2) AND deleted = false + ) + ELSE true + END + -- Filter by workspace_owner_id + AND CASE + WHEN $3 :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN + workspace_owner_id = $3 + ELSE true + END + -- Filter by workspace_owner_email + AND CASE + WHEN $4 :: text != '' THEN + workspace_owner_id = ( + SELECT id FROM users + WHERE email = $4 AND deleted = false + ) + ELSE true + END + -- Filter by type + AND CASE + WHEN $5 :: text != '' THEN + type = $5 :: connection_type + ELSE true + END + -- Filter by user_id + AND CASE + WHEN $6 :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN + user_id = $6 + ELSE true + END + -- Filter by username + AND CASE + WHEN $7 :: text != '' THEN + user_id = ( + SELECT id FROM users + WHERE lower(username) = lower($7) AND deleted = false + ) + ELSE true + END + -- Filter by user_email + AND CASE + WHEN $8 :: text != '' THEN + users.email = $8 + ELSE true + END + -- Filter by connected_after + AND CASE + WHEN $9 :: timestamp with time zone != '0001-01-01 00:00:00Z' THEN + connect_time >= $9 + ELSE true + END + -- Filter by connected_before + AND CASE + WHEN $10 :: timestamp with time zone != '0001-01-01 00:00:00Z' THEN + connect_time <= $10 + ELSE true + END + -- Filter by workspace_id + AND CASE + WHEN $11 :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN + connection_logs.workspace_id = $11 + ELSE true + END + -- Filter by connection_id + AND CASE + WHEN $12 :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN + connection_logs.connection_id = $12 + ELSE true + END + -- Filter by whether the session has a disconnect_time + AND CASE + WHEN $13 :: text != '' THEN + (($13 = 'ongoing' AND disconnect_time IS NULL) OR + ($13 = 'completed' AND disconnect_time IS NOT NULL)) AND + -- Exclude web events, since we don't know their close time. + "type" NOT IN ('workspace_app', 'port_forwarding') + ELSE true + END + -- Authorize Filter clause will be injected below in + -- CountAuthorizedConnectionLogs + -- @authorize_filter + -- NOTE: See the CountAuditLogs LIMIT note. + LIMIT NULLIF($14::int, 0) + 1 +) AS limited_count ` type CountConnectionLogsParams struct { @@ -2072,6 +2089,7 @@ type CountConnectionLogsParams struct { WorkspaceID uuid.UUID `db:"workspace_id" json:"workspace_id"` ConnectionID uuid.UUID `db:"connection_id" json:"connection_id"` Status string `db:"status" json:"status"` + CountCap int32 `db:"count_cap" json:"count_cap"` } func (q *sqlQuerier) CountConnectionLogs(ctx context.Context, arg CountConnectionLogsParams) (int64, error) { @@ -2089,6 +2107,7 @@ func (q *sqlQuerier) CountConnectionLogs(ctx context.Context, arg CountConnectio arg.WorkspaceID, arg.ConnectionID, arg.Status, + arg.CountCap, ) var count int64 err := row.Scan(&count) diff --git a/coderd/database/queries/auditlogs.sql b/coderd/database/queries/auditlogs.sql index 63e8c721c8..a0315235cc 100644 --- a/coderd/database/queries/auditlogs.sql +++ b/coderd/database/queries/auditlogs.sql @@ -149,94 +149,105 @@ VALUES ( RETURNING *; -- name: CountAuditLogs :one -SELECT COUNT(*) -FROM audit_logs - LEFT JOIN users ON audit_logs.user_id = users.id - LEFT JOIN organizations ON audit_logs.organization_id = organizations.id - -- First join on workspaces to get the initial workspace create - -- to workspace build 1 id. This is because the first create is - -- is a different audit log than subsequent starts. - LEFT JOIN workspaces ON audit_logs.resource_type = 'workspace' - AND audit_logs.resource_id = workspaces.id - -- Get the reason from the build if the resource type - -- is a workspace_build - LEFT JOIN workspace_builds wb_build ON audit_logs.resource_type = 'workspace_build' - AND audit_logs.resource_id = wb_build.id - -- Get the reason from the build #1 if this is the first - -- workspace create. - LEFT JOIN workspace_builds wb_workspace ON audit_logs.resource_type = 'workspace' - AND audit_logs.action = 'create' - AND workspaces.id = wb_workspace.workspace_id - AND wb_workspace.build_number = 1 -WHERE - -- Filter resource_type - CASE - WHEN @resource_type::text != '' THEN resource_type = @resource_type::resource_type - ELSE true - END - -- Filter resource_id - AND CASE - WHEN @resource_id::uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN resource_id = @resource_id - ELSE true - END - -- Filter organization_id - AND CASE - WHEN @organization_id::uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN audit_logs.organization_id = @organization_id - ELSE true - END - -- Filter by resource_target - AND CASE - WHEN @resource_target::text != '' THEN resource_target = @resource_target - ELSE true - END - -- Filter action - AND CASE - WHEN @action::text != '' THEN action = @action::audit_action - ELSE true - END - -- Filter by user_id - AND CASE - WHEN @user_id::uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN user_id = @user_id - ELSE true - END - -- Filter by username - AND CASE - WHEN @username::text != '' THEN user_id = ( - SELECT id - FROM users - WHERE lower(username) = lower(@username) - AND deleted = false - ) - ELSE true - END - -- Filter by user_email - AND CASE - WHEN @email::text != '' THEN users.email = @email - ELSE true - END - -- Filter by date_from - AND CASE - WHEN @date_from::timestamp with time zone != '0001-01-01 00:00:00Z' THEN "time" >= @date_from - ELSE true - END - -- Filter by date_to - AND CASE - WHEN @date_to::timestamp with time zone != '0001-01-01 00:00:00Z' THEN "time" <= @date_to - ELSE true - END - -- Filter by build_reason - AND CASE - WHEN @build_reason::text != '' THEN COALESCE(wb_build.reason::text, wb_workspace.reason::text) = @build_reason - ELSE true - END - -- Filter request_id - AND CASE - WHEN @request_id::uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN audit_logs.request_id = @request_id - ELSE true - END - -- Authorize Filter clause will be injected below in CountAuthorizedAuditLogs - -- @authorize_filter -; +SELECT COUNT(*) FROM ( + SELECT 1 + FROM audit_logs + LEFT JOIN users ON audit_logs.user_id = users.id + LEFT JOIN organizations ON audit_logs.organization_id = organizations.id + -- First join on workspaces to get the initial workspace create + -- to workspace build 1 id. This is because the first create is + -- is a different audit log than subsequent starts. + LEFT JOIN workspaces ON audit_logs.resource_type = 'workspace' + AND audit_logs.resource_id = workspaces.id + -- Get the reason from the build if the resource type + -- is a workspace_build + LEFT JOIN workspace_builds wb_build ON audit_logs.resource_type = 'workspace_build' + AND audit_logs.resource_id = wb_build.id + -- Get the reason from the build #1 if this is the first + -- workspace create. + LEFT JOIN workspace_builds wb_workspace ON audit_logs.resource_type = 'workspace' + AND audit_logs.action = 'create' + AND workspaces.id = wb_workspace.workspace_id + AND wb_workspace.build_number = 1 + WHERE + -- Filter resource_type + CASE + WHEN @resource_type::text != '' THEN resource_type = @resource_type::resource_type + ELSE true + END + -- Filter resource_id + AND CASE + WHEN @resource_id::uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN resource_id = @resource_id + ELSE true + END + -- Filter organization_id + AND CASE + WHEN @organization_id::uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN audit_logs.organization_id = @organization_id + ELSE true + END + -- Filter by resource_target + AND CASE + WHEN @resource_target::text != '' THEN resource_target = @resource_target + ELSE true + END + -- Filter action + AND CASE + WHEN @action::text != '' THEN action = @action::audit_action + ELSE true + END + -- Filter by user_id + AND CASE + WHEN @user_id::uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN user_id = @user_id + ELSE true + END + -- Filter by username + AND CASE + WHEN @username::text != '' THEN user_id = ( + SELECT id + FROM users + WHERE lower(username) = lower(@username) + AND deleted = false + ) + ELSE true + END + -- Filter by user_email + AND CASE + WHEN @email::text != '' THEN users.email = @email + ELSE true + END + -- Filter by date_from + AND CASE + WHEN @date_from::timestamp with time zone != '0001-01-01 00:00:00Z' THEN "time" >= @date_from + ELSE true + END + -- Filter by date_to + AND CASE + WHEN @date_to::timestamp with time zone != '0001-01-01 00:00:00Z' THEN "time" <= @date_to + ELSE true + END + -- Filter by build_reason + AND CASE + WHEN @build_reason::text != '' THEN COALESCE(wb_build.reason::text, wb_workspace.reason::text) = @build_reason + ELSE true + END + -- Filter request_id + AND CASE + WHEN @request_id::uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN audit_logs.request_id = @request_id + ELSE true + END + -- Authorize Filter clause will be injected below in CountAuthorizedAuditLogs + -- @authorize_filter + -- Avoid a slow scan on a large table with joins. The caller + -- passes the count cap and we add 1 so the frontend can detect + -- capping and show "... of N+". A cap of 0 means no limit (NULLIF + -- -> NULL + 1 = NULL). + -- NOTE: Parameterizing this so that we can easily change from, + -- e.g., 2000 to 5000. However, use literal NULL (or no LIMIT) + -- here if disabling the capping on a large table permanently. + -- This way the PG planner can plan parallel execution for + -- potential large wins. + LIMIT NULLIF(@count_cap::int, 0) + 1 +) AS limited_count; -- name: DeleteOldAuditLogConnectionEvents :exec DELETE FROM audit_logs diff --git a/coderd/database/queries/connectionlogs.sql b/coderd/database/queries/connectionlogs.sql index eb2d1b0cb1..4df92fdad4 100644 --- a/coderd/database/queries/connectionlogs.sql +++ b/coderd/database/queries/connectionlogs.sql @@ -133,111 +133,113 @@ OFFSET @offset_opt; -- name: CountConnectionLogs :one -SELECT - COUNT(*) AS count -FROM - connection_logs -JOIN users AS workspace_owner ON - connection_logs.workspace_owner_id = workspace_owner.id -LEFT JOIN users ON - connection_logs.user_id = users.id -JOIN organizations ON - connection_logs.organization_id = organizations.id -WHERE - -- Filter organization_id - CASE - WHEN @organization_id :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN - connection_logs.organization_id = @organization_id - ELSE true - END - -- Filter by workspace owner username - AND CASE - WHEN @workspace_owner :: text != '' THEN - workspace_owner_id = ( - SELECT id FROM users - WHERE lower(username) = lower(@workspace_owner) AND deleted = false - ) - ELSE true - END - -- Filter by workspace_owner_id - AND CASE - WHEN @workspace_owner_id :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN - workspace_owner_id = @workspace_owner_id - ELSE true - END - -- Filter by workspace_owner_email - AND CASE - WHEN @workspace_owner_email :: text != '' THEN - workspace_owner_id = ( - SELECT id FROM users - WHERE email = @workspace_owner_email AND deleted = false - ) - ELSE true - END - -- Filter by type - AND CASE - WHEN @type :: text != '' THEN - type = @type :: connection_type - ELSE true - END - -- Filter by user_id - AND CASE - WHEN @user_id :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN - user_id = @user_id - ELSE true - END - -- Filter by username - AND CASE - WHEN @username :: text != '' THEN - user_id = ( - SELECT id FROM users - WHERE lower(username) = lower(@username) AND deleted = false - ) - ELSE true - END - -- Filter by user_email - AND CASE - WHEN @user_email :: text != '' THEN - users.email = @user_email - ELSE true - END - -- Filter by connected_after - AND CASE - WHEN @connected_after :: timestamp with time zone != '0001-01-01 00:00:00Z' THEN - connect_time >= @connected_after - ELSE true - END - -- Filter by connected_before - AND CASE - WHEN @connected_before :: timestamp with time zone != '0001-01-01 00:00:00Z' THEN - connect_time <= @connected_before - ELSE true - END - -- Filter by workspace_id - AND CASE - WHEN @workspace_id :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN - connection_logs.workspace_id = @workspace_id - ELSE true - END - -- Filter by connection_id - AND CASE - WHEN @connection_id :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN - connection_logs.connection_id = @connection_id - ELSE true - END - -- Filter by whether the session has a disconnect_time - AND CASE - WHEN @status :: text != '' THEN - ((@status = 'ongoing' AND disconnect_time IS NULL) OR - (@status = 'completed' AND disconnect_time IS NOT NULL)) AND - -- Exclude web events, since we don't know their close time. - "type" NOT IN ('workspace_app', 'port_forwarding') - ELSE true - END - -- Authorize Filter clause will be injected below in - -- CountAuthorizedConnectionLogs - -- @authorize_filter -; +SELECT COUNT(*) AS count FROM ( + SELECT 1 + FROM + connection_logs + JOIN users AS workspace_owner ON + connection_logs.workspace_owner_id = workspace_owner.id + LEFT JOIN users ON + connection_logs.user_id = users.id + JOIN organizations ON + connection_logs.organization_id = organizations.id + WHERE + -- Filter organization_id + CASE + WHEN @organization_id :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN + connection_logs.organization_id = @organization_id + ELSE true + END + -- Filter by workspace owner username + AND CASE + WHEN @workspace_owner :: text != '' THEN + workspace_owner_id = ( + SELECT id FROM users + WHERE lower(username) = lower(@workspace_owner) AND deleted = false + ) + ELSE true + END + -- Filter by workspace_owner_id + AND CASE + WHEN @workspace_owner_id :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN + workspace_owner_id = @workspace_owner_id + ELSE true + END + -- Filter by workspace_owner_email + AND CASE + WHEN @workspace_owner_email :: text != '' THEN + workspace_owner_id = ( + SELECT id FROM users + WHERE email = @workspace_owner_email AND deleted = false + ) + ELSE true + END + -- Filter by type + AND CASE + WHEN @type :: text != '' THEN + type = @type :: connection_type + ELSE true + END + -- Filter by user_id + AND CASE + WHEN @user_id :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN + user_id = @user_id + ELSE true + END + -- Filter by username + AND CASE + WHEN @username :: text != '' THEN + user_id = ( + SELECT id FROM users + WHERE lower(username) = lower(@username) AND deleted = false + ) + ELSE true + END + -- Filter by user_email + AND CASE + WHEN @user_email :: text != '' THEN + users.email = @user_email + ELSE true + END + -- Filter by connected_after + AND CASE + WHEN @connected_after :: timestamp with time zone != '0001-01-01 00:00:00Z' THEN + connect_time >= @connected_after + ELSE true + END + -- Filter by connected_before + AND CASE + WHEN @connected_before :: timestamp with time zone != '0001-01-01 00:00:00Z' THEN + connect_time <= @connected_before + ELSE true + END + -- Filter by workspace_id + AND CASE + WHEN @workspace_id :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN + connection_logs.workspace_id = @workspace_id + ELSE true + END + -- Filter by connection_id + AND CASE + WHEN @connection_id :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN + connection_logs.connection_id = @connection_id + ELSE true + END + -- Filter by whether the session has a disconnect_time + AND CASE + WHEN @status :: text != '' THEN + ((@status = 'ongoing' AND disconnect_time IS NULL) OR + (@status = 'completed' AND disconnect_time IS NOT NULL)) AND + -- Exclude web events, since we don't know their close time. + "type" NOT IN ('workspace_app', 'port_forwarding') + ELSE true + END + -- Authorize Filter clause will be injected below in + -- CountAuthorizedConnectionLogs + -- @authorize_filter + -- NOTE: See the CountAuditLogs LIMIT note. + LIMIT NULLIF(@count_cap::int, 0) + 1 +) AS limited_count; -- name: UpsertConnectionLog :one INSERT INTO connection_logs ( diff --git a/coderd/rbac/regosql/compile_test.go b/coderd/rbac/regosql/compile_test.go index 7bea7f76fd..f58f808559 100644 --- a/coderd/rbac/regosql/compile_test.go +++ b/coderd/rbac/regosql/compile_test.go @@ -282,6 +282,40 @@ neq(input.object.owner, ""); p("'10d03e62-7703-4df5-a358-4f76577d4e2f' = id :: text") + " AND " + p("id :: text != ''") + " AND " + p("'' = ''"), ), }, + { + Name: "AuditLogUUID", + Queries: []string{ + `"8c0b9bdc-a013-4b14-a49b-5747bc335708" = input.object.org_owner`, + `input.object.org_owner != ""`, + `neq(input.object.org_owner, "8c0b9bdc-a013-4b14-a49b-5747bc335708")`, + `input.object.org_owner in {"8c0b9bdc-a013-4b14-a49b-5747bc335708", "05f58202-4bfc-43ce-9ba4-5ff6e0174a71"}`, + `"read" in input.object.acl_group_list[input.object.org_owner]`, + }, + ExpectedSQL: p( + p("audit_logs.organization_id = '8c0b9bdc-a013-4b14-a49b-5747bc335708'::uuid") + " OR " + + p("audit_logs.organization_id IS NOT NULL") + " OR " + + p("audit_logs.organization_id != '8c0b9bdc-a013-4b14-a49b-5747bc335708'::uuid") + " OR " + + p("audit_logs.organization_id = ANY(ARRAY ['05f58202-4bfc-43ce-9ba4-5ff6e0174a71'::uuid,'8c0b9bdc-a013-4b14-a49b-5747bc335708'::uuid])") + " OR " + + "(false)"), + VariableConverter: regosql.AuditLogConverter(), + }, + { + Name: "ConnectionLogUUID", + Queries: []string{ + `"8c0b9bdc-a013-4b14-a49b-5747bc335708" = input.object.org_owner`, + `input.object.org_owner != ""`, + `neq(input.object.org_owner, "8c0b9bdc-a013-4b14-a49b-5747bc335708")`, + `input.object.org_owner in {"8c0b9bdc-a013-4b14-a49b-5747bc335708"}`, + `"read" in input.object.acl_group_list[input.object.org_owner]`, + }, + ExpectedSQL: p( + p("connection_logs.organization_id = '8c0b9bdc-a013-4b14-a49b-5747bc335708'::uuid") + " OR " + + p("connection_logs.organization_id IS NOT NULL") + " OR " + + p("connection_logs.organization_id != '8c0b9bdc-a013-4b14-a49b-5747bc335708'::uuid") + " OR " + + p("connection_logs.organization_id = ANY(ARRAY ['8c0b9bdc-a013-4b14-a49b-5747bc335708'::uuid])") + " OR " + + "(false)"), + VariableConverter: regosql.ConnectionLogConverter(), + }, } for _, tc := range testCases { diff --git a/coderd/rbac/regosql/configs.go b/coderd/rbac/regosql/configs.go index 355a49756d..22302a5296 100644 --- a/coderd/rbac/regosql/configs.go +++ b/coderd/rbac/regosql/configs.go @@ -53,7 +53,7 @@ func WorkspaceConverter() *sqltypes.VariableConverter { func AuditLogConverter() *sqltypes.VariableConverter { matcher := sqltypes.NewVariableConverter().RegisterMatcher( resourceIDMatcher(), - sqltypes.StringVarMatcher("COALESCE(audit_logs.organization_id :: text, '')", []string{"input", "object", "org_owner"}), + sqltypes.UUIDVarMatcher("audit_logs.organization_id", []string{"input", "object", "org_owner"}), // Audit logs have no user owner, only owner by an organization. sqltypes.AlwaysFalse(userOwnerMatcher()), ) @@ -67,7 +67,7 @@ func AuditLogConverter() *sqltypes.VariableConverter { func ConnectionLogConverter() *sqltypes.VariableConverter { matcher := sqltypes.NewVariableConverter().RegisterMatcher( resourceIDMatcher(), - sqltypes.StringVarMatcher("COALESCE(connection_logs.organization_id :: text, '')", []string{"input", "object", "org_owner"}), + sqltypes.UUIDVarMatcher("connection_logs.organization_id", []string{"input", "object", "org_owner"}), // Connection logs have no user owner, only owner by an organization. sqltypes.AlwaysFalse(userOwnerMatcher()), ) diff --git a/coderd/rbac/regosql/sqltypes/uuid.go b/coderd/rbac/regosql/sqltypes/uuid.go new file mode 100644 index 0000000000..bcf95c8411 --- /dev/null +++ b/coderd/rbac/regosql/sqltypes/uuid.go @@ -0,0 +1,114 @@ +package sqltypes + +import ( + "fmt" + "strings" + + "github.com/open-policy-agent/opa/ast" + "golang.org/x/xerrors" +) + +var ( + _ VariableMatcher = astUUIDVar{} + _ Node = astUUIDVar{} + _ SupportsEquality = astUUIDVar{} +) + +// astUUIDVar is a variable that represents a UUID column. Unlike +// astStringVar it emits native UUID comparisons (column = 'val'::uuid) +// instead of text-based ones (COALESCE(column::text, ”) = 'val'). +// This allows PostgreSQL to use indexes on UUID columns. +type astUUIDVar struct { + Source RegoSource + FieldPath []string + ColumnString string +} + +func UUIDVarMatcher(sqlColumn string, regoPath []string) VariableMatcher { + return astUUIDVar{FieldPath: regoPath, ColumnString: sqlColumn} +} + +func (astUUIDVar) UseAs() Node { return astUUIDVar{} } + +func (u astUUIDVar) ConvertVariable(rego ast.Ref) (Node, bool) { + left, err := RegoVarPath(u.FieldPath, rego) + if err == nil && len(left) == 0 { + return astUUIDVar{ + Source: RegoSource(rego.String()), + FieldPath: u.FieldPath, + ColumnString: u.ColumnString, + }, true + } + + return nil, false +} + +func (u astUUIDVar) SQLString(_ *SQLGenerator) string { + return u.ColumnString +} + +// EqualsSQLString handles equality comparisons for UUID columns. +// Rego always produces string literals, so we accept AstString and +// cast the literal to ::uuid in the output SQL. This lets PG use +// native UUID indexes instead of falling back to text comparisons. +// nolint:revive +func (u astUUIDVar) EqualsSQLString(cfg *SQLGenerator, not bool, other Node) (string, error) { + switch other.UseAs().(type) { + case AstString: + // The other side is a rego string literal like + // "8c0b9bdc-a013-4b14-a49b-5747bc335708". Emit a comparison + // that casts the literal to uuid so PG can use indexes: + // column = 'val'::uuid + // instead of the text-based: + // 'val' = COALESCE(column::text, '') + s, ok := other.(AstString) + if !ok { + return "", xerrors.Errorf("expected AstString, got %T", other) + } + if s.Value == "" { + // Empty string in rego means "no value". Compare the + // column against NULL since UUID columns represent + // absent values as NULL, not empty strings. + op := "IS NULL" + if not { + op = "IS NOT NULL" + } + return fmt.Sprintf("%s %s", u.ColumnString, op), nil + } + return fmt.Sprintf("%s %s '%s'::uuid", + u.ColumnString, equalsOp(not), s.Value), nil + case astUUIDVar: + return basicSQLEquality(cfg, not, u, other), nil + default: + return "", xerrors.Errorf("unsupported equality: %T %s %T", + u, equalsOp(not), other) + } +} + +// ContainedInSQL implements SupportsContainedIn so that a UUID column +// can appear in membership checks like `col = ANY(ARRAY[...])`. The +// array elements are rego strings, so we cast each to ::uuid. +func (u astUUIDVar) ContainedInSQL(_ *SQLGenerator, haystack Node) (string, error) { + arr, ok := haystack.(ASTArray) + if !ok { + return "", xerrors.Errorf("unsupported containedIn: %T in %T", u, haystack) + } + + if len(arr.Value) == 0 { + return "false", nil + } + + // Build ARRAY['uuid1'::uuid, 'uuid2'::uuid, ...] + values := make([]string, 0, len(arr.Value)) + for _, v := range arr.Value { + s, ok := v.(AstString) + if !ok { + return "", xerrors.Errorf("expected AstString array element, got %T", v) + } + values = append(values, fmt.Sprintf("'%s'::uuid", s.Value)) + } + + return fmt.Sprintf("%s = ANY(ARRAY [%s])", + u.ColumnString, + strings.Join(values, ",")), nil +} diff --git a/coderd/searchquery/search.go b/coderd/searchquery/search.go index 59ec3e0492..af5b82f89e 100644 --- a/coderd/searchquery/search.go +++ b/coderd/searchquery/search.go @@ -67,7 +67,7 @@ func AuditLogs(ctx context.Context, db database.Store, query string) (database.G } // Prepare the count filter, which uses the same parameters as the GetAuditLogsOffsetParams. - // nolint:exhaustruct // UserID is not obtained from the query parameters. + // nolint:exhaustruct // UserID and CountCap are not obtained from the query parameters. countFilter := database.CountAuditLogsParams{ RequestID: filter.RequestID, ResourceID: filter.ResourceID, @@ -124,6 +124,7 @@ func ConnectionLogs(ctx context.Context, db database.Store, query string, apiKey } // This MUST be kept in sync with the above + // nolint:exhaustruct // CountCap is not obtained from the query parameters. countFilter := database.CountConnectionLogsParams{ OrganizationID: filter.OrganizationID, WorkspaceOwner: filter.WorkspaceOwner, diff --git a/codersdk/audit.go b/codersdk/audit.go index 0b2eca7d79..434adde89f 100644 --- a/codersdk/audit.go +++ b/codersdk/audit.go @@ -209,6 +209,7 @@ type AuditLogsRequest struct { type AuditLogResponse struct { AuditLogs []AuditLog `json:"audit_logs"` Count int64 `json:"count"` + CountCap int64 `json:"count_cap"` } type CreateTestAuditLogRequest struct { diff --git a/codersdk/connectionlog.go b/codersdk/connectionlog.go index 3e2acec6df..61e1ccbb30 100644 --- a/codersdk/connectionlog.go +++ b/codersdk/connectionlog.go @@ -96,6 +96,7 @@ type ConnectionLogsRequest struct { type ConnectionLogResponse struct { ConnectionLogs []ConnectionLog `json:"connection_logs"` Count int64 `json:"count"` + CountCap int64 `json:"count_cap"` } func (c *Client) ConnectionLogs(ctx context.Context, req ConnectionLogsRequest) (ConnectionLogResponse, error) { diff --git a/docs/reference/api/audit.md b/docs/reference/api/audit.md index c717a75d51..9ec39fbf69 100644 --- a/docs/reference/api/audit.md +++ b/docs/reference/api/audit.md @@ -88,7 +88,8 @@ curl -X GET http://coder-server:8080/api/v2/audit?limit=0 \ "user_agent": "string" } ], - "count": 0 + "count": 0, + "count_cap": 0 } ``` diff --git a/docs/reference/api/enterprise.md b/docs/reference/api/enterprise.md index a20c80671c..199c6f2e3a 100644 --- a/docs/reference/api/enterprise.md +++ b/docs/reference/api/enterprise.md @@ -289,7 +289,8 @@ curl -X GET http://coder-server:8080/api/v2/connectionlog?limit=0 \ "workspace_owner_username": "string" } ], - "count": 0 + "count": 0, + "count_cap": 0 } ``` diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index c718248be0..e5edca0fa8 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -1417,7 +1417,8 @@ "user_agent": "string" } ], - "count": 0 + "count": 0, + "count_cap": 0 } ``` @@ -1427,6 +1428,7 @@ |--------------|-------------------------------------------------|----------|--------------|-------------| | `audit_logs` | array of [codersdk.AuditLog](#codersdkauditlog) | false | | | | `count` | integer | false | | | +| `count_cap` | integer | false | | | ## codersdk.AuthMethod @@ -1845,7 +1847,8 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in "workspace_owner_username": "string" } ], - "count": 0 + "count": 0, + "count_cap": 0 } ``` @@ -1855,6 +1858,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in |-------------------|-----------------------------------------------------------|----------|--------------|-------------| | `connection_logs` | array of [codersdk.ConnectionLog](#codersdkconnectionlog) | false | | | | `count` | integer | false | | | +| `count_cap` | integer | false | | | ## codersdk.ConnectionLogSSHInfo diff --git a/enterprise/coderd/connectionlog.go b/enterprise/coderd/connectionlog.go index 05e3a40b2d..c37e2ce497 100644 --- a/enterprise/coderd/connectionlog.go +++ b/enterprise/coderd/connectionlog.go @@ -16,6 +16,9 @@ import ( "github.com/coder/coder/v2/codersdk" ) +// NOTE: See the auditLogCountCap note. +const connectionLogCountCap = 2000 + // @Summary Get connection logs // @ID get-connection-logs // @Security CoderSessionToken @@ -49,6 +52,7 @@ func (api *API) connectionLogs(rw http.ResponseWriter, r *http.Request) { // #nosec G115 - Safe conversion as pagination limit is expected to be within int32 range filter.LimitOpt = int32(page.Limit) + countFilter.CountCap = connectionLogCountCap count, err := api.Database.CountConnectionLogs(ctx, countFilter) if dbauthz.IsNotAuthorizedError(err) { httpapi.Forbidden(rw) @@ -63,6 +67,7 @@ func (api *API) connectionLogs(rw http.ResponseWriter, r *http.Request) { httpapi.Write(ctx, rw, http.StatusOK, codersdk.ConnectionLogResponse{ ConnectionLogs: []codersdk.ConnectionLog{}, Count: 0, + CountCap: connectionLogCountCap, }) return } @@ -80,6 +85,7 @@ func (api *API) connectionLogs(rw http.ResponseWriter, r *http.Request) { httpapi.Write(ctx, rw, http.StatusOK, codersdk.ConnectionLogResponse{ ConnectionLogs: convertConnectionLogs(dblogs), Count: count, + CountCap: connectionLogCountCap, }) } diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 9771c08acb..7c7325dbfb 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -744,6 +744,7 @@ export interface AuditLog { export interface AuditLogResponse { readonly audit_logs: readonly AuditLog[]; readonly count: number; + readonly count_cap: number; } // From codersdk/audit.go @@ -1050,6 +1051,7 @@ export interface ConnectionLog { export interface ConnectionLogResponse { readonly connection_logs: readonly ConnectionLog[]; readonly count: number; + readonly count_cap: number; } // From codersdk/connectionlog.go diff --git a/site/src/components/PaginationWidget/PaginationContainer.mocks.ts b/site/src/components/PaginationWidget/PaginationContainer.mocks.ts index e638e1e3db..7466529af5 100644 --- a/site/src/components/PaginationWidget/PaginationContainer.mocks.ts +++ b/site/src/components/PaginationWidget/PaginationContainer.mocks.ts @@ -18,6 +18,7 @@ export const mockPaginationResultBase: ResultBase = { limit: 25, hasNextPage: false, hasPreviousPage: false, + countIsCapped: false, goToPreviousPage: () => {}, goToNextPage: () => {}, goToFirstPage: () => {}, @@ -33,6 +34,7 @@ export const mockInitialRenderResult: PaginationResult = { hasPreviousPage: false, totalRecords: undefined, totalPages: undefined, + countIsCapped: false, }; export const mockSuccessResult: PaginationResult = { diff --git a/site/src/components/PaginationWidget/PaginationContainer.stories.tsx b/site/src/components/PaginationWidget/PaginationContainer.stories.tsx index a07800108f..f221498414 100644 --- a/site/src/components/PaginationWidget/PaginationContainer.stories.tsx +++ b/site/src/components/PaginationWidget/PaginationContainer.stories.tsx @@ -119,3 +119,54 @@ export const SecondPageWithData: Story = { children: