Issue with replication_group_id

Daemoen commented 4 months ago

Description

Default setting of var.engine appears to conflict with replication_group_id. Same as #6 which is now closed with no validation.

[x] ✋ I have searched the open/closed issues and my issue is not listed

Versions

Module version [Required]:

Terraform version:


╰─ tf --version
Terraform v1.9.0
on darwin_arm64

provider registry.terraform.io/hashicorp/aws v5.53.0
provider registry.terraform.io/hashicorp/cloudinit v2.3.4
provider registry.terraform.io/hashicorp/helm v2.14.0
provider registry.terraform.io/hashicorp/kubernetes v2.31.0
provider registry.terraform.io/hashicorp/null v3.2.2
provider registry.terraform.io/hashicorp/random v3.6.2
provider registry.terraform.io/hashicorp/time v0.11.2
provider registry.terraform.io/hashicorp/tls v4.0.5

Reproduction Code [Required]

elasticache.tf:

module "elasticache" {
  source = "github.com/terraform-aws-modules/terraform-aws-elasticache?ref=a483023ae9800c047d63206da7676afa79f1d82c"

  for_each = var.ec_instances

  cluster_id                 = "${local.name}-${each.value.cluster_id}-ec"
  create_cluster             = true
  cluster_mode_enabled       = false
  engine_version             = each.value.engine_version
  node_type                  = each.value.node_type
  create_replication_group   = each.value.create_replication_group
  replication_group_id       = each.value.create_replication_group ? each.value.replication_group_id : null
  at_rest_encryption_enabled = true
  transit_encryption_enabled = each.value.transit_encryption_enabled
  maintenance_window         = each.value.maintenance_window
  apply_immediately          = true

  # Replication Group Settings
  num_cache_clusters          = 2
  automatic_failover_enabled  = true
  multi_az_enabled            = true
  preferred_cache_cluster_azs = local.preferred_azs

  # Security group
  vpc_id = module.vpc.vpc_id
  security_group_rules = {
    for cidr in module.vpc.private_subnets_cidr_blocks : "ingress_${cidr}" => {
      description = "Allow traffic from ${cidr}"
      cidr_ipv4   = cidr
    }
  }

  # Subnet Group
  subnet_ids = module.vpc.elasticache_subnet_ids

  # Parameter Group
  create_parameter_group = true
  parameter_group_family = "redis7"
  parameters = [
    {
      name  = "latency-tracking"
      value = "yes"
    }
  ]

  tags = merge(local.tags, {
    Name = each.value.cluster_id
  })
}

envs/stg.tfvars (ec settings)

#####
# Elasticache Settings
#####
ec_instances = {
  instance1 = {
    cluster_id                 = "instance1"
    node_type                  = "cache.t4g.small"
    engine_version             = "7.1"
    create_replication_group   = true
    replication_group_id       = "instance1-stg-redis"
    multi_az_enabled           = true
    transit_encryption_enabled = true
    maintenance_window         = "sun:22:00-mon:00:00"
  }
}

Steps to reproduce the behavior:

Yes, using workspaces. Have not changed between them at all, however. Yes, I have cleared the local cache. tf plan -var-file=envs/stg.tfvars to plan the changeset

Expected behavior

Changeset validates and creates the cluster with the correct replication group settings

Actual behavior

Changeset complains that engine and replication_group_id cannot be set together.

Terminal Output Screenshot(s)

Planning failed. Terraform encountered an error while generating this plan.

╷
│ Error: Invalid combination of arguments
│
│   with module.elasticache["instance1"].aws_elasticache_cluster.this[0],
│   on .terraform/modules/elasticache/main.tf line 23, in resource "aws_elasticache_cluster" "this":
│   23:   engine                     = var.engine
│
│ "engine": only one of `engine,replication_group_id` can be specified, but `engine,replication_group_id` were specified.
╵
╷
│ Error: Invalid combination of arguments
│
│   with module.elasticache["instance1"].aws_elasticache_cluster.this[0],
│   on .terraform/modules/elasticache/main.tf line 49, in resource "aws_elasticache_cluster" "this":
│   49:   replication_group_id         = var.create && var.create_replication_group ? aws_elasticache_replication_group.this[0].id : var.replication_group_id
│
│ "replication_group_id": only one of `engine,replication_group_id` can be specified, but `engine,replication_group_id` were specified.

Additional context

Daemoen commented 4 months ago

It's also worth noting that with the above configuration, it does create a cluster_address, but because it is redis, that's not a valid output, even though its the returned result in this case there are no primary/replicas, so it generates cluster instead...

Daemoen commented 4 months ago

Also noticed that encryption at rest is not being set. Seems that numerous of the values that I am providing are being completely ignored. (At least when viewed in the console)

Daemoen commented 4 months ago

I spent some time looking into the code. All encryption settings are enabled in replication groups only. Unfortunately, it kind of agrees with the aws documentation, but it does also disagree with it, depending on the pages you look at. The docs are a bit of a mess. The bug with replication_group_id and engine is definitely still a major issue.

antonbabenko commented 3 months ago

This issue has been resolved in version 1.2.2 :tada:

github-actions[bot] commented 2 months ago

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

terraform-aws-modules / terraform-aws-elasticache