Add provider tests and fix S3 secret issue

[mbillow]

There was an attribute left out of my initial merge request (#1139) that is preventing the complete setup of resources with those changes. Fixing that error and adding tests to catch similar issues in the future.

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

Relates #1139

Release note for CHANGELOG:

BUGS:
- `resource/vault_raft_snapshot_agent_config`: Fixed issue where AWS Secret Access Key was ignored.

Output from acceptance testing:

$ TESTARGS="--run RaftSnapshotAgentConfig" TF_ACC_ENTERPRISE="true" make testacc
...
=== RUN   TestAccRaftSnapshotAgentConfig_basic
--- PASS: TestAccRaftSnapshotAgentConfig_basic (1.46s)
=== RUN   TestAccRaftSnapshotAgentConfig_import
--- PASS: TestAccRaftSnapshotAgentConfig_import (0.48s)
PASS

[mbillow]

@vinay-gopalan Hey! 😄 If you could take a quick look at this, it is just a field I forgot in my initial PR. I added some tests to make sure this doesn't happen again.

[mbillow]

This is spooky and should probably be looked into on the Vault side. When this string isn't set, Vault returns false instead of null or an empty string like the other raft snapshot storage providers. Handling it here in the provider for now though.

[mbillow]

I mainly want to confirm whether the google_endpoint returned as false fix is necessary.

~/docker/vault-enterprise-raft
➜ VAULT_ADDR="http://localhost:28200" vault write sys/storage/raft/snapshot-auto/config/example interval=1 retain=1 path_prefix=demo storage_type=google-gcs google_gcs_bucket=demo
Success! Data written to: sys/storage/raft/snapshot-auto/config/example

~/docker/vault-enterprise-raft
➜ VAULT_ADDR="http://localhost:28200" vault read sys/storage/raft/snapshot-auto/config/example
Key                           Value
---                           -----
file_prefix                   vault-snapshot
google_disable_tls            false
google_endpoint               false
google_gcs_bucket             demo
google_service_account_key    n/a
interval                      1
path_prefix                   demo
retain                        1
storage_type                  google-gcs

~/docker/vault-enterprise-raft
➜ VAULT_ADDR="http://localhost:28200" vault write sys/storage/raft/snapshot-auto/config/example2 interval=1 retain=1 path_prefix=demo storage_type=aws-s3 aws_s3_bucket=demo aws_s3_region=us-east-1
Success! Data written to: sys/storage/raft/snapshot-auto/config/example2

~/docker/vault-enterprise-raft
➜ VAULT_ADDR="http://localhost:28200" vault read sys/storage/raft/snapshot-auto/config/example2
Key                              Value
---                              -----
aws_access_key_id                n/a
aws_s3_bucket                    demo
aws_s3_disable_tls               false
aws_s3_enable_kms                false
aws_s3_endpoint                  n/a
aws_s3_force_path_style          false
aws_s3_kms_key                   n/a
aws_s3_region                    us-east-1
aws_s3_server_side_encryption    false
aws_secret_access_key            n/a
aws_session_token                n/a
file_prefix                      vault-snapshot
interval                         1
path_prefix                      demo
retain                           1
storage_type                     aws-s3

You'll notice that google_endpoint is false when unset, but aws_s3_endpoint is null when unset. According to the docs, it is supposed to be an optional value of type string.... so false is definitely not right.

[vinay-gopalan]

I mainly want to confirm whether the google_endpoint returned as false fix is necessary.

~/docker/vault-enterprise-raft
➜ VAULT_ADDR="http://localhost:28200" vault write sys/storage/raft/snapshot-auto/config/example interval=1 retain=1 path_prefix=demo storage_type=google-gcs google_gcs_bucket=demo
Success! Data written to: sys/storage/raft/snapshot-auto/config/example

~/docker/vault-enterprise-raft
➜ VAULT_ADDR="http://localhost:28200" vault read sys/storage/raft/snapshot-auto/config/example
Key                           Value
---                           -----
file_prefix                   vault-snapshot
google_disable_tls            false
google_endpoint               false
google_gcs_bucket             demo
google_service_account_key    n/a
interval                      1
path_prefix                   demo
retain                        1
storage_type                  google-gcs

~/docker/vault-enterprise-raft
➜ VAULT_ADDR="http://localhost:28200" vault write sys/storage/raft/snapshot-auto/config/example2 interval=1 retain=1 path_prefix=demo storage_type=aws-s3 aws_s3_bucket=demo aws_s3_region=us-east-1
Success! Data written to: sys/storage/raft/snapshot-auto/config/example2

~/docker/vault-enterprise-raft
➜ VAULT_ADDR="http://localhost:28200" vault read sys/storage/raft/snapshot-auto/config/example2
Key                              Value
---                              -----
aws_access_key_id                n/a
aws_s3_bucket                    demo
aws_s3_disable_tls               false
aws_s3_enable_kms                false
aws_s3_endpoint                  n/a
aws_s3_force_path_style          false
aws_s3_kms_key                   n/a
aws_s3_region                    us-east-1
aws_s3_server_side_encryption    false
aws_secret_access_key            n/a
aws_session_token                n/a
file_prefix                      vault-snapshot
interval                         1
path_prefix                      demo
retain                           1
storage_type                     aws-s3

You'll notice that google_endpoint is false when unset, but aws_s3_endpoint is null when unset. According to the docs, it is supposed to be an optional value of type string.... so false is definitely not right.

Yup! Sorry I was under the impression it was sending non-null values for all requests. I confirmed with the google_gcs storage type what you're seeing (I was initially working with local), and will open an issue on the Vault side accordingly. Your current fix for the TF Vault Provider looks good however. Thanks for bringing this up and apologies again for missing these fields in the first review! 🙇

[vinay-gopalan]

LGTM 👍

[vinay-gopalan]

We're holding off on merging things until this PR to upgrade to TF SDK v2 goes in #1175

Will merge your PR once that goes through! Thanks :)

[mbillow]

Thanks for bringing this up and apologies again for missing these fields in the first review!

I should have had tests for the cloud providers from the start. Feel free to pin this one squarely on my laziness. 😛

We're holding off on merging things until this PR to upgrade to TF SDK v2 goes in #1175

I can barely hold in my excitement for the day running TF in debug mode doesn't output 50 SDK deprecation warnings per resource. 😉

Thanks again for reviewing.

[benashz]

@mbillow this fix will go out in the v2.24.1 release tomorrow.