Skip to content

Commit

Permalink
resource/aws_cloudhsm_v2_hsm: Prevent orphaned HSM Instances by addit…
Browse files Browse the repository at this point in the history 
…ionally matching on ENI identifier during lookup

Reference: #8648
Reference: #16796

Also implements the paginated function to prevent missed matches in large environments and tidies up the existing test.

Output from acceptance testing:

```
--- PASS: TestAccAWSCloudHsmV2Hsm_basic (856.31s)
```
  • Loading branch information
@bflad
bflad committed Apr 6, 2021
1 parent e06ee9e commit e6568e8
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 66 deletions.
3 changes: 3 additions & 0 deletions .changelog/pending.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
resource/aws_cloudhsm_v2_hsm: Prevent orphaned HSM Instances by additionally matching on ENI identifier during lookup
```
79 changes: 56 additions & 23 deletions aws/resource_aws_cloudhsm2_hsm.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,37 +77,56 @@ func resourceAwsCloudHsmV2HsmImport(
return []*schema.ResourceData{d}, nil
}

func describeHsm(conn *cloudhsmv2.CloudHSMV2, hsmId string) (*cloudhsmv2.Hsm, error) {
out, err := conn.DescribeClusters(&cloudhsmv2.DescribeClustersInput{})
if err != nil {
log.Printf("[WARN] Error on descibing CloudHSM v2 Cluster: %s", err)
return nil, err
}
func describeHsm(conn *cloudhsmv2.CloudHSMV2, hsmID string, eniID string) (*cloudhsmv2.Hsm, error) {
input := &cloudhsmv2.DescribeClustersInput{}

var result *cloudhsmv2.Hsm

var hsm *cloudhsmv2.Hsm
err := conn.DescribeClustersPages(input, func(page *cloudhsmv2.DescribeClustersOutput, lastPage bool) bool {
if page == nil {
return !lastPage
}

for _, cluster := range page.Clusters {
if cluster == nil {
continue
}

for _, c := range out.Clusters {
for _, h := range c.Hsms {
if aws.StringValue(h.HsmId) == hsmId {
hsm = h
break
for _, hsm := range cluster.Hsms {
if hsm == nil {
continue
}

// CloudHSMv2 HSM instances can be recreated, but the ENI ID will
// remain consistent. Without this ENI matching, HSM instances
// instances can become orphaned.
if aws.StringValue(hsm.HsmId) == hsmID || aws.StringValue(hsm.EniId) == eniID {
result = hsm
return false
}
}
}

return !lastPage
})

if err != nil {
return nil, err
}

return hsm, nil
return result, nil
}

func resourceAwsCloudHsmV2HsmRefreshFunc(conn *cloudhsmv2.CloudHSMV2, id string) resource.StateRefreshFunc {
return func() (interface{}, string, error) {
hsm, err := describeHsm(conn, id)
hsm, err := describeHsm(conn, id, "")

if hsm == nil {
return 42, "destroyed", nil
if err != nil {
return nil, "", err
}

if hsm.State != nil {
log.Printf("[DEBUG] CloudHSMv2 Cluster status (%s): %s", id, *hsm.State)
if hsm == nil {
return nil, "", nil
}

return hsm, aws.StringValue(hsm.State), err
Expand Down Expand Up @@ -180,13 +199,27 @@ func resourceAwsCloudHsmV2HsmCreate(d *schema.ResourceData, meta interface{}) er
}

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

hsm, err := describeHsm(meta.(*AWSClient).cloudhsmv2conn, d.Id())
hsm, err := describeHsm(conn, d.Id(), d.Get("hsm_eni_id").(string))

if err != nil {
return fmt.Errorf("error reading CloudHSMv2 HSM (%s): %w", d.Id(), err)
}

if hsm == nil {
log.Printf("[WARN] CloudHSMv2 HSM (%s) not found", d.Id())
if d.IsNewResource() {
return fmt.Errorf("error reading CloudHSMv2 HSM (%s): not found after creation", d.Id())
}

log.Printf("[WARN] CloudHSMv2 HSM (%s) not found, removing from state", d.Id())
d.SetId("")
return err
return nil
}

// When matched by ENI ID, the ID should updated.
if aws.StringValue(hsm.HsmId) != d.Id() {
d.SetId(aws.StringValue(hsm.HsmId))
}

log.Printf("[INFO] Reading CloudHSMv2 HSM Information: %s", d.Id())
Expand Down Expand Up @@ -240,7 +273,7 @@ func resourceAwsCloudHsmV2HsmDelete(d *schema.ResourceData, meta interface{}) er

func waitForCloudhsmv2HsmActive(conn *cloudhsmv2.CloudHSMV2, id string, timeout time.Duration) error {
stateConf := &resource.StateChangeConf{
Pending: []string{cloudhsmv2.HsmStateCreateInProgress, "destroyed"},
Pending: []string{cloudhsmv2.HsmStateCreateInProgress},
Target: []string{cloudhsmv2.HsmStateActive},
Refresh: resourceAwsCloudHsmV2HsmRefreshFunc(conn, id),
Timeout: timeout,
Expand All @@ -256,7 +289,7 @@ func waitForCloudhsmv2HsmActive(conn *cloudhsmv2.CloudHSMV2, id string, timeout
func waitForCloudhsmv2HsmDeletion(conn *cloudhsmv2.CloudHSMV2, id string, timeout time.Duration) error {
stateConf := &resource.StateChangeConf{
Pending: []string{cloudhsmv2.HsmStateDeleteInProgress},
Target: []string{"destroyed"},
Target: []string{},
Refresh: resourceAwsCloudHsmV2HsmRefreshFunc(conn, id),
Timeout: timeout,
MinTimeout: 30 * time.Second,
Expand Down
74 changes: 31 additions & 43 deletions aws/resource_aws_cloudhsm2_hsm_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,48 +2,48 @@ package aws

import (
"fmt"
"regexp"
"testing"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/cloudhsmv2"
"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 TestAccAWSCloudHsmV2Hsm_basic(t *testing.T) {
resource.Test(t, resource.TestCase{
resourceName := "aws_cloudhsm_v2_hsm.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
ErrorCheck: testAccErrorCheck(t, cloudhsmv2.EndpointsID),
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSCloudHsmV2HsmDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSCloudHsmV2Hsm(),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSCloudHsmV2HsmExists("aws_cloudhsm_v2_hsm.hsm"),
resource.TestCheckResourceAttrSet("aws_cloudhsm_v2_hsm.hsm", "hsm_id"),
resource.TestCheckResourceAttrSet("aws_cloudhsm_v2_hsm.hsm", "hsm_state"),
resource.TestCheckResourceAttrSet("aws_cloudhsm_v2_hsm.hsm", "hsm_eni_id"),
resource.TestCheckResourceAttrSet("aws_cloudhsm_v2_hsm.hsm", "ip_address"),
Config: testAccAWSCloudHsmV2HsmConfig(),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckAWSCloudHsmV2HsmExists(resourceName),
resource.TestCheckResourceAttrPair(resourceName, "availability_zone", "aws_subnet.test.0", "availability_zone"),
resource.TestCheckResourceAttrPair(resourceName, "cluster_id", "aws_cloudhsm_v2_cluster.test", "id"),
resource.TestMatchResourceAttr(resourceName, "hsm_eni_id", regexp.MustCompile(`^eni-.+`)),
resource.TestMatchResourceAttr(resourceName, "hsm_id", regexp.MustCompile(`^hsm-.+`)),
resource.TestCheckResourceAttr(resourceName, "hsm_state", cloudhsmv2.HsmStateActive),
resource.TestCheckResourceAttrSet(resourceName, "ip_address"),
resource.TestCheckResourceAttrPair(resourceName, "subnet_id", "aws_subnet.test.0", "id"),
),
},
{
ResourceName: "aws_cloudhsm_v2_hsm.hsm",
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
},
})
}

func testAccAWSCloudHsmV2Hsm() string {
return fmt.Sprintf(`
variable "subnets" {
default = ["10.0.1.0/24", "10.0.2.0/24"]
type = list(string)
}
func testAccAWSCloudHsmV2HsmConfig() string {
return `
data "aws_availability_zones" "available" {
state = "available"
Expand All @@ -53,40 +53,28 @@ data "aws_availability_zones" "available" {
}
}
resource "aws_vpc" "cloudhsm_v2_test_vpc" {
resource "aws_vpc" "test" {
cidr_block = "10.0.0.0/16"
tags = {
Name = "terraform-testacc-aws_cloudhsm_v2_hsm-resource-basic"
}
}
resource "aws_subnet" "cloudhsm_v2_test_subnets" {
count = 2
vpc_id = aws_vpc.cloudhsm_v2_test_vpc.id
cidr_block = element(var.subnets, count.index)
map_public_ip_on_launch = false
availability_zone = element(data.aws_availability_zones.available.names, count.index)
resource "aws_subnet" "test" {
count = 2
tags = {
Name = "tf-acc-aws_cloudhsm_v2_hsm-resource-basic"
}
availability_zone = element(data.aws_availability_zones.available.names, count.index)
cidr_block = cidrsubnet(aws_vpc.test.cidr_block, 8, count.index)
vpc_id = aws_vpc.test.id
}
resource "aws_cloudhsm_v2_cluster" "cloudhsm_v2_cluster" {
resource "aws_cloudhsm_v2_cluster" "test" {
hsm_type = "hsm1.medium"
subnet_ids = [aws_subnet.cloudhsm_v2_test_subnets[0].id, aws_subnet.cloudhsm_v2_test_subnets[1].id]
tags = {
Name = "tf-acc-aws_cloudhsm_v2_hsm-resource-basic-%d"
}
subnet_ids = aws_subnet.test[*].id
}
resource "aws_cloudhsm_v2_hsm" "hsm" {
subnet_id = aws_subnet.cloudhsm_v2_test_subnets[0].id
cluster_id = aws_cloudhsm_v2_cluster.cloudhsm_v2_cluster.cluster_id
resource "aws_cloudhsm_v2_hsm" "test" {
cluster_id = aws_cloudhsm_v2_cluster.test.cluster_id
subnet_id = aws_subnet.test[0].id
}
`, acctest.RandInt())
`
}

func testAccCheckAWSCloudHsmV2HsmDestroy(s *terraform.State) error {
Expand All @@ -97,7 +85,7 @@ func testAccCheckAWSCloudHsmV2HsmDestroy(s *terraform.State) error {
continue
}

hsm, err := describeHsm(conn, rs.Primary.ID)
hsm, err := describeHsm(conn, rs.Primary.ID, rs.Primary.Attributes["hsm_eni_id"])

if err != nil {
return err
Expand All @@ -120,7 +108,7 @@ func testAccCheckAWSCloudHsmV2HsmExists(name string) resource.TestCheckFunc {
return fmt.Errorf("Not found: %s", name)
}

_, err := describeHsm(conn, it.Primary.ID)
_, err := describeHsm(conn, it.Primary.ID, it.Primary.Attributes["hsm_eni_id"])
if err != nil {
return fmt.Errorf("CloudHSM cluster not found: %s", err)
}
Expand Down

0 comments on commit e6568e8

Please sign in to comment.