provider/vault: vault_auth_backend resource

[Mongey]

mitchellh/mapstructure bump is needed to handle a conversion fromjson.Number to int in vault/api

[apparentlymart]

Hi @Mongey! Thanks for implementing this.

Am I understanding correctly that this is for enabling Vault auth backends? If so, I'd like to suggest to call it vault_auth_backend rather than just vault_auth. It feels clearer to me to use a count noun here, so we can describe what this does as "enable an auth backend" rather than just "enable an auth".

One other bit of design feedback is that although by default Vault will mount a given auth backend at a path whose name matches the backend name, this is not actually required and it's possible to mount the same auth backend multiple times e.g. to support multiple different AWS accounts, or Github organizations. To support that, perhaps we could have an additional optional+computed path argument that gives the path name, with it defaulting to the same value as given in type if not specified.

I didn't have time yet to dig into the code in detail or test it but I will take a look at this more deeply soon.

[Mongey]

Thanks for the feedback @apparentlymart

If so, I'd like to suggest to call it vault_auth_backend rather than just vault_auth.

💯 I'll update

... it's possible to mount the same auth backend multiple times e.g. to support multiple different AWS accounts, or Github organizations.

Ah cool, I didn't really understand what the path arg was when I was implementing this.

To support that, perhaps we could have an additional optional+computed path argument that gives the path name, with it defaulting to the same value as given in type if not specified.

Makes sense, ^ mimics the behaviour of the vault cli auth-enable command.

[jen20]

Hi @Mongey! There are a few changes that need to be made to this before it can be merged. I'm going to go ahead and pull it down locally to fix it up, and then will open a new PR with the changes.

[jason-riddle]

Any update on this?

[Mongey]

@stack72 / @apparentlymart 👀 This might need to be re-assigned 😓

[stack72]

Hi @Mongey

Apologies for this - yes @jen20 no longer works with us :(

Can you rebase it and I will start a review?

Paul

[Mongey]

@stack72 🙇 rebased the conflict away

[stack72]

I think if we fall out, we should log a message to let the user know that their state has been refreshed - because it wasn't found

[stack72]

Hi @Mongey

So I just tested this and it works as expected :) When i ran your tests, I could see my Vault log as follows:

2017/03/16 10:41:04.499466 [INFO ] core: enabled credential backend: path=github/ type=github
2017/03/16 10:41:05.030997 [INFO ] core: disabled credential backend: path=github/
2017/03/16 10:41:05.033431 [INFO ] core: enabled credential backend: path=ldap/ type=ldap
2017/03/16 10:41:05.385489 [INFO ] core: disabled credential backend: path=ldap/

The tests are also green:

% make testacc TEST=./builtin/providers/vault                                                      ✭
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/03/16 12:40:55 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/vault -v  -timeout 120m
=== RUN   TestDataSourceGenericSecret
--- PASS: TestDataSourceGenericSecret (0.67s)
=== RUN   TestProvider
--- PASS: TestProvider (0.00s)
=== RUN   TestResourceAuth
--- PASS: TestResourceAuth (1.04s)
=== RUN   TestResourceGenericSecret
--- PASS: TestResourceGenericSecret (0.87s)
=== RUN   TestResourcePolicy
--- PASS: TestResourcePolicy (0.85s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/vault	3.450s

I left 1 minor nit - but that's not worth blocking the PR over :)

Paul

[stack72]

Also, just a FYI, the failure in the build here is unrelated to this PR. The failure is as follows:

go test -timeout=30s -parallel=4 github.com/hashicorp/terraform/state/remote github.com/hashicorp/terraform/terraform 
ok  	github.com/hashicorp/terraform/state/remote	0.772s
map[string]dag.Vertex{}
"module.middle.module.inner.null"
map[string]dag.Vertex{}
"module.middle.null"
--- FAIL: TestContext2Plan_computedValueInMap (0.01s)
	context_plan_test.go:2994: err: 2 error(s) occurred:
		
		* module.test_mod.var.services: variable services in module test_mod should be type list, got map
		* shadow graph: module.test_mod.var.services: variable services in module test_mod should be type list, got map
map[string]dag.Vertex{}
"aws"
FAIL
FAIL	github.com/hashicorp/terraform/terraform	4.011s
make: *** [test] Error 123

Merging this for now and will speak to the team about the failure

[stack72]

Actually, will speak to the team first - I believe the test failure is because of the updated vendoring

[coen-hyde]

Any progress on this? I'm pretty excited about having more of Vault configured via Terraform. Avoids the custom provisioning scripts.

[Mongey]

@stack72 looks like mapstructure got updated in another PR. Rebased

[stack72]

This LGTM now @Mongey :) Nice work on this!

Paul

[leighmhart]

Sounds interesting! Any docs for this?

[Mongey]

@allandrick I can follow up with a PR. (This might help in the time being)

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.