Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

d/aws_kms_key: Allow multi-region key ID or ARN for key_id parameter value #29266

Merged
merged 3 commits into from
Feb 7, 2023
Merged

d/aws_kms_key: Allow multi-region key ID or ARN for key_id parameter value #29266

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
66e3cd4
d/aws_kms_key: Additional acceptance tests.
ewbankkit Feb 6, 2023
2a108f3
Support multi-region keys in 'ValidateKey' and 'ValidateKeyOrAlias'.
ewbankkit Feb 6, 2023
04e908f
Add CHANGELOG entry.
ewbankkit Feb 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/29266.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
data-source/aws_kms_key: Reinstate support for KMS multi-Region key ID or ARN values for the `key_id` argument
```
8 changes: 2 additions & 6 deletions internal/service/kms/diff.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
package kms

import (
"regexp"
"strings"

"github.com/aws/aws-sdk-go-v2/aws/arn"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-provider-aws/internal/verify"
)

func DiffSuppressKey(_, oldValue, newValue string, _ *schema.ResourceData) bool {
Expand Down Expand Up @@ -76,8 +74,7 @@ func keyIdFromARN(s string) string {
}

func keyIdFromARNResource(s string) string {
re := regexp.MustCompile(`^key/(` + verify.UUIDRegexPattern + ")$")
matches := re.FindStringSubmatch(s)
matches := keyIdResourceRegex.FindStringSubmatch(s)
if matches == nil || len(matches) != 2 {
return ""
}
Expand All @@ -95,8 +92,7 @@ func keyAliasFromARN(s string) string {
}

func keyAliasNameFromARNResource(s string) string {
re := regexp.MustCompile("^" + AliasNameRegexPattern + "$")
if re.MatchString(s) {
if aliasNameRegex.MatchString(s) {
return s
}

Expand Down
216 changes: 210 additions & 6 deletions internal/service/kms/key_data_source_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
"github.com/hashicorp/terraform-provider-aws/internal/acctest"
)

func TestAccKMSKeyDataSource_basic(t *testing.T) {
func TestAccKMSKeyDataSource_byKeyARN(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I like the more specific naming here. It does break the convention of a _basic test, but because the data source accepts one of ARN, ID, alias ARN, or alias ID, it's better this way

resourceName := "aws_kms_key.test"
dataSourceName := "data.aws_kms_key.test"
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
Expand All @@ -21,7 +21,109 @@ func TestAccKMSKeyDataSource_basic(t *testing.T) {
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
Steps: []resource.TestStep{
{
Config: testAccKeyDataSourceConfig_basic(rName),
Config: testAccKeyDataSourceConfig_byKeyARN(rName),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttrPair(dataSourceName, "arn", resourceName, "arn"),
acctest.CheckResourceAttrAccountID(dataSourceName, "aws_account_id"),
resource.TestCheckResourceAttrSet(dataSourceName, "creation_date"),
resource.TestCheckResourceAttrPair(dataSourceName, "customer_master_key_spec", resourceName, "customer_master_key_spec"),
resource.TestCheckNoResourceAttr(dataSourceName, "deletion_date"),
resource.TestCheckResourceAttrPair(dataSourceName, "description", resourceName, "description"),
resource.TestCheckResourceAttrPair(dataSourceName, "enabled", resourceName, "is_enabled"),
resource.TestCheckResourceAttr(dataSourceName, "expiration_model", ""),
resource.TestCheckResourceAttr(dataSourceName, "key_manager", "CUSTOMER"),
resource.TestCheckResourceAttr(dataSourceName, "key_state", "Enabled"),
resource.TestCheckResourceAttrPair(dataSourceName, "key_usage", resourceName, "key_usage"),
resource.TestCheckResourceAttrPair(dataSourceName, "multi_region", resourceName, "multi_region"),
resource.TestCheckResourceAttr(dataSourceName, "multi_region_configuration.#", "0"),
resource.TestCheckResourceAttr(dataSourceName, "origin", "AWS_KMS"),
resource.TestCheckNoResourceAttr(dataSourceName, "valid_to"),
),
},
},
})
}

func TestAccKMSKeyDataSource_byKeyID(t *testing.T) {
resourceName := "aws_kms_key.test"
dataSourceName := "data.aws_kms_key.test"
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(t) },
ErrorCheck: acctest.ErrorCheck(t, kms.EndpointsID),
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
Steps: []resource.TestStep{
{
Config: testAccKeyDataSourceConfig_byKeyID(rName),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttrPair(dataSourceName, "arn", resourceName, "arn"),
acctest.CheckResourceAttrAccountID(dataSourceName, "aws_account_id"),
resource.TestCheckResourceAttrSet(dataSourceName, "creation_date"),
resource.TestCheckResourceAttrPair(dataSourceName, "customer_master_key_spec", resourceName, "customer_master_key_spec"),
resource.TestCheckNoResourceAttr(dataSourceName, "deletion_date"),
resource.TestCheckResourceAttrPair(dataSourceName, "description", resourceName, "description"),
resource.TestCheckResourceAttrPair(dataSourceName, "enabled", resourceName, "is_enabled"),
resource.TestCheckResourceAttr(dataSourceName, "expiration_model", ""),
resource.TestCheckResourceAttr(dataSourceName, "key_manager", "CUSTOMER"),
resource.TestCheckResourceAttr(dataSourceName, "key_state", "Enabled"),
resource.TestCheckResourceAttrPair(dataSourceName, "key_usage", resourceName, "key_usage"),
resource.TestCheckResourceAttrPair(dataSourceName, "multi_region", resourceName, "multi_region"),
resource.TestCheckResourceAttr(dataSourceName, "multi_region_configuration.#", "0"),
resource.TestCheckResourceAttr(dataSourceName, "origin", "AWS_KMS"),
resource.TestCheckNoResourceAttr(dataSourceName, "valid_to"),
),
},
},
})
}

func TestAccKMSKeyDataSource_byAliasARN(t *testing.T) {
resourceName := "aws_kms_key.test"
dataSourceName := "data.aws_kms_key.test"
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(t) },
ErrorCheck: acctest.ErrorCheck(t, kms.EndpointsID),
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
Steps: []resource.TestStep{
{
Config: testAccKeyDataSourceConfig_byAliasARN(rName),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttrPair(dataSourceName, "arn", resourceName, "arn"),
acctest.CheckResourceAttrAccountID(dataSourceName, "aws_account_id"),
resource.TestCheckResourceAttrSet(dataSourceName, "creation_date"),
resource.TestCheckResourceAttrPair(dataSourceName, "customer_master_key_spec", resourceName, "customer_master_key_spec"),
resource.TestCheckNoResourceAttr(dataSourceName, "deletion_date"),
resource.TestCheckResourceAttrPair(dataSourceName, "description", resourceName, "description"),
resource.TestCheckResourceAttrPair(dataSourceName, "enabled", resourceName, "is_enabled"),
resource.TestCheckResourceAttr(dataSourceName, "expiration_model", ""),
resource.TestCheckResourceAttr(dataSourceName, "key_manager", "CUSTOMER"),
resource.TestCheckResourceAttr(dataSourceName, "key_state", "Enabled"),
resource.TestCheckResourceAttrPair(dataSourceName, "key_usage", resourceName, "key_usage"),
resource.TestCheckResourceAttrPair(dataSourceName, "multi_region", resourceName, "multi_region"),
resource.TestCheckResourceAttr(dataSourceName, "multi_region_configuration.#", "0"),
resource.TestCheckResourceAttr(dataSourceName, "origin", "AWS_KMS"),
resource.TestCheckNoResourceAttr(dataSourceName, "valid_to"),
),
},
},
})
}

func TestAccKMSKeyDataSource_byAliasID(t *testing.T) {
resourceName := "aws_kms_key.test"
dataSourceName := "data.aws_kms_key.test"
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(t) },
ErrorCheck: acctest.ErrorCheck(t, kms.EndpointsID),
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
Steps: []resource.TestStep{
{
Config: testAccKeyDataSourceConfig_byAliasID(rName),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttrPair(dataSourceName, "arn", resourceName, "arn"),
acctest.CheckResourceAttrAccountID(dataSourceName, "aws_account_id"),
Expand Down Expand Up @@ -78,7 +180,46 @@ func TestAccKMSKeyDataSource_grantToken(t *testing.T) {
})
}

func TestAccKMSKeyDataSource_multiRegionConfiguration(t *testing.T) {
func TestAccKMSKeyDataSource_multiRegionConfigurationByARN(t *testing.T) {
resourceName := "aws_kms_key.test"
dataSourceName := "data.aws_kms_key.test"
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(t) },
ErrorCheck: acctest.ErrorCheck(t, kms.EndpointsID),
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
Steps: []resource.TestStep{
{
Config: testAccKeyDataSourceConfig_multiRegionByARN(rName),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttrPair(dataSourceName, "arn", resourceName, "arn"),
acctest.CheckResourceAttrAccountID(dataSourceName, "aws_account_id"),
resource.TestCheckResourceAttrSet(dataSourceName, "creation_date"),
resource.TestCheckResourceAttrPair(dataSourceName, "customer_master_key_spec", resourceName, "customer_master_key_spec"),
resource.TestCheckNoResourceAttr(dataSourceName, "deletion_date"),
resource.TestCheckResourceAttrPair(dataSourceName, "description", resourceName, "description"),
resource.TestCheckResourceAttrPair(dataSourceName, "enabled", resourceName, "is_enabled"),
resource.TestCheckResourceAttr(dataSourceName, "expiration_model", ""),
resource.TestCheckResourceAttr(dataSourceName, "key_manager", "CUSTOMER"),
resource.TestCheckResourceAttr(dataSourceName, "key_state", "Enabled"),
resource.TestCheckResourceAttrPair(dataSourceName, "key_usage", resourceName, "key_usage"),
resource.TestCheckResourceAttrPair(dataSourceName, "multi_region", resourceName, "multi_region"),
resource.TestCheckResourceAttr(dataSourceName, "multi_region_configuration.#", "1"),
resource.TestCheckResourceAttr(dataSourceName, "multi_region_configuration.0.multi_region_key_type", "PRIMARY"),
resource.TestCheckResourceAttr(dataSourceName, "multi_region_configuration.0.primary_key.#", "1"),
resource.TestCheckResourceAttrPair(dataSourceName, "multi_region_configuration.0.primary_key.0.arn", resourceName, "arn"),
resource.TestCheckResourceAttr(dataSourceName, "multi_region_configuration.0.primary_key.0.region", acctest.Region()),
resource.TestCheckResourceAttr(dataSourceName, "multi_region_configuration.0.replica_keys.#", "0"),
resource.TestCheckResourceAttr(dataSourceName, "origin", "AWS_KMS"),
resource.TestCheckNoResourceAttr(dataSourceName, "valid_to"),
),
},
},
})
}

func TestAccKMSKeyDataSource_multiRegionConfigurationByID(t *testing.T) {
resourceName := "aws_kms_key.test"
dataSourceName := "data.aws_kms_key.test"
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
Expand All @@ -89,7 +230,7 @@ func TestAccKMSKeyDataSource_multiRegionConfiguration(t *testing.T) {
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
Steps: []resource.TestStep{
{
Config: testAccKeyDataSourceConfig_multiRegion(rName),
Config: testAccKeyDataSourceConfig_multiRegionByID(rName),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttrPair(dataSourceName, "arn", resourceName, "arn"),
acctest.CheckResourceAttrAccountID(dataSourceName, "aws_account_id"),
Expand Down Expand Up @@ -117,7 +258,20 @@ func TestAccKMSKeyDataSource_multiRegionConfiguration(t *testing.T) {
})
}

func testAccKeyDataSourceConfig_basic(rName string) string {
func testAccKeyDataSourceConfig_byKeyARN(rName string) string {
return fmt.Sprintf(`
resource "aws_kms_key" "test" {
description = %[1]q
deletion_window_in_days = 7
}

data "aws_kms_key" "test" {
key_id = aws_kms_key.test.arn
}
`, rName)
}

func testAccKeyDataSourceConfig_byKeyID(rName string) string {
return fmt.Sprintf(`
resource "aws_kms_key" "test" {
description = %[1]q
Expand All @@ -130,6 +284,42 @@ data "aws_kms_key" "test" {
`, rName)
}

func testAccKeyDataSourceConfig_byAliasARN(rName 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/%[1]s"
target_key_id = aws_kms_key.test.id
}

data "aws_kms_key" "test" {
key_id = aws_kms_alias.test.arn
}
`, rName)
}

func testAccKeyDataSourceConfig_byAliasID(rName 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/%[1]s"
target_key_id = aws_kms_key.test.id
}

data "aws_kms_key" "test" {
key_id = aws_kms_alias.test.id
}
`, rName)
}

func testAccKeyDataSourceConfig_grantToken(rName string) string {
return acctest.ConfigCompose(testAccGrantBaseConfig(rName), fmt.Sprintf(`
resource "aws_kms_grant" "test" {
Expand All @@ -146,7 +336,21 @@ data "aws_kms_key" "test" {
`, rName))
}

func testAccKeyDataSourceConfig_multiRegion(rName string) string {
func testAccKeyDataSourceConfig_multiRegionByARN(rName string) string {
return fmt.Sprintf(`
resource "aws_kms_key" "test" {
description = %[1]q
deletion_window_in_days = 7
multi_region = true
}

data "aws_kms_key" "test" {
key_id = aws_kms_key.test.arn
}
`, rName)
}

func testAccKeyDataSourceConfig_multiRegionByID(rName string) string {
return fmt.Sprintf(`
resource "aws_kms_key" "test" {
description = %[1]q
Expand Down
19 changes: 14 additions & 5 deletions internal/service/kms/validate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,16 @@ import (
"github.com/hashicorp/terraform-provider-aws/internal/verify"
)

const AliasNameRegexPattern = `alias/[a-zA-Z0-9/_-]+`
const (
aliasNameRegexPattern = `alias/[a-zA-Z0-9/_-]+`
multiRegionKeyIdPattern = `mrk-[a-f0-9]{32}`
)

var (
aliasNameRegex = regexp.MustCompile(`^` + aliasNameRegexPattern + `$`)
keyIdRegex = regexp.MustCompile(`^` + verify.UUIDRegexPattern + `|` + multiRegionKeyIdPattern + `$`)
keyIdResourceRegex = regexp.MustCompile(`^key/(` + verify.UUIDRegexPattern + `|` + multiRegionKeyIdPattern + `)$`)
)

func validGrantName(v interface{}, k string) (ws []string, es []error) {
value := v.(string)
Expand All @@ -28,7 +37,7 @@ func validGrantName(v interface{}, k string) (ws []string, es []error) {
func validNameForDataSource(v interface{}, k string) (ws []string, es []error) {
value := v.(string)

if !regexp.MustCompile("^" + AliasNameRegexPattern + "$").MatchString(value) {
if !aliasNameRegex.MatchString(value) {
es = append(es, fmt.Errorf(
"%q must begin with 'alias/' and be comprised of only [a-zA-Z0-9/_-]", k))
}
Expand All @@ -42,7 +51,7 @@ func validNameForResource(v interface{}, k string) (ws []string, es []error) {
es = append(es, fmt.Errorf("%q cannot begin with reserved AWS CMK prefix 'alias/aws/'", k))
}

if !regexp.MustCompile("^" + AliasNameRegexPattern + "$").MatchString(value) {
if !aliasNameRegex.MatchString(value) {
es = append(es, fmt.Errorf(
"%q must begin with 'alias/' and be comprised of only [a-zA-Z0-9/_-]", k))
}
Expand All @@ -61,7 +70,7 @@ var ValidateKeyOrAlias = validation.Any(
validateKeyAliasARN,
)

var validateKeyId = validation.StringMatch(regexp.MustCompile("^"+verify.UUIDRegexPattern+"$"), "must be a KMS Key ID")
var validateKeyId = validation.StringMatch(keyIdRegex, "must be a KMS Key ID")

func validateKeyARN(v any, k string) (ws []string, errors []error) {
value, ok := v.(string)
Expand All @@ -87,7 +96,7 @@ func validateKeyARN(v any, k string) (ws []string, errors []error) {
return
}

var validateKeyAliasName = validation.StringMatch(regexp.MustCompile("^"+AliasNameRegexPattern+"$"), "must be a KMS Key Alias")
var validateKeyAliasName = validation.StringMatch(aliasNameRegex, "must be a KMS Key Alias")

func validateKeyAliasARN(v any, k string) (ws []string, errors []error) {
value, ok := v.(string)
Expand Down
14 changes: 14 additions & 0 deletions internal/service/kms/validate_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,16 @@ func TestValidateKeyOrAlias(t *testing.T) {
ErrCount: 0,
valid: true,
},
{
Value: "mrk-f827515944fb43f9b902a09d2c8b554f",
ErrCount: 0,
valid: true,
},
{
Value: "arn:aws:kms:us-west-2:111122223333:key/mrk-a835af0b39c94b86a21a8fc9535df681", //lintignore:AWSAT003,AWSAT005
ErrCount: 0,
valid: true,
},
{
Value: "arn:aws:kms:us-west-2:111122223333:alias/arbitrary-key", //lintignore:AWSAT003,AWSAT005
ErrCount: 0,
Expand Down Expand Up @@ -193,6 +203,10 @@ func TestValidateKeyARN(t *testing.T) {
in: "arn:aws:kms:us-west-2:123456789012:key/57ff7a43-341d-46b6-aee3-a450c9de6dc8", // lintignore:AWSAT003,AWSAT005
valid: true,
},
"kms mrk key id": {
in: "arn:aws:kms:us-west-2:111122223333:key/mrk-a835af0b39c94b86a21a8fc9535df681", // lintignore:AWSAT003,AWSAT005
valid: true,
},
"kms non-key id": {
in: "arn:aws:kms:us-west-2:123456789012:something/else", // lintignore:AWSAT003,AWSAT005
valid: false,
Expand Down