From f9ac35058cfb9a43db3e43803fd7aeeda72fba69 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Sun, 1 Dec 2019 15:44:05 -0500 Subject: [PATCH] Add support for DynamoDB customer managed CMKs for server-side encryption. --- aws/data_source_aws_dynamodb_table.go | 4 ++ aws/data_source_aws_dynamodb_table_test.go | 37 +++++----- aws/resource_aws_dynamodb_table.go | 62 +++++++++++++--- aws/resource_aws_dynamodb_table_test.go | 80 +++++++++++++++------ aws/structure.go | 29 +++++--- website/docs/r/dynamodb_table.html.markdown | 7 +- 6 files changed, 161 insertions(+), 58 deletions(-) diff --git a/aws/data_source_aws_dynamodb_table.go b/aws/data_source_aws_dynamodb_table.go index b05b94836be..2bd4d3dcb51 100644 --- a/aws/data_source_aws_dynamodb_table.go +++ b/aws/data_source_aws_dynamodb_table.go @@ -178,6 +178,10 @@ func dataSourceAwsDynamoDbTable() *schema.Resource { Type: schema.TypeBool, Computed: true, }, + "kms_key_arn": { + Type: schema.TypeString, + Computed: true, + }, }, }, }, diff --git a/aws/data_source_aws_dynamodb_table_test.go b/aws/data_source_aws_dynamodb_table_test.go index bd2d6a58e4b..e27c30824a6 100644 --- a/aws/data_source_aws_dynamodb_table_test.go +++ b/aws/data_source_aws_dynamodb_table_test.go @@ -9,6 +9,7 @@ import ( ) func TestAccDataSourceAwsDynamoDbTable_basic(t *testing.T) { + datasourceName := "data.aws_dynamodb_table.test" tableName := fmt.Sprintf("testaccawsdynamodbtable-basic-%s", acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum)) resource.ParallelTest(t, resource.TestCase{ @@ -18,21 +19,21 @@ func TestAccDataSourceAwsDynamoDbTable_basic(t *testing.T) { { Config: testAccDataSourceAwsDynamoDbTableConfigBasic(tableName), Check: resource.ComposeAggregateTestCheckFunc( - resource.TestCheckResourceAttr("data.aws_dynamodb_table.dynamodb_table_test", "name", tableName), - resource.TestCheckResourceAttr("data.aws_dynamodb_table.dynamodb_table_test", "read_capacity", "20"), - resource.TestCheckResourceAttr("data.aws_dynamodb_table.dynamodb_table_test", "write_capacity", "20"), - resource.TestCheckResourceAttr("data.aws_dynamodb_table.dynamodb_table_test", "hash_key", "UserId"), - resource.TestCheckResourceAttr("data.aws_dynamodb_table.dynamodb_table_test", "range_key", "GameTitle"), - resource.TestCheckResourceAttr("data.aws_dynamodb_table.dynamodb_table_test", "attribute.#", "3"), - resource.TestCheckResourceAttr("data.aws_dynamodb_table.dynamodb_table_test", "global_secondary_index.#", "1"), - resource.TestCheckResourceAttr("data.aws_dynamodb_table.dynamodb_table_test", "ttl.#", "1"), - resource.TestCheckResourceAttr("data.aws_dynamodb_table.dynamodb_table_test", "tags.%", "2"), - resource.TestCheckResourceAttr("data.aws_dynamodb_table.dynamodb_table_test", "tags.Name", "dynamodb-table-1"), - resource.TestCheckResourceAttr("data.aws_dynamodb_table.dynamodb_table_test", "tags.Environment", "test"), - resource.TestCheckResourceAttr("data.aws_dynamodb_table.dynamodb_table_test", "server_side_encryption.#", "0"), - resource.TestCheckResourceAttr("data.aws_dynamodb_table.dynamodb_table_test", "billing_mode", "PROVISIONED"), - resource.TestCheckResourceAttr("data.aws_dynamodb_table.dynamodb_table_test", "point_in_time_recovery.#", "1"), - resource.TestCheckResourceAttr("data.aws_dynamodb_table.dynamodb_table_test", "point_in_time_recovery.0.enabled", "false"), + resource.TestCheckResourceAttr(datasourceName, "name", tableName), + resource.TestCheckResourceAttr(datasourceName, "read_capacity", "20"), + resource.TestCheckResourceAttr(datasourceName, "write_capacity", "20"), + resource.TestCheckResourceAttr(datasourceName, "hash_key", "UserId"), + resource.TestCheckResourceAttr(datasourceName, "range_key", "GameTitle"), + resource.TestCheckResourceAttr(datasourceName, "attribute.#", "3"), + resource.TestCheckResourceAttr(datasourceName, "global_secondary_index.#", "1"), + resource.TestCheckResourceAttr(datasourceName, "ttl.#", "1"), + resource.TestCheckResourceAttr(datasourceName, "tags.%", "2"), + resource.TestCheckResourceAttr(datasourceName, "tags.Name", "dynamodb-table-1"), + resource.TestCheckResourceAttr(datasourceName, "tags.Environment", "test"), + resource.TestCheckResourceAttr(datasourceName, "server_side_encryption.#", "0"), + resource.TestCheckResourceAttr(datasourceName, "billing_mode", "PROVISIONED"), + resource.TestCheckResourceAttr(datasourceName, "point_in_time_recovery.#", "1"), + resource.TestCheckResourceAttr(datasourceName, "point_in_time_recovery.0.enabled", "false"), ), }, }, @@ -41,7 +42,7 @@ func TestAccDataSourceAwsDynamoDbTable_basic(t *testing.T) { func testAccDataSourceAwsDynamoDbTableConfigBasic(tableName string) string { return fmt.Sprintf(` -resource "aws_dynamodb_table" "dynamodb_table_test" { +resource "aws_dynamodb_table" "test" { name = "%s" read_capacity = 20 write_capacity = 20 @@ -79,8 +80,8 @@ resource "aws_dynamodb_table" "dynamodb_table_test" { } } -data "aws_dynamodb_table" "dynamodb_table_test" { - name = "${aws_dynamodb_table.dynamodb_table_test.name}" +data "aws_dynamodb_table" "test" { + name = "${aws_dynamodb_table.test.name}" } `, tableName) } diff --git a/aws/resource_aws_dynamodb_table.go b/aws/resource_aws_dynamodb_table.go index a1c7e4aa594..557bbebf48a 100644 --- a/aws/resource_aws_dynamodb_table.go +++ b/aws/resource_aws_dynamodb_table.go @@ -254,7 +254,12 @@ func resourceAwsDynamoDbTable() *schema.Resource { "enabled": { Type: schema.TypeBool, Required: true, - ForceNew: true, + }, + "kms_key_arn": { + Type: schema.TypeString, + Optional: true, + Computed: true, + ValidateFunc: validateArn, }, }, }, @@ -345,13 +350,7 @@ func resourceAwsDynamoDbTableCreate(d *schema.ResourceData, meta interface{}) er } if v, ok := d.GetOk("server_side_encryption"); ok { - options := v.([]interface{}) - if options[0] == nil { - return fmt.Errorf("At least one field is expected inside server_side_encryption") - } - - s := options[0].(map[string]interface{}) - req.SSESpecification = expandDynamoDbEncryptAtRestOptions(s) + req.SSESpecification = expandDynamoDbEncryptAtRestOptions(v.([]interface{})) } var output *dynamodb.CreateTableOutput @@ -562,6 +561,21 @@ func resourceAwsDynamoDbTableUpdate(d *schema.ResourceData, meta interface{}) er } } + if d.HasChange("server_side_encryption") { + // "ValidationException: One or more parameter values were invalid: Server-Side Encryption modification must be the only operation in the request". + _, err := conn.UpdateTable(&dynamodb.UpdateTableInput{ + TableName: aws.String(d.Id()), + SSESpecification: expandDynamoDbEncryptAtRestOptions(d.Get("server_side_encryption").([]interface{})), + }) + if err != nil { + return fmt.Errorf("error updating DynamoDB Table (%s) SSE: %s", d.Id(), err) + } + + if err := waitForDynamoDbSSEUpdateToBeCompleted(d.Id(), d.Timeout(schema.TimeoutUpdate), conn); err != nil { + return fmt.Errorf("error waiting for DynamoDB Table (%s) SSE update: %s", d.Id(), err) + } + } + if d.HasChange("ttl") { if err := updateDynamoDbTimeToLive(d.Id(), d.Get("ttl").([]interface{}), conn); err != nil { return fmt.Errorf("error updating DynamoDB Table (%s) time to live: %s", d.Id(), err) @@ -972,6 +986,38 @@ func waitForDynamoDbTtlUpdateToBeCompleted(tableName string, toEnable bool, conn return err } +func waitForDynamoDbSSEUpdateToBeCompleted(tableName string, timeout time.Duration, conn *dynamodb.DynamoDB) error { + stateConf := &resource.StateChangeConf{ + Pending: []string{ + dynamodb.SSEStatusDisabling, + dynamodb.SSEStatusEnabling, + dynamodb.SSEStatusUpdating, + }, + Target: []string{ + dynamodb.SSEStatusDisabled, + dynamodb.SSEStatusEnabled, + }, + Timeout: timeout, + Refresh: func() (interface{}, string, error) { + result, err := conn.DescribeTable(&dynamodb.DescribeTableInput{ + TableName: aws.String(tableName), + }) + if err != nil { + return 42, "", err + } + + // Disabling SSE returns null SSEDescription. + if result.Table.SSEDescription == nil { + return result, dynamodb.SSEStatusDisabled, nil + } + return result, aws.StringValue(result.Table.SSEDescription.Status), nil + }, + } + _, err := stateConf.WaitForState() + + return err +} + func isDynamoDbTableOptionDisabled(v interface{}) bool { options := v.([]interface{}) if len(options) == 0 { diff --git a/aws/resource_aws_dynamodb_table_test.go b/aws/resource_aws_dynamodb_table_test.go index 085b694e284..21b37ad009e 100644 --- a/aws/resource_aws_dynamodb_table_test.go +++ b/aws/resource_aws_dynamodb_table_test.go @@ -473,8 +473,8 @@ func TestAccAWSDynamoDbTable_enablePitr(t *testing.T) { } func TestAccAWSDynamoDbTable_BillingMode_PayPerRequestToProvisioned(t *testing.T) { - rName := acctest.RandomWithPrefix("TerraformTestTable-") resourceName := "aws_dynamodb_table.test" + rName := acctest.RandomWithPrefix("TerraformTestTable-") resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -503,8 +503,8 @@ func TestAccAWSDynamoDbTable_BillingMode_PayPerRequestToProvisioned(t *testing.T } func TestAccAWSDynamoDbTable_BillingMode_ProvisionedToPayPerRequest(t *testing.T) { - rName := acctest.RandomWithPrefix("TerraformTestTable-") resourceName := "aws_dynamodb_table.test" + rName := acctest.RandomWithPrefix("TerraformTestTable-") resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -533,8 +533,8 @@ func TestAccAWSDynamoDbTable_BillingMode_ProvisionedToPayPerRequest(t *testing.T } func TestAccAWSDynamoDbTable_BillingMode_GSI_PayPerRequestToProvisioned(t *testing.T) { - rName := acctest.RandomWithPrefix("TerraformTestTable-") resourceName := "aws_dynamodb_table.test" + rName := acctest.RandomWithPrefix("TerraformTestTable-") resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -563,8 +563,8 @@ func TestAccAWSDynamoDbTable_BillingMode_GSI_PayPerRequestToProvisioned(t *testi } func TestAccAWSDynamoDbTable_BillingMode_GSI_ProvisionedToPayPerRequest(t *testing.T) { - rName := acctest.RandomWithPrefix("TerraformTestTable-") resourceName := "aws_dynamodb_table.test" + rName := acctest.RandomWithPrefix("TerraformTestTable-") resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -675,8 +675,8 @@ func TestAccAWSDynamoDbTable_tags(t *testing.T) { // https://github.com/hashicorp/terraform/issues/13243 func TestAccAWSDynamoDbTable_gsiUpdateCapacity(t *testing.T) { var conf dynamodb.DescribeTableOutput - name := acctest.RandString(10) resourceName := "aws_dynamodb_table.test" + name := acctest.RandString(10) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -720,8 +720,8 @@ func TestAccAWSDynamoDbTable_gsiUpdateCapacity(t *testing.T) { func TestAccAWSDynamoDbTable_gsiUpdateOtherAttributes(t *testing.T) { var conf dynamodb.DescribeTableOutput - name := acctest.RandString(10) resourceName := "aws_dynamodb_table.test" + name := acctest.RandString(10) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -797,8 +797,8 @@ func TestAccAWSDynamoDbTable_gsiUpdateOtherAttributes(t *testing.T) { // https://github.com/terraform-providers/terraform-provider-aws/issues/566 func TestAccAWSDynamoDbTable_gsiUpdateNonKeyAttributes(t *testing.T) { var conf dynamodb.DescribeTableOutput - name := acctest.RandString(10) resourceName := "aws_dynamodb_table.test" + name := acctest.RandString(10) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -877,8 +877,8 @@ func TestAccAWSDynamoDbTable_gsiUpdateNonKeyAttributes(t *testing.T) { // ValidationException: Time to live has been modified multiple times within a fixed interval func TestAccAWSDynamoDbTable_Ttl_Enabled(t *testing.T) { var table dynamodb.DescribeTableOutput - rName := acctest.RandomWithPrefix("TerraformTestTable-") resourceName := "aws_dynamodb_table.test" + rName := acctest.RandomWithPrefix("TerraformTestTable-") resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -906,8 +906,8 @@ func TestAccAWSDynamoDbTable_Ttl_Enabled(t *testing.T) { // ValidationException: Time to live has been modified multiple times within a fixed interval func TestAccAWSDynamoDbTable_Ttl_Disabled(t *testing.T) { var table dynamodb.DescribeTableOutput - rName := acctest.RandomWithPrefix("TerraformTestTable-") resourceName := "aws_dynamodb_table.test" + rName := acctest.RandomWithPrefix("TerraformTestTable-") resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -1003,9 +1003,11 @@ func TestAccAWSDynamoDbTable_attributeUpdateValidation(t *testing.T) { } func TestAccAWSDynamoDbTable_encryption(t *testing.T) { - var confEncEnabled, confEncDisabled, confBasic dynamodb.DescribeTableOutput + var confBYOK, confEncEnabled, confEncDisabled dynamodb.DescribeTableOutput resourceName := "aws_dynamodb_table.test" rName := acctest.RandomWithPrefix("TerraformTestTable-") + kmsKeyResourceName := "aws_kms_key.test" + kmsAliasDatasourceName := "data.aws_kms_alias.dynamodb" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -1013,11 +1015,12 @@ func TestAccAWSDynamoDbTable_encryption(t *testing.T) { CheckDestroy: testAccCheckAWSDynamoDbTableDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSDynamoDbConfigInitialStateWithEncryption(rName, true), + Config: testAccAWSDynamoDbConfigInitialStateWithEncryptionBYOK(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckInitialAWSDynamoDbTableExists(resourceName, &confEncEnabled), + testAccCheckInitialAWSDynamoDbTableExists(resourceName, &confBYOK), resource.TestCheckResourceAttr(resourceName, "server_side_encryption.#", "1"), resource.TestCheckResourceAttr(resourceName, "server_side_encryption.0.enabled", "true"), + resource.TestCheckResourceAttrPair(resourceName, "server_side_encryption.0.kms_key_arn", kmsKeyResourceName, "arn"), ), }, { @@ -1026,26 +1029,28 @@ func TestAccAWSDynamoDbTable_encryption(t *testing.T) { ImportStateVerify: true, }, { - Config: testAccAWSDynamoDbConfigInitialStateWithEncryption(rName, false), + Config: testAccAWSDynamoDbConfigInitialStateWithEncryptionAmazonCMK(rName, false), Check: resource.ComposeTestCheckFunc( testAccCheckInitialAWSDynamoDbTableExists(resourceName, &confEncDisabled), resource.TestCheckResourceAttr(resourceName, "server_side_encryption.#", "0"), func(s *terraform.State) error { - if confEncDisabled.Table.CreationDateTime.Equal(*confEncEnabled.Table.CreationDateTime) { - return fmt.Errorf("DynamoDB table not recreated when changing SSE") + if !confEncDisabled.Table.CreationDateTime.Equal(*confBYOK.Table.CreationDateTime) { + return fmt.Errorf("DynamoDB table recreated when changing SSE") } return nil }, ), }, { - Config: testAccAWSDynamoDbConfig_basic(rName), + Config: testAccAWSDynamoDbConfigInitialStateWithEncryptionAmazonCMK(rName, true), Check: resource.ComposeTestCheckFunc( - testAccCheckInitialAWSDynamoDbTableExists(resourceName, &confBasic), - resource.TestCheckResourceAttr(resourceName, "server_side_encryption.#", "0"), + testAccCheckInitialAWSDynamoDbTableExists(resourceName, &confEncEnabled), + resource.TestCheckResourceAttr(resourceName, "server_side_encryption.#", "1"), + resource.TestCheckResourceAttr(resourceName, "server_side_encryption.0.enabled", "true"), + resource.TestCheckResourceAttrPair(resourceName, "server_side_encryption.0.kms_key_arn", kmsAliasDatasourceName, "target_key_arn"), func(s *terraform.State) error { - if !confBasic.Table.CreationDateTime.Equal(*confEncDisabled.Table.CreationDateTime) { - return fmt.Errorf("DynamoDB table was recreated unexpectedly") + if !confEncEnabled.Table.CreationDateTime.Equal(*confEncDisabled.Table.CreationDateTime) { + return fmt.Errorf("DynamoDB table recreated when changing SSE") } return nil }, @@ -1569,8 +1574,16 @@ resource "aws_dynamodb_table" "test" { `, rName) } -func testAccAWSDynamoDbConfigInitialStateWithEncryption(rName string, enabled bool) string { +func testAccAWSDynamoDbConfigInitialStateWithEncryptionAmazonCMK(rName string, enabled bool) string { return fmt.Sprintf(` +data "aws_kms_alias" "dynamodb" { + name = "alias/aws/dynamodb" +} + +resource "aws_kms_key" "test" { + description = "DynamoDbTest" +} + resource "aws_dynamodb_table" "test" { name = "%s" read_capacity = 1 @@ -1589,6 +1602,31 @@ resource "aws_dynamodb_table" "test" { `, rName, enabled) } +func testAccAWSDynamoDbConfigInitialStateWithEncryptionBYOK(rName string) string { + return fmt.Sprintf(` +resource "aws_kms_key" "test" { + description = "DynamoDbTest" +} + +resource "aws_dynamodb_table" "test" { + name = "%s" + read_capacity = 2 + write_capacity = 2 + hash_key = "TestTableHashKey" + + attribute { + name = "TestTableHashKey" + type = "S" + } + + server_side_encryption { + enabled = true + kms_key_arn = "${aws_kms_key.test.arn}" + } +} +`, rName) +} + func testAccAWSDynamoDbConfigAddSecondaryGSI(rName string) string { return fmt.Sprintf(` resource "aws_dynamodb_table" "test" { diff --git a/aws/structure.go b/aws/structure.go index 69c32f9c069..8a83a93d098 100644 --- a/aws/structure.go +++ b/aws/structure.go @@ -4351,10 +4351,11 @@ func flattenAwsDynamoDbTableResource(d *schema.ResourceData, table *dynamodb.Tab } sseOptions := []map[string]interface{}{} - if table.SSEDescription != nil { - m := map[string]interface{}{} - m["enabled"] = aws.StringValue(table.SSEDescription.Status) == dynamodb.SSEStatusEnabled - sseOptions = []map[string]interface{}{m} + if sseDescription := table.SSEDescription; sseDescription != nil { + sseOptions = []map[string]interface{}{{ + "enabled": aws.StringValue(sseDescription.Status) == dynamodb.SSEStatusEnabled, + "kms_key_arn": aws.StringValue(sseDescription.KMSMasterKeyArn), + }} } err = d.Set("server_side_encryption", sseOptions) if err != nil { @@ -4478,14 +4479,24 @@ func expandDynamoDbKeySchema(data map[string]interface{}) []*dynamodb.KeySchemaE return keySchema } -func expandDynamoDbEncryptAtRestOptions(m map[string]interface{}) *dynamodb.SSESpecification { - options := dynamodb.SSESpecification{} +func expandDynamoDbEncryptAtRestOptions(vOptions []interface{}) *dynamodb.SSESpecification { + options := &dynamodb.SSESpecification{} - if v, ok := m["enabled"]; ok { - options.Enabled = aws.Bool(v.(bool)) + enabled := false + if len(vOptions) > 0 { + mOptions := vOptions[0].(map[string]interface{}) + + enabled = mOptions["enabled"].(bool) + if enabled { + if vKmsKeyArn, ok := mOptions["kms_key_arn"].(string); ok && vKmsKeyArn != "" { + options.KMSMasterKeyId = aws.String(vKmsKeyArn) + options.SSEType = aws.String(dynamodb.SSETypeKms) + } + } } + options.Enabled = aws.Bool(enabled) - return &options + return options } func expandDynamoDbTableItemAttributes(input string) (map[string]*dynamodb.AttributeValue, error) { diff --git a/website/docs/r/dynamodb_table.html.markdown b/website/docs/r/dynamodb_table.html.markdown index 4744f287462..3026d53ee6d 100644 --- a/website/docs/r/dynamodb_table.html.markdown +++ b/website/docs/r/dynamodb_table.html.markdown @@ -134,9 +134,12 @@ The `timeouts` block allows you to specify [timeouts](https://www.terraform.io/d #### `server_side_encryption` -* `enabled` - (Required) Whether or not to enable encryption at rest using an AWS managed Customer Master Key. +* `enabled` - (Required) Whether or not to enable encryption at rest using an AWS managed KMS customer master key (CMK). +* `kms_key_arn` - (Optional) The ARN of the CMK that should be used for the AWS KMS encryption. +This attribute should only be specified if the key is different from the default DynamoDB CMK, `alias/aws/dynamodb`. + If `enabled` is `false` then server-side encryption is set to AWS owned CMK (shown as `DEFAULT` in the AWS console). -If `enabled` is `true` then server-side encryption is set to AWS managed CMK (shown as `KMS` in the AWS console). +If `enabled` is `true` and no `kms_master_key_id` is specified then server-side encryption is set to AWS managed CMK (shown as `KMS` in the AWS console). The [AWS KMS documentation](https://docs.aws.amazon.com/kms/latest/developerguide/concepts.html) explains the difference between AWS owned and AWS managed CMKs. #### `point_in_time_recovery`