feat: add audit package #1046
feat: add audit package #1046
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1046 +/- ##
==========================================
+ Coverage 66.24% 66.36% +0.12%
==========================================
Files 255 263 +8
Lines 16120 16413 +293
Branches 156 156
==========================================
+ Hits 10678 10893 +215
- Misses 4335 4395 +60
- Partials 1107 1125 +18
Continue to review full report at Codecov.
|
e7d0201 to
d548842
Compare
coderd/audit/table.go
Outdated
| &database.User{}: { | ||
| "id": ActionIgnore, // Never changes. | ||
| "email": ActionAuditable, // A user can edit their email. | ||
| "name": ActionAuditable, // A user can edit their name. | ||
| "revoked": ActionAuditable, // An admin can revoke a user. This is different from deletion, which is implicit. | ||
| "login_type": ActionAuditable, // An admin can update the login type of a user. | ||
| "hashed_password": ActionSecret, // A user can change their own password. | ||
| "created_at": ActionIgnore, // Never changes. | ||
| "updated_at": ActionIgnore, // Changes, but is implicit and not helpful in a diff. | ||
| "username": ActionIgnore, // A user cannot change their username. | ||
| }, | ||
| &database.Workspace{}: { | ||
| "id": ActionIgnore, // Never changes. | ||
| "created_at": ActionIgnore, // Never changes. | ||
| "updated_at": ActionIgnore, // Changes, but is implicit and not helpful in a diff. | ||
| "owner_id": ActionIgnore, // We don't allow workspaces to change ownership. | ||
| "template_id": ActionIgnore, // We don't allow workspaces to change templates. | ||
| "deleted": ActionIgnore, // Changes, but is implicit when a delete event is fired. | ||
| "name": ActionIgnore, // We don't allow workspaces to change names. | ||
| "autostart_schedule": ActionAuditable, // Autostart schedules are directly editable by users. | ||
| "autostop_schedule": ActionAuditable, // Autostart schedules are directly editable by users. | ||
| }, |
There was a problem hiding this comment.
Big fan of how this is centralized.
coderd/audit/table.go
Outdated
| "id": ActionIgnore, // Never changes. | ||
| "created_at": ActionIgnore, // Never changes. | ||
| "updated_at": ActionIgnore, // Changes, but is implicit and not helpful in a diff. | ||
| "owner_id": ActionIgnore, // We don't allow workspaces to change ownership. | ||
| "template_id": ActionIgnore, // We don't allow workspaces to change templates. | ||
| "deleted": ActionIgnore, // Changes, but is implicit when a delete event is fired. | ||
| "name": ActionIgnore, // We don't allow workspaces to change names. |
There was a problem hiding this comment.
Should we do ignore by default? I assume most actions won't need to be audited, so adding them manually won't be a big deal.
There was a problem hiding this comment.
Theoretically it is because it's the first iota. But, I'd like to make this a runtime error if any fields aren't in this list so we have a definitive list for generating the docs later.
There was a problem hiding this comment.
I suppose that's better than implicit because when editing the schema it becomes impossible to not decide whether something should be audited.
coderd/audit/table.go
Outdated
| // ActionIgnore ignores diffing for the field. | ||
| ActionIgnore = iota | ||
| // ActionAuditable includes the value in the diff if the value changed. | ||
| ActionAuditable |
There was a problem hiding this comment.
Thoughts on Track, Store, or Log instead? I'm just imaging the caller experience of audit.Track or something.
There was a problem hiding this comment.
I think I like Track better 👍
ea1aac7 to
67f813c
Compare
| // TODO: this might need to be in the database package. | ||
| type DiffMap map[string]interface{} | ||
|
|
||
| func Empty[T Auditable]() T { |
There was a problem hiding this comment.
Is this just used for tests? Might be better to toss it in the test pkg.
There was a problem hiding this comment.
I intend for this to be used by external callers, mainly because you can't pass nil anymore since we don't accept pointers. I thought it was a nice self describing replacement for nil.
67f813c to
d0ebb78
Compare
Implements: #467