Backport of core: Defer on transitive dependencies for data resources with conditions into v1.2 #31034
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
teamterraform
force-pushed
the
backport/f-data-conditions-during-apply/privately-sacred-panda
branch
from
0e3d393
to
51e69ac
Compare
We have two different reasons why a data resource might be read only during apply, rather than during planning as usual: the configuration contains unknown values, or the data resource as a whole depends on a managed resource which itself has a change pending. However, we didn't previously distinguish these two in a way that allowed the UI to describe the difference, and so we confusingly reported both as "config refers to values not yet known", which in turn led to a number of reasonable questions about why Terraform was claiming that but then immediately below showing the configuration entirely known. Now we'll use our existing "ActionReason" mechanism to tell the UI layer which of the two reasons applies to a particular data resource instance. The "dependency pending" situation tends to happen in conjunction with "config unknown", so we'll prefer to refer that the configuration is unknown if both are true.
…ions When a data resource is used for the purposes of verifying a condition about an object managed elsewhere (e.g. if the managed resource doesn't directly export all of the information required for the condition) it's important that we defer the data resource read to the apply step if the corresponding managed resource has any changes pending. Typically we'd expect that to come "for free" but unfortunately we have a pragmatic special case in our handling of data resources where we normally defer to the apply step only if a _direct_ dependency of the data resource has a change pending, and allow a plan-time read if there's a pending change in an indirect dependency. This allowed us to preserve some compatibility with the questionable historical behavior of always reading data resources proactively unless the configuration contains unknown values, since the arguably-more-correct behavior would've been a regression for anyone who had been depending on that before. Since preconditions and postconditions didn't exist until now, we are not constrained in the same way by backward compatibility, and so we can adopt the more correct behavior in the case where a data resource has conditions specified. This does unfortunately make the handling of data resources with conditions subtly inconsistent with those that don't, but this is a better situation than the alternative where it would be easy to get into a trapped situation where the remote system is invalid and it's impossible to plan the change that would make it valid again because the conditions evaluate too soon, prior to the fix being applied.
apparentlymart
force-pushed
the
backport/f-data-conditions-during-apply/privately-sacred-panda
branch
from
51e69ac
to
8b38fc1
Compare
apparentlymart
deleted the
backport/f-data-conditions-during-apply/privately-sacred-panda
branch
|
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.
Backport
This PR is auto-generated from #31028 to be assessed for backporting due to the inclusion of the label 1.2-backport.
The below text is copied from the body of the original PR.
@apparentlymart subsequently replaced the branch the bot generated both because he dislikes how the bot creates useless commit messages and because this backport also needs to include #30971 in order to give suitable feedback.
When a data resource is used for the purposes of verifying a condition about an object managed elsewhere (e.g. if the managed resource doesn't directly export all of the information required for the condition) it's important that we defer the data resource read to the apply step if the corresponding managed resource has any changes pending.
Typically we'd expect that to come "for free" but unfortunately we have a pragmatic special case in our handling of data resources where we normally defer to the apply step only if a direct dependency of the data resource has a change pending, and allow a plan-time read if there's a pending change in an indirect dependency. This allowed us to preserve some compatibility with the questionable historical behavior of always reading data resources proactively unless the configuration contains unknown values, since the arguably-more-correct behavior would've been a regression for anyone who had been depending on that before.
Since preconditions and postconditions didn't exist until now, we are not constrained in the same way by backward compatibility, and so we can adopt the more correct behavior in the case where a data resource has conditions specified. This does unfortunately make the handling of data resources with conditions subtly inconsistent with those that don't, but this is a better situation than the alternative where it would be easy to get into a trapped situation where the remote system is invalid and it's impossible to plan the change that would make it valid again because the conditions evaluate too soon, prior to the fix being applied.