Skip to content

Commit

Permalink
awsproviderlint: Enforce kms_key_id as ARN attribute in AWSAT001 check (
Browse files Browse the repository at this point in the history
#13996)

Previously, test failures could include these outside AWS Commercial:

```
--- FAIL: TestAccAWSEBSSnapshot_withKms (88.99s)
testing.go:684: Step 0 error: Check failed: Check 2/2 error: aws_ebs_snapshot.test: Attribute 'kms_key_id' didn't match "^arn:aws:kms:[a-z]{2}-[a-z]+-\\d{1}:[0-9]{12}:key/[a-z0-9-]{36}$", got "arn:aws-us-gov:kms:us-gov-west-1:123456789012:key/59dff49a-68bc-407d-959c-deb91757d55b"
```

New AWSAT001 check test failures:

```
aws/resource_aws_db_instance_test.go:187:6: AWSAT001: prefer resource.TestCheckResourceAttrPair() or ARN check functions (e.g. testAccMatchResourceAttrRegionalARN)
aws/resource_aws_ebs_snapshot_test.go:127:6: AWSAT001: prefer resource.TestCheckResourceAttrPair() or ARN check functions (e.g. testAccMatchResourceAttrRegionalARN)
aws/resource_aws_fsx_windows_file_system_test.go:92:6: AWSAT001: prefer resource.TestCheckResourceAttrPair() or ARN check functions (e.g. testAccMatchResourceAttrRegionalARN)
aws/resource_aws_rds_cluster_instance_test.go:207:6: AWSAT001: prefer resource.TestCheckResourceAttrPair() or ARN check functions (e.g. testAccMatchResourceAttrRegionalARN)
```

Output from acceptance testing in AWS Commercial:

```
--- PASS: TestAccAWSEBSSnapshot_withKms (41.72s)
--- PASS: TestAccAWSDBInstance_kmsKey (486.55s)
--- PASS: TestAccAWSRDSClusterInstance_kmsKey (691.04s)
```

Output from acceptance testing in AWS GovCloud (US) (hardcoded AZ and service availability PreCheck test failures will be separately addressed in the future):

```
--- PASS: TestAccAWSEBSSnapshot_withKms (60.17s)
--- PASS: TestAccAWSDBInstance_kmsKey (479.81s)

--- FAIL: TestAccAWSRDSClusterInstance_kmsKey (17.37s)
    TestAccAWSRDSClusterInstance_kmsKey: testing.go:684: Step 0 error: errors during apply:

        Error: error creating RDS cluster: InvalidVPCNetworkStateFault: Availability zones '[us-west-2a, us-west-2b, us-west-2c]' are unavailable in this region, please choose another zone set.

--- FAIL: TestAccAWSFsxWindowsFileSystem_basic (1743.81s)
    TestAccAWSFsxWindowsFileSystem_basic: testing.go:684: Step 0 error: errors during apply:

        Error: Error creating FSx filesystem: RequestError: send request failed
        caused by: Post "https://fsx.us-gov-west-1.amazonaws.com/": dial tcp: lookup fsx.us-gov-west-1.amazonaws.com: no such host
```
  • Loading branch information
bflad authored Jun 30, 2020
1 parent 269dd7c commit 3f12b61
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 12 deletions.
20 changes: 16 additions & 4 deletions aws/resource_aws_db_instance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,8 @@ func TestAccAWSDBInstance_generatedName(t *testing.T) {

func TestAccAWSDBInstance_kmsKey(t *testing.T) {
var v rds.DBInstance
keyRegex := regexp.MustCompile("^arn:aws:kms:")
kmsKeyResourceName := "aws_kms_key.foo"
resourceName := "aws_db_instance.bar"

ri := acctest.RandInt()
config := fmt.Sprintf(testAccAWSDBInstanceConfigKmsKeyId, ri)
Expand All @@ -182,12 +183,23 @@ func TestAccAWSDBInstance_kmsKey(t *testing.T) {
{
Config: config,
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSDBInstanceExists("aws_db_instance.bar", &v),
testAccCheckAWSDBInstanceExists(resourceName, &v),
testAccCheckAWSDBInstanceAttributes(&v),
resource.TestMatchResourceAttr(
"aws_db_instance.bar", "kms_key_id", keyRegex),
resource.TestCheckResourceAttrPair(resourceName, "kms_key_id", kmsKeyResourceName, "arn"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{
"apply_immediately",
"delete_automated_backups",
"final_snapshot_identifier",
"password",
"skip_final_snapshot",
},
},
},
})
}
Expand Down
4 changes: 2 additions & 2 deletions aws/resource_aws_ebs_snapshot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ func TestAccAWSEBSSnapshot_withDescription(t *testing.T) {
func TestAccAWSEBSSnapshot_withKms(t *testing.T) {
var v ec2.Snapshot
rName := fmt.Sprintf("tf-acc-ebs-snapshot-kms-%s", acctest.RandString(7))
kmsKeyResourceName := "aws_kms_key.test"
resourceName := "aws_ebs_snapshot.test"

resource.ParallelTest(t, resource.TestCase{
Expand All @@ -124,8 +125,7 @@ func TestAccAWSEBSSnapshot_withKms(t *testing.T) {
Config: testAccAwsEbsSnapshotConfigWithKms(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckSnapshotExists(resourceName, &v),
resource.TestMatchResourceAttr(resourceName, "kms_key_id",
regexp.MustCompile(`^arn:aws:kms:[a-z]{2}-[a-z]+-\d{1}:[0-9]{12}:key/[a-z0-9-]{36}$`)),
resource.TestCheckResourceAttrPair(resourceName, "kms_key_id", kmsKeyResourceName, "arn"),
),
},
{
Expand Down
2 changes: 1 addition & 1 deletion aws/resource_aws_fsx_windows_file_system_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func TestAccAWSFsxWindowsFileSystem_basic(t *testing.T) {
resource.TestCheckResourceAttr(resourceName, "copy_tags_to_backups", "false"),
resource.TestMatchResourceAttr(resourceName, "daily_automatic_backup_start_time", regexp.MustCompile(`^\d\d:\d\d$`)),
resource.TestMatchResourceAttr(resourceName, "dns_name", regexp.MustCompile(`fs-.+\..+`)),
resource.TestMatchResourceAttr(resourceName, "kms_key_id", regexp.MustCompile(`^arn:`)),
testAccMatchResourceAttrRegionalARN(resourceName, "kms_key_id", "kms", regexp.MustCompile(`key/.+`)),
resource.TestCheckResourceAttr(resourceName, "network_interface_ids.#", "1"),
testAccCheckResourceAttrAccountID(resourceName, "owner_id"),
resource.TestCheckResourceAttr(resourceName, "security_group_ids.#", "0"),
Expand Down
4 changes: 2 additions & 2 deletions aws/resource_aws_rds_cluster_instance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ func TestAccAWSRDSClusterInstance_generatedName(t *testing.T) {

func TestAccAWSRDSClusterInstance_kmsKey(t *testing.T) {
var v rds.DBInstance
keyRegex := regexp.MustCompile("^arn:aws:kms:")
kmsKeyResourceName := "aws_kms_key.foo"
resourceName := "aws_rds_cluster_instance.cluster_instances"

resource.ParallelTest(t, resource.TestCase{
Expand All @@ -204,7 +204,7 @@ func TestAccAWSRDSClusterInstance_kmsKey(t *testing.T) {
Config: testAccAWSClusterInstanceConfigKmsKey(acctest.RandInt()),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSClusterInstanceExists(resourceName, &v),
resource.TestMatchResourceAttr(resourceName, "kms_key_id", keyRegex),
resource.TestCheckResourceAttrPair(resourceName, "kms_key_id", kmsKeyResourceName, "arn"),
),
},
{
Expand Down
6 changes: 3 additions & 3 deletions awsproviderlint/passes/AWSAT001/AWSAT001.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,16 +70,16 @@ func run(pass *analysis.Pass) (interface{}, error) {
}

func AttributeNameAppearsArnRelated(attributeName string) bool {
if attributeName == "arn" {
if attributeName == "arn" || attributeName == "kms_key_id" {
return true
}

if strings.HasSuffix(attributeName, "_arn") {
if strings.HasSuffix(attributeName, "_arn") || strings.HasSuffix(attributeName, "_kms_key_id") {
return true
}

// Handle flatmap nested attribute
if strings.HasSuffix(attributeName, ".arn") {
if strings.HasSuffix(attributeName, ".arn") || strings.HasSuffix(attributeName, ".kms_key_id") {
return true
}

Expand Down
20 changes: 20 additions & 0 deletions awsproviderlint/passes/AWSAT001/AWSAT001_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,21 +33,41 @@ func TestAttributeNameAppearsArnRelated(t *testing.T) {
AttributeName: "arn",
Expected: true,
},
{
Name: "equals kms_key_id",
AttributeName: "kms_key_id",
Expected: true,
},
{
Name: "arn suffix",
AttributeName: "some_arn",
Expected: true,
},
{
Name: "kms_key_id suffix",
AttributeName: "some_kms_key_id",
Expected: true,
},
{
Name: "nested attribute equals arn",
AttributeName: "config_block.0.arn",
Expected: true,
},
{
Name: "nested attribute equals kms_key_id",
AttributeName: "config_block.0.kms_key_id",
Expected: true,
},
{
Name: "nested attribute arn suffix",
AttributeName: "config_block.0.some_arn",
Expected: true,
},
{
Name: "nested attribute kms_key_id suffix",
AttributeName: "config_block.0.some_kms_key_id",
Expected: true,
},
}

for _, testCase := range testCases {
Expand Down

0 comments on commit 3f12b61

Please sign in to comment.