Fix `bound_claims` not getting unset for `vault_jwt_auth_backend_role` #1469

lawliet89 · 2022-05-30T11:19:52Z

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

When bound_claims in vault_jwt_auth_backend_role is unset, it does not get removed from the role, resulting in a permanent diff and unexpected results.

Release note for CHANGELOG:

Fix `bound_claims` not getting removed for `vault_jwt_auth_backend_role`

Output from acceptance testing:

$ $ VAULT_ADDR=http://127.0.0.1:8200 VAULT_TOKEN=12345 TESTARGS="--run TestAccJWTAuthBackendRole" make testacc
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test -v --run TestAccJWTAuthBackendRole -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]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-vault/codegen   (cached) [no tests to run]
?       github.com/hashicorp/terraform-provider-vault/generated [no test files]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-vault/generated/datasources/transform/decode    (cached) [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-vault/generated/datasources/transform/encode    (cached) [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-vault/generated/resources/transform/alphabet    (cached) [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-vault/generated/resources/transform/role        (cached) [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-vault/generated/resources/transform/template    (cached) [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-vault/generated/resources/transform/transformation      (cached) [no tests to run]
?       github.com/hashicorp/terraform-provider-vault/helper    [no test files]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-vault/internal/identity/entity  (cached) [no tests to run]
?       github.com/hashicorp/terraform-provider-vault/schema    [no test files]
?       github.com/hashicorp/terraform-provider-vault/testutil  [no test files]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-vault/util      (cached) [no tests to run]
=== RUN   TestAccJWTAuthBackendRole_import
--- PASS: TestAccJWTAuthBackendRole_import (0.66s)
=== RUN   TestAccJWTAuthBackendRole_basic
--- PASS: TestAccJWTAuthBackendRole_basic (0.55s)
=== RUN   TestAccJWTAuthBackendRole_update
--- PASS: TestAccJWTAuthBackendRole_update (0.96s)
=== RUN   TestAccJWTAuthBackendRole_full
--- PASS: TestAccJWTAuthBackendRole_full (0.70s)
=== RUN   TestAccJWTAuthBackendRoleOIDC_full
--- PASS: TestAccJWTAuthBackendRoleOIDC_full (1.19s)
=== RUN   TestAccJWTAuthBackendRoleOIDC_disableParsing
--- PASS: TestAccJWTAuthBackendRoleOIDC_disableParsing (1.19s)
=== RUN   TestAccJWTAuthBackendRole_fullUpdate
--- PASS: TestAccJWTAuthBackendRole_fullUpdate (1.77s)
PASS
ok      github.com/hashicorp/terraform-provider-vault/vault     7.042s

If the else clause I added in the resource is removed, the test above fails with

=== RUN   TestAccJWTAuthBackendRole_fullUpdate
    resource_jwt_auth_backend_role_test.go:371: Step 3/3 error: Check failed: 1 error occurred:
                * Check 23/23 error: vault_jwt_auth_backend_role.role: Attribute 'bound_claims.%' expected "0", got "2"

thereby confirming the bug.

benashz · 2022-05-30T16:32:02Z

vault/resource_jwt_auth_backend_role.go

@@ -406,6 +406,8 @@ func jwtAuthBackendRoleDataToWrite(d *schema.ResourceData, create bool) map[stri
 			boundClaims[key] = claims
 		}
 		data["bound_claims"] = boundClaims
+	} else {
+		data["bound_claims"] = make(map[string]interface{}, 0)


The empty map is defined on line 396 above. If we move that before the conditional we can drop the else{} and always set bound_claims

I've done this.

benashz · 2022-05-30T16:36:27Z

vault/resource_jwt_auth_backend_role_test.go

@@ -477,6 +479,58 @@ func TestAccJWTAuthBackendRole_fullUpdate(t *testing.T) {
 						"verbose_oidc_logging", "false"),
 				),
 			},
+			// Repeat test case again to remove attributes like `bound_claims`


Would you mind moving these test check funcs to a common set that can be shared between the various steps. Please use

terraform-provider-vault/vault/resource_pki_secret_backend_sign_test.go

Line 190 in e3b6f46

Check: resource.ComposeTestCheckFunc(

as an example.

The resource name/path should also be factored out to a test variable called resourceName, e.g.

terraform-provider-vault/vault/resource_pki_secret_backend_role_test.go

Line 18 in e3b6f46

resourceName := "vault_pki_secret_backend_role.test"

Thanks

I reduced a bit of code repetition but I did not refactor out the test steps into a separate function yet. This seems like more of a refactoring effort that might make the size of this PR bigger than necessary.

Ok, we will take care of that after merge. Thanks!

benashz

Thank for your contribution to HashiCorp!

Fix bound_claims not getting removed for vault_jwt_auth_backend_role

7e1ebb7

github-actions bot added the size/S label May 30, 2022

lawliet89 changed the title ~~Fix bound_claims not getting removed for vault_jwt_auth_backend_role~~ Fix bound_claims not getting unset for vault_jwt_auth_backend_role May 30, 2022

benashz reviewed May 30, 2022

View reviewed changes

benashz added this to the 3.7.0 milestone May 30, 2022

benashz added the bug label May 30, 2022

lawliet89 added 2 commits May 31, 2022 10:25

Refactor empty map creation

49ef663

Remove repeated test code

c9b3b38

github-actions bot added size/M and removed size/S labels May 31, 2022

benashz approved these changes May 31, 2022

View reviewed changes

benashz merged commit 3833607 into hashicorp:main May 31, 2022

lawliet89 deleted the rm-boundclaims branch June 1, 2022 12:09

marcboudreau pushed a commit to marcboudreau/terraform-provider-vault that referenced this pull request Nov 6, 2022

Support unsetting jwt_auth_backend_role.bound_claims (hashicorp#1469)

a4eb8b0

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix `bound_claims` not getting unset for `vault_jwt_auth_backend_role` #1469

Fix `bound_claims` not getting unset for `vault_jwt_auth_backend_role` #1469

lawliet89 commented May 30, 2022 •

edited

Loading

benashz May 30, 2022

lawliet89 May 31, 2022

benashz May 30, 2022 •

edited

Loading

lawliet89 May 31, 2022 •

edited

Loading

benashz May 31, 2022

benashz left a comment

Fix bound_claims not getting unset for vault_jwt_auth_backend_role #1469

Fix bound_claims not getting unset for vault_jwt_auth_backend_role #1469

Conversation

lawliet89 commented May 30, 2022 • edited Loading

Community Note

benashz May 30, 2022

Choose a reason for hiding this comment

lawliet89 May 31, 2022

Choose a reason for hiding this comment

benashz May 30, 2022 • edited Loading

Choose a reason for hiding this comment

lawliet89 May 31, 2022 • edited Loading

Choose a reason for hiding this comment

benashz May 31, 2022

Choose a reason for hiding this comment

benashz left a comment

Choose a reason for hiding this comment

Fix `bound_claims` not getting unset for `vault_jwt_auth_backend_role` #1469

Fix `bound_claims` not getting unset for `vault_jwt_auth_backend_role` #1469

lawliet89 commented May 30, 2022 •

edited

Loading

benashz May 30, 2022 •

edited

Loading

lawliet89 May 31, 2022 •

edited

Loading