Fix resource_mongodbatlas_secret_role missing roles

[jgrumboe]

Description

This PR fixes a problem where vault_mongodbatlas_secret_role is not sending the roles attribute at every API call for Vault`s mongodbatlas roles API endpoint.

Closes #2025

Checklist

• Added CHANGELOG entry (only for user-facing changes)
• Acceptance tests where run against all supported Vault Versions

Output from acceptance testing:

❯ make testacc TESTARGS='-run=TestAccMongoDBAtlasSecretRole_basic'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test -run=TestAccMongoDBAtlasSecretRole_basic -timeout 30m ./...
?       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]
?       github.com/hashicorp/terraform-provider-vault/generated [no test files]
ok      github.com/hashicorp/terraform-provider-vault/codegen   0.569s [no tests to run]
?       github.com/hashicorp/terraform-provider-vault/helper    [no test files]
?       github.com/hashicorp/terraform-provider-vault/internal/consts   [no test files]
?       github.com/hashicorp/terraform-provider-vault/internal/identity/group   [no test files]
?       github.com/hashicorp/terraform-provider-vault/internal/identity/mfa     [no test files]
?       github.com/hashicorp/terraform-provider-vault/internal/pki      [no test files]
ok      github.com/hashicorp/terraform-provider-vault/generated/datasources/transform/decode    0.556s [no tests to run]
ok      github.com/hashicorp/terraform-provider-vault/generated/datasources/transform/encode    1.052s [no tests to run]
ok      github.com/hashicorp/terraform-provider-vault/generated/resources/transform/alphabet    1.407s [no tests to run]
?       github.com/hashicorp/terraform-provider-vault/schema    [no test files]
ok      github.com/hashicorp/terraform-provider-vault/generated/resources/transform/role        1.782s [no tests to run]
ok      github.com/hashicorp/terraform-provider-vault/generated/resources/transform/template    2.174s [no tests to run]
ok      github.com/hashicorp/terraform-provider-vault/generated/resources/transform/transformation      2.500s [no tests to run]
ok      github.com/hashicorp/terraform-provider-vault/internal/identity/entity  2.809s [no tests to run]
ok      github.com/hashicorp/terraform-provider-vault/internal/provider 1.715s [no tests to run]
ok      github.com/hashicorp/terraform-provider-vault/testutil  1.936s [no tests to run]
ok      github.com/hashicorp/terraform-provider-vault/util      2.196s [no tests to run]
ok      github.com/hashicorp/terraform-provider-vault/vault     5.625s

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

[fairclothjm]

@jgrumboe Thanks for the contribution! Could you please make the following updates:

• use d.GetOk in favor of d.HasChange as suggested
• add a changelog entry
• add a test that captures the failures scenario we are addressing in this PR

[jgrumboe]

@fairclothjm sure, I will look it this.

I have a question regarding the test: the problem we want to test against is to check the payload for the Vault API request. I've looked at other tests and just saw testing results. How can I check the payload of the request?

[fairclothjm]

@jgrumboe Thanks, I will try to shed some light here. Currently we have 3 TestSteps that cover vault_mongodbatlas_secret_role in TestAccMongoDBAtlasSecretRole_basic. To test this behavior we can add another TestStep that is identical to the first TestStep

terraform-provider-vault/vault/resource_mongodbatlas_secret_role_test.go

Lines 44 to 63 in b98c875

	{
	Config: testAccMongoDBAtlasSecretBackendRole_basic(mount, name, false),
	Check: resource.ComposeTestCheckFunc(
	resource.TestCheckResourceAttr(resourceName, consts.FieldMount, mount),
	resource.TestCheckResourceAttr(resourceName, consts.FieldName, name),
	resource.TestCheckResourceAttr(resourceName, consts.FieldOrganizationID, organizationID),
	resource.TestCheckResourceAttr(resourceName, consts.FieldProjectID, projectID),
	resource.TestCheckResourceAttr(resourceName, consts.FieldRoles+".#", "1"),
	resource.TestCheckResourceAttr(resourceName, consts.FieldRoles+".0", roles),
	resource.TestCheckResourceAttr(resourceName, consts.FieldIPAddresses+".#", "2"),
	resource.TestCheckResourceAttr(resourceName, consts.FieldIPAddresses+".0", "192.168.1.3"),
	resource.TestCheckResourceAttr(resourceName, consts.FieldIPAddresses+".1", "192.168.1.4"),
	resource.TestCheckResourceAttr(resourceName, consts.FieldCIDRBlocks+".#", "1"),
	resource.TestCheckResourceAttr(resourceName, consts.FieldCIDRBlocks+".0", cidrBlocks),
	resource.TestCheckResourceAttr(resourceName, consts.FieldProjectRoles+".#", "1"),
	resource.TestCheckResourceAttr(resourceName, consts.FieldProjectRoles+".0", projectRoles),
	resource.TestCheckResourceAttr(resourceName, consts.FieldTTL, ttl),
	resource.TestCheckResourceAttr(resourceName, consts.FieldMaxTTL, maxTtl),
	),
	},

except that we update the TTL, for example. So in TestStep 1, we will be testing that we can apply a configuration. In TestStep 2 we will be asserting that we can apply that same configuration except that we are updating some field other than the role.

There is no need to check the payload for the Vault API request. When TF applies a configuration it will write to Vault, then immediately read back the data from the Vault API, set the TF state to what was read from the Vault API, and finally our test will assert that the TF state is what we expect.

Hope that helps!

[jgrumboe]

Yes, that helps!
The problem was that updating attributes except roles would result in an incomplete API request. So, I will add a step in between updating fields but keep roles the same; with the old logic, it fails, but with the fix, it's working. As a last test step, we also update roles.

[jgrumboe]

@fairclothjm I've splitted the original test function (with isUpdate condition) into multiple for better readability.

[fairclothjm]

LGTM! Thanks @jgrumboe !