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.

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

merged 1 commit into from
Aug 3, 2021

Conversation

nitrocode
Copy link
Member

what

  • Add enabled toggle

why

  • Only if enabled=true will this module create resources

references

N/A

@nitrocode nitrocode requested review from a team as code owners August 3, 2021 13:51
@nitrocode nitrocode requested review from srhopkins and brcnblc August 3, 2021 13:51
@nitrocode
Copy link
Member Author

/test 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 ?

Copy link
Member

@aknysh aknysh left a comment

Choose a reason for hiding this comment

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

please see comments

@nitrocode nitrocode requested a review from aknysh August 3, 2021 15:01
@aknysh aknysh merged commit c61835f into master Aug 3, 2021
@aknysh aknysh deleted the enabled branch August 3, 2021 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants