Skip to content

Conversation

Nuru
Copy link
Contributor

@Nuru Nuru commented Oct 23, 2021

what

  • Upgrade to new Cloud Posse security group standards
  • Enable proper operation when DNS zone is created at the same time as the cluster
  • Enable create_before_destroy on security groups by default
  • Update AWS provider version constraint to properly require a version that has all the features the module uses
  • Add additional outputs for the Redis Replication Group

why

  • Further standardize all Cloud Posse modules
  • Supply bug fixes and features requested via "issues".

references

Works around Terraform bug with using coalesce() or compact() in resource inputs.

@Nuru Nuru requested review from a team as code owners October 23, 2021 22:29
@Nuru Nuru requested review from Gowiem, jamengual, aknysh, bradj, nitrocode and osterman and removed request for a team October 23, 2021 22:29
@Nuru
Copy link
Contributor Author

Nuru commented Oct 23, 2021

/test all

README.yaml Outdated
If this is not desired behavior, set `transit_encryption_enabled=false`.
This module creates, by default, a new security group for the Elasticache Redis Cluster. When necessary, it will
replace that security group with a new one. In order to allow terraform to fully manage the security group, you
Copy link
Member

Choose a reason for hiding this comment

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

When necessary, it will replace that security group with a new one
Not clear which one is "that" and which one is new (b/c the previous sentence says "This module creates, by default, a new security group").
Can you elaborate on this sentence?

EOT
}
locals {
use_legacy_egress = var.egress_cidr_blocks != null && var.egress_cidr_blocks != ["0.0.0.0/0"] && var.allow_all_egress != true
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
use_legacy_egress = var.egress_cidr_blocks != null && var.egress_cidr_blocks != ["0.0.0.0/0"] && var.allow_all_egress != true
use_legacy_egress = var.egress_cidr_blocks != null && !(length(var.egress_cidr_blocks) == 1 && var.egress_cidr_blocks[0] == "0.0.0.0/0") && var.allow_all_egress == false

Terraform does not support comparing lists using == and != operators, and can appear to work but will produce unexpected results

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.

LGTM, a few comments

@Nuru
Copy link
Contributor Author

Nuru commented Oct 24, 2021

/test all

@Nuru Nuru requested a review from aknysh October 24, 2021 04:56
brian-weis-msr pushed a commit to Measurabl/terraform-aws-elasticache-redis that referenced this pull request Apr 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants