Add storage_encrypted as an optional parameter to aws_rds_cluster

[bsiegel]

Provides a basic implementation of #4414, matching the current implementation for aws_db_instance added in #1041. This basic implementation allows the user to set the value of the StorageEncrypted flag at creation time, but does not allow customizing the KMS parameters as requested in #4414. I feel like that should be done in a separate PR, so that it can be added in a uniform way to all AWS resources that can accept KMS parameters.

[stack72]

The documentation update suggests that the default for this value is false - I think we need to actually have this in the schema as follows:

"storage_encrypted": &schema.Schema{
         Type:     schema.TypeBool,
    Optional: true,
        Default: false,
    ForceNew: true,
}

thoughts?

[bsiegel]

The default is false if the parameter is omitted from the request, so I see two options. I could either update the schema to include the default value, or I could only set the StorageEncrypted value on the CreateDBClusterInput object if a value is provided. Which do you think is best?

[stack72]

personally, I would suggest to be explicit in the terraform schema. This will safeguard against changes in AWS behaviour :)

[bsiegel]

Sounds good! Fixed in b3864db

[bsiegel]

I wasn't sure about the interplay between the values of StorageEncrypted on the cluster itself vs. the cluster instances, until I located this in the AWS CloudFormation user guide:

StorageEncrypted

Indicates whether the database instance is encrypted.

If you specify the DBClusterIdentifier, DBSnapshotIdentifier, or SourceDBInstanceIdentifier property, do not specify this property. The value is inherited from the cluster, snapshot, or source database instance.

[stack72]

@bsiegel I think we need to add some tests to make sure this works as expected when used in combination with these properties. I think these limitations should then be added to the docs

[bsiegel]

I think we need to add some tests to make sure this works as expected when used in combination with these properties. I think these limitations should then be added to the docs

These limitations are on the DBInstance itself, not on the cluster. In this case, the DBInstance is exposed via the aws_rds_cluster_instance resource which does not permit specifying a storage_encrypted flag.

I would definitely be up for adding some tests though. I'll see what I can come up with.

[bsiegel]

@stack72 Added some tests, let me know if you think those are sufficient.

[stack72]

Hi @bsiegel - running the TestAccAWSRDSCluster now and pending they all pass, will merge :) Thanks for the PR here

[stack72]

make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSRDSCluster' 2>~/tf.log
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /vendor/)
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSRDSCluster -timeout 120m
=== RUN   TestAccAWSRDSClusterInstance_basic
--- PASS: TestAccAWSRDSClusterInstance_basic (731.41s)
=== RUN   TestAccAWSRDSCluster_basic
--- PASS: TestAccAWSRDSCluster_basic (125.26s)
=== RUN   TestAccAWSRDSCluster_encrypted
--- PASS: TestAccAWSRDSCluster_encrypted (115.32s)
=== RUN   TestAccAWSRDSCluster_backupsUpdate
--- PASS: TestAccAWSRDSCluster_backupsUpdate (126.96s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    1098.973s

[ghost]

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.