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

EBS default encryption #8771

Merged
merged 8 commits into from
Jun 20, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions aws/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,8 @@ func Provider() terraform.ResourceProvider {
"aws_dynamodb_table": resourceAwsDynamoDbTable(),
"aws_dynamodb_table_item": resourceAwsDynamoDbTableItem(),
"aws_dynamodb_global_table": resourceAwsDynamoDbGlobalTable(),
"aws_ebs_default_kms_key": resourceAwsEbsDefaultKmsKey(),
"aws_ebs_encryption_by_default": resourceAwsEbsEncryptionByDefault(),
"aws_ebs_snapshot": resourceAwsEbsSnapshot(),
"aws_ebs_snapshot_copy": resourceAwsEbsSnapshotCopy(),
"aws_ebs_volume": resourceAwsEbsVolume(),
Expand Down
69 changes: 69 additions & 0 deletions aws/resource_aws_ebs_default_kms_key.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package aws

import (
"fmt"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ec2"

"github.com/hashicorp/terraform/helper/schema"
)

func resourceAwsEbsDefaultKmsKey() *schema.Resource {
return &schema.Resource{
Copy link
Contributor

Choose a reason for hiding this comment

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

We should add import support from the get-go, e.g. via the key ARN as the ID 🚀

Importer: &schema.ResourceImporter{
	State: schema.ImportStatePassthrough,
},

In the testing:

{
	ResourceName:      resourceName,
	ImportState:       true,
	ImportStateVerify: true,
},

And the documentation:

## Import

EBS Default KMS Key can be imported with the KMS Key ARN, e.g.

```console
$ terraform import aws_ebs_default_kms_key.example arn:aws:kms:us-east-1:123456789012:key/abcd-1234
```

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since resourceAwsEbsDefaultKmsKeyRead() doesn't actually use the resource's ID for the AWS API call (there's a single default EBS CMK per region per account) do we want to check that the KMS key ARN passed to terraform import is really the current default EBS CMK?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the Terraform code corresponding to the imported resource has the same key_arn value as the ARN passed to terraform import then I think that having ForceNew on the key_arn attribute will cause the resource to be recreated.

Create: resourceAwsEbsDefaultKmsKeyCreate,
Read: resourceAwsEbsDefaultKmsKeyRead,
Delete: resourceAwsEbsDefaultKmsKeyDelete,
Importer: &schema.ResourceImporter{
State: schema.ImportStatePassthrough,
},

Schema: map[string]*schema.Schema{
"key_arn": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: validateArn,
},
},
}
}

func resourceAwsEbsDefaultKmsKeyCreate(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).ec2conn

resp, err := conn.ModifyEbsDefaultKmsKeyId(&ec2.ModifyEbsDefaultKmsKeyIdInput{
KmsKeyId: aws.String(d.Get("key_arn").(string)),
})
if err != nil {
return fmt.Errorf("error creating EBS default KMS key: %s", err)
}

d.SetId(aws.StringValue(resp.KmsKeyId))

return resourceAwsEbsDefaultKmsKeyRead(d, meta)
}

func resourceAwsEbsDefaultKmsKeyRead(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).ec2conn

resp, err := conn.GetEbsDefaultKmsKeyId(&ec2.GetEbsDefaultKmsKeyIdInput{})
if err != nil {
return fmt.Errorf("error reading EBS default KMS key: %s", err)
}

d.Set("key_arn", resp.KmsKeyId)

return nil
}

func resourceAwsEbsDefaultKmsKeyDelete(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).ec2conn

_, err := conn.ResetEbsDefaultKmsKeyId(&ec2.ResetEbsDefaultKmsKeyIdInput{})
if err != nil {
return fmt.Errorf("error deleting EBS default KMS key: %s", err)
}

return nil
}
123 changes: 123 additions & 0 deletions aws/resource_aws_ebs_default_kms_key_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
package aws

import (
"fmt"
"testing"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/arn"
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/hashicorp/terraform/helper/resource"
"github.com/hashicorp/terraform/terraform"
)

func TestAccAWSEBSDefaultKmsKey_basic(t *testing.T) {
resourceName := "aws_ebs_default_kms_key.test"
resourceNameKey := "aws_kms_key.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAwsEbsDefaultKmsKeyDestroy,
Steps: []resource.TestStep{
{
Config: testAccAwsEbsDefaultKmsKeyConfig_basic,
Check: resource.ComposeTestCheckFunc(
testAccCheckEbsDefaultKmsKey(resourceName),
resource.TestCheckResourceAttrPair(resourceName, "key_arn", resourceNameKey, "arn"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
},
})
}

func testAccCheckAwsEbsDefaultKmsKeyDestroy(s *terraform.State) error {
arn, err := testAccAwsEbsDefaultKmsKeyAwsManagedDefaultKey()
if err != nil {
return err
}

conn := testAccProvider.Meta().(*AWSClient).ec2conn

resp, err := conn.GetEbsDefaultKmsKeyId(&ec2.GetEbsDefaultKmsKeyIdInput{})
if err != nil {
return err
}

// Verify that the default key is now the account's AWS-managed default CMK.
if aws.StringValue(resp.KmsKeyId) != arn.String() {
return fmt.Errorf("Default CMK (%s) is not the account's AWS-managed default CMK (%s)", aws.StringValue(resp.KmsKeyId), arn.String())
}

return nil
}

func testAccCheckEbsDefaultKmsKey(name string) resource.TestCheckFunc {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it worth having this function do the inverse of the destroy check?

return func(s *terraform.State) error {
rs, ok := s.RootModule().Resources[name]
if !ok {
return fmt.Errorf("Not found: %s", name)
}

if rs.Primary.ID == "" {
return fmt.Errorf("No ID is set")
}

arn, err := testAccAwsEbsDefaultKmsKeyAwsManagedDefaultKey()
if err != nil {
return err
}

conn := testAccProvider.Meta().(*AWSClient).ec2conn

resp, err := conn.GetEbsDefaultKmsKeyId(&ec2.GetEbsDefaultKmsKeyIdInput{})
if err != nil {
return err
}

// Verify that the default key is not the account's AWS-managed default CMK.
if aws.StringValue(resp.KmsKeyId) == arn.String() {
return fmt.Errorf("Default CMK (%s) is the account's AWS-managed default CMK (%s)", aws.StringValue(resp.KmsKeyId), arn.String())
}

return nil
}
}

// testAccAwsEbsDefaultKmsKeyAwsManagedDefaultKey returns' the account's AWS-managed default CMK.
func testAccAwsEbsDefaultKmsKeyAwsManagedDefaultKey() (*arn.ARN, error) {
conn := testAccProvider.Meta().(*AWSClient).kmsconn

alias, err := findKmsAliasByName(conn, "alias/aws/ebs", nil)
if err != nil {
return nil, err
}

aliasARN, err := arn.Parse(aws.StringValue(alias.AliasArn))
if err != nil {
return nil, err
}

arn := arn.ARN{
Partition: aliasARN.Partition,
Service: aliasARN.Service,
Region: aliasARN.Region,
AccountID: aliasARN.AccountID,
Resource: fmt.Sprintf("key/%s", aws.StringValue(alias.TargetKeyId)),
}

return &arn, nil
}

const testAccAwsEbsDefaultKmsKeyConfig_basic = `
resource "aws_kms_key" "test" {}

resource "aws_ebs_default_kms_key" "test" {
key_arn = "${aws_kms_key.test.arn}"
}
`
88 changes: 88 additions & 0 deletions aws/resource_aws_ebs_encryption_by_default.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
package aws

import (
"fmt"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ec2"

"github.com/hashicorp/terraform/helper/resource"
"github.com/hashicorp/terraform/helper/schema"
)

func resourceAwsEbsEncryptionByDefault() *schema.Resource {
return &schema.Resource{
Create: resourceAwsEbsEncryptionByDefaultCreate,
Read: resourceAwsEbsEncryptionByDefaultRead,
Update: resourceAwsEbsEncryptionByDefaultUpdate,
Delete: resourceAwsEbsEncryptionByDefaultDelete,
Copy link
Contributor

Choose a reason for hiding this comment

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

If we are going to do nothing in the Delete function, this can use schema.Noop instead of a custom function.

Suggested change
Delete: resourceAwsEbsEncryptionByDefaultDelete,
Delete: schema.Noop,

The documentation in that case should also explicitly state that it is only removing Terraform's management of the setting.

Instead of an empty Delete function though, I think a better user experience would be do disable encryption when this resource is removed. 👍


Schema: map[string]*schema.Schema{
"enabled": {
Type: schema.TypeBool,
Optional: true,
Default: true,
},
},
}
}

func resourceAwsEbsEncryptionByDefaultCreate(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).ec2conn

enabled := d.Get("enabled").(bool)
if err := setEbsEncryptionByDefault(conn, enabled); err != nil {
return fmt.Errorf("error creating EBS encryption by default (%t): %s", enabled, err)
}

d.SetId(resource.UniqueId())

return resourceAwsEbsEncryptionByDefaultRead(d, meta)
}

func resourceAwsEbsEncryptionByDefaultRead(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).ec2conn

resp, err := conn.GetEbsEncryptionByDefault(&ec2.GetEbsEncryptionByDefaultInput{})
if err != nil {
return fmt.Errorf("error reading EBS encryption by default: %s", err)
}

d.Set("enabled", aws.BoolValue(resp.EbsEncryptionByDefault))

return nil
}

func resourceAwsEbsEncryptionByDefaultUpdate(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).ec2conn

enabled := d.Get("enabled").(bool)
if err := setEbsEncryptionByDefault(conn, enabled); err != nil {
return fmt.Errorf("error updating EBS encryption by default (%t): %s", enabled, err)
}

return resourceAwsEbsEncryptionByDefaultRead(d, meta)
}

func resourceAwsEbsEncryptionByDefaultDelete(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).ec2conn

// Removing the resource disables default encryption.
if err := setEbsEncryptionByDefault(conn, false); err != nil {
return fmt.Errorf("error disabling EBS encryption by default: %s", err)
}

return nil
}

func setEbsEncryptionByDefault(conn *ec2.EC2, enabled bool) error {
var err error

if enabled {
_, err = conn.EnableEbsEncryptionByDefault(&ec2.EnableEbsEncryptionByDefaultInput{})
} else {
_, err = conn.DisableEbsEncryptionByDefault(&ec2.DisableEbsEncryptionByDefaultInput{})
}

return err
}
86 changes: 86 additions & 0 deletions aws/resource_aws_ebs_encryption_by_default_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
package aws

import (
"fmt"
"testing"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/hashicorp/terraform/helper/resource"
"github.com/hashicorp/terraform/terraform"
)

func TestAccAWSEBSEncryptionByDefault_basic(t *testing.T) {
resourceName := "aws_ebs_encryption_by_default.test"

resource.ParallelTest(t, resource.TestCase{
Copy link
Contributor

Choose a reason for hiding this comment

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

If the Delete function is to remain empty, can you please add an explicit CheckDestroy: nil, to this TestCase?

Ideally though, this resource seems like it should enable encryption by default when its added and disable encryption when its removed. The CheckDestroy in that case would verify that encryption is disabled. 😄

PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAwsEncryptionByDefaultDestroy,
Steps: []resource.TestStep{
{
Config: testAccAwsEbsEncryptionByDefaultConfig(false),
Check: resource.ComposeTestCheckFunc(
testAccCheckEbsEncryptionByDefault(resourceName, false),
resource.TestCheckResourceAttr(resourceName, "enabled", "false"),
),
},
{
Config: testAccAwsEbsEncryptionByDefaultConfig(true),
Check: resource.ComposeTestCheckFunc(
testAccCheckEbsEncryptionByDefault(resourceName, true),
resource.TestCheckResourceAttr(resourceName, "enabled", "true"),
),
},
},
})
}

func testAccCheckAwsEncryptionByDefaultDestroy(s *terraform.State) error {
conn := testAccProvider.Meta().(*AWSClient).ec2conn

response, err := conn.GetEbsEncryptionByDefault(&ec2.GetEbsEncryptionByDefaultInput{})
if err != nil {
return err
}

if aws.BoolValue(response.EbsEncryptionByDefault) != false {
return fmt.Errorf("EBS encryption by default not disabled on resource removal")
}

return nil
}

func testAccCheckEbsEncryptionByDefault(n string, enabled bool) resource.TestCheckFunc {
return func(s *terraform.State) error {
rs, ok := s.RootModule().Resources[n]
if !ok {
return fmt.Errorf("Not found: %s", n)
}

if rs.Primary.ID == "" {
return fmt.Errorf("No ID is set")
}

conn := testAccProvider.Meta().(*AWSClient).ec2conn

response, err := conn.GetEbsEncryptionByDefault(&ec2.GetEbsEncryptionByDefaultInput{})
if err != nil {
return err
}

if aws.BoolValue(response.EbsEncryptionByDefault) != enabled {
return fmt.Errorf("EBS encryption by default is not in expected state (%t)", enabled)
}

return nil
}
}

func testAccAwsEbsEncryptionByDefaultConfig(enabled bool) string {
return fmt.Sprintf(`
resource "aws_ebs_encryption_by_default" "test" {
enabled = %[1]t
}
`, enabled)
}
8 changes: 8 additions & 0 deletions website/aws.erb
Original file line number Diff line number Diff line change
Expand Up @@ -1135,6 +1135,14 @@
<a href="/docs/providers/aws/r/ami_launch_permission.html">aws_ami_launch_permission</a>
</li>

<li>
<a href="/docs/providers/aws/r/ebs_default_kms_key.html">aws_ebs_default_kms_key</a>
</li>

<li>
<a href="/docs/providers/aws/r/ebs_encryption_by_default.html">aws_ebs_encryption_by_default</a>
</li>

<li>
<a href="/docs/providers/aws/r/ebs_snapshot.html">aws_ebs_snapshot</a>
</li>
Expand Down
Loading