provider/aws: aws_iam_policy_document data source #6881

jen20 · 2016-05-25T23:14:42Z

This brings over the work done by @apparentlymart and @radeksimko in PR #3124, and converts it into a data source for the AWS provider.

It adds a helper to construct IAM policy documents using familiar Terraform concepts. It makes Terraform-style interpolations easier and resolves the syntax conflict between Terraform interpolations and IAM policy variables by changing the latter to use &{...} for its interpolations.

Its use is completely optional and users are free to go on using literal heredocs, file interpolations or whatever else; this just adds another option that fits more naturally into a Terraform config.

Currently the tests are failing, which needs investigation.

apparentlymart · 2016-05-25T23:35:19Z

website/source/docs/providers/aws/d/iam_policy_document.html.markdown

+such as the `aws_iam_policy` resource.
+
+```
+resource "aws_iam_policy_document" "example" {


This should say data instead of resource now. 😀

apparentlymart · 2016-05-25T23:56:14Z

Apart from that docs nit this LGTM, though I suppose I'm kinda reviewing my own code here. 😀

phinze · 2016-05-26T22:25:07Z

Apart from the outstanding docs tweak and test issue, this PR LGTM!

jen20 · 2016-05-27T00:47:14Z

Thanks - I reviewed the code as I ported it and it looked fine. I'll push another commit that more accurately reflects the provenance of this work though since it's almost entirely unaltered from @apparentlymart's original PR! The docs have been updated - I think we should add a giant note at the top about the differing native interpolation syntax though.

jen20 · 2016-05-27T00:47:45Z

There is stil the issue of the failing acceptance test which I believe is exposing a core bug in destroy:

    testing.go:300: Error destroying resource! WARNING: Dangling resources
        may exist. The full state and error is shown below.

        Error: Error applying: 1 error(s) occurred:

        * data.aws_iam_policy_document.test: unknown resource type: aws_iam_policy_document

        State: <no state>

During accpeptance tests of some of the first data sources (see hashicorp#6881 and hashicorp#6911), "unknown resource type" errors were coming up. It looks like DataSourcesMap was not being taken into account in *schema.Provider.Apply(), causing this error. Also, added a skip in *schema.Resource.Apply() if *schema.Resource.Destroy() is not defined, as data sources do not define this, fixing the first error resulted in a nil pointer dereference.

During accpeptance tests of some of the first data sources (see hashicorp#6881 and hashicorp#6911), "unknown resource type" errors have been coming up. Traced it down to the ResourceCountTransformer, which transforms destroy nodes to a graphNodeExpandedResourceDestroy node. This node's EvalTree() was still indiscriminately using EvalApply for all resource types, versus EvalReadDataApply. This accounts for both cases via EvalIf.

vancluever · 2016-05-30T00:08:35Z

Just mentioned one thing in #4278 and I think I see one more too:

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.
~~Looks like some elements might be misspelled - at least according to the AWS docs and what I've always used (ie: Actions versus Action) I'll annotate the code.~~ Scratch that - forgot to look at the struct tags :P

Looking forward to this one too as it's one less branch I need to maintain on my side!

vancluever · 2016-05-30T06:07:04Z

Just gave this a whirl, gist below has the diff from the S3 bucket policy:

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

Few things probably need to be done here:

omitempty needs to be removed from Sid
We have to sort arrays, in reverse alphabetical order
Anything that takes a list value needs to be able to take a string value as well, which will be used in place of single element lists.

vancluever · 2016-05-30T16:31:52Z

Hey guys, here's what I got for now:

paybyphone@7d4c122

Still gotta work on fixing the tests, will probably grab a couple from the policy generator and toss them in.

vancluever · 2016-05-30T17:16:28Z

Hmm, okay, the policy generator is generating policies that are not what the API would give back either. I'm just going to adjust the test, maybe if we get some people trying this out and send in any feedback if they get a diff that would be good.

vancluever · 2016-05-30T17:49:30Z

Updated the commit with the updated test. Removed the blank element from condition, not too sure if that's a condition that we really need to handle, hopefully people won't be putting blank values in their document, however I'm not too sure if this might come up in interpolation?

@apparentlymart

This brings over the work done by @apparentlymart and @radeksimko in PR #3124, and converts it into a data source for the AWS provider: This commit adds a helper to construct IAM policy documents using familiar Terraform concepts. It makes Terraform-style interpolations easier and resolves the syntax conflict between Terraform interpolations and IAM policy variables by changing the latter to use &{...} for its interpolations. Its use is completely optional and users are free to go on using literal heredocs, file interpolations or whatever else; this just adds another option that fits more naturally into a Terraform config.

phinze · 2016-05-31T16:15:56Z

@jen20 If you drop the [WIP] and comment w/ the passing test this LGTM!

jen20 · 2016-05-31T17:51:02Z

Tests now passing:

make testacc TEST=./builtin/providers/aws TESTARGS="-run TestAccAWSIAMPolicyDocument"
==> Checking that code complies with gofmt requirements...
/Users/James/Code/go/bin/stringer
go generate $(go list ./... | grep -v /vendor/)
2016/05/31 12:49:58 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run TestAccAWSIAMPolicyDocument -timeout 120m
=== RUN   TestAccAWSIAMPolicyDocument
--- PASS: TestAccAWSIAMPolicyDocument (6.40s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    6.416s

ghost · 2020-04-25T02:18:07Z

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.

apparentlymart reviewed May 25, 2016
View reviewed changes

jen20 added enhancement provider/aws labels May 27, 2016

jen20 force-pushed the iam-policy-datasource branch from 7b1aa59 to 2c32037 Compare May 27, 2016 00:33

vancluever mentioned this pull request May 28, 2016

provider/aws: aws_ami_description - data source edition (now aws_ami) #6911

Merged

vancluever mentioned this pull request May 28, 2016

[WIP] core: Ensure that DataSourcesMap is handled during apply w/destroy #6913

Closed

vancluever mentioned this pull request May 29, 2016

aws_s3_bucket - Fix policy normalization to match AWS API #4278

Closed

jen20 force-pushed the iam-policy-datasource branch from 2c32037 to c91d62f Compare May 31, 2016 16:08

jen20 changed the title ~~[WIP]: provider/aws: aws_iam_policy_document data source~~ provider/aws: aws_iam_policy_document data source May 31, 2016

jen20 merged commit 50a32b5 into master May 31, 2016

jen20 deleted the iam-policy-datasource branch May 31, 2016 17:51

jen20 mentioned this pull request May 31, 2016

aws_iam_policy_document logical resource #3124

Closed

3 tasks

vancluever mentioned this pull request May 31, 2016

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

Merged

blackjid mentioned this pull request Jul 5, 2016

aws_iam_policy_document data source diff with single element arrays #7495

Closed

ghost locked and limited conversation to collaborators Apr 25, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

provider/aws: aws_iam_policy_document data source #6881

provider/aws: aws_iam_policy_document data source #6881

jen20 commented May 25, 2016

apparentlymart May 25, 2016

apparentlymart commented May 25, 2016

phinze commented May 26, 2016

jen20 commented May 27, 2016

jen20 commented May 27, 2016

vancluever commented May 30, 2016 •

edited

Loading

vancluever commented May 30, 2016

vancluever commented May 30, 2016 •

edited

Loading

vancluever commented May 30, 2016

vancluever commented May 30, 2016

phinze commented May 31, 2016

jen20 commented May 31, 2016

ghost commented Apr 25, 2020

provider/aws: aws_iam_policy_document data source #6881

provider/aws: aws_iam_policy_document data source #6881

Conversation

jen20 commented May 25, 2016

apparentlymart May 25, 2016

Choose a reason for hiding this comment

apparentlymart commented May 25, 2016

phinze commented May 26, 2016

jen20 commented May 27, 2016

jen20 commented May 27, 2016

vancluever commented May 30, 2016 • edited Loading

vancluever commented May 30, 2016

vancluever commented May 30, 2016 • edited Loading

vancluever commented May 30, 2016

vancluever commented May 30, 2016

phinze commented May 31, 2016

jen20 commented May 31, 2016

ghost commented Apr 25, 2020

vancluever commented May 30, 2016 •

edited

Loading

vancluever commented May 30, 2016 •

edited

Loading