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

[Bug]: cannot enable transit encryption on aws_elasticache_replication_group (engine=redis) #33403

Closed
jpriebe opened this issue Sep 11, 2023 · 12 comments · Fixed by #33451
Closed
Assignees
Labels
bug Addresses a defect in current functionality. prioritized Part of the maintainer teams immediate focus. To be addressed within the current quarter. regression Pertains to a degraded workflow resulting from an upstream patch or internal enhancement. service/elasticache Issues and PRs that pertain to the elasticache service.
Milestone

Comments

@jpriebe
Copy link

jpriebe commented Sep 11, 2023

Terraform Core Version

1.2.9

AWS Provider Version

5.16.1

Affected Resource(s)

  • aws_elasticache_cluster
  • aws_elasticache_replication_group

Note: the title of this issue may be a little misleading. Technically, you can create an elasticache redis replication group with transit encryption enabled. The problem is that every apply after that will replace the cluster. A resource that gets replaced on every apply (especially a stateful resource) is not a very useful resource. So in effect, you really can't create a replication group with transit encryption enabled.

Expected Behavior

Our code provisions an aws_elasticache_cluster as a member of an aws_elasticache_replication_group (redis). We set

transit_encryption_enabled = true

on the aws_elasticache_replication_group. For elasticache redis, transit encryption must be enabled at the replication group level.

No code changes were made to our elasticache terraform. We do not expect to see any changes to our existing clusters.

Actual Behavior

When we ran our pipelines today, we pulled down version 5.16.1 of the provider, and the plan indicated that it wants to replace the aws_elasticache_cluster.

  # module.platform-tenant[0].module.elasticache["foo"].aws_elasticache_cluster.redis must be replaced
-/+ resource "aws_elasticache_cluster" "redis" {
     ...
      ~ transit_encryption_enabled = true -> false # forces replacement
 }

If you apply this change, your next plan will indicate the same change. You will have to replace the cluster every time you apply.

Relevant Error/Panic Output Snippet

No response

Terraform Configuration Files

Any basic aws_elasticache_replication_group will suffice to replicate. You just have to set transit_encryption_enabled to true in the replication group and leave it unspecified in the cluster (this has been the way to enable transit encryption for redis clusters).

resource "aws_elasticache_replication_group" "this" {
  ...
  transit_encryption_enabled = true
}

resource "aws_elasticache_cluster" "this" {
  cluster_id           = "foo"
  replication_group_id = aws_elasticache_replication_group.this.id
}

Steps to Reproduce

Use AWS provider 5.16.0 or up.

Apply your replication group and cluster terraform, and you will get a cluster with transit encryption enabled. Then run a plan (with no code changes), and you will see a change that forces replacement of the cluster.

Debug Output

No response

Panic Output

No response

Important Factoids

No response

References

No response

Would you like to implement a fix?

None

@jpriebe jpriebe added the bug Addresses a defect in current functionality. label Sep 11, 2023
@github-actions
Copy link

Community Note

Voting for Prioritization

  • Please vote on this issue by adding a 👍 reaction to the original post to help the community and maintainers prioritize this request.
  • Please see our prioritization guide for information on how we prioritize.
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.

Volunteering to Work on This Issue

  • If you are interested in working on this issue, please leave a comment.
  • If this would be your first contribution, please review the contribution guide.

@github-actions github-actions bot added the service/elasticache Issues and PRs that pertain to the elasticache service. label Sep 11, 2023
@terraform-aws-provider terraform-aws-provider bot added the needs-triage Waiting for first response or review from a maintainer. label Sep 11, 2023
@jpriebe
Copy link
Author

jpriebe commented Sep 11, 2023

I thought the workaround would be to simply add

  transit_encryption_enabled = true

to the aws_elasticache_cluster. But that resulted in this error:

Error: aws_elasticache_cluster does not support transit encryption using the redis engine, use aws_elasticache_replication_group instead

@alex-phillips
Copy link

I've encountered this issue as well. We do not specify the variable in our case and even after recreating the cluster, the next plan sees the value set to true and wants to force a new replacement. This seems to be never-ending and terraform is never happy with the existing cluster.

@jpriebe
Copy link
Author

jpriebe commented Sep 11, 2023

I think that in the case of an elasticache redis cluster with transit_encryption_enabled=true in the replication group (and no explicit transit_encryption_enabled in the cluster resource, the provider's resourceClusterRead() function is getting a value of true here.

Because it defaults to false, when you run a plan, terraform is showing a change of this property fromtrue to false.

AFAIK, the only way to enable transit encryption for an elasticache redis cluster is to do it at the replication group level, which we have done. But with this latest code change, every plan will want to replace the cluster. This is a big problem.

Also, I don't think that replacing the cluster in this scenario makes sense. If this property can't be used for elasticache redis to control transit encryption, why should a change to its value force a replacement of the cluster?

@rgopathoti
Copy link

rgopathoti commented Sep 11, 2023

I too facing the same issue,
we have delared "transit_encryption_enabled = true" in aws_elasticache_replication_group resource block , but my plan trying to overwrite it " ~ transit_encryption_enabled = true -> false # forces replacement"

@jpriebe jpriebe changed the title [Bug]: missing property of aws_elasticache_cluster forces replacement of cluster [Bug]: cannot enable transit encryption on aws_elasticache_replication_group (engine=redis) Sep 11, 2023
@justinretzolk
Copy link
Member

justinretzolk commented Sep 11, 2023

Hey @jpriebe 👋 Thank you for taking the time to raise this! To make sure that I understand completely, was this working in a previous version of the AWS provider, and has since stopped working?

Edit: I see that transit_encryption_enabled was added to the aws_elasticache_cluster resource in 5.16.0, with #26987, please disregard the above question.

@justinretzolk justinretzolk added regression Pertains to a degraded workflow resulting from an upstream patch or internal enhancement. and removed needs-triage Waiting for first response or review from a maintainer. labels Sep 11, 2023
@terraform-aws-provider terraform-aws-provider bot added the prioritized Part of the maintainer teams immediate focus. To be addressed within the current quarter. label Sep 11, 2023
@jar-b
Copy link
Member

jar-b commented Sep 12, 2023

Hello everyone - apologies for the regression this change introduced. For those currently impacted and awaiting a fix, the ignore_changes lifecycle argument may help to prevent the cluster from being re-created in the meantime.

resource "aws_elasticache_cluster" "example" {
  # ...

  lifecycle {
    ignore_changes = [
      transit_encryption_enabled,
    ]
  }
}

@jpriebe
Copy link
Author

jpriebe commented Sep 12, 2023

Hello everyone - apologies for the regression this change introduced. For those currently impacted and awaiting a fix, the ignore_changes lifecycle argument may help to prevent the cluster from being re-created in the meantime.

resource "aws_elasticache_cluster" "example" {
  # ...

  lifecycle {
    ignore_changes = [
      transit_encryption_enabled,
    ]
  }
}

thanks, @jar-b. That looks promising.

For the time being, we're hoping for a speedy fix, so we have just constrained our provider version to < 5.16.0. When the fix is released, we'll remove that constraint.

@Atken
Copy link

Atken commented Sep 12, 2023

Hello everyone - apologies for the regression this change introduced. For those currently impacted and awaiting a fix, the ignore_changes lifecycle argument may help to prevent the cluster from being re-created in the meantime.

resource "aws_elasticache_cluster" "example" {
  # ...

  lifecycle {
    ignore_changes = [
      transit_encryption_enabled,
    ]
  }
}

I had attempted this as a temp workaround but, unfortunately, it doesn't work. With this lifecycle argument in place a plan results in "Error: aws_elasticache_cluster does not support transit encryption using the redis engine, use aws_elasticache_replication_group instead"

Placing the same lifecycle argument into the replication group resource has no effect. Terraform still attempts to change the transit_encryption_enabled value to false which forces a replacement.

As jpriebe mentioned we've just limited our provider for now which alleviates the issue until a fix can be implemented. Just wanted to share that lifecycle doesn't provide the anticipated result

@jar-b
Copy link
Member

jar-b commented Sep 12, 2023

🤦 - thanks @Atken, forgot about the CustomizeDiff functions that were implemented around this which happen regardless of whether that lifecycle argument is set.

In the case of redis engine types I suspect this attribute is technically computed. It can't be set during Create/Update operations, but if the cluster is used with a replication group where the feature is enabled it returns true. With those conditions we'll have to reconsider the CustomizeDiff function logic, and potentially whether we read this attribute at all or just rely on what is configured.

@jar-b jar-b self-assigned this Sep 12, 2023
@github-actions github-actions bot added this to the v5.17.0 milestone Sep 14, 2023
@github-actions github-actions bot removed the bug Addresses a defect in current functionality. label Sep 14, 2023
@github-actions
Copy link

This functionality has been released in v5.17.0 of the Terraform AWS Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 15, 2023
@justinretzolk justinretzolk added the bug Addresses a defect in current functionality. label Feb 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Addresses a defect in current functionality. prioritized Part of the maintainer teams immediate focus. To be addressed within the current quarter. regression Pertains to a degraded workflow resulting from an upstream patch or internal enhancement. service/elasticache Issues and PRs that pertain to the elasticache service.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants