provider/aws: Fixing IAM data source policy generation to prevent spurious diffs

[vancluever]

Just re-submitting this as a new PR, this was "submitted" in #6881.

As mentioned there, there are a number of things AWS likes to do with a policy (namely resource policies versus IAM ones), that cause spurious diffs in Terraform even though nothing has changed, ie: sorting arrays, inserting an empty Sid attribute, and converting single-element lists to a string value.

Having the data source affords us the opportunity to ensure that these problems do not persist as we can now tweak the policy generation in one place.

Again, an example of the issue in action:

https://gist.github.com/vancluever/526e9c1d8153436e58244847b8fb20f1

The changes in this commit fixes things to allow a policy generated via the aws_iam_policy_document data source to properly pass a TF plan/apply without any changes when nothing has indeed changed.

References for this: I list a few here, however if you search in the issue backlog there are more:

S3: #6952
SNS: #6909
S3, discussing ordering: #5978
ElasticSearch: #5067
My original PR: #4278

[stack72]

Hi @vancluever

Apologies for not getting back to this sooner, please can you rebase this from master and then we can get it tested and merged?

Thanks

Paul

[vancluever]

Woohoo! Will do @stack72!

[vancluever]

@stack72 - rebase complete and ready for review!

[stack72]

Hi @vancluever

This LGTM! Thanks :)

% make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSIAMPolicyDocument'
==> Checking that code complies with gofmt requirements...
/Users/stacko/Code/go/bin/stringer
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/08/10 12:05:04 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSIAMPolicyDocument -timeout 120m
=== RUN   TestAccAWSIAMPolicyDocument
--- PASS: TestAccAWSIAMPolicyDocument (17.99s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    18.022s

P.

[vancluever]

Yay! Thanks @stack72!

[dtolnay]

@stack72 @apparentlymart This change forces all code that deals with policy documents to copy this switch statement all over the place, as seen multiple times in the PR:

switch v := in.(type) {
case string:
    // handle string
case []string:
    // handle slice
default:
    // panic
 }

I think #7785 is a strictly better fix and I think this one should be reverted.

[ghost]

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.