Skip to content

feat: add workspace build start/stop to audit log #4744

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Oct 25, 2022

Conversation

Kira-Pilot
Copy link
Member

resolves #4625

This PR replaces #4636 which had a duplicate migration.
Screen Shot 2022-10-25 at 10 37 37 AM

@Kira-Pilot Kira-Pilot requested a review from a team as a code owner October 25, 2022 14:37
@Kira-Pilot Kira-Pilot requested review from code-asher and kylecarbs and removed request for a team October 25, 2022 14:37
Copy link
Member

@code-asher code-asher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still trying to figure out how to get the audit log enabled on v2 dev so I have not tested it yet but it looks good to me.

if alog.ResourceType == database.ResourceTypeWorkspaceBuild {
workspaceBytes := []byte(alog.AdditionalFields)
var workspaceResourceInfo WorkspaceResourceInfo
_ = json.Unmarshal(workspaceBytes, &workspaceResourceInfo)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this error be worth surfacing (even just through the logger) or is it expected that this might sometimes not contain workspace info?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure!

@@ -278,28 +278,59 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) {
return
}

// we only want to create audit logs for delete builds right now
auditor := api.Auditor.Load()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really related to your PR since this was already here so feel free to ignore but I wonder if the auditor should be set up before the authorization check so we can log unauthorized request attempts.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good eye! We have a separate ticket for this!

"workspaceName": workspace.Name,
}

wriBytes, _ := json.Marshal(workspaceResourceInfo)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems impossible for this to error but swallowed errors make me anxious 😨 Should we log it just in case?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, definitely!

Copy link
Member

@code-asher code-asher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested with much success 🎉 One very minor thing that caught my eye was that the workspace name is bolded on create/delete but not update but we would need to refactor how we do the description to support that I think.

@Kira-Pilot
Copy link
Member Author

@code-asher thanks! I have a follow-up PR with that bold fix :)

@Kira-Pilot Kira-Pilot merged commit 3c5e292 into main Oct 25, 2022
@Kira-Pilot Kira-Pilot deleted the add-start-stop-audit/kira-pilot branch October 25, 2022 19:34
@github-actions github-actions bot locked and limited conversation to collaborators Oct 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

audit: log workspace start and stop actions
2 participants