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

terraform: Write state if sensitivity changes #27128

Merged
merged 1 commit into from
Dec 3, 2020

Conversation

alisdair
Copy link
Contributor

@alisdair alisdair commented Dec 3, 2020

When applying, we return early if only sensitivity changed between the before and after values of the changeset. This avoids unnecessarily invoking the provider.

Previously, we did not write the new value for a resource to the state when this happened. The result was a permanent diff for resource updates which only change sensitivity, as the apply step is skipped and the state is unchanged.

This commit adds a state write to this shortcut return path, and fixes a test for this exact case which was accidentally relying on a value diff caused by an incorrect manual state value.

Fixes #26959. Targeting release in 0.14.1.

When applying, we return early if only sensitivity changed between the
before and after values of the changeset. This avoids unnecessarily
invoking the provider.

Previously, we did not write the new value for a resource to the state
when this happened. The result was a permanent diff for resource updates
which only change sensitivity, as the apply step is skipped and the
state is unchanged.

This commit adds a state write to this shortcut return path, and fixes a
test for this exact case which was accidentally relying on a value diff
caused by an incorrect manual state value.
@alisdair alisdair added the 0.14-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged label Dec 3, 2020
@alisdair alisdair added this to the v0.14.x milestone Dec 3, 2020
@alisdair alisdair requested a review from a team December 3, 2020 21:00
@alisdair alisdair self-assigned this Dec 3, 2020
@codecov
Copy link

codecov bot commented Dec 3, 2020

Codecov Report

Merging #27128 (50f4d79) into master (2ce71ab) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted Files Coverage Δ
terraform/eval_apply.go 74.41% <100.00%> (+0.53%) ⬆️
communicator/communicator.go 77.19% <0.00%> (-3.51%) ⬇️
states/statefile/version4.go 57.96% <0.00%> (-0.24%) ⬇️
terraform/evaluate.go 53.30% <0.00%> (+0.41%) ⬆️
dag/marshal.go 63.49% <0.00%> (+1.58%) ⬆️
terraform/node_resource_plan.go 98.05% <0.00%> (+1.94%) ⬆️
internal/providercache/dir.go 73.46% <0.00%> (+6.12%) ⬆️

@alisdair alisdair merged commit 6bacc7a into master Dec 3, 2020
@alisdair alisdair deleted the alisdair/sensitivity-change-permadiff branch December 3, 2020 21:48
@ghost
Copy link

ghost commented Jan 3, 2021

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 Jan 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
0.14-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged
Projects
None yet
2 participants