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.

Sign up for GitHub

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

Support encryption at rest for DynamoDB tables #3303

Merged
merged 8 commits into from
Mar 6, 2018
+208 −8
Merged

Support encryption at rest for DynamoDB tables #3303

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
2114a3b
Support encryption at rest for DynamoDB tables.
ewbankkit Feb 9, 2018
1aa8400
Extra check on empty SSE options.
ewbankkit Mar 2, 2018
774d415
Acceptance test changes after review.
ewbankkit Mar 2, 2018
80af905
'encrypt_at_rest' -> 'server_side_encryption'.
ewbankkit Mar 2, 2018
5223968
Revert "Extra check on empty SSE options."
ewbankkit Mar 2, 2018
b4ddab7
'encrypt_at_rest' -> 'server_side_encryption'.
ewbankkit Mar 2, 2018
5c58790
Update 'CustomizeDiff' to prevent incorrect 'ForceNew's.
ewbankkit Mar 2, 2018
0eb8914
Additional encryption acceptance test steps.
ewbankkit Mar 5, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions aws/data_source_aws_dynamodb_table.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,20 @@ func dataSourceAwsDynamoDbTable() *schema.Resource {
Type: schema.TypeInt,
Computed: true,
},
"encrypt_at_rest": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😅 Sorry for this, but could you rename all these encrypt_at_rest references to server_side_encryption so they better match the DynamoDB API SSE?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do

Type: schema.TypeList,
Optional: true,
Computed: true,
MaxItems: 1,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"enabled": {
Type: schema.TypeBool,
Computed: true,
},
},
},
},
},
}
}
Expand Down
11 changes: 6 additions & 5 deletions aws/data_source_aws_dynamodb_table_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ func TestAccDataSourceAwsDynamoDbTable_basic(t *testing.T) {
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", "encrypt_at_rest.#", "0"),
),
},
},
Expand All @@ -41,22 +42,22 @@ func testAccDataSourceAwsDynamoDbTableConfigBasic(tableName string) string {
write_capacity = 20
hash_key = "UserId"
range_key = "GameTitle"

attribute {
name = "UserId"
type = "S"
}

attribute {
name = "GameTitle"
type = "S"
}

attribute {
name = "TopScore"
type = "N"
}

global_secondary_index {
name = "GameTitleIndex"
hash_key = "GameTitle"
Expand All @@ -66,7 +67,7 @@ func testAccDataSourceAwsDynamoDbTableConfigBasic(tableName string) string {
projection_type = "INCLUDE"
non_key_attributes = ["UserId"]
}

tags {
Name = "dynamodb-table-1"
Environment = "test"
Expand Down
25 changes: 25 additions & 0 deletions aws/resource_aws_dynamodb_table.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,21 @@ func resourceAwsDynamoDbTable() *schema.Resource {
Type: schema.TypeString,
Computed: true,
},
"encrypt_at_rest": {
Type: schema.TypeList,
Optional: true,
Computed: true,
MaxItems: 1,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"enabled": {
Type: schema.TypeBool,
Required: true,
ForceNew: true,
},
},
},
},
"tags": tagsSchema(),
},
}
Expand Down Expand Up @@ -250,6 +265,16 @@ func resourceAwsDynamoDbTableCreate(d *schema.ResourceData, meta interface{}) er
}
}

if v, ok := d.GetOk("encrypt_at_rest"); ok {
options := v.([]interface{})
if options[0] == nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this needs to check len(options) == 0

s := make([]interface{}, 0)
if s[0] == nil {
	log.Println("All is well")
}

# panic: runtime error: index out of range

s := make([]interface{}, 0)
if len(s) == 0 {
	log.Println("All is well")
}

# 2018/03/01 15:19:00 All is well

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally speaking though, we forego error checking against our own schema.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't GetOk() guard against len(options) == 0?

return fmt.Errorf("At least one field is expected inside encrypt_at_rest")
}

s := options[0].(map[string]interface{})
req.SSESpecification = expandDynamoDbEncryptAtRestOptions(s)
}

var output *dynamodb.CreateTableOutput
err := resource.Retry(2*time.Minute, func() *resource.RetryError {
var err error
Expand Down
88 changes: 85 additions & 3 deletions aws/resource_aws_dynamodb_table_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ func TestAccAWSDynamoDbTable_basic(t *testing.T) {
Config: testAccAWSDynamoDbConfigInitialState(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckInitialAWSDynamoDbTableExists("aws_dynamodb_table.basic-dynamodb-table", &conf),
testAccCheckInitialAWSDynamoDbTableConf("aws_dynamodb_table.basic-dynamodb-table"),
testAccCheckInitialAWSDynamoDbTableConf("aws_dynamodb_table.basic-dynamodb-table", false),
),
},
{
Expand Down Expand Up @@ -368,7 +368,7 @@ func TestAccAWSDynamoDbTable_tags(t *testing.T) {
Config: testAccAWSDynamoDbConfigTags(),
Check: resource.ComposeTestCheckFunc(
testAccCheckInitialAWSDynamoDbTableExists("aws_dynamodb_table.basic-dynamodb-table", &conf),
testAccCheckInitialAWSDynamoDbTableConf("aws_dynamodb_table.basic-dynamodb-table"),
testAccCheckInitialAWSDynamoDbTableConf("aws_dynamodb_table.basic-dynamodb-table", false),
resource.TestCheckResourceAttr(
"aws_dynamodb_table.basic-dynamodb-table", "tags.%", "3"),
),
Expand Down Expand Up @@ -625,6 +625,27 @@ func testAccCheckDynamoDbTableTimeToLiveWasUpdated(n string) resource.TestCheckF
}
}

func TestAccAWSDynamoDbTable_encryption(t *testing.T) {
var conf dynamodb.DescribeTableOutput

rName := acctest.RandomWithPrefix("TerraformTestTable-")

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSDynamoDbTableDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSDynamoDbConfigInitialStateWithEncryption(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckInitialAWSDynamoDbTableExists("aws_dynamodb_table.basic-dynamodb-table", &conf),
testAccCheckInitialAWSDynamoDbTableConf("aws_dynamodb_table.basic-dynamodb-table", true),
),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please check the Terraform state here?

resource.TestCheckResourceAttr("aws_dynamodb_table.basic-dynamodb-table", "server_side_encryption.#", "1"),
resource.TestCheckResourceAttr("aws_dynamodb_table.basic-dynamodb-table", "server_side_encryption.0.enabled", "true"),

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By doing this and removing the extraneous attributes I can now revert the modifications to testAccCheckInitialAWSDynamoDbTableConf so as not to need the sse bool parameter

},
},
})
}

func TestResourceAWSDynamoDbTableStreamViewType_validation(t *testing.T) {
cases := []struct {
Value string
Expand Down Expand Up @@ -725,7 +746,7 @@ func testAccCheckInitialAWSDynamoDbTableExists(n string, table *dynamodb.Describ
}
}

func testAccCheckInitialAWSDynamoDbTableConf(n string) resource.TestCheckFunc {
func testAccCheckInitialAWSDynamoDbTableConf(n string, sse bool) resource.TestCheckFunc {
return func(s *terraform.State) error {
log.Printf("[DEBUG] Trying to create initial table state!")
rs, ok := s.RootModule().Resources[n]
Expand Down Expand Up @@ -761,6 +782,16 @@ func testAccCheckInitialAWSDynamoDbTableConf(n string) resource.TestCheckFunc {
return fmt.Errorf("Provisioned read capacity was %d, not 10!", table.ProvisionedThroughput.ReadCapacityUnits)
}

if sse {
if table.SSEDescription == nil || *table.SSEDescription.Status != dynamodb.SSEStatusEnabled {
return fmt.Errorf("SSE status was not %s", dynamodb.SSEStatusEnabled)
}
} else {
if table.SSEDescription != nil && *table.SSEDescription.Status != dynamodb.SSEStatusDisabled {
return fmt.Errorf("SSE status was %s, not %s", *table.SSEDescription.Status, dynamodb.SSEStatusDisabled)
}
}

attrCount := len(table.AttributeDefinitions)
gsiCount := len(table.GlobalSecondaryIndexes)
lsiCount := len(table.LocalSecondaryIndexes)
Expand Down Expand Up @@ -927,6 +958,57 @@ resource "aws_dynamodb_table" "basic-dynamodb-table" {
`, rName)
}

func testAccAWSDynamoDbConfigInitialStateWithEncryption(rName string) string {
return fmt.Sprintf(`
resource "aws_dynamodb_table" "basic-dynamodb-table" {
name = "%s"
read_capacity = 10
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please set the test capacities to 1? 💸

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do

write_capacity = 20
hash_key = "TestTableHashKey"
range_key = "TestTableRangeKey"

attribute {
name = "TestTableHashKey"
type = "S"
}

attribute {
name = "TestTableRangeKey"
type = "S"
}

attribute {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please remove extraneous attributes/indexes here? We only really care about the encryption setting.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do

name = "TestLSIRangeKey"
type = "N"
}

attribute {
name = "TestGSIRangeKey"
type = "S"
}

local_secondary_index {
name = "TestTableLSI"
range_key = "TestLSIRangeKey"
projection_type = "ALL"
}

global_secondary_index {
name = "InitialTestTableGSI"
hash_key = "TestTableHashKey"
range_key = "TestGSIRangeKey"
write_capacity = 10
read_capacity = 10
projection_type = "KEYS_ONLY"
}

encrypt_at_rest {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: Indentation got thrown off here

Copy link
Contributor Author

@ewbankkit ewbankkit Mar 2, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do, copy and paste flub

enabled = true
}
}
`, rName)
}

func testAccAWSDynamoDbConfigAddSecondaryGSI(rName string) string {
return fmt.Sprintf(`
resource "aws_dynamodb_table" "basic-dynamodb-table" {
Expand Down
21 changes: 21 additions & 0 deletions aws/structure.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3522,6 +3522,17 @@ func flattenAwsDynamoDbTableResource(d *schema.ResourceData, table *dynamodb.Tab
return err
}

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}
}
err = d.Set("encrypt_at_rest", sseOptions)
if err != nil {
return err
}

d.Set("arn", table.TableArn)

return nil
Expand Down Expand Up @@ -3610,6 +3621,16 @@ func expandDynamoDbKeySchema(data map[string]interface{}) []*dynamodb.KeySchemaE
return keySchema
}

func expandDynamoDbEncryptAtRestOptions(m map[string]interface{}) *dynamodb.SSESpecification {
options := dynamodb.SSESpecification{}

if v, ok := m["enabled"]; ok {
options.Enabled = aws.Bool(v.(bool))
}

return &options
}

func flattenVpcEndpointServiceAllowedPrincipals(allowedPrincipals []*ec2.AllowedPrincipal) []string {
result := make([]string, 0, len(allowedPrincipals))
for _, allowedPrincipal := range allowedPrincipals {
Expand Down
5 changes: 5 additions & 0 deletions website/docs/r/dynamodb_table.html.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ definition after you have created the resource.
attributes, etc.
* `stream_enabled` - (Optional) Indicates whether Streams are to be enabled (true) or disabled (false).
* `stream_view_type` - (Optional) When an item in the table is modified, StreamViewType determines what information is written to the table's stream. Valid values are `KEYS_ONLY`, `NEW_IMAGE`, `OLD_IMAGE`, `NEW_AND_OLD_IMAGES`.
* `encrypt_at_rest` - (Optional) Encrypt at rest options.
* `tags` - (Optional) A map of tags to populate on the created table.

### Timeouts
Expand Down Expand Up @@ -130,6 +131,10 @@ The `timeouts` block allows you to specify [timeouts](https://www.terraform.io/d
projection type; a list of attributes to project into the index. These
do not need to be defined as attributes on the table.

#### `encrypt_at_rest`

* `enabled` - (Required) Whether to enable encryption at rest. If the `encrypt_at_rest` block is not provided then this defaults to `false`.

### A note about attributes

Only define attributes on the table object that are going to be used as:
Expand Down