Skip to content

Commit 6786ca2

Browse files
Kira-Pilotcoadler
Kira-Pilot
and
coadler
authored
Audit build outcomes/kira pilot (#5143)
* auditing failed builds * logging workspace build successes * remove duplicate workspace build entry * fixed workspacebuilds_test * PR feedback * lint and migrations * fix nil auditors * workspace_build test * fixed workspaces_teest Co-authored-by: Colin Adler <colin1adler@gmail.com>
1 parent 1f20cab commit 6786ca2

18 files changed

+184
-85
lines changed

‎coderd/audit.go

+4-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package coderd
22

33
import (
4+
"database/sql"
45
"encoding/json"
56
"fmt"
67
"net"
@@ -129,7 +130,7 @@ func (api *API) generateFakeAuditLog(rw http.ResponseWriter, r *http.Request) {
129130
Time: params.Time,
130131
UserID: user.ID,
131132
Ip: ipNet,
132-
UserAgent: r.UserAgent(),
133+
UserAgent: sql.NullString{String: r.UserAgent(), Valid: true},
133134
ResourceType: database.ResourceType(params.ResourceType),
134135
ResourceID: params.ResourceID,
135136
ResourceTarget: user.Username,
@@ -163,6 +164,7 @@ func convertAuditLog(dblog database.GetAuditLogsOffsetRow) codersdk.AuditLog {
163164
_ = json.Unmarshal(dblog.Diff, &diff)
164165

165166
var user *codersdk.User
167+
166168
if dblog.UserUsername.Valid {
167169
user = &codersdk.User{
168170
ID: dblog.UserID,
@@ -186,7 +188,7 @@ func convertAuditLog(dblog database.GetAuditLogsOffsetRow) codersdk.AuditLog {
186188
Time: dblog.Time,
187189
OrganizationID: dblog.OrganizationID,
188190
IP: ip,
189-
UserAgent: dblog.UserAgent,
191+
UserAgent: dblog.UserAgent.String,
190192
ResourceType: codersdk.ResourceType(dblog.ResourceType),
191193
ResourceID: dblog.ResourceID,
192194
ResourceTarget: dblog.ResourceTarget,

‎coderd/audit/request.go

+50-1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package audit
22

33
import (
44
"context"
5+
"database/sql"
56
"encoding/json"
67
"fmt"
78
"net"
@@ -32,6 +33,20 @@ type Request[T Auditable] struct {
3233
New T
3334
}
3435

36+
type BuildAuditParams[T Auditable] struct {
37+
Audit Auditor
38+
Log slog.Logger
39+
40+
UserID uuid.UUID
41+
JobID uuid.UUID
42+
Status int
43+
Action database.AuditAction
44+
AdditionalFields json.RawMessage
45+
46+
New T
47+
Old T
48+
}
49+
3550
func ResourceTarget[T Auditable](tgt T) string {
3651
switch typed := any(tgt).(type) {
3752
case database.Organization:
@@ -147,7 +162,7 @@ func InitRequest[T Auditable](w http.ResponseWriter, p *RequestParams) (*Request
147162
Time: database.Now(),
148163
UserID: httpmw.APIKey(p.Request).UserID,
149164
Ip: ip,
150-
UserAgent: p.Request.UserAgent(),
165+
UserAgent: sql.NullString{String: p.Request.UserAgent(), Valid: true},
151166
ResourceType: either(req.Old, req.New, ResourceType[T]),
152167
ResourceID: either(req.Old, req.New, ResourceID[T]),
153168
ResourceTarget: either(req.Old, req.New, ResourceTarget[T]),
@@ -164,6 +179,40 @@ func InitRequest[T Auditable](w http.ResponseWriter, p *RequestParams) (*Request
164179
}
165180
}
166181

182+
// BuildAudit creates an audit log for a workspace build.
183+
// The audit log is committed upon invocation.
184+
func BuildAudit[T Auditable](ctx context.Context, p *BuildAuditParams[T]) {
185+
// As the audit request has not been initiated directly by a user, we omit
186+
// certain user details.
187+
ip := parseIP("")
188+
// We do not show diffs for build audit logs
189+
var diffRaw = []byte("{}")
190+
191+
if p.AdditionalFields == nil {
192+
p.AdditionalFields = json.RawMessage("{}")
193+
}
194+
195+
err := p.Audit.Export(ctx, database.AuditLog{
196+
ID: uuid.New(),
197+
Time: database.Now(),
198+
UserID: p.UserID,
199+
Ip: ip,
200+
UserAgent: sql.NullString{},
201+
ResourceType: either(p.Old, p.New, ResourceType[T]),
202+
ResourceID: either(p.Old, p.New, ResourceID[T]),
203+
ResourceTarget: either(p.Old, p.New, ResourceTarget[T]),
204+
Action: p.Action,
205+
Diff: diffRaw,
206+
StatusCode: int32(p.Status),
207+
RequestID: p.JobID,
208+
AdditionalFields: p.AdditionalFields,
209+
})
210+
if err != nil {
211+
p.Log.Error(ctx, "export audit log", slog.Error(err))
212+
return
213+
}
214+
}
215+
167216
func either[T Auditable, R any](old, new T, fn func(T) R) R {
168217
if ResourceID(new) != uuid.Nil {
169218
return fn(new)

‎coderd/coderd.go

+1
Original file line numberDiff line numberDiff line change
@@ -681,6 +681,7 @@ func (api *API) CreateInMemoryProvisionerDaemon(ctx context.Context, debounce ti
681681
Telemetry: api.Telemetry,
682682
Tags: tags,
683683
QuotaCommitter: &api.QuotaCommitter,
684+
Auditor: &api.Auditor,
684685
AcquireJobDebounce: debounce,
685686
Logger: api.Logger.Named(fmt.Sprintf("provisionerd-%s", daemon.Name)),
686687
})

‎coderd/database/dump.sql

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

‎coderd/database/migrations/000082_add_audit_log_nullable_fields.down.sql

+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
ALTER TABLE audit_logs ALTER COLUMN ip SET NOT NULL;
2+
ALTER TABLE audit_logs ALTER COLUMN user_agent SET NOT NULL;

‎coderd/database/migrations/000082_add_audit_log_nullable_fields.up.sql

+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
ALTER TABLE audit_logs ALTER COLUMN ip DROP NOT NULL;
2+
ALTER TABLE audit_logs ALTER COLUMN user_agent DROP NOT NULL;

‎coderd/database/models.go

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

‎coderd/database/queries.sql.go

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

‎coderd/provisionerdserver/provisionerdserver.go

+86-2
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"encoding/json"
77
"errors"
88
"fmt"
9+
"net/http"
910
"net/url"
1011
"reflect"
1112
"sync"
@@ -21,6 +22,7 @@ import (
2122

2223
"cdr.dev/slog"
2324

25+
"github.com/coder/coder/coderd/audit"
2426
"github.com/coder/coder/coderd/database"
2527
"github.com/coder/coder/coderd/parameter"
2628
"github.com/coder/coder/coderd/telemetry"
@@ -46,6 +48,7 @@ type Server struct {
4648
Pubsub database.Pubsub
4749
Telemetry telemetry.Reporter
4850
QuotaCommitter *atomic.Pointer[proto.QuotaCommitter]
51+
Auditor *atomic.Pointer[audit.Auditor]
4952

5053
AcquireJobDebounce time.Duration
5154
}
@@ -522,6 +525,43 @@ func (server *Server) FailJob(ctx context.Context, failJob *proto.FailedJob) (*p
522525
case *proto.FailedJob_TemplateImport_:
523526
}
524527

528+
// if failed job is a workspace build, audit the outcome
529+
if job.Type == database.ProvisionerJobTypeWorkspaceBuild {
530+
auditor := server.Auditor.Load()
531+
build, getBuildErr := server.Database.GetWorkspaceBuildByJobID(ctx, job.ID)
532+
if getBuildErr != nil {
533+
server.Logger.Error(ctx, "failed to create audit log - get build err", slog.Error(err))
534+
} else {
535+
auditAction := auditActionFromTransition(build.Transition)
536+
workspace, getWorkspaceErr := server.Database.GetWorkspaceByID(ctx, build.WorkspaceID)
537+
if getWorkspaceErr != nil {
538+
server.Logger.Error(ctx, "failed to create audit log - get workspace err", slog.Error(err))
539+
} else {
540+
// We pass the workspace name to the Auditor so that it
541+
// can form a friendly string for the user.
542+
workspaceResourceInfo := map[string]string{
543+
"workspaceName": workspace.Name,
544+
}
545+
546+
wriBytes, err := json.Marshal(workspaceResourceInfo)
547+
if err != nil {
548+
server.Logger.Error(ctx, "could not marshal workspace name", slog.Error(err))
549+
}
550+
551+
audit.BuildAudit(ctx, &audit.BuildAuditParams[database.WorkspaceBuild]{
552+
Audit: *auditor,
553+
Log: server.Logger,
554+
UserID: job.InitiatorID,
555+
JobID: job.ID,
556+
Action: auditAction,
557+
New: build,
558+
Status: http.StatusInternalServerError,
559+
AdditionalFields: wriBytes,
560+
})
561+
}
562+
}
563+
}
564+
525565
data, err := json.Marshal(ProvisionerJobLogsNotifyMessage{EndOfLogs: true})
526566
if err != nil {
527567
return nil, xerrors.Errorf("marshal job log: %w", err)
@@ -600,11 +640,14 @@ func (server *Server) CompleteJob(ctx context.Context, completed *proto.Complete
600640
return nil, xerrors.Errorf("get workspace build: %w", err)
601641
}
602642

643+
var workspace database.Workspace
644+
var getWorkspaceError error
645+
603646
err = server.Database.InTx(func(db database.Store) error {
604647
now := database.Now()
605648
var workspaceDeadline time.Time
606-
workspace, err := db.GetWorkspaceByID(ctx, workspaceBuild.WorkspaceID)
607-
if err == nil {
649+
workspace, getWorkspaceError = db.GetWorkspaceByID(ctx, workspaceBuild.WorkspaceID)
650+
if getWorkspaceError == nil {
608651
if workspace.Ttl.Valid {
609652
workspaceDeadline = now.Add(time.Duration(workspace.Ttl.Int64))
610653
}
@@ -704,6 +747,34 @@ func (server *Server) CompleteJob(ctx context.Context, completed *proto.Complete
704747
return nil, xerrors.Errorf("complete job: %w", err)
705748
}
706749

750+
// audit the outcome of the workspace build
751+
if getWorkspaceError == nil {
752+
auditor := server.Auditor.Load()
753+
auditAction := auditActionFromTransition(workspaceBuild.Transition)
754+
755+
// We pass the workspace name to the Auditor so that it
756+
// can form a friendly string for the user.
757+
workspaceResourceInfo := map[string]string{
758+
"workspaceName": workspace.Name,
759+
}
760+
761+
wriBytes, err := json.Marshal(workspaceResourceInfo)
762+
if err != nil {
763+
server.Logger.Error(ctx, "marshal resource info", slog.Error(err))
764+
}
765+
766+
audit.BuildAudit(ctx, &audit.BuildAuditParams[database.WorkspaceBuild]{
767+
Audit: *auditor,
768+
Log: server.Logger,
769+
UserID: job.InitiatorID,
770+
JobID: job.ID,
771+
Action: auditAction,
772+
New: workspaceBuild,
773+
Status: http.StatusOK,
774+
AdditionalFields: wriBytes,
775+
})
776+
}
777+
707778
err = server.Pubsub.Publish(codersdk.WorkspaceNotifyChannel(workspaceBuild.WorkspaceID), []byte{})
708779
if err != nil {
709780
return nil, xerrors.Errorf("update workspace: %w", err)
@@ -1015,6 +1086,19 @@ func convertWorkspaceTransition(transition database.WorkspaceTransition) (sdkpro
10151086
}
10161087
}
10171088

1089+
func auditActionFromTransition(transition database.WorkspaceTransition) database.AuditAction {
1090+
switch transition {
1091+
case database.WorkspaceTransitionStart:
1092+
return database.AuditActionStart
1093+
case database.WorkspaceTransitionStop:
1094+
return database.AuditActionStop
1095+
case database.WorkspaceTransitionDelete:
1096+
return database.AuditActionDelete
1097+
default:
1098+
return database.AuditActionWrite
1099+
}
1100+
}
1101+
10181102
// WorkspaceProvisionJob is the payload for the "workspace_provision" job type.
10191103
type WorkspaceProvisionJob struct {
10201104
WorkspaceBuildID uuid.UUID `json:"workspace_build_id"`

‎coderd/provisionerdserver/provisionerdserver_test.go

+11
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,15 @@ import (
55
"database/sql"
66
"encoding/json"
77
"net/url"
8+
"sync/atomic"
89
"testing"
910
"time"
1011

1112
"github.com/google/uuid"
1213
"github.com/stretchr/testify/require"
1314

1415
"cdr.dev/slog/sloggers/slogtest"
16+
"github.com/coder/coder/coderd/audit"
1517
"github.com/coder/coder/coderd/database"
1618
"github.com/coder/coder/coderd/database/databasefake"
1719
"github.com/coder/coder/coderd/provisionerdserver"
@@ -21,6 +23,13 @@ import (
2123
sdkproto "github.com/coder/coder/provisionersdk/proto"
2224
)
2325

26+
func mockAuditor() *atomic.Pointer[audit.Auditor] {
27+
ptr := &atomic.Pointer[audit.Auditor]{}
28+
mock := audit.Auditor(audit.NewMock())
29+
ptr.Store(&mock)
30+
return ptr
31+
}
32+
2433
func TestAcquireJob(t *testing.T) {
2534
t.Parallel()
2635
t.Run("Debounce", func(t *testing.T) {
@@ -36,6 +45,7 @@ func TestAcquireJob(t *testing.T) {
3645
Pubsub: pubsub,
3746
Telemetry: telemetry.NewNoop(),
3847
AcquireJobDebounce: time.Hour,
48+
Auditor: mockAuditor(),
3949
}
4050
job, err := srv.AcquireJob(context.Background(), nil)
4151
require.NoError(t, err)
@@ -799,5 +809,6 @@ func setup(t *testing.T) *provisionerdserver.Server {
799809
Database: db,
800810
Pubsub: pubsub,
801811
Telemetry: telemetry.NewNoop(),
812+
Auditor: mockAuditor(),
802813
}
803814
}

0 commit comments

Comments
 (0)