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

date source referencing managed resource proposes unnecessary changes under 0.14 #27171

Closed
jrobison-sb opened this issue Dec 7, 2020 · 3 comments
Assignees
Labels
bug confirmed a Terraform Core team member has reproduced this issue explained a Terraform Core team member has described the root cause of this issue in code v0.14 Issues (primarily bugs) reported against v0.14 releases

Comments

@jrobison-sb
Copy link

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Terraform CLI and Terraform AWS Provider Version

$ terraform -v
Terraform v0.14.0
+ provider registry.terraform.io/hashicorp/aws v3.20.0

Affected Resource(s)

  • data.aws_iam_policy_document
  • aws_iam_policy

Terraform Configuration Files

resource "aws_s3_bucket" "b" {
  bucket_prefix = "terraform-0-14-test-"
  acl           = "private"
  # server_side_encryption_configuration {
  #   rule {
  #     apply_server_side_encryption_by_default {
  #       sse_algorithm = "aws:kms"
  #     }
  #   }
  # }
}

data "aws_iam_policy_document" "b" {
  statement {
    effect    = "Deny"
    actions   = ["s3:GetObject"]
    resources = ["${aws_s3_bucket.b.arn}/*"]
  }
}

resource "aws_iam_policy" "policy" {
  name        = "terraform_014_test"
  description = "My test policy"
  policy      = data.aws_iam_policy_document.b.json
}

Expected Behavior

terraform plan should show diffs in this IAM policy only when the ARN of this bucket changes, and in 0.13.5 it works as expected.

Actual Behavior

But in 0.14.0 plan diffs show that the IAM policy will be changed even when the ARN of the bucket does not change.

This will be easiest to explain in the Steps to Reproduce section.

Steps to Reproduce

  1. Use 0.13.5 and apply the above infrastructure, with S3 encryption commented out. It will apply as expected.
  2. Uncomment the S3 encryption and run a plan using 0.13.5, it will show that only one resource (the bucket) is going to change. Which is the expected behavior.
  # aws_s3_bucket.b will be updated in-place
  ~ resource "aws_s3_bucket" "b" {
        acl                         = "private"
        arn                         = "arn:aws:s3:::terraform-0-14-test-20201204174955070900000001"
        bucket                      = "terraform-0-14-test-20201204174955070900000001"
        bucket_domain_name          = "terraform-0-14-test-20201204174955070900000001.s3.amazonaws.com"
        bucket_prefix               = "terraform-0-14-test-"
        bucket_regional_domain_name = "terraform-0-14-test-20201204174955070900000001.s3.amazonaws.com"
        force_destroy               = false
        hosted_zone_id              = "Z3AQBSTGFYJSTF"
        id                          = "terraform-0-14-test-20201204174955070900000001"
        region                      = "us-east-1"
        request_payer               = "BucketOwner"
        tags                        = {}

      + server_side_encryption_configuration {
          + rule {
              + apply_server_side_encryption_by_default {
                  + sse_algorithm = "aws:kms"
                }
            }
        }

        versioning {
            enabled    = false
            mfa_delete = false
        }
    }

Plan: 0 to add, 1 to change, 0 to destroy.
  1. Comment out the S3 encryption again and terraform destroy.
  2. Install 0.14.0 and init to get set up.
  3. Run an apply with the S3 encryption commented out, using 0.14.0. It will apply as expected.
  4. Uncomment the S3 encryption and run a plan using 0.14.0. It will report that the IAM policy is going to change, even though the bucket is being updated in-place and the bucket ARN isn't going to change.
Terraform will perform the following actions:

  # data.aws_iam_policy_document.b will be read during apply
  # (config refers to values not yet known)
 <= data "aws_iam_policy_document" "b"  {
      ~ id      = "900537039" -> (known after apply)
      ~ json    = jsonencode(
            {
              - Statement = [
                  - {
                      - Action   = "s3:GetObject"
                      - Effect   = "Deny"
                      - Resource = "arn:aws:s3:::terraform-0-14-test-20201204175506660300000001/*"
                      - Sid      = ""
                    },
                ]
              - Version   = "2012-10-17"
            }
        ) -> (known after apply)
      - version = "2012-10-17" -> null

      ~ statement {
          - not_actions   = [] -> null
          - not_resources = [] -> null
            # (3 unchanged attributes hidden)
        }
    }

  # aws_iam_policy.policy will be updated in-place
  ~ resource "aws_iam_policy" "policy" {
        id          = "arn:aws:iam::1234567890:policy/terraform_014_test"
        name        = "terraform_014_test"
      ~ policy      = jsonencode(
            {
              - Statement = [
                  - {
                      - Action   = "s3:GetObject"
                      - Effect   = "Deny"
                      - Resource = "arn:aws:s3:::terraform-0-14-test-20201204175506660300000001/*"
                      - Sid      = ""
                    },
                ]
              - Version   = "2012-10-17"
            }
        ) -> (known after apply)
        # (3 unchanged attributes hidden)
    }

  # aws_s3_bucket.b will be updated in-place
  ~ resource "aws_s3_bucket" "b" {
        id                          = "terraform-0-14-test-20201204175506660300000001"
        tags                        = {}
        # (10 unchanged attributes hidden)

      + server_side_encryption_configuration {
          + rule {
              + apply_server_side_encryption_by_default {
                  + sse_algorithm = "aws:kms"
                }
            }
        }

        # (1 unchanged block hidden)
    }

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

The bucket will be updated in-place and it's ARN isn't going to change. So 0.14.0 should report no change to the IAM policy, the same way that 0.13.5 reports no change to the IAM policy.

Other factoids

This behavior isn't specific to S3 either. I was able to reproduce the same thing with an in-place change to a aws_codepipeline resource, for example, and data.aws_iam_policy_document behaved similarly to the above when it referenced the ARN of that pipeline. The S3 case is just the simplest way to reproduce this with the least amount of moving parts.

Initially I opened this issue on the AWS provider. But since the behavior that I'm seeing differs between 0.13 and 0.14, it was suggested in hashicorp/terraform-provider-aws#16598 (comment) that this issue be moved over to the Terraform repo.

References

hashicorp/terraform-provider-aws#16598

@jbardin
Copy link
Member

jbardin commented Dec 7, 2020

Thanks for the extensive example @jrobison-sb! The data source not being able to be read during plan does make the plan far less useful, though as things update during apply, the unknown values should resolve and cause no change to the policy. Not being able to determine this from the plan however is not optimal.

This appears to be an unfortunate side effect of the resolution to #25961. Due to the amount of legacy configuration that relied on the behavior of data sources behaving the way they did, we chose to recreate that behavior in the new plan code. The reason it appears to not work in the current version, is that the often used but unintended behavior was only reliable on the first apply, and could read the incorrect data on subsequent refresh+plan cycles. Correcting that possibility leads to data sources sometimes showing more changes than they did previously.

What happens now is referencing a managed resource from a data source creates an implied depends_on dependency, so that any change in the referenced resource forces the data source to be deferred until apply. Because this is treated like depends_on, it means it should act only on a direct reference to another managed resource. What this also means is that you can work around the behavior by making the reference indirectly.

I believe the following should make the data source plan as expected:

resource "aws_s3_bucket" "b" {
  bucket_prefix = "terraform-0-14-test-"
  acl           = "private"
  server_side_encryption_configuration {
    rule {
      apply_server_side_encryption_by_default {
        sse_algorithm = "aws:kms"
      }
    }
  }
}

locals {
  arn = aws_s3_bucket.b.arn
}

data "aws_iam_policy_document" "b" {
  statement {
    effect    = "Deny"
    actions   = ["s3:GetObject"]
    resources = ["${local.arn}/*"]
  }
}

resource "aws_iam_policy" "policy" {
  name        = "terraform_014_test"
  description = "My test policy"
  policy      = data.aws_iam_policy_document.b.json
}

I need to test some more, but I think these "implied depends_on" references are reaching out too far in some cases. While that doesn't effect this particular example, I'm going to look into that further and make sure the effect is as limited as it was intended to be.

@jbardin jbardin added confirmed a Terraform Core team member has reproduced this issue explained a Terraform Core team member has described the root cause of this issue in code and removed new new issue not yet triaged labels Dec 7, 2020
@apparentlymart apparentlymart added the v0.14 Issues (primarily bugs) reported against v0.14 releases label Dec 8, 2020
@jbardin jbardin self-assigned this Dec 8, 2020
@jbardin jbardin changed the title data.aws_iam_policy_document proposes unnecessary changes under 0.14 date source referencing managed resource proposes unnecessary changes under 0.14 Dec 11, 2020
@jbardin
Copy link
Member

jbardin commented Dec 11, 2020

With the added note in the docs I'm going to close this for now.
The behavior was intentional in order to remain compatible with a large amount of existing code, so while it may not be the most intuitive, it is something that is expected right now.

@ghost
Copy link

ghost commented Jan 11, 2021

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.

@hashicorp hashicorp locked as resolved and limited conversation to collaborators Jan 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug confirmed a Terraform Core team member has reproduced this issue explained a Terraform Core team member has described the root cause of this issue in code v0.14 Issues (primarily bugs) reported against v0.14 releases
Projects
None yet
Development

No branches or pull requests

3 participants