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

secrets/db: improves error logs for static role rotation #22253

Merged
merged 3 commits into from Aug 8, 2023

Conversation

austingebauer
Copy link
Member

This PR improves the error logs that can result from failed database static role rotations by including the role and database names. This will help Vault operators know where to start troubleshooting when these logs appear.

Fixes: #21433

@austingebauer austingebauer added this to the 1.15 milestone Aug 8, 2023
@austingebauer austingebauer requested a review from a team August 8, 2023 21:38
@github-actions github-actions bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Aug 8, 2023
@github-actions
Copy link

github-actions bot commented Aug 8, 2023

Build Results:
All builds succeeded! ✅

@github-actions
Copy link

github-actions bot commented Aug 8, 2023

CI Results:
All Go tests succeeded! ✅

@calvn
Copy link
Member

calvn commented Aug 8, 2023

I wonder if it's worth leaving a changelog note on this as an improvement to the DB engine.

@fairclothjm
Copy link
Contributor

fairclothjm commented Aug 8, 2023

Does the mount_accessor not work for identifying the backend? That is included in the logs but maybe it isn't as human friendly?

EDIT: mount_accessor would not help to identify what database config and role failed rotation

@fairclothjm
Copy link
Contributor

Should we initialize the logger with hclog.With() https://github.com/hashicorp/go-hclog/blob/f7ed9e449c8dc3102bfcb86a8bda2855e07645b8/logger.go#L190 so that all logs are consistent and we don't have to worry about missing this if we add log lines in the future?

@austingebauer
Copy link
Member Author

@calvn - Thanks, will add the changelog entry.

@fairclothjm - Good idea. Let me see on the logger.

@austingebauer
Copy link
Member Author

@fairclothjm - Change to use hclog.With() in e544f01. Good suggestion, that cleaned up nicely.

@austingebauer austingebauer added backport/1.13.x Backport changes to `release/1.13.x` backport/1.14.x Backport changes to `release/1.14.x` labels Aug 8, 2023
@austingebauer austingebauer merged commit a70aaf2 into main Aug 8, 2023
101 checks passed
@austingebauer austingebauer deleted the improve-db-static-rotation-logs branch August 8, 2023 23:28
hellobontempo pushed a commit that referenced this pull request Aug 18, 2023
* secrets/db: improves error logs for static role rotation

* use logger.With to add incremental context

* adds changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.13.x Backport changes to `release/1.13.x` backport/1.14.x Backport changes to `release/1.14.x` 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.

Log error message for failed database password rotation does not list database
3 participants