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

[WIP] r/aws_dynamodb_table: Support 11/15/2018 Server-Side Encryption enhancements #5666

Closed
wants to merge 1 commit into from

Conversation

ewbankkit
Copy link
Contributor

@ewbankkit ewbankkit commented Aug 23, 2018

Fixes #5637.

Currently WIP:

$ terraform apply
aws_dynamodb_table.basic-dynamodb-table: Refreshing state... (ID: test-enc-01)
aws_dynamodb_table.basic-dynamodb-table: Modifying... (ID: test-enc-01)
  server_side_encryption.0.enabled:  "false" => "true"
  server_side_encryption.0.sse_type: "" => "AES256"
Error applying plan:

1 error(s) occurred:

* aws_dynamodb_table.basic-dynamodb-table: 1 error(s) occurred:

* aws_dynamodb_table.basic-dynamodb-table: ValidationException: One or more parameter values were invalid: Update table encryption is coming to your account soon
	status code: 400, request id: BTJKPO1DQ5RMBQ6TDT4IT85LDVVV4KQNSO5AEMVJF66Q9ASUAAJG

Terraform does not automatically rollback in the face of errors.
Instead, your Terraform state file has been partially updated with
any resources that successfully completed. Please address the error
above and apply again to incrementally change your infrastructure.

Still TODO:

  • Update aws_dynamodb_table data source
  • More acceptance tests

@bflad bflad added enhancement Requests to existing resources that expand the functionality or scope. waiting-response Maintainers are waiting on response from community or contributor. service/dynamodb Issues and PRs that pertain to the dynamodb service. labels Sep 6, 2018
@bflad
Copy link
Contributor

bflad commented Oct 9, 2018

@ewbankkit any chance this could be updated? Thanks!

@ghost ghost added size/L Managed by automation to categorize the size of a PR. documentation Introduces or discusses updates to documentation. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. labels Oct 10, 2018
@ewbankkit
Copy link
Contributor Author

@bflad Rebased to remove conflicts. The service changes have not yet been released:

$ make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSDynamoDbTable_encryption'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -run=TestAccAWSDynamoDbTable_encryption -timeout 120m
=== RUN   TestAccAWSDynamoDbTable_encryption
--- FAIL: TestAccAWSDynamoDbTable_encryption (4.49s)
    testing.go:527: Step 0 error: Error applying: 1 error occurred:
        	* aws_dynamodb_table.basic-dynamodb-table: 1 error occurred:
        	* aws_dynamodb_table.basic-dynamodb-table: ValidationException: One or more parameter values were invalid: Unsupported SSEType AES256
        	status code: 400, request id: 9S3LR33ADF60MHAKRG598KMAL3VV4KQNSO5AEMVJF66Q9ASUAAJG
        
        
        
        
FAIL
FAIL	github.com/terraform-providers/terraform-provider-aws/aws	4.500s
GNUmakefile:20: recipe for target 'testacc' failed
make: *** [testacc] Error 1

@nhoughto
Copy link

I believe they have been released today 👍🏼

@ghost ghost removed the waiting-response Maintainers are waiting on response from community or contributor. label Nov 16, 2018
@ewbankkit
Copy link
Contributor Author

Yes, I think this is the announcement. I will try and finish this today.

@ewbankkit
Copy link
Contributor Author

Running the tests unmodified I still get the same error:

$ make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSDynamoDbTable_encryption'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -parallel 20 -run=TestAccAWSDynamoDbTable_encryption -timeout 120m
=== RUN   TestAccAWSDynamoDbTable_encryption
=== PAUSE TestAccAWSDynamoDbTable_encryption
=== CONT  TestAccAWSDynamoDbTable_encryption
--- FAIL: TestAccAWSDynamoDbTable_encryption (4.39s)
    testing.go:538: Step 0 error: Error applying: 1 error occurred:
        	* aws_dynamodb_table.basic-dynamodb-table: 1 error occurred:
        	* aws_dynamodb_table.basic-dynamodb-table: ValidationException: One or more parameter values were invalid: SSEType AES256 is not supported
        	status code: 400, request id: SGMHVUIPQ8LUDLL6M87JRTKM8VVV4KQNSO5AEMVJF66Q9ASUAAJG
        
        
        
        
FAIL
FAIL	github.com/terraform-providers/terraform-provider-aws/aws	4.398s
GNUmakefile:20: recipe for target 'testacc' failed
make: *** [testacc] Error 1

and if I change the default sse_type from AES256 to KMS I get a different error:

$ make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSDynamoDbTable_encryption'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -parallel 20 -run=TestAccAWSDynamoDbTable_encryption -timeout 120m
=== RUN   TestAccAWSDynamoDbTable_encryption
=== PAUSE TestAccAWSDynamoDbTable_encryption
=== CONT  TestAccAWSDynamoDbTable_encryption
--- FAIL: TestAccAWSDynamoDbTable_encryption (19.66s)
    testing.go:538: Step 0 error: Error applying: 1 error occurred:
        	* aws_dynamodb_table.basic-dynamodb-table: 1 error occurred:
        	* aws_dynamodb_table.basic-dynamodb-table: ValidationException: One or more parameter values were invalid: Update table encryption is coming to your account soon
        	status code: 400, request id: PIH23LT13T3FDGF0MD9LDA5OS7VV4KQNSO5AEMVJF66Q9ASUAAJG
        
        
        
        
FAIL
FAIL	github.com/terraform-providers/terraform-provider-aws/aws	19.679s
GNUmakefile:20: recipe for target 'testacc' failed
make: *** [testacc] Error 1

Now, suppose I create a new table via the console and accept all defaults. I see

tf-test-default-encryption

and at the API level:

$ aws --region us-east-1 dynamodb describe-table --table-name tf-test-default-encryption
{
    "Table": {
        "TableArn": "arn:aws:dynamodb:us-east-1:000000000000:table/tf-test-default-encryption", 
        "AttributeDefinitions": [
            {
                "AttributeName": "name", 
                "AttributeType": "S"
            }
        ], 
        "ProvisionedThroughput": {
            "NumberOfDecreasesToday": 0, 
            "WriteCapacityUnits": 5, 
            "ReadCapacityUnits": 5
        }, 
        "TableSizeBytes": 0, 
        "TableName": "tf-test-default-encryption", 
        "TableStatus": "ACTIVE", 
        "TableId": "965e2d09-2e0a-44c4-a604-9da95ff08547", 
        "KeySchema": [
            {
                "KeyType": "HASH", 
                "AttributeName": "name"
            }
        ], 
        "ItemCount": 0, 
        "CreationDateTime": 1542379431.603
    }
}

If I create a new table via the console and choose KMS encryption, I see

tf-test-explicit-encryption

and at the API level:

$ aws --region us-east-1 dynamodb describe-table --table-name tf-test-explicit-encryption
{
    "Table": {
        "TableArn": "arn:aws:dynamodb:us-east-1:000000000000:table/tf-test-explicit-encryption", 
        "AttributeDefinitions": [
            {
                "AttributeName": "name", 
                "AttributeType": "S"
            }
        ], 
        "ProvisionedThroughput": {
            "NumberOfDecreasesToday": 0, 
            "WriteCapacityUnits": 5, 
            "ReadCapacityUnits": 5
        }, 
        "TableSizeBytes": 0, 
        "TableName": "tf-test-explicit-encryption", 
        "TableStatus": "ACTIVE", 
        "TableId": "eeee1609-5ad3-4e28-a28e-ce80736e95f9", 
        "SSEDescription": {
            "Status": "ENABLED", 
            "KMSMasterKeyArn": "arn:aws:kms:us-east-1:000000000000:key/2c42622a-8fb4-4147-b98c-8dac4aa33bd4", 
            "SSEType": "KMS"
        }, 
        "KeySchema": [
            {
                "KeyType": "HASH", 
                "AttributeName": "name"
            }
        ], 
        "ItemCount": 0, 
        "CreationDateTime": 1542379472.602
    }
}

@ghost ghost added size/XL Managed by automation to categorize the size of a PR. and removed size/L Managed by automation to categorize the size of a PR. labels Nov 16, 2018
@ewbankkit ewbankkit changed the title [WIP] r/aws_dynamodb_table: Support Server-Side Encryption enhancements r/aws_dynamodb_table: Support 11/15/2018 Server-Side Encryption enhancements Nov 16, 2018
@ewbankkit
Copy link
Contributor Author

Re-scoped this PR to implement the functionality available today.
Omitting server_side_encryption or specifying

server_side_encryption {
  enabled = false
}

creates a DynamoDB table with DEFAULT encryption - "AWS owned CMK".

Specifying

server_side_encryption {
  enabled = false
}

or

server_side_encryption {
  enabled  = false
  sse_type = "KMS"
}

or even

server_side_encryption {
  enabled           = false
  sse_type          = "KMS"
  kms_master_key_id = "alias/aws/dynamodb"
}

creates a DynamoDB table with _KMS_encryption - "AWS managed CMK".

@@ -15,14 +15,30 @@ func dataSourceAwsDynamoDbTable() *schema.Resource {
return &schema.Resource{
Read: dataSourceAwsDynamoDbTableRead,
Schema: map[string]*schema.Schema{
"arn": {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reordered the data_source_aws_dynamodb_table attributes to match resource_aws_dynamodb_table for easier checking of missing attributes.

},
"server_side_encryption": {
"tags": tagsSchemaComputed(),
"point_in_time_recovery": {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added point_in_time_recovery as the documentation says that the data source returns all the same attributes as the resource.

@ewbankkit
Copy link
Contributor Author

As-is this PR does not address any of the enhancements listed in the original issue's first comment but will form a foundation for when those features are available.

@ewbankkit
Copy link
Contributor Author

Acceptance tests:

$ make testacc TEST=./aws/ TESTARGS='-run=TestAccDataSourceAwsDynamoDbTable_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -parallel 20 -run=TestAccDataSourceAwsDynamoDbTable_ -timeout 120m
=== RUN   TestAccDataSourceAwsDynamoDbTable_basic
=== PAUSE TestAccDataSourceAwsDynamoDbTable_basic
=== CONT  TestAccDataSourceAwsDynamoDbTable_basic
--- PASS: TestAccDataSourceAwsDynamoDbTable_basic (136.68s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	136.689s

$ make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSDynamoDbTable_encryption'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -parallel 20 -run=TestAccAWSDynamoDbTable_encryption -timeout 120m
=== RUN   TestAccAWSDynamoDbTable_encryptionDefault
=== PAUSE TestAccAWSDynamoDbTable_encryptionDefault
=== CONT  TestAccAWSDynamoDbTable_encryptionDefault
--- PASS: TestAccAWSDynamoDbTable_encryptionDefault (121.69s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	121.717s

@ewbankkit ewbankkit changed the title r/aws_dynamodb_table: Support 11/15/2018 Server-Side Encryption enhancements [WIP] r/aws_dynamodb_table: Support 11/15/2018 Server-Side Encryption enhancements Nov 19, 2018
@ewbankkit
Copy link
Contributor Author

Moving back to WIP as I haven't convinced myself of the value of this yet 😄.
Let's wait and see what DynamoDB encryption updates are coming...

@jpalomaki
Copy link

FWIW, I just created a DynamoDB table, specifying:

server_side_encryption {
    enabled = true
}

which created the table with encryption type KMS, while I specifically hoped to get DEFAULT.

So at least I see value in this, being able to distinguish between the two explicitly.

@ewbankkit
Copy link
Contributor Author

Closed. Replaced by #7499.

@ewbankkit ewbankkit closed this Feb 10, 2019
@ewbankkit ewbankkit deleted the issue-5637 branch February 10, 2019 23:14
@ewbankkit
Copy link
Contributor Author

@jpalomaki This all boils down to a documentation update.
To get DEFAULT encryption you should specify enabled = false in the server_side_encryption block (or leave the whole block out). This is kind of weird but was the way that AWS implemented encrypt everything in the API.

@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
documentation Introduces or discusses updates to documentation. enhancement Requests to existing resources that expand the functionality or scope. service/dynamodb Issues and PRs that pertain to the dynamodb service. size/XL Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

r/aws_dynamodb_table: Support 11/15/2018 Server-Side Encryption enhancements
4 participants