Add audit links/kira pilot #5156
Conversation
| BuildNumber string | ||
| } | ||
|
|
||
| func (api *API) auditLogIsResourceDeleted(ctx context.Context, alog database.GetAuditLogsOffsetRow) bool { |
There was a problem hiding this comment.
@coadler I'm concerned the queries in this switch will take a while to resolve. Is there a better way to determine if a resource has been deleted?
There was a problem hiding this comment.
I don't think there's a great way to do so without something hacky like reflection here.
There was a problem hiding this comment.
I guess the only way to really fix this would be to return if the resource is deleted in the GetAuditLogsOffset query.
There was a problem hiding this comment.
As per our Slack convo, will try to add that as a fast follow!
| {auditLog.resource_link ? ( | ||
| <Link component={RouterLink} to={auditLog.resource_link}> | ||
| <strong>{target}</strong> | ||
| </Link> |
There was a problem hiding this comment.
Having a link on workspace name for workspace_build strings is a little awkward - it might be better if this link was built around the word build instead of on the workspace target. However, that add a surprising amount of complexity, so I figured this was okay for now. If you object, let me know!
coderd/audit.go
Outdated
| User: user, | ||
| Description: auditLogDescription(dblog), | ||
| ResourceLink: api.auditLogResourceLink(ctx, dblog), |
There was a problem hiding this comment.
Because this calls api.auditLogIsResourceDeleted as well, you are making the same database calls twice here. One way to speed this up and save the work would be to call api.auditLogIsResourceDeleted before this struct and pass it into both IsDeleted and also as an arg into api.auditLogResourceLink.
There was a problem hiding this comment.
Woops, you're totally right! Pushing a fix now.
| BuildNumber string | ||
| } | ||
|
|
||
| func (api *API) auditLogIsResourceDeleted(ctx context.Context, alog database.GetAuditLogsOffsetRow) bool { |
There was a problem hiding this comment.
I guess the only way to really fix this would be to return if the resource is deleted in the GetAuditLogsOffset query.
coderd/audit.go
Outdated
| template, err := api.Database.GetTemplateByID(ctx, alog.ResourceID) | ||
| if err != nil { | ||
| api.Logger.Error(ctx, "could not fetch template", slog.Error(err)) | ||
| } |
There was a problem hiding this comment.
I think we should also check if xerrors.Is(err, sql.ErrNoRows) { return true }, in case the assumption that rows are never deleted changes.
coderd/audit.go
Outdated
| case database.ResourceTypeUser: | ||
| user, err := api.Database.GetUserByID(ctx, alog.ResourceID) | ||
| if err != nil { | ||
| api.Logger.Error(ctx, "could not fetch user", slog.Error(err)) |
There was a problem hiding this comment.
small nit; normally we don't prefix log lines with could not/failed to because the error level implies it
We've added support for links in certain audit log entries, like those for resources
workspaceandworkspace_build:as well as
templateanduser:These links do not appear if the resource is deleted, as we do not have great UI support for deleted resources at this moment in time. Instead, we show a
deletedlabel next to the audit log description.