Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

aws_secret_backend_role: support role_arns argument #407

Merged
merged 6 commits into from
Jun 3, 2019

Conversation

anatolebeuzon
Copy link
Contributor

This PR adds support in the provider for the role_arns parameter of AWS secret backend roles, added in Vault 0.11.

Copy link
Contributor

@tyrannosaurus-becks tyrannosaurus-becks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@oxlay thanks for working on this!

@@ -70,6 +70,16 @@ func awsSecretBackendRoleResource() *schema.Resource {
Required: true,
Description: "Role credential type.",
},
"role_arns": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do role_arns conflict with other fields in practice? The documentation doesn't say it does. Should we remove it from the conflicts with clauses?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • policy_document apparently does not conflict with role_arns (see test using both), so I've removed the ConflictsWith clause for this one
  • policy_arns and policy_arn are only valid when credential_type is iam_user, while role_arns is only valid when credential_type is assumed_role and prohibited otherwise (per the doc), so in practice, they are mutually exclusive
  • for policy, the doc says "cannot be mixed with the parameters listed above", hence the ConflictsWith clause

roleARNs = append(roleARNs, roleIfc.(string))
}

if policy == "" && len(policyARNs) == 0 && len(roleARNs) == 0 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure this check is needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you talking about the whole if { } or just the && len(roleARNs) == 0 part?

@tyrannosaurus-becks tyrannosaurus-becks self-assigned this May 7, 2019
@hExPY
Copy link

hExPY commented May 9, 2019

+1

@asksmruti
Copy link

Could you please make release out of this change? Desperately need this feature

@mgerlach
Copy link

+1

@simplyzee
Copy link

+1

@anatolebeuzon
Copy link
Contributor Author

@tyrannosaurus-becks any chance you can take a second look soon? 😄

Copy link
Contributor

@tyrannosaurus-becks tyrannosaurus-becks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Thank you!

@tyrannosaurus-becks tyrannosaurus-becks merged commit cc0cd7e into hashicorp:master Jun 3, 2019
dandandy pushed a commit to dandandy/terraform-provider-vault that referenced this pull request Jun 17, 2021
aws_secret_backend_role: support role_arns argument
@mdgreenfield mdgreenfield deleted the master branch December 29, 2022 23:42
@mdgreenfield mdgreenfield restored the master branch December 29, 2022 23:42
@mdgreenfield mdgreenfield deleted the master branch December 29, 2022 23:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants