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

database: Emit event notifications #24718

Merged

Conversation

swenson
Copy link
Contributor

@swenson swenson commented Jan 8, 2024

Including for failures to write credentials and failure to rotate.

@swenson swenson added this to the 1.16.0-rc1 milestone Jan 8, 2024
@swenson swenson requested a review from a team January 8, 2024 23:49
@swenson swenson requested review from a team as code owners January 8, 2024 23:49
@github-actions github-actions bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Jan 8, 2024
Copy link

github-actions bot commented Jan 9, 2024

CI Results:
All Go tests succeeded! ✅

Copy link

github-actions bot commented Jan 9, 2024

Build Results:
All builds succeeded! ✅

Copy link
Contributor

@fairclothjm fairclothjm left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@tomhjp tomhjp left a comment

Choose a reason for hiding this comment

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

LGTM, just a couple of nits

if err == nil {
b.dbEvent(ctx, "rotate-root", req.Path, name, true)
} else {
b.dbEvent(ctx, "rotate-root-fail", req.Path, name, false)
Copy link
Contributor

Choose a reason for hiding this comment

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

The failure modes are unlikely, but I think it should be possible for modified to be true in this case, if for example the write to storage fails after the password successfully got updated. I think the same is true for a couple of other APIs as well.

@@ -585,6 +600,34 @@ func TestBackend_basic(t *testing.T) {
t.Fatalf("Creds should not exist")
}
}
assert.Equal(t, 9, len(eventSender.Events))
assert.Equal(t, "database/config-write", string(eventSender.Events[0].Type))
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Would be nice to use a for loop to reduce the copy-paste/visual noise here :)

Christopher Swenson added 4 commits February 5, 2024 10:03
@swenson swenson force-pushed the vault-21858/rotate-fail-events/vault-16968/database-events branch from a8298b0 to 14979e8 Compare February 5, 2024 18:04
@swenson
Copy link
Contributor Author

swenson commented Feb 5, 2024

Thanks!

@swenson swenson merged commit 55d2dfb into main Feb 5, 2024
114 checks passed
@swenson swenson deleted the vault-21858/rotate-fail-events/vault-16968/database-events branch February 5, 2024 18:30
Monkeychip pushed a commit that referenced this pull request Feb 5, 2024
Including for failures to write credentials and failure to rotate.
Monkeychip pushed a commit that referenced this pull request Feb 7, 2024
Including for failures to write credentials and failure to rotate.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

3 participants