-
Notifications
You must be signed in to change notification settings - Fork 9.5k
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
Reject or warn about additional attributes during object type conversion #33570
Comments
Hi @asaba-hashi, In the Terraform language an object type constraint accepts any value that has at least the attributes specified. Any additional attributes are discarded during type conversion. Therefore this seems like Terraform working as designed, similar to what would happen if a module author declared an input variable of an object type. This is a design tradeoff of the type system that is intended to allow the producer and consumer of an argument to evolve independently:
Although the ability to "provide more" is not important when the object is written out literally using an object constructor as you showed here, it is important to reduce coupling when the entire object is being provided from further away, such as in another module or as an output attribute of another resource type. If you would prefer a more rigid interpretation then I think you will need to declare a nested block type called If this is a request to change Terraform's type system to reject attributes that are not declared rather than just discarding them during conversion then unfortunately I don't see a path to get there within the Terraform v1.x Compatibility Promises, even if we were to decide that the compatibility considerations I described above were not important. |
terraform validate
or warn
Thanks for the thorough explanation! As a new framework based provider, I only chose a nested attributes because of the following note in the docs, so I've opened up a doc issue for the framework in the meantime:
|
It sounds like there is some potential conflation of concerns because any particular schema definition recommendations will cause provider developers to try to choose an appropriate practitioner experience for them. When is it more "correct" for a provider developer to choose blocks over attributes? Nested blocks, while having the strict attribute name validation, are inherently more difficult to work with as a practitioner since they require explicit per attribute re-mapping and potential usage of dynamic block expressions. My understanding of the intention of adding nested attributes into schema definitions was to simplify the practitioner experience in this regard, where the behaviors mentioned by @apparentlymart are actually a benefit in many cases. |
Indeed, unfortunately it is the fact that blocks are "more difficult to work with" that makes it reasonable to do the stricter validation we've historically done for them: the arguments inside a block must always be written out individually and explicitly, which means we know that any extraneous arguments were put there directly by the author of the block. In return for the more onerous requirements at the definition site we get more information about author intent and so can justify stricter error checking. On the other hand, arguments of an object type add some more flexibility because now it's possible to dynamically generate an entire object, or who wholesale assign an object from one resource to another, and various other situations where the exact set of attributes is not explicitly specified in the source code immediately at the assignment site. That means we take this different approach so that the source of the object and the location that it's assigned to will not be so tightly coupled. Without this compromise, it would be a potential breaking change to add an attribute to any existing module output value or resource type schema, which would make it challenging to evolve providers and modules to meet new requirements. (The idea here is similar to how in JSON-based API design it's common to ask the recipient of a JSON document to ignore properties they don't recognize, rather than immediately returning an error, so that there is a clear path to backward-compatible API evolution.) These two design approaches have different tradeoffs, and so I don't think it's really possible to rule that one is unambiguously "better" than the other. Instead, like most API design decisions, one needs to understand the benefits and consequences of each approach and decide which one best suits the problem. I think it's fair to say that the value-based approach offers more flexibility and is therefore probably a reasonable default choice where the answer is unclear, but I wouldn't assert that it's universally better to use that over blocks. For example, in my old experimental testing provider I decided to use nested blocks to describe each assertion because there is little reason to be generating a set of test assertions dynamically, and the block structure allows Terraform to give better feedback for incorrect usage in return for the relative rigidity. But I would not claim that tradeoff as universal; for most real infrastructure objects (as opposed to utility providers) it seems more important to be flexible in allowing dynamic definitions, even though that generates slightly less helpful feedback for incorrect usage. |
I'm confused about the reasoning for allowing unknown nested attributes. Is it to enable forward compatibility with configs that target newer versions of the provider? If that is the case, then why are resources with unknown non-nested attributes rejected? |
Terraform Version
Terraform Configuration Files
Debug Output
n/a
Expected Behavior
terraform validate
should fail with a message that an unknown key is present.Actual Behavior
terraform validate/plan/apply
all ran succesfully without errors or warning.Steps to Reproduce
terraform validate
.Additional Context
It was suggested by @bflad to open an issue in this tracker instead, including some notes about what is being passed: hashicorp/terraform-plugin-framework#805 (comment)
References
There appear to be a number of related issues already open:
The text was updated successfully, but these errors were encountered: