-
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
plans: indicate when resource deleted due to move #31695
Conversation
891b5d5
to
af64bda
Compare
af64bda
to
e59f116
Compare
e59f116
to
aee13f7
Compare
Add a new ChangeReason, ReasonDeleteBecauseNoMoveTarget, to provide better information in cases where a planned deletion is due to moving a resource to a target not in configuration. Consider a case in which a resource instance exists in state at address A, and the user adds a moved block to move A to address B. Whether by the user's intention or not, address B does not exist in configuration. Terraform combines the move from A to B, and the lack of configuration for B, into a single delete action for the (previously nonexistent) entity B. Prior to this commit, the Terraform plan will report that resource B will be destroyed because it does not exist in configuration, without explicitly connecting this to the move. This commit provides the user an additional clue as to what has happened, in a case in which Terraform has elided a user's action and inaction into one potentially destructive change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The UI looks good to me.
I think we could achieve this same UI result without adding a new change reason. Essentially, taking the logic from NodePlannableResourceInstanceOrphan
and moving it to the diff renderer. I'm not sure if that's better or worse, but it'd be a smaller diff.
Thank you. I agree that this could be done in principle in the diff renderer. It would require a bit of refactoring which I think would make the code less clear and harder to maintain, since we would have to add logic to say that in this one particular case, the |
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. |
Add a new
ChangeReason
,ReasonDeleteBecauseNoMoveTarget
, to provide better information in cases where a planned deletion is due to moving a resource to a target not in configuration.This provides the user an additional clue as to what has happened, in a case in which Terraform has elided a user's action and inaction into one potentially destructive change.
Example
Consider a state which contains the resource
null_resource.foo
.Configuration
Steps
terraform plan
Before this PR
After this PR