Refined Unknown Values, allowing some operations with unknown values to produce known results #33234
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jbardin
approved these changes
May 24, 2023
cty's new "refinements" concept allows us to reduce the range of unknown values from our functions. This initial changeset focuses only on declaring which functions are guaranteed to return a non-null result, which is a helpful baseline refinement because it allows "== null" and "!= null" tests to produce known results even when the given value is otherwise unknown. This commit also includes some updates to test results that are now refined based on cty's own built-in refinement behaviors, just as a result of us having updated cty in the previous commit.
The "id" attribute of this resource type is generated by the provider itself and can never be null, so we'll refine the range of its unknown result in case that helps downstream expressions to produce known results even when the exact value hasn't yet been planned.
If the string to be tested is an unknown value that's been refined with a prefix and the prefix we're being asked to test is in turn a prefix of that known prefix then we can return a known answer despite the inputs not being fully known. There are also some other similar deductions we can make about other combinations of inputs. This extra analysis could be useful in a custom condition check that requires a string with a particular prefix, since it can allow the condition to fail even on partially-unknown input, thereby giving earlier feedback about a problem.
If the original value was unknown but its range was refined then the provider must return a value that is within the refined range, because otherwise downstream planning decisions could be invalidated. This relies on cty's definition of whether a value is in a refined range, which has pretty good coverage for the "false" case and so should give a pretty good signal, but it'll probably improve over time and so providers must not rely on any loopholes in the current implementation and must keep their promises even if Terraform can't currently check them.
Providers that existed prior to refinements (all of them, at the time of writing) cannot preserve refinements sent in unknown values in the configuration, and even if one day providers _are_ aware of refinements there we might add new ones that existing providers don't know how to handle. For that reason we'll absolve providers of the responsibility of preserving refinements from config into plan by fixing some cases where we were incorrectly using RawEquals to compare values; that function isn't appropriate for comparing values that might be unknown. However, to avoid a disruptive change right now this initial fix just strips off the refinements before comparing. Ideally this should be using Value.Equals and handling unknown values more explicitly, but we'll save that for a possible later improvement. This does not include a similar exception for validating whether a final value conforms to a plan because the plan value and the final value are both produced by the same provider and so providers ought to be able to be consistent with their _own_ treatment of refinements, if any. Configuration is special because Terraform itself generates that, and so it can potentially contain refinements that a particular provider has no awareness of.
This is actually a description of the "cty" library's encoding of refined values, but from the perspective of the plugin protocol it's an implementation detail that Terraform Core outsources that to a third-party library, and current server-side implementations of the protocol use an independent implementation of this format which will need to be compatible with what cty does.
apparentlymart
force-pushed
the
f-cty-refinements
branch
from
f1c0ec1
to
e0ef274
Compare
|
Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch. |
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This introduces a new concept called Refinements from upstream
cty, which is the foundation of the Terraform language type system.For example, this allows an known value to be refined as "not null", which means that
v != nullandv == nullcan return a known true or false even if the actual value isn't yet known. There are also various other refinements to help with some other common situations, such as testing whether the length of a collection is greater than zero, whether the length of a string is greater than zero, etc.The individual commits have more detailed commit messages, so it might help to review this on a per-commit basis rather than as a large flat diff.
I'm proposing this now -- very early in the v1.6 development period -- so that there will be as much opportunity as possible for these changes to "soak" throughout later development and any alpha releases we'll produce during that development period.
This is a change in
cty's behavior within the bounds of its compatibility policy, which allows unknown values to become "more known". As noted in those docs, anything usingcty.Value.RawEqualsis a potential hazard under that policy and so that function is supposed to be used only in test code. Terraform does incorrectly use it in non-test code in a few select spots, so this PR either replaces those withEqualscalls (where it was relatively non-invasive) or strips the refinements before comparing to avoid a change in behavior.This is the sort of thing that can get more precise over time as we learn where further annotation will be useful, so this PR is mostly focused just on getting the new concept wired in but it does include some initial refinements in addition to those provided automatically by
cty:startswithfunction can take the "string prefix" refinement into account.Because
cty's MessagePack serialization now supports refinements the Terraform provider protocol effectively does too, and so this includes some tweaks to theplans/objchangerules to take that into account and some initial documentation on the serialization of refinements. However, I don't recommend that the provider framework start supporting this just yet until we have some more experience with it just within the core language. Eventually it would be useful for the plugin framework to allow providers to at least signal that certain attributes can never be null, but it'll be much harder to tweak the design of that once it's deployed in real providers so best to wait to see.I have a separate commit in HCL that also makes HCL's own operators refine unknown values in their results, such as refining arithmetic operations to represent that they never produce null results, but I've intentionally not included that here because this PR is already quite broad. I'll work on getting the HCL side of this merged later on, and then open a separate PR to incorporate the changes from HCL.
Closes #31078
Closes #15498
Closes #31035