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

aws_iam_policy_document data source diff with single element arrays #7495

Closed
blackjid opened this issue Jul 5, 2016 · 5 comments · Fixed by #8675
Closed

aws_iam_policy_document data source diff with single element arrays #7495

blackjid opened this issue Jul 5, 2016 · 5 comments · Fixed by #8675

Comments

@blackjid
Copy link

blackjid commented Jul 5, 2016

Terraform Version

Terraform v0.7.0-rc2 (46a0709)

Affected Resource(s)

  • aws_iam_policy_document

Terraform Configuration Files

data "aws_iam_policy_document" "key_store_kms" {
  statement {
    actions = [
      "kms:*"
    ]
    resources = [
      "*"
    ]
    effect = "Allow"
    principals {
      type = "AWS"
      identifiers = ["arn:aws:iam::0958430598340:root"]
    }
  }
}

resource "aws_kms_key" "key_store" {
  description = "Protects secrets stored in S3"
  policy = "${data.aws_iam_policy_document.key_store_kms.json}"
}

Expected Behavior

The policy shouldn't be marked to be modified in the plan phase. All the arguments that received a [] with only one element, should be rendered as a string.

Actual Behavior

The policy is marked to be modified every time I run terraform plan and apply.

Steps to Reproduce

Please list the steps required to reproduce the issue, for example:

  1. terraform plan
  2. terraform apply
  3. terraform plan

References

  • GH-1234
    The is a mention about the problem but it's hidden in a merge PR.

    There needs to be some logic to ensure that array items like Actions get converted to strings instead of []strings if there is only one element (this was one of the things that was causing recurring diffs in unchanged policies). So those elements probably need to be interface{} and then handled accordingly.

@apparentlymart
Copy link
Contributor

Hi @blackjid. Sorry for the troubles here.

As you noted, this is likely due to the fact that lots of AWS APIs will normalize IAM policies they return, which can cause them to appear differently than is rendered by this resource. It would be good to do an audit of various different AWS services that deal with IAM policies and see what normalizations are consistent and which are different.

I expect we can encode certain normalizations directly in this data source, like the one you mentioned of not using arrays with only one element. However, I suspect there will be cases where the exact normalization differs slightly from one AWS API to the next so certain resources may need to do some additional normalization work to adapt to these specifics.

@blackjid
Copy link
Author

blackjid commented Jul 5, 2016

Thanks for the explanations @apparentlymart.

The tests I did were in the using the policy in the aws_kms_key resource. I've edited the issue adding that info

@dtolnay
Copy link
Contributor

dtolnay commented Jul 23, 2016

I have a fix in #7785.

@joshuaspence
Copy link
Contributor

+1

stack72 added a commit that referenced this issue Sep 6, 2016
Fixes #7495

```
% make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSKmsKey_policy'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/09/06 10:44:20 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSKmsKey_policy
-timeout 120m
=== RUN   TestAccAWSKmsKey_importBasic
--- PASS: TestAccAWSKmsKey_importBasic (166.19s)
=== RUN   TestAccAWSKmsKey_basic
--- PASS: TestAccAWSKmsKey_basic (215.33s)
=== RUN   TestAccAWSKmsKey_policy
--- PASS: TestAccAWSKmsKey_policy (116.81s)
=== RUN   TestAccAWSKmsKey_isEnabled
--- PASS: TestAccAWSKmsKey_isEnabled (1008.31s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws
1689.957s
```
stack72 added a commit that referenced this issue Sep 12, 2016
Fixes #7495

```
% make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSKmsKey_policy'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/09/06 10:44:20 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSKmsKey_policy
-timeout 120m
=== RUN   TestAccAWSKmsKey_importBasic
--- PASS: TestAccAWSKmsKey_importBasic (166.19s)
=== RUN   TestAccAWSKmsKey_basic
--- PASS: TestAccAWSKmsKey_basic (215.33s)
=== RUN   TestAccAWSKmsKey_policy
--- PASS: TestAccAWSKmsKey_policy (116.81s)
=== RUN   TestAccAWSKmsKey_isEnabled
--- PASS: TestAccAWSKmsKey_isEnabled (1008.31s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws
1689.957s
```
@ghost
Copy link

ghost commented Apr 22, 2020

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.

@ghost ghost locked and limited conversation to collaborators Apr 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.