Upgrade to use TF SDK v2 #1175

benashz · 2021-09-21T14:32:44Z

The Vault Terraform provider is currently using v1 of the terraform-plugin-sdk, which is no longer seeing active feature development. The TF Plugin SDK v2 was released on July 30, 2020 and is considered to be the default SDK for all TF plugins. This PR includes the bulk of what is need to upgrade to v2. There are a few potentially breaking changes:

Support for pgp_key encryption on the vault_token resource has been dropped. This is inline with upstream SDK changes.
Multiple deprecated fields have been removed from some resources
Terraform 0.12.0+ is now required

Community Note

Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

Release note for CHANGELOG:

TBD:....

Output from acceptance testing:

$ make testacc TESTARGS='-v'
==> Checking that code complies with gofmt requirements...
go build -o terraform-provider-vault
mv terraform-provider-vault ~/.terraform.d/plugins/
TF_ACC=1 go test $(go list ./...) -v -v -timeout 120m
?       github.com/hashicorp/terraform-provider-vault   [no test files]
?       github.com/hashicorp/terraform-provider-vault/cmd/coverage      [no test files]
?       github.com/hashicorp/terraform-provider-vault/cmd/generate      [no test files]
=== RUN   TestCodeFilePath
--- PASS: TestCodeFilePath (0.00s)
=== RUN   TestDocFilePath
--- PASS: TestDocFilePath (0.00s)
=== RUN   TestStripCurlyBraces
=== RUN   TestStripCurlyBraces/{test}
=== RUN   TestStripCurlyBraces/{{name}}
=== RUN   TestStripCurlyBraces/name
=== RUN   TestStripCurlyBraces/{name
--- PASS: TestStripCurlyBraces (0.00s)
    --- PASS: TestStripCurlyBraces/{test} (0.00s)
    --- PASS: TestStripCurlyBraces/{{name}} (0.00s)
    --- PASS: TestStripCurlyBraces/name (0.00s)
    --- PASS: TestStripCurlyBraces/{name (0.00s)

[...]

=== RUN   TestAccResourceTerraformCloudSecretCredsTeamBasic

--- SKIP: TestAccResourceTerraformCloudSecretCredsTeamBasic (0.00s)

[...]

--- SKIP: TestTerraformCloudSecretRole (0.00s)
=== RUN   TestTerraformCloudSecretBackendRoleNameFromPath
--- PASS: TestTerraformCloudSecretBackendRoleNameFromPath (0.00s)
=== RUN   TestTerraformCloudSecretBackendRoleBackendFromPath
--- PASS: TestTerraformCloudSecretBackendRoleBackendFromPath (0.00s)
=== RUN   TestAccTokenAuthBackendRoleImport
--- PASS: TestAccTokenAuthBackendRoleImport (1.70s)
=== RUN   TestAccTokenAuthBackendRole
--- PASS: TestAccTokenAuthBackendRole (1.45s)
=== RUN   TestAccTokenAuthBackendRoleUpdate
--- PASS: TestAccTokenAuthBackendRoleUpdate (4.74s)
=== RUN   TestAccTokenAuthBackendRoleDeprecated
--- PASS: TestAccTokenAuthBackendRoleDeprecated (2.57s)
=== RUN   TestResourceToken_basic
--- PASS: TestResourceToken_basic (1.47s)
=== RUN   TestResourceToken_import
--- PASS: TestResourceToken_import (1.83s)
=== RUN   TestResourceToken_full
--- PASS: TestResourceToken_full (1.41s)
=== RUN   TestResourceToken_lookup
--- PASS: TestResourceToken_lookup (1.45s)
=== RUN   TestResourceToken_expire
--- PASS: TestResourceToken_expire (15.25s)
=== RUN   TestResourceToken_renew
--- PASS: TestResourceToken_renew (25.45s)
=== RUN   TestAccTransitCacheConfig
--- PASS: TestAccTransitCacheConfig (4.78s)
=== RUN   TestTransitSecretBackendKey_basic
--- PASS: TestTransitSecretBackendKey_basic (2.83s)
=== RUN   TestTransitSecretBackendKey_rsa4096
--- PASS: TestTransitSecretBackendKey_rsa4096 (3.54s)
=== RUN   TestTransitSecretBackendKey_import
--- PASS: TestTransitSecretBackendKey_import (1.82s)
=== RUN   TestExpandAuthMethodTune
--- PASS: TestExpandAuthMethodTune (0.00s)
=== RUN   TestFlattenAuthMethodTune
--- PASS: TestFlattenAuthMethodTune (0.00s)
=== RUN   TestValidateNoTrailingSlash
--- PASS: TestValidateNoTrailingSlash (0.00s)
PASS
ok      github.com/hashicorp/terraform-provider-vault/vault     543.495s

vault/data_identity_entity_test.go

tvoran · 2021-09-21T18:54:57Z

vault/gcp.go

+	"github.com/hashicorp/terraform-provider-vault/helper"
+
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"


I think usually local imports are after the others?

Suggested change

"github.com/hashicorp/terraform-provider-vault/helper"

"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"

"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"

"github.com/hashicorp/terraform-provider-vault/helper"

Not sure if this is canonical but goimports puts local imports first. Maybe just run goimports -w . at the root of the repo? And that should make everything consistent, it certainly looks like this PR inherited a lot of pre-existing inconsistency.

I am not sure what Goland uses, but it seems to group the local module's imports last.

Looks like it defaults to gofmt which groups the local packages last.

vault/resource_approle_auth_backend_role_secret_id_test.go

vault/resource_auth_backend_test.go

tvoran

Just a few more small comments. Have you also run the tests that require credentials and access to external systems? i.e. the database backends, anything that requires AWS creds, etc. Those tend to be skipped if the appropriate environment variables aren't set.

vault/resource_aws_secret_backend.go

vault/resource_github_auth_backend.go

vault/resource_jwt_auth_backend.go

vault/resource_token.go

tomhjp

LGTM pending some minor comments from Theron! 👍

go.mod

vault/provider_test.go

- go mod tidy - standardize imports

- make getJwtAuthBackendIfPresent() reusable by other callers

- use hashicorp/vault Docker repo.

- ci: move acceptance test environment to the test run

- drop extra environment debug

tvoran

😎

Just a thought that we'll probably want to manually run the enterprise-specific tests before releasing, since those aren't tested in CI right now.

benashz · 2021-09-29T13:34:08Z

😎

Just a thought that we'll probably want to manually run the enterprise-specific tests before releasing, since those aren't tested in CI right now.

👍 I tested locally against vault-enterprise and all tests passed.

calvn · 2021-09-30T23:45:55Z

Support for pgp_key encryption on the vault_token resource has been dropped. This is inline with upstream SDK changes.

@benashz Can you expand on this a bit? Is there a way to prevent accidental write of the raw token value to the state file?

calvn · 2021-09-30T23:49:10Z

vault/data_source_gcp_auth_backend_role.go

@@ -122,9 +122,9 @@ func gcpAuthBackendRoleRead(d *schema.ResourceData, meta interface{}) error {
 	log.Printf("[DEBUG] Read gcp auth backend role %q ID", path)

 	if resp == nil {
-		d.SetId("")


I see that there are other places where we explicitly call d.SetId("") before returning if the value (e.g. resp) that we're about to reference is nil. How come we don't have to in this case?

Good catch!

I did a bit of digging and this should actually be an error, since the role does exist. Looks like a discovered issue.

PR on its way!

benashz · 2021-10-01T13:45:48Z

Support for pgp_key encryption on the vault_token resource has been dropped. This is inline with upstream SDK changes.

@benashz Can you expand on this a bit? Is there a way to prevent accidental write of the raw token value to the state file?

Not that I am aware of. The TF team recommends persisting the state to encrypted storage. Adding a cautionary note for this resource might be a good mitigation?

Some options that come to mind are:

port the pgp crypto code from SDKv1
remove the resource completely
add a big note of caution to the resources documentation, pointing them to TF's guidance on securing the TF state.

github-actions bot added dependencies size/XL labels Sep 21, 2021

benashz requested review from tomhjp and tvoran September 21, 2021 16:31

tvoran reviewed Sep 21, 2021

View reviewed changes

benashz mentioned this pull request Sep 22, 2021

Updated: 'terraform-plugin-sdk' to 'v2' #1109

Closed

tvoran reviewed Sep 22, 2021

View reviewed changes

vault/resource_aws_secret_backend.go Outdated Show resolved Hide resolved

vault/resource_github_auth_backend.go Outdated Show resolved Hide resolved

vault/resource_jwt_auth_backend.go Show resolved Hide resolved

vault/resource_token.go Outdated Show resolved Hide resolved

tomhjp approved these changes Sep 24, 2021

View reviewed changes

go.mod Outdated Show resolved Hide resolved

vault/provider_test.go Show resolved Hide resolved

benashz added 15 commits September 28, 2021 09:32

fix: upgrade sdkv2, compilation

fe53d4f

fix: use sdk schema provider in tests

1c7bea1

fix: update to sdk v2.7.1

9cd5914

- go mod tidy - standardize imports

fix: update test config to HCL2

fe61d08

fix: resource_auth_backend: drop deprecated inputs

7056cb0

- make getJwtAuthBackendIfPresent() reusable by other callers

fix: further acceptance updates.

6e50160

fix: circleci: bump no_output_timeout to 20m

1c93724

fix: pin terraform version for tests

8e33266

- use hashicorp/vault Docker repo.

fix: bump go test timeout to 20m

a9c8de3

fix: ci: use make target for acceptance tests

3edf4a0

- ci: move acceptance test environment to the test run

fix: drop extraneous resource import alias

d2cdcdb

fix: remove empty if conditional blocks

672570c

fix: remove internal migration note on pgp crypto

b43cc4d

fix: ci: use terraform-v1.0.7

7e1c6f0

- drop extra environment debug

fix: upgrade to the lates TF plugin sdk v2.8.0

b3891ae

benashz force-pushed the VAULT-2189/tf-plugin-sdk-v2 branch from a5572a0 to b3891ae Compare September 28, 2021 13:33

benashz requested a review from tvoran September 28, 2021 17:05

vinay-gopalan mentioned this pull request Sep 28, 2021

Add provider tests and fix S3 secret issue #1173

Merged

tvoran approved these changes Sep 28, 2021

View reviewed changes

benashz merged commit e349937 into main Sep 29, 2021

benashz deleted the VAULT-2189/tf-plugin-sdk-v2 branch September 29, 2021 19:23

calvn reviewed Sep 30, 2021

View reviewed changes

benashz added this to the 3.0.0 milestone Oct 27, 2021

rgevaert mentioned this pull request Nov 19, 2021

unknown type <nil> for cidr_list in response for SecretID after upgrade to 1.9.0+ent hashicorp/vault#13226

Closed

tvoran mentioned this pull request Oct 31, 2022

Migrate to terraform-plugin-sdk/v2 #1104

Closed

fairclothjm mentioned this pull request May 18, 2023

auth_backend: remove some deprecated fields #777

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade to use TF SDK v2 #1175

Upgrade to use TF SDK v2 #1175

benashz commented Sep 21, 2021 •

edited

Loading

tvoran Sep 21, 2021 •

edited

Loading

tomhjp Sep 24, 2021

benashz Sep 27, 2021

benashz Sep 27, 2021

tvoran left a comment •

edited

Loading

tomhjp left a comment •

edited

Loading

tvoran left a comment

benashz commented Sep 29, 2021

calvn commented Sep 30, 2021

calvn Sep 30, 2021

benashz Oct 1, 2021 •

edited

Loading

benashz commented Oct 1, 2021

		"github.com/hashicorp/terraform-provider-vault/helper"

		"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"

Upgrade to use TF SDK v2 #1175

Upgrade to use TF SDK v2 #1175

Conversation

benashz commented Sep 21, 2021 • edited Loading

Community Note

tvoran Sep 21, 2021 • edited Loading

Choose a reason for hiding this comment

tomhjp Sep 24, 2021

Choose a reason for hiding this comment

benashz Sep 27, 2021

Choose a reason for hiding this comment

benashz Sep 27, 2021

Choose a reason for hiding this comment

tvoran left a comment • edited Loading

Choose a reason for hiding this comment

tomhjp left a comment • edited Loading

Choose a reason for hiding this comment

tvoran left a comment

Choose a reason for hiding this comment

benashz commented Sep 29, 2021

calvn commented Sep 30, 2021

calvn Sep 30, 2021

Choose a reason for hiding this comment

benashz Oct 1, 2021 • edited Loading

Choose a reason for hiding this comment

benashz commented Oct 1, 2021

benashz commented Sep 21, 2021 •

edited

Loading

tvoran Sep 21, 2021 •

edited

Loading

tvoran left a comment •

edited

Loading

tomhjp left a comment •

edited

Loading

benashz Oct 1, 2021 •

edited

Loading