diff --git a/.changelog/18580.txt b/.changelog/18580.txt new file mode 100644 index 000000000000..1b72b5724ff8 --- /dev/null +++ b/.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 +``` diff --git a/aws/resource_aws_cloudhsm2_hsm.go b/aws/resource_aws_cloudhsm2_hsm.go index c5a936c0af7a..9455d4ae1dcd 100644 --- a/aws/resource_aws_cloudhsm2_hsm.go +++ b/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, diff --git a/aws/resource_aws_cloudhsm2_hsm_test.go b/aws/resource_aws_cloudhsm2_hsm_test.go index 6302fd19835c..e7f8d06d19c7 100644 --- a/aws/resource_aws_cloudhsm2_hsm_test.go +++ b/aws/resource_aws_cloudhsm2_hsm_test.go @@ -2,34 +2,39 @@ 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, }, @@ -37,13 +42,8 @@ func TestAccAWSCloudHsmV2Hsm_basic(t *testing.T) { }) } -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) }