terraform-plugin-sdk icon indicating copy to clipboard operation
terraform-plugin-sdk copied to clipboard

DiffSuppressFunc doesn't work for list-type attributes

Open kristian-lesko opened this issue 5 years ago • 6 comments

SDK version

{
  "Path": "github.com/hashicorp/terraform-plugin-sdk",
  "Version": "v1.13.0"
}

Relevant provider source code

https://github.com/hashicorp/terraform-plugin-sdk/blob/79b4af5fffc8a33bcff6183efc99f3006d7fde5c/helper/schema/schema.go#L265

func resourceService() *schema.Resource {
...
                        "test_attribute": &schema.Schema{
                                Type:     schema.TypeList,
                                Optional: true,
                                Elem:     &schema.Schema{Type: schema.TypeString},
                        },
...

Terraform Configuration Files

resource "example_resource" "example_name" {
  test_attribute = ["value1", "value2", "value3"]
}

Expected Behavior

It should be possible to implement a DiffSuppressFunction that would enable ignoring the order on an attribute that is a list of strings.

Actual Behavior

The DiffSuppressFunction schema only expects a string as the old and new attribute values; it doesn't seem possible to use on an attribute whose type is a list of strings.

kristian-lesko avatar Jun 17 '20 08:06 kristian-lesko

A variation on this: DiffSuppressFunc is also not supported for schema.TypeMap. This prevents a provider from implementing diff suppression on a map, for example:

https://www.terraform.io/docs/providers/kubernetes/r/resource_quota.html https://www.terraform.io/docs/providers/kubernetes/r/limit_range.html

Both of these resources use maps of resource quantities, mapping from a resource type (e.g. cpu) to a quantity (with an optional unit). CPU is often unit-less (2 = "2 cores") but can also have units (2000m = "2000 millicores").

The provider suppresses diffs where possible (when explicit fields are used) but cannot apply the same logic if the keys are not known in advance:

https://github.com/hashicorp/terraform-provider-kubernetes/blob/64c66f7634de8ca27d79fcda6b7d058ffaec460c/kubernetes/schema_container.go#L97-L121

bendrucker avatar Jun 18 '20 23:06 bendrucker

Another benefit of making DiffSuppressFunc work with TypeList attributes would be the ability to easily create index-able non-unique element sets. e.g. TypeList attribute with a DiffSuppressFunc that ignores any diff due to an ordering mismatch.

CyrusJavan avatar Dec 02 '20 00:12 CyrusJavan

In my experimentation, adding a DiffSuppressFunc to a TypeList element results in the func running on every element individually, but not the whole element. Although it does have some bugs, such as #743.

The current behavior doesn't allow considering the who list and doing things like shuffling order, but it does allow for examining individual elements and suppressing those diffs.

toumorokoshi avatar Apr 21 '21 17:04 toumorokoshi

This behavior is really unnatural and undocumented. I had to debug my provider to figure out what's going on under the hood. I figured a simple way to do diff suppress against the list rather than the elements:

func ipListDiffSuppress(key, oldValue, newValue string, d *schema.ResourceData) bool {
	// For a list, the key is path to the element, rather than the list.
	// E.g. "node_groups.2.ips.0"
	lastDotIndex := strings.LastIndex(key, ".")
	if lastDotIndex != -1 {
		key = string(key[:lastDotIndex])
	}

	oldData, newData := d.GetChange(key)
	if oldData == nil || newData == nil {
		return false
	}

	oldIps := common.ConcreteList[string](oldData.([]any)) // from []any to []string
	newIps := common.ConcreteList[string](newData.([]any))
	sort.Strings(oldIps)
	sort.Strings(newIps)

	return reflect.DeepEqual(oldIps, newIps)
}

Bug, as @toumorokoshi said, this will cause the function called on every element in the list. If you want to mitigate the problem, you can use a memory cache whose key is address of the list attribute.

Looking forward to a real DiffSuppressFunc for the TypeList.

diabloneo avatar Sep 07 '22 01:09 diabloneo

I ran into a similar situation with another provider and we are using @diabloneo's technique to hack a diff suppress on list of strings that are not maintained in order by the underlying API.

monde avatar Dec 02 '22 18:12 monde

Is there any fix planned for this? Or will this only be addressed in the new plugin framework?

Ravikc avatar Oct 31 '23 06:10 Ravikc