New Resource: vault_cert_auth_backend_role #123
Conversation
|
@angrylogic maybe if you add the new-resource label then they'll pay attention to this? I know they're overloaded at this time but maybe it would make a visual statement that Hashi should prioritize this provider if we had a ton of "new resource" labels applied to the relevant open PRs. |
There was a problem hiding this comment.
@angrylogic thank you for contributing this! It looks great!
I checked out the branch and ran the acceptance tests and they're not passing for me. Here are the steps to reproduce:
- Run Vault 11.1 locally via
vault server -dev - Export TF_ACC=1, VAULT_TOKEN=whatever, and VAULT_ADDR=http://localhost:8200
And then when I run the tests I get:
--- FAIL: TestCertAuthBackend_basic (0.03s)
testing.go:434: Step 0 error: Check failed: resource not found in state
Let me know if I'm doing something wrong.
Happy to approve and merge once you get a chance to look at it. Apologies for the long delay while the repo was being transitioned from the Terraform team to the Vault team.
| Delete: certAuthResourceRead, | ||
|
|
||
| Schema: map[string]*schema.Schema{ | ||
| "name": &schema.Schema{ |
There was a problem hiding this comment.
Would it be possible to strip the redundant field name declarations here and on the rest of the fields?
There was a problem hiding this comment.
@tyrannosaurus-becks - I'm not sure I follow what you mean re: redundant field names here.
There was a problem hiding this comment.
Sorry, I didn't phrase it very well. My code editor complains if, in a map like map[string]*schema.Schema, then &schema.Schema is also declared below. Here's a visual example:
It's underlining and greying out those words because they can be safely omitted.
I know it hasn't always been that way, but it would be great to go that way going forward. I'm thinking of doing a quick pass through the pre-existing code updating stuff like that.
|
|
||
| log.Printf("[DEBUG] Writing %q to cert auth backend", path) | ||
| _, err := client.Logical().Write(path, data) | ||
| d.SetId(path) |
There was a problem hiding this comment.
Could this be moved up a line so it's not between the err and the err check?
There was a problem hiding this comment.
No problem, will fix.
|
I'll go ahead and get those tests and changes sorted out early next week - same for the other PR. Thanks for taking a look at these @tyrannosaurus-becks ! |
This PR allows for managing the Vault Cert authentication backend.
I had renamed the resource but forgotten to update the references in the state file. Tests are now passing.
angrylogic
force-pushed
the
resource_cert_auth_backend_role
branch
from
16c9303
to
bae50e7
Compare
ghost
added
the
|
@tyrannosaurus-becks -- Rebased and I think I've covered all the requested changes. Tests passing for me now: Thanks! |
…ackend_role New Resource: vault_cert_auth_backend_role
This PR allows for managing the Vault Cert authentication backend.