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

New Guide: AWS IAM Policy Documents #6016

Merged
merged 2 commits into from
Oct 5, 2018
Merged

Conversation

bflad
Copy link
Contributor

@bflad bflad commented Sep 27, 2018

We have fielded quite a few issues and documentation requests surrounding IAM policy document building in the past. This guide is designed to highlight some examples how Terraform and the AWS provider allow for many configuration styles to implement these policies. We then can cross reference this guide in all documentation where IAM policy documents are used.

Closes #5984

Changes proposed in this pull request:

  • New Guide: AWS IAM Policy Documents
  • Link to AWS IAM Policy Documents guide from various resources that require building these documents

Output from acceptance testing: N/A

@bflad bflad added documentation Introduces or discusses updates to documentation. provider Pertains to the provider itself, rather than any interaction with AWS. hashibot/ignore labels Sep 27, 2018
@bflad bflad requested a review from a team September 27, 2018 17:25
@ghost ghost added size/L Managed by automation to categorize the size of a PR. size/XL Managed by automation to categorize the size of a PR. and removed size/L Managed by automation to categorize the size of a PR. labels Sep 27, 2018
The IAM policy document format allows context variables to be interpolated
into various strings within a statement. The native IAM policy document format
uses `${...}`-style syntax that is in conflict with Terraform's interpolation
syntax, so this data source instead uses `&{...}` syntax for interpolations that
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This suggests using &{...} syntax but L17 of the guide suggests using $${...}. Which one is officially recommended? It should probably be consistent.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rifelpet outside the aws_iam_policy_document data source, the $$ syntax must be used for escaping, but it does also work within the data source too.

Given this example configuration:

terraform {
  required_version = "0.11.8"
}

provider "aws" {
  region  = "us-east-1"
  version = "1.38.0"
}

data "aws_iam_policy_document" "example" {
  statement {
    actions   = ["s3:GetObject"]
    resources = ["arn:aws:s3:::$${aws:username}/*"]
  }
}

resource "aws_iam_policy" "example" {
  name_prefix = "escaped-iam-variable-example"
  policy      = "${data.aws_iam_policy_document.example.json}"
}

output "example" {
  value = "${aws_iam_policy.example.policy}"
}
$ terraform apply
...
aws_iam_policy.example: Creating...
  arn:         "" => "<computed>"
  name:        "" => "<computed>"
  name_prefix: "" => "escaped-iam-variable-example"
  path:        "" => "/"
  policy:      "" => "{\n  \"Version\": \"2012-10-17\",\n  \"Statement\": [\n    {\n      \"Sid\": \"\",\n      \"Effect\": \"Allow\",\n      \"Action\": \"s3:GetObject\",\n      \"Resource\": \"arn:aws:s3:::${aws:username}/*\"\n    }\n  ]\n}"
aws_iam_policy.example: Creation complete after 1s (ID: arn:aws:iam::--OMITTED--:policy/escape...able-example20181001201640491400000001)

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

Outputs:

example = {
  "Version": "2012-10-17",
  "Statement": [
    {
      "Sid": "",
      "Effect": "Allow",
      "Action": "s3:GetObject",
      "Resource": "arn:aws:s3:::${aws:username}/*"
    }
  ]
}

And verified via the AWS CLI:

$ aws iam get-policy-version --policy arn:aws:iam::--OMITTED--:policy/escaped-iam-variable-example20181001201640491400000001 --version-id v1
{
    "PolicyVersion": {
        "Document": {
            "Version": "2012-10-17",
            "Statement": [
                {
                    "Sid": "",
                    "Effect": "Allow",
                    "Action": "s3:GetObject",
                    "Resource": "arn:aws:s3:::${aws:username}/*"
                }
            ]
        },
        "VersionId": "v1",
        "IsDefaultVersion": true,
        "CreateDate": "2018-10-01T20:16:40Z"
    }
}

I am not sure if there were additional reasons for the original implementation of &{...} syntax though. It should definitely still be noted in the data source documentation that the data source supports that syntax until if/when its removed. If you have suggestions for the verbiage (its just copy paste of the existing documentation), please let us know.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the &{...} format originated with the datasource itself two years ago. As a TF user I think it would be good to settle on one of the two formats, though I agree mentioning the alternative format would be beneficial for users trying to understand an existing aws_iam_policy_document that may use the alternative format. I'm not sure if its worth deprecating one entirely, but it's something to consider.

Copy link
Member

@YakDriver YakDriver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a very useful guide!

document will overwrite statements with the same `sid` in the current document.
Statements without an `sid` cannot be overwritten. Can be combined with
`source_json`.
* `statement` (Required) - A nested configuration block (described below)
Copy link
Member

@YakDriver YakDriver Oct 3, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be very helpful to make statement optional so that policies can be layered more flexibly. Even if statement policy docs provide the foundational level, when layering policies, you need the capability to just compose policies without needing a new statement. To get around this, you have to specify a dummy statement that will be purposely overridden, when all you really want is the source_json and override_json. See PR #6052.

website/docs/d/iam_policy_document.html.markdown Outdated Show resolved Hide resolved
@ghost ghost added size/L Managed by automation to categorize the size of a PR. and removed size/XL Managed by automation to categorize the size of a PR. labels Oct 5, 2018
This guide is designed to highlight some examples how Terraform and the AWS provider allow for many configuration styles to implement these policies. We then can cross reference this guide in all documentation where IAM policy documents are used.
@bflad bflad force-pushed the d-iam-policy-documents-guide branch from 56dbd52 to aa49feb Compare October 5, 2018 02:39
Copy link
Member

@radeksimko radeksimko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a handful of nitpicks, mostly unimportant.
:shipit:

- Policy layering - create policy documents that combine and/or overwrite other policy documents
- Built-in policy error checking

Otherwise in simple cases, such as a staticly defined assume role policy for an IAM role, Terraform's [multiple line heredoc syntax](#multiple-line-heredoc-syntax) allows the easiest formatting without any indirection of a separate data source configuration or file.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

staticly -> statically

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also it may be worth linking to a document/docs page which explains what is "assume role policy".
AFAIK it often causes some confusion for IAM beginners as they mismatch that with the actual resource policy.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


Otherwise in simple cases, such as a staticly defined assume role policy for an IAM role, Terraform's [multiple line heredoc syntax](#multiple-line-heredoc-syntax) allows the easiest formatting without any indirection of a separate data source configuration or file.

Additional methods are possible, such [single line string syntax](#single-line-string-syntax), the [file() interpolation function](#file-interpolation-function), and the [template_file data source](#template_file-data-source), however their usage is discouraged due to their complexity.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are possible -> are available? Admittedly I'm not a native english speaker, but "available methods" sounds a bit more natural compared to "possible methods". Feel free to ignore this though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 that's a good catch


### Multiple Line Heredoc Syntax

Interpolation is available within the heredoc string if necessary.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we link to any doc page which explains HEREDOC? I'm not sure if we have such page in our docs 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out the 0.12 branch has a whole section on it! https://github.com/hashicorp/terraform/blob/v0.12-dev/website/docs/configuration/expressions.html.md#string-literals

Unfortunately the current documentation has it listed as just a bullet point under syntax 😅 https://www.terraform.io/docs/configuration/syntax.html (which will be removed with 0.12 documentation updates)

Not sure if we link to the soon-to-be removed place or somewhere else. Open to opinions!


To decouple the IAM policy JSON from the Terraform configuration, Terraform has a built-in [`file()` interpolation function](/docs/configuration/interpolation.html#file-path-), which can read the contents of a local file into the configuration. Interpolation is _not_ available when using the `file()` function by itself.

For example, in a file called `policy.json`:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We seem to be missing a verb here ^ 👓


To enable interpolation in decoupled files, the [`template_file` data source](/docs/providers/template/d/file.html) is available.

For example, in a file called `policy.json.tpl`:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We seem to be missing a verb here ^ 👓

* Link to IAM assume role policy documentation
* Minor verbiage adjustments
@bflad bflad added this to the v1.40.0 milestone Oct 5, 2018
@bflad bflad merged commit 511700e into master Oct 5, 2018
@bflad bflad deleted the d-iam-policy-documents-guide branch October 5, 2018 13:44
bflad added a commit that referenced this pull request Oct 10, 2018
@github-actions
Copy link

I'm going to lock this pull request 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 related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Introduces or discusses updates to documentation. provider Pertains to the provider itself, rather than any interaction with AWS. size/L Managed by automation to categorize the size of a PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants