Skip to content

failing resource instance apply should keep prior state#37981

Merged
DanielMSchmidt merged 3 commits intomainfrom
fails-in-apply-should-not-set-empty-state
Dec 11, 2025
Merged

failing resource instance apply should keep prior state#37981
DanielMSchmidt merged 3 commits intomainfrom
fails-in-apply-should-not-set-empty-state

Conversation

@DanielMSchmidt
Copy link
Copy Markdown
Contributor

@DanielMSchmidt DanielMSchmidt commented Dec 9, 2025

This is a code path that can not be taken in normal Terraform, but when TF is used through the RPC API we can end up in these situation, so we should make sure Terraform does not remove the state when an apply fails.

Fixes #

Target Release

1.15.x

Rollback Plan

  • If a change needs to be reverted, we will roll out an update to the code within 7 days.

Changes to Security Controls

Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.

CHANGELOG entry

  • This change is user-facing and I added a changelog entry.
  • This change is not user-facing.

nfagerlund
nfagerlund previously approved these changes Dec 9, 2025
Copy link
Copy Markdown
Member

@nfagerlund nfagerlund left a comment

Choose a reason for hiding this comment

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

I manually tested this with my live repro of the bug, using a Terraform build with this patch applied but without the locals fix from #37980. We're good!

  • As expected, the plan showed "known after apply" for attrs but didn't error, and allowed me to proceed to apply. (#37980 fixes that.)
  • As expected, the apply errored and failed with aws_amplify_app.default still contains unknown values during apply (this is a bug in Terraform; please report it!).
  • As hoped for, a subsequent plan and apply with the configuration repaired produced no changes! The errored resource was not removed from state.

dsa0x
dsa0x previously approved these changes Dec 10, 2025
@DanielMSchmidt DanielMSchmidt dismissed stale reviews from dsa0x and nfagerlund via 7df49b1 December 11, 2025 09:29
@DanielMSchmidt DanielMSchmidt force-pushed the fails-in-apply-should-not-set-empty-state branch from 1959981 to 7df49b1 Compare December 11, 2025 09:29
@DanielMSchmidt DanielMSchmidt added the 1.14-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged label Dec 11, 2025
@DanielMSchmidt DanielMSchmidt force-pushed the fails-in-apply-should-not-set-empty-state branch from 7df49b1 to 9dfd55c Compare December 11, 2025 09:30
@DanielMSchmidt DanielMSchmidt merged commit 889d605 into main Dec 11, 2025
13 checks passed
@DanielMSchmidt DanielMSchmidt deleted the fails-in-apply-should-not-set-empty-state branch December 11, 2025 09:36
@github-actions
Copy link
Copy Markdown
Contributor

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.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 11, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

1.14-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants