Change credential_types output to credential_type #5975
Conversation
|
@joelthompson does this seem like the right thing to do? |
| "policy_document": r.PolicyDocument, | ||
| "default_sts_ttl": int64(r.DefaultSTSTTL.Seconds()), | ||
| "max_sts_ttl": int64(r.MaxSTSTTL.Seconds()), | ||
| "credential_type": strings.Join(r.CredentialTypes, ","), |
There was a problem hiding this comment.
Should it also be wrapped with square brackets if the length is over 1? I ask because of this example in the issue: "credential_types": ["assumed_role"].
There was a problem hiding this comment.
Note too, that the role_arns is an array type as well and should just be role_arn. I'd also assume that when you set role_arn that it conflicts with policy_arn and policy.
There was a problem hiding this comment.
@tyrannosaurus-becks The brackets are just the way it's being displayed. They're caused by the fact that it's coming back as an array even though the input is (now) a single string. The only reason it would be multiple values at this point is legacy roles, so this is a way to still allow legacy roles to display possibly more than one type while making the output the same type as the input.
@jasonmcintosh I'd also assume that when you set role_arn that it conflicts with policy_arn and policy. -- can you explain?
There was a problem hiding this comment.
so if you set role_arn, that should be a conflicts with policy_document values. As I believe you don't want to try and modify a role.... just trying to think of the combinations and how they interact on this.
There was a problem hiding this comment.
@jasonmcintosh I'm not really following. Why should role_arns be role_arn? The logic allows for multiple values.
There was a problem hiding this comment.
@jefferai Not saying role_arns should be role_arn - saying role_arns should conflict with policy_arn and policy - as you can't have both a role_arn you're explicitly providing as well as a dynamically generated account.
Will see if this solves one of my headaches using terraform resource vault_generic_secret to create various vault roles. Still need to get the vault provider in terraform to work with this as well. https://github.com/terraform-providers/terraform-provider-vault/blob/master/vault/resource_aws_secret_backend_role.go . currently doesn't support role_arns :(
There was a problem hiding this comment.
Hey @jasonmcintosh, agree that role_arns and policy_arns don't make sense together. The role creation code, I believe, prohibits creating roles with both values.
role_arns and policy_document are compatible -- the latter will scope down the the permissions granted to a role being assumed.
Is there something I'm missing in what you're saying?
| "policy_document": r.PolicyDocument, | ||
| "default_sts_ttl": int64(r.DefaultSTSTTL.Seconds()), | ||
| "max_sts_ttl": int64(r.MaxSTSTTL.Seconds()), | ||
| "credential_type": strings.Join(r.CredentialTypes, ","), |
There was a problem hiding this comment.
Hey @jasonmcintosh, agree that role_arns and policy_arns don't make sense together. The role creation code, I believe, prohibits creating roles with both values.
role_arns and policy_document are compatible -- the latter will scope down the the permissions granted to a role being assumed.
Is there something I'm missing in what you're saying?
|
If code operates that way sounds good to me! I’ll check this out do some tests. May work on an integration test project to validate current provider against current vault. |
Fixes #5972