Skip to content

Update tests, add import for auth backends. - #12

Merged
paddycarver merged 4 commits into
masterfrom
paddy_auth_backend_import
Aug 29, 2017
Merged

Update tests, add import for auth backends.#12
paddycarver merged 4 commits into
masterfrom
paddy_auth_backend_import

Conversation

@paddycarver

Copy link
Copy Markdown
Contributor

Auth backends now use the path as the ID, not the name. This will allow
us to have multiple of the same auth backend mounted on the same vault
server. It will also allow us to match the auth backend using just the
path, instead of the path and the type combination.

This is largely necessary for import, where we only have the ID, so we
need the ID to be able to uniquely identify the resource, which the type
can't do, but the path can.

We also updated the initial test to randomise the path the auth backend
is mounted at, instead of mounting it at a static path, which makes
tests more resilient and less likely to fail due to collisions.

Read now sets all the fields in state.

Auth backends now use the path as the ID, not the name. This will allow
us to have multiple of the same auth backend mounted on the same vault
server. It will also allow us to match the auth backend using just the
path, instead of the path and the type combination.

This is largely necessary for import, where we only have the ID, so we
need the ID to be able to uniquely identify the resource, which the type
can't do, but the path can.

We also updated the initial test to randomise the path the auth backend
is mounted at, instead of mounting it at a static path, which makes
tests more resilient and less likely to fail due to collisions.

Read now sets all the fields in state.
@paddycarver

Copy link
Copy Markdown
Contributor Author

I'm almost positive I need to do a state migration to account for the change in the ID.

We're using helper/acctest now, so it needs to be vendored.
This was referenced Aug 26, 2017
@apparentlymart

Copy link
Copy Markdown
Contributor

I think you're right about a state migration being needed here.

That alone doesn't make this a breaking change though, since state migrations happen automatically without any user interaction. Perhaps you meant something else as the breaking change here?

@paddycarver

paddycarver commented Aug 28, 2017

Copy link
Copy Markdown
Contributor Author

Yeah, anyone that uses vault_auth_backend.foo.id in their config will get a different value than they would have before the PR, which is why I considered it breaking. Agreed state migration is BC.

Granted, that is a pretty small breaking change, but figured it was worth calling out in the CHANGELOG anyways.

@apparentlymart

Copy link
Copy Markdown
Contributor

Ahh yes, that's correct. I expected that people would use .type for this, but indeed people could be depending on .id so worth noting as a breaking change in the CHANGELOG.

Add a migration that changes our auth backend ID for us, and a test to
ensure it's working correctly.
@paddycarver

Copy link
Copy Markdown
Contributor Author

Updated with the migration and the test for the migration, this should be ready to go now.

Left a var out of a Fatalf, that's embarrassing.
@paddycarver
paddycarver merged commit b7e11f0 into master Aug 29, 2017
@paddycarver
paddycarver deleted the paddy_auth_backend_import branch August 29, 2017 20:09
dandandy pushed a commit to dandandy/terraform-provider-vault that referenced this pull request Jun 17, 2021
…ackend_import

Update tests, add import for auth backends.
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