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

Add enabled toggle #36

Merged
merged 1 commit into from
Aug 3, 2021
Merged

Add enabled toggle #36

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
26 changes: 17 additions & 9 deletions main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
data "aws_caller_identity" "current" {}

locals {
kms_key_id = (var.encryption_enabled && var.kms_master_key_id != "") ? var.kms_master_key_id : ""
enabled = module.this.enabled

kms_key_id = local.enabled && var.encryption_enabled && var.kms_master_key_id != "" ? var.kms_master_key_id : ""
}

resource "aws_sns_topic" "this" {
count = local.enabled ? 1 : 0

name = module.this.id
display_name = replace(module.this.id, ".", "-") # dots are illegal in display names and for .fifo topics required as part of the name (AWS SNS by design)
kms_master_key_id = local.kms_key_id
Expand All @@ -15,9 +19,9 @@ resource "aws_sns_topic" "this" {
}

resource "aws_sns_topic_subscription" "this" {
for_each = var.subscribers
for_each = local.enabled ? var.subscribers : {}
Copy link
Member

Choose a reason for hiding this comment

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

@nitrocode did you test it with enabled=false?
usually in this case terraform complains that both sides of the ? operator must have the same types (not only it must be a map, but all items must be the same with the same types)

Copy link
Member Author

Choose a reason for hiding this comment

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

@aknysh var.subscribers is a map so both types are the same.

$ git clone [email protected]:cloudposse/terraform-aws-sns-topic.git
$ cd terraform-aws-sns-topic
$ terraform init
$ echo '\nenabled = false\n' >> fixtures.us-east-2.tfvars
$ terraform plan -var-file=fixtures.us-east-2.tfvars
Refreshing Terraform state in-memory prior to plan...
The refreshed state will be used to calculate this plan, but will not be
persisted to local or remote state storage.

module.sns.data.aws_caller_identity.current: Refreshing state...

------------------------------------------------------------------------

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:

Terraform will perform the following actions:

Plan: 0 to add, 0 to change, 0 to destroy.

Copy link
Member

Choose a reason for hiding this comment

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

var.subscribers is a map so both types are the same

yes, but in similar cases Terraform complains that not only the container types must be the same (maps), but they must have the the items and the same item types

Copy link
Member Author

Choose a reason for hiding this comment

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

What would you propose to use here instead ?


topic_arn = aws_sns_topic.this.arn
topic_arn = join("", aws_sns_topic.this.*.arn)
protocol = var.subscribers[each.key].protocol
endpoint = var.subscribers[each.key].endpoint
endpoint_auto_confirms = var.subscribers[each.key].endpoint_auto_confirms
Expand All @@ -27,16 +31,20 @@ resource "aws_sns_topic_subscription" "this" {
}

resource "aws_sns_topic_policy" "this" {
arn = aws_sns_topic.this.arn
policy = length(var.sns_topic_policy_json) > 0 ? var.sns_topic_policy_json : data.aws_iam_policy_document.aws_sns_topic_policy.json
count = local.enabled ? 1 : 0

arn = join("", aws_sns_topic.this.*.arn)
policy = length(var.sns_topic_policy_json) > 0 ? var.sns_topic_policy_json : join("", data.aws_iam_policy_document.aws_sns_topic_policy.*.json)
}

data "aws_iam_policy_document" "aws_sns_topic_policy" {
count = local.enabled ? 1 : 0

policy_id = "SNSTopicsPub"
statement {
effect = "Allow"
actions = ["sns:Publish"]
resources = [aws_sns_topic.this.arn]
resources = aws_sns_topic.this.*.arn

dynamic "principals" {
for_each = length(var.allowed_aws_services_for_sns_published) > 0 ? ["_enable"] : []
Expand All @@ -60,7 +68,7 @@ data "aws_iam_policy_document" "aws_sns_topic_policy" {

# TODO enable when PR gets merged https://github.com/terraform-providers/terraform-provider-aws/issues/10931
resource "aws_sqs_queue" "dead_letter_queue" {
count = var.sqs_dlq_enabled ? 1 : 0
count = local.enabled && var.sqs_dlq_enabled ? 1 : 0

name = module.this.id
max_message_size = var.sqs_dlq_max_message_size
Expand All @@ -71,7 +79,7 @@ resource "aws_sqs_queue" "dead_letter_queue" {
}

data "aws_iam_policy_document" "sqs-queue-policy" {
count = var.sqs_dlq_enabled ? 1 : 0
count = local.enabled && var.sqs_dlq_enabled ? 1 : 0

policy_id = "${join("", aws_sqs_queue.dead_letter_queue.*.arn)}/SNSDeadLetterQueue"

Expand All @@ -88,7 +96,7 @@ data "aws_iam_policy_document" "sqs-queue-policy" {
condition {
test = "ArnEquals"
variable = "aws:SourceArn"
values = [aws_sns_topic.this.arn]
values = aws_sns_topic.this.*.arn
}
}
}