Skip to content
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

tfjson: Add DeferredChanges and Complete to Plan JSON #123

Merged
merged 2 commits into from May 2, 2024

Conversation

austinvalle
Copy link
Member

Ref: hashicorp/terraform#35065

This PR adds the JSON data for deferred actions in the planfile. This data is only available when utilizing the deferred action experiment introduced in Terraform v1.9.0-alpha20240404. The actual JSON output changes referenced above will be released in the upcoming alpha release.

@austinvalle austinvalle added the enhancement New feature or request label May 1, 2024
@austinvalle austinvalle added this to the v0.22.0 milestone May 1, 2024
@austinvalle austinvalle requested review from a team as code owners May 1, 2024 20:35
@bflad
Copy link
Member

bflad commented May 2, 2024

Copy link
Member

@radeksimko radeksimko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@radeksimko
Copy link
Member

should we add the top-level Applyable and Complete plan output flags as well?

👍🏻 to that but also it would be fine with me if this was part of a separate PR.

Copy link
Member

@bflad bflad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change looks good to me 🚀 As @radeksimko says we can do the Applyable/Complete separately too, it doesn't matter to me.

@austinvalle
Copy link
Member Author

@austinvalle should we add the top-level Applyable and Complete plan output flags as well?

https://github.com/hashicorp/terraform/pull/34642/files#diff-aec0f9962c6764cbde3325886b9b81650e1007d96d7693a9dcf0df9b53f09d2eR66-R67

Thanks for bringing this up, @liamcervante and I had a discussion in the RFC + briefly offline about this. For some reason I took away that we didn't need the complete flag, but I think reading this back, we should actually add it to combine with checking len(DeferredChanges) == 0.

Added in 46496e3

As for applyable, I think based on Martin's description in the PR we shouldn't need that for deferred actions, so I'll leave that out until it's actually needed.

@austinvalle austinvalle changed the title tfjson: Add DeferredChanges to Plan JSON tfjson: Add DeferredChanges and Complete to Plan JSON May 2, 2024
@austinvalle austinvalle merged commit 3b8a921 into main May 2, 2024
8 checks passed
@austinvalle austinvalle deleted the av/deferral-json branch May 2, 2024 19:40
@bflad
Copy link
Member

bflad commented May 2, 2024

Awesome, thanks @austinvalle!!

@apparentlymart
Copy link
Member

Indeed "applyable" is not directly related to deferred actions.

We might say that one goal of deferred actions is for more plans to be applyable, because today unknown values can make a plan non-applyable by causing errors during planning.

But the deferred actions feature doesn't need to directly interact with it. The Stacks-shaped equivalent of it does help HCP Terraform decide whether it should show the "approve" button for a particular stack plan, and the workspaces equivalent could potentially use that in future rather than having its own copy of similar logic to make that decision, but the decision for whether or not to do that is totally independent of deferred actions.

@austinvalle
Copy link
Member Author

Thanks @apparentlymart!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants