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

feat: Allow configuring transit_encryption_mode #231

Merged
merged 10 commits into from
Jun 14, 2024

Conversation

amontalban
Copy link
Contributor

@amontalban amontalban commented May 23, 2024

what

Allow configuring transit_encryption_mode.

why

This was added in AWS Provider v5.47.0 as part of hashicorp/terraform-provider-aws#30403

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

references

@amontalban amontalban requested review from a team as code owners May 23, 2024 21:37
@mergify mergify bot added the triage Needs triage label May 23, 2024
@PLeS207
Copy link

PLeS207 commented May 24, 2024

These changes were released in v1.4.0.

@gberenice
Copy link
Contributor

/terratest

@gberenice
Copy link
Contributor

@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
Copy link
Member

/terratest

@nitrocode
Copy link
Member

/terratest

@nitrocode
Copy link
Member

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
Copy link
Member

/terratest

@nitrocode
Copy link
Member

/terratest

@nitrocode
Copy link
Member

/terratest

@nitrocode
Copy link
Member

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

@Laakso
Copy link

Laakso commented Jun 12, 2024

@amontalban LGTM apart from the readme rebuild 👍

Copy link
Contributor

mergify bot commented Jun 14, 2024

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

@mergify mergify bot added the conflict This PR has conflicts label Jun 14, 2024
@mergify mergify bot removed the conflict This PR has conflicts label Jun 14, 2024
@amontalban
Copy link
Contributor Author

Thanks @nitrocode 🙏

@Laakso README has been updated.

@nitrocode
Copy link
Member

/terratest

@nitrocode nitrocode enabled auto-merge (squash) June 14, 2024 06:29
@mergify mergify bot removed the triage Needs triage label Jun 14, 2024
@nitrocode nitrocode merged commit 35d13d1 into cloudposse:main Jun 14, 2024
35 checks passed
@nitrocode
Copy link
Member

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

@z0rc
Copy link

z0rc commented Jun 14, 2024

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
Copy link

z0rc commented Jun 14, 2024

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
Copy link
Contributor Author

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
Copy link

snieg commented Jun 14, 2024

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" {

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.

Enable encryption in transit preferred
7 participants