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

backend/local: revert duplicate ctx creation #26041

Closed
wants to merge 5 commits into from

Conversation

mildwonkey
Copy link
Contributor

@mildwonkey mildwonkey commented Aug 28, 2020

A recent change to the local backend had a side effect of causing all input vars to be requested twice (due to two calls to contextDirect) and
provider input values to be requested but not stored (those were populated
during Validate, but Validate was called on the "extra" context and not
the returned context).

This PR modifies the graphWalker to substitute an empty state when creating the validate graph.

Fixes #26027 and #26035

I've modified and added tests to confirm both problems input var problems have been fixed; I do not think there's a test that covers the earlier issue. I verified that it's still fixed locally using the reproduction in the original issue: (#25752)

A recent change to the backend had a side effect of causing all input
vars to be requested twice (due to two calls to `contextDirect`) and
provider input values to be omitted entirely (those were populated
during Validate, but Validate was called on the "extra" context and not
the returned context).

This PR adds a Context.StatelessCopy function which returns a nearly-identical copy of
a terraform.Context with the state "zeroed out", which can then be
Validate()d. A better, future refactor should remove the need for this
function and instead modify Validate() itself to ignore state.

Fixes #26027

I've added tests that confirm both behaviors; I am not sure what test
covers the behavior that lead to this change in the first place but I
did verify locally using the reproduction in the original PR.
@codecov
Copy link

codecov bot commented Aug 28, 2020

Codecov Report

Merging #26041 into master will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Impacted Files Coverage Δ
backend/local/backend_local.go 41.23% <100.00%> (-0.90%) ⬇️
terraform/context.go 86.07% <100.00%> (+0.07%) ⬆️
command/ui_input.go 61.03% <0.00%> (-5.20%) ⬇️
dag/walk.go 92.54% <0.00%> (-0.63%) ⬇️
terraform/evaluate.go 53.93% <0.00%> (-0.44%) ⬇️

@mildwonkey
Copy link
Contributor Author

SO CLOSE! There's an eval node called in Validate which is updating state, and that seems to be the root of that one test failure. I'll try again monday.

@mildwonkey mildwonkey closed this Aug 28, 2020
@ghost
Copy link

ghost commented Oct 13, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked as resolved and limited conversation to collaborators Oct 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Terraform 0.13.1 prompting for variables twice
1 participant