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

aws_elasticache_cluster Encryption in-transit + Encryption at-rest arguments #2823

Closed
thalesac opened this issue Jan 2, 2018 · 11 comments
Closed
Labels
enhancement Requests to existing resources that expand the functionality or scope. good first issue Call to action for new contributors looking for a place to start. Smaller or straightforward issues. service/elasticache Issues and PRs that pertain to the elasticache service.

Comments

@thalesac
Copy link

thalesac commented Jan 2, 2018

Terraform Version

0.11.1

Affected Resource(s)

aws_elasticache_cluster

Expected Behavior

aws_elasticache_cluster should support encryption in-transit + encryption at-rest parameters.

Actual Behavior

Those parameters doesn't exist

Important Factoids

Enabling encryption in-transit / at-rest can only be done when creating a Redis cluster using Redis version 3.2.6 only

References

https://www.terraform.io/docs/providers/aws/r/elasticache_cluster.html
https://docs.aws.amazon.com/AmazonElastiCache/latest/UserGuide/at-rest-encryption.html
https://docs.aws.amazon.com/AmazonElastiCache/latest/UserGuide/in-transit-encryption.html

@jen20 jen20 added enhancement Requests to existing resources that expand the functionality or scope. good first issue Call to action for new contributors looking for a place to start. Smaller or straightforward issues. labels Jan 2, 2018
@radeksimko radeksimko added the service/elasticache Issues and PRs that pertain to the elasticache service. label Jan 28, 2018
@vpadronblanco
Copy link
Contributor

Hi, I tried asking the following through the gitter room to have no answer. I am new to contributing and I am interested in this project. I had two questions about the project's source code and others on the scope of the issue.

  1. In 'terraform-providers-aws/aws', what are the prefixes for the resources used for? 'resource_aws_', 'data_source_aws_', and 'import_aws_'. I have my guesses but overall I am asking as to what is the convention for contributing towards new features
  2. The CRUD functions often have a parameter 'meta'. Where is that instantiated? My guess it comes from the metadata that is parsed from the terraform file or the terraform 'state'.
  3. With respect to this project: What is the scope of this issue? I am understanding that an update on this resource (to toggle in-transit and at-rest encryption) would require to backup the existing cluster, and restore from backup.

Any help is greatly appreciated :)

@bflad
Copy link
Contributor

bflad commented Feb 1, 2018

Hi @vpadronblanco! Very excited to see your interest here. I can try to help answer your questions here. (I was actually in the process of getting myself on Gitter this morning, funny enough.)

In 'terraform-providers-aws/aws', what are the prefixes for the resources used for? 'resource_aws_', 'data_source_aws_', and 'import_aws_'. I have my guesses but overall I am asking as to what is the convention for contributing towards new features

Your guesses are probably right. 😄 The naming prefixes are just a simple flat structure to keep the resources/datasources organized without too much overhead of needing to import other Go packages within the project itself. We have been talking recently about potentially adding some more structure, but there are no definite plans for this and I can point you towards those discussions if interested.

Briefly, the files for resources are:

  • resource_aws_X.go - actual implementation
  • resource_aws_X_test.go - unit and acceptance testing for above implementation
  • provider.go - a line to let the provider to know about the resource code
  • website/docs/r/X.html.go - website documentation for the resource
  • website/aws.erb - a few lines to link the resource documentation to the sidebar navigation

You may see some cases where we have broken out various pieces of resource code into other files like structure.go, validators.go, etc. We're currently very inconsistent with these and have not set out a "style guide" to prescribe usage like that (at least that I have seen). In general and until we formalize any structure guidelines are in place, its safe to assume everything is okay in just the resource_aws_X.go if its only for that resource.

The CRUD functions often have a parameter 'meta'. Where is that instantiated? My guess it comes from the metadata that is parsed from the terraform file or the terraform 'state'.

That comes from Terraform core. In general, you don't need to worry about it except for the fact it contains our *AWSClient to make API connections, e.g. conn := meta.(*AWSClient).acmconn

With respect to this project: What is the scope of this issue? I am understanding that an update on this resource (to toggle in-transit and at-rest encryption) would require to backup the existing cluster, and restore from backup.

In that case (I have not investigated the AWS-side implementation details here), an acceptable first implementation might mark the new encryption attribute(s) as ForceNew: true to tell Terraform that it must recreate the resource, instead of trying to implement all the "complicated" logic mentioned above. Enhancements can always be made after the first implementation to get it working in the "this works when creating a new resource" and "the resource understands on reading whether or not its encrypted currently".

@vpadronblanco
Copy link
Contributor

vpadronblanco commented Feb 1, 2018

What I see between the 'data_source_aws_' and 'resource_aws_' is that they have the same schema, the latter has the CRUD functions but sometimes for the same resource, the datasource is missing.

That comes from Terraform core

Yes, I got up to the Apply method (on a Provider reference) in hashicorp/terraform/helper/schema/resource.go; but couldn't find anything higher. At any rate, your explanation on 'meta' works fine for the time being (and thank you for that)

For the rest, I will be working on an MVP to at least have the cluster created with those specs.

Thank you so much for your help and excuse me if I bothered with these questions.

@vpadronblanco
Copy link
Contributor

Upon further studying I've been looking at the SDK and it seems that the resource doesn't accept that input link here. Whereas other resources - such as Elasticache Replication Groups do have the input for at-rest and in-transit encryption link here. Maybe it is out of scope (not intended to do encryption at this level but at replication level) or we should open an issue with 'aws-sdk-go' for them to do the enhancement on their API first.

@bflad
Copy link
Contributor

bflad commented Feb 2, 2018

There are differences between Elasticache cache clusters (Redis or Memcache) and replication groups (cluster mode Redis). While we use the AWS Go SDK which is automatically generated by AWS, the authoritative source of truth to double check is the upstream API reference: https://docs.aws.amazon.com/AmazonElastiCache/latest/APIReference/API_CreateCacheCluster.html

You'll notice there as well that cache clusters do not support the encryption options. 🙁 Unfortunately the Go SDK maintainers won't have anything to do with adding that API, as its not implemented in the Elasticache service itself.

@vpadronblanco
Copy link
Contributor

Got it! Thanks Brian!

Should the issue be closed then? Since it is not part of the service.

@bflad
Copy link
Contributor

bflad commented Feb 2, 2018

Given the above, I'm going to close this issue, since the Elasticache service itself does not support these encryption options for cache clusters.

Note: aws_elasticache_replication_group does support this encryption in the AWS provider already

@bflad bflad closed this as completed Feb 2, 2018
@ndench
Copy link
Contributor

ndench commented Mar 13, 2018

It appears that the Elasticache service now supports encryption for Redis Clusters with cluster mode disabled: https://docs.aws.amazon.com/AmazonElastiCache/latest/UserGuide/at-rest-encryption.html#at-rest-encryption-enable-api-redis-classic-rg

@tonyxiao
Copy link

@bflad It appears that at rest / in transit encryption are supported by AWS CLI. https://docs.aws.amazon.com/cli/latest/reference/elasticache/create-cache-cluster.html

I didn't find it option in the API reference though. Does the AWS CLI somehow uses a different API from the upstream api you mentioned here? https://docs.aws.amazon.com/AmazonElastiCache/latest/APIReference/API_CreateCacheCluster.html

@siavashs
Copy link
Contributor

siavashs commented Jan 31, 2019

As I checked while the CLI provides --auth-token, it fails if you use it:

An error occurred (InvalidParameterValue) when calling the CreateCacheCluster operation:
The AUTH token is only supported when encryption-in-transit is enabled

But there is no option for that! And if you set TransitEncryptionEnabled in the json payload, you'll get this error:

Parameter validation failed:
Unknown parameter in input: "TransitEncryptionEnabled", must be one of ...

This is a broken implementation on the API side which affects the CLI and all SDKs.
The latest ElastiCache API version is 2015-02-02 so I guess they will fix it in the new release hopefully!
For now one has to use the web console to create such a cluster.

@ghost
Copy link

ghost commented Apr 1, 2020

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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

@ghost ghost locked and limited conversation to collaborators Apr 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Requests to existing resources that expand the functionality or scope. good first issue Call to action for new contributors looking for a place to start. Smaller or straightforward issues. service/elasticache Issues and PRs that pertain to the elasticache service.
Projects
None yet
Development

No branches or pull requests

8 participants