Skip to content
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

Audit: check if context is already cancelled when assessing viability for audit #27531

Merged
merged 2 commits into from
Jun 18, 2024

Conversation

peteski22
Copy link

@peteski22 peteski22 commented Jun 18, 2024

Description

When the audit system is asked to log a request or response, it now checks the viability of the existing supplied context. The context is considered unviable if it has <5s remaining on its deadline, and a new context is created for use.

However, we're seeing issues where some users are getting audit log failures as the incoming context is already cancelled entirely. This means that audit will fail and report telemetry etc. which isn't what users want.

This PR includes the cancelled context in its evaluation of viability, and will create a new context with a 5s timeout if it is cancelled.

NOTE: We don't just return immediately from LogRequest and LogResponse if the context was cancelled, as we want to make every effort to capture that Vault was asked to service a request - in the audit log.

Should resolve: #27521

@peteski22 peteski22 added core/audit hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed backport/ent/1.16.x+ent Changes are backported to 1.16.x+ent backport/1.17.x labels Jun 18, 2024
@peteski22 peteski22 added this to the 1.16.5 milestone Jun 18, 2024
Copy link

github-actions bot commented Jun 18, 2024

CI Results:
All Go tests succeeded! ✅

@peteski22 peteski22 marked this pull request as ready for review June 18, 2024 18:42
Copy link
Contributor

@VioletHynes VioletHynes left a comment

Choose a reason for hiding this comment

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

Nice! Looks good to me :)

@peteski22 peteski22 enabled auto-merge (squash) June 18, 2024 18:51
Copy link

Build Results:
All builds succeeded! ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/ent/1.16.x+ent Changes are backported to 1.16.x+ent core/audit hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Still seeing "event not processed by enough 'sink' nodes" error in vault log
2 participants