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

Combined Database backend: remove create/delete support #6951

Merged
merged 4 commits into from Jun 23, 2019

Conversation

catsby
Copy link
Member

@catsby catsby commented Jun 21, 2019

After #6834 was merged we decided to remove the support for creating or deleting database users with the new Static Roles feature. Instead, Vault requires the database user already exist at the time the Static Role is created, and continue to exist for the duration that Vault will manage password rotation. When destroying the Static Role in Vault, we will no longer offer or attempt to also delete the database user. We felt that the core purpose / benefit of this feature is to automate password rotation, and as such we should not cross over the boundary into managing the existence of the database user as well.

This PR removes all database statement fields except rotation_statements, as well as the optional toggle to delete the database user, from the Static Role configuration.

Misc. details:

  • refactored out some of the "common" methods like pathRoleReadCommon. Static Roles no longer have any statements other than rotation_statements, so the common methods seemed superfluous.
  • refactored tests to always create the database user in advance
  • refactored some tests that used to check for database user being created or removed, since Static Roles no longer do that
  • refactored the PostgreSQL implementation to only reference the rotation statements

Copy link
Contributor

@tyrannosaurus-becks tyrannosaurus-becks left a comment

Choose a reason for hiding this comment

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

Do we need to worry about this test failure?

=== RUN   TestPostgresSQL_SetCredentials
--- FAIL: TestPostgresSQL_SetCredentials (2.61s)
    postgresql_test.go:356: err: empty rotation statements
FAIL plugins/database/postgresql.TestPostgresSQL_SetCredentials (2.61s)
FAIL plugins/database/postgresql

@catsby
Copy link
Member Author

catsby commented Jun 21, 2019

Do we need to worry about this test failure?

@tyrannosaurus-becks Yes, thanks for pointing that out 😄 I fixed that in 1367ae4

@catsby catsby merged commit dc4e378 into master Jun 23, 2019
catsby added a commit that referenced this pull request Jun 24, 2019
* master: (47 commits)
  Add the ability to use a dev Consul node for dev storage (#6965)
  Update CHANGELOG.md
  Correct API docs examples (#6963)
  Fix test
  changelog++
  Allow turning on client auth in test clusters (#6958)
  Update vendoring
  Update SDK version
  Make CA certificate optional in ClientTLSConfig
  Update vendor
  Combined Database backend: remove create/delete support (#6951)
  Bump sdk
  Move tls config creation to tlsutil (#6956)
  Update JWT tips (#6955)
  raft join tls (#6932)
  changelog++
  UI - add kmip engine (#6936)
  Pass context to Cassandra queries (#6954)
  Minor clean up JWT provider docs (#6952)
  update azure instructions (#6858)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants