mandating auth_token breaks module for use when not using in-transit encryption

[stephgosling]

Related to, and a result of the merge of #22, it's now impossible to disable in-transit encryption because of the presence of this line in the aws_elasticache_replication_group default resource. If one tries the supplied example from the module you'll see it's currently broken because of this.

I've had a good look at the module and I cannot think of a way to resolve this cleanly:

• setting the default value for transit_encryption_enabled to false still fails if auth_token is in the resource as the provider complains: aws_elasticache_replication_group.default: Error creating Elasticache Replication Group: InvalidParameterValue: The AUTH token is only supported when encryption-in-transit is enabled
• creating a second resource without auth_token is the only way I can think of and then doing some combined magic with count based on var.enabled as well as, say, the length of var.auth_token but even that gets very ugly quickly as all of the dependent resources (e.g. cloudwatch alerts) then break

Any thoughts or a pair of eyes appreciated!

[osterman]

@stephgosling that's unfortunate! Generally, if we have to decide between two tradeoffs, we'll lean towards the more secure tradeoff. In this case, that would be enforcing transit encryption since that would be generally advisable for any multi-az deployment. This would then sacrifice the non-recommend alternative of disabling transit encryption.

Your suggested workaround is the best one I can think of. There might be more options with 0.12, if you're willing to wait it out. @aknysh, @joshmyers would you be alright with the proposed workaround?

The downside with the suggested workaround is that enabling encryption, will cause a destruction of the cluster.

[stephgosling]

@osterman it's a toughie isn't it? on the one hand I hear your security argument but there's a bit of me that is upset because the encrypt-in-transit stuff is an Amazon add-on (wrapping everything in stunnel) and causes much hoop-jumping, which in my case is utterly pointless beyond a rubber stamp for GDPR ;)

FWIW, the docs state that In-transit encryption is optional and can only be enabled on Redis replication groups when they are created so recreation of the resource is a thing anyway.

I'm happy to sit tight as I have forked and removed auth_token, so as you say lets wait and see what 0.12 brings.

Cheers,

Steph

[pnduati]

@stephgosling @osterman any update on this issue?

[aknysh]

@stephgosling @pnduati @osterman this one is not easy. just did some testing and can confirm everything @stephgosling explained. looks like the only way now is to create two resources with auth_token and without (cloudwatch alerts and terraform-aws-route53-cluster-hostname can be updated as well using some coalesce and join things), or wait for TF 0.12

[rverma-nikiai]

Or simply can maintain 2 repos or release two tags with some sed magic to comment that particular line during the release cycle. The idea is to treat both of them as different modules.

[bsAdvanon]

0.12 is here now. Is this on the roadmap of being upgraded to the new version? If yes when? @aknysh @osterman

[aknysh]

@bsAdvanon we are actively working on converting all our modules to TF 0.12 since we need to convert more than 120 modules, it's difficult to say which one we'll convert when, but we are planing on finishing all of them in the next 1-2 weeks (this particular one could be done in a few days)

[nathanielassis]

In terraform 12 its simple:

auth_token = var.enable_encryption_in_transit ? var.auth_token : null

[jfluhmann]

In terraform 12 its simple:

auth_token = var.enable_encryption_in_transit ? var.auth_token : null

This worked perfect for me. I applied it when using the module (until a/the fix is added):

module "redis" {
    source  = "cloudposse/elasticache-redis/aws"
    ....
    at_rest_encryption_enabled = true
    transit_encryption_enabled = false
    auth_token = null
    ....
}

[marcincuber]

You don't require auth_token when transit_encryption_enabled = true so I think a much nicer solution would be to set the following in the module:

variable "auth_token" {
  type        = string
  default     = ""
}

resource "aws_elasticache_replication_group" "redis" {
  engine = "redis"
  transit_encryption_enabled = true
  auth_token                 = var.auth_token != "" ? var.auth_token : null
  ...
}

[aknysh]

thanks everybody for the suggestions, we'll look into it. Why, when calling the module, not just provide auth_token and transit_encryption_enabled as required. auth_token could be set to null if not needed

[aknysh]

@marcincuber @nathanielassis @jfluhmann @bsAdvanon @rverma-nikiai The module has been upgraded to TF 0.12 https://github.com/cloudposse/terraform-aws-elasticache-redis/releases/tag/0.12.0 so the suggestion above (set auth_token to null when not needed) will work, since TF 0.12 considers null as a "missing" value as if it was never provided