Skip to content

Commit b31b0fd

Browse files
Kira-PilotEmyrk
Kira-Pilot
and
Emyrk
authored
fix: audit log broken build links (#5895)
* pushing for guidance * added test * PR feedback * fixed tests * Update coderd/audit.go Co-authored-by: Steven Masley <Emyrk@users.noreply.github.com> * runnig make gen --------- Co-authored-by: Steven Masley <Emyrk@users.noreply.github.com>
1 parent 88b5d42 commit b31b0fd

File tree

15 files changed

+145
-58
lines changed

15 files changed

+145
-58
lines changed

‎coderd/apidoc/docs.go

+6
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/apidoc/swagger.json

+6
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/audit.go

+18-14
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"golang.org/x/xerrors"
1818

1919
"cdr.dev/slog"
20+
"github.com/coder/coder/coderd/audit"
2021
"github.com/coder/coder/coderd/database"
2122
"github.com/coder/coder/coderd/httpapi"
2223
"github.com/coder/coder/coderd/httpmw"
@@ -147,6 +148,9 @@ func (api *API) generateFakeAuditLog(rw http.ResponseWriter, r *http.Request) {
147148
if params.Time.IsZero() {
148149
params.Time = time.Now()
149150
}
151+
if len(params.AdditionalFields) == 0 {
152+
params.AdditionalFields = json.RawMessage("{}")
153+
}
150154

151155
_, err = api.Database.InsertAuditLog(ctx, database.InsertAuditLogParams{
152156
ID: uuid.New(),
@@ -160,7 +164,7 @@ func (api *API) generateFakeAuditLog(rw http.ResponseWriter, r *http.Request) {
160164
Action: database.AuditAction(params.Action),
161165
Diff: diff,
162166
StatusCode: http.StatusOK,
163-
AdditionalFields: []byte("{}"),
167+
AdditionalFields: params.AdditionalFields,
164168
})
165169
if err != nil {
166170
httpapi.InternalServerError(rw, err)
@@ -180,12 +184,6 @@ func (api *API) convertAuditLogs(ctx context.Context, dblogs []database.GetAudit
180184
return alogs
181185
}
182186

183-
type AdditionalFields struct {
184-
WorkspaceName string `json:"workspace_name"`
185-
BuildNumber string `json:"build_number"`
186-
BuildReason database.BuildReason `json:"build_reason"`
187-
}
188-
189187
func (api *API) convertAuditLog(ctx context.Context, dblog database.GetAuditLogsOffsetRow) codersdk.AuditLog {
190188
ip, _ := netip.AddrFromSlice(dblog.Ip.IPNet.IP)
191189

@@ -213,16 +211,18 @@ func (api *API) convertAuditLog(ctx context.Context, dblog database.GetAuditLogs
213211

214212
var (
215213
additionalFieldsBytes = []byte(dblog.AdditionalFields)
216-
additionalFields AdditionalFields
214+
additionalFields audit.AdditionalFields
217215
err = json.Unmarshal(additionalFieldsBytes, &additionalFields)
218216
)
219217
if err != nil {
220218
api.Logger.Error(ctx, "unmarshal additional fields", slog.Error(err))
221-
resourceInfo := map[string]string{
222-
"workspaceName": "unknown",
223-
"buildNumber": "unknown",
224-
"buildReason": "unknown",
219+
resourceInfo := audit.AdditionalFields{
220+
WorkspaceName: "unknown",
221+
BuildNumber: "unknown",
222+
BuildReason: "unknown",
223+
WorkspaceOwner: "unknown",
225224
}
225+
226226
dblog.AdditionalFields, err = json.Marshal(resourceInfo)
227227
api.Logger.Error(ctx, "marshal additional fields", slog.Error(err))
228228
}
@@ -259,7 +259,7 @@ func (api *API) convertAuditLog(ctx context.Context, dblog database.GetAuditLogs
259259
}
260260
}
261261

262-
func auditLogDescription(alog database.GetAuditLogsOffsetRow, additionalFields AdditionalFields) string {
262+
func auditLogDescription(alog database.GetAuditLogsOffsetRow, additionalFields audit.AdditionalFields) string {
263263
str := fmt.Sprintf("{user} %s",
264264
codersdk.AuditAction(alog.Action).Friendly(),
265265
)
@@ -344,14 +344,16 @@ func (api *API) auditLogIsResourceDeleted(ctx context.Context, alog database.Get
344344
}
345345
}
346346

347-
func (api *API) auditLogResourceLink(ctx context.Context, alog database.GetAuditLogsOffsetRow, additionalFields AdditionalFields) string {
347+
func (api *API) auditLogResourceLink(ctx context.Context, alog database.GetAuditLogsOffsetRow, additionalFields audit.AdditionalFields) string {
348348
switch alog.ResourceType {
349349
case database.ResourceTypeTemplate:
350350
return fmt.Sprintf("/templates/%s",
351351
alog.ResourceTarget)
352+
352353
case database.ResourceTypeUser:
353354
return fmt.Sprintf("/users?filter=%s",
354355
alog.ResourceTarget)
356+
355357
case database.ResourceTypeWorkspace:
356358
workspace, getWorkspaceErr := api.Database.GetWorkspaceByID(ctx, alog.ResourceID)
357359
if getWorkspaceErr != nil {
@@ -363,6 +365,7 @@ func (api *API) auditLogResourceLink(ctx context.Context, alog database.GetAudit
363365
}
364366
return fmt.Sprintf("/@%s/%s",
365367
workspaceOwner.Username, alog.ResourceTarget)
368+
366369
case database.ResourceTypeWorkspaceBuild:
367370
if len(additionalFields.WorkspaceName) == 0 || len(additionalFields.BuildNumber) == 0 {
368371
return ""
@@ -381,6 +384,7 @@ func (api *API) auditLogResourceLink(ctx context.Context, alog database.GetAudit
381384
}
382385
return fmt.Sprintf("/@%s/%s/builds/%s",
383386
workspaceOwner.Username, additionalFields.WorkspaceName, additionalFields.BuildNumber)
387+
384388
default:
385389
return ""
386390
}

‎coderd/audit/audit.go

+7
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,13 @@ type Auditor interface {
1212
diff(old, new any) Map
1313
}
1414

15+
type AdditionalFields struct {
16+
WorkspaceName string `json:"workspace_name"`
17+
BuildNumber string `json:"build_number"`
18+
BuildReason database.BuildReason `json:"build_reason"`
19+
WorkspaceOwner string `json:"workspace_owner"`
20+
}
21+
1522
func NewNop() Auditor {
1623
return nop{}
1724
}

‎coderd/audit_test.go

+48
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,17 @@ package coderd_test
22

33
import (
44
"context"
5+
"encoding/json"
6+
"fmt"
7+
"strconv"
58
"testing"
69
"time"
710

811
"github.com/stretchr/testify/require"
912

13+
"github.com/coder/coder/coderd/audit"
1014
"github.com/coder/coder/coderd/coderdtest"
15+
"github.com/coder/coder/coderd/database"
1116
"github.com/coder/coder/codersdk"
1217
)
1318

@@ -36,6 +41,49 @@ func TestAuditLogs(t *testing.T) {
3641
require.Equal(t, int64(1), alogs.Count)
3742
require.Len(t, alogs.AuditLogs, 1)
3843
})
44+
45+
t.Run("WorkspaceBuildAuditLink", func(t *testing.T) {
46+
t.Parallel()
47+
48+
var (
49+
ctx = context.Background()
50+
client = coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
51+
user = coderdtest.CreateFirstUser(t, client)
52+
version = coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
53+
template = coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
54+
)
55+
56+
coderdtest.AwaitTemplateVersionJob(t, client, version.ID)
57+
workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID)
58+
coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID)
59+
60+
buildResourceInfo := audit.AdditionalFields{
61+
WorkspaceName: workspace.Name,
62+
BuildNumber: strconv.FormatInt(int64(workspace.LatestBuild.BuildNumber), 10),
63+
BuildReason: database.BuildReason(string(workspace.LatestBuild.Reason)),
64+
}
65+
66+
wriBytes, err := json.Marshal(buildResourceInfo)
67+
require.NoError(t, err)
68+
69+
err = client.CreateTestAuditLog(ctx, codersdk.CreateTestAuditLogRequest{
70+
Action: codersdk.AuditActionStop,
71+
ResourceType: codersdk.ResourceTypeWorkspaceBuild,
72+
ResourceID: workspace.LatestBuild.ID,
73+
AdditionalFields: wriBytes,
74+
})
75+
require.NoError(t, err)
76+
77+
auditLogs, err := client.AuditLogs(ctx, codersdk.AuditLogsRequest{
78+
Pagination: codersdk.Pagination{
79+
Limit: 1,
80+
},
81+
})
82+
require.NoError(t, err)
83+
buildNumberString := strconv.FormatInt(int64(workspace.LatestBuild.BuildNumber), 10)
84+
require.Equal(t, auditLogs.AuditLogs[0].ResourceLink, fmt.Sprintf("/@%s/%s/builds/%s",
85+
workspace.OwnerName, workspace.Name, buildNumberString))
86+
})
3987
}
4088

4189
func TestAuditLogsFilter(t *testing.T) {

‎coderd/provisionerdserver/provisionerdserver.go

+9-8
Original file line numberDiff line numberDiff line change
@@ -553,12 +553,13 @@ func (server *Server) FailJob(ctx context.Context, failJob *proto.FailedJob) (*p
553553
if prevBuildErr != nil {
554554
previousBuild = database.WorkspaceBuild{}
555555
}
556+
556557
// We pass the below information to the Auditor so that it
557558
// can form a friendly string for the user to view in the UI.
558-
buildResourceInfo := map[string]string{
559-
"workspaceName": workspace.Name,
560-
"buildNumber": strconv.FormatInt(int64(build.BuildNumber), 10),
561-
"buildReason": fmt.Sprintf("%v", build.Reason),
559+
buildResourceInfo := audit.AdditionalFields{
560+
WorkspaceName: workspace.Name,
561+
BuildNumber: strconv.FormatInt(int64(build.BuildNumber), 10),
562+
BuildReason: database.BuildReason(string(build.Reason)),
562563
}
563564

564565
wriBytes, err := json.Marshal(buildResourceInfo)
@@ -816,10 +817,10 @@ func (server *Server) CompleteJob(ctx context.Context, completed *proto.Complete
816817

817818
// We pass the below information to the Auditor so that it
818819
// can form a friendly string for the user to view in the UI.
819-
buildResourceInfo := map[string]string{
820-
"workspaceName": workspace.Name,
821-
"buildNumber": strconv.FormatInt(int64(workspaceBuild.BuildNumber), 10),
822-
"buildReason": fmt.Sprintf("%v", workspaceBuild.Reason),
820+
buildResourceInfo := audit.AdditionalFields{
821+
WorkspaceName: workspace.Name,
822+
BuildNumber: strconv.FormatInt(int64(workspaceBuild.BuildNumber), 10),
823+
BuildReason: database.BuildReason(string(workspaceBuild.Reason)),
823824
}
824825

825826
wriBytes, err := json.Marshal(buildResourceInfo)

‎coderd/workspaces.go

+21-11
Original file line numberDiff line numberDiff line change
@@ -279,19 +279,29 @@ func (api *API) workspaceByOwnerAndName(rw http.ResponseWriter, r *http.Request)
279279
// @Router /organizations/{organization}/members/{user}/workspaces [post]
280280
func (api *API) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Request) {
281281
var (
282-
ctx = r.Context()
283-
organization = httpmw.OrganizationParam(r)
284-
apiKey = httpmw.APIKey(r)
285-
auditor = api.Auditor.Load()
286-
user = httpmw.UserParam(r)
287-
aReq, commitAudit = audit.InitRequest[database.Workspace](rw, &audit.RequestParams{
288-
Audit: *auditor,
289-
Log: api.Logger,
290-
Request: r,
291-
Action: database.AuditActionCreate,
292-
})
282+
ctx = r.Context()
283+
organization = httpmw.OrganizationParam(r)
284+
apiKey = httpmw.APIKey(r)
285+
auditor = api.Auditor.Load()
286+
user = httpmw.UserParam(r)
287+
workspaceResourceInfo = audit.AdditionalFields{
288+
WorkspaceOwner: user.Username,
289+
}
293290
)
294291

292+
wriBytes, err := json.Marshal(workspaceResourceInfo)
293+
if err != nil {
294+
api.Logger.Warn(ctx, "marshal workspace owner name")
295+
}
296+
297+
aReq, commitAudit := audit.InitRequest[database.Workspace](rw, &audit.RequestParams{
298+
Audit: *auditor,
299+
Log: api.Logger,
300+
Request: r,
301+
Action: database.AuditActionCreate,
302+
AdditionalFields: wriBytes,
303+
})
304+
295305
defer commitAudit()
296306

297307
if !api.Authorize(r, rbac.ActionCreate,

‎codersdk/audit.go

+6-5
Original file line numberDiff line numberDiff line change
@@ -118,11 +118,12 @@ type AuditLogResponse struct {
118118
}
119119

120120
type CreateTestAuditLogRequest struct {
121-
Action AuditAction `json:"action,omitempty" enums:"create,write,delete,start,stop"`
122-
ResourceType ResourceType `json:"resource_type,omitempty" enums:"template,template_version,user,workspace,workspace_build,git_ssh_key,auditable_group"`
123-
ResourceID uuid.UUID `json:"resource_id,omitempty" format:"uuid"`
124-
Time time.Time `json:"time,omitempty" format:"date-time"`
125-
BuildReason BuildReason `json:"build_reason,omitempty" enums:"autostart,autostop,initiator"`
121+
Action AuditAction `json:"action,omitempty" enums:"create,write,delete,start,stop"`
122+
ResourceType ResourceType `json:"resource_type,omitempty" enums:"template,template_version,user,workspace,workspace_build,git_ssh_key,auditable_group"`
123+
ResourceID uuid.UUID `json:"resource_id,omitempty" format:"uuid"`
124+
AdditionalFields json.RawMessage `json:"additional_fields,omitempty"`
125+
Time time.Time `json:"time,omitempty" format:"date-time"`
126+
BuildReason BuildReason `json:"build_reason,omitempty" enums:"autostart,autostop,initiator"`
126127
}
127128

128129
// AuditLogs retrieves audit logs from the given page.

‎docs/api/audit.md

+1
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ curl -X POST http://coder-server:8080/api/v2/audit/testgenerate \
106106
```json
107107
{
108108
"action": "create",
109+
"additional_fields": [0],
109110
"build_reason": "autostart",
110111
"resource_id": "4d5215ed-38bb-48ed-879a-fdb9ca58522f",
111112
"resource_type": "template",

‎docs/api/schemas.md

+9-7
Original file line numberDiff line numberDiff line change
@@ -991,6 +991,7 @@ CreateParameterRequest is a structure used to create a new parameter value for a
991991
```json
992992
{
993993
"action": "create",
994+
"additional_fields": [0],
994995
"build_reason": "autostart",
995996
"resource_id": "4d5215ed-38bb-48ed-879a-fdb9ca58522f",
996997
"resource_type": "template",
@@ -1000,13 +1001,14 @@ CreateParameterRequest is a structure used to create a new parameter value for a
10001001

10011002
### Properties
10021003

1003-
| Name | Type | Required | Restrictions | Description |
1004-
| --------------- | ---------------------------------------------- | -------- | ------------ | ----------- |
1005-
| `action` | [codersdk.AuditAction](#codersdkauditaction) | false | | |
1006-
| `build_reason` | [codersdk.BuildReason](#codersdkbuildreason) | false | | |
1007-
| `resource_id` | string | false | | |
1008-
| `resource_type` | [codersdk.ResourceType](#codersdkresourcetype) | false | | |
1009-
| `time` | string | false | | |
1004+
| Name | Type | Required | Restrictions | Description |
1005+
| ------------------- | ---------------------------------------------- | -------- | ------------ | ----------- |
1006+
| `action` | [codersdk.AuditAction](#codersdkauditaction) | false | | |
1007+
| `additional_fields` | array of integer | false | | |
1008+
| `build_reason` | [codersdk.BuildReason](#codersdkbuildreason) | false | | |
1009+
| `resource_id` | string | false | | |
1010+
| `resource_type` | [codersdk.ResourceType](#codersdkresourcetype) | false | | |
1011+
| `time` | string | false | | |
10101012

10111013
#### Enumerated Values
10121014

‎site/src/api/typesGenerated.ts

+1
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,7 @@ export interface CreateTestAuditLogRequest {
201201
readonly action?: AuditAction
202202
readonly resource_type?: ResourceType
203203
readonly resource_id?: string
204+
readonly additional_fields?: Record<string, string>
204205
readonly time?: string
205206
readonly build_reason?: BuildReason
206207
}

‎site/src/components/AuditLogRow/AuditLogDescription.test.tsx

+2-2
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ describe("AuditLogDescription", () => {
3838
const AuditLogWithRepeat = {
3939
...MockAuditLogWithWorkspaceBuild,
4040
additional_fields: {
41-
workspaceName: "workspace",
41+
workspace_name: "workspace",
4242
},
4343
}
4444
render(<AuditLogDescription auditLog={AuditLogWithRepeat} />)
@@ -55,7 +55,7 @@ describe("AuditLogDescription", () => {
5555
)
5656
expect(
5757
getByTextContent(
58-
`TestUser created workspace bruno-dev on behalf of ${MockWorkspaceCreateAuditLogForDifferentOwner.additional_fields.workspaceOwner}`,
58+
`TestUser created workspace bruno-dev on behalf of ${MockWorkspaceCreateAuditLogForDifferentOwner.additional_fields.workspace_owner}`,
5959
),
6060
).toBeDefined()
6161
})

0 commit comments

Comments
 (0)