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

DiffSuppressFunc doesn't work for list-type attributes #477

Open
kristian-lesko opened this issue Jun 17, 2020 · 6 comments
Open

DiffSuppressFunc doesn't work for list-type attributes #477

kristian-lesko opened this issue Jun 17, 2020 · 6 comments
Labels
breaking-change Implementation which breaks compatibility or other promises enhancement New feature or request subsystem/types Issues and feature requests related to the type system of Terraform and our shims around it.

Comments

@kristian-lesko
Copy link

SDK version

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

Relevant provider source code

type SchemaDiffSuppressFunc func(k, old, new string, d *ResourceData) bool

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 kristian-lesko added the bug Something isn't working label Jun 17, 2020
@bendrucker
Copy link

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

@CyrusJavan
Copy link

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.

@toumorokoshi
Copy link

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.

alexissavin added a commit to alexissavin/terraform-provider-solidserver that referenced this issue Jun 16, 2021
alexissavin added a commit to alexissavin/terraform-provider-solidserver that referenced this issue Jun 17, 2021
@diabloneo
Copy link

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.

@monde
Copy link

monde commented Dec 2, 2022

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.

@Ravikc
Copy link

Ravikc commented Oct 31, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Implementation which breaks compatibility or other promises enhancement New feature or request 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

8 participants