SetNestedAttribute with nested computed attributes

Terraform Providers Plugin Development
1

I have a SetNestedAttribute which contains nested computed attributes -

"metric_fields": schema.SetNestedAttribute{
				Optional: true,
				NestedObject: schema.NestedAttributeObject{
					Attributes: map[string]schema.Attribute{
                        "target_base_metric_name": schema.StringAttribute{
							Required: true,
						},
						"source_field": schema.StringAttribute{
							Required: true,
						},
						"aggregations": schema.SingleNestedAttribute{
							Optional: true,
							Computed: true,
							PlanModifiers: []planmodifier.Object{
								objectplanmodifier.UseStateForUnknown(),
							},
							Attributes: map[string]schema.Attribute{
								"min": schema.SingleNestedAttribute{
									Optional: true,
									Computed: true,
									PlanModifiers: []planmodifier.Object{
										objectplanmodifier.RequiresReplaceIfConfigured(),
									},
									Attributes: map[string]schema.Attribute{
										"enable": schema.BoolAttribute{
											Optional: true,
											Computed: true,
											PlanModifiers: []planmodifier.Bool{
												boolplanmodifier.UseStateForUnknown(),
											},
										},
										"target_metric_name": schema.StringAttribute{
											Computed: true,
											PlanModifiers: []planmodifier.String{
												stringplanmodifier.UseStateForUnknown(),
											},
										},
									},
								},
								"max": schema.SingleNestedAttribute{
									Optional: true,
									Computed: true,
									Attributes: map[string]schema.Attribute{
										"enable": schema.BoolAttribute{
											Optional: true,
											Computed: true,
											PlanModifiers: []planmodifier.Bool{
												boolplanmodifier.UseStateForUnknown(),
											},
										},
										"target_metric_name": schema.StringAttribute{
											Computed: true,
											PlanModifiers: []planmodifier.String{
												stringplanmodifier.UseStateForUnknown(),
											},
										},
									},
								},
							},
						},
					},
				},
			},

I wanted to avoid the plan phase to show updates from the existing state values, so I added UseStateForUnknown() for all the computed attributes.

after applying the next config -

...
metric_fields = [
    {
      target_base_metric_name = "method"
      source_field            = "method"
    },
    {
      target_base_metric_name = "geo_point"
      source_field            = "remote_addr_geoip.location_geopoint"
      aggregations = {
        max = {
          enable = false
        }
        min = {
          enable = false
        }
      }
    }
  ]
...

I can see the state was configured correctly -

"metric_fields": [
              {
                "aggregations": {
                  "max": {
                    "enable": false,
                    "target_metric_name": "cx_max"
                  },
                  "min": {
                    "enable": false,
                    "target_metric_name": "cx_min"
                  },
                },
                "source_field": "remote_addr_geoip.location_geopoint",
                "target_base_metric_name": "geo_point"
              },
              {
                "aggregations": {
                  "max": {
                    "enable": true,
                    "target_metric_name": "cx_max"
                  },
                  "min": {
                    "enable": true,
                    "target_metric_name": "cx_min"
                  },
                },
                "source_field": "method",
                "target_base_metric_name": "method"
              }
            ],

However, when I run plan, it seems like the provider want to entirely replace the whole set -

~ metric_fields = [
          - { # forces replacement
              - aggregations            = {
                  - max       = {
                      - enable             = false -> null
                      - target_metric_name = "cx_max" -> null
                    } -> null
                  - min       = {
                      - enable             = false -> null
                      - target_metric_name = "cx_min" -> null
                    } -> null
                } -> null
              - source_field            = "remote_addr_geoip.location_geopoint" -> null
              - target_base_metric_name = "geo_point" -> null
            },
          - { # forces replacement
              - aggregations            = {
                  - max       = {
                      - enable             = true -> null
                      - target_metric_name = "cx_max" -> null
                    } -> null
                  - min       = {
                      - enable             = true -> null
                      - target_metric_name = "cx_min" -> null
                    } -> null
              - source_field            = "method" -> null
              - target_base_metric_name = "method" -> null
            },
          + { # forces replacement
              + aggregations            = {
                  + max       = {
                      + enable             = false
                      + target_metric_name = (known after apply)
                    }
                  + min       = {
                      + enable             = false
                      + target_metric_name = (known after apply)
                    }
                }
              + source_field            = "remote_addr_geoip.location_geopoint"
              + target_base_metric_name = "geo_point"
            },
          + { # forces replacement
              + aggregations            = {
                  + max       = {
                      + enable             = (known after apply)
                      + target_metric_name = (known after apply)
                    } -> (known after apply)
                  + min       = {
                      + enable             = (known after apply)
                      + target_metric_name = (known after apply)
                    } -> (known after apply)
                } -> (known after apply)
              + source_field            = "method"
              + target_base_metric_name = "method"
            },
        ]

Beside that, it seems like it make the whole resource to be replaced -

coralogix_events2metric.logs2metric must be replaced
-/+ resource "coralogix_events2metric" "logs2metric" { ...

Is there something I can do to avoid this?

2

For a situation this complex, it would be better if you posted a Git repository containing your complete code. In particular, I can see you must have some defaulting logic associated with your enable attributes, which isn’t visible in the code snippets shown here. It might be relevant.

The replacement of the whole resource is occurring because you asked Terraform to do that, by using objectplanmodifier.RequiresReplaceIfConfigured().

In a set, object identity is determined by a full comparison of the value. That means you cannot make any change at all to an entry within a set, without it showing as a full delete and add of the entire entry. Because of that, in my opinion, it is almost always incorrect to define any set of object type in Terraform, as the diff that is displayed will be unhelpful to Terraform operators.

In your example, consider what happens if you toggle the enable value - rather than a clean diff from true to false and vice versa, the entire { target_base_metric_name = ..., source_field = ..., aggregations = ... } object will be deleted and re-added (in the diff displayed by Terraform, anyway - the provider code is free to ignore that and do an in-place update internally, if desired).

3

Hi @maxb, thanks for your help!
This is the PR. The resource is coralogix_events2metric.

About your comment - I work with an API that doesn’t keep the creation order, so os there another solution than using Sets in that case?

4

Oh… so you are migrating an existing provider to framework?

That does add a big additional constraint, as presumably you don’t want to force your users to rewrite all their existing configurations, and discard their existing states and re-import everything.

However that will force you to stick with your existing sets, and retain the messy diffs.

Had you been in a greenfield implementation scenario, I would have recommended using maps whereever possible - as if you can give your nested objects names, it lets users and Terraform agree on whether an object is being updated in place, vs. removed and re-added.

I don’t know about your API, but if one of source_field or target_metric_base_name was required to be unique within a metric_fields, they would have been an ideal map key to name the rest of the structure.

5

I’ll take that into consideration, but I’d probably rather cause a one-time pain than permanently bring back messy diffs.
A map would probably be a good solution in my case, thanks!
Just out of curiosity - what causes Set to return messy diffs in my case?

6

As mentioned above:

Does that make sense?

7

I meant why does it show messy diff even when nothing in the set was change?
Also, why does it force replacing the entire resource?

8

I already addressed those questions in a previous reply:

(Something in the set has changed, via your actions, or an unexpected issue with the code.)

EDIT: Sorry, I realise you have since provided a Git link - I haven’t had time to look through it yet to learn more, but the issue is that fundamentally something is changed.

1 Like
9

@maxb
UPDATE: I found my bug - I was missingplanmodifier.UseStateForUnknown() for some of the nested computed attributes.
Thanks!

10

Having looked at your code, I wonder if you’re going to run into further issues partially related to Default values within sets cause crashes and/or incorrect behaviour · Issue #783 · hashicorp/terraform-plugin-framework · GitHub that I filed yesterday…

I’m not totally sure, but be watchful for weird stuff happening when you terraform plan after you have one of your newly modified resources created and in the state file, in cases where some of the nested computed attributes are not specified in the configuration, and are left up to the server to default.

1 Like
11

It didn’t happen in my case, but thanks for letting me know