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

d.Set a non-computed schema.Set property during terraform apply will not take effect #618

Closed
magodo opened this issue Oct 20, 2020 · 3 comments
Labels
bug Something isn't working subsystem/types Issues and feature requests related to the type system of Terraform and our shims around it.

Comments

@magodo
Copy link

magodo commented Oct 20, 2020

SDK version

v.1.14.0

Relevant provider source code

The schema definition of a non-computed Set: link

			"addr": {
				Type:     schema.TypeSet,
				Optional: true,
				Elem: &schema.Resource{
					Schema: map[string]*schema.Schema{
						"country": {
							Type:     schema.TypeString,
							Required: true,
						},
						"city": {
							Type:     schema.TypeString,
							Optional: true,
						},
						"roads": {
							Type:     schema.TypeMap,
							Optional: true,
							Elem: &schema.Schema{
								Type: schema.TypeString,
							},
						},
					},
				},
			},

Another computed set definition: link

			"output_addr": {
				Type:     schema.TypeList,
				Computed: true,
				Elem: &schema.Resource{
					Schema: map[string]*schema.Schema{
						"country": {
							Type:     schema.TypeString,
							Computed: true,
						},
						"city": {
							Type:     schema.TypeString,
							Computed: true,
						},
						"roads": {
							Type:     schema.TypeMap,
							Computed: true,
							Elem: &schema.Schema{
								Type: schema.TypeString,
							},
						},
					},
				},
			},

Both will be flattend during the Read callback, wihch will be called in Create.

Now changing the flatten function a bit, instead of setting the actual country, change it to setting a const value foo:

		output = append(output, map[string]interface{}{
			"country": "foo",
			"city":    city,
		})

Terraform Configuration Files

resource "demo_resource_foo" "magodo" {
  name = "magodo"
  addr {
    country = "China"
  }
}

Expected Behavior

With a terraform apply, both the addr.x.country and output_addr.x.country is set to foo.

Actual Behavior

resource "demo_resource_foo" "magodo" {
    id             = "magodo"
    name           = "magodo"
    output_addr    = [
        {
            city    = ""
            country = "xxx"
            roads   = {}
        },
    ]
    output_contact = []

    addr {
        country = "China"
    }
}

Only the output_addr is set during the Read in apply phase, the addr although is flattend into the ResourceData, but is never finalized to the state.

Steps to Reproduce

@magodo magodo added the bug Something isn't working label Oct 20, 2020
@paddycarver paddycarver added the subsystem/types Issues and feature requests related to the type system of Terraform and our shims around it. label Jan 6, 2021
@magodo
Copy link
Author

magodo commented Mar 30, 2021

Yet Another Simplified Example

Given following schema:

            "locations_deprecated": {
                Type:     schema.TypeSet,
                Optional: true,
                Elem: &schema.Schema{
                    Type: schema.TypeString,
                },
            },
            "locations": {
                Type:     schema.TypeSet,
                Optional: true,
                Elem: &schema.Schema{
                    Type: schema.TypeString,
                },
            },

And following code in Create and Read function:

func resourceBarCreateOrUpdate(d *schema.ResourceData, m interface{}) error {
   ...
    param.Locations = expandLocationSlicePtr(d.Get("locations").(*schema.Set).List())
    if param.Locations == nil {
        param.Locations = expandLocationSlicePtr(d.Get("locations_deprecated").(*schema.Set).List())
    }
   ...
}

func resourceBarCreateOrUpdate(d *schema.ResourceData, m interface{}) error {
   ...
    if err := d.Set("locations", flattenLocationSlicePtr(resp.Locations)); err != nil {
        return fmt.Errorf(`setting "locations": %v`, err)
    }

    if err := d.Set("locations_deprecated", flattenLocationSlicePtr(resp.Locations)); err != nil {
        return fmt.Errorf(`setting "locations_deprecated": %v`, err)
    }
   ...
}

Then the issue can be reproduced as below:

💤 cat main.tf
resource "demo_resource_bar" "magodo" {
  name = "magodo"
  locations = ["a"]
}

💤 tf apply -auto-approve
demo_resource_bar.magodo: Creating...
demo_resource_bar.magodo: Creation complete after 0s [id=magodo]

Apply complete! Resources: 1 added, 0 changed, 0 destroyed.

💤 tf show
# demo_resource_bar.magodo:
resource "demo_resource_bar" "magodo" {
    id                   = "magodo"
    locations            = [
        "a",
    ]
    locations_deprecated = [
        "a",
    ]
    name                 = "magodo"
    phone                = 0
}

💤 sed -i 's;"a";"b";' main.tf

💤 tf apply -auto-approve
tf demo_resource_bar.magodo: Refreshing state... [id=magodo]
shdemo_resource_bar.magodo: Modifying... [id=magodo]
odemo_resource_bar.magodo: Modifications complete after 0s [id=magodo]

Apply complete! Resources: 0 added, 1 changed, 0 destroyed.

💤 tf show
# demo_resource_bar.magodo:
resource "demo_resource_bar" "magodo" {
    id                   = "magodo"
    locations            = [
        "b",
    ]
    locations_deprecated = []
    name                 = "magodo"
    phone                = 0
}

💤 tf refresh
demo_resource_bar.magodo: Refreshing state... [id=magodo]

💤 tf show
# demo_resource_bar.magodo:
resource "demo_resource_bar" "magodo" {
    id                   = "magodo"
    locations            = [
        "b",
    ]
    locations_deprecated = [
        "b",
    ]
    name                 = "magodo"
    phone                = 0
}

As shown above, after updating the locations at the first time, only the locations is got updated, while the locations_deprecated is not. A follow-up refresh updated both.

The source code is here: https://github.com/magodo/terraform-provider-demo/tree/d_set_on_set_with_no_diff

The Issue doesn't happen for TypeList

Changing the type of both properties from TypeSet and TypeList. Then the second apply will update both properties, as expected.

The source code is here: https://github.com/magodo/terraform-provider-demo/tree/d_set_on_list

Assumption

The sympton seems like the d.Set() on TypeSet only impact the ones that have diff?

@bflad
Copy link
Contributor

bflad commented Mar 21, 2022

Hi @magodo 👋 Thanks for raising this and sorry you ran into trouble here.

Looking at both of these cases, this appears to be expected behavior with how Terraform works between the configuration, plan, and state phases of a resource lifecycle. For reference, this documentation might be helpful: https://github.com/hashicorp/terraform/blob/20e9d8e28c32de78ebb8ae0ba2a3a00b70ee89f4/docs/resource-instance-change-lifecycle.md

The gist here is that the provider (via the Schema and potentially CustomizeDiff) must signal to Terraform CLI the proper expectations about a value for an attribute. If the resource schema denotes Required, the configuration value must match the plan value which must match the value saved into the state (note that the SDK will allow you to incorrectly modify the value without returning its own error). To signal that a value may be configured or come from the provider, the schema should be marked with Optional and Computed. If the value is not expected to be configured and instead should be updated due to other configuration changes, it should be marked Computed and a CustomizeDiff function should be implemented that calls (*ResourceDiff).SetNewComputed() or (*ResourceDiff).SetNew() if the new value is wholly known. Helpers like customdiff.ComputedIf can potentially simplify that type of CustomizeDiff logic.

Recent versions of Terraform CLI when used with this SDK should generate warning logs in these data mismatch scenarios, which can be seen by running with TF_LOG=TRACE terraform apply. Newer SDKs, such as terraform-plugin-framework, will return error diagnostics instead of warning logs in these cases.

Hope this helps.

@bflad bflad closed this as completed Mar 21, 2022
@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working subsystem/types Issues and feature requests related to the type system of Terraform and our shims around it.
Projects
None yet
Development

No branches or pull requests

3 participants