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

Dynamic type attribute does not work with sets #1008

Closed
mawasile opened this issue Jun 12, 2024 · 4 comments
Closed

Dynamic type attribute does not work with sets #1008

mawasile opened this issue Jun 12, 2024 · 4 comments
Labels
bug Something isn't working

Comments

@mawasile
Copy link

Module version

1.9.0

Relevant provider source code

resource implementation:
https://github.com/microsoft/terraform-provider-power-platform/blob/main/internal/powerplatform/services/data_record/resource_data_record.go

func (r *DataRecordResource) Schema(ctx context.Context, req resource.SchemaRequest, resp *resource.SchemaResponse) {
	resp.Schema = schema.Schema{
		Description:         "The Power Platform Data Record Resource allows the management of configuration records that are stored in Dataverse as records. This resource is not recommended for managing business data or other data that may be changed by Dataverse users in the context of normal business activities.",
		MarkdownDescription: "The Power Platform Data Record Resource allows the management of configuration records that are stored in Dataverse as records. This resource is not recommended for managing business data or other data that may be changed by Dataverse users in the context of normal business activities.",
		Attributes: map[string]schema.Attribute{
			"id": schema.StringAttribute{
				MarkdownDescription: "Unique id (guid)",
				Description:         "Unique id (guid)",
				Computed:            true,
				PlanModifiers: []planmodifier.String{
					stringplanmodifier.UseStateForUnknown(),
				},
			},
			"environment_id": schema.StringAttribute{
				Description: "Id of the Dynamics 365 environment",
				Required:    true,
				PlanModifiers: []planmodifier.String{
					stringplanmodifier.RequiresReplace(),
				},
			},
			"table_logical_name": schema.StringAttribute{
				Description: "Logical name of the data record table",
				Required:    true,
				PlanModifiers: []planmodifier.String{
					stringplanmodifier.RequiresReplace(),
				},
			},
			"columns": schema.DynamicAttribute{
				Description: "Columns of the data record table",
				Required:    true,
			},
		},
	}
}

Terraform Configuration Files

terraform {
  required_providers {
    powerplatform = {
      source = "microsoft/power-platform"
    }
  }
}

provider "powerplatform" {
  use_cli = true
}



resource "powerplatform_environment" "data_record_example_env" {
  display_name     = "powerplatform_data_record_example"
  location         = "europe"
  environment_type = "Sandbox"
  dataverse = {
    language_code     = "1033"
    currency_code     = "USD"
    security_group_id = "00000000-0000-0000-0000-000000000000"
  }
}


resource "powerplatform_data_record" "account" {
  environment_id     = powerplatform_environment.data_record_example_env.id
  table_logical_name = "account"

  columns = {
    name = "Contoso"

    contact_customer_accounts = toset([
      {
        table_logical_name = "contact",
        data_record_id     = powerplatform_data_record.contact.id
      }
    ])

  }
}

resource "powerplatform_data_record" "contact" {
  environment_id     = powerplatform_environment.data_record_example_env.id
  table_logical_name = "contact"

  columns = {
    firstname = "John"
    lastname  = "Doe"
  }
}

Debug Output

apply.log and plan.log can be found here: https://gist.github.com/mawasile/f61ce8807e1011983b9089f97dba542b

Expected Behavior

We use dynamic type for "columns" attribute and inside that we also allow to create collection of values. if that collection is a list everything is working fine but when we use toset() then in terraform plan we get in-place update which is incorrect as nothing had changed. Just by removing toset in contact_customer_accounts from the above example, fixes that issue.

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # powerplatform_data_record.account will be updated in-place
  ~ resource "powerplatform_data_record" "account" {
        id                 = "830dc891-9b28-ef11-840a-000d3ab60f37"
        # (3 unchanged attributes hidden)
    }

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

Actual Behavior

toset should not generate dirty state when used in dynamic attribute

Steps to Reproduce

  1. have a terraform resource with dynamic attribute with a set attribute inside that dynamic type
  2. terraform apply
  3. terraform plan => plan is not empty
@mawasile mawasile added the bug Something isn't working label Jun 12, 2024
@austinvalle
Copy link
Member

austinvalle commented Jul 19, 2024

Hey there @mawasile 👋🏻, thanks for reporting the issue with an example and sorry you're running into trouble here.

I wasn't able to recreate this behavior with the dynamic attribute isolated in a sandbox, but the CLI output you shared seems to indicate that Terraform core is detecting a difference, but the plan renderer cannot find where the difference is. With dynamic schemas it can be a little difficult to pinpoint where that difference is being introduced.

Looking through your provider code, I do see some code that could be a good place to start investigating.

So columns is a Required attribute, which to Terraform means that the value that is set in the Terraform configuration must be preserved during plan/apply. One piece of code I found interesting is here:

https://github.com/microsoft/terraform-provider-power-platform/blob/291015bd65b7a712eab20a56c92c7bcc275480c4/internal/powerplatform/services/data_record/resource_data_record.go#L163-L168

This code is attempting to refresh a configuration value, so if the value being refreshed is not an exact match, Terraform will detect drift continuously, until the practitioner updates their configuration to match the new refreshed value. I would expect, since this value is not Computed, that it should not be refreshed.

Regardless, I'd be interested to know if there is a difference being generated by the r.convertColumnsToState function. With sets that contain objects in particular, Terraform core has difficulties with it's proposed new state algorithm that you might be inadvertently running into by refreshing the config value, even if the updated value should match.

I'd start by looking at that piece of code, but I'd think that removing that refresh of the columns config value should prevent that state drift.

Let me know what you find!

@austinvalle austinvalle added the waiting-response Issues or pull requests waiting for an external response label Jul 19, 2024
@github-actions github-actions bot removed the waiting-response Issues or pull requests waiting for an external response label Jul 19, 2024
@austinvalle austinvalle added the waiting-response Issues or pull requests waiting for an external response label Jul 19, 2024
@mawasile
Copy link
Author

mawasile commented Jan 7, 2025

Hi @austinvalle, sorry for late response on that bug. Some time had passed since I've opened this issue and definitely something change so now my workaround with using tolist instead of toset does not work and the plan is always not empty.

The issue is in the following piece:

 contact_customer_accounts = toset([ //tolist(... has the same issue now
      {
        table_logical_name = "contact",
        data_record_id     = powerplatform_data_record.contact.id
      }
   ])

I've changed the attribute to be optional and computed since you are right it should be as it needs to be refreshed every time.

"columns": schema.DynamicAttribute{
    MarkdownDescription: "Columns of the data record table",
    Optional: true,
    Computed: true,
},

That also means, I can't remove refresh logic for columns attribute in Read method.
Does it mean, that dynamic type does not support collections by design or this is a bug?

my current versions:
github.com/hashicorp/terraform-plugin-framework v1.13.0
terraform version -> Terraform v1.10.2 on linux_amd64

@github-actions github-actions bot removed the waiting-response Issues or pull requests waiting for an external response label Jan 7, 2025
@austinvalle
Copy link
Member

austinvalle commented Jan 10, 2025

Now that I'm looking at the code again, I think I see what the issue could be.

So first a little background on types in Terraform. Since you're using a dynamic configuration attribute, Terraform will decide what the type of the value is. With literals, the following would be determined as a tuple[object]:

# The literal for [ ] will always be a tuple if you're using a dynamic attribute
 contact_customer_accounts = [
      {
        table_logical_name = "contact",
        data_record_id     = powerplatform_data_record.contact.id
      }
   ]

And with the toset / tolist it will be coereced into either a list(object) or set(object)

 contact_customer_accounts = toset([
      {
        table_logical_name = "contact",
        data_record_id     = powerplatform_data_record.contact.id
      }
   ])

The reason this is important, is not only can differing values cause drift, but also differing types. My guess is that this piece of code is causing the "invisible" diff by refreshing the type from set(object) => tuple[object], which the configuration then attempts to change it back tuple[object] => set(object).

My guess is that the Terraform renderer UI doesn't have support for displaying when the values are equivalent, but the types are different (possible feature request?). If what I said above is what's happening, it could look like this:

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # powerplatform_data_record.account will be updated in-place
  ~ resource "powerplatform_data_record" "account" {
        id                 = "830dc891-9b28-ef11-840a-000d3ab60f37"
      - columns            =  (old type) -> (new type)
        # (2 unchanged attributes hidden)
    }

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

The fix for that would be to ensure when you refresh values, that you preserve the original type, which might require a little more complicated logic here. (Some of the pains of working with dynamic data unfortunately)

Let me know if that helps!

@mawasile
Copy link
Author

thanks @austinvalle that did the trick!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 14, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants