diff --git a/.changelog/18117.txt b/.changelog/18117.txt new file mode 100644 index 00000000000..bbbc8e18b60 --- /dev/null +++ b/.changelog/18117.txt @@ -0,0 +1,7 @@ +```release-note:enhancement +resource/aws_kms_key: Add `bypass_policy_lockout_safety_check` argument +``` + +```release-note:enhancement +resource/aws_kms_external_key: Add `bypass_policy_lockout_safety_check` argument +``` diff --git a/aws/data_source_aws_kms_alias.go b/aws/data_source_aws_kms_alias.go index 3d8509a246f..21ee79abd00 100644 --- a/aws/data_source_aws_kms_alias.go +++ b/aws/data_source_aws_kms_alias.go @@ -2,26 +2,25 @@ package aws import ( "fmt" - "log" "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/service/kms" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/kms/finder" ) func dataSourceAwsKmsAlias() *schema.Resource { return &schema.Resource{ Read: dataSourceAwsKmsAliasRead, Schema: map[string]*schema.Schema{ + "arn": { + Type: schema.TypeString, + Computed: true, + }, "name": { Type: schema.TypeString, Required: true, ValidateFunc: validateAwsKmsName, }, - "arn": { - Type: schema.TypeString, - Computed: true, - }, "target_key_arn": { Type: schema.TypeString, Computed: true, @@ -36,27 +35,13 @@ func dataSourceAwsKmsAlias() *schema.Resource { func dataSourceAwsKmsAliasRead(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).kmsconn - params := &kms.ListAliasesInput{} - target := d.Get("name") - var alias *kms.AliasListEntry - log.Printf("[DEBUG] Reading KMS Alias: %s", params) - err := conn.ListAliasesPages(params, func(page *kms.ListAliasesOutput, lastPage bool) bool { - for _, entity := range page.Aliases { - if aws.StringValue(entity.AliasName) == target { - alias = entity - return false - } - } + target := d.Get("name").(string) - return true - }) - if err != nil { - return fmt.Errorf("Error fetch KMS alias list: %w", err) - } + alias, err := finder.AliasByName(conn, target) - if alias == nil { - return fmt.Errorf("No alias with name %q found in this region.", target) + if err != nil { + return fmt.Errorf("error reading KMS Alias (%s): %w", target, err) } d.SetId(aws.StringValue(alias.AliasArn)) @@ -73,16 +58,14 @@ func dataSourceAwsKmsAliasRead(d *schema.ResourceData, meta interface{}) error { // // https://docs.aws.amazon.com/kms/latest/APIReference/API_ListAliases.html - req := &kms.DescribeKeyInput{ - KeyId: aws.String(target.(string)), - } - resp, err := conn.DescribeKey(req) + keyMetadata, err := finder.KeyByID(conn, target) + if err != nil { - return fmt.Errorf("Error calling KMS DescribeKey: %w", err) + return fmt.Errorf("error reading KMS Key (%s): %w", target, err) } - d.Set("target_key_arn", resp.KeyMetadata.Arn) - d.Set("target_key_id", resp.KeyMetadata.KeyId) + d.Set("target_key_arn", keyMetadata.Arn) + d.Set("target_key_id", keyMetadata.KeyId) return nil } diff --git a/aws/data_source_aws_kms_alias_test.go b/aws/data_source_aws_kms_alias_test.go index f4eed540139..d35745474e9 100644 --- a/aws/data_source_aws_kms_alias_test.go +++ b/aws/data_source_aws_kms_alias_test.go @@ -8,11 +8,10 @@ import ( "github.com/aws/aws-sdk-go/service/kms" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" - "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" ) func TestAccDataSourceAwsKmsAlias_AwsService(t *testing.T) { - name := "alias/aws/s3" + rName := "alias/aws/s3" resourceName := "data.aws_kms_alias.test" resource.ParallelTest(t, resource.TestCase{ @@ -21,11 +20,10 @@ func TestAccDataSourceAwsKmsAlias_AwsService(t *testing.T) { Providers: testAccProviders, Steps: []resource.TestStep{ { - Config: testAccDataSourceAwsKmsAlias_name(name), + Config: testAccDataSourceAwsKmsAliasConfig(rName), Check: resource.ComposeTestCheckFunc( - testAccDataSourceAwsKmsAliasCheckExists(resourceName), - testAccCheckResourceAttrRegionalARN(resourceName, "arn", "kms", name), - resource.TestCheckResourceAttr(resourceName, "name", name), + testAccCheckResourceAttrRegionalARN(resourceName, "arn", "kms", rName), + resource.TestCheckResourceAttr(resourceName, "name", rName), testAccMatchResourceAttrRegionalARN(resourceName, "target_key_arn", "kms", regexp.MustCompile(`key/[a-z0-9]{8}-([a-z0-9]{4}-){3}[a-z0-9]{12}`)), resource.TestMatchResourceAttr(resourceName, "target_key_id", regexp.MustCompile("^[a-z0-9]{8}-([a-z0-9]{4}-){3}[a-z0-9]{12}$")), ), @@ -35,7 +33,7 @@ func TestAccDataSourceAwsKmsAlias_AwsService(t *testing.T) { } func TestAccDataSourceAwsKmsAlias_CMK(t *testing.T) { - rInt := acctest.RandInt() + rName := acctest.RandomWithPrefix("tf-acc-test") aliasResourceName := "aws_kms_alias.test" datasourceAliasResourceName := "data.aws_kms_alias.test" @@ -45,9 +43,8 @@ func TestAccDataSourceAwsKmsAlias_CMK(t *testing.T) { Providers: testAccProviders, Steps: []resource.TestStep{ { - Config: testAccDataSourceAwsKmsAlias_CMK(rInt), + Config: testAccDataSourceAwsKmsAliasConfigCMK(rName), Check: resource.ComposeTestCheckFunc( - testAccDataSourceAwsKmsAliasCheckExists(datasourceAliasResourceName), resource.TestCheckResourceAttrPair(datasourceAliasResourceName, "arn", aliasResourceName, "arn"), resource.TestCheckResourceAttrPair(datasourceAliasResourceName, "target_key_arn", aliasResourceName, "target_key_arn"), resource.TestCheckResourceAttrPair(datasourceAliasResourceName, "target_key_id", aliasResourceName, "target_key_id"), @@ -57,37 +54,28 @@ func TestAccDataSourceAwsKmsAlias_CMK(t *testing.T) { }) } -func testAccDataSourceAwsKmsAliasCheckExists(name string) resource.TestCheckFunc { - return func(s *terraform.State) error { - _, ok := s.RootModule().Resources[name] - if !ok { - return fmt.Errorf("root module has no resource called %s", name) - } - - return nil - } -} - -func testAccDataSourceAwsKmsAlias_name(name string) string { +func testAccDataSourceAwsKmsAliasConfig(rName string) string { return fmt.Sprintf(` data "aws_kms_alias" "test" { - name = "%s" + name = %[1]q } -`, name) +`, rName) } -func testAccDataSourceAwsKmsAlias_CMK(rInt int) string { +func testAccDataSourceAwsKmsAliasConfigCMK(rName string) string { return fmt.Sprintf(` resource "aws_kms_key" "test" { - description = "Terraform acc test" + description = %[1]q deletion_window_in_days = 7 } resource "aws_kms_alias" "test" { - name = "alias/tf-acc-key-alias-%d" + name = "alias/%[1]s" target_key_id = aws_kms_key.test.key_id } -%s -`, rInt, testAccDataSourceAwsKmsAlias_name("${aws_kms_alias.test.name}")) +data "aws_kms_alias" "test" { + name = aws_kms_alias.test.name +} +`, rName) } diff --git a/aws/internal/keyvaluetags/key_value_tags.go b/aws/internal/keyvaluetags/key_value_tags.go index e22c61cac08..31d4834c54f 100644 --- a/aws/internal/keyvaluetags/key_value_tags.go +++ b/aws/internal/keyvaluetags/key_value_tags.go @@ -428,6 +428,34 @@ func (tags KeyValueTags) ContainsAll(target KeyValueTags) bool { return true } +// Equal returns whether or two sets of key-value tags are equal. +func (tags KeyValueTags) Equal(other KeyValueTags) bool { + if tags == nil && other == nil { + return true + } + + if tags == nil || other == nil { + return false + } + + if len(tags) != len(other) { + return false + } + + for k, v := range tags { + o, ok := other[k] + if !ok { + return false + } + + if !v.Equal(o) { + return false + } + } + + return true +} + // Hash returns a stable hash value. // The returned value may be negative (i.e. not suitable for a 'Set' function). func (tags KeyValueTags) Hash() int { diff --git a/aws/internal/keyvaluetags/key_value_tags_test.go b/aws/internal/keyvaluetags/key_value_tags_test.go index ef432d1afbc..9e3a4b20e4c 100644 --- a/aws/internal/keyvaluetags/key_value_tags_test.go +++ b/aws/internal/keyvaluetags/key_value_tags_test.go @@ -1824,6 +1824,132 @@ func TestKeyValueTagsContainsAll(t *testing.T) { } } +func TestKeyValueTagsEqual(t *testing.T) { + testCases := []struct { + name string + source KeyValueTags + target KeyValueTags + want bool + }{ + { + name: "nil", + source: nil, + target: nil, + want: true, + }, + { + name: "empty", + source: New(map[string]string{}), + target: New(map[string]string{}), + want: true, + }, + { + name: "source_nil", + source: nil, + target: New(map[string]string{ + "key1": "value1", + }), + want: false, + }, + { + name: "source_empty", + source: New(map[string]string{}), + target: New(map[string]string{ + "key1": "value1", + }), + want: false, + }, + { + name: "target_nil", + source: New(map[string]string{ + "key1": "value1", + }), + target: nil, + want: false, + }, + { + name: "target_empty", + source: New(map[string]string{ + "key1": "value1", + }), + target: New(map[string]string{}), + want: false, + }, + { + name: "nil value matches", + source: New(map[string]*string{ + "key1": nil, + }), + target: New(map[string]*string{ + "key1": nil, + }), + want: true, + }, + { + name: "exact_match", + source: New(map[string]string{ + "key1": "value1", + "key2": "value2", + }), + target: New(map[string]string{ + "key1": "value1", + "key2": "value2", + }), + want: true, + }, + { + name: "source_contains_all", + source: New(map[string]string{ + "key1": "value1", + "key2": "value2", + "key3": "value3", + }), + target: New(map[string]string{ + "key1": "value1", + "key3": "value3", + }), + want: false, + }, + { + name: "source_does_not_contain_all", + source: New(map[string]string{ + "key1": "value1", + "key2": "value2", + "key3": "value3", + }), + target: New(map[string]string{ + "key1": "value1", + "key4": "value4", + }), + want: false, + }, + { + name: "target_value_neq", + source: New(map[string]string{ + "key1": "value1", + "key2": "value2", + "key3": "value3", + }), + target: New(map[string]string{ + "key1": "value1", + "key2": "value2", + "key3": "value4", + }), + want: false, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + got := testCase.source.Equal(testCase.target) + + if got != testCase.want { + t.Errorf("unexpected Equal: %t", got) + } + }) + } +} + func TestKeyValueTagsHash(t *testing.T) { testCases := []struct { name string diff --git a/aws/internal/naming/naming_test.go b/aws/internal/naming/naming_test.go index 0242eb1265c..c3f0486279d 100644 --- a/aws/internal/naming/naming_test.go +++ b/aws/internal/naming/naming_test.go @@ -253,6 +253,11 @@ func TestNamePrefixFromName(t *testing.T) { Input: "terraform-20060102150405000000000001", Expected: strPtr("terraform-"), }, + { + TestName: "KMS alias prefix", + Input: "alias/20210723150229087000000002", + Expected: strPtr("alias/"), + }, } for _, testCase := range testCases { diff --git a/aws/internal/service/kms/arn.go b/aws/internal/service/kms/arn.go new file mode 100644 index 00000000000..d54fd3bb334 --- /dev/null +++ b/aws/internal/service/kms/arn.go @@ -0,0 +1,60 @@ +package kms + +import ( + "fmt" + "strings" + + "github.com/aws/aws-sdk-go/aws/arn" +) + +const ( + ARNSeparator = "/" + ARNService = "kms" +) + +// AliasARNToKeyARN converts an alias ARN to a CMK ARN. +func AliasARNToKeyARN(inputARN, keyID string) (string, error) { + parsedARN, err := arn.Parse(inputARN) + + if err != nil { + return "", fmt.Errorf("error parsing ARN (%s): %w", inputARN, err) + } + + if actual, expected := parsedARN.Service, ARNService; actual != expected { + return "", fmt.Errorf("expected service %s in ARN (%s), got: %s", expected, inputARN, actual) + } + + outputARN := arn.ARN{ + Partition: parsedARN.Partition, + Service: parsedARN.Service, + Region: parsedARN.Region, + AccountID: parsedARN.AccountID, + Resource: strings.Join([]string{"key", keyID}, ARNSeparator), + }.String() + + return outputARN, nil +} + +// KeyARNOrIDEqual returns whether two CMK ARNs or IDs are equal. +func KeyARNOrIDEqual(arnOrID1, arnOrID2 string) bool { + if arnOrID1 == arnOrID2 { + return true + } + + // Key ARN: arn:aws:kms:us-east-2:111122223333:key/1234abcd-12ab-34cd-56ef-1234567890ab + // Key ID: 1234abcd-12ab-34cd-56ef-1234567890ab + arn1, err := arn.Parse(arnOrID1) + firstIsARN := err == nil + arn2, err := arn.Parse(arnOrID2) + secondIsARN := err == nil + + if firstIsARN && !secondIsARN { + return arn1.Resource == "key/"+arnOrID2 + } + + if secondIsARN && !firstIsARN { + return arn2.Resource == "key/"+arnOrID1 + } + + return false +} diff --git a/aws/internal/service/kms/arn_test.go b/aws/internal/service/kms/arn_test.go new file mode 100644 index 00000000000..24766949025 --- /dev/null +++ b/aws/internal/service/kms/arn_test.go @@ -0,0 +1,135 @@ +package kms_test + +import ( + "regexp" + "testing" + + tfkms "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/kms" +) + +func TestAliasARNToKeyARN(t *testing.T) { + testCases := []struct { + TestName string + InputARN string + ExpectedError *regexp.Regexp + ExpectedARN string + }{ + { + TestName: "empty ARN", + InputARN: "", + ExpectedError: regexp.MustCompile(`error parsing ARN`), + }, + { + TestName: "unparsable ARN", + InputARN: "test", + ExpectedError: regexp.MustCompile(`error parsing ARN`), + }, + { + TestName: "invalid ARN service", + InputARN: "arn:aws:ec2:us-west-2:123456789012:alias/test-alias", + ExpectedError: regexp.MustCompile(`expected service kms`), + }, + { + TestName: "valid ARN", + InputARN: "arn:aws:kms:us-west-2:123456789012:alias/test-alias", + ExpectedARN: "arn:aws:kms:us-west-2:123456789012:key/test-key", + }, + } + + for _, testCase := range testCases { + t.Run(testCase.TestName, func(t *testing.T) { + got, err := tfkms.AliasARNToKeyARN(testCase.InputARN, "test-key") + + if err == nil && testCase.ExpectedError != nil { + t.Fatalf("expected error %s, got no error", testCase.ExpectedError.String()) + } + + if err != nil && testCase.ExpectedError == nil { + t.Fatalf("got unexpected error: %s", err) + } + + if err != nil && !testCase.ExpectedError.MatchString(err.Error()) { + t.Fatalf("expected error %s, got: %s", testCase.ExpectedError.String(), err) + } + + if got != testCase.ExpectedARN { + t.Errorf("got %s, expected %s", got, testCase.ExpectedARN) + } + }) + } +} + +func TestKeyARNOrIDEqual(t *testing.T) { + testCases := []struct { + name string + first string + second string + want bool + }{ + { + name: "empty", + first: "", + second: "", + want: true, + }, + { + name: "equal IDs", + first: "1234abcd-12ab-34cd-56ef-1234567890ab", + second: "1234abcd-12ab-34cd-56ef-1234567890ab", + want: true, + }, + { + name: "not equal IDs", + first: "1234abcd-12ab-34cd-56ef-1234567890ab", + second: "1234abcd-12ab-34cd-56ef-1234567890ac", + }, + { + name: "equal ARNs", + first: "arn:aws:kms:us-east-2:111122223333:key/1234abcd-12ab-34cd-56ef-1234567890ab", + second: "arn:aws:kms:us-east-2:111122223333:key/1234abcd-12ab-34cd-56ef-1234567890ab", + want: true, + }, + { + name: "not equal ARNs", + first: "arn:aws:kms:us-east-2:111122223333:key/1234abcd-12ab-34cd-56ef-1234567890ab", + second: "arn:aws:kms:us-east-2:111122224444:key/1234abcd-12ab-34cd-56ef-1234567890ab", + }, + { + name: "equal first ID, second ARN", + first: "1234abcd-12ab-34cd-56ef-1234567890ab", + second: "arn:aws:kms:us-east-2:111122223333:key/1234abcd-12ab-34cd-56ef-1234567890ab", + want: true, + }, + { + name: "equal first ARN, second ID", + first: "arn:aws:kms:us-east-2:111122223333:key/1234abcd-12ab-34cd-56ef-1234567890ab", + second: "1234abcd-12ab-34cd-56ef-1234567890ab", + want: true, + }, + { + name: "not equal first ID, second ARN", + first: "1234abcd-12ab-34cd-56ef-1234567890ab", + second: "arn:aws:kms:us-east-2:111122223333:key/1234abcd-12ab-34cd-56ef-1234567890ac", + }, + { + name: "not equal first ARN, second ID", + first: "arn:aws:kms:us-east-2:111122223333:key/1234abcd-12ab-34cd-56ef-1234567890ab", + second: "1234abcd-12ab-34cd-56ef-1234567890ac", + }, + { + name: "not equal first ID, second incorrect ARN", + first: "1234abcd-12ab-34cd-56ef-1234567890ab", + second: "arn:aws:kms:us-east-2:111122223333:alias/1234abcd-12ab-34cd-56ef-1234567890ab", + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + got := tfkms.KeyARNOrIDEqual(testCase.first, testCase.second) + + if got != testCase.want { + t.Errorf("unexpected Equal: %t", got) + } + }) + } +} diff --git a/aws/internal/service/kms/enum.go b/aws/internal/service/kms/enum.go new file mode 100644 index 00000000000..a7e255d21bf --- /dev/null +++ b/aws/internal/service/kms/enum.go @@ -0,0 +1,9 @@ +package kms + +const ( + AliasNamePrefix = "alias/" +) + +const ( + PolicyNameDefault = "default" +) diff --git a/aws/internal/service/kms/finder/finder.go b/aws/internal/service/kms/finder/finder.go new file mode 100644 index 00000000000..a07d0459403 --- /dev/null +++ b/aws/internal/service/kms/finder/finder.go @@ -0,0 +1,134 @@ +package finder + +import ( + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/kms" + "github.com/hashicorp/aws-sdk-go-base/tfawserr" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" +) + +func AliasByName(conn *kms.KMS, name string) (*kms.AliasListEntry, error) { + input := &kms.ListAliasesInput{} + var output *kms.AliasListEntry + + err := conn.ListAliasesPages(input, func(page *kms.ListAliasesOutput, lastPage bool) bool { + if page == nil { + return !lastPage + } + + for _, alias := range page.Aliases { + if aws.StringValue(alias.AliasName) == name { + output = alias + + return false + } + } + + return !lastPage + }) + + if err != nil { + return nil, err + } + + if output == nil { + return nil, &resource.NotFoundError{} + } + + return output, nil +} + +func KeyByID(conn *kms.KMS, id string) (*kms.KeyMetadata, error) { + input := &kms.DescribeKeyInput{ + KeyId: aws.String(id), + } + + output, err := conn.DescribeKey(input) + + if tfawserr.ErrCodeEquals(err, kms.ErrCodeNotFoundException) { + return nil, &resource.NotFoundError{ + LastError: err, + LastRequest: input, + } + } + + if err != nil { + return nil, err + } + + if output == nil || output.KeyMetadata == nil { + return nil, &resource.NotFoundError{ + Message: "Empty result", + LastRequest: input, + } + } + + keyMetadata := output.KeyMetadata + + // Once the CMK is in the pending deletion state Terraform considers it logically deleted. + if state := aws.StringValue(keyMetadata.KeyState); state == kms.KeyStatePendingDeletion { + return nil, &resource.NotFoundError{ + Message: state, + LastRequest: input, + } + } + + return keyMetadata, nil +} + +func KeyPolicyByKeyIDAndPolicyName(conn *kms.KMS, keyID, policyName string) (*string, error) { + input := &kms.GetKeyPolicyInput{ + KeyId: aws.String(keyID), + PolicyName: aws.String(policyName), + } + + output, err := conn.GetKeyPolicy(input) + + if tfawserr.ErrCodeEquals(err, kms.ErrCodeNotFoundException) { + return nil, &resource.NotFoundError{ + LastError: err, + LastRequest: input, + } + } + + if err != nil { + return nil, err + } + + if output == nil { + return nil, &resource.NotFoundError{ + Message: "Empty result", + LastRequest: input, + } + } + + return output.Policy, nil +} + +func KeyRotationEnabledByKeyID(conn *kms.KMS, keyID string) (*bool, error) { + input := &kms.GetKeyRotationStatusInput{ + KeyId: aws.String(keyID), + } + + output, err := conn.GetKeyRotationStatus(input) + + if tfawserr.ErrCodeEquals(err, kms.ErrCodeNotFoundException) { + return nil, &resource.NotFoundError{ + LastError: err, + LastRequest: input, + } + } + + if err != nil { + return nil, err + } + + if output == nil { + return nil, &resource.NotFoundError{ + Message: "Empty result", + LastRequest: input, + } + } + + return output.KeyRotationEnabled, nil +} diff --git a/aws/internal/service/kms/waiter/status.go b/aws/internal/service/kms/waiter/status.go index 194f73a9347..dbef1ad726d 100644 --- a/aws/internal/service/kms/waiter/status.go +++ b/aws/internal/service/kms/waiter/status.go @@ -4,25 +4,22 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/kms" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/kms/finder" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" ) -// KeyState fetches the Key and its State -func KeyState(conn *kms.KMS, keyID string) resource.StateRefreshFunc { +func KeyState(conn *kms.KMS, id string) resource.StateRefreshFunc { return func() (interface{}, string, error) { - input := &kms.DescribeKeyInput{ - KeyId: aws.String(keyID), - } - - output, err := conn.DescribeKey(input) + output, err := finder.KeyByID(conn, id) - if err != nil { - return nil, kms.KeyStateUnavailable, err + if tfresource.NotFound(err) { + return nil, "", nil } - if output == nil || output.KeyMetadata == nil { - return output, kms.KeyStateUnavailable, nil + if err != nil { + return nil, "", err } - return output, aws.StringValue(output.KeyMetadata.KeyState), nil + return output, aws.StringValue(output.KeyState), nil } } diff --git a/aws/internal/service/kms/waiter/waiter.go b/aws/internal/service/kms/waiter/waiter.go index b1fbc22155c..7b4765a39fc 100644 --- a/aws/internal/service/kms/waiter/waiter.go +++ b/aws/internal/service/kms/waiter/waiter.go @@ -3,32 +3,210 @@ package waiter import ( "time" + "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/kms" + "github.com/hashicorp/aws-sdk-go-base/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + awspolicy "github.com/jen20/awspolicyequivalence" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags" + iamwaiter "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/iam/waiter" + tfkms "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/kms" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/kms/finder" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" ) const ( // Maximum amount of time to wait for KeyState to return PendingDeletion KeyStatePendingDeletionTimeout = 20 * time.Minute + + KeyDeletedTimeout = 20 * time.Minute + KeyDescriptionPropagationTimeout = 5 * time.Minute + KeyMaterialImportedTimeout = 10 * time.Minute + KeyRotationUpdatedTimeout = 10 * time.Minute + KeyStatePropagationTimeout = 20 * time.Minute + KeyValidToPropagationTimeout = 5 * time.Minute + + PropagationTimeout = 2 * time.Minute ) -// KeyStatePendingDeletion waits for KeyState to return PendingDeletion -func KeyStatePendingDeletion(conn *kms.KMS, keyID string) (*kms.DescribeKeyOutput, error) { +// IAMPropagation retries the specified function if the returned error indicates an IAM eventual consistency issue. +// If the retries time out the specified function is called one last time. +func IAMPropagation(f func() (interface{}, error)) (interface{}, error) { + return tfresource.RetryWhenAwsErrCodeEquals(iamwaiter.PropagationTimeout, f, kms.ErrCodeMalformedPolicyDocumentException) +} + +func KeyDeleted(conn *kms.KMS, id string) (*kms.KeyMetadata, error) { stateConf := &resource.StateChangeConf{ - Pending: []string{ - kms.KeyStateDisabled, - kms.KeyStateEnabled, - }, - Target: []string{kms.KeyStatePendingDeletion}, - Refresh: KeyState(conn, keyID), - Timeout: KeyStatePendingDeletionTimeout, + Pending: []string{kms.KeyStateDisabled, kms.KeyStateEnabled}, + Target: []string{}, + Refresh: KeyState(conn, id), + Timeout: KeyDeletedTimeout, } outputRaw, err := stateConf.WaitForState() - if output, ok := outputRaw.(*kms.DescribeKeyOutput); ok { + if output, ok := outputRaw.(*kms.KeyMetadata); ok { return output, err } return nil, err } + +func KeyDescriptionPropagated(conn *kms.KMS, id string, description string) error { + checkFunc := func() (bool, error) { + output, err := finder.KeyByID(conn, id) + + if tfresource.NotFound(err) { + return false, nil + } + + if err != nil { + return false, err + } + + return aws.StringValue(output.Description) == description, nil + } + opts := tfresource.WaitOpts{ + ContinuousTargetOccurence: 5, + MinTimeout: 2 * time.Second, + } + + return tfresource.WaitUntil(KeyDescriptionPropagationTimeout, checkFunc, opts) +} + +func KeyMaterialImported(conn *kms.KMS, id string) (*kms.KeyMetadata, error) { + stateConf := &resource.StateChangeConf{ + Pending: []string{kms.KeyStatePendingImport}, + Target: []string{kms.KeyStateDisabled, kms.KeyStateEnabled}, + Refresh: KeyState(conn, id), + Timeout: KeyMaterialImportedTimeout, + } + + outputRaw, err := stateConf.WaitForState() + + if output, ok := outputRaw.(*kms.KeyMetadata); ok { + return output, err + } + + return nil, err +} + +func KeyPolicyPropagated(conn *kms.KMS, id, policy string) error { + checkFunc := func() (bool, error) { + output, err := finder.KeyPolicyByKeyIDAndPolicyName(conn, id, tfkms.PolicyNameDefault) + + if tfresource.NotFound(err) { + return false, nil + } + + if err != nil { + return false, err + } + + equivalent, err := awspolicy.PoliciesAreEquivalent(aws.StringValue(output), policy) + + if err != nil { + return false, err + } + + return equivalent, nil + } + opts := tfresource.WaitOpts{ + ContinuousTargetOccurence: 5, + MinTimeout: 1 * time.Second, + } + + return tfresource.WaitUntil(PropagationTimeout, checkFunc, opts) +} + +func KeyRotationEnabledPropagated(conn *kms.KMS, id string, enabled bool) error { + checkFunc := func() (bool, error) { + output, err := finder.KeyRotationEnabledByKeyID(conn, id) + + if tfresource.NotFound(err) { + return false, nil + } + + if err != nil { + return false, err + } + + return aws.BoolValue(output) == enabled, nil + } + opts := tfresource.WaitOpts{ + ContinuousTargetOccurence: 5, + MinTimeout: 1 * time.Second, + } + + return tfresource.WaitUntil(PropagationTimeout, checkFunc, opts) +} + +func KeyStatePropagated(conn *kms.KMS, id string, enabled bool) error { + checkFunc := func() (bool, error) { + output, err := finder.KeyByID(conn, id) + + if tfresource.NotFound(err) { + return false, nil + } + + if err != nil { + return false, err + } + + return aws.BoolValue(output.Enabled) == enabled, nil + } + opts := tfresource.WaitOpts{ + ContinuousTargetOccurence: 15, + MinTimeout: 2 * time.Second, + } + + return tfresource.WaitUntil(KeyStatePropagationTimeout, checkFunc, opts) +} + +func KeyValidToPropagated(conn *kms.KMS, id string, validTo string) error { + checkFunc := func() (bool, error) { + output, err := finder.KeyByID(conn, id) + + if tfresource.NotFound(err) { + return false, nil + } + + if err != nil { + return false, err + } + + if output.ValidTo != nil { + return aws.TimeValue(output.ValidTo).Format(time.RFC3339) == validTo, nil + } + + return validTo == "", nil + } + opts := tfresource.WaitOpts{ + ContinuousTargetOccurence: 5, + MinTimeout: 2 * time.Second, + } + + return tfresource.WaitUntil(KeyValidToPropagationTimeout, checkFunc, opts) +} + +func TagsPropagated(conn *kms.KMS, id string, tags keyvaluetags.KeyValueTags) error { + checkFunc := func() (bool, error) { + output, err := keyvaluetags.KmsListTags(conn, id) + + if tfawserr.ErrCodeEquals(err, kms.ErrCodeNotFoundException) { + return false, nil + } + + if err != nil { + return false, err + } + + return output.Equal(tags), nil + } + opts := tfresource.WaitOpts{ + ContinuousTargetOccurence: 5, + MinTimeout: 1 * time.Second, + } + + return tfresource.WaitUntil(PropagationTimeout, checkFunc, opts) +} diff --git a/aws/internal/tfresource/errors.go b/aws/internal/tfresource/errors.go index 032ea8f42e9..407804df7d7 100644 --- a/aws/internal/tfresource/errors.go +++ b/aws/internal/tfresource/errors.go @@ -27,12 +27,9 @@ func TimedOut(err error) bool { // SetLastError sets the LastError field on the error if supported. // If lastErr is nil it is ignored. func SetLastError(err, lastErr error) { - var te *resource.TimeoutError - var use *resource.UnexpectedStateError - - if ok := errors.As(err, &te); ok && te.LastError == nil { + if te := (*resource.TimeoutError)(nil); errors.As(err, &te) && te.LastError == nil { te.LastError = lastErr - } else if ok := errors.As(err, &use); ok && use.LastError == nil { + } else if use := (*resource.UnexpectedStateError)(nil); errors.As(err, &use) && use.LastError == nil { use.LastError = lastErr } } diff --git a/aws/internal/tfresource/retry.go b/aws/internal/tfresource/retry.go index 56b891a4258..4dc26737c62 100644 --- a/aws/internal/tfresource/retry.go +++ b/aws/internal/tfresource/retry.go @@ -10,8 +10,9 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" ) -// RetryWhenAwsErrCodeEquals retries the specified function when it returns one of the specified AWS error code. -func RetryWhenAwsErrCodeEquals(timeout time.Duration, f func() (interface{}, error), codes ...string) (interface{}, error) { +// RetryWhenContext retries the function `f` when the error it returns satisfies `predicate`. +// `f` is retried until `timeout` expires. +func RetryWhenContext(ctx context.Context, timeout time.Duration, f func() (interface{}, error), predicate func(error) bool) (interface{}, error) { var output interface{} err := resource.Retry(timeout, func() *resource.RetryError { @@ -19,10 +20,8 @@ func RetryWhenAwsErrCodeEquals(timeout time.Duration, f func() (interface{}, err output, err = f() - for _, code := range codes { - if tfawserr.ErrCodeEquals(err, code) { - return resource.RetryableError(err) - } + if predicate(err) { + return resource.RetryableError(err) } if err != nil { @@ -43,6 +42,52 @@ func RetryWhenAwsErrCodeEquals(timeout time.Duration, f func() (interface{}, err return output, nil } +// RetryWhen retries the function `f` when the error it returns satisfies `predicate`. +// `f` is retried until `timeout` expires. +func RetryWhen(timeout time.Duration, f func() (interface{}, error), predicate func(error) bool) (interface{}, error) { + return RetryWhenContext(context.Background(), timeout, f, predicate) +} + +// RetryWhenAwsErrCodeEqualsContext retries the specified function when it returns one of the specified AWS error code. +func RetryWhenAwsErrCodeEqualsContext(ctx context.Context, timeout time.Duration, f func() (interface{}, error), codes ...string) (interface{}, error) { + return RetryWhenContext(ctx, timeout, f, func(err error) bool { + // https://github.com/hashicorp/aws-sdk-go-base/pull/55 has been merged. + // Once aws-sdk-go-base has been updated, use variadic version of ErrCodeEquals. + for _, code := range codes { + if tfawserr.ErrCodeEquals(err, code) { + return true + } + } + + return false + }) +} + +// RetryWhenAwsErrCodeEquals retries the specified function when it returns one of the specified AWS error code. +func RetryWhenAwsErrCodeEquals(timeout time.Duration, f func() (interface{}, error), codes ...string) (interface{}, error) { + return RetryWhenAwsErrCodeEqualsContext(context.Background(), timeout, f, codes...) +} + +// RetryWhenNotFoundContext retries the specified function when it returns a resource.NotFoundError. +func RetryWhenNotFoundContext(ctx context.Context, timeout time.Duration, f func() (interface{}, error)) (interface{}, error) { + return RetryWhenContext(ctx, timeout, f, NotFound) +} + +// RetryWhenNotFound retries the specified function when it returns a resource.NotFoundError. +func RetryWhenNotFound(timeout time.Duration, f func() (interface{}, error)) (interface{}, error) { + return RetryWhenNotFoundContext(context.Background(), timeout, f) +} + +// RetryWhenNewResourceNotFoundContext retries the specified function when it returns a resource.NotFoundError and `isNewResource` is true. +func RetryWhenNewResourceNotFoundContext(ctx context.Context, timeout time.Duration, f func() (interface{}, error), isNewResource bool) (interface{}, error) { + return RetryWhenContext(ctx, timeout, f, func(err error) bool { return isNewResource && NotFound(err) }) +} + +// RetryWhenNewResourceNotFound retries the specified function when it returns a resource.NotFoundError and `isNewResource` is true. +func RetryWhenNewResourceNotFound(timeout time.Duration, f func() (interface{}, error), isNewResource bool) (interface{}, error) { + return RetryWhenNewResourceNotFoundContext(context.Background(), timeout, f, isNewResource) +} + // RetryConfigContext allows configuration of StateChangeConf's various time arguments. // This is especially useful for AWS services that are prone to throttling, such as Route53, where // the default durations cause problems. To not use a StateChangeConf argument and revert to the diff --git a/aws/internal/tfresource/retry_test.go b/aws/internal/tfresource/retry_test.go index 2edbf74723d..d02c74c098a 100644 --- a/aws/internal/tfresource/retry_test.go +++ b/aws/internal/tfresource/retry_test.go @@ -75,6 +75,155 @@ func TestRetryWhenAwsErrCodeEquals(t *testing.T) { } } +func TestRetryWhenNewResourceNotFound(t *testing.T) { + var retryCount int32 + + testCases := []struct { + Name string + F func() (interface{}, error) + NewResource bool + ExpectError bool + }{ + { + Name: "no error", + F: func() (interface{}, error) { + return nil, nil + }, + }, + { + Name: "no error new resource", + F: func() (interface{}, error) { + return nil, nil + }, + NewResource: true, + }, + { + Name: "non-retryable other error", + F: func() (interface{}, error) { + return nil, errors.New("TestCode") + }, + ExpectError: true, + }, + { + Name: "non-retryable other error new resource", + F: func() (interface{}, error) { + return nil, errors.New("TestCode") + }, + NewResource: true, + ExpectError: true, + }, + { + Name: "non-retryable AWS error", + F: func() (interface{}, error) { + return nil, awserr.New("Testing", "Testing", nil) + }, + ExpectError: true, + }, + { + Name: "retryable NotFoundError not new resource", + F: func() (interface{}, error) { + return nil, &resource.NotFoundError{} + }, + ExpectError: true, + }, + { + Name: "retryable NotFoundError new resource timeout", + F: func() (interface{}, error) { + return nil, &resource.NotFoundError{} + }, + NewResource: true, + ExpectError: true, + }, + { + Name: "retryable NotFoundError success new resource", + F: func() (interface{}, error) { + if atomic.CompareAndSwapInt32(&retryCount, 0, 1) { + return nil, &resource.NotFoundError{} + } + + return nil, nil + }, + NewResource: true, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.Name, func(t *testing.T) { + retryCount = 0 + + _, err := tfresource.RetryWhenNotFound(5*time.Second, testCase.F) + + if testCase.ExpectError && err == nil { + t.Fatal("expected error") + } else if !testCase.ExpectError && err != nil { + t.Fatalf("unexpected error: %s", err) + } + }) + } +} + +func TestRetryWhenNotFound(t *testing.T) { + var retryCount int32 + + testCases := []struct { + Name string + F func() (interface{}, error) + ExpectError bool + }{ + { + Name: "no error", + F: func() (interface{}, error) { + return nil, nil + }, + }, + { + Name: "non-retryable other error", + F: func() (interface{}, error) { + return nil, errors.New("TestCode") + }, + ExpectError: true, + }, + { + Name: "non-retryable AWS error", + F: func() (interface{}, error) { + return nil, awserr.New("Testing", "Testing", nil) + }, + ExpectError: true, + }, + { + Name: "retryable NotFoundError timeout", + F: func() (interface{}, error) { + return nil, &resource.NotFoundError{} + }, + ExpectError: true, + }, + { + Name: "retryable NotFoundError success", + F: func() (interface{}, error) { + if atomic.CompareAndSwapInt32(&retryCount, 0, 1) { + return nil, &resource.NotFoundError{} + } + + return nil, nil + }, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.Name, func(t *testing.T) { + retryCount = 0 + + _, err := tfresource.RetryWhenNotFound(5*time.Second, testCase.F) + + if testCase.ExpectError && err == nil { + t.Fatal("expected error") + } else if !testCase.ExpectError && err != nil { + t.Fatalf("unexpected error: %s", err) + } + }) + } +} + func TestRetryConfigContext_error(t *testing.T) { t.Parallel() diff --git a/aws/internal/tfresource/wait.go b/aws/internal/tfresource/wait.go new file mode 100644 index 00000000000..1a9ef7f89c4 --- /dev/null +++ b/aws/internal/tfresource/wait.go @@ -0,0 +1,64 @@ +package tfresource + +import ( + "context" + "time" + + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" +) + +type WaitOpts struct { + ContinuousTargetOccurence int // Number of times the target state has to occur continuously. + Delay time.Duration // Wait this time before starting checks. + MinTimeout time.Duration // Smallest time to wait before refreshes. + PollInterval time.Duration // Override MinTimeout/backoff and only poll this often. +} + +const ( + targetStateError = "ERROR" + targetStateFalse = "FALSE" + targetStateTrue = "TRUE" +) + +// WaitUntilContext waits for the function `f` to return `true`. +// If `f` returns an error, return immediately with that error. +// If `timeout` is exceeded before `f` returns `true`, return an error. +// Waits between calls to `f` using exponential backoff, except when waiting for the target state to reoccur. +func WaitUntilContext(ctx context.Context, timeout time.Duration, f func() (bool, error), opts WaitOpts) error { + refresh := func() (interface{}, string, error) { + done, err := f() + + if err != nil { + return nil, targetStateError, err + } + + if done { + return "", targetStateTrue, nil + } + + return "", targetStateFalse, nil + } + + stateConf := &resource.StateChangeConf{ + Pending: []string{targetStateFalse}, + Target: []string{targetStateTrue}, + Refresh: refresh, + Timeout: timeout, + ContinuousTargetOccurence: opts.ContinuousTargetOccurence, + Delay: opts.Delay, + MinTimeout: opts.MinTimeout, + PollInterval: opts.PollInterval, + } + + _, err := stateConf.WaitForStateContext(ctx) + + return err +} + +// WaitUntil waits for the function `f` to return `true`. +// If `f` returns an error, return immediately with that error. +// If `timeout` is exceeded before `f` returns `true`, return an error. +// Waits between calls to `f` using exponential backoff, except when waiting for the target state to reoccur. +func WaitUntil(timeout time.Duration, f func() (bool, error), opts WaitOpts) error { + return WaitUntilContext(context.Background(), timeout, f, opts) +} diff --git a/aws/internal/tfresource/wait_test.go b/aws/internal/tfresource/wait_test.go new file mode 100644 index 00000000000..8062b0e14d5 --- /dev/null +++ b/aws/internal/tfresource/wait_test.go @@ -0,0 +1,65 @@ +package tfresource_test + +import ( + "errors" + "sync/atomic" + "testing" + "time" + + "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" +) + +func TestWaitUntil(t *testing.T) { + var retryCount int32 + + testCases := []struct { + Name string + F func() (bool, error) + ExpectError bool + }{ + { + Name: "no error", + F: func() (bool, error) { + return true, nil + }, + }, + { + Name: "immediate error", + F: func() (bool, error) { + return false, errors.New("TestCode") + }, + ExpectError: true, + }, + { + Name: "never reaches state", + F: func() (bool, error) { + return false, nil + }, + ExpectError: true, + }, + { + Name: "retry then success", + F: func() (bool, error) { + if atomic.CompareAndSwapInt32(&retryCount, 0, 1) { + return true, nil + } + + return false, nil + }, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.Name, func(t *testing.T) { + retryCount = 0 + + err := tfresource.WaitUntil(5*time.Second, testCase.F, tfresource.WaitOpts{}) + + if testCase.ExpectError && err == nil { + t.Fatal("expected error") + } else if !testCase.ExpectError && err != nil { + t.Fatalf("unexpected error: %s", err) + } + }) + } +} diff --git a/aws/resource_aws_ebs_default_kms_key_test.go b/aws/resource_aws_ebs_default_kms_key_test.go index 28c5d0f26f8..68b25d457d2 100644 --- a/aws/resource_aws_ebs_default_kms_key_test.go +++ b/aws/resource_aws_ebs_default_kms_key_test.go @@ -9,6 +9,7 @@ import ( "github.com/aws/aws-sdk-go/service/ec2" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" + kmsfinder "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/kms/finder" ) func TestAccAWSEBSDefaultKmsKey_basic(t *testing.T) { @@ -94,7 +95,7 @@ func testAccCheckEbsDefaultKmsKey(name string) resource.TestCheckFunc { func testAccAwsEbsDefaultKmsKeyAwsManagedDefaultKey() (*arn.ARN, error) { conn := testAccProvider.Meta().(*AWSClient).kmsconn - alias, err := findKmsAliasByName(conn, "alias/aws/ebs", nil) + alias, err := kmsfinder.AliasByName(conn, "alias/aws/ebs") if err != nil { return nil, err } diff --git a/aws/resource_aws_kms_alias.go b/aws/resource_aws_kms_alias.go index a47216d81b3..c4229acc8ba 100644 --- a/aws/resource_aws_kms_alias.go +++ b/aws/resource_aws_kms_alias.go @@ -3,14 +3,16 @@ package aws import ( "fmt" "log" - "strings" - "time" "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/arn" "github.com/aws/aws-sdk-go/service/kms" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/hashicorp/aws-sdk-go-base/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/naming" + tfkms "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/kms" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/kms/finder" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/kms/waiter" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" ) func resourceAwsKmsAlias() *schema.Resource { @@ -21,7 +23,7 @@ func resourceAwsKmsAlias() *schema.Resource { Delete: resourceAwsKmsAliasDelete, Importer: &schema.ResourceImporter{ - State: resourceAwsKmsAliasImport, + State: schema.ImportStatePassthrough, }, Schema: map[string]*schema.Schema{ @@ -29,29 +31,35 @@ func resourceAwsKmsAlias() *schema.Resource { Type: schema.TypeString, Computed: true, }, + "name": { Type: schema.TypeString, Optional: true, + Computed: true, ForceNew: true, ConflictsWith: []string{"name_prefix"}, ValidateFunc: validateAwsKmsName, }, + "name_prefix": { Type: schema.TypeString, Optional: true, + Computed: true, ForceNew: true, ConflictsWith: []string{"name"}, ValidateFunc: validateAwsKmsName, }, - "target_key_id": { - Type: schema.TypeString, - Required: true, - DiffSuppressFunc: suppressEquivalentTargetKeyIdAndARN, - }, + "target_key_arn": { Type: schema.TypeString, Computed: true, }, + + "target_key_id": { + Type: schema.TypeString, + Required: true, + DiffSuppressFunc: suppressEquivalentKmsKeyARNOrID, + }, }, } } @@ -59,71 +67,64 @@ func resourceAwsKmsAlias() *schema.Resource { func resourceAwsKmsAliasCreate(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).kmsconn - var name string - if v, ok := d.GetOk("name"); ok { - name = v.(string) - } else if v, ok := d.GetOk("name_prefix"); ok { - name = resource.PrefixedUniqueId(v.(string)) - } else { - name = resource.PrefixedUniqueId("alias/") + namePrefix := d.Get("name_prefix").(string) + if namePrefix == "" { + namePrefix = tfkms.AliasNamePrefix } + name := naming.Generate(d.Get("name").(string), namePrefix) - targetKeyId := d.Get("target_key_id").(string) - - log.Printf("[DEBUG] KMS alias create name: %s, target_key: %s", name, targetKeyId) - - req := &kms.CreateAliasInput{ + input := &kms.CreateAliasInput{ AliasName: aws.String(name), - TargetKeyId: aws.String(targetKeyId), + TargetKeyId: aws.String(d.Get("target_key_id").(string)), } - // KMS is eventually consistent - _, err := retryOnAwsCode("NotFoundException", func() (interface{}, error) { - return conn.CreateAlias(req) - }) + // KMS is eventually consistent. + log.Printf("[DEBUG] Creating KMS Alias: %s", input) + + _, err := tfresource.RetryWhenAwsErrCodeEquals(waiter.KeyRotationUpdatedTimeout, func() (interface{}, error) { + return conn.CreateAlias(input) + }, kms.ErrCodeNotFoundException) + if err != nil { - return err + return fmt.Errorf("error creating KMS Alias (%s): %w", name, err) } + d.SetId(name) + return resourceAwsKmsAliasRead(d, meta) } func resourceAwsKmsAliasRead(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).kmsconn - var alias *kms.AliasListEntry - var err error - if d.IsNewResource() { - alias, err = retryFindKmsAliasByName(conn, d.Id()) - } else { - alias, err = findKmsAliasByName(conn, d.Id(), nil) - } - if err != nil { - return err - } - if alias == nil { - log.Printf("[DEBUG] Removing KMS Alias (%s) as it's already gone", d.Id()) + outputRaw, err := tfresource.RetryWhenNewResourceNotFound(waiter.PropagationTimeout, func() (interface{}, error) { + return finder.AliasByName(conn, d.Id()) + }, d.IsNewResource()) + + if !d.IsNewResource() && tfresource.NotFound(err) { + log.Printf("[WARN] KMS Alias (%s) not found, removing from state", d.Id()) d.SetId("") return nil } - log.Printf("[DEBUG] Found KMS Alias: %s", alias) + if err != nil { + return fmt.Errorf("error reading KMS Alias (%s): %w", d.Id(), err) + } - d.Set("arn", alias.AliasArn) - d.Set("target_key_id", alias.TargetKeyId) + alias := outputRaw.(*kms.AliasListEntry) + aliasARN := aws.StringValue(alias.AliasArn) + targetKeyID := aws.StringValue(alias.TargetKeyId) + targetKeyARN, err := tfkms.AliasARNToKeyARN(aliasARN, targetKeyID) - aliasARN, err := arn.Parse(*alias.AliasArn) if err != nil { return err } - targetKeyARN := arn.ARN{ - Partition: aliasARN.Partition, - Service: aliasARN.Service, - Region: aliasARN.Region, - AccountID: aliasARN.AccountID, - Resource: fmt.Sprintf("key/%s", *alias.TargetKeyId), - } - d.Set("target_key_arn", targetKeyARN.String()) + + d.Set("arn", aliasARN) + d.Set("name", alias.AliasName) + d.Set("name_prefix", naming.NamePrefixFromName(aws.StringValue(alias.AliasName))) + d.Set("target_key_arn", targetKeyARN) + d.Set("target_key_id", targetKeyID) return nil } @@ -132,109 +133,41 @@ func resourceAwsKmsAliasUpdate(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).kmsconn if d.HasChange("target_key_id") { - err := resourceAwsKmsAliasTargetUpdate(conn, d) - if err != nil { - return err + input := &kms.UpdateAliasInput{ + AliasName: aws.String(d.Id()), + TargetKeyId: aws.String(d.Get("target_key_id").(string)), } - return resourceAwsKmsAliasRead(d, meta) - } - return nil -} - -func resourceAwsKmsAliasTargetUpdate(conn *kms.KMS, d *schema.ResourceData) error { - name := d.Get("name").(string) - targetKeyId := d.Get("target_key_id").(string) - log.Printf("[DEBUG] KMS alias: %s, update target: %s", name, targetKeyId) + log.Printf("[DEBUG] Updating KMS Alias: %s", input) + _, err := conn.UpdateAlias(input) - req := &kms.UpdateAliasInput{ - AliasName: aws.String(name), - TargetKeyId: aws.String(targetKeyId), + if err != nil { + return fmt.Errorf("error updating KMS Alias (%s): %w", d.Id(), err) + } } - _, err := conn.UpdateAlias(req) - return err + return resourceAwsKmsAliasRead(d, meta) } func resourceAwsKmsAliasDelete(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).kmsconn - req := &kms.DeleteAliasInput{ + log.Printf("[DEBUG] Deleting KMS Alias: (%s)", d.Id()) + _, err := conn.DeleteAlias(&kms.DeleteAliasInput{ AliasName: aws.String(d.Id()), - } - _, err := conn.DeleteAlias(req) - if err != nil { - return err - } - - log.Printf("[DEBUG] KMS Alias: (%s) deleted.", d.Id()) - - return nil -} - -func retryFindKmsAliasByName(conn *kms.KMS, name string) (*kms.AliasListEntry, error) { - var resp *kms.AliasListEntry - err := resource.Retry(1*time.Minute, func() *resource.RetryError { - var err error - resp, err = findKmsAliasByName(conn, name, nil) - if err != nil { - return resource.NonRetryableError(err) - } - if resp == nil { - return resource.RetryableError(&resource.NotFoundError{}) - } - return nil }) - if isResourceTimeoutError(err) { - resp, err = findKmsAliasByName(conn, name, nil) - } - - return resp, err -} - -// API by default limits results to 50 aliases -// This is how we make sure we won't miss any alias -// See http://docs.aws.amazon.com/kms/latest/APIReference/API_ListAliases.html -func findKmsAliasByName(conn *kms.KMS, name string, marker *string) (*kms.AliasListEntry, error) { - req := kms.ListAliasesInput{ - Limit: aws.Int64(int64(100)), - } - if marker != nil { - req.Marker = marker + if tfawserr.ErrCodeEquals(err, kms.ErrCodeNotFoundException) { + return nil } - log.Printf("[DEBUG] Listing KMS aliases: %s", req) - resp, err := conn.ListAliases(&req) if err != nil { - return nil, err - } - - for _, entry := range resp.Aliases { - if *entry.AliasName == name { - return entry, nil - } - } - if *resp.Truncated { - log.Printf("[DEBUG] KMS alias list is truncated, listing more via %s", *resp.NextMarker) - return findKmsAliasByName(conn, name, resp.NextMarker) + return fmt.Errorf("error deleting KMS Alias (%s): %w", d.Id(), err) } - return nil, nil -} - -func resourceAwsKmsAliasImport(d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) { - d.Set("name", d.Id()) - return []*schema.ResourceData{d}, nil + return nil } -func suppressEquivalentTargetKeyIdAndARN(k, old, new string, d *schema.ResourceData) bool { - newARN, err := arn.Parse(new) - if err != nil { - log.Printf("[DEBUG] %q can not be parsed as an ARN: %q", new, err) - return false - } - - resource := strings.TrimPrefix(newARN.Resource, "key/") - return old == resource +func suppressEquivalentKmsKeyARNOrID(k, old, new string, d *schema.ResourceData) bool { + return tfkms.KeyARNOrIDEqual(old, new) } diff --git a/aws/resource_aws_kms_alias_test.go b/aws/resource_aws_kms_alias_test.go index 810ea7e511b..86f0be77b1c 100644 --- a/aws/resource_aws_kms_alias_test.go +++ b/aws/resource_aws_kms_alias_test.go @@ -2,19 +2,24 @@ package aws import ( "fmt" + "regexp" "testing" - "time" "github.com/aws/aws-sdk-go/service/kms" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/naming" + tfkms "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/kms" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/kms/finder" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" ) func TestAccAWSKmsAlias_basic(t *testing.T) { - rInt := acctest.RandInt() - kmsAliasTimestamp := time.Now().Format(time.RFC1123) + var alias kms.AliasListEntry + rName := acctest.RandomWithPrefix("tf-acc-test") resourceName := "aws_kms_alias.test" + keyResourceName := "aws_kms_key.test" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -23,10 +28,13 @@ func TestAccAWSKmsAlias_basic(t *testing.T) { CheckDestroy: testAccCheckAWSKmsAliasDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSKmsSingleAlias(rInt, kmsAliasTimestamp), + Config: testAccAWSKmsAliasConfigName(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSKmsAliasExists(resourceName), - resource.TestCheckResourceAttrSet(resourceName, "target_key_arn"), + testAccCheckAWSKmsAliasExists(resourceName, &alias), + testAccMatchResourceAttrRegionalARN(resourceName, "arn", "kms", regexp.MustCompile(`alias/.+`)), + resource.TestCheckResourceAttr(resourceName, "name", tfkms.AliasNamePrefix+rName), + resource.TestCheckResourceAttrPair(resourceName, "target_key_arn", keyResourceName, "arn"), + resource.TestCheckResourceAttrPair(resourceName, "target_key_id", keyResourceName, "id"), ), }, { @@ -34,19 +42,36 @@ func TestAccAWSKmsAlias_basic(t *testing.T) { ImportState: true, ImportStateVerify: true, }, + }, + }) +} + +func TestAccAWSKmsAlias_disappears(t *testing.T) { + var alias kms.AliasListEntry + rName := acctest.RandomWithPrefix("tf-acc-test") + resourceName := "aws_kms_alias.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + ErrorCheck: testAccErrorCheck(t, kms.EndpointsID), + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSKmsAliasDestroy, + Steps: []resource.TestStep{ { - Config: testAccAWSKmsSingleAlias_modified(rInt, kmsAliasTimestamp), + Config: testAccAWSKmsAliasConfigName(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSKmsAliasExists(resourceName), + testAccCheckAWSKmsAliasExists(resourceName, &alias), + testAccCheckResourceDisappears(testAccProvider, resourceAwsKmsAlias(), resourceName), ), + ExpectNonEmptyPlan: true, }, }, }) } -func TestAccAWSKmsAlias_name_prefix(t *testing.T) { - rInt := acctest.RandInt() - kmsAliasTimestamp := time.Now().Format(time.RFC1123) +func TestAccAWSKmsAlias_Name_Generated(t *testing.T) { + var alias kms.AliasListEntry + rName := acctest.RandomWithPrefix("tf-acc-test") resourceName := "aws_kms_alias.test" resource.ParallelTest(t, resource.TestCase{ @@ -56,10 +81,11 @@ func TestAccAWSKmsAlias_name_prefix(t *testing.T) { CheckDestroy: testAccCheckAWSKmsAliasDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSKmsSingleAlias(rInt, kmsAliasTimestamp), + Config: testAccAWSKmsAliasConfigNameGenerated(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSKmsAliasExists("aws_kms_alias.name_prefix"), - resource.TestCheckResourceAttrSet("aws_kms_alias.name_prefix", "target_key_arn"), + testAccCheckAWSKmsAliasExists(resourceName, &alias), + resource.TestMatchResourceAttr(resourceName, "name", regexp.MustCompile(fmt.Sprintf("%s[[:xdigit:]]{%d}", tfkms.AliasNamePrefix, resource.UniqueIDSuffixLength))), + resource.TestCheckResourceAttr(resourceName, "name_prefix", tfkms.AliasNamePrefix), ), }, { @@ -71,9 +97,9 @@ func TestAccAWSKmsAlias_name_prefix(t *testing.T) { }) } -func TestAccAWSKmsAlias_no_name(t *testing.T) { - rInt := acctest.RandInt() - kmsAliasTimestamp := time.Now().Format(time.RFC1123) +func TestAccAWSKmsAlias_NamePrefix(t *testing.T) { + var alias kms.AliasListEntry + rName := acctest.RandomWithPrefix("tf-acc-test") resourceName := "aws_kms_alias.test" resource.ParallelTest(t, resource.TestCase{ @@ -83,10 +109,11 @@ func TestAccAWSKmsAlias_no_name(t *testing.T) { CheckDestroy: testAccCheckAWSKmsAliasDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSKmsSingleAlias(rInt, kmsAliasTimestamp), + Config: testAccAWSKmsAliasConfigNamePrefix(rName, tfkms.AliasNamePrefix+"tf-acc-test-prefix-"), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSKmsAliasExists("aws_kms_alias.nothing"), - resource.TestCheckResourceAttrSet("aws_kms_alias.nothing", "target_key_arn"), + testAccCheckAWSKmsAliasExists(resourceName, &alias), + naming.TestCheckResourceAttrNameFromPrefix(resourceName, "name", tfkms.AliasNamePrefix+"tf-acc-test-prefix-"), + resource.TestCheckResourceAttr(resourceName, "name_prefix", tfkms.AliasNamePrefix+"tf-acc-test-prefix-"), ), }, { @@ -98,10 +125,12 @@ func TestAccAWSKmsAlias_no_name(t *testing.T) { }) } -func TestAccAWSKmsAlias_multiple(t *testing.T) { - rInt := acctest.RandInt() - kmsAliasTimestamp := time.Now().Format(time.RFC1123) +func TestAccAWSKmsAlias_UpdateKeyID(t *testing.T) { + var alias kms.AliasListEntry + rName := acctest.RandomWithPrefix("tf-acc-test") resourceName := "aws_kms_alias.test" + key1ResourceName := "aws_kms_key.test" + key2ResourceName := "aws_kms_key.test2" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -110,12 +139,52 @@ func TestAccAWSKmsAlias_multiple(t *testing.T) { CheckDestroy: testAccCheckAWSKmsAliasDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSKmsMultipleAliases(rInt, kmsAliasTimestamp), + Config: testAccAWSKmsAliasConfigName(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSKmsAliasExists("aws_kms_alias.test"), - resource.TestCheckResourceAttrSet("aws_kms_alias.test", "target_key_arn"), - testAccCheckAWSKmsAliasExists("aws_kms_alias.test2"), - resource.TestCheckResourceAttrSet("aws_kms_alias.test2", "target_key_arn"), + testAccCheckAWSKmsAliasExists(resourceName, &alias), + resource.TestCheckResourceAttrPair(resourceName, "target_key_arn", key1ResourceName, "arn"), + resource.TestCheckResourceAttrPair(resourceName, "target_key_id", key1ResourceName, "id"), + ), + }, + { + Config: testAccAWSKmsAliasConfigUpdatedKeyId(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSKmsAliasExists(resourceName, &alias), + resource.TestCheckResourceAttrPair(resourceName, "target_key_arn", key2ResourceName, "arn"), + resource.TestCheckResourceAttrPair(resourceName, "target_key_id", key2ResourceName, "id"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +func TestAccAWSKmsAlias_MultipleAliasesForSameKey(t *testing.T) { + var alias kms.AliasListEntry + rName := acctest.RandomWithPrefix("tf-acc-test") + resourceName := "aws_kms_alias.test" + alias2ResourceName := "aws_kms_alias.test2" + keyResourceName := "aws_kms_key.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + ErrorCheck: testAccErrorCheck(t, kms.EndpointsID), + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSKmsAliasDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSKmsAliasConfigMultiple(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSKmsAliasExists(resourceName, &alias), + resource.TestCheckResourceAttrPair(resourceName, "target_key_arn", keyResourceName, "arn"), + resource.TestCheckResourceAttrPair(resourceName, "target_key_id", keyResourceName, "id"), + testAccCheckAWSKmsAliasExists(alias2ResourceName, &alias), + resource.TestCheckResourceAttrPair(alias2ResourceName, "target_key_arn", keyResourceName, "arn"), + resource.TestCheckResourceAttrPair(alias2ResourceName, "target_key_id", keyResourceName, "id"), ), }, { @@ -128,8 +197,8 @@ func TestAccAWSKmsAlias_multiple(t *testing.T) { } func TestAccAWSKmsAlias_ArnDiffSuppress(t *testing.T) { - rInt := acctest.RandInt() - kmsAliasTimestamp := time.Now().Format(time.RFC1123) + var alias kms.AliasListEntry + rName := acctest.RandomWithPrefix("tf-acc-test") resourceName := "aws_kms_alias.test" resource.ParallelTest(t, resource.TestCase{ @@ -139,10 +208,10 @@ func TestAccAWSKmsAlias_ArnDiffSuppress(t *testing.T) { CheckDestroy: testAccCheckAWSKmsAliasDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSKmsArnDiffSuppress(rInt, kmsAliasTimestamp), + Config: testAccAWSKmsAliasConfigDiffSuppress(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSKmsAliasExists("aws_kms_alias.test"), - resource.TestCheckResourceAttrSet("aws_kms_alias.test", "target_key_arn"), + testAccCheckAWSKmsAliasExists(resourceName, &alias), + resource.TestCheckResourceAttrSet(resourceName, "target_key_arn"), ), }, { @@ -153,7 +222,7 @@ func TestAccAWSKmsAlias_ArnDiffSuppress(t *testing.T) { { ExpectNonEmptyPlan: false, PlanOnly: true, - Config: testAccAWSKmsArnDiffSuppress(rInt, kmsAliasTimestamp), + Config: testAccAWSKmsAliasConfigDiffSuppress(rName), }, }, }) @@ -167,107 +236,136 @@ func testAccCheckAWSKmsAliasDestroy(s *terraform.State) error { continue } - entry, err := findKmsAliasByName(conn, rs.Primary.ID, nil) + _, err := finder.AliasByName(conn, rs.Primary.ID) + + if tfresource.NotFound(err) { + continue + } + if err != nil { return err } - if entry != nil { - return fmt.Errorf("KMS alias still exists:\n%#v", entry) - } - return nil + return fmt.Errorf("KMS Alias %s still exists", rs.Primary.ID) } return nil } -func testAccCheckAWSKmsAliasExists(name string) resource.TestCheckFunc { +func testAccCheckAWSKmsAliasExists(name string, v *kms.AliasListEntry) resource.TestCheckFunc { return func(s *terraform.State) error { - _, ok := s.RootModule().Resources[name] + rs, ok := s.RootModule().Resources[name] if !ok { return fmt.Errorf("Not found: %s", name) } + if rs.Primary.ID == "" { + return fmt.Errorf("No KMS Alias ID is set") + } + + conn := testAccProvider.Meta().(*AWSClient).kmsconn + + output, err := finder.AliasByName(conn, rs.Primary.ID) + + if err != nil { + return err + } + + *v = *output + return nil } } -func testAccAWSKmsSingleAlias(rInt int, timestamp string) string { +func testAccAWSKmsAliasConfigName(rName string) string { return fmt.Sprintf(` resource "aws_kms_key" "test" { - description = "Terraform acc test One %s" + description = %[1]q deletion_window_in_days = 7 } -resource "aws_kms_key" "test2" { - description = "Terraform acc test Two %s" +resource "aws_kms_alias" "test" { + name = "alias/%[1]s" + target_key_id = aws_kms_key.test.id +} +`, rName) +} + +func testAccAWSKmsAliasConfigNameGenerated(rName string) string { + return fmt.Sprintf(` +resource "aws_kms_key" "test" { + description = %[1]q deletion_window_in_days = 7 } -resource "aws_kms_alias" "name_prefix" { - name_prefix = "alias/tf-acc-key-alias-%d" - target_key_id = aws_kms_key.test.key_id +resource "aws_kms_alias" "test" { + target_key_id = aws_kms_key.test.id +} +`, rName) } -resource "aws_kms_alias" "nothing" { - target_key_id = aws_kms_key.test.key_id +func testAccAWSKmsAliasConfigNamePrefix(rName, namePrefix string) string { + return fmt.Sprintf(` +resource "aws_kms_key" "test" { + description = %[1]q + deletion_window_in_days = 7 } resource "aws_kms_alias" "test" { - name = "alias/tf-acc-key-alias-%d" - target_key_id = aws_kms_key.test.key_id + name_prefix = %[2]q + target_key_id = aws_kms_key.test.id } -`, timestamp, timestamp, rInt, rInt) +`, rName, namePrefix) } -func testAccAWSKmsSingleAlias_modified(rInt int, timestamp string) string { +func testAccAWSKmsAliasConfigUpdatedKeyId(rName string) string { return fmt.Sprintf(` resource "aws_kms_key" "test" { - description = "Terraform acc test One %s" + description = %[1]q deletion_window_in_days = 7 } resource "aws_kms_key" "test2" { - description = "Terraform acc test Two %s" + description = "%[1]s-2" deletion_window_in_days = 7 } resource "aws_kms_alias" "test" { - name = "alias/tf-acc-key-alias-%d" - target_key_id = aws_kms_key.test2.key_id + name = "alias/%[1]s" + target_key_id = aws_kms_key.test2.id } -`, timestamp, timestamp, rInt) +`, rName) } -func testAccAWSKmsMultipleAliases(rInt int, timestamp string) string { +func testAccAWSKmsAliasConfigMultiple(rName string) string { return fmt.Sprintf(` resource "aws_kms_key" "test" { - description = "Terraform acc test One %s" + description = %[1]q deletion_window_in_days = 7 } resource "aws_kms_alias" "test" { - name = "alias/tf-acc-alias-test-%d" + name = "alias/%[1]s-1" target_key_id = aws_kms_key.test.key_id } resource "aws_kms_alias" "test2" { - name = "alias/tf-acc-alias-test2-%d" + name = "alias/%[1]s-2" target_key_id = aws_kms_key.test.key_id } -`, timestamp, rInt, rInt) +`, rName) } -func testAccAWSKmsArnDiffSuppress(rInt int, timestamp string) string { +func testAccAWSKmsAliasConfigDiffSuppress(rName string) string { return fmt.Sprintf(` resource "aws_kms_key" "test" { - description = "Terraform acc test test %s" + description = %[1]q deletion_window_in_days = 7 } resource "aws_kms_alias" "test" { - name = "alias/tf-acc-key-alias-%d" + name = "alias/%[1]s" target_key_id = aws_kms_key.test.arn } -`, timestamp, rInt) +`, rName) } diff --git a/aws/resource_aws_kms_external_key.go b/aws/resource_aws_kms_external_key.go index 933dd6e7dd7..81ea14ae22b 100644 --- a/aws/resource_aws_kms_external_key.go +++ b/aws/resource_aws_kms_external_key.go @@ -12,13 +12,12 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/kms" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/hashicorp/aws-sdk-go-base/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/structure" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags" - iamwaiter "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/iam/waiter" "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/kms/waiter" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" ) func resourceAwsKmsExternalKey() *schema.Resource { @@ -39,40 +38,54 @@ func resourceAwsKmsExternalKey() *schema.Resource { Type: schema.TypeString, Computed: true, }, + + "bypass_policy_lockout_safety_check": { + Type: schema.TypeBool, + Optional: true, + Default: false, + }, + "deletion_window_in_days": { Type: schema.TypeInt, Optional: true, Default: 30, ValidateFunc: validation.IntBetween(7, 30), }, + "description": { Type: schema.TypeString, Optional: true, ValidateFunc: validation.StringLenBetween(0, 8192), }, + "enabled": { Type: schema.TypeBool, Optional: true, Computed: true, }, + "expiration_model": { Type: schema.TypeString, Computed: true, }, + "key_material_base64": { Type: schema.TypeString, Optional: true, ForceNew: true, Sensitive: true, }, + "key_state": { Type: schema.TypeString, Computed: true, }, + "key_usage": { Type: schema.TypeString, Computed: true, }, + "policy": { Type: schema.TypeString, Optional: true, @@ -83,8 +96,10 @@ func resourceAwsKmsExternalKey() *schema.Resource { validation.StringIsJSON, ), }, + "tags": tagsSchema(), "tags_all": tagsSchemaComputed(), + "valid_to": { Type: schema.TypeString, Optional: true, @@ -100,8 +115,9 @@ func resourceAwsKmsExternalKeyCreate(d *schema.ResourceData, meta interface{}) e tags := defaultTagsConfig.MergeTags(keyvaluetags.New(d.Get("tags").(map[string]interface{}))) input := &kms.CreateKeyInput{ - KeyUsage: aws.String(kms.KeyUsageTypeEncryptDecrypt), - Origin: aws.String(kms.OriginTypeExternal), + BypassPolicyLockoutSafetyCheck: aws.Bool(d.Get("bypass_policy_lockout_safety_check").(bool)), + KeyUsage: aws.String(kms.KeyUsageTypeEncryptDecrypt), + Origin: aws.String(kms.OriginTypeExternal), } if v, ok := d.GetOk("description"); ok { @@ -116,46 +132,43 @@ func resourceAwsKmsExternalKeyCreate(d *schema.ResourceData, meta interface{}) e input.Tags = tags.IgnoreAws().KmsTags() } - var output *kms.CreateKeyOutput - err := resource.Retry(iamwaiter.PropagationTimeout, func() *resource.RetryError { - var err error - - output, err = conn.CreateKey(input) + // AWS requires any principal in the policy to exist before the key is created. + // The KMS service's awareness of principals is limited by "eventual consistency". + // KMS will report this error until it can validate the policy itself. + // They acknowledge this here: + // http://docs.aws.amazon.com/kms/latest/APIReference/API_CreateKey.html + log.Printf("[DEBUG] Creating KMS External Key: %s", input) - // AWS requires any principal in the policy to exist before the key is created. - // The KMS service's awareness of principals is limited by "eventual consistency". - // KMS will report this error until it can validate the policy itself. - // They acknowledge this here: - // http://docs.aws.amazon.com/kms/latest/APIReference/API_CreateKey.html - if isAWSErr(err, kms.ErrCodeMalformedPolicyDocumentException, "") { - return resource.RetryableError(err) - } - - if err != nil { - return resource.NonRetryableError(err) - } - - return nil + outputRaw, err := waiter.IAMPropagation(func() (interface{}, error) { + return conn.CreateKey(input) }) - if isResourceTimeoutError(err) { - output, err = conn.CreateKey(input) - } - if err != nil { - return fmt.Errorf("error creating KMS External Key: %s", err) + return fmt.Errorf("error creating KMS External Key: %w", err) } - d.SetId(aws.StringValue(output.KeyMetadata.KeyId)) + d.SetId(aws.StringValue(outputRaw.(*kms.CreateKeyOutput).KeyMetadata.KeyId)) if v, ok := d.GetOk("key_material_base64"); ok { - if err := importKmsExternalKeyMaterial(conn, d.Id(), v.(string), d.Get("valid_to").(string)); err != nil { + validTo := d.Get("valid_to").(string) + + if err := importKmsExternalKeyMaterial(conn, d.Id(), v.(string), validTo); err != nil { return fmt.Errorf("error importing KMS External Key (%s) material: %s", d.Id(), err) } - if !d.Get("enabled").(bool) { - if err := updateKmsKeyStatus(conn, d.Id(), false); err != nil { - return fmt.Errorf("error disabling KMS External Key (%s): %s", d.Id(), err) + if _, err := waiter.KeyMaterialImported(conn, d.Id()); err != nil { + return fmt.Errorf("error waiting for KMS External Key (%s) material import: %w", d.Id(), err) + } + + if err := waiter.KeyValidToPropagated(conn, d.Id(), validTo); err != nil { + return fmt.Errorf("error waiting for KMS External Key (%s) valid_to propagation: %w", d.Id(), err) + } + + // The key can only be disabled if key material has been imported, else: + // "KMSInvalidStateException: arn:aws:kms:us-west-2:123456789012:key/47e3edc1-945f-413b-88b1-e7341c2d89f7 is pending import." + if enabled := d.Get("enabled").(bool); !enabled { + if err := updateKmsKeyEnabled(conn, d.Id(), enabled); err != nil { + return err } } } @@ -168,89 +181,32 @@ func resourceAwsKmsExternalKeyRead(d *schema.ResourceData, meta interface{}) err defaultTagsConfig := meta.(*AWSClient).DefaultTagsConfig ignoreTagsConfig := meta.(*AWSClient).IgnoreTagsConfig - input := &kms.DescribeKeyInput{ - KeyId: aws.String(d.Id()), - } - - var output *kms.DescribeKeyOutput - // Retry for KMS eventual consistency on creation - err := resource.Retry(2*time.Minute, func() *resource.RetryError { - var err error - - output, err = conn.DescribeKey(input) - - if d.IsNewResource() && isAWSErr(err, kms.ErrCodeNotFoundException, "") { - return resource.RetryableError(err) - } - - if err != nil { - return resource.NonRetryableError(err) - } - - return nil - }) - - if isResourceTimeoutError(err) { - output, err = conn.DescribeKey(input) - } - - if !d.IsNewResource() && isAWSErr(err, kms.ErrCodeNotFoundException, "") { - log.Printf("[WARN] KMS External Key (%s) not found, removing from state", d.Id()) - d.SetId("") - return nil - } - - if err != nil { - return fmt.Errorf("error describing KMS External Key (%s): %s", d.Id(), err) - } + key, err := findKmsKey(conn, d.Id(), d.IsNewResource()) - if output == nil || output.KeyMetadata == nil { - return fmt.Errorf("error describing KMS External Key (%s): empty response", d.Id()) - } - - metadata := output.KeyMetadata - - if aws.StringValue(metadata.KeyState) == kms.KeyStatePendingDeletion { + if !d.IsNewResource() && tfresource.NotFound(err) { log.Printf("[WARN] KMS External Key (%s) not found, removing from state", d.Id()) d.SetId("") return nil } - getKeyPolicyInput := &kms.GetKeyPolicyInput{ - KeyId: metadata.KeyId, - PolicyName: aws.String("default"), - } - - getKeyPolicyOutput, err := conn.GetKeyPolicy(getKeyPolicyInput) - - if err != nil { - return fmt.Errorf("error getting KMS External Key (%s) policy: %s", d.Id(), err) - } - - if getKeyPolicyOutput == nil { - return fmt.Errorf("error getting KMS External Key (%s) policy: empty response", d.Id()) - } - - policy, err := structure.NormalizeJsonString(aws.StringValue(getKeyPolicyOutput.Policy)) - if err != nil { - return fmt.Errorf("error normalizing KMS External Key (%s) policy: %s", d.Id(), err) + return err } - d.Set("arn", metadata.Arn) - d.Set("description", metadata.Description) - d.Set("enabled", metadata.Enabled) - d.Set("expiration_model", metadata.ExpirationModel) - d.Set("key_state", metadata.KeyState) - d.Set("key_usage", metadata.KeyUsage) - d.Set("policy", policy) - - tags, err := keyvaluetags.KmsListTags(conn, d.Id()) - if err != nil { - return fmt.Errorf("error listing tags for KMS Key (%s): %s", d.Id(), err) + d.Set("arn", key.metadata.Arn) + d.Set("description", key.metadata.Description) + d.Set("enabled", key.metadata.Enabled) + d.Set("expiration_model", key.metadata.ExpirationModel) + d.Set("key_state", key.metadata.KeyState) + d.Set("key_usage", key.metadata.KeyUsage) + d.Set("policy", key.policy) + if key.metadata.ValidTo != nil { + d.Set("valid_to", aws.TimeValue(key.metadata.ValidTo).Format(time.RFC3339)) + } else { + d.Set("valid_to", nil) } - tags = tags.IgnoreAws().IgnoreConfig(ignoreTagsConfig) + tags := key.tags.IgnoreAws().IgnoreConfig(ignoreTagsConfig) //lintignore:AWSR002 if err := d.Set("tags", tags.RemoveDefaultConfig(defaultTagsConfig).Map()); err != nil { @@ -261,63 +217,50 @@ func resourceAwsKmsExternalKeyRead(d *schema.ResourceData, meta interface{}) err return fmt.Errorf("error setting tags_all: %w", err) } - d.Set("valid_to", "") - if metadata.ValidTo != nil { - d.Set("valid_to", aws.TimeValue(metadata.ValidTo).Format(time.RFC3339)) - } - return nil } func resourceAwsKmsExternalKeyUpdate(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).kmsconn - if d.HasChange("enabled") && d.Get("enabled").(bool) && d.Get("key_state") != kms.KeyStatePendingImport { - // Enable before any attributes will be modified - if err := updateKmsKeyStatus(conn, d.Id(), d.Get("enabled").(bool)); err != nil { + if hasChange, enabled, state := d.HasChange("enabled"), d.Get("enabled").(bool), d.Get("key_state").(string); hasChange && enabled && state != kms.KeyStatePendingImport { + // Enable before any attributes are modified. + if err := updateKmsKeyEnabled(conn, d.Id(), enabled); err != nil { return err } } if d.HasChange("description") { - input := &kms.UpdateKeyDescriptionInput{ - Description: aws.String(d.Get("description").(string)), - KeyId: aws.String(d.Id()), - } - - if _, err := conn.UpdateKeyDescription(input); err != nil { - return fmt.Errorf("error updating KMS External Key (%s) description: %s", d.Id(), err) + if err := updateKmsKeyDescription(conn, d.Id(), d.Get("description").(string)); err != nil { + return err } } if d.HasChange("policy") { - policy, err := structure.NormalizeJsonString(d.Get("policy").(string)) - - if err != nil { - return fmt.Errorf("error parsing KMS External Key (%s) policy JSON: %s", d.Id(), err) + if err := updateKmsKeyPolicy(conn, d.Id(), d.Get("policy").(string), d.Get("bypass_policy_lockout_safety_check").(bool)); err != nil { + return err } + } + + if d.HasChange("valid_to") { + validTo := d.Get("valid_to").(string) - input := &kms.PutKeyPolicyInput{ - KeyId: aws.String(d.Id()), - Policy: aws.String(policy), - PolicyName: aws.String("default"), + if err := importKmsExternalKeyMaterial(conn, d.Id(), d.Get("key_material_base64").(string), validTo); err != nil { + return fmt.Errorf("error importing KMS External Key (%s) material: %s", d.Id(), err) } - if _, err := conn.PutKeyPolicy(input); err != nil { - return fmt.Errorf("error updating KMS External Key (%s) policy: %s", d.Id(), err) + if _, err := waiter.KeyMaterialImported(conn, d.Id()); err != nil { + return fmt.Errorf("error waiting for KMS External Key (%s) material import: %w", d.Id(), err) } - } - if d.HasChange("valid_to") { - if err := importKmsExternalKeyMaterial(conn, d.Id(), d.Get("key_material_base64").(string), d.Get("valid_to").(string)); err != nil { - return fmt.Errorf("error importing KMS External Key (%s) material: %s", d.Id(), err) + if err := waiter.KeyValidToPropagated(conn, d.Id(), validTo); err != nil { + return fmt.Errorf("error waiting for KMS External Key (%s) valid_to propagation: %w", d.Id(), err) } } - if d.HasChange("enabled") && !d.Get("enabled").(bool) && d.Get("key_state") != kms.KeyStatePendingImport { - // Only disable when all attributes are modified - // because we cannot modify disabled keys - if err := updateKmsKeyStatus(conn, d.Id(), d.Get("enabled").(bool)); err != nil { + if hasChange, enabled, state := d.HasChange("enabled"), d.Get("enabled").(bool), d.Get("key_state").(string); hasChange && !enabled && state != kms.KeyStatePendingImport { + // Only disable after all attributes have been modified because we cannot modify disabled keys. + if err := updateKmsKeyEnabled(conn, d.Id(), enabled); err != nil { return err } } @@ -326,7 +269,11 @@ func resourceAwsKmsExternalKeyUpdate(d *schema.ResourceData, meta interface{}) e o, n := d.GetChange("tags_all") if err := keyvaluetags.KmsUpdateTags(conn, d.Id(), o, n); err != nil { - return fmt.Errorf("error updating KMS External Key (%s) tags: %s", d.Id(), err) + return fmt.Errorf("error updating KMS External Key (%s) tags: %w", d.Id(), err) + } + + if err := waiter.TagsPropagated(conn, d.Id(), keyvaluetags.New(n)); err != nil { + return fmt.Errorf("error waiting for KMS External Key (%s) tag propagation: %w", d.Id(), err) } } @@ -337,92 +284,73 @@ func resourceAwsKmsExternalKeyDelete(d *schema.ResourceData, meta interface{}) e conn := meta.(*AWSClient).kmsconn input := &kms.ScheduleKeyDeletionInput{ - KeyId: aws.String(d.Id()), - PendingWindowInDays: aws.Int64(int64(d.Get("deletion_window_in_days").(int))), + KeyId: aws.String(d.Id()), } + if v, ok := d.GetOk("deletion_window_in_days"); ok { + input.PendingWindowInDays = aws.Int64(int64(v.(int))) + } + + log.Printf("[DEBUG] Deleting KMS External Key: (%s)", d.Id()) _, err := conn.ScheduleKeyDeletion(input) - if isAWSErr(err, kms.ErrCodeNotFoundException, "") { + if tfawserr.ErrCodeEquals(err, kms.ErrCodeNotFoundException) { return nil } - if err != nil { - return fmt.Errorf("error scheduling deletion for KMS Key (%s): %w", d.Id(), err) - } - - _, err = waiter.KeyStatePendingDeletion(conn, d.Id()) - - if isAWSErr(err, kms.ErrCodeNotFoundException, "") { + if tfawserr.ErrMessageContains(err, kms.ErrCodeInvalidStateException, "is pending deletion") { return nil } if err != nil { - return fmt.Errorf("error waiting for KMS Key (%s) to schedule deletion: %w", d.Id(), err) + return fmt.Errorf("error deleting KMS External Key (%s): %w", d.Id(), err) + } + + if _, err := waiter.KeyDeleted(conn, d.Id()); err != nil { + return fmt.Errorf("error waiting for KMS External Key (%s) to delete: %w", d.Id(), err) } return nil } func importKmsExternalKeyMaterial(conn *kms.KMS, keyID, keyMaterialBase64, validTo string) error { - getParametersForImportInput := &kms.GetParametersForImportInput{ - KeyId: aws.String(keyID), - WrappingAlgorithm: aws.String(kms.AlgorithmSpecRsaesOaepSha256), - WrappingKeySpec: aws.String(kms.WrappingKeySpecRsa2048), - } - - var getParametersForImportOutput *kms.GetParametersForImportOutput - // Handle KMS eventual consistency - err := resource.Retry(1*time.Minute, func() *resource.RetryError { - var err error - - getParametersForImportOutput, err = conn.GetParametersForImport(getParametersForImportInput) - - if isAWSErr(err, kms.ErrCodeNotFoundException, "") { - return resource.RetryableError(err) - } - - if err != nil { - return resource.NonRetryableError(err) - } - - return nil - }) - - if isResourceTimeoutError(err) { - getParametersForImportOutput, err = conn.GetParametersForImport(getParametersForImportInput) - } + // Wait for propagation since KMS is eventually consistent. + outputRaw, err := tfresource.RetryWhenAwsErrCodeEquals(waiter.PropagationTimeout, func() (interface{}, error) { + return conn.GetParametersForImport(&kms.GetParametersForImportInput{ + KeyId: aws.String(keyID), + WrappingAlgorithm: aws.String(kms.AlgorithmSpecRsaesOaepSha256), + WrappingKeySpec: aws.String(kms.WrappingKeySpecRsa2048), + }) + }, kms.ErrCodeNotFoundException) if err != nil { - return fmt.Errorf("error getting parameters for import: %s", err) + return fmt.Errorf("error getting parameters for import: %w", err) } - if getParametersForImportOutput == nil { - return fmt.Errorf("error getting parameters for import: empty response") - } + output := outputRaw.(*kms.GetParametersForImportOutput) keyMaterial, err := base64.StdEncoding.DecodeString(keyMaterialBase64) if err != nil { - return fmt.Errorf("error Base64 decoding key material: %s", err) + return fmt.Errorf("error Base64 decoding key material: %w", err) } - publicKey, err := x509.ParsePKIXPublicKey(getParametersForImportOutput.PublicKey) + publicKey, err := x509.ParsePKIXPublicKey(output.PublicKey) if err != nil { - return fmt.Errorf("error parsing public key: %s", err) + return fmt.Errorf("error parsing public key: %w", err) } encryptedKeyMaterial, err := rsa.EncryptOAEP(sha256.New(), rand.Reader, publicKey.(*rsa.PublicKey), keyMaterial, []byte{}) if err != nil { - return fmt.Errorf("error encrypting key material: %s", err) + return fmt.Errorf("error encrypting key material: %w", err) } - importKeyMaterialInput := &kms.ImportKeyMaterialInput{ + input := &kms.ImportKeyMaterialInput{ EncryptedKeyMaterial: encryptedKeyMaterial, ExpirationModel: aws.String(kms.ExpirationModelTypeKeyMaterialDoesNotExpire), - ImportToken: getParametersForImportOutput.ImportToken, + ImportToken: output.ImportToken, KeyId: aws.String(keyID), } @@ -430,34 +358,20 @@ func importKmsExternalKeyMaterial(conn *kms.KMS, keyID, keyMaterialBase64, valid t, err := time.Parse(time.RFC3339, validTo) if err != nil { - return fmt.Errorf("error parsing valid to timestamp: %s", err) + return fmt.Errorf("error parsing valid_to timestamp: %w", err) } - importKeyMaterialInput.ExpirationModel = aws.String(kms.ExpirationModelTypeKeyMaterialExpires) - importKeyMaterialInput.ValidTo = aws.Time(t) + input.ExpirationModel = aws.String(kms.ExpirationModelTypeKeyMaterialExpires) + input.ValidTo = aws.Time(t) } - // Handle KMS eventual consistency - err = resource.Retry(1*time.Minute, func() *resource.RetryError { - _, err := conn.ImportKeyMaterial(importKeyMaterialInput) - - if isAWSErr(err, kms.ErrCodeNotFoundException, "") { - return resource.RetryableError(err) - } - - if err != nil { - return resource.NonRetryableError(err) - } - - return nil - }) - - if isResourceTimeoutError(err) { - _, err = conn.ImportKeyMaterial(importKeyMaterialInput) - } + // Wait for propagation since KMS is eventually consistent. + _, err = tfresource.RetryWhenAwsErrCodeEquals(waiter.PropagationTimeout, func() (interface{}, error) { + return conn.ImportKeyMaterial(input) + }, kms.ErrCodeNotFoundException) if err != nil { - return fmt.Errorf("error importing key material: %s", err) + return fmt.Errorf("error importing key material: %w", err) } return nil diff --git a/aws/resource_aws_kms_external_key_test.go b/aws/resource_aws_kms_external_key_test.go index dd5fda2094d..92e0f78762a 100644 --- a/aws/resource_aws_kms_external_key_test.go +++ b/aws/resource_aws_kms_external_key_test.go @@ -8,14 +8,18 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/kms" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" awspolicy "github.com/jen20/awspolicyequivalence" + tfkms "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/kms" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/kms/finder" "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/kms/waiter" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" ) func TestAccAWSKmsExternalKey_basic(t *testing.T) { - var key1 kms.KeyMetadata + var key kms.KeyMetadata resourceName := "aws_kms_external_key.test" resource.ParallelTest(t, resource.TestCase{ @@ -27,8 +31,9 @@ func TestAccAWSKmsExternalKey_basic(t *testing.T) { { Config: testAccAWSKmsExternalKeyConfig(), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSKmsExternalKeyExists(resourceName, &key1), + testAccCheckAWSKmsExternalKeyExists(resourceName, &key), testAccMatchResourceAttrRegionalARN(resourceName, "arn", "kms", regexp.MustCompile(`key/.+`)), + resource.TestCheckResourceAttr(resourceName, "bypass_policy_lockout_safety_check", "false"), resource.TestCheckResourceAttr(resourceName, "deletion_window_in_days", "30"), resource.TestCheckResourceAttr(resourceName, "enabled", "false"), resource.TestCheckResourceAttr(resourceName, "expiration_model", ""), @@ -45,6 +50,7 @@ func TestAccAWSKmsExternalKey_basic(t *testing.T) { ImportState: true, ImportStateVerify: true, ImportStateVerifyIgnore: []string{ + "bypass_policy_lockout_safety_check", "deletion_window_in_days", "key_material_base64", }, @@ -54,7 +60,7 @@ func TestAccAWSKmsExternalKey_basic(t *testing.T) { } func TestAccAWSKmsExternalKey_disappears(t *testing.T) { - var key1 kms.KeyMetadata + var key kms.KeyMetadata resourceName := "aws_kms_external_key.test" resource.ParallelTest(t, resource.TestCase{ @@ -66,8 +72,8 @@ func TestAccAWSKmsExternalKey_disappears(t *testing.T) { { Config: testAccAWSKmsExternalKeyConfig(), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSKmsExternalKeyExists(resourceName, &key1), - testAccCheckAWSKmsExternalKeyDisappears(&key1), + testAccCheckAWSKmsExternalKeyExists(resourceName, &key), + testAccCheckResourceDisappears(testAccProvider, resourceAwsKmsExternalKey(), resourceName), ), ExpectNonEmptyPlan: true, }, @@ -77,6 +83,7 @@ func TestAccAWSKmsExternalKey_disappears(t *testing.T) { func TestAccAWSKmsExternalKey_DeletionWindowInDays(t *testing.T) { var key1, key2 kms.KeyMetadata + rName := acctest.RandomWithPrefix("tf-acc-test") resourceName := "aws_kms_external_key.test" resource.ParallelTest(t, resource.TestCase{ @@ -86,7 +93,7 @@ func TestAccAWSKmsExternalKey_DeletionWindowInDays(t *testing.T) { CheckDestroy: testAccCheckAWSKmsExternalKeyDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSKmsExternalKeyConfigDeletionWindowInDays(8), + Config: testAccAWSKmsExternalKeyConfigDeletionWindowInDays(rName, 8), Check: resource.ComposeTestCheckFunc( testAccCheckAWSKmsExternalKeyExists(resourceName, &key1), resource.TestCheckResourceAttr(resourceName, "deletion_window_in_days", "8"), @@ -97,12 +104,13 @@ func TestAccAWSKmsExternalKey_DeletionWindowInDays(t *testing.T) { ImportState: true, ImportStateVerify: true, ImportStateVerifyIgnore: []string{ + "bypass_policy_lockout_safety_check", "deletion_window_in_days", "key_material_base64", }, }, { - Config: testAccAWSKmsExternalKeyConfigDeletionWindowInDays(7), + Config: testAccAWSKmsExternalKeyConfigDeletionWindowInDays(rName, 7), Check: resource.ComposeTestCheckFunc( testAccCheckAWSKmsExternalKeyExists(resourceName, &key2), testAccCheckAWSKmsExternalKeyNotRecreated(&key1, &key2), @@ -115,6 +123,7 @@ func TestAccAWSKmsExternalKey_DeletionWindowInDays(t *testing.T) { func TestAccAWSKmsExternalKey_Description(t *testing.T) { var key1, key2 kms.KeyMetadata + rName := acctest.RandomWithPrefix("tf-acc-test") resourceName := "aws_kms_external_key.test" resource.ParallelTest(t, resource.TestCase{ @@ -124,10 +133,10 @@ func TestAccAWSKmsExternalKey_Description(t *testing.T) { CheckDestroy: testAccCheckAWSKmsExternalKeyDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSKmsExternalKeyConfigDescription("description1"), + Config: testAccAWSKmsExternalKeyConfigDescription(rName + "-1"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSKmsExternalKeyExists(resourceName, &key1), - resource.TestCheckResourceAttr(resourceName, "description", "description1"), + resource.TestCheckResourceAttr(resourceName, "description", rName+"-1"), ), }, { @@ -135,16 +144,17 @@ func TestAccAWSKmsExternalKey_Description(t *testing.T) { ImportState: true, ImportStateVerify: true, ImportStateVerifyIgnore: []string{ + "bypass_policy_lockout_safety_check", "deletion_window_in_days", "key_material_base64", }, }, { - Config: testAccAWSKmsExternalKeyConfigDescription("description2"), + Config: testAccAWSKmsExternalKeyConfigDescription(rName + "-2"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSKmsExternalKeyExists(resourceName, &key2), testAccCheckAWSKmsExternalKeyNotRecreated(&key1, &key2), - resource.TestCheckResourceAttr(resourceName, "description", "description2"), + resource.TestCheckResourceAttr(resourceName, "description", rName+"-2"), ), }, }, @@ -153,6 +163,7 @@ func TestAccAWSKmsExternalKey_Description(t *testing.T) { func TestAccAWSKmsExternalKey_Enabled(t *testing.T) { var key1, key2, key3 kms.KeyMetadata + rName := acctest.RandomWithPrefix("tf-acc-test") resourceName := "aws_kms_external_key.test" resource.ParallelTest(t, resource.TestCase{ @@ -162,7 +173,7 @@ func TestAccAWSKmsExternalKey_Enabled(t *testing.T) { CheckDestroy: testAccCheckAWSKmsExternalKeyDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSKmsExternalKeyConfigEnabled(false), + Config: testAccAWSKmsExternalKeyConfigEnabled(rName, false), Check: resource.ComposeTestCheckFunc( testAccCheckAWSKmsExternalKeyExists(resourceName, &key1), resource.TestCheckResourceAttr(resourceName, "enabled", "false"), @@ -173,12 +184,13 @@ func TestAccAWSKmsExternalKey_Enabled(t *testing.T) { ImportState: true, ImportStateVerify: true, ImportStateVerifyIgnore: []string{ + "bypass_policy_lockout_safety_check", "deletion_window_in_days", "key_material_base64", }, }, { - Config: testAccAWSKmsExternalKeyConfigEnabled(true), + Config: testAccAWSKmsExternalKeyConfigEnabled(rName, true), Check: resource.ComposeTestCheckFunc( testAccCheckAWSKmsExternalKeyExists(resourceName, &key2), testAccCheckAWSKmsExternalKeyNotRecreated(&key1, &key2), @@ -186,7 +198,7 @@ func TestAccAWSKmsExternalKey_Enabled(t *testing.T) { ), }, { - Config: testAccAWSKmsExternalKeyConfigEnabled(false), + Config: testAccAWSKmsExternalKeyConfigEnabled(rName, false), Check: resource.ComposeTestCheckFunc( testAccCheckAWSKmsExternalKeyExists(resourceName, &key3), testAccCheckAWSKmsExternalKeyNotRecreated(&key2, &key3), @@ -199,6 +211,7 @@ func TestAccAWSKmsExternalKey_Enabled(t *testing.T) { func TestAccAWSKmsExternalKey_KeyMaterialBase64(t *testing.T) { var key1, key2 kms.KeyMetadata + rName := acctest.RandomWithPrefix("tf-acc-test") resourceName := "aws_kms_external_key.test" resource.ParallelTest(t, resource.TestCase{ @@ -209,7 +222,7 @@ func TestAccAWSKmsExternalKey_KeyMaterialBase64(t *testing.T) { Steps: []resource.TestStep{ { // ACCEPTANCE TESTING ONLY -- NEVER EXPOSE YOUR KEY MATERIAL - Config: testAccAWSKmsExternalKeyConfigKeyMaterialBase64("Wblj06fduthWggmsT0cLVoIMOkeLbc2kVfMud77i/JY="), + Config: testAccAWSKmsExternalKeyConfigKeyMaterialBase64(rName, "Wblj06fduthWggmsT0cLVoIMOkeLbc2kVfMud77i/JY="), Check: resource.ComposeTestCheckFunc( testAccCheckAWSKmsExternalKeyExists(resourceName, &key1), resource.TestCheckResourceAttr(resourceName, "key_material_base64", "Wblj06fduthWggmsT0cLVoIMOkeLbc2kVfMud77i/JY="), @@ -220,13 +233,14 @@ func TestAccAWSKmsExternalKey_KeyMaterialBase64(t *testing.T) { ImportState: true, ImportStateVerify: true, ImportStateVerifyIgnore: []string{ + "bypass_policy_lockout_safety_check", "deletion_window_in_days", "key_material_base64", }, }, { // ACCEPTANCE TESTING ONLY -- NEVER EXPOSE YOUR KEY MATERIAL - Config: testAccAWSKmsExternalKeyConfigKeyMaterialBase64("O1zsg06cKRCsZnoT5oizMlwHEtnk0HoOmBLkFtwh2Vw="), + Config: testAccAWSKmsExternalKeyConfigKeyMaterialBase64(rName, "O1zsg06cKRCsZnoT5oizMlwHEtnk0HoOmBLkFtwh2Vw="), Check: resource.ComposeTestCheckFunc( testAccCheckAWSKmsExternalKeyExists(resourceName, &key2), testAccCheckAWSKmsExternalKeyRecreated(&key1, &key2), @@ -239,6 +253,7 @@ func TestAccAWSKmsExternalKey_KeyMaterialBase64(t *testing.T) { func TestAccAWSKmsExternalKey_Policy(t *testing.T) { var key1, key2 kms.KeyMetadata + rName := acctest.RandomWithPrefix("tf-acc-test") policy1 := `{"Version":"2012-10-17","Id":"kms-tf-1","Statement":[{"Sid":"Enable IAM User Permissions 1","Effect":"Allow","Principal":{"AWS":"*"},"Action":"kms:*","Resource":"*"}]}` policy2 := `{"Version":"2012-10-17","Id":"kms-tf-1","Statement":[{"Sid":"Enable IAM User Permissions 2","Effect":"Allow","Principal":{"AWS":"*"},"Action":"kms:*","Resource":"*"}]}` resourceName := "aws_kms_external_key.test" @@ -250,7 +265,7 @@ func TestAccAWSKmsExternalKey_Policy(t *testing.T) { CheckDestroy: testAccCheckAWSKmsExternalKeyDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSKmsExternalKeyConfigPolicy(policy1), + Config: testAccAWSKmsExternalKeyConfigPolicy(rName, policy1), Check: resource.ComposeTestCheckFunc( testAccCheckAWSKmsExternalKeyExists(resourceName, &key1), testAccCheckAWSKmsExternalKeyHasPolicy(resourceName, policy1), @@ -261,12 +276,13 @@ func TestAccAWSKmsExternalKey_Policy(t *testing.T) { ImportState: true, ImportStateVerify: true, ImportStateVerifyIgnore: []string{ + "bypass_policy_lockout_safety_check", "deletion_window_in_days", "key_material_base64", }, }, { - Config: testAccAWSKmsExternalKeyConfigPolicy(policy2), + Config: testAccAWSKmsExternalKeyConfigPolicy(rName, policy2), Check: resource.ComposeTestCheckFunc( testAccCheckAWSKmsExternalKeyExists(resourceName, &key2), testAccCheckAWSKmsExternalKeyNotRecreated(&key1, &key2), @@ -277,8 +293,43 @@ func TestAccAWSKmsExternalKey_Policy(t *testing.T) { }) } +func TestAccAWSKmsExternalKey_PolicyBypass(t *testing.T) { + var key kms.KeyMetadata + rName := acctest.RandomWithPrefix("tf-acc-test") + policy := `{"Version":"2012-10-17","Id":"kms-tf-1","Statement":[{"Sid":"Enable IAM User Permissions 1","Effect":"Allow","Principal":{"AWS":"*"},"Action":"kms:*","Resource":"*"}]}` + resourceName := "aws_kms_external_key.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + ErrorCheck: testAccErrorCheck(t, kms.EndpointsID), + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSKmsExternalKeyDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSKmsExternalKeyConfigPolicyBypass(rName, policy), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSKmsExternalKeyExists(resourceName, &key), + testAccCheckAWSKmsExternalKeyHasPolicy(resourceName, policy), + resource.TestCheckResourceAttr(resourceName, "bypass_policy_lockout_safety_check", "true"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "bypass_policy_lockout_safety_check", + "deletion_window_in_days", + "key_material_base64", + }, + }, + }, + }) +} + func TestAccAWSKmsExternalKey_Tags(t *testing.T) { var key1, key2, key3 kms.KeyMetadata + rName := acctest.RandomWithPrefix("tf-acc-test") resourceName := "aws_kms_external_key.test" resource.ParallelTest(t, resource.TestCase{ @@ -288,7 +339,7 @@ func TestAccAWSKmsExternalKey_Tags(t *testing.T) { CheckDestroy: testAccCheckAWSKmsExternalKeyDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSKmsExternalKeyConfigTags1("value1"), + Config: testAccAWSKmsExternalKeyConfigTags1(rName, "key1", "value1"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSKmsExternalKeyExists(resourceName, &key1), resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), @@ -300,12 +351,13 @@ func TestAccAWSKmsExternalKey_Tags(t *testing.T) { ImportState: true, ImportStateVerify: true, ImportStateVerifyIgnore: []string{ + "bypass_policy_lockout_safety_check", "deletion_window_in_days", "key_material_base64", }, }, { - Config: testAccAWSKmsExternalKeyConfigTags2("value1updated", "value2"), + Config: testAccAWSKmsExternalKeyConfigTags2(rName, "key1", "value1updated", "key2", "value2"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSKmsExternalKeyExists(resourceName, &key2), testAccCheckAWSKmsExternalKeyNotRecreated(&key1, &key2), @@ -315,12 +367,12 @@ func TestAccAWSKmsExternalKey_Tags(t *testing.T) { ), }, { - Config: testAccAWSKmsExternalKeyConfigTags1("value1updated"), + Config: testAccAWSKmsExternalKeyConfigTags1(rName, "key2", "value2"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSKmsExternalKeyExists(resourceName, &key3), testAccCheckAWSKmsExternalKeyNotRecreated(&key2, &key3), resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), - resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1updated"), + resource.TestCheckResourceAttr(resourceName, "tags.key2", "value2"), ), }, }, @@ -329,6 +381,7 @@ func TestAccAWSKmsExternalKey_Tags(t *testing.T) { func TestAccAWSKmsExternalKey_ValidTo(t *testing.T) { var key1, key2, key3, key4 kms.KeyMetadata + rName := acctest.RandomWithPrefix("tf-acc-test") resourceName := "aws_kms_external_key.test" validTo1 := time.Now().UTC().Add(1 * time.Hour).Format(time.RFC3339) validTo2 := time.Now().UTC().Add(2 * time.Hour).Format(time.RFC3339) @@ -340,7 +393,7 @@ func TestAccAWSKmsExternalKey_ValidTo(t *testing.T) { CheckDestroy: testAccCheckAWSKmsExternalKeyDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSKmsExternalKeyConfigValidTo(validTo1), + Config: testAccAWSKmsExternalKeyConfigValidTo(rName, validTo1), Check: resource.ComposeTestCheckFunc( testAccCheckAWSKmsExternalKeyExists(resourceName, &key1), resource.TestCheckResourceAttr(resourceName, "expiration_model", "KEY_MATERIAL_EXPIRES"), @@ -352,12 +405,13 @@ func TestAccAWSKmsExternalKey_ValidTo(t *testing.T) { ImportState: true, ImportStateVerify: true, ImportStateVerifyIgnore: []string{ + "bypass_policy_lockout_safety_check", "deletion_window_in_days", "key_material_base64", }, }, { - Config: testAccAWSKmsExternalKeyConfigEnabled(true), + Config: testAccAWSKmsExternalKeyConfigEnabled(rName, true), Check: resource.ComposeTestCheckFunc( testAccCheckAWSKmsExternalKeyExists(resourceName, &key2), testAccCheckAWSKmsExternalKeyNotRecreated(&key1, &key2), @@ -366,7 +420,7 @@ func TestAccAWSKmsExternalKey_ValidTo(t *testing.T) { ), }, { - Config: testAccAWSKmsExternalKeyConfigValidTo(validTo1), + Config: testAccAWSKmsExternalKeyConfigValidTo(rName, validTo1), Check: resource.ComposeTestCheckFunc( testAccCheckAWSKmsExternalKeyExists(resourceName, &key3), testAccCheckAWSKmsExternalKeyNotRecreated(&key2, &key3), @@ -375,7 +429,7 @@ func TestAccAWSKmsExternalKey_ValidTo(t *testing.T) { ), }, { - Config: testAccAWSKmsExternalKeyConfigValidTo(validTo2), + Config: testAccAWSKmsExternalKeyConfigValidTo(rName, validTo2), Check: resource.ComposeTestCheckFunc( testAccCheckAWSKmsExternalKeyExists(resourceName, &key4), testAccCheckAWSKmsExternalKeyNotRecreated(&key3, &key4), @@ -395,20 +449,18 @@ func testAccCheckAWSKmsExternalKeyHasPolicy(name string, expectedPolicyText stri } if rs.Primary.ID == "" { - return fmt.Errorf("No KMS Key ID is set") + return fmt.Errorf("No KMS External Key ID is set") } conn := testAccProvider.Meta().(*AWSClient).kmsconn - out, err := conn.GetKeyPolicy(&kms.GetKeyPolicyInput{ - KeyId: aws.String(rs.Primary.ID), - PolicyName: aws.String("default"), - }) + output, err := finder.KeyPolicyByKeyIDAndPolicyName(conn, rs.Primary.ID, tfkms.PolicyNameDefault) + if err != nil { return err } - actualPolicyText := *out.Policy + actualPolicyText := aws.StringValue(output) equivalent, err := awspolicy.PoliciesAreEquivalent(actualPolicyText, expectedPolicyText) if err != nil { @@ -431,19 +483,17 @@ func testAccCheckAWSKmsExternalKeyDestroy(s *terraform.State) error { continue } - out, err := conn.DescribeKey(&kms.DescribeKeyInput{ - KeyId: aws.String(rs.Primary.ID), - }) + _, err := finder.KeyByID(conn, rs.Primary.ID) - if err != nil { - return err + if tfresource.NotFound(err) { + continue } - if aws.StringValue(out.KeyMetadata.KeyState) == kms.KeyStatePendingDeletion { - continue + if err != nil { + return err } - return fmt.Errorf("KMS key still exists:\n%#v", out.KeyMetadata) + return fmt.Errorf("KMS External Key %s still exists", rs.Primary.ID) } return nil @@ -457,45 +507,22 @@ func testAccCheckAWSKmsExternalKeyExists(name string, key *kms.KeyMetadata) reso } if rs.Primary.ID == "" { - return fmt.Errorf("No KMS Key ID is set") + return fmt.Errorf("No KMS External Key ID is set") } conn := testAccProvider.Meta().(*AWSClient).kmsconn - o, err := retryOnAwsCode("NotFoundException", func() (interface{}, error) { - return conn.DescribeKey(&kms.DescribeKeyInput{ - KeyId: aws.String(rs.Primary.ID), - }) + outputRaw, err := tfresource.RetryWhenNotFound(waiter.PropagationTimeout, func() (interface{}, error) { + return finder.KeyByID(conn, rs.Primary.ID) }) - if err != nil { - return err - } - out := o.(*kms.DescribeKeyOutput) - - *key = *out.KeyMetadata - - return nil - } -} - -func testAccCheckAWSKmsExternalKeyDisappears(key *kms.KeyMetadata) resource.TestCheckFunc { - return func(s *terraform.State) error { - conn := testAccProvider.Meta().(*AWSClient).kmsconn - - input := &kms.ScheduleKeyDeletionInput{ - KeyId: key.KeyId, - PendingWindowInDays: aws.Int64(int64(7)), - } - - _, err := conn.ScheduleKeyDeletion(input) if err != nil { return err } - _, err = waiter.KeyStatePendingDeletion(conn, aws.StringValue(key.KeyId)) + *key = *(outputRaw.(*kms.KeyMetadata)) - return err + return nil } } @@ -525,87 +552,109 @@ resource "aws_kms_external_key" "test" {} ` } -func testAccAWSKmsExternalKeyConfigDeletionWindowInDays(deletionWindowInDays int) string { +func testAccAWSKmsExternalKeyConfigDeletionWindowInDays(rName string, deletionWindowInDays int) string { return fmt.Sprintf(` resource "aws_kms_external_key" "test" { - deletion_window_in_days = %[1]d + description = %[1]q + deletion_window_in_days = %[2]d } -`, deletionWindowInDays) +`, rName, deletionWindowInDays) } -func testAccAWSKmsExternalKeyConfigDescription(description string) string { +func testAccAWSKmsExternalKeyConfigDescription(rName string) string { return fmt.Sprintf(` resource "aws_kms_external_key" "test" { description = %[1]q deletion_window_in_days = 7 } -`, description) +`, rName) } -func testAccAWSKmsExternalKeyConfigEnabled(enabled bool) string { +func testAccAWSKmsExternalKeyConfigEnabled(rName string, enabled bool) string { return fmt.Sprintf(` # ACCEPTANCE TESTING ONLY -- NEVER EXPOSE YOUR KEY MATERIAL resource "aws_kms_external_key" "test" { + description = %[1]q deletion_window_in_days = 7 - enabled = %[1]t + enabled = %[2]t key_material_base64 = "Wblj06fduthWggmsT0cLVoIMOkeLbc2kVfMud77i/JY=" } -`, enabled) +`, rName, enabled) } -func testAccAWSKmsExternalKeyConfigKeyMaterialBase64(keyMaterialBase64 string) string { +func testAccAWSKmsExternalKeyConfigKeyMaterialBase64(rName, keyMaterialBase64 string) string { return fmt.Sprintf(` resource "aws_kms_external_key" "test" { + description = %[1]q deletion_window_in_days = 7 - key_material_base64 = %[1]q + key_material_base64 = %[2]q } -`, keyMaterialBase64) +`, rName, keyMaterialBase64) } -func testAccAWSKmsExternalKeyConfigPolicy(policy string) string { +func testAccAWSKmsExternalKeyConfigPolicy(rName, policy string) string { return fmt.Sprintf(` resource "aws_kms_external_key" "test" { + description = %[1]q deletion_window_in_days = 7 policy = < 0 { - req.Tags = tags.IgnoreAws().KmsTags() + input.Tags = tags.IgnoreAws().KmsTags() } - var resp *kms.CreateKeyOutput // AWS requires any principal in the policy to exist before the key is created. // The KMS service's awareness of principals is limited by "eventual consistency". // They acknowledge this here: // http://docs.aws.amazon.com/kms/latest/APIReference/API_CreateKey.html - err := resource.Retry(iamwaiter.PropagationTimeout, func() *resource.RetryError { - var err error - resp, err = conn.CreateKey(req) - if isAWSErr(err, kms.ErrCodeMalformedPolicyDocumentException, "") { - return resource.RetryableError(err) - } - if err != nil { - return resource.NonRetryableError(err) - } - return nil + log.Printf("[DEBUG] Creating KMS Key: %s", input) + + outputRaw, err := waiter.IAMPropagation(func() (interface{}, error) { + return conn.CreateKey(input) }) - if isResourceTimeoutError(err) { - resp, err = conn.CreateKey(req) - } + if err != nil { - return err + return fmt.Errorf("error creating KMS Key: %w", err) } - d.SetId(aws.StringValue(resp.KeyMetadata.KeyId)) - d.Set("key_id", resp.KeyMetadata.KeyId) + d.SetId(aws.StringValue(outputRaw.(*kms.CreateKeyOutput).KeyMetadata.KeyId)) + d.Set("key_id", d.Id()) if enableKeyRotation := d.Get("enable_key_rotation").(bool); enableKeyRotation { - if err := updateKmsKeyRotationStatus(conn, d); err != nil { + if err := updateKmsKeyRotationEnabled(conn, d.Id(), enableKeyRotation); err != nil { return err } } if enabled := d.Get("is_enabled").(bool); !enabled { - if err := updateKmsKeyStatus(conn, d.Id(), enabled); err != nil { + if err := updateKmsKeyEnabled(conn, d.Id(), enabled); err != nil { return err } } + // Wait for propagation since KMS is eventually consistent. + if v, ok := d.GetOk("policy"); ok { + if err := waiter.KeyPolicyPropagated(conn, d.Id(), v.(string)); err != nil { + return fmt.Errorf("error waiting for KMS Key (%s) policy propagation: %w", d.Id(), err) + } + } + + if len(tags) > 0 { + if err := waiter.TagsPropagated(conn, d.Id(), tags); err != nil { + return fmt.Errorf("error waiting for KMS Key (%s) tag propagation: %w", d.Id(), err) + } + } + return resourceAwsKmsKeyRead(d, meta) } @@ -152,92 +176,28 @@ func resourceAwsKmsKeyRead(d *schema.ResourceData, meta interface{}) error { defaultTagsConfig := meta.(*AWSClient).DefaultTagsConfig ignoreTagsConfig := meta.(*AWSClient).IgnoreTagsConfig - req := &kms.DescribeKeyInput{ - KeyId: aws.String(d.Id()), - } - - var resp *kms.DescribeKeyOutput - var err error - if d.IsNewResource() { - var out interface{} - out, err = retryOnAwsCode(kms.ErrCodeNotFoundException, func() (interface{}, error) { - return conn.DescribeKey(req) - }) - resp, _ = out.(*kms.DescribeKeyOutput) - } else { - resp, err = conn.DescribeKey(req) - } - if err != nil { - return err - } - metadata := resp.KeyMetadata + key, err := findKmsKey(conn, d.Id(), d.IsNewResource()) - if aws.StringValue(metadata.KeyState) == kms.KeyStatePendingDeletion { - log.Printf("[WARN] Removing KMS key %s because it's already gone", d.Id()) + if !d.IsNewResource() && tfresource.NotFound(err) { + log.Printf("[WARN] KMS Key (%s) not found, removing from state", d.Id()) d.SetId("") return nil } - d.Set("arn", metadata.Arn) - d.Set("key_id", metadata.KeyId) - d.Set("description", metadata.Description) - d.Set("key_usage", metadata.KeyUsage) - d.Set("customer_master_key_spec", metadata.CustomerMasterKeySpec) - d.Set("is_enabled", metadata.Enabled) - - pOut, err := retryOnAwsCode(kms.ErrCodeNotFoundException, func() (interface{}, error) { - return conn.GetKeyPolicy(&kms.GetKeyPolicyInput{ - KeyId: aws.String(d.Id()), - PolicyName: aws.String("default"), - }) - }) if err != nil { return err } - p := pOut.(*kms.GetKeyPolicyOutput) - policy, err := structure.NormalizeJsonString(*p.Policy) - if err != nil { - return fmt.Errorf("policy contains an invalid JSON: %w", err) - } - d.Set("policy", policy) - - out, err := retryOnAwsCode(kms.ErrCodeNotFoundException, func() (interface{}, error) { - return conn.GetKeyRotationStatus(&kms.GetKeyRotationStatusInput{ - KeyId: aws.String(d.Id()), - }) - }) - if err != nil { - return err - } - krs, _ := out.(*kms.GetKeyRotationStatusOutput) - d.Set("enable_key_rotation", krs.KeyRotationEnabled) - - var tags keyvaluetags.KeyValueTags - err = resource.Retry(2*time.Minute, func() *resource.RetryError { - var err error - tags, err = keyvaluetags.KmsListTags(conn, d.Id()) - - if d.IsNewResource() && isAWSErr(err, kms.ErrCodeNotFoundException, "") { - return resource.RetryableError(err) - } - - if err != nil { - return resource.NonRetryableError(err) - } - - return nil - }) - - if isResourceTimeoutError(err) { - tags, err = keyvaluetags.KmsListTags(conn, d.Id()) - } - - if err != nil { - return fmt.Errorf("error listing tags for KMS Key (%s): %w", d.Id(), err) - } + d.Set("arn", key.metadata.Arn) + d.Set("customer_master_key_spec", key.metadata.CustomerMasterKeySpec) + d.Set("description", key.metadata.Description) + d.Set("enable_key_rotation", key.rotation) + d.Set("is_enabled", key.metadata.Enabled) + d.Set("key_id", key.metadata.KeyId) + d.Set("key_usage", key.metadata.KeyUsage) + d.Set("policy", key.policy) - tags = tags.IgnoreAws().IgnoreConfig(ignoreTagsConfig) + tags := key.tags.IgnoreAws().IgnoreConfig(ignoreTagsConfig) //lintignore:AWSR002 if err := d.Set("tags", tags.RemoveDefaultConfig(defaultTagsConfig).Map()); err != nil { @@ -254,34 +214,34 @@ func resourceAwsKmsKeyRead(d *schema.ResourceData, meta interface{}) error { func resourceAwsKmsKeyUpdate(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).kmsconn - if d.HasChange("is_enabled") && d.Get("is_enabled").(bool) { - // Enable before any attributes will be modified - if err := updateKmsKeyStatus(conn, d.Id(), d.Get("is_enabled").(bool)); err != nil { + if hasChange, enabled := d.HasChange("is_enabled"), d.Get("is_enabled").(bool); hasChange && enabled { + // Enable before any attributes are modified. + if err := updateKmsKeyEnabled(conn, d.Id(), enabled); err != nil { return err } } - if d.HasChange("enable_key_rotation") { - if err := updateKmsKeyRotationStatus(conn, d); err != nil { + if hasChange, enableKeyRotation := d.HasChange("enable_key_rotation"), d.Get("enable_key_rotation").(bool); hasChange { + if err := updateKmsKeyRotationEnabled(conn, d.Id(), enableKeyRotation); err != nil { return err } } if d.HasChange("description") { - if err := resourceAwsKmsKeyDescriptionUpdate(conn, d); err != nil { + if err := updateKmsKeyDescription(conn, d.Id(), d.Get("description").(string)); err != nil { return err } } + if d.HasChange("policy") { - if err := resourceAwsKmsKeyPolicyUpdate(conn, d); err != nil { + if err := updateKmsKeyPolicy(conn, d.Id(), d.Get("policy").(string), d.Get("bypass_policy_lockout_safety_check").(bool)); err != nil { return err } } - if d.HasChange("is_enabled") && !d.Get("is_enabled").(bool) { - // Only disable when all attributes are modified - // because we cannot modify disabled keys - if err := updateKmsKeyStatus(conn, d.Id(), d.Get("is_enabled").(bool)); err != nil { + if hasChange, enabled := d.HasChange("is_enabled"), d.Get("is_enabled").(bool); hasChange && !enabled { + // Only disable after all attributes have been modified because we cannot modify disabled keys. + if err := updateKmsKeyEnabled(conn, d.Id(), enabled); err != nil { return err } } @@ -292,207 +252,232 @@ func resourceAwsKmsKeyUpdate(d *schema.ResourceData, meta interface{}) error { if err := keyvaluetags.KmsUpdateTags(conn, d.Id(), o, n); err != nil { return fmt.Errorf("error updating KMS Key (%s) tags: %w", d.Id(), err) } + + if err := waiter.TagsPropagated(conn, d.Id(), keyvaluetags.New(n)); err != nil { + return fmt.Errorf("error waiting for KMS Key (%s) tag propagation: %w", d.Id(), err) + } } return resourceAwsKmsKeyRead(d, meta) } -func resourceAwsKmsKeyDescriptionUpdate(conn *kms.KMS, d *schema.ResourceData) error { - description := d.Get("description").(string) - keyId := d.Get("key_id").(string) +func resourceAwsKmsKeyDelete(d *schema.ResourceData, meta interface{}) error { + conn := meta.(*AWSClient).kmsconn + + input := &kms.ScheduleKeyDeletionInput{ + KeyId: aws.String(d.Id()), + } - log.Printf("[DEBUG] KMS key: %s, update description: %s", keyId, description) + if v, ok := d.GetOk("deletion_window_in_days"); ok { + input.PendingWindowInDays = aws.Int64(int64(v.(int))) + } - req := &kms.UpdateKeyDescriptionInput{ - Description: aws.String(description), - KeyId: aws.String(keyId), + log.Printf("[DEBUG] Deleting KMS Key: (%s)", d.Id()) + _, err := conn.ScheduleKeyDeletion(input) + + if tfawserr.ErrCodeEquals(err, kms.ErrCodeNotFoundException) { + return nil } - _, err := conn.UpdateKeyDescription(req) - return err -} + if tfawserr.ErrMessageContains(err, kms.ErrCodeInvalidStateException, "is pending deletion") { + return nil + } -func resourceAwsKmsKeyPolicyUpdate(conn *kms.KMS, d *schema.ResourceData) error { - policy, err := structure.NormalizeJsonString(d.Get("policy").(string)) if err != nil { - return fmt.Errorf("policy contains an invalid JSON: %w", err) + return fmt.Errorf("error deleting KMS Key (%s): %w", d.Id(), err) } - keyId := d.Get("key_id").(string) - - log.Printf("[DEBUG] KMS key: %s, update policy: %s", keyId, policy) - req := &kms.PutKeyPolicyInput{ - KeyId: aws.String(keyId), - Policy: aws.String(policy), - PolicyName: aws.String("default"), + if _, err := waiter.KeyDeleted(conn, d.Id()); err != nil { + return fmt.Errorf("error waiting for KMS Key (%s) to delete: %w", d.Id(), err) } - _, err = conn.PutKeyPolicy(req) - return err + return nil } -func updateKmsKeyStatus(conn *kms.KMS, id string, shouldBeEnabled bool) error { - var err error +type kmsKey struct { + metadata *kms.KeyMetadata + policy string + rotation *bool + tags keyvaluetags.KeyValueTags +} - if shouldBeEnabled { - log.Printf("[DEBUG] Enabling KMS key %q", id) - _, err = conn.EnableKey(&kms.EnableKeyInput{ - KeyId: aws.String(id), - }) - } else { - log.Printf("[DEBUG] Disabling KMS key %q", id) - _, err = conn.DisableKey(&kms.DisableKeyInput{ - KeyId: aws.String(id), - }) - } +func findKmsKey(conn *kms.KMS, keyID string, isNewResource bool) (*kmsKey, error) { + // Wait for propagation since KMS is eventually consistent. + outputRaw, err := tfresource.RetryWhenNewResourceNotFound(waiter.PropagationTimeout, func() (interface{}, error) { + var err error + var key kmsKey + + key.metadata, err = finder.KeyByID(conn, keyID) + + if err != nil { + return nil, fmt.Errorf("error reading KMS Key (%s): %w", keyID, err) + } + + policy, err := finder.KeyPolicyByKeyIDAndPolicyName(conn, keyID, tfkms.PolicyNameDefault) + + if err != nil { + return nil, fmt.Errorf("error reading KMS Key (%s) policy: %w", keyID, err) + } + + key.policy, err = structure.NormalizeJsonString(aws.StringValue(policy)) + + if err != nil { + return nil, fmt.Errorf("policy contains invalid JSON: %w", err) + } + + if aws.StringValue(key.metadata.Origin) == kms.OriginTypeAwsKms { + key.rotation, err = finder.KeyRotationEnabledByKeyID(conn, keyID) - if err != nil { - return fmt.Errorf("Failed to set KMS key %q status to %t: %q", - id, shouldBeEnabled, err.Error()) - } - - // Wait for propagation since KMS is eventually consistent - wait := resource.StateChangeConf{ - Pending: []string{fmt.Sprintf("%t", !shouldBeEnabled)}, - Target: []string{fmt.Sprintf("%t", shouldBeEnabled)}, - Timeout: 20 * time.Minute, - MinTimeout: 2 * time.Second, - ContinuousTargetOccurence: 15, - Refresh: func() (interface{}, string, error) { - log.Printf("[DEBUG] Checking if KMS key %s enabled status is %t", - id, shouldBeEnabled) - resp, err := conn.DescribeKey(&kms.DescribeKeyInput{ - KeyId: aws.String(id), - }) if err != nil { - if isAWSErr(err, kms.ErrCodeNotFoundException, "") { - return nil, fmt.Sprintf("%t", !shouldBeEnabled), nil - } - return resp, "FAILED", err + return nil, fmt.Errorf("error reading KMS Key (%s) rotation enabled: %w", keyID, err) } - status := fmt.Sprintf("%t", *resp.KeyMetadata.Enabled) - log.Printf("[DEBUG] KMS key %s status received: %s, retrying", id, status) + } - return resp, status, nil - }, - } + key.tags, err = keyvaluetags.KmsListTags(conn, keyID) + + if tfawserr.ErrCodeEquals(err, kms.ErrCodeNotFoundException) { + return nil, &resource.NotFoundError{LastError: err} + } + + if err != nil { + return nil, fmt.Errorf("error listing tags for KMS Key (%s): %w", keyID, err) + } + + return &key, nil + }, isNewResource) - _, err = wait.WaitForState() if err != nil { - return fmt.Errorf("Failed setting KMS key status to %t: %w", shouldBeEnabled, err) + return nil, err } - return nil + return outputRaw.(*kmsKey), nil } -func updateKmsKeyRotationStatus(conn *kms.KMS, d *schema.ResourceData) error { - shouldEnableRotation := d.Get("enable_key_rotation").(bool) +func updateKmsKeyDescription(conn *kms.KMS, keyID string, description string) error { + input := &kms.UpdateKeyDescriptionInput{ + Description: aws.String(description), + KeyId: aws.String(keyID), + } - err := resource.Retry(10*time.Minute, func() *resource.RetryError { - err := handleKeyRotation(conn, shouldEnableRotation, aws.String(d.Id())) + log.Printf("[DEBUG] Updating KMS Key description: %s", input) + _, err := conn.UpdateKeyDescription(input) - if err != nil { - if isAWSErr(err, kms.ErrCodeDisabledException, "") { - return resource.RetryableError(err) - } - if isAWSErr(err, kms.ErrCodeNotFoundException, "") { - return resource.RetryableError(err) - } + if err != nil { + return fmt.Errorf("error updating KMS Key (%s) description: %w", keyID, err) + } - return resource.NonRetryableError(err) - } + // Wait for propagation since KMS is eventually consistent. + err = waiter.KeyDescriptionPropagated(conn, keyID, description) - return nil - }) - if isResourceTimeoutError(err) { - err = handleKeyRotation(conn, shouldEnableRotation, aws.String(d.Id())) + if err != nil { + return fmt.Errorf("error waiting for KMS Key (%s) description propagation: %w", keyID, err) } - if err != nil { - return fmt.Errorf("Failed to set key rotation for %q to %t: %q", - d.Id(), shouldEnableRotation, err.Error()) - } - - // Wait for propagation since KMS is eventually consistent - wait := resource.StateChangeConf{ - Pending: []string{fmt.Sprintf("%t", !shouldEnableRotation)}, - Target: []string{fmt.Sprintf("%t", shouldEnableRotation)}, - Timeout: 5 * time.Minute, - MinTimeout: 1 * time.Second, - ContinuousTargetOccurence: 5, - Refresh: func() (interface{}, string, error) { - log.Printf("[DEBUG] Checking if KMS key %s rotation status is %t", - d.Id(), shouldEnableRotation) - - out, err := retryOnAwsCode(kms.ErrCodeNotFoundException, func() (interface{}, error) { - return conn.GetKeyRotationStatus(&kms.GetKeyRotationStatusInput{ - KeyId: aws.String(d.Id()), - }) + return nil +} + +func updateKmsKeyEnabled(conn *kms.KMS, keyID string, enabled bool) error { + updateFunc := func() (interface{}, error) { + var err error + + log.Printf("[DEBUG] Updating KMS Key (%s) key enabled: %t", keyID, enabled) + if enabled { + _, err = conn.EnableKey(&kms.EnableKeyInput{ + KeyId: aws.String(keyID), }) - if err != nil { - return 42, "", err - } - resp, _ := out.(*kms.GetKeyRotationStatusOutput) + } else { + _, err = conn.DisableKey(&kms.DisableKeyInput{ + KeyId: aws.String(keyID), + }) + } - status := fmt.Sprintf("%t", *resp.KeyRotationEnabled) - log.Printf("[DEBUG] KMS key %s rotation status received: %s, retrying", d.Id(), status) + return nil, err + } - return resp, status, nil - }, + _, err := tfresource.RetryWhenAwsErrCodeEquals(waiter.KeyRotationUpdatedTimeout, updateFunc, kms.ErrCodeNotFoundException) + + if err != nil { + return fmt.Errorf("error updating KMS Key (%s) key enabled (%t): %w", keyID, enabled, err) } - _, err = wait.WaitForState() + // Wait for propagation since KMS is eventually consistent. + err = waiter.KeyStatePropagated(conn, keyID, enabled) + if err != nil { - return fmt.Errorf("Failed setting KMS key rotation status to %t: %s", shouldEnableRotation, err) + return fmt.Errorf("error waiting for KMS Key (%s) key state propagation: %w", keyID, err) } return nil } -func handleKeyRotation(conn *kms.KMS, shouldEnableRotation bool, keyId *string) error { - var err error - if shouldEnableRotation { - log.Printf("[DEBUG] Enabling key rotation for KMS key %q", *keyId) - _, err = conn.EnableKeyRotation(&kms.EnableKeyRotationInput{ - KeyId: keyId, - }) - } else { - log.Printf("[DEBUG] Disabling key rotation for KMS key %q", *keyId) - _, err = conn.DisableKeyRotation(&kms.DisableKeyRotationInput{ - KeyId: keyId, - }) - } - return err -} - -func resourceAwsKmsKeyDelete(d *schema.ResourceData, meta interface{}) error { - conn := meta.(*AWSClient).kmsconn - keyId := d.Get("key_id").(string) +func updateKmsKeyPolicy(conn *kms.KMS, keyID string, policy string, bypassPolicyLockoutSafetyCheck bool) error { + policy, err := structure.NormalizeJsonString(policy) - req := &kms.ScheduleKeyDeletionInput{ - KeyId: aws.String(keyId), + if err != nil { + return fmt.Errorf("policy contains invalid JSON: %w", err) } - if v, exists := d.GetOk("deletion_window_in_days"); exists { - req.PendingWindowInDays = aws.Int64(int64(v.(int))) + + updateFunc := func() (interface{}, error) { + var err error + + input := &kms.PutKeyPolicyInput{ + BypassPolicyLockoutSafetyCheck: aws.Bool(bypassPolicyLockoutSafetyCheck), + KeyId: aws.String(keyID), + Policy: aws.String(policy), + PolicyName: aws.String(tfkms.PolicyNameDefault), + } + + log.Printf("[DEBUG] Updating KMS Key policy: %s", input) + _, err = conn.PutKeyPolicy(input) + + return nil, err } - _, err := conn.ScheduleKeyDeletion(req) - if isAWSErr(err, kms.ErrCodeNotFoundException, "") { - return nil + _, err = tfresource.RetryWhenAwsErrCodeEquals(waiter.PropagationTimeout, updateFunc, kms.ErrCodeNotFoundException) + + if err != nil { + return fmt.Errorf("error updating KMS Key (%s) policy: %w", keyID, err) } + // Wait for propagation since KMS is eventually consistent. + err = waiter.KeyPolicyPropagated(conn, keyID, policy) + if err != nil { - return fmt.Errorf("error scheduling deletion for KMS Key (%s): %w", d.Id(), err) + return fmt.Errorf("error waiting for KMS Key (%s) policy propagation: %w", keyID, err) } - _, err = waiter.KeyStatePendingDeletion(conn, d.Id()) + return nil +} - if isAWSErr(err, kms.ErrCodeNotFoundException, "") { - return nil +func updateKmsKeyRotationEnabled(conn *kms.KMS, keyID string, enabled bool) error { + updateFunc := func() (interface{}, error) { + var err error + + log.Printf("[DEBUG] Updating KMS Key (%s) key rotation enabled: %t", keyID, enabled) + if enabled { + _, err = conn.EnableKeyRotation(&kms.EnableKeyRotationInput{ + KeyId: aws.String(keyID), + }) + } else { + _, err = conn.DisableKeyRotation(&kms.DisableKeyRotationInput{ + KeyId: aws.String(keyID), + }) + } + + return nil, err + } + + _, err := tfresource.RetryWhenAwsErrCodeEquals(waiter.KeyRotationUpdatedTimeout, updateFunc, kms.ErrCodeNotFoundException, kms.ErrCodeDisabledException) + + if err != nil { + return fmt.Errorf("error updating KMS Key (%s) key rotation enabled (%t): %w", keyID, enabled, err) } + // Wait for propagation since KMS is eventually consistent. + err = waiter.KeyRotationEnabledPropagated(conn, keyID, enabled) + if err != nil { - return fmt.Errorf("error waiting for KMS Key (%s) to schedule deletion: %w", d.Id(), err) + return fmt.Errorf("error waiting for KMS Key (%s) key rotation propagation: %w", keyID, err) } return nil diff --git a/aws/resource_aws_kms_key_test.go b/aws/resource_aws_kms_key_test.go index fce4b5b8a3f..d0fd6064cd6 100644 --- a/aws/resource_aws_kms_key_test.go +++ b/aws/resource_aws_kms_key_test.go @@ -3,6 +3,7 @@ package aws import ( "fmt" "log" + "regexp" "testing" "github.com/aws/aws-sdk-go/aws" @@ -11,6 +12,9 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" awspolicy "github.com/jen20/awspolicyequivalence" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/kms/finder" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/kms/waiter" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" ) func init() { @@ -72,7 +76,6 @@ func testSweepKmsKeys(region string) error { func TestAccAWSKmsKey_basic(t *testing.T) { var key kms.KeyMetadata - rName := fmt.Sprintf("tf-testacc-kms-key-%s", acctest.RandString(13)) resourceName := "aws_kms_key.test" resource.ParallelTest(t, resource.TestCase{ @@ -82,7 +85,7 @@ func TestAccAWSKmsKey_basic(t *testing.T) { CheckDestroy: testAccCheckAWSKmsKeyDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSKmsKey(rName), + Config: testAccAWSKmsKeyConfig(), Check: resource.ComposeTestCheckFunc( testAccCheckAWSKmsKeyExists(resourceName, &key), resource.TestCheckResourceAttr(resourceName, "customer_master_key_spec", "SYMMETRIC_DEFAULT"), @@ -94,7 +97,7 @@ func TestAccAWSKmsKey_basic(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"deletion_window_in_days"}, + ImportStateVerifyIgnore: []string{"deletion_window_in_days", "bypass_policy_lockout_safety_check"}, }, }, }) @@ -102,7 +105,7 @@ func TestAccAWSKmsKey_basic(t *testing.T) { func TestAccAWSKmsKey_asymmetricKey(t *testing.T) { var key kms.KeyMetadata - rName := fmt.Sprintf("tf-testacc-kms-key-%s", acctest.RandString(13)) + rName := acctest.RandomWithPrefix("tf-acc-test") resourceName := "aws_kms_key.test" resource.ParallelTest(t, resource.TestCase{ @@ -125,7 +128,7 @@ func TestAccAWSKmsKey_asymmetricKey(t *testing.T) { func TestAccAWSKmsKey_disappears(t *testing.T) { var key kms.KeyMetadata - rName := fmt.Sprintf("tf-testacc-kms-key-%s", acctest.RandString(13)) + rName := acctest.RandomWithPrefix("tf-acc-test") resourceName := "aws_kms_key.test" resource.ParallelTest(t, resource.TestCase{ @@ -135,7 +138,7 @@ func TestAccAWSKmsKey_disappears(t *testing.T) { CheckDestroy: testAccCheckAWSKmsKeyDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSKmsKey(rName), + Config: testAccAWSKmsKeyConfigName(rName), Check: resource.ComposeTestCheckFunc( testAccCheckAWSKmsKeyExists(resourceName, &key), testAccCheckResourceDisappears(testAccProvider, resourceAwsKmsKey(), resourceName), @@ -148,7 +151,7 @@ func TestAccAWSKmsKey_disappears(t *testing.T) { func TestAccAWSKmsKey_policy(t *testing.T) { var key kms.KeyMetadata - rName := fmt.Sprintf("tf-testacc-kms-key-%s", acctest.RandString(13)) + rName := acctest.RandomWithPrefix("tf-acc-test") resourceName := "aws_kms_key.test" expectedPolicyText := `{"Version":"2012-10-17","Id":"kms-tf-1","Statement":[{"Sid":"Enable IAM User Permissions","Effect":"Allow","Principal":{"AWS":"*"},"Action":"kms:*","Resource":"*"}]}` @@ -169,7 +172,7 @@ func TestAccAWSKmsKey_policy(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"deletion_window_in_days"}, + ImportStateVerifyIgnore: []string{"deletion_window_in_days", "bypass_policy_lockout_safety_check"}, }, { Config: testAccAWSKmsKey_removedPolicy(rName), @@ -181,6 +184,67 @@ func TestAccAWSKmsKey_policy(t *testing.T) { }) } +func TestAccAWSKmsKey_policyBypass(t *testing.T) { + var key kms.KeyMetadata + rName := acctest.RandomWithPrefix("tf-acc-test") + resourceName := "aws_kms_key.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + ErrorCheck: testAccErrorCheck(t, kms.EndpointsID), + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSKmsKeyDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSKmsKey_policyBypass(rName, false), + ExpectError: regexp.MustCompile(`The new key policy will not allow you to update the key policy in the future`), + }, + { + Config: testAccAWSKmsKey_policyBypass(rName, true), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSKmsKeyExists(resourceName, &key), + resource.TestCheckResourceAttr(resourceName, "bypass_policy_lockout_safety_check", "true"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"deletion_window_in_days", "bypass_policy_lockout_safety_check"}, + }, + }, + }) +} + +func TestAccAWSKmsKey_policyBypassUpdate(t *testing.T) { + var before, after kms.KeyMetadata + rName := acctest.RandomWithPrefix("tf-acc-test") + resourceName := "aws_kms_key.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + ErrorCheck: testAccErrorCheck(t, kms.EndpointsID), + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSKmsKeyDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSKmsKeyConfigName(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSKmsKeyExists(resourceName, &before), + resource.TestCheckResourceAttr(resourceName, "bypass_policy_lockout_safety_check", "false"), + ), + }, + { + Config: testAccAWSKmsKey_policyBypass(rName, true), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSKmsKeyExists(resourceName, &after), + resource.TestCheckResourceAttr(resourceName, "bypass_policy_lockout_safety_check", "true"), + ), + }, + }, + }) +} + func TestAccAWSKmsKey_Policy_IamRole(t *testing.T) { var key kms.KeyMetadata rName := acctest.RandomWithPrefix("tf-acc-test") @@ -202,7 +266,7 @@ func TestAccAWSKmsKey_Policy_IamRole(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"deletion_window_in_days"}, + ImportStateVerifyIgnore: []string{"deletion_window_in_days", "bypass_policy_lockout_safety_check"}, }, }, }) @@ -230,7 +294,7 @@ func TestAccAWSKmsKey_Policy_IamServiceLinkedRole(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"deletion_window_in_days"}, + ImportStateVerifyIgnore: []string{"deletion_window_in_days", "bypass_policy_lockout_safety_check"}, }, }, }) @@ -238,7 +302,7 @@ func TestAccAWSKmsKey_Policy_IamServiceLinkedRole(t *testing.T) { func TestAccAWSKmsKey_isEnabled(t *testing.T) { var key1, key2, key3 kms.KeyMetadata - rName := fmt.Sprintf("tf-testacc-kms-key-%s", acctest.RandString(13)) + rName := acctest.RandomWithPrefix("tf-acc-test") resourceName := "aws_kms_key.test" resource.ParallelTest(t, resource.TestCase{ @@ -260,7 +324,7 @@ func TestAccAWSKmsKey_isEnabled(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"deletion_window_in_days"}, + ImportStateVerifyIgnore: []string{"deletion_window_in_days", "bypass_policy_lockout_safety_check"}, }, { Config: testAccAWSKmsKey_disabled(rName), @@ -286,7 +350,7 @@ func TestAccAWSKmsKey_isEnabled(t *testing.T) { func TestAccAWSKmsKey_tags(t *testing.T) { var key kms.KeyMetadata - rName := fmt.Sprintf("tf-testacc-kms-key-%s", acctest.RandString(13)) + rName := acctest.RandomWithPrefix("tf-acc-test") resourceName := "aws_kms_key.test" resource.ParallelTest(t, resource.TestCase{ @@ -296,23 +360,34 @@ func TestAccAWSKmsKey_tags(t *testing.T) { CheckDestroy: testAccCheckAWSKmsKeyDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSKmsKey_tags(rName), + Config: testAccAWSKmsKeyConfigTags1(rName, "key1", "value1"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSKmsKeyExists(resourceName, &key), - resource.TestCheckResourceAttr(resourceName, "tags.%", "3"), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1"), ), }, { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"deletion_window_in_days"}, + ImportStateVerifyIgnore: []string{"deletion_window_in_days", "bypass_policy_lockout_safety_check"}, }, { - Config: testAccAWSKmsKey(rName), + Config: testAccAWSKmsKeyConfigTags2(rName, "key1", "value1updated", "key2", "value2"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSKmsKeyExists(resourceName, &key), - resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), + resource.TestCheckResourceAttr(resourceName, "tags.%", "2"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1updated"), + resource.TestCheckResourceAttr(resourceName, "tags.key2", "value2"), + ), + }, + { + Config: testAccAWSKmsKeyConfigTags1(rName, "key2", "value2"), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSKmsKeyExists(resourceName, &key), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.key2", "value2"), ), }, }, @@ -363,19 +438,17 @@ func testAccCheckAWSKmsKeyDestroy(s *terraform.State) error { continue } - out, err := conn.DescribeKey(&kms.DescribeKeyInput{ - KeyId: aws.String(rs.Primary.ID), - }) + _, err := finder.KeyByID(conn, rs.Primary.ID) - if err != nil { - return err + if tfresource.NotFound(err) { + continue } - if *out.KeyMetadata.KeyState == "PendingDeletion" { - return nil + if err != nil { + return err } - return fmt.Errorf("KMS key still exists:\n%#v", out.KeyMetadata) + return fmt.Errorf("KMS Key %s still exists", rs.Primary.ID) } return nil @@ -394,17 +467,15 @@ func testAccCheckAWSKmsKeyExists(name string, key *kms.KeyMetadata) resource.Tes conn := testAccProvider.Meta().(*AWSClient).kmsconn - o, err := retryOnAwsCode("NotFoundException", func() (interface{}, error) { - return conn.DescribeKey(&kms.DescribeKeyInput{ - KeyId: aws.String(rs.Primary.ID), - }) + outputRaw, err := tfresource.RetryWhenNotFound(waiter.PropagationTimeout, func() (interface{}, error) { + return finder.KeyByID(conn, rs.Primary.ID) }) + if err != nil { return err } - out := o.(*kms.DescribeKeyOutput) - *key = *out.KeyMetadata + *key = *(outputRaw.(*kms.KeyMetadata)) return nil } @@ -412,16 +483,21 @@ func testAccCheckAWSKmsKeyExists(name string, key *kms.KeyMetadata) resource.Tes func testAccCheckAWSKmsKeyIsEnabled(key *kms.KeyMetadata, isEnabled bool) resource.TestCheckFunc { return func(s *terraform.State) error { - if *key.Enabled != isEnabled { - return fmt.Errorf("Expected key %q to have is_enabled=%t, given %t", - *key.Arn, isEnabled, *key.Enabled) + if got, want := aws.BoolValue(key.Enabled), isEnabled; got != want { + return fmt.Errorf("Expected key %q to have is_enabled=%t, given %t", aws.StringValue(key.Arn), want, got) } return nil } } -func testAccAWSKmsKey(rName string) string { +func testAccAWSKmsKeyConfig() string { + return ` +resource "aws_kms_key" "test" {} +` +} + +func testAccAWSKmsKeyConfigName(rName string) string { return fmt.Sprintf(` resource "aws_kms_key" "test" { description = %[1]q @@ -469,6 +545,46 @@ POLICY `, rName) } +func testAccAWSKmsKey_policyBypass(rName string, bypassFlag bool) string { + return fmt.Sprintf(` +data "aws_caller_identity" "current" {} + +resource "aws_kms_key" "test" { + description = %[1]q + deletion_window_in_days = 7 + + bypass_policy_lockout_safety_check = %[2]t + + policy = <<-POLICY + { + "Version": "2012-10-17", + "Id": "kms-tf-1", + "Statement": [ + { + "Sid": "Enable IAM User Permissions", + "Effect": "Allow", + "Principal": { + "AWS": "${data.aws_caller_identity.current.arn}" + }, + "Action": [ + "kms:CreateKey", + "kms:DescribeKey", + "kms:ScheduleKeyDeletion", + "kms:Describe*", + "kms:Get*", + "kms:List*", + "kms:TagResource", + "kms:UntagResource" + ], + "Resource": "*" + } + ] + } + POLICY +} +`, rName, bypassFlag) +} + func testAccAWSKmsKeyConfigPolicyIamRole(rName string) string { return fmt.Sprintf(` data "aws_partition" "current" {} @@ -582,10 +698,6 @@ func testAccAWSKmsKey_removedPolicy(rName string) string { resource "aws_kms_key" "test" { description = %[1]q deletion_window_in_days = 7 - - tags = { - Name = %[1]q - } } `, rName) } @@ -596,10 +708,6 @@ resource "aws_kms_key" "test" { description = %[1]q deletion_window_in_days = 7 enable_key_rotation = true - - tags = { - Name = %[1]q - } } `, rName) } @@ -611,10 +719,6 @@ resource "aws_kms_key" "test" { deletion_window_in_days = 7 enable_key_rotation = false is_enabled = false - - tags = { - Name = %[1]q - } } `, rName) } @@ -626,24 +730,31 @@ resource "aws_kms_key" "test" { deletion_window_in_days = 7 enable_key_rotation = true is_enabled = true +} +`, rName) +} + +func testAccAWSKmsKeyConfigTags1(rName, tagKey1, tagValue1 string) string { + return fmt.Sprintf(` +resource "aws_kms_key" "test" { + description = %[1]q tags = { - Name = %[1]q + %[2]q = %[3]q } } -`, rName) +`, rName, tagKey1, tagValue1) } -func testAccAWSKmsKey_tags(rName string) string { +func testAccAWSKmsKeyConfigTags2(rName, tagKey1, tagValue1, tagKey2, tagValue2 string) string { return fmt.Sprintf(` resource "aws_kms_key" "test" { description = %[1]q tags = { - Name = %[1]q - Key1 = "Value One" - Description = "Very interesting" + %[2]q = %[3]q + %[4]q = %[5]q } } -`, rName) +`, rName, tagKey1, tagValue1, tagKey2, tagValue2) } diff --git a/website/docs/r/kms_external_key.html.markdown b/website/docs/r/kms_external_key.html.markdown index 7a35412beac..c9e07089516 100644 --- a/website/docs/r/kms_external_key.html.markdown +++ b/website/docs/r/kms_external_key.html.markdown @@ -24,6 +24,7 @@ resource "aws_kms_external_key" "example" { The following arguments are supported: +* `bypass_policy_lockout_safety_check` - (Optional) Specifies whether to disable the policy lockout check performed when creating or updating the key's policy. Setting this value to `true` increases the risk that the key becomes unmanageable. For more information, refer to the scenario in the [Default Key Policy](https://docs.aws.amazon.com/kms/latest/developerguide/key-policies.html#key-policy-default-allow-root-enable-iam) section in the AWS Key Management Service Developer Guide. Defaults to `false`. * `deletion_window_in_days` - (Optional) Duration in days after which the key is deleted after destruction of the resource. Must be between `7` and `30` days. Defaults to `30`. * `description` - (Optional) Description of the key. * `enabled` - (Optional) Specifies whether the key is enabled. Keys pending import can only be `false`. Imported keys default to `true` unless expired. diff --git a/website/docs/r/kms_key.html.markdown b/website/docs/r/kms_key.html.markdown index 0a063d31a6b..a167ca6c57d 100644 --- a/website/docs/r/kms_key.html.markdown +++ b/website/docs/r/kms_key.html.markdown @@ -32,6 +32,7 @@ Valid values: `SYMMETRIC_DEFAULT`, `RSA_2048`, `RSA_3072`, `RSA_4096`, `ECC_NIS ~> **NOTE:** Note: All KMS keys must have a key policy. If a key policy is not specified, AWS gives the KMS key a [default key policy](https://docs.aws.amazon.com/kms/latest/developerguide/key-policies.html#key-policy-default) that gives all principals in the owning account unlimited access to all KMS operations for the key. This default key policy effectively delegates all access control to IAM policies and KMS grants. +* `bypass_policy_lockout_safety_check` - (Optional) Specifies whether to disable the policy lockout check performed when creating or updating the key's policy. Setting this value to `true` increases the risk that the CMK becomes unmanageable. For more information, refer to the scenario in the [Default Key Policy](https://docs.aws.amazon.com/kms/latest/developerguide/key-policies.html#key-policy-default-allow-root-enable-iam) section in the AWS Key Management Service Developer Guide. Defaults to `false`. * `deletion_window_in_days` - (Optional) Duration in days after which the key is deleted after destruction of the resource, must be between 7 and 30 days. Defaults to 30 days. * `is_enabled` - (Optional) Specifies whether the key is enabled. Defaults to true. * `enable_key_rotation` - (Optional) Specifies whether [key rotation](http://docs.aws.amazon.com/kms/latest/developerguide/rotate-keys.html) is enabled. Defaults to false.