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

TypeSet resource wanting to destroy and recreate everything when diff is found #1210

Closed
teddylear opened this issue Jun 12, 2023 · 2 comments
Labels
bug Something isn't working

Comments

@teddylear
Copy link

Main issue is that it appears value of TypeSet below want to destroy and create all the resources in set instead of just updating the one value in the set. Using this as an example, but it has been reported as an issue in other places as well. Is this expected behavior or a bug? If expected, is there another value instead of TypeSet that should be used to avoid issues like this for providers? Or is this a use case for a new type?

SDK version

github.com/hashicorp/terraform-plugin-sdk/v2 v2.26.1

Relevant provider source code

From this file

"inline_policy": {
	Type:     schema.TypeSet,
	Optional: true,
	Computed: true,
	Elem: &schema.Resource{
		Schema: map[string]*schema.Schema{
			"name": {
				Type:     schema.TypeString,
				Optional: true, // semantically required but syntactically optional to allow empty inline_policy
				ValidateFunc: validation.All(
					validation.StringIsNotEmpty,
					validRolePolicyName,
				),
			},
			"policy": {
				Type:                  schema.TypeString,
				Optional:              true, // semantically required but syntactically optional to allow empty inline_policy
				ValidateFunc:          verify.ValidIAMPolicyJSON,
				DiffSuppressFunc:      verify.SuppressEquivalentPolicyDiffs,
				DiffSuppressOnRefresh: true,
				StateFunc: func(v interface{}) string {
					json, _ := verify.LegacyPolicyNormalize(v)
					return json
				},
			},
		},
	},
	DiffSuppressFunc: func(k, _, _ string, d *schema.ResourceData) bool {
		if d.Id() == "" {
			return false
		}

		return !inlinePoliciesActualDiff(d)
	},

```go
...

Terraform Configuration Files

resource "aws_iam_role" "main" {
  assume_role_policy = data.aws_iam_policy_document.trust.json
  name = "terraform-issue-reproduction"

  inline_policy {
    name = "InlinePolicy"
    policy = data.aws_iam_policy_document.inline.json
  }
}

# Inline policy document that we'll be changing
data "aws_iam_policy_document" "inline" {
  statement {
    sid = "S3"
    actions = ["s3:ListBucket"]
    resources = [
      "arn:aws:s3:::some-bucket-a",
      # After the first apply, uncomment this next line
      # "arn:aws:s3:::some-bucket-b",
    ]
  }
}

# Minimal, arbitrary trust statement to make the role valid
data "aws_iam_policy_document" "trust" {
  statement {
    actions = ["sts:AssumeRole"]
    principals {
      type        = "Service"
      identifiers = ["apigateway.amazonaws.com"]
    }
  }
}

Expected Behavior

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:

  # aws_iam_role.main will be updated in-place
  ~ resource "aws_iam_role" "main" {
        id                    = "terraform-issue-reproduction"
        name                  = "terraform-issue-reproduction"
        tags                  = {}
        # (9 unchanged attributes hidden)

      ~ inline_policy {
            name   = "InlinePolicy"
          ~ policy = jsonencode(
              ~ {
                  ~ Statement = [
                      ~ {
                            Action   = "s3:ListBucket"
                            Effect   = "Allow"
                          ~ Resource = [
                                "arn:aws:s3:::some-bucket-a",
                              + "arn:aws:s3:::some-bucket-b",
                            ]
                            Sid      = "S3"
                        },
                    ]
                    Version   = "2012-10-17"
                }
            )
        }
    }

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

Actual Behavior

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:

  # aws_iam_role.main will be updated in-place
  ~ resource "aws_iam_role" "main" {
        id                    = "terraform-issue-reproduction"
        name                  = "terraform-issue-reproduction"
        tags                  = {}
        # (9 unchanged attributes hidden)

      + inline_policy {
          + name   = "InlinePolicy"
          + policy = jsonencode(
                {
                  + Statement = [
                      + {
                          + Action   = "s3:ListBucket"
                          + Effect   = "Allow"
                          + Resource = [
                              + "arn:aws:s3:::some-bucket-b",
                              + "arn:aws:s3:::some-bucket-a",
                            ]
                          + Sid      = "S3"
                        },
                    ]
                  + Version   = "2012-10-17"
                }
            )
        }
      - inline_policy {
          - name   = "InlinePolicy" -> null
          - policy = jsonencode(
                {
                  - Statement = [
                      - {
                          - Action   = "s3:ListBucket"
                          - Effect   = "Allow"
                          - Resource = "arn:aws:s3:::some-bucket-a"
                          - Sid      = "S3"
                        },
                    ]
                  - Version   = "2012-10-17"
                }
            ) -> null
        }
    }

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

Steps to Reproduce

terraform apply
Uncomment the line mentioning some-bucket-b
terraform apply

References

Copy of issue described in this terraform-aws-provider issue here

@teddylear teddylear added the bug Something isn't working label Jun 12, 2023
@bflad
Copy link
Member

bflad commented Jun 13, 2023

Hi @teddylear 👋 Thank you for raising this.

It looks like the similar HashiCorp Discuss topic mostly covered the behavior here from the perspective of Terraform's type system in the post from @maxb:

Since identity within sets is entirely defined by the value of the nested object, any change will be rendered as deletion of the old object, and addition of a new one.

It is worth noting that this is only how Terraform displays the diff. Whether the provider actually implements the change that way, or not, is entirely up to the provider code.

If you wanted Terraform to understand that an old entry and a new entry were related, and should be shown as a change, you need to use a list or map type, instead of a set.

To give some additional color to that, terraform-plugin-sdk has a lot of unfortunate/magic logic with set types while the newer terraform-plugin-framework more directly exposes the entire type system "as-is" from Terraform's perspective to providers. Provider developers may opt to move towards terraform-plugin-framework for use cases such as these to take advantage of the more direct type system mappings or use newer Terraform schema capabilities such as map nested attributes to alleviate plan difference issues, but some of those changes may involve breaking changes for existing configurations.

More recent versions of Terraform (I believe 1.3+) may be able to render set differences slightly better than older versions where some of the set elements can be aligned on configuration values, however the design is inherently limited by how the provider represents the full data of the set element.

The type system handling within terraform-plugin-sdk is relatively in maintenance mode for critical bug fixes while the maintainers are focused on the type system handling within terraform-plugin-framework. Given that and to set expectations that this issue is unlikely to be resolved with any near-term changes in this particular Go module, I'm going to close this issue. Please note though that this practitioner experience report is very valid and it might be best to follow up in other more actionable ways:

  • Working with the provider developers to migrate the affected resource to terraform-plugin-framework to see if the act of migrating to the better aligned type system may help the plan difference output in this case
  • Once using the framework, determining if the schema should be changed to represent the data differently using its enhanced schema capabilities (although please note this generally means a breaking change for practitioners)
  • Once using the framework, determining if a custom type might be more suitable for this use case, for example to create an "unordered list" type that accounts for the name attribute value as a primary key for element alignment
  • Creating a feature request in the Terraform repository to further discuss potential enhancements to the plan difference output, since that is where that logic occurs (although please note that they may first request usage of the framework over this SDK since it more accurately represents data)

Hope this helps and thanks again for writing up this issue to help improve Terraform.

@bflad bflad closed this as not planned Won't fix, can't repro, duplicate, stale Jun 13, 2023
@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 Jul 14, 2023
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