DiffSuppressFunc should be used for "Objects have changed outside of Terraform" logic

[NiklasWagner]

Terraform Version

Terraform v1.0.5
on linux_amd64
+ provider registry.terraform.io/hashicorp/aws v3.55.0

Terraform Configuration Files

provider "aws" {
  region  = "eu-west-1"
  profile = "xpay"
}

data "aws_caller_identity" "this" {}

data "aws_iam_policy_document" "allow_ecr_read" {
  statement {
    effect = "Allow"

    principals {
      type = "AWS"
      identifiers = [
        "arn:aws:iam::${data.aws_caller_identity.this.account_id}:root",
        # the account ids must be valid, otherwise terraform/aws will time out
        "arn:aws:iam::173546646657:root",
        "arn:aws:iam::733016082947:root",
        "arn:aws:iam::159609169035:root",
      ]
    }

    actions = [
      "ecr:DescribeRepositories",
      "ecr:ListImages",
      "ecr:DescribeImages",
    ]
  }
}

resource "aws_ecr_repository" "this" {
  name = "aaa2-ecr-bug"
}

resource "aws_ecr_repository_policy" "this" {
  repository = aws_ecr_repository.this.name
  policy     = data.aws_iam_policy_document.allow_ecr_read.json
}

Debug Output

https://gist.github.com/NiklasWagner/b48d8ccbc30118d2d0aac1cd89a1f00a

Crash Output

Expected Behavior

Running terraform apply multiple times should not show any "Objects have changed outside of Terraform" warnings.

The DiffSuppressFunc should be used before the planing phase(?).

Actual Behavior

Running terraform apply multiple times results in "Objects have changed outside of Terraform" warnings. This is due to the AWS API responding with the policy document in different order each time.

The Terraform AWS provider uses a DiffSuppressFunc which already handles the random order issue correctly by using a hashmap.

Saidly (it looks like that) the DiffSuppressFunc is not used when comparing resources within the "Objects have changed outside of Terraform" logic.

$ t apply
aws_ecr_repository.this: Refreshing state... [id=aaa2-ecr-bug]
aws_ecr_repository_policy.this: Refreshing state... [id=aaa2-ecr-bug]

Note: Objects have changed outside of Terraform

Terraform detected the following changes made outside of Terraform since the last "terraform apply":

  # aws_ecr_repository_policy.this has been changed
  ~ resource "aws_ecr_repository_policy" "this" {
        id          = "aaa2-ecr-bug"
      ~ policy      = jsonencode(
          ~ {
              ~ Statement = [
                  ~ {
                      ~ Principal = {
                          ~ AWS = [
                              + "arn:aws:iam::733016082947:root",
                                "arn:aws:iam::630737231557:root",
                              - "arn:aws:iam::173546646657:root",
                                "arn:aws:iam::159609169035:root",
                              - "arn:aws:iam::733016082947:root",
                              + "arn:aws:iam::173546646657:root",
                            ]
                        }
                        # (3 unchanged elements hidden)
                    },
                ]
                # (1 unchanged element hidden)
            }
        )
        # (2 unchanged attributes hidden)
    }

Unless you have made equivalent changes to your configuration, or ignored the relevant attributes using ignore_changes, the following plan may include actions to undo or respond to these changes.

──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────

No changes. Your infrastructure matches the configuration.

Your configuration already matches the changes detected above. If you'd like to update the Terraform state to match, create and apply a refresh-only plan:
  terraform apply -refresh-only

Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

Steps to Reproduce

1. terraform init
2. terraform apply (should create the resources)
3. terraform apply (should show a diff in the Pricipal ARN list, if not run multiple times)

Additional Context

References

Multiple resources are affected in the AWS provider. All with the same policy document issue in different resources:

• iam_role inline_policy: Issue importing iam role with inline policies wanting to be deleted and recreated when there is no difference terraform-provider-aws#19444
• Terraform detects spurious bucket policy changes before each apply after upgrading to 0.15.4 terraform-provider-aws#19627
• Unhelpful "changes made outside of terraform" due to AWS vs tf representation in 1.0 terraform-provider-aws#20107
• Inconsistent ordering of values in codeartifact_repository_permissions_policy terraform-provider-aws#17024
• s3 bucket policy with not_principal always changes due to ordering terraform-provider-aws#13144
• S3 bucket policy condition block displays endless diff with values array of length 1 terraform-provider-aws#20456
• aws_ecr_repository_policy tries to enforce order of principals when AWS insists on a different random ordering. terraform-provider-aws#19365

[apparentlymart]

Hi @NiklasWagner! Thanks for opening this issue.

What you've described here is a Terraform SDK behavior rather than a Terraform Core behavior, so I've transferred this issue into the Terraform SDK repository.

While I'm not on the team responsible for the SDK, I do want to note here that the current SDK behaviors (warts and all) are generally frozen at this point due to how many providers are relying on them, and so I expect this issue is more likely to be addressed by the new provider development framework, which is not constrained by remaining compatible with accidental misbehaviors in existing providers. (There's so much provider code out there at this point that we've found that every bug and design oversight is inevitably being relied upon by someone, somewhere.)

There's actually already hashicorp/terraform-plugin-framework#70 tracking the new-framework version of this same request, so you might find it interesting to track that one to see how that design discussion develops.

[NiklasWagner]

Hi @apparentlymart

thanks for moving my issue to the correct place. Its understandable that the behaviour of the existing SDK is probably not going to change but its great to see that Hashicorp is already working on a new framework and already have a existing issue basically covering my report :)

Thanks for the quick reply, looking forward to have this design issue fixed at some point.

[bflad]

The next version of terraform-plugin-sdk/v2, unreleased at the moment, but slated to be v2.11.0 adds an additional DiffSuppressOnRefresh field to helper/schema.Schema. When enabled, this checks the DiffSuppressFunc during refresh. If the planned value matches the old state value via the DiffSuppressFunc, the old value will be preserved (or put another way, the planned value will be ignored). This should help mitigate cases like these where the DiffSuppressFunc implementations were likely put in place with the expectation that this would already happen in the SDK, however it previously did not.

We expect DiffSuppressOnRefresh to be generally safe to apply in most cases where DiffSuppressFunc is already being used, however it is not enabled by default for backwards compatibility.

[github-actions]

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.