From 87857fec38e5dbaf8d13848f3aef6a731b04072d Mon Sep 17 00:00:00 2001 From: Sarhad Date: Mon, 23 Sep 2024 19:05:00 +0400 Subject: [PATCH 1/5] fix(DMVP-4760): Add policy attachment support --- main.tf | 4 ++-- variables.tf | 12 ++++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/main.tf b/main.tf index aedcd10..bd95f7a 100644 --- a/main.tf +++ b/main.tf @@ -23,8 +23,8 @@ module "bucket" { website = var.website - policy = local.is_public ? data.aws_iam_policy_document.public[0].json : "" - attach_policy = local.is_public + policy = local.is_public ? data.aws_iam_policy_document.public[0].json : var.policy + attach_policy = local.is_public || var.attach_policy // To Do: Add support for merging two policies } // have initial index.html file content diff --git a/variables.tf b/variables.tf index 2a4f723..63c9269 100644 --- a/variables.tf +++ b/variables.tf @@ -133,3 +133,15 @@ variable "event_notification_config" { events = ["s3:ObjectCreated:*"] } } + +variable "policy" { + description = "Bucket policies to be attached to the bucket in case custom policies are needed for the bucket" + type = any + default = "" # Default case is empty, to be compatibile with the previous behavior +} + +variable "attach_policy" { + description = "Flag to control if the policy should be attached to the bucket" + type = bool + default = false +} From 823163976bb359ab8c6daa0fda7bee9cbf095e7b Mon Sep 17 00:00:00 2001 From: Sarhad Date: Mon, 23 Sep 2024 19:30:58 +0400 Subject: [PATCH 2/5] fix(DMVP) add test case --- tests/policy/1-example.tf | 29 +++++++++++++++++++++++++++++ tests/policy/README.md | 29 +++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+) create mode 100644 tests/policy/1-example.tf create mode 100644 tests/policy/README.md diff --git a/tests/policy/1-example.tf b/tests/policy/1-example.tf new file mode 100644 index 0000000..112ccd4 --- /dev/null +++ b/tests/policy/1-example.tf @@ -0,0 +1,29 @@ +module "external-policies" { + source = "../.." + + name = "policies-bucket" + attach_policy = true + + policy = { + "Version": "2012-10-17", + "Statement": [ + { + "Sid": "module-test-statID", + "Effect": "Allow", + "Principal": { + "AWS": "arn:aws:iam::*:user/*" + }, + "Action": [ + "s3:GetObject", + "s3:PutObject", + "s3:PutObjectAcl", + "s3:ListBucket", + ], + "Resource": [ + "arn:aws:s3:::policies-bucket", + "arn:aws:s3:::policies-bucket/*" + ] + } + ] + } +} \ No newline at end of file diff --git a/tests/policy/README.md b/tests/policy/README.md new file mode 100644 index 0000000..08c03ff --- /dev/null +++ b/tests/policy/README.md @@ -0,0 +1,29 @@ +# private + + +## Requirements + +No requirements. + +## Providers + +No providers. + +## Modules + +| Name | Source | Version | +|------|--------|---------| +| [policies](#module\_policies) | ../.. | n/a | + +## Resources + +No resources. + +## Inputs + +No inputs. + +## Outputs + +No outputs. + From 39a70c430d765490d430a7369e00b80034acc5a1 Mon Sep 17 00:00:00 2001 From: Sarhad Date: Tue, 24 Sep 2024 11:19:37 +0400 Subject: [PATCH 3/5] fix example --- tests/policy/1-example.tf | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/tests/policy/1-example.tf b/tests/policy/1-example.tf index 112ccd4..c16758f 100644 --- a/tests/policy/1-example.tf +++ b/tests/policy/1-example.tf @@ -1,17 +1,23 @@ + +provider "aws" { + region = "eu-central-1" +} + module "external-policies" { source = "../.." - name = "policies-bucket" + acl = "private" + name = "policies-bucket-example-wpas" attach_policy = true - policy = { + policy = jsonencode({ "Version": "2012-10-17", "Statement": [ { - "Sid": "module-test-statID", + "Sid": "module-test-statID-stqw", "Effect": "Allow", "Principal": { - "AWS": "arn:aws:iam::*:user/*" + "AWS": "*" }, "Action": [ "s3:GetObject", @@ -20,10 +26,10 @@ module "external-policies" { "s3:ListBucket", ], "Resource": [ - "arn:aws:s3:::policies-bucket", - "arn:aws:s3:::policies-bucket/*" + "arn:aws:s3:::policies-bucket-example-wpas", + "arn:aws:s3:::policies-bucket-example-wpas/*", ] } ] - } + }) } \ No newline at end of file From d5197c2651885fac6d6ed6afda56879ebd5e8c20 Mon Sep 17 00:00:00 2001 From: Sarhad Date: Tue, 24 Sep 2024 17:16:45 +0400 Subject: [PATCH 4/5] fix(DMVP-4760) refactor --- main.tf | 5 ++-- policy.tf | 29 ++++++++++++++++++++++ tests/policy/custom-policy/1-example.tf | 33 +++++++++++++++++++++++++ tests/policy/no-policy/1-example.tf | 11 +++++++++ variables.tf | 18 +++++++------- 5 files changed, 85 insertions(+), 11 deletions(-) create mode 100644 tests/policy/custom-policy/1-example.tf create mode 100644 tests/policy/no-policy/1-example.tf diff --git a/main.tf b/main.tf index bd95f7a..7782c9e 100644 --- a/main.tf +++ b/main.tf @@ -19,12 +19,13 @@ module "bucket" { object_ownership = var.object_ownership cors_rule = var.cors_rule + versioning = var.versioning website = var.website - policy = local.is_public ? data.aws_iam_policy_document.public[0].json : var.policy - attach_policy = local.is_public || var.attach_policy // To Do: Add support for merging two policies + policy = local.is_public ? data.aws_iam_policy_document.public[0].json : try(data.aws_iam_policy_document.bucket_policy.0.json, "") + attach_policy = local.is_public || length(var.bucket_iam_policy) > 0 // To Do: Add support for merging two policies } // have initial index.html file content diff --git a/policy.tf b/policy.tf index 4d420f5..df751db 100644 --- a/policy.tf +++ b/policy.tf @@ -11,3 +11,32 @@ data "aws_iam_policy_document" "public" { resources = ["arn:aws:s3:::${var.name}/*"] } } + +data "aws_iam_policy_document" "bucket_policy" { + count = length(var.bucket_iam_policy) > 0 ? 1 : 0 + + dynamic "statement" { + for_each = var.bucket_iam_policy + + content { + effect = lookup(statement.value, "effect", "Allow") + actions = statement.value.actions + resources = ["arn:aws:s3:::${var.name}", "arn:aws:s3:::${var.name}/*"] + + principals { + type = statement.value.principals.type + identifiers = statement.value.principals.identifiers + } + + dynamic "condition" { + for_each = length(statement.value.conditions) > 0 ? statement.value.conditions : [] + + content { + test = condition.value.type # Condition type (e.g., StringEquals) + variable = condition.value.key # Condition variable (e.g., "SAML:aud") + values = condition.value.value # Condition values (list of strings) + } + } + } + } +} diff --git a/tests/policy/custom-policy/1-example.tf b/tests/policy/custom-policy/1-example.tf new file mode 100644 index 0000000..deda42e --- /dev/null +++ b/tests/policy/custom-policy/1-example.tf @@ -0,0 +1,33 @@ + +provider "aws" { + region = "eu-central-1" +} + +module "custom-policies" { + source = "../../.." + + acl = "private" + name = "policies-bucket-example-wpas-2" + + + bucket_iam_policy = [ + { + effect = "Allow" + actions = [ + "s3:GetObject", + "s3:PutObject", + "s3:PutObjectAcl", + "s3:ListBucket", + ] + principals = { + type = "AWS" + identifiers = ["*"] + } + # conditions = [{ + # value = [1.2] + # key = "s3:TlsVersion" + # type = "NumericLessThan" + # }] + } + ] +} diff --git a/tests/policy/no-policy/1-example.tf b/tests/policy/no-policy/1-example.tf new file mode 100644 index 0000000..36f3827 --- /dev/null +++ b/tests/policy/no-policy/1-example.tf @@ -0,0 +1,11 @@ +provider "aws" { + region = "eu-central-1" +} + +module "no-policies" { + source = "../../.." + + acl = "public" + name = "policies-bucket-example-wpas" + +} diff --git a/variables.tf b/variables.tf index 63c9269..3ac0843 100644 --- a/variables.tf +++ b/variables.tf @@ -134,14 +134,14 @@ variable "event_notification_config" { } } -variable "policy" { - description = "Bucket policies to be attached to the bucket in case custom policies are needed for the bucket" - type = any - default = "" # Default case is empty, to be compatibile with the previous behavior -} -variable "attach_policy" { - description = "Flag to control if the policy should be attached to the bucket" - type = bool - default = false +variable "bucket_iam_policy" { + type = list(object({ + effect = optional(string, "Allow") # Effect of the policy (Allow or Deny) + actions = list(string) # Actions like sts:AssumeRole + principals = any # Principals (e.g., AWS, Service, Federated) + conditions = optional(any, []) # Optional conditions for assume role + })) + description = "AWS bucket policy" + default = [] } From f70c1abe7162af61d174b9ec100a52af860b460a Mon Sep 17 00:00:00 2001 From: Sarhad Date: Tue, 24 Sep 2024 17:40:52 +0400 Subject: [PATCH 5/5] fix(DMVP-4760): refactor --- README.md | 11 +++++-- main.tf | 1 - tests/cors-config/1-example.tf | 14 ++++----- tests/policy/1-example.tf | 35 ---------------------- tests/policy/custom-policy/README.md | 29 ++++++++++++++++++ tests/policy/no-policy/README.md | 29 ++++++++++++++++++ tests/sqs-event-notifications/1-example.tf | 1 - tests/sqs-event-notifications/README.md | 29 ++++++++++++++++++ 8 files changed, 103 insertions(+), 46 deletions(-) delete mode 100644 tests/policy/1-example.tf create mode 100644 tests/policy/custom-policy/README.md create mode 100644 tests/policy/no-policy/README.md create mode 100644 tests/sqs-event-notifications/README.md diff --git a/README.md b/README.md index 943c2d5..c882b58 100644 --- a/README.md +++ b/README.md @@ -96,8 +96,12 @@ module "my_bucket" { | Name | Type | |------|------| +| [aws_s3_bucket_notification.bucket_notification](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_notification) | resource | | [aws_s3_object.index](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_object) | resource | +| [aws_sqs_queue.queue](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/sqs_queue) | resource | +| [aws_iam_policy_document.bucket_policy](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source | | [aws_iam_policy_document.public](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source | +| [aws_iam_policy_document.queue](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source | ## Inputs @@ -106,12 +110,15 @@ module "my_bucket" { | [acl](#input\_acl) | The acl config for bucket, NOTE: 'acl' conflicts with 'grant' and 'owner'. | `string` | `"private"` | no | | [block\_public\_acls](#input\_block\_public\_acls) | Whether Amazon S3 should block public ACLs for this bucket. | `bool` | `false` | no | | [block\_public\_policy](#input\_block\_public\_policy) | Whether Amazon S3 should block public bucket policies for this bucket. | `bool` | `false` | no | -| [bucket\_files](#input\_bucket\_files) | Initial content for bucket, use acl and pattern params if you need more control. | 
object({
    path = string
  })
| 
{
  "path": ""
}
| no | +| [bucket\_files](#input\_bucket\_files) | Initial content for bucket, use acl and pattern params if you need more control. | 
object({
    path = string
  })
| 
{
  "path": ""
}
| no | +| [bucket\_iam\_policy](#input\_bucket\_iam\_policy) | AWS bucket policy | 
list(object({
    effect     = optional(string, "Allow") # Effect of the policy (Allow or Deny)
    actions    = list(string)              # Actions like sts:AssumeRole
    principals = any                       # Principals (e.g., AWS, Service, Federated)
    conditions = optional(any, [])         # Optional conditions for assume role
  }))
| `[]` | no | | [control\_object\_ownership](#input\_control\_object\_ownership) | Manage S3 Bucket Ownership Controls on this bucket or not. | `bool` | `false` | no | +| [cors\_rule](#input\_cors\_rule) | List of maps containing rules for Cross-Origin Resource Sharing. | `any` | `[]` | no | | [create\_iam\_user](#input\_create\_iam\_user) | Whether to create specific api access user to this created bucket. | `bool` | `false` | no | | [create\_index\_html](#input\_create\_index\_html) | Whether to create and initial index.html file with default data. | `bool` | `false` | no | +| [event\_notification\_config](#input\_event\_notification\_config) | n/a | 
object({
    target_type   = string,                                        // Target type for the S3 event notification, can be "sqs" or "null". Other target types can be implemented in the future.
    name_suffix   = string,                                        // Suffix to add to the target name.
    filter_prefix = string,                                        // Prefix to filter object key names for the event notification.
    events        = optional(list(string), ["s3:ObjectCreated:*"]) // List of S3 events that trigger the notification. Defaults to "s3:ObjectCreated:*".
  })
| 
{
  "events": [
    "s3:ObjectCreated:*"
  ],
  "filter_prefix": "test/",
  "name_suffix": "event",
  "target_type": "null"
}
| no | | [grant](#input\_grant) | The ACL policy grant. NOTE: conflicts with 'acl'. | `any` | `[]` | no | -| [iam\_user\_actions](#input\_iam\_user\_actions) | The allowed actions that created user can perform on this created bucket. | `list(string)` | 
[
  "s3:PutObject",
  "s3:ListBucket",
  "s3:GetObject",
  "s3:GetObjectVersion",
  "s3:GetBucketAcl",
  "s3:DeleteObject",
  "s3:DeleteObjectVersion",
  "s3:PutLifecycleConfiguration",
  "s3:PutObjectAcl"
]
| no | +| [iam\_user\_actions](#input\_iam\_user\_actions) | The allowed actions that created user can perform on this created bucket. | `list(string)` | 
[
  "s3:PutObject",
  "s3:ListBucket",
  "s3:GetObject",
  "s3:GetObjectVersion",
  "s3:GetBucketAcl",
  "s3:DeleteObject",
  "s3:DeleteObjectVersion",
  "s3:PutLifecycleConfiguration",
  "s3:PutObjectAcl"
]
| no | | [iam\_user\_name](#input\_iam\_user\_name) | The name of user, NOTE: this is optional and if it is not passed in use place the name will be generated based on bucket name. | `string` | `""` | no | | [ignore\_public\_acls](#input\_ignore\_public\_acls) | Whether Amazon S3 should ignore public ACLs for this bucket. | `bool` | `false` | no | | [name](#input\_name) | Bucket name. | `string` | n/a | yes | diff --git a/main.tf b/main.tf index 7782c9e..f474480 100644 --- a/main.tf +++ b/main.tf @@ -19,7 +19,6 @@ module "bucket" { object_ownership = var.object_ownership cors_rule = var.cors_rule - versioning = var.versioning website = var.website diff --git a/tests/cors-config/1-example.tf b/tests/cors-config/1-example.tf index 1b21efa..6fc4bec 100644 --- a/tests/cors-config/1-example.tf +++ b/tests/cors-config/1-example.tf @@ -11,11 +11,11 @@ module "private-explicitly" { acl = "private" cors_rule = [ - { - allowed_methods = ["HEAD","GET","PUT", "POST"] - allowed_origins = ["https://modules.tf", "https://dasmeta.modules.tf"] - allowed_headers = ["*"] - expose_headers = ["ETag","Access-Control-Allow-Origin"] - } -] + { + allowed_methods = ["HEAD", "GET", "PUT", "POST"] + allowed_origins = ["https://modules.tf", "https://dasmeta.modules.tf"] + allowed_headers = ["*"] + expose_headers = ["ETag", "Access-Control-Allow-Origin"] + } + ] } diff --git a/tests/policy/1-example.tf b/tests/policy/1-example.tf deleted file mode 100644 index c16758f..0000000 --- a/tests/policy/1-example.tf +++ /dev/null @@ -1,35 +0,0 @@ - -provider "aws" { - region = "eu-central-1" -} - -module "external-policies" { - source = "../.." - - acl = "private" - name = "policies-bucket-example-wpas" - attach_policy = true - - policy = jsonencode({ - "Version": "2012-10-17", - "Statement": [ - { - "Sid": "module-test-statID-stqw", - "Effect": "Allow", - "Principal": { - "AWS": "*" - }, - "Action": [ - "s3:GetObject", - "s3:PutObject", - "s3:PutObjectAcl", - "s3:ListBucket", - ], - "Resource": [ - "arn:aws:s3:::policies-bucket-example-wpas", - "arn:aws:s3:::policies-bucket-example-wpas/*", - ] - } - ] - }) -} \ No newline at end of file diff --git a/tests/policy/custom-policy/README.md b/tests/policy/custom-policy/README.md new file mode 100644 index 0000000..3b86812 --- /dev/null +++ b/tests/policy/custom-policy/README.md @@ -0,0 +1,29 @@ +# custom-policy + + +## Requirements + +No requirements. + +## Providers + +No providers. + +## Modules + +| Name | Source | Version | +|------|--------|---------| +| [custom-policies](#module\_custom-policies) | ../../.. | n/a | + +## Resources + +No resources. + +## Inputs + +No inputs. + +## Outputs + +No outputs. + diff --git a/tests/policy/no-policy/README.md b/tests/policy/no-policy/README.md new file mode 100644 index 0000000..4586a7c --- /dev/null +++ b/tests/policy/no-policy/README.md @@ -0,0 +1,29 @@ +# no-policy + + +## Requirements + +No requirements. + +## Providers + +No providers. + +## Modules + +| Name | Source | Version | +|------|--------|---------| +| [no-policies](#module\_no-policies) | ../../.. | n/a | + +## Resources + +No resources. + +## Inputs + +No inputs. + +## Outputs + +No outputs. + diff --git a/tests/sqs-event-notifications/1-example.tf b/tests/sqs-event-notifications/1-example.tf index 7441e46..da2d0a6 100644 --- a/tests/sqs-event-notifications/1-example.tf +++ b/tests/sqs-event-notifications/1-example.tf @@ -10,4 +10,3 @@ module "private" { events = ["s3:ObjectCreated:CompleteMultipartUpload"] } } - diff --git a/tests/sqs-event-notifications/README.md b/tests/sqs-event-notifications/README.md new file mode 100644 index 0000000..8ddcd70 --- /dev/null +++ b/tests/sqs-event-notifications/README.md @@ -0,0 +1,29 @@ +# sqs-event-notifications + + +## Requirements + +No requirements. + +## Providers + +No providers. + +## Modules + +| Name | Source | Version | +|------|--------|---------| +| [private](#module\_private) | ../.. | n/a | + +## Resources + +No resources. + +## Inputs + +No inputs. + +## Outputs + +No outputs. +