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

fix: prevent creating empty replicas record #126

Merged
merged 3 commits into from
Nov 23, 2021

Conversation

syphernl
Copy link
Contributor

what

  • Prevent creating empty DNS replicas record when cluster_size < 1

why

  • If the cluster_size = 0 this would result in an attempt to create an empty DNS record, which is not permitted by the Route53 API

references

@syphernl syphernl requested review from a team as code owners November 23, 2021 08:37
Copy link

@bridgecrew bridgecrew bot left a comment

Choose a reason for hiding this comment

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

Bridgecrew has found infrastructure configuration errors in this PR ⬇️

main.tf Outdated
@@ -134,7 +134,7 @@ resource "aws_rds_cluster" "primary" {

# https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/rds_cluster#replication_source_identifier
resource "aws_rds_cluster" "secondary" {
count = local.enabled && ! local.is_regional_cluster ? 1 : 0
count = local.enabled && !local.is_regional_cluster ? 1 : 0
Copy link

@bridgecrew bridgecrew bot Nov 23, 2021

Choose a reason for hiding this comment

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

MEDIUM   Ensure RDS instances have backup policy
    Resource: aws_rds_cluster.secondary | ID: BC_AWS_GENERAL_46

How to Fix

resource "aws_rds_cluster" "test" {
  ...
+ backup_retention_period = 35
}

Description

This check examines the attribute **backup_retention_period** this should have a value 1-35, and checks if its set to 0 which would disable the backup.

This check is currently under review and maybe suppressed in future releases.

Dependent Resources



Path Resource Connecting Attribute
/main.tf aws_rds_cluster_instance.default depends_on
cluster coalesce(join(, aws_rds_cluster.primary.*.id), join(, aws_rds_cluster.secondary.*.id)) resource_id

main.tf Outdated
@@ -134,7 +134,7 @@ resource "aws_rds_cluster" "primary" {

# https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/rds_cluster#replication_source_identifier
resource "aws_rds_cluster" "secondary" {
count = local.enabled && ! local.is_regional_cluster ? 1 : 0
count = local.enabled && !local.is_regional_cluster ? 1 : 0
Copy link

@bridgecrew bridgecrew bot Nov 23, 2021

Choose a reason for hiding this comment

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

MEDIUM   Ensure Postgres RDS has Query Logging enabled
    Resource: aws_rds_cluster.secondary | ID: BC_AWS_GENERAL_96

Description

TBD Dependent Resources

Path Resource Connecting Attribute
/main.tf aws_rds_cluster_instance.default depends_on
cluster coalesce(join(, aws_rds_cluster.primary.*.id), join(, aws_rds_cluster.secondary.*.id)) resource_id

main.tf Outdated
@@ -134,7 +134,7 @@ resource "aws_rds_cluster" "primary" {

# https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/rds_cluster#replication_source_identifier
resource "aws_rds_cluster" "secondary" {
count = local.enabled && ! local.is_regional_cluster ? 1 : 0
count = local.enabled && !local.is_regional_cluster ? 1 : 0
Copy link

@bridgecrew bridgecrew bot Nov 23, 2021

Choose a reason for hiding this comment

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

LOW   Ensure RDS clusters have an AWS Backup backup plan
    Resource: aws_rds_cluster.secondary | ID: BC_AWS_GENERAL_49

How to Fix

resource "aws_rds_cluster" "rds_cluster_good" {
  cluster_identifier      = "aurora-cluster-demo"
  engine                  = "aurora-mysql"
  engine_version          = "5.7.mysql_aurora.2.03.2"
  availability_zones      = ["us-west-2a", "us-west-2b", "us-west-2c"]
  database_name           = "mydb"
  master_username         = "foo"
  master_password         = "bar"
}


resource "aws_backup_plan" "example" {
  name = "tf_example_backup_plan"

  rule {
    rule_name         = "tf_example_backup_rule"
    target_vault_name = "vault-name"
    schedule          = "cron(0 12 * * ? *)"
  }
}

resource "aws_backup_selection" "backup_good" {
  iam_role_arn = "arn:partition:service:region:account-id:resource-id"
  name         = "tf_example_backup_selection"
  plan_id      = aws_backup_plan.example.id

  resources = [
    aws_rds_cluster.rds_cluster_good.arn
  ]
}

Description

TBA

Dependent Resources



Path Resource Connecting Attribute
/main.tf aws_rds_cluster_instance.default depends_on
cluster coalesce(join(, aws_rds_cluster.primary.*.id), join(, aws_rds_cluster.secondary.*.id)) resource_id

@max-lobur
Copy link
Contributor

/test all

@max-lobur
Copy link
Contributor

Thanks for your PR @syphernl !
I have created #127 to track BridgeCrew.
This PR is good to go!

@max-lobur max-lobur merged commit 163d3c2 into cloudposse:master Nov 23, 2021
@syphernl syphernl deleted the fix/empty_readers_record branch November 23, 2021 21:18
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.

3 participants