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.

Sign up for GitHub

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

Jump to bottom

feat: define iam_policy_statements object syntax #26

Merged
merged 8 commits into from
Jun 10, 2023
Merged

feat: define iam_policy_statements object syntax #26

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
f9191c9
feat: define iam_policy_statements object syntax
gberenice Jun 8, 2023
419fb4b
fix: update README
gberenice Jun 8, 2023
43364f4
fix: adjust dynamic statements to stricter object syntax
gberenice Jun 9, 2023
8f2f753
chore: adds variable for IAM policy version
gberenice Jun 9, 2023
315c312
Add `iam_policy`, deprecate `iam_policy_statements`, fix tflint
Nuru Jun 9, 2023
97ea586
Address reviewer feedback
Nuru Jun 10, 2023
6783c85
Update example in README
Nuru Jun 10, 2023
ba46cc8
Add description to example inputs
Nuru Jun 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ Available targets:

| Name | Version |
|------|---------|
| <a name="requirement_terraform"></a> [terraform](#requirement\_terraform) | >= 1.0 |
| <a name="requirement_terraform"></a> [terraform](#requirement\_terraform) | >= 1.3.0 |
| <a name="requirement_aws"></a> [aws](#requirement\_aws) | >= 4.0 |
| <a name="requirement_http"></a> [http](#requirement\_http) | >= 3.0 |

Expand Down Expand Up @@ -217,7 +217,7 @@ Available targets:
| <a name="input_iam_override_policy_documents"></a> [iam\_override\_policy\_documents](#input\_iam\_override\_policy\_documents) | List of IAM policy documents that are merged together into the exported document. In merging, statements with non-blank sids will override statements with the same sid from earlier documents in the list. Statements with non-blank sids will also override statements with the same sid from documents provided in the `source_json` and `source_policy_documents` arguments. Non-overriding statements will be added to the exported document. | `list(string)` | `null` | no |
| <a name="input_iam_policy_enabled"></a> [iam\_policy\_enabled](#input\_iam\_policy\_enabled) | If set to true will create IAM policy in AWS | `bool` | `false` | no |
| <a name="input_iam_policy_id"></a> [iam\_policy\_id](#input\_iam\_policy\_id) | ID for the policy document. | `string` | `null` | no |
| <a name="input_iam_policy_statements"></a> [iam\_policy\_statements](#input\_iam\_policy\_statements) | Map of IAM policy statements to use in the policy. This can be used with or instead of the `var.iam_source_json_url`. | `any` | `{}` | no |
| <a name="input_iam_policy_statements"></a> [iam\_policy\_statements](#input\_iam\_policy\_statements) | Map of IAM policy statements to use in the policy. This can be used with or instead of the `var.iam_source_json_url`. | <pre>object({<br> sid = optional(string, "")<br> effect = optional(string, "")<br> actions = optional(list(string), [])<br> not_actions = optional(list(string), [])<br> resources = optional(list(string), [])<br> not_resources = optional(list(string), [])<br> conditions = optional(list(object({<br> test = string<br> variable = string<br> values = list(string)<br> })), [])<br> principals = optional(list(object({<br> type = string<br> identifiers = list(string)<br> })), [])<br> not_principals = optional(list(object({<br> type = string<br> identifiers = list(string)<br> })), [])<br> })</pre> | `{}` | no |
| <a name="input_iam_source_json_url"></a> [iam\_source\_json\_url](#input\_iam\_source\_json\_url) | IAM source JSON policy to download and use as `source_json` argument. This is useful when using a 3rd party service that provides their own policy. This can be used with or instead of the `var.iam_policy_statements`. | `string` | `null` | no |
| <a name="input_iam_source_policy_documents"></a> [iam\_source\_policy\_documents](#input\_iam\_source\_policy\_documents) | List of IAM policy documents that are merged together into the exported document. Statements defined in `source_policy_documents` or `source_json` must have unique sids. Statements with the same sid from documents assigned to the `override_json` and `override_policy_documents` arguments will override source statements. | `list(string)` | `null` | no |
| <a name="input_id_length_limit"></a> [id\_length\_limit](#input\_id\_length\_limit) | Limit `id` to this many characters (minimum 6).<br>Set to `0` for unlimited length.<br>Set to `null` for keep the existing setting, which defaults to `0`.<br>Does not affect `id_full`. | `number` | `null` | no |
Expand Down
4 changes: 2 additions & 2 deletions docs/terraform.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

| Name | Version |
|------|---------|
| <a name="requirement_terraform"></a> [terraform](#requirement\_terraform) | >= 1.0 |
| <a name="requirement_terraform"></a> [terraform](#requirement\_terraform) | >= 1.3.0 |
| <a name="requirement_aws"></a> [aws](#requirement\_aws) | >= 4.0 |
| <a name="requirement_http"></a> [http](#requirement\_http) | >= 3.0 |

Expand Down Expand Up @@ -43,7 +43,7 @@
| <a name="input_iam_override_policy_documents"></a> [iam\_override\_policy\_documents](#input\_iam\_override\_policy\_documents) | List of IAM policy documents that are merged together into the exported document. In merging, statements with non-blank sids will override statements with the same sid from earlier documents in the list. Statements with non-blank sids will also override statements with the same sid from documents provided in the `source_json` and `source_policy_documents` arguments. Non-overriding statements will be added to the exported document. | `list(string)` | `null` | no |
| <a name="input_iam_policy_enabled"></a> [iam\_policy\_enabled](#input\_iam\_policy\_enabled) | If set to true will create IAM policy in AWS | `bool` | `false` | no |
| <a name="input_iam_policy_id"></a> [iam\_policy\_id](#input\_iam\_policy\_id) | ID for the policy document. | `string` | `null` | no |
| <a name="input_iam_policy_statements"></a> [iam\_policy\_statements](#input\_iam\_policy\_statements) | Map of IAM policy statements to use in the policy. This can be used with or instead of the `var.iam_source_json_url`. | `any` | `{}` | no |
| <a name="input_iam_policy_statements"></a> [iam\_policy\_statements](#input\_iam\_policy\_statements) | Map of IAM policy statements to use in the policy. This can be used with or instead of the `var.iam_source_json_url`. | <pre>object({<br> sid = optional(string, "")<br> effect = optional(string, "")<br> actions = optional(list(string), [])<br> not_actions = optional(list(string), [])<br> resources = optional(list(string), [])<br> not_resources = optional(list(string), [])<br> conditions = optional(list(object({<br> test = string<br> variable = string<br> values = list(string)<br> })), [])<br> principals = optional(list(object({<br> type = string<br> identifiers = list(string)<br> })), [])<br> not_principals = optional(list(object({<br> type = string<br> identifiers = list(string)<br> })), [])<br> })</pre> | `{}` | no |
| <a name="input_iam_source_json_url"></a> [iam\_source\_json\_url](#input\_iam\_source\_json\_url) | IAM source JSON policy to download and use as `source_json` argument. This is useful when using a 3rd party service that provides their own policy. This can be used with or instead of the `var.iam_policy_statements`. | `string` | `null` | no |
| <a name="input_iam_source_policy_documents"></a> [iam\_source\_policy\_documents](#input\_iam\_source\_policy\_documents) | List of IAM policy documents that are merged together into the exported document. Statements defined in `source_policy_documents` or `source_json` must have unique sids. Statements with the same sid from documents assigned to the `override_json` and `override_policy_documents` arguments will override source statements. | `list(string)` | `null` | no |
| <a name="input_id_length_limit"></a> [id\_length\_limit](#input\_id\_length\_limit) | Limit `id` to this many characters (minimum 6).<br>Set to `0` for unlimited length.<br>Set to `null` for keep the existing setting, which defaults to `0`.<br>Does not affect `id_full`. | `number` | `null` | no |
Expand Down
24 changes: 22 additions & 2 deletions variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,27 @@ variable "iam_source_json_url" {
}

variable "iam_policy_statements" {
type = any
type = object({
sid = optional(string, "")
effect = optional(string, "")
actions = optional(list(string), [])
not_actions = optional(list(string), [])
resources = optional(list(string), [])
not_resources = optional(list(string), [])
Copy link
Sponsor Contributor

Choose a reason for hiding this comment

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

  • Add version here and in main.tf.
  • Move statement components to list of objects.
  • Inputs that are not going to be expanded into blocks by dynamic should default to null.
  • In main.tf, replace lookups like
lookup(statement.value, "sid", statement.key)

With direct references

statement.value.sid
Suggested change
type = object({
sid = optional(string, "")
effect = optional(string, "")
actions = optional(list(string), [])
not_actions = optional(list(string), [])
resources = optional(list(string), [])
not_resources = optional(list(string), [])
type = object({
version = optional(string, null)
statement = list(object({
sid = optional(string, null)
effect = optional(string, null)
actions = optional(list(string), null)
not_actions = optional(list(string), null)
resources = optional(list(string), null)
not_resources = optional(list(string), null)
})), [])

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Nuru thanks a lot for your review!

  • Version arguments should be set for data source aws_iam_policy_document, so I added it as a separate variable.
  • I updated statements to the map of objects - that's what is expected in the example.
  • Inputs were updated.
  • Lookups were replaced👍

Please let me know your thoughts.

Copy link
Sponsor Contributor

Choose a reason for hiding this comment

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

@gberenice I forgot we want this to be backward compatible and that means the input is a map. I will take it from here. Thank you very much for your contributions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Nuru sounds good, thank you!

conditions = optional(list(object({
test = string
variable = string
values = list(string)
})), [])
principals = optional(list(object({
type = string
identifiers = list(string)
})), [])
not_principals = optional(list(object({
type = string
identifiers = list(string)
})), [])
})
description = "Map of IAM policy statements to use in the policy. This can be used with or instead of the `var.iam_source_json_url`."
Copy link
Sponsor Contributor

Choose a reason for hiding this comment

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

Change "Map of" to "Object describing"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Change "Map of" to "Object describing"

@Nuru from what I can see - a map is expected here, so I fixed the variable definition.

default = {}
}
Expand Down Expand Up @@ -38,4 +58,4 @@ variable "iam_override_policy_documents" {
type = list(string)
description = "List of IAM policy documents that are merged together into the exported document. In merging, statements with non-blank sids will override statements with the same sid from earlier documents in the list. Statements with non-blank sids will also override statements with the same sid from documents provided in the `source_json` and `source_policy_documents` arguments. Non-overriding statements will be added to the exported document."
default = null
}
}
2 changes: 1 addition & 1 deletion versions.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
terraform {
required_version = ">= 1.0"
required_version = ">= 1.3.0"

required_providers {
# Update these to reflect the actual requirements of your module
Expand Down