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

feat: Added variable to control the create log group permission #514

Merged
merged 1 commit into from
Nov 22, 2023
Merged

feat: Added variable to control the create log group permission #514

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -742,6 +742,7 @@ No modules.
| <a name="input_assume_role_policy_statements"></a> [assume\_role\_policy\_statements](#input\_assume\_role\_policy\_statements) | Map of dynamic policy statements for assuming Lambda Function role (trust relationship) | `any` | `{}` | no |
| <a name="input_attach_async_event_policy"></a> [attach\_async\_event\_policy](#input\_attach\_async\_event\_policy) | Controls whether async event policy should be added to IAM role for Lambda Function | `bool` | `false` | no |
| <a name="input_attach_cloudwatch_logs_policy"></a> [attach\_cloudwatch\_logs\_policy](#input\_attach\_cloudwatch\_logs\_policy) | Controls whether CloudWatch Logs policy should be added to IAM role for Lambda Function | `bool` | `true` | no |
| <a name="input_attach_create_log_group_permission"></a> [attach\_create\_log\_group\_permission](#input\_attach\_create\_log\_group\_permission) | Controls whether to add the create log group permission to the CloudWatch logs policy | `bool` | `true` | no |
| <a name="input_attach_dead_letter_policy"></a> [attach\_dead\_letter\_policy](#input\_attach\_dead\_letter\_policy) | Controls whether SNS/SQS dead letter notification policy should be added to IAM role for Lambda Function | `bool` | `false` | no |
| <a name="input_attach_network_policy"></a> [attach\_network\_policy](#input\_attach\_network\_policy) | Controls whether VPC/network policy should be added to IAM role for Lambda Function | `bool` | `false` | no |
| <a name="input_attach_policies"></a> [attach\_policies](#input\_attach\_policies) | Controls whether list of policies should be added to IAM role for Lambda Function | `bool` | `false` | no |
Expand Down
1 change: 1 addition & 0 deletions examples/complete/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ Note that this example may create resources which cost money. Run `terraform des
| <a name="module_lambda_function"></a> [lambda\_function](#module\_lambda\_function) | ../../ | n/a |
| <a name="module_lambda_function_existing_package_local"></a> [lambda\_function\_existing\_package\_local](#module\_lambda\_function\_existing\_package\_local) | ../../ | n/a |
| <a name="module_lambda_function_for_each"></a> [lambda\_function\_for\_each](#module\_lambda\_function\_for\_each) | ../../ | n/a |
| <a name="module_lambda_function_no_create_log_group_permission"></a> [lambda\_function\_no\_create\_log\_group\_permission](#module\_lambda\_function\_no\_create\_log\_group\_permission) | ../../ | n/a |
| <a name="module_lambda_function_with_package_deploying_externally"></a> [lambda\_function\_with\_package\_deploying\_externally](#module\_lambda\_function\_with\_package\_deploying\_externally) | ../../ | n/a |
| <a name="module_lambda_layer_local"></a> [lambda\_layer\_local](#module\_lambda\_layer\_local) | ../../ | n/a |
| <a name="module_lambda_layer_s3"></a> [lambda\_layer\_s3](#module\_lambda\_layer\_s3) | ../../ | n/a |
Expand Down
17 changes: 17 additions & 0 deletions examples/complete/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,23 @@ module "lambda_function_with_package_deploying_externally" {
ignore_source_code_hash = true
}

####################################################
# Lambda Function no create log group permission
####################################################

module "lambda_function_no_create_log_group_permission" {
source = "../../"

function_name = "${random_pet.this.id}-lambda-no-create-log-group-permission"
handler = "index.lambda_handler"
runtime = "python3.8"

create_package = false
local_existing_package = "../fixtures/python3.8-zip/existing_package.zip"

attach_create_log_group_permission = false
}

###########
# Disabled
###########
Expand Down
2 changes: 1 addition & 1 deletion iam.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ data "aws_iam_policy_document" "logs" {
effect = "Allow"

actions = compact([
!var.use_existing_cloudwatch_log_group ? "logs:CreateLogGroup" : "",
!var.use_existing_cloudwatch_log_group && var.attach_create_log_group_permission ? "logs:CreateLogGroup" : "",
Copy link
Member

Choose a reason for hiding this comment

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

If you want to reuse the existing cloudwatch log group (from previous runs), set use_existing_cloudwatch_log_group = true.

I am not sure that we need to have an identical variable attach_create_log_group_permission.

Right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The scenario I am dealing with is ephemeral environments. When I first create a new environment the cloudwatch log group does not exist. So I need use_existing_cloudwatch_log_group set to false. When I destroy the environment, the log group may or may not persist. I then want to recreate that environment at a later date which, if the group exists, will fail.

use_existing_cloudwatch_log_group only works if you know the CloudWatch group will be there, I do not.

As stated in the feature request, a work around would be to create the CloudWatch group separately. However, I would rather contain the logic to a single module.

Copy link
Contributor Author

@jake-naughton jake-naughton Nov 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

To be honest, since we provision the CloudWatch group anyway, having CreateLogGroup on the Lambda seems redundant to me unless there is something I'm missing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@antonbabenko thoughts?

"logs:CreateLogStream",
"logs:PutLogEvents"
])
Expand Down
6 changes: 6 additions & 0 deletions variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,12 @@ variable "attach_cloudwatch_logs_policy" {
default = true
}

variable "attach_create_log_group_permission" {
description = "Controls whether to add the create log group permission to the CloudWatch logs policy"
type = bool
default = true
}

variable "attach_dead_letter_policy" {
description = "Controls whether SNS/SQS dead letter notification policy should be added to IAM role for Lambda Function"
type = bool
Expand Down
1 change: 1 addition & 0 deletions wrappers/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ module "wrapper" {
assume_role_policy_statements = try(each.value.assume_role_policy_statements, var.defaults.assume_role_policy_statements, {})
attach_async_event_policy = try(each.value.attach_async_event_policy, var.defaults.attach_async_event_policy, false)
attach_cloudwatch_logs_policy = try(each.value.attach_cloudwatch_logs_policy, var.defaults.attach_cloudwatch_logs_policy, true)
attach_create_log_group_permission = try(each.value.attach_create_log_group_permission, var.defaults.attach_create_log_group_permission, true)
attach_dead_letter_policy = try(each.value.attach_dead_letter_policy, var.defaults.attach_dead_letter_policy, false)
attach_network_policy = try(each.value.attach_network_policy, var.defaults.attach_network_policy, false)
attach_policies = try(each.value.attach_policies, var.defaults.attach_policies, false)
Expand Down