resource/aws_cloudhsm_v2_hsm: Prevent orphaned HSM Instances by additionally matching on ENI identifier during lookup

.changelog/18580.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+resource/aws_cloudhsm_v2_hsm: Prevent orphaned HSM Instances by additionally matching on ENI identifier during lookup
+```

aws/resource_aws_cloudhsm2_hsm.go

@@ -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
@@ -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())
@@ -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,
@@ -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,

aws/resource_aws_cloudhsm2_hsm_test.go

@@ -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"
 
@@ -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 {
@@ -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
@@ -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)
  }