From f294fe53a7b8048802c1e64d13b234f8af37228a Mon Sep 17 00:00:00 2001 From: Almenon Date: Wed, 18 Jan 2023 17:21:56 -0800 Subject: [PATCH 1/9] change requirements --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index cbb4db5..5a710d7 100644 --- a/README.md +++ b/README.md @@ -86,13 +86,13 @@ Apache-2.0 Licensed. See [LICENSE](https://github.com/aws-ia/terraform-aws-mwaa/ | Name | Version | |------|---------| | [terraform](#requirement\_terraform) | >= 1.0.0 | -| [aws](#requirement\_aws) | ~> 4.20.0 | +| [aws](#requirement\_aws) | >= 4.20.0 | ## Providers | Name | Version | |------|---------| -| [aws](#provider\_aws) | ~> 4.20.0 | +| [aws](#provider\_aws) | ~= 4.20.0 | ## Modules From d2a452bd8b85c348c505dc4b8cdd00936ffd3e2c Mon Sep 17 00:00:00 2001 From: Almenon Date: Fri, 27 Jan 2023 14:10:06 -0800 Subject: [PATCH 2/9] restrict permissions to be more secure --- data.tf | 72 ++++++++++++++++++++++++++++++++++------------------ variables.tf | 6 +++++ 2 files changed, 54 insertions(+), 24 deletions(-) diff --git a/data.tf b/data.tf index 8d0e323..9424650 100644 --- a/data.tf +++ b/data.tf @@ -55,7 +55,9 @@ data "aws_iam_policy_document" "mwaa" { statement { effect = "Allow" actions = [ - "s3:*" + "s3:GetObject*", + "s3:GetBucket*", + "s3:List*" ] resources = [ local.source_bucket_arn, @@ -84,9 +86,7 @@ data "aws_iam_policy_document" "mwaa" { actions = [ "logs:DescribeLogGroups", "cloudwatch:PutMetricData", - "batch:DescribeJobs", - "batch:ListJobs", - "eks:*" + "s3:GetAccountPublicAccessBlock" ] resources = [ "*" @@ -96,39 +96,63 @@ data "aws_iam_policy_document" "mwaa" { statement { effect = "Allow" actions = [ - "sqs:ChangeMessageVisibility", - "sqs:DeleteMessage", - "sqs:GetQueueAttributes", - "sqs:GetQueueUrl", - "sqs:ReceiveMessage", - "sqs:SendMessage" + "secretsmanager:GetSecretValue", ] resources = [ - "arn:${data.aws_partition.current.id}:sqs:${data.aws_region.current.name}:*:airflow-celery-*" + "arn:aws:secretsmanager:${data.aws_region.current.name}:${data.aws_caller_identity.current.account_id}:secret:${var.secret_prefix}/*" ] } statement { effect = "Allow" actions = [ - "kms:Decrypt", - "kms:DescribeKey", - "kms:GenerateDataKey*", - "kms:Encrypt" + "secretsmanager:GetResourcePolicy", + "secretsmanager:DescribeSecret", + "secretsmanager:ListSecretVersionIds" ] - not_resources = [ - "arn:${data.aws_partition.current.id}:kms:*:${data.aws_caller_identity.current.account_id}:key/*" + resources = [ + "arn:aws:secretsmanager:${data.aws_region.current.name}:${data.aws_caller_identity.current.account_id}:*" ] - condition { - test = "StringLike" - variable = "kms:ViaService" + } - values = [ - "sqs.${data.aws_region.current.name}.amazonaws.com" - ] - } + statement { + effect = "Allow" + actions = [ + "sqs:ChangeMessageVisibility", + "sqs:DeleteMessage", + "sqs:GetQueueAttributes", + "sqs:GetQueueUrl", + "sqs:ReceiveMessage", + "sqs:SendMessage" + ] + resources = [ + "arn:${data.aws_partition.current.id}:sqs:${data.aws_region.current.name}:*:airflow-celery-*" + ] } + # todo: define var.kms_id + # statement { + # effect = "Allow" + # actions = [ + # "kms:Decrypt", + # "kms:DescribeKey", + # "kms:GenerateDataKey*", + # "kms:Encrypt" + # ] + # resources = [ + # "arn:${data.aws_partition.current.id}:kms:*:${data.aws_caller_identity.current.account_id}:key/{var.kms_id}" + # ] + # condition { + # test = "StringLike" + # variable = "kms:ViaService" + + # values = [ + # "sqs.${data.aws_region.current.name}.amazonaws.com", + # "s3.{your-region}.amazonaws.com" + # ] + # } + # } + statement { effect = "Allow" actions = [ diff --git a/variables.tf b/variables.tf index f9442ec..8c85c0c 100644 --- a/variables.tf +++ b/variables.tf @@ -232,3 +232,9 @@ variable "requirements_s3_object_version" { type = string default = null } + +variable "secret_prefix" { + description = "Optional) The prefix for the Secret Manager secrets MWAA can have access to" + type = string + default = "NoneThisPrefixDoesNotExist1521662733" +} From 14e99fa9f951f38078a51189f2a6b918eabc2221 Mon Sep 17 00:00:00 2001 From: Almenon Date: Fri, 27 Jan 2023 17:17:24 -0800 Subject: [PATCH 3/9] uncomment --- data.tf | 46 ++++++++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/data.tf b/data.tf index 9424650..a149e51 100644 --- a/data.tf +++ b/data.tf @@ -130,28 +130,30 @@ data "aws_iam_policy_document" "mwaa" { ] } - # todo: define var.kms_id - # statement { - # effect = "Allow" - # actions = [ - # "kms:Decrypt", - # "kms:DescribeKey", - # "kms:GenerateDataKey*", - # "kms:Encrypt" - # ] - # resources = [ - # "arn:${data.aws_partition.current.id}:kms:*:${data.aws_caller_identity.current.account_id}:key/{var.kms_id}" - # ] - # condition { - # test = "StringLike" - # variable = "kms:ViaService" - - # values = [ - # "sqs.${data.aws_region.current.name}.amazonaws.com", - # "s3.{your-region}.amazonaws.com" - # ] - # } - # } + # See note in https://docs.aws.amazon.com/mwaa/latest/userguide/mwaa-create-role.html + # if MWAA is using a AWS managed KMS key, we have to give permission to the key in ?? account + # We don't know what account AWS puts their key in so we use not_resources to grant access to all + # accounts except for ours + statement { + effect = "Allow" + actions = [ + "kms:Decrypt", + "kms:DescribeKey", + "kms:GenerateDataKey*", + "kms:Encrypt" + ] + not_resources = [ + "arn:${data.aws_partition.current.id}:kms:*:${data.aws_caller_identity.current.account_id}:key/*" + ] + condition { + test = "StringLike" + variable = "kms:ViaService" + + values = [ + "sqs.${data.aws_region.current.name}.amazonaws.com" + ] + } + } statement { effect = "Allow" From f78fdb4c4961ec32582f61f72deb439f8a290d18 Mon Sep 17 00:00:00 2001 From: Almenon Date: Fri, 27 Jan 2023 17:23:49 -0800 Subject: [PATCH 4/9] make prefix a bit more explicit --- data.tf | 2 +- variables.tf | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/data.tf b/data.tf index a149e51..85cc2f5 100644 --- a/data.tf +++ b/data.tf @@ -99,7 +99,7 @@ data "aws_iam_policy_document" "mwaa" { "secretsmanager:GetSecretValue", ] resources = [ - "arn:aws:secretsmanager:${data.aws_region.current.name}:${data.aws_caller_identity.current.account_id}:secret:${var.secret_prefix}/*" + "arn:aws:secretsmanager:${data.aws_region.current.name}:${data.aws_caller_identity.current.account_id}:secret:${var.secret_prefix}" ] } diff --git a/variables.tf b/variables.tf index 8c85c0c..8eda36d 100644 --- a/variables.tf +++ b/variables.tf @@ -236,5 +236,5 @@ variable "requirements_s3_object_version" { variable "secret_prefix" { description = "Optional) The prefix for the Secret Manager secrets MWAA can have access to" type = string - default = "NoneThisPrefixDoesNotExist1521662733" + default = "NoneThisPrefixDoesNotExist1521662733/*" } From fdf8e55de7d3f023608527fd6ecfcfe18414fce2 Mon Sep 17 00:00:00 2001 From: almenon Date: Sun, 19 Mar 2023 11:05:05 -0700 Subject: [PATCH 5/9] fix whitespace --- data.tf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/data.tf b/data.tf index 85cc2f5..b811b2d 100644 --- a/data.tf +++ b/data.tf @@ -133,7 +133,7 @@ data "aws_iam_policy_document" "mwaa" { # See note in https://docs.aws.amazon.com/mwaa/latest/userguide/mwaa-create-role.html # if MWAA is using a AWS managed KMS key, we have to give permission to the key in ?? account # We don't know what account AWS puts their key in so we use not_resources to grant access to all - # accounts except for ours + # accounts except for ours statement { effect = "Allow" actions = [ From 18e0c9148ebfb414bf324b86d69440704e254179 Mon Sep 17 00:00:00 2001 From: almenon Date: Sun, 19 Mar 2023 15:22:43 -0700 Subject: [PATCH 6/9] update docs --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 5a710d7..8627f1b 100644 --- a/README.md +++ b/README.md @@ -149,6 +149,7 @@ No modules. | [requirements\_s3\_object\_version](#input\_requirements\_s3\_object\_version) | Optional) The requirements.txt file version you want to use. | `string` | `null` | no | | [requirements\_s3\_path](#input\_requirements\_s3\_path) | (Optional) The relative path to the requirements.txt file on your Amazon S3 storage bucket. For example, requirements.txt. If a relative path is provided in the request, then requirements\_s3\_object\_version is required. | `string` | `null` | no | | [schedulers](#input\_schedulers) | (Optional) The number of schedulers that you want to run in your environment. | `string` | `null` | no | +| [secret\_prefix](#input\_secret\_prefix) | Optional) The prefix for the Secret Manager secrets MWAA can have access to | `string` | `"NoneThisPrefixDoesNotExist1521662733/*"` | no | | [security\_group\_ids](#input\_security\_group\_ids) | Security group IDs for MWAA | `list(string)` | `[]` | no | | [source\_bucket\_arn](#input\_source\_bucket\_arn) | (Required) The Amazon Resource Name (ARN) of your Amazon S3 storage bucket. For example, arn:aws:s3:::airflow-mybucketname | `string` | `null` | no | | [source\_bucket\_name](#input\_source\_bucket\_name) | New bucket will be created with the given name for MWAA when create\_s3\_bucket=true | `string` | `null` | no | From 483d3bea5f707c346b1c41681d723e94ce28f32d Mon Sep 17 00:00:00 2001 From: Almenon Date: Thu, 11 May 2023 08:23:35 -0700 Subject: [PATCH 7/9] remove secretmanager access One can use iam_role_additional_policies instead --- data.tf | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/data.tf b/data.tf index b811b2d..ab7cdfd 100644 --- a/data.tf +++ b/data.tf @@ -93,28 +93,6 @@ data "aws_iam_policy_document" "mwaa" { ] } - statement { - effect = "Allow" - actions = [ - "secretsmanager:GetSecretValue", - ] - resources = [ - "arn:aws:secretsmanager:${data.aws_region.current.name}:${data.aws_caller_identity.current.account_id}:secret:${var.secret_prefix}" - ] - } - - statement { - effect = "Allow" - actions = [ - "secretsmanager:GetResourcePolicy", - "secretsmanager:DescribeSecret", - "secretsmanager:ListSecretVersionIds" - ] - resources = [ - "arn:aws:secretsmanager:${data.aws_region.current.name}:${data.aws_caller_identity.current.account_id}:*" - ] - } - statement { effect = "Allow" actions = [ From a4e97d8ad944300366d6d2876e47fd97dbe0e958 Mon Sep 17 00:00:00 2001 From: Almenon Date: Thu, 11 May 2023 08:24:33 -0700 Subject: [PATCH 8/9] remove secretmanager access one can use iam_role_additional_policies instead --- README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/README.md b/README.md index 1a983dc..2a93ec9 100644 --- a/README.md +++ b/README.md @@ -149,7 +149,6 @@ No modules. | [requirements\_s3\_object\_version](#input\_requirements\_s3\_object\_version) | Optional) The requirements.txt file version you want to use. | `string` | `null` | no | | [requirements\_s3\_path](#input\_requirements\_s3\_path) | (Optional) The relative path to the requirements.txt file on your Amazon S3 storage bucket. For example, requirements.txt. If a relative path is provided in the request, then requirements\_s3\_object\_version is required. | `string` | `null` | no | | [schedulers](#input\_schedulers) | (Optional) The number of schedulers that you want to run in your environment. | `string` | `null` | no | -| [secret\_prefix](#input\_secret\_prefix) | Optional) The prefix for the Secret Manager secrets MWAA can have access to | `string` | `"NoneThisPrefixDoesNotExist1521662733/*"` | no | | [security\_group\_ids](#input\_security\_group\_ids) | Security group IDs for MWAA | `list(string)` | `[]` | no | | [source\_bucket\_arn](#input\_source\_bucket\_arn) | (Required) The Amazon Resource Name (ARN) of your Amazon S3 storage bucket. For example, arn:aws:s3:::airflow-mybucketname | `string` | `null` | no | | [source\_bucket\_name](#input\_source\_bucket\_name) | New bucket will be created with the given name for MWAA when create\_s3\_bucket=true | `string` | `null` | no | From 8bd4cc4229ce51934f787d869babe6f1a58eba58 Mon Sep 17 00:00:00 2001 From: Almenon Date: Thu, 11 May 2023 08:25:57 -0700 Subject: [PATCH 9/9] remove secret prefix no longer needed --- variables.tf | 6 ------ 1 file changed, 6 deletions(-) diff --git a/variables.tf b/variables.tf index 8eda36d..f9442ec 100644 --- a/variables.tf +++ b/variables.tf @@ -232,9 +232,3 @@ variable "requirements_s3_object_version" { type = string default = null } - -variable "secret_prefix" { - description = "Optional) The prefix for the Secret Manager secrets MWAA can have access to" - type = string - default = "NoneThisPrefixDoesNotExist1521662733/*" -}