-
Notifications
You must be signed in to change notification settings - Fork 9.6k
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
Don't silently convert object types to map(string) just because their attributes all have primitive types #34727
Comments
Hi @jaffel-lc, I think in this case the problem is that Terraform is trying its best to find some way to make your conditional expression valid through automatic type conversions: key_config = var.kms_key == "AWSOwnedKey" ? local.kms_key_config["AWSOwnedKey"] : local.kms_key_config["CustomerKey"] Terraform needs to decide what type
Without any type conversions this conditional expression would be invalid because the two results are of different types. Terraform is noticing that it's possible to convert a boolean value to a string and therefore both of these can potentially convert to I assume from your description that what you really wanted is for this local value to have the type kms_key_config = tomap({
AWSOwnedKey = {
AWSOwnedKey = true
KmsARN = null
}
CustomerKey = {
AWSOwnedKey = false
KmsARN = var.kms_key
}
}) This will cause This is, then, Terraform behaving as designed, rather than a bug. This is an unfortunate situation where Terraform would ideally have returned a clear error about the type mismatch, but cannot do so because of backward compatibility with Terraform v0.11 and earlier, which didn't yet have either the To avoid breaking existing modules that were relying on the ability to construct maps in this way, modern Terraform unfortunately tends to guess that authors intended to construct a No new editions are planned at this time, but I'm going to relabel this as an enhancement and add it to our list of candidates for potential changes to make in a future language edition. (A new language edition is a relatively expensive proposition, so we're intending to wait until there are enough candidates to justify the additional complexity that would imply.) |
Your reply make a lot of sense. Thank you. However, I had already found that when AWSOwnedKey is false, passing the KmsARN key - set to anything - caused a key error on KmsARN when amazon tries to verify the resulting policy JSON. AWS doesn't want a KmsARN key with any value - null or not - when AWSOwnedKey == true So what I wanted was exactly the assignment I wrote, so that my code would contain the three lines starting with the key_config assignment, instead of the four lines that follow it. I guess, that I would call it a misfeature, if not a bug, that terraform doesn't create the resulting variables type verbatim form the right side of the assignment; or at least track that the value was bool, and not string - maintaining the original bool type [with an indicator bit in your symbol table], and not converting it to string. Because just prior to opening this issue, I opened this issue about TF not being able to use string comparisons on variables in count operations. So I would expect
to generate a plan-time error, rendering string-encoded boolean values useless. |
Indeed, when you are constructing something that's going to be sent essentially verbatim as JSON (or similar) then you may need to postprocess it to transform it from the conventions of Terraform's type system to the conventions of whatever language you are targeting. That's a common problem when using types from one language to generate data for another language. This is often managed in Terraform by providers offering functionality to automatically transform a Terraform-friendly data structure into an equivalent that's friendly to a particular remote system. However, that's been tough to design for a number of features due to the functionality typically needing to get embedded inside the implementation of a particular resource type. Terraform v1.8 is going to introduce the possibility of providers to contribute their own functions (in the same sense that Of course it will take some time for the provider ecosystem to make full use of this new capability, but what you've described here seems like a good use-case for a function in the AWS provider which knows the JSON schema that AWS is expecting in this context and produces it automatically. |
Indeed, I consider the map manipulation under debate here to be pre-processing. What is at issue is TF performing implicit type conversion of a bool literal to a string literal, and that terraform should be smart enough to tell the difference between "true" and true, especially when "true" appears nowhere in my code at all. If I'd mixed them freely, I could maybe understand it in light of the promise you mentioned above. Since HCL is dynamically-typed language , instead of getting confused about the what type to use on the left side of the assignment before evaluating the left side of the ternary operator, clone the result type from the results after the right side of the assignment is fully evaluated. |
The kind of pre-processing I meant was something that should happen after you've built a data structure in Terraform's type system. By analogy to Go: it's typical to first build a data structure made mostly of struct types and then map that to correctly-shaped JSON as a separate step. Terraform object types are analogous to struct types, and so essentially the same principle applies here: you'd start by building a data structure made of Terraform object types (which come with the assumption that all attributes are always present but sometimes they are Trying to do both the building of the data structure and the adaptation for another system's encoding format at the same time makes things considerably more difficult, because you end up needing to work around Terraform's expression operators, rather than working with them. I'm not sure exactly which AWS-specific format you are trying to generate here -- I assume it's not just an IAM policy document -- but the existing |
These are aws_opensearchserverless_* policies. I have to write my own opensearchserverless module, because I haven't been able to find one by Hashicorp in the module registry. I am not asking terraform to be aware that I wanted to convert to JSON after my lookup, or to more generally do "the adaptation for another system's encoding format"; I just wanted the bool literal in the original map to survive the lookup and assignment as a bool literal, so I could create the other system's encoding format. |
Thanks for that extra context! To be clear, I'm just trying to give you context about why Terraform behaves the way it does and why I perceive it as an example of translating between type systems rather than as a bug, since your earlier feedback seemed to be implicitly asking about that. Terraform perhaps obscures this problem more than some other languages due to it having a structural type system that is built around "JSON-like" concepts, but it's important to remember that Terraform's type system and JSON's type system (such that it is) are not exactly equivalent, and so there will be some cases where you can't "just" write exactly what you would write in JSON. My understanding of the state of this issue is that it represents the fact that Terraform gave you poor feedback about what was going on here, despite it being intended behavior: it guessed incorrectly about what you wanted your expression to mean, rather than returning a type mismatch error as I think it ideally should have. This issue is therefore representing hopefully finding some way to improve that, which might have to wait until a future edition of the Terraform language. My other remarks about provider-contributed functions are intended as separate ideas for how Terraform could help you solve your problem despite the Terraform type system and the AWS policy document format not being exactly aligned. Specifically (now that I know which resource types you're talking about) I imagine the That would ultimately be a change in the AWS provider rather than in Terraform Core though, so would not be directly in scope for this issue. |
Thank you very much Martin. |
Hi @jaffel-lc I have crossed this issue because the same use case (Create module for Opensearch serverless). Also you are welcome to use the module and help to improve it |
Terraform Version
Terraform Configuration Files
Debug Output
https://gist.github.com/jaffel-lc/4bb463fa4086c11efd7e283a2b05f366
Expected Behavior
output value with-security_policy,AWSOwnedKey == false
output value security_policy,AWSOwnedKey == true
Actual Behavior
output value security_policy,AWSOwnedKey == "false"
output value with-security_policy,AWSOwnedKey == "true"
These outputs are based on local variables in the module, and when I try to use those locals in an aws_opensearchserverless_security_policy as the value of the policy parameter I receive this error:
operation error OpenSearchServerless: CreateSecurityPolicy, https response error StatusCode: 400, RequestID: 086b1f49-5575-47ed-b5db-5c61546e240f, ValidationException: Policy json is invalid, error: [$.AWSOwnedKey: string found, boolean expected, │$.AWSOwnedKey: must be a constant value true, $ .KmsARN: is missing but it is required, $.AWSOwnedKey: must be a constant value false]
The difference between security_policy and aws_policy is that security_policy uses an additional kms_key_config map lookup into another local variable before the merge(), while aws_policy merges directly from kms_key_config
In that extra lookup, the boolean value is being converted to string.
Here a gist with the outputs: https://gist.github.com/jaffel-lc/909a106c1c299e5a9f6e0be6c7d82b72
Steps to Reproduce
terraform init
terraform apply
Additional Context
n/a
References
n/a
The text was updated successfully, but these errors were encountered: