EC2 constraints for vault_aws_auth_backend_role should be lists

[gregsymons]

Hi there,

Thank you for opening an issue. Please note that we try to keep the Terraform issue tracker reserved for bug reports and feature requests. For general usage questions, please see: https://www.terraform.io/community.html.

Terraform Version

0.11.4

Affected Resource(s)

Please list the resources as a list, for example:

• vault_aws_auth_backend_role

If this issue appears to affect multiple resources, it may be an issue with Terraform's core, so please mention this.

Terraform Configuration Files

resource "vault_aws_auth_backend_role" "system" {
  role = "system"
  auth_type = "ec2"
  bound_ami_id = [ "${concat(data.aws_ami_ids.released_amis.ids, data.aws_ami_ids.testing_amis.ids)}" ]
  policies = "system"
}

Expected Behavior

The list of AMI ids should be applied as constraints to the new role.

Actual Behavior

Planning fails:

Acquiring state lock. This may take a few moments...
Releasing state lock. This may take a few moments...

Error: vault_aws_auth_backend_role.system: bound_ami_id must be a single value, not a list

Steps to Reproduce

Please list the steps required to reproduce the issue, for example:

1. terraform plan

Important Factoids

All of the EC2 auth constraints for the AWS auth background take lists, according to the api docs. But looking at the source (and the TF docs) they're all implemented as strings for Terraform.

[gregsymons]

Turns out you can workaround it by specifying a JSON array for the string, but that seems awkward.

[paddycarver]

If I'm reading this right, the ability to use a list instead of a single item was added in hashicorp/vault#3907, which was authored three months after the resource was, which at least explains the discrepancy. 😄

If I'm reading this right, the list feature would only be available in Vault 0.9.6, which is (at the time of this writing) the latest non-RC Vault release.

My personal 2¢ on the topic is that we have three options:

A) leave it as it is. It's definitely ergonomically weird, but there is a sensible workaround. Perhaps we could document it.

B) switch it to a list (or, more likely, a set). This would afford us better ergonomics, but would be awful strange for anyone on Vault 0.9.5 or below, who can't use lists, and can only use strings. So they'd essentially have to set up a list (or set) containing a single item, which is ergonomically weird for them. This would also be a breaking change, which makes it an awful hard sell, because anyone using the resource would have to rewrite their config (minorly, but still) or not upgrade the provider. Which isn't ideal.

C) Add a new plural version of the fields that accept sets. This will make the code more complicated, and may be a bit confusing (having bound_ami_id and bound_ami_ids and understanding why), but would arguably let us have the appropriate ergonomics for each.

At the moment, I'd say since A has a suitable workaround, while it's not ideal, it's an acceptable solution for me. B would be a hard sell. C I don't consider compelling enough to write the code myself for it, but I'm sure a PR would be considered.

[gregsymons]

Now that's interesting, since I was hitting a Vault 8.3 server, and it accepted the list with no complaint. I haven't actually tried to use the role yet, since I haven't written the thing that uses it. I'll have to take a closer look at it.

Also, reading the api docs more closely, the value can be either a comma-separated string or a JSON array. Given that, I suppose it makes the ergonomics issue for A less important. I also don't really think option C is worth it. If you want to close this issue, I wouldn't object.

[joelthompson]

@paddycarver -- yep, that was the PR. As the culprit behind this breaking change in Vault... apologies for the confusion here. It's a delicate balance.

Regarding A, the one thing that I'm not sure how Terraform will handle is the Vault API will always return a JSON list. So, you can supply a string of comma-separated values on the input, but upon reading the role, you'll get a JSON list of strings as the output. Might that cause TF to think that the resource has drifted and try to reconverge it?

Honestly, it would be nice if TF's typing system wasn't so strict and would allow users to specify as either a list or a string, but I don't know if that has been discussed before.

Also, @gregsymons -- not sure how precisely 0.8.3 would have handled a list input, but I doubt that using the role would have given you what you were looking for.

If I'm reading this right, the list feature would only be available in Vault 0.9.6, which is (at the time of this writing) the latest non-RC Vault release.

0.10 was released on Tuesday :)

[paddycarver]

Regarding A, the one thing that I'm not sure how Terraform will handle is the Vault API will always return a JSON list. So, you can supply a string of comma-separated values on the input, but upon reading the role, you'll get a JSON list of strings as the output. Might that cause TF to think that the resource has drifted and try to reconverge it?

At the moment, I believe the Terraform provider will think there's a permanent diff, because it will try to set a list in a field that expects a string, which will silently fail. I can fix this with minimal code in a backwards compatible way to just always serialize the API response down to a comma-separated list of IDs.

Honestly, it would be nice if TF's typing system wasn't so strict and would allow users to specify as either a list or a string, but I don't know if that has been discussed before.

My understanding is Terraform's typing system is going to stay strongly-typed, and if at all possible, backwards compatibility for which types are returned from the API is super helpful. Other API clients have strong typing needs, too, I imagine--after all, this would have broken any Go code I had written against the API.

[Lngramos]

Turns out you can workaround it by specifying a JSON array for the string, but that seems awkward.

Hi @gregsymons, would you mind sharing an example of JSON array workaround solution please? I haven't managed to get something like that working yet.

[tyrannosaurus-becks]

This is supported now! https://github.com/terraform-providers/terraform-provider-vault/blob/master/vault/resource_aws_auth_backend_role.go#L50-L51