Terraform-provider-aws: better AWS policy layering

[tvald]

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 "me too" comments, 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

Description

2890 resolved #2672 with a source_json and override_json field which enables layering of AWS policies. We leverage this mechanism to merge S3 bucket policies from a library of policies, since a bucket can only have one policy (cf #409).

Unfortunately, only two policies can be combined at a time (one each as source_json and override_json), leading to multi-step merges with temporary intermediate policies. You also have to know the name of a statement in the override_json policy in order to insert a dummy statement field, since the statement field is required even if you're combining other policies.

I propose:
1) make the statement field optional
2) change source_json and override_json to accept arrays, where priority for duplicate statements is given to later elements in the array

This makes it easy to merge any number of policies.

Affected Resource

• aws_iam_policy_document

Potential Terraform Configuration

Current:

data "aws_iam_policy_document" "tmp_merge" {
  source_json = "${data.aws_iam_policy_document.DenyIncorrectEncryptionHeader.json}"
  override_json = "${data.aws_iam_policy_document.DenyUnencryptedObjectUploads.json}"
  statement {
    sid = "DenyUnencryptedObjectUploads" # will be overridden
  }
}

data "aws_iam_policy_document" "secure_bucket" {
  policy_id = "SecureBucketPolicy"
  source_json = "${data.aws_iam_policy_document.tmp_merge.json}"
  override_json = "${data.aws_iam_policy_document.DenyUnencryptedConnections.json}"
  statement {
    sid = "DenyUnencryptedConnections" # will be overridden
  }
}

Proposed:

data "aws_iam_policy_document" "secure_bucket" {
  policy_id = "SecureBucketPolicy"
  source_json = [
    "${data.aws_iam_policy_document.DenyIncorrectEncryptionHeader.json}",
    "${data.aws_iam_policy_document.DenyUnencryptedObjectUploads.json}",
    "${data.aws_iam_policy_document.DenyUnencryptedConnections.json}",
  }
}

References

• #2672
• #2890
• #409

[lorengordon]

I'd like to see this. statement should definitely be optional now. Though, it seemed when I tried to use both source_json and override_json on the same data source that only source_json was effective.

[skang0601]

This would be a very useful feature for my org. I'd like to start supplying a centralized repository of IAM Policy documents for other teams to consume.
Creating a bunch of IAM Policy is not very scalable due to policy attachment limits on IAM roles. If I can just supply the IAM policy documents and let users craft their own custom policies, it would be a much smoother process.

[YakDriver]

@tvald @skang0601 @lorengordon Have a look at PR #6052 to see if this will help with the overall solution.

[tvald]

@YakDriver commented in #6052:

@tvald Thanks for the input! I'm not currently working on part 2 of #5047 but wanted to get your thoughts on the design approach to part 2: a new data source (perhaps aws_iam_policy_list) with 1 main attribute (perhaps policy_json_list) and possibly a policy_id attribute. The list of policies would just be merged in order so those coming later would override earlier. The list, of course, could include aws_iam_policy_document data sources.

Sure, the approach sounds fine to me and solves this issue.

[bflad]

I'm not sure if we need to create a separate data source just for accepting lists of sources/overrides -- adding new TypeList attribute(s) that conflict with the existing attributes and performs the merging through each element of the list iteratively seems like it should work just fine.

I mention attributes plurally because it enables this scenario if something like source_jsons/source_json_list and override_jsons/override_json_list are available:

• source_json_list: Accepts multiple JSON strings and iteratively merges them. If any statement ID collisions are found, it can return an error.
• override_json_list: Accepts multiple JSON strings and iteratively merges them by overriding previous statements if there are statement ID collisions.

[tvald]

Sure, that approach also solves this issue. I have no preference between a new data source or a new attribute.

The slightly different semantics of source_json_list vs override_json_list could trip folks up, though, so it would need to be clearly documented.

[eterna2]

any updates on this?
if not what are the workaround now?