cloudposse / terraform-aws-elasticache-redis

Terraform module to provision an ElastiCache Redis Cluster
https://cloudposse.com/accelerate
Apache License 2.0
143 stars 248 forks source link

feat: Allow configuring transit_encryption_mode #231

Closed amontalban closed 5 months ago

amontalban commented 6 months ago

what

Allow configuring transit_encryption_mode.

why

This was added in AWS Provider v5.47.0 as part of https://github.com/hashicorp/terraform-provider-aws/pull/30403

This is needed if you want to migrate to in-transit encryption with no downtime.

references

PLeS207 commented 6 months ago

These changes were released in v1.4.0.

gberenice commented 5 months ago

/terratest

gberenice commented 5 months ago

@amontalban tests are failing:

│ Error: creating ElastiCache Replication Group (eg-test-redis-test-34242): InvalidParameterCombination: Transit encryption mode is not supported for engine version 6.2.6. Please use engine version 7.0.5 or higher.

Could you please update values in examples/complete https://github.com/cloudposse/terraform-aws-elasticache-redis/blob/e2ed69974e224c728274389e7ec2f8ea619f7dca/examples/complete/fixtures.us-east-2.tfvars to match expected configuration?

nitrocode commented 5 months ago

/terratest

nitrocode commented 5 months ago

/terratest

nitrocode commented 5 months ago

Underlying: error while running command: exit status 1; ╷ │ Error: creating ElastiCache Parameter Group (eg-test-redis-test-8876-redis7-2): InvalidParameterValue: CacheParameterGroupFamily redis7.2 is not a valid parameter group family.

https://docs.aws.amazon.com/AmazonElastiCache/latest/red-ug/ParameterGroups.Redis.html#ParameterGroups.Redis.7

nitrocode commented 5 months ago

/terratest

nitrocode commented 5 months ago

/terratest

nitrocode commented 5 months ago

/terratest

nitrocode commented 5 months ago

I was able to fix the tests. Please rebuild the readme with the addition of the variable.

Laakso commented 5 months ago

@amontalban LGTM apart from the readme rebuild 👍

mergify[bot] commented 5 months ago

💥 This pull request now has conflicts. Could you fix it @amontalban? 🙏

amontalban commented 5 months ago

Thanks @nitrocode 🙏

@Laakso README has been updated.

nitrocode commented 5 months ago

/terratest

nitrocode commented 5 months ago

One thing we forgot to do was bump the minimum aws provider version to 5.47.0 in versions.tf

z0rc commented 5 months ago

Setting this argument to preferred by default is wrong, it should be null. Alternatively you can conditionally set this argument in terraform resource only when var.transit_encryption_enabled == true

I have module deployment that has transit_encryption_enabled = false (because software doesn't support tls for redis). Applying updated module now results in:

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # module.redis.aws_elasticache_replication_group.default[0] will be updated in-place
  ~ resource "aws_elasticache_replication_group" "default" {
        id                         = "core-01-harbor"
        tags                       = {
            "Name" = "core-01-harbor"
        }
      + transit_encryption_mode    = "preferred"
        # (36 unchanged attributes hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.

Do you want to perform these actions in workspace "core-01"?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

module.redis.aws_elasticache_replication_group.default[0]: Modifying... [id=core-01-harbor]
module.redis.aws_elasticache_replication_group.default[0]: Still modifying... [id=core-01-harbor, 10s elapsed]
module.redis.aws_elasticache_replication_group.default[0]: Still modifying... [id=core-01-harbor, 20s elapsed]
module.redis.aws_elasticache_replication_group.default[0]: Still modifying... [id=core-01-harbor, 30s elapsed]
╷
│ Error: modifying ElastiCache Replication Group (core-01-harbor): InvalidParameterCombination: To modify transit encryption mode, set encryption-in-transit as enabled.
│   status code: 400, request id: d7341ee0-06ea-4e33-8eb7-bfdb54189d7f
│
│   with module.redis.aws_elasticache_replication_group.default[0],
│   on .terraform/modules/redis/main.tf line 157, in resource "aws_elasticache_replication_group" "default":
│  157: resource "aws_elasticache_replication_group" "default" {
│
╵
z0rc commented 5 months ago

Also I have number of existing redis clusters with engine version 6.2.6, update to 7.x hasn't happened yet. How this case should be handled with module parameters now?

amontalban commented 5 months ago

Setting this argument to preferred by default is wrong, it should be null. Alternatively you can conditionally set this argument in terraform resource only when var.transit_encryption_enabled == true

I have module deployment that has transit_encryption_enabled = false (because software doesn't support tls for redis). Applying updated module now results in:

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # module.redis.aws_elasticache_replication_group.default[0] will be updated in-place
  ~ resource "aws_elasticache_replication_group" "default" {
        id                         = "core-01-harbor"
        tags                       = {
            "Name" = "core-01-harbor"
        }
      + transit_encryption_mode    = "preferred"
        # (36 unchanged attributes hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.

Do you want to perform these actions in workspace "core-01"?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

module.redis.aws_elasticache_replication_group.default[0]: Modifying... [id=core-01-harbor]
module.redis.aws_elasticache_replication_group.default[0]: Still modifying... [id=core-01-harbor, 10s elapsed]
module.redis.aws_elasticache_replication_group.default[0]: Still modifying... [id=core-01-harbor, 20s elapsed]
module.redis.aws_elasticache_replication_group.default[0]: Still modifying... [id=core-01-harbor, 30s elapsed]
╷
│ Error: modifying ElastiCache Replication Group (core-01-harbor): InvalidParameterCombination: To modify transit encryption mode, set encryption-in-transit as enabled.
│     status code: 400, request id: d7341ee0-06ea-4e33-8eb7-bfdb54189d7f
│
│   with module.redis.aws_elasticache_replication_group.default[0],
│   on .terraform/modules/redis/main.tf line 157, in resource "aws_elasticache_replication_group" "default":
│  157: resource "aws_elasticache_replication_group" "default" {
│
╵

Hey @z0rc thank you for catching that issue, I was blindsided with my use case that I forgot to test it with it.

I have created #238, can you check if it works with that, please?

Thanks!

snieg commented 5 months ago

I have exactly the same issue. (var.transit_encryption_enabled is set to false)

Error: modifying ElastiCache Replication Group (re-unified-staging-ld-relay-cache): InvalidParameterCombination: To modify transit encryption mode, set encryption-in-transit as enabled.
    status code: 400, request id: ded6f0d1-9bd0-4673-8174-bafc1127e00d

  with module.ld_relay_redis.aws_elasticache_replication_group.default[0],
  on .terraform/modules/ld_relay_redis/main.tf line 157, in resource "aws_elasticache_replication_group" "default":
 157: resource "aws_elasticache_replication_group" "default" {