Allow client_id to be configured on vault_identity_oidc_role resources #815

ianferguson · 2020-07-10T19:17:44Z

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

Allow client_id to be configured on vault_identity_oidc_role resources

Vault v1.4.0 added support for configuring the client_id of OIDC Identity Roles (via hashicorp/vault#8165)

This pull request adds support to the vault_identity_oidc_role resource for configuring the client_id parameter now supported by Vault v1.4.0+

Release note for CHANGELOG:

* resource/vault_identity_oidc_role: `client_id` parameter can optionally be configured ([#815](https://github.com/terraform-providers/terraform-provider-vault/pull/815)).

Output from acceptance testing:

existing integration test with no client_id set to help ensure backwards compatibility was maintained
integration test with a configured client_id was added
added client_id update to the existing integration test of updating the resource

> make testacc TESTARGS='-run TestAccIdentityOidcRole.*'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -run TestAccIdentityOidcRole.* -timeout 120m
?       github.com/terraform-providers/terraform-provider-vault [no test files]
?       github.com/terraform-providers/terraform-provider-vault/cmd/coverage    [no test files]
?       github.com/terraform-providers/terraform-provider-vault/cmd/generate    [no test files]
testing: warning: no tests to run
PASS
ok      github.com/terraform-providers/terraform-provider-vault/codegen 0.025s [no tests to run]
?       github.com/terraform-providers/terraform-provider-vault/generated       [no test files]
testing: warning: no tests to run
PASS
ok      github.com/terraform-providers/terraform-provider-vault/generated/datasources/transform/decode  0.037s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/terraform-providers/terraform-provider-vault/generated/datasources/transform/encode  0.037s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/terraform-providers/terraform-provider-vault/generated/resources/transform/alphabet  0.037s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/terraform-providers/terraform-provider-vault/generated/resources/transform/role      0.106s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/terraform-providers/terraform-provider-vault/generated/resources/transform/template  0.036s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/terraform-providers/terraform-provider-vault/generated/resources/transform/transformation    0.031s [no tests to run]
?       github.com/terraform-providers/terraform-provider-vault/schema  [no test files]
testing: warning: no tests to run
PASS
ok      github.com/terraform-providers/terraform-provider-vault/util    0.037s [no tests to run]
=== RUN   TestAccIdentityOidcRole
--- PASS: TestAccIdentityOidcRole (0.60s)
=== RUN   TestAccIdentityOidcRoleWithClientId
--- PASS: TestAccIdentityOidcRoleWithClientId (0.40s)
=== RUN   TestAccIdentityOidcRoleUpdate
--- PASS: TestAccIdentityOidcRoleUpdate (0.46s)
PASS
ok      github.com/terraform-providers/terraform-provider-vault/vault   1.490s

ianferguson · 2020-09-08T13:03:29Z

I know pull request reviews on this repository are done periodically, but was wondering if there was an estimate when the next round of reviews might be, and if that would include this PR?

ianferguson · 2020-10-21T16:47:53Z

@Valarissa would it be possible for this PR to be reviewed in the coming days or weeks?

ianferguson · 2020-11-19T20:38:24Z

@jasonodonnell would it be possible for this PR to be reviewed and hopefully included in a future release?

jasonodonnell · 2020-11-20T00:07:58Z

Hi @ianferguson! Yes, I will get this reviewed sometime next week.

jasonodonnell · 2020-11-23T17:35:20Z

vault/resource_identity_oidc_role_test.go

+resource "vault_identity_oidc_role" "role" {
+	name = "%s"
+	key = vault_identity_oidc_key.key.name
+    client_id = "%s"


Suggested change

client_id = "%s"

client_id = "%s"

vault/resource_identity_oidc_role_test.go

jasonodonnell

Minor formatting but LGTM!

ianferguson · 2020-11-30T19:42:58Z

@jasonodonnell thanks for the review! I've addressed the feedback, updated the changelog note to be in the now current release that is queued up and rebased on to the current master branch in df130a7,

Let me know if there is anything else I can do to help get this merged!

jasonodonnell · 2020-12-14T19:36:47Z

@ianferguson Can you remove the CHANGELOG edit? We'll update that once we merge this (you can see there's a conflict now).

Output of integration tests: ``` > pwd /Users/ian.ferguson/git/terraform-provider-vault/vault > env TF_ACC=true go test ./ -run 'TestAccIdentityOidcRole.*' -v === RUN TestAccIdentityOidcRole --- PASS: TestAccIdentityOidcRole (0.24s) === RUN TestAccIdentityOidcRoleWithClientId --- PASS: TestAccIdentityOidcRoleWithClientId (0.24s) === RUN TestAccIdentityOidcRoleUpdate --- PASS: TestAccIdentityOidcRoleUpdate (0.40s) PASS ok github.com/terraform-providers/terraform-provider-vault/vault (cached) ```

ianferguson · 2020-12-14T20:18:59Z

@jasonodonnell done in 71ad7f3

jasonodonnell · 2020-12-14T20:30:21Z

Thanks @ianferguson!

hashicorp#815) Output of integration tests: ``` > pwd /Users/ian.ferguson/git/terraform-provider-vault/vault > env TF_ACC=true go test ./ -run 'TestAccIdentityOidcRole.*' -v === RUN TestAccIdentityOidcRole --- PASS: TestAccIdentityOidcRole (0.24s) === RUN TestAccIdentityOidcRoleWithClientId --- PASS: TestAccIdentityOidcRoleWithClientId (0.24s) === RUN TestAccIdentityOidcRoleUpdate --- PASS: TestAccIdentityOidcRoleUpdate (0.40s) PASS ok github.com/terraform-providers/terraform-provider-vault/vault (cached) ```

ghost added size/M documentation labels Jul 10, 2020

ianferguson force-pushed the ianferguson/support_client_id_param_on_vault_identity_oidc_role branch from 62a1dc6 to e384c83 Compare July 10, 2020 19:18

jasonodonnell self-requested a review November 20, 2020 00:08

jasonodonnell added this to the 2.16.1 milestone Nov 20, 2020

jasonodonnell reviewed Nov 23, 2020

View reviewed changes

vault/resource_identity_oidc_role_test.go Outdated Show resolved Hide resolved

jasonodonnell self-requested a review November 23, 2020 17:38

jasonodonnell approved these changes Nov 23, 2020

View reviewed changes

ianferguson force-pushed the ianferguson/support_client_id_param_on_vault_identity_oidc_role branch from e384c83 to df130a7 Compare November 30, 2020 19:38

ianferguson force-pushed the ianferguson/support_client_id_param_on_vault_identity_oidc_role branch from df130a7 to 71ad7f3 Compare December 14, 2020 20:18

jasonodonnell merged commit c48f526 into hashicorp:master Dec 14, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow client_id to be configured on vault_identity_oidc_role resources #815

Allow client_id to be configured on vault_identity_oidc_role resources #815

ianferguson commented Jul 10, 2020 •

edited

Loading

ianferguson commented Sep 8, 2020

ianferguson commented Oct 21, 2020

ianferguson commented Nov 19, 2020

jasonodonnell commented Nov 20, 2020

jasonodonnell Nov 23, 2020

jasonodonnell left a comment

ianferguson commented Nov 30, 2020

jasonodonnell commented Dec 14, 2020

ianferguson commented Dec 14, 2020

jasonodonnell commented Dec 14, 2020

Allow client_id to be configured on vault_identity_oidc_role resources #815

Allow client_id to be configured on vault_identity_oidc_role resources #815

Conversation

ianferguson commented Jul 10, 2020 • edited Loading

Community Note