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

alpha1: DiffSuppressFunc/StateFunc's are ignored in some circumstances #19211

Closed
tombuildsstuff opened this issue Oct 29, 2018 · 3 comments · Fixed by #19226
Closed

alpha1: DiffSuppressFunc/StateFunc's are ignored in some circumstances #19211

tombuildsstuff opened this issue Oct 29, 2018 · 3 comments · Fixed by #19226
Labels
Milestone

Comments

@tombuildsstuff
Copy link
Contributor

Terraform Version

./terraform -v
Terraform v0.12.0-alpha1
+ provider.azurerm v1.17.0-4-g4ed5a4b3

Terraform Configuration Files

resource "azurerm_resource_group" "test" {
  name = "tom-dev0.12"
  location = "West Europe"
  tags = {
    hello = "world"
  }
}

Debug Output

n/a

Crash Output

n/a

Expected Behavior

  • Apply
  • Run Plan
  • No Diff's

Actual Behavior

Plan shows the following diff:

$ envchain azurerm ./terraform plan
provider.azurerm.environment
  Enter a value: public


Refreshing Terraform state in-memory prior to plan...
The refreshed state will be used to calculate this plan, but will not be
persisted to local or remote state storage.

azurerm_resource_group.test: Refreshing state... [id=/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/tom-dev0.12]

------------------------------------------------------------------------

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # azurerm_resource_group.test will be updated in-place
  ~ resource "azurerm_resource_group" "test" {
        id       = "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/tom-dev0.12"
      ~ location = "westeurope" -> "West Europe"
        name     = "tom-dev0.12"
        tags     = {
            "hello" = "world"
        }
    }

Plan: 0 to add, 1 to change, 0 to destroy.

Steps to Reproduce

  1. terraform init
  2. terraform apply
  3. terraform plan

Additional Context

  • 0.12 alpha

References

n/a

@apparentlymart
Copy link
Contributor

Thanks for reporting this, @tombuildsstuff!

Some notes as context for when we look into this deeper:

These DiffSuppressFunc and StateFunc features are implemented entirely within the SDK and Terraform Core isn't aware of them. From Terraform Core's perspective, we pass the configuration, state, and a proposed new state (merger of config+state) to PlanResourceChange and then the provider is responsible for producing a planned new state, which should include the results of applying these customization features like DiffSuppressFunc, and also CustomizeDiff, etc.

Since the SDK logic itself has not been changed significantly, it is more likely that this is an oversight in the code that interfaces the existing provider SDK code with the new protocol. My initial theory is that the provider SDK code responds to DiffSuppressFunc by removing the attribute from the old-style diff altogether, which then allows the value from config to "shine through" when that diff is processed. If that is the bug, then probably the solution would be to merge the provider's proposed diff into the prior state value rather than the "proposed new state", thus leaving unchanged anything that isn't present in the diff the provider generates.

@apparentlymart
Copy link
Contributor

For future reference:

My theory as to what was going wrong here was close, but it turns out that we were correctly applying the diff to the prior state in most cases. The bug was that if the provider returned an entirely empty diff then we were preferring the diff that was produced by the logic in core rather than just using the prior state exactly as-is.

As a result, this issue only cropped up in cases where all of the proposed changes get undone by DiffSuppressFunc/StateFunc; any other change present would've caused the suppression to be honored.

This may have a common root cause with #19151. We'll test that again shortly.

@ghost
Copy link

ghost commented Mar 31, 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 and limited conversation to collaborators Mar 31, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants