Upgrade to 0.12 #446
Merged
Upgrade to 0.12 #446
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
skip_tls_verify expects a boolean, but defaulted to an empty string, which causes an error under 0.12.
The test case for updating a v2 generic secret had a check function that hardcoded in the initial value, and then proceeded to check that against the updated value, which would never work. This updates to add a check function for the updated config, with the updated value coded in.
0.12 complained about our plan changing because we were interpolating a backend's path. Backend paths cannot be specified using a trailing / but are always set in state with a trailing /. That means the plan will always change when creating a backend and using it in the same request. This commit changes that to set the backend in state with no trailing slash, which resolves the issue because it will always match user input now. This may be considered a breaking change.
Modify provider_test to load GCP test creds from file _or_ accept them as the contents of the file. Also, run go mod tidy.
The service_account_project field the tests were expecting from the API has been transitioned to the project field. Update the test to check the right field.
We're storing the path in state without the trailing slash, and we need to update the test that assumes we'll have the trailing slash.
Blocks can no longer be specified using =.
Blocks can no longer be specified using =.
Update the error message we're testing agains, as it has changed in 0.12.
The basic LDAP auth backend group test had policies specified as a list, with the util function for turning a slice into a Terraform list inside that list. Except the util helper also returned the square brackets, so we had `[[item, item]]` instead of `[item, item]`. In Terraform 0.11, this was fine. In Terraform 0.12, it is not.
It appears 0.12 doesn't consider HasChange true when the change has been marked by CustomizeDiff? By refactoring the logic for checking if renewal is necessary into a helper function and calling that function again instead of HasChange we can work around this limitation.
Write to a different path with every test run, so our version is predictable.
Test that sending the version explicitly works, and also test that retrieving the latest works.
Make our postgres database secret backend use a list of statements for creation, revocation, rollback, and renew, as the API has updated to use a list for those fields. This is a breaking change, as we're changing the schema type, but we coded in support for previous versions of the API that returned those fields as strings, as well.
The bound principal arns for the AWS auth role backend were being set incorrectly, because we were keying off what the user had in the config to determine whether the response should be a string or a list. But the server always responded with a list, no matter what was sent. We updated to remove the string version, as we're doing a major release anyways, and to test the response type before setting it so we don't set the wrong type. Login was also trying to set a lease_id that doesn't exist in the schema, so we just removed that Set call.
Remove all the deprecated singular versions from the aws_auth_backend_role resource, and always set the plural versions. Fixes an error with d.Set, should be backwards compatible with Vault versions. Is a breaking change for the provider.
Fix a type when calling d.Set on the client_secret for the azure_auth_backend_config resource.
Finish fixing d.Sets that are using the wrong types: * pki_secret_backend_config_urls' issuing_certificates, crl_distribution_points, and ocsp_servers are now lists, to match the API. * pki_secret_backend_role's allowed_uri_sans, allowed_other_sans, ou, organization, country, locality, province, street_address, and postal_code fields are now lists, to match the API. * pki_secret_backend_sign's ca_chain field is now a list, to match the API. * token's no_parent, renewable, and num_uses fields are now computed. Only set the client secret for Azure's auth backend if it's in the API response, so we don't overwrite it with empty data. Even with TF_SCHEMA_PANIC_ON_ERROR, all non-database/rabbitMQ/enterprise tests now pass.
Don't remove the values from the database_secret_backend_connection resource's cassandra block that the API doesn't return. Because we overwrite the whole block in state, we need to fall back on what's in state, which presumably came from the config. We'll miss any drift, but the API doesn't surface that information, so there's not much we can do about that.
Keep on keeping on with tracking the latest.
Don't rely on a cluster having a secret/ KV mount already, create one for each test.
Anything that was deprecated in Terraform (not in Vault 1.1) is now marked as removed, in anticipation of a 2.0.0.
Fix the handling of the aws_auth_backend_login nonce field, as it's only returned on the first request, and we need to not overwrite it with an empty string on subsequent reads. Also, fix the tests so they can be run using an environment variable, and stop using removed fields in the tests.
0.12 changed the format of an error message on us, update the test to match.
When a namespace isn't found during read, log that and remove it from state instead of panicking.
The vault API accepts the vhost for rabbitmq_secret_backend_role as a string, but always returns it as a JSON object. There's no Terraform equivalent (maps can't have complex types as the values), so instead we turn it into a list (sets were having funky behavior with the hashes, and I didn't have time to fully look into it). This is a better UX than a JSON object marshalled into a string in the config, and allows reads to be consistent. It is, however, a breaking change.
When doing an ImportStateVerify for rabbitmq_secret_backends, the verify_connection field isn't in the API response and so can't be set, so we need to ignore it.
ghost
added
the
This was referenced Jun 13, 2019
lawliet89
reviewed
Jun 14, 2019
| @@ -146,6 +147,7 @@ func authBackendRead(d *schema.ResourceData, meta interface{}) error { | |||
| } | |||
|
|
|||
| for path, auth := range auths { | |||
| path = strings.TrimSuffix(path, "/") | |||
2 tasks
tyrannosaurus-becks
approved these changes
Jun 18, 2019
|
Wow, @paddycarver , thank you! |
We have some computed optional blocks that will exhibit the computed optional problem of 0.12. In 0.11, these blocks could be set like so `block = []` to set the block to an explicit empty value. This could be used to signal a return to the API default, or as a way to delete the entry the user entered. Because 0.12 makes a strict delineation between blocks and attributes, we need to tell Terraform to treat these blocks as attributes to preserve this behavior. See https://www.terraform.io/docs/extend/terraform-0.12-compatibility.html#computed-resource-attributes for more info.
|
@tyrannosaurus-becks I had to push c3e9342 (commit message has more details) to fix one last set of issues, this should be code complete and good to go now. Does this still look good to you? If so, I'll push an upgrade guide PR in the morning and we can roll this out. |
paultyng
approved these changes
Jun 19, 2019
This was referenced Jun 20, 2019
|
Note this PR did not update the documentation for how to use |
dandandy
pushed a commit
to dandandy/terraform-provider-vault
that referenced
this pull request
Jun 17, 2021
Upgrade to 0.12
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This monster PR upgrades the provider to use the Terraform 0.12 SDK. Because a breaking change was required for that, it also removes deprecated fields.
I also ran all the acceptance tests, and quite a few were broken, so I fixed tests as I went. This now passes all acceptance tests, including tests that rely on external databases or APIs.
A manual examination of whether the
github_auth_backend.tune,okta_auth_backend.user,okta_auth_backend.group, andjwt_auth_backend.tunefields fall prey to the optional computed block issue. I'll update in this PR when that's done, but I figured it best to share what I had when all the tests pass, to get a start on review.It may be helpful to browse changes by commit.