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

Cannot assign DB replica to different subnet #145

Open
tmeijn opened this issue Sep 29, 2022 · 4 comments
Open

Cannot assign DB replica to different subnet #145

tmeijn opened this issue Sep 29, 2022 · 4 comments
Labels
bug 🐛 An issue with the system

Comments

@tmeijn
Copy link

tmeijn commented Sep 29, 2022

Describe the Bug

In version 0.38.7 you could specify different subnets for the replicas. This would result in a db subnet group being created specifically for the replica and assigned to it:

image

In version 0.38.8 this is no longer possible due to the logic introduced in PR #142

Now a replica configuration will always evaluate to null, which in my case causes the replica to take on the DB subnet group of the primary instance.

Use case

Adobe Experience Platform Postgres Connector requires a publicly accessible database, so we have decided to create a replica in a public subnet, while the primary is in the private subnet. Version 0.38.7 allows this configuration and works, while version 0.38.8 does not.

Expected Behavior

I expect that when I specify subnets to the module, a DB subnet will be created and assigned to the replica.

Steps to Reproduce

Module configuration:

module "rds_replica_analytics" {
  source     = "cloudposse/rds/aws"
  version    = "0.38.7"
  context    = module.default_label.context
  name       = "analytics"
  attributes = ["db", "readonly"]

  # Instance config
  replicate_source_db             = module.rds_instance.instance_id
  instance_class                  = "db.t4g.micro"
  storage_type                    = "gp2"
  storage_encrypted               = true
  kms_key_arn                     = aws_kms_key.kms_rds_key.arn
  publicly_accessible             = true
  auto_minor_version_upgrade      = false
  allow_major_version_upgrade     = false
  apply_immediately               = true
  skip_final_snapshot             = true
  copy_tags_to_snapshot           = true
  backup_retention_period         = 2
  backup_window                   = "02:30-03:30"
  maintenance_window              = "mon:03:30-mon:04:30"
  enabled_cloudwatch_logs_exports = ["postgresql"]

  # Network config
  vpc_id              = module.vpc.vpc_id
  subnet_ids          = module.dynamic_subnets.public_subnet_ids // version `0.38.8` now ignores this
  dns_zone_id         = module.core_zone.zone_id
  host_name           = "db-replica"
  allowed_cidr_blocks = var.rds_replica_allowed_cidrs
  security_group_ids  = [module.bastion.security_group_id]

  # Database config
  db_parameter_group = "postgres14"
  database_port      = 5432

  # Should not be needed, bug in module PR
  engine         = "postgres"
  engine_version = "14.2"
}
@tmeijn tmeijn added the bug 🐛 An issue with the system label Sep 29, 2022
@nitrocode
Copy link
Member

Instead of reverting pr 142, we should find a way to allow both null subnets for replicas and allow specifying subnets for replicas.

@Benbentwo could you look into this? Perhaps what we need is var.subnet_ids for the primary and var.replica_subnet_ids as an escape hatch for this use case which could default to null.

@tmeijn
Copy link
Author

tmeijn commented Sep 29, 2022

@nitrocode If I understand the previous behavior correctly:

db_subnet_group_name = local.db_subnet_group_name_provided ? var.db_subnet_group_name : (
    local.subnet_ids_provided ? join("", aws_db_subnet_group.default.*.name) : null
  )

if local.subnet_ids_provided is false, and no external db_subnet is provided, then it returns null

subnet_ids_provided = var.subnet_ids != null && length(var.subnet_ids) > 0

var.subnet_ids is default []

So not providing subnet_ids to the module should evaluate to the same behavior PR #142 added, while also allowing the case that is described in this issue.

@tmeijn
Copy link
Author

tmeijn commented Oct 31, 2022

@nitrocode friendly reminder.

@nitrocode
Copy link
Member

I unfortunately do not have the time at the moment but let me cc other folks in who may be able to review.

cc @aknysh @milldr

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 An issue with the system
Projects
None yet
Development

No branches or pull requests

2 participants