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

Add database secret backend connection resource. #37

Merged
merged 4 commits into from
Feb 22, 2018

Conversation

paddycarver
Copy link
Contributor

Add a resource for creating and managing database secret backend
connections. These allow users to generate dynamic database access
credentials using Vault.

The design of this resource is a bit weird, because the design of the
API is a bit weird. There are a core set of fields that every database
plugin supports, and then each plugin has its own parameters specific to
that plugin. However, the SQL-ish databases use the same structure,
though this is not documented as intentional, and appears to be
coincidental.

My resolution to this was to use a base resource that contains at its
root the common fields for all databases. Then each supported plugin got
its own field within the resource, under which its specific parameters
are specified. Finally, helper functions mean that very little logic is
duplicated for each of the SQL databases that share configuration
structure, but if any one of them strays in the future, we can add
special handling for it without a backwards compatibility problem.

The shortcoming of this is that no custom database backends can be used
unless they're added to the resource. I examined using a similar
approach to generic_secret, and allowing a raw JSON string to be
specified, but the end result wasn't much an improvement over
generic_secret, so I abandoned it. For users that need custom backend
support, they'll need to continue relying on generic_secret or write
their own resource. I can't think of any way to avoid that.

Testing, of course, is a pain, seeing as we have 7 different databases
to test against. I opted to test against 5 of them. I used docker
containers for all but postgres (which I had installed locally already
and am confident we can use a Docker container for). The two I did not
write tests for were Oracle and SAP HanaDB. The Oracle plugin isn't
shipped as part of Vault, as far as I can tell, and is user-installed.
Rather than complicating our testing setup, I chose not to write a test
for it. SAP HanaDB I attempted to get running in a Docker container, but
it requires such finicky setup I felt it would be more trouble than it
was worth to get the database supported on our CI platform. I felt OK
with both these decisions given that they both use the SQL configuration
style, meaning three other tests already follow almost identical code
paths.

It may take some prodding (I'm not 100% sure how our CI system launches
the necessary Docker containers, but I know that it has the capability)
but I feel confident we'll be able to support these tests as nightly CI
runs.

paddycarver added a commit that referenced this pull request Nov 13, 2017
Add a resource that will allow the creation and management of database
secret backend roles. These roles are what are ultimately used to
generate temporary credentials for the database.

This relies on #37, which should be merged before this.
apparentlymart
apparentlymart previously approved these changes Dec 6, 2017
Copy link
Contributor

@apparentlymart apparentlymart left a comment

Choose a reason for hiding this comment

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

LGTM!

The idea of using a different nested block for each supported database seems like a reasonable compromise here within our current constraints.

* `allowed_roles` - (Optional) A list of roles that are allowed to use this
connection.

* `cassandra` - (Optional) Configuration options for Cassandra connections.
Copy link
Contributor

Choose a reason for hiding this comment

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

For these, I think it would help to explicitly say "A nested block containing ..." to help explain how the attributes described in the subsequent sections are to be used, since this is not usual idiom so it may not be immediately obvious what we're expecting here.

Perhaps also a paragraph at the end of this list explaining that at exactly one of these must be provided and the chosen block decides which database plugin is used... I'm thinking about someone who is familiar with Vault trying to map their understanding onto these docs, where we've kinda hidden the idea of a "plugin" and so we don't have that hook here to help someone figure out how that concept maps.

@paddycarver paddycarver merged commit 1fd1765 into master Feb 22, 2018
@tyrannosaurus-becks tyrannosaurus-becks deleted the paddy_database_secret_backend_connection branch February 16, 2019 00:21
dandandy pushed a commit to dandandy/terraform-provider-vault that referenced this pull request Jun 17, 2021
Add a resource that will allow the creation and management of database
secret backend roles. These roles are what are ultimately used to
generate temporary credentials for the database.

This relies on hashicorp#37, which should be merged before this.
dandandy pushed a commit to dandandy/terraform-provider-vault that referenced this pull request Jun 17, 2021
…se_secret_backend_connection

Add database secret backend connection resource.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants