diff --git a/.changelog/20688.txt b/.changelog/20688.txt new file mode 100644 index 00000000000..b72ef409a1f --- /dev/null +++ b/.changelog/20688.txt @@ -0,0 +1,7 @@ +```release-note:bug +resource/aws_ec2_client_vpn_authorization_rule: Don't raise an error when `InvalidClientVpnEndpointId.NotFound` is returned during refresh +``` + +```release-note:enhancement +resource/aws_ec2_client_vpn_authorization_rule: Configurable Create and Delete timeouts +``` \ No newline at end of file diff --git a/internal/service/ec2/client_vpn_authorization_rule.go b/internal/service/ec2/client_vpn_authorization_rule.go index 9abfe6978b4..bd64b9f639d 100644 --- a/internal/service/ec2/client_vpn_authorization_rule.go +++ b/internal/service/ec2/client_vpn_authorization_rule.go @@ -3,12 +3,14 @@ package ec2 import ( "fmt" "log" + "strings" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" "github.com/hashicorp/aws-sdk-go-base/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-provider-aws/internal/conns" + "github.com/hashicorp/terraform-provider-aws/internal/tfresource" "github.com/hashicorp/terraform-provider-aws/internal/verify" ) @@ -18,21 +20,15 @@ func ResourceClientVPNAuthorizationRule() *schema.Resource { Read: resourceClientVPNAuthorizationRuleRead, Delete: resourceClientVPNAuthorizationRuleDelete, Importer: &schema.ResourceImporter{ - State: resourceClientVPNAuthorizationRuleImport, + State: schema.ImportStatePassthrough, + }, + + Timeouts: &schema.ResourceTimeout{ + Create: schema.DefaultTimeout(ClientVPNAuthorizationRuleCreatedTimeout), + Delete: schema.DefaultTimeout(ClientVPNAuthorizationRuleDeletedTimeout), }, Schema: map[string]*schema.Schema{ - "client_vpn_endpoint_id": { - Type: schema.TypeString, - Required: true, - ForceNew: true, - }, - "target_network_cidr": { - Type: schema.TypeString, - Required: true, - ForceNew: true, - ValidateFunc: verify.ValidCIDRNetworkAddress, - }, "access_group_id": { Type: schema.TypeString, Optional: true, @@ -45,11 +41,22 @@ func ResourceClientVPNAuthorizationRule() *schema.Resource { ForceNew: true, ExactlyOneOf: []string{"access_group_id", "authorize_all_groups"}, }, + "client_vpn_endpoint_id": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + }, "description": { Type: schema.TypeString, Optional: true, ForceNew: true, }, + "target_network_cidr": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + ValidateFunc: verify.ValidCIDRNetworkAddress, + }, }, } } @@ -58,11 +65,11 @@ func resourceClientVPNAuthorizationRuleCreate(d *schema.ResourceData, meta inter conn := meta.(*conns.AWSClient).EC2Conn endpointID := d.Get("client_vpn_endpoint_id").(string) - targetNetworkCidr := d.Get("target_network_cidr").(string) + targetNetworkCIDR := d.Get("target_network_cidr").(string) input := &ec2.AuthorizeClientVpnIngressInput{ ClientVpnEndpointId: aws.String(endpointID), - TargetNetworkCidr: aws.String(targetNetworkCidr), + TargetNetworkCidr: aws.String(targetNetworkCIDR), } var accessGroupID string @@ -79,54 +86,50 @@ func resourceClientVPNAuthorizationRuleCreate(d *schema.ResourceData, meta inter input.Description = aws.String(v.(string)) } - id := ClientVPNAuthorizationRuleCreateID(endpointID, targetNetworkCidr, accessGroupID) + id := ClientVPNAuthorizationRuleCreateResourceID(endpointID, targetNetworkCIDR, accessGroupID) - log.Printf("[DEBUG] Creating Client VPN authorization rule: %#v", input) + log.Printf("[DEBUG] Creating EC2 Client VPN Authorization Rule: %s", input) _, err := conn.AuthorizeClientVpnIngress(input) - if err != nil { - return fmt.Errorf("error creating Client VPN authorization rule %q: %w", id, err) - } - _, err = WaitClientVPNAuthorizationRuleAuthorized(conn, id) if err != nil { - return fmt.Errorf("error waiting for Client VPN authorization rule %q to be active: %w", id, err) + return fmt.Errorf("error authorizing EC2 Client VPN Authorization Rule (%s): %w", id, err) } d.SetId(id) + if _, err := WaitClientVPNAuthorizationRuleCreated(conn, endpointID, targetNetworkCIDR, accessGroupID, d.Timeout(schema.TimeoutCreate)); err != nil { + return fmt.Errorf("error waiting for EC2 Client VPN Authorization Rule (%s) create: %w", d.Id(), err) + } + return resourceClientVPNAuthorizationRuleRead(d, meta) } func resourceClientVPNAuthorizationRuleRead(d *schema.ResourceData, meta interface{}) error { conn := meta.(*conns.AWSClient).EC2Conn - result, err := FindClientVPNAuthorizationRule(conn, - d.Get("client_vpn_endpoint_id").(string), - d.Get("target_network_cidr").(string), - d.Get("access_group_id").(string), - ) + endpointID, targetNetworkCIDR, accessGroupID, err := ClientVPNAuthorizationRuleParseResourceID(d.Id()) - if tfawserr.ErrMessageContains(err, ErrCodeInvalidClientVpnAuthorizationRuleNotFound, "") { - log.Printf("[WARN] EC2 Client VPN authorization rule (%s) not found, removing from state", d.Id()) - d.SetId("") - return nil - } if err != nil { - return fmt.Errorf("error reading Client VPN authorization rule: %w", err) + return err } - if result == nil || len(result.AuthorizationRules) == 0 || result.AuthorizationRules[0] == nil { - log.Printf("[WARN] EC2 Client VPN authorization rule (%s) not found, removing from state", d.Id()) + rule, err := FindClientVPNAuthorizationRuleByEndpointIDTargetNetworkCIDRAndGroupID(conn, endpointID, targetNetworkCIDR, accessGroupID) + + if !d.IsNewResource() && tfresource.NotFound(err) { + log.Printf("[WARN] EC2 Client VPN Authorization Rule (%s) not found, removing from state", d.Id()) d.SetId("") return nil } - rule := result.AuthorizationRules[0] - d.Set("client_vpn_endpoint_id", rule.ClientVpnEndpointId) - d.Set("target_network_cidr", rule.DestinationCidr) + if err != nil { + return fmt.Errorf("error reading EC2 Client VPN Authorization Rule (%s): %w", d.Id(), err) + } + d.Set("access_group_id", rule.GroupId) d.Set("authorize_all_groups", rule.AccessAll) + d.Set("client_vpn_endpoint_id", rule.ClientVpnEndpointId) d.Set("description", rule.Description) + d.Set("target_network_cidr", rule.DestinationCidr) return nil } @@ -134,51 +137,61 @@ func resourceClientVPNAuthorizationRuleRead(d *schema.ResourceData, meta interfa func resourceClientVPNAuthorizationRuleDelete(d *schema.ResourceData, meta interface{}) error { conn := meta.(*conns.AWSClient).EC2Conn + endpointID, targetNetworkCIDR, accessGroupID, err := ClientVPNAuthorizationRuleParseResourceID(d.Id()) + + if err != nil { + return err + } + input := &ec2.RevokeClientVpnIngressInput{ - ClientVpnEndpointId: aws.String(d.Get("client_vpn_endpoint_id").(string)), - TargetNetworkCidr: aws.String(d.Get("target_network_cidr").(string)), + ClientVpnEndpointId: aws.String(endpointID), RevokeAllGroups: aws.Bool(d.Get("authorize_all_groups").(bool)), + TargetNetworkCidr: aws.String(targetNetworkCIDR), } - if v, ok := d.GetOk("access_group_id"); ok { - input.AccessGroupId = aws.String(v.(string)) + if accessGroupID != "" { + input.AccessGroupId = aws.String(accessGroupID) + } + + log.Printf("[DEBUG] Deleting EC2 Client VPN Authorization Rule: %s", d.Id()) + _, err = conn.RevokeClientVpnIngress(input) + + if tfawserr.ErrCodeEquals(err, ErrCodeInvalidClientVpnEndpointIdNotFound, ErrCodeInvalidClientVpnAuthorizationRuleNotFound) { + return nil } - log.Printf("[DEBUG] Revoking Client VPN authorization rule %q", d.Id()) - err := deleteClientVpnAuthorizationRule(conn, input) if err != nil { - return fmt.Errorf("error revoking Client VPN authorization rule %q: %w", d.Id(), err) + return fmt.Errorf("error revoking EC2 Client VPN Authorization Rule (%s): %w", d.Id(), err) + } + + if _, err := WaitClientVPNAuthorizationRuleDeleted(conn, endpointID, targetNetworkCIDR, accessGroupID, d.Timeout(schema.TimeoutDelete)); err != nil { + return fmt.Errorf("error waiting for EC2 Client VPN Authorization Rule (%s) delete: %w", d.Id(), err) } return nil } -func resourceClientVPNAuthorizationRuleImport(d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) { - endpointID, targetNetworkCidr, accessGroupID, err := ClientVPNAuthorizationRuleParseID(d.Id()) - if err != nil { - return nil, err +const clientVPNAuthorizationRuleIDSeparator = "," + +func ClientVPNAuthorizationRuleCreateResourceID(endpointID, targetNetworkCIDR, accessGroupID string) string { + parts := []string{endpointID, targetNetworkCIDR} + if accessGroupID != "" { + parts = append(parts, accessGroupID) } + id := strings.Join(parts, clientVPNAuthorizationRuleIDSeparator) - d.Set("client_vpn_endpoint_id", endpointID) - d.Set("target_network_cidr", targetNetworkCidr) - d.Set("access_group_id", accessGroupID) - return []*schema.ResourceData{d}, nil + return id } -func deleteClientVpnAuthorizationRule(conn *ec2.EC2, input *ec2.RevokeClientVpnIngressInput) error { - id := ClientVPNAuthorizationRuleCreateID( - aws.StringValue(input.ClientVpnEndpointId), - aws.StringValue(input.TargetNetworkCidr), - aws.StringValue(input.AccessGroupId)) +func ClientVPNAuthorizationRuleParseResourceID(id string) (string, string, string, error) { + parts := strings.Split(id, clientVPNAuthorizationRuleIDSeparator) - _, err := conn.RevokeClientVpnIngress(input) - if tfawserr.ErrMessageContains(err, ErrCodeInvalidClientVpnAuthorizationRuleNotFound, "") { - return nil - } - if err != nil { - return err + if len(parts) == 2 && parts[0] != "" && parts[1] != "" { + return parts[0], parts[1], "", nil } - _, err = WaitClientVPNAuthorizationRuleRevoked(conn, id) + if len(parts) == 3 && parts[0] != "" && parts[1] != "" && parts[2] != "" { + return parts[0], parts[1], parts[2], nil + } - return err + return "", "", "", fmt.Errorf("unexpected format for ID (%[1]s), expected endpoint-id%[2]starget-network-cidr or endpoint-id%[2]starget-network-cidr%[2]sgroup-id", id, clientVPNAuthorizationRuleIDSeparator) } diff --git a/internal/service/ec2/client_vpn_authorization_rule_test.go b/internal/service/ec2/client_vpn_authorization_rule_test.go index 1249d14533c..ed592632304 100644 --- a/internal/service/ec2/client_vpn_authorization_rule_test.go +++ b/internal/service/ec2/client_vpn_authorization_rule_test.go @@ -6,18 +6,18 @@ import ( "testing" "github.com/aws/aws-sdk-go/service/ec2" - "github.com/hashicorp/aws-sdk-go-base/tfawserr" sdkacctest "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" "github.com/hashicorp/terraform-provider-aws/internal/acctest" "github.com/hashicorp/terraform-provider-aws/internal/conns" tfec2 "github.com/hashicorp/terraform-provider-aws/internal/service/ec2" + "github.com/hashicorp/terraform-provider-aws/internal/tfresource" ) func testAccClientVPNAuthorizationRule_basic(t *testing.T) { var v ec2.AuthorizationRule - rStr := sdkacctest.RandString(5) + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_ec2_client_vpn_authorization_rule.test" subnetResourceName := "aws_subnet.test.0" @@ -28,7 +28,7 @@ func testAccClientVPNAuthorizationRule_basic(t *testing.T) { CheckDestroy: testAccCheckClientVPNAuthorizationRuleDestroy, Steps: []resource.TestStep{ { - Config: testAccEc2ClientVpnAuthorizationRuleConfigBasic(rStr), + Config: testAccEc2ClientVpnAuthorizationRuleConfigBasic(rName), Check: resource.ComposeTestCheckFunc( testAccCheckClientVPNAuthorizationRuleExists(resourceName, &v), resource.TestCheckResourceAttrPair(resourceName, "target_network_cidr", subnetResourceName, "cidr_block"), @@ -45,9 +45,55 @@ func testAccClientVPNAuthorizationRule_basic(t *testing.T) { }) } +func testAccClientVPNAuthorizationRule_disappears(t *testing.T) { + var v ec2.AuthorizationRule + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_ec2_client_vpn_authorization_rule.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheckClientVPNSyncronize(t); acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckClientVPNAuthorizationRuleDestroy, + Steps: []resource.TestStep{ + { + Config: testAccEc2ClientVpnAuthorizationRuleConfigBasic(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckClientVPNAuthorizationRuleExists(resourceName, &v), + acctest.CheckResourceDisappears(acctest.Provider, tfec2.ResourceClientVPNAuthorizationRule(), resourceName), + ), + ExpectNonEmptyPlan: true, + }, + }, + }) +} + +func testAccClientVPNAuthorizationRule_Disappears_endpoint(t *testing.T) { + var v ec2.AuthorizationRule + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_ec2_client_vpn_authorization_rule.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheckClientVPNSyncronize(t); acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckClientVPNAuthorizationRuleDestroy, + Steps: []resource.TestStep{ + { + Config: testAccEc2ClientVpnAuthorizationRuleConfigBasic(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckClientVPNAuthorizationRuleExists(resourceName, &v), + acctest.CheckResourceDisappears(acctest.Provider, tfec2.ResourceClientVPNEndpoint(), "aws_ec2_client_vpn_endpoint.test"), + ), + ExpectNonEmptyPlan: true, + }, + }, + }) +} + func testAccClientVPNAuthorizationRule_groups(t *testing.T) { - var v1, v2, v3, v4 ec2.AuthorizationRule - rStr := sdkacctest.RandString(5) + var v ec2.AuthorizationRule + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource1Name := "aws_ec2_client_vpn_authorization_rule.test1" resource2Name := "aws_ec2_client_vpn_authorization_rule.test2" subnetResourceName := "aws_subnet.test.0" @@ -73,9 +119,9 @@ func testAccClientVPNAuthorizationRule_groups(t *testing.T) { CheckDestroy: testAccCheckClientVPNAuthorizationRuleDestroy, Steps: []resource.TestStep{ { - Config: testAccEc2ClientVpnAuthorizationRuleConfigGroups(rStr, groups1), + Config: testAccEc2ClientVpnAuthorizationRuleConfigGroups(rName, groups1), Check: resource.ComposeTestCheckFunc( - testAccCheckClientVPNAuthorizationRuleExists(resource1Name, &v1), + testAccCheckClientVPNAuthorizationRuleExists(resource1Name, &v), resource.TestCheckResourceAttrPair(resource1Name, "target_network_cidr", subnetResourceName, "cidr_block"), resource.TestCheckResourceAttr(resource1Name, "authorize_all_groups", "false"), resource.TestCheckResourceAttr(resource1Name, "access_group_id", group1Name), @@ -87,14 +133,14 @@ func testAccClientVPNAuthorizationRule_groups(t *testing.T) { ImportStateVerify: true, }, { - Config: testAccEc2ClientVpnAuthorizationRuleConfigGroups(rStr, groups2), + Config: testAccEc2ClientVpnAuthorizationRuleConfigGroups(rName, groups2), Check: resource.ComposeTestCheckFunc( - testAccCheckClientVPNAuthorizationRuleExists(resource1Name, &v2), + testAccCheckClientVPNAuthorizationRuleExists(resource1Name, &v), resource.TestCheckResourceAttrPair(resource1Name, "target_network_cidr", subnetResourceName, "cidr_block"), resource.TestCheckResourceAttr(resource1Name, "authorize_all_groups", "false"), resource.TestCheckResourceAttr(resource1Name, "access_group_id", group1Name), - testAccCheckClientVPNAuthorizationRuleExists(resource2Name, &v3), + testAccCheckClientVPNAuthorizationRuleExists(resource2Name, &v), resource.TestCheckResourceAttrPair(resource2Name, "target_network_cidr", subnetResourceName, "cidr_block"), resource.TestCheckResourceAttr(resource2Name, "authorize_all_groups", "false"), resource.TestCheckResourceAttr(resource2Name, "access_group_id", group2Name), @@ -106,9 +152,9 @@ func testAccClientVPNAuthorizationRule_groups(t *testing.T) { ImportStateVerify: true, }, { - Config: testAccEc2ClientVpnAuthorizationRuleConfigGroups(rStr, groups3), + Config: testAccEc2ClientVpnAuthorizationRuleConfigGroups(rName, groups3), Check: resource.ComposeTestCheckFunc( - testAccCheckClientVPNAuthorizationRuleExists(resource2Name, &v4), + testAccCheckClientVPNAuthorizationRuleExists(resource2Name, &v), resource.TestCheckResourceAttrPair(resource2Name, "target_network_cidr", subnetResourceName, "cidr_block"), resource.TestCheckResourceAttr(resource2Name, "authorize_all_groups", "false"), resource.TestCheckResourceAttr(resource2Name, "access_group_id", group2Name), @@ -118,9 +164,9 @@ func testAccClientVPNAuthorizationRule_groups(t *testing.T) { }) } -func testAccClientVPNAuthorizationRule_Subnets(t *testing.T) { - var v1, v2, v3 ec2.AuthorizationRule - rStr := sdkacctest.RandString(5) +func testAccClientVPNAuthorizationRule_subnets(t *testing.T) { + var v ec2.AuthorizationRule + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource1Name := "aws_ec2_client_vpn_authorization_rule.test1" resource2Name := "aws_ec2_client_vpn_authorization_rule.test2" @@ -144,14 +190,14 @@ func testAccClientVPNAuthorizationRule_Subnets(t *testing.T) { CheckDestroy: testAccCheckClientVPNAuthorizationRuleDestroy, Steps: []resource.TestStep{ { - Config: testAccEc2ClientVpnAuthorizationRuleConfigSubnets(rStr, subnetCount, case1), + Config: testAccEc2ClientVpnAuthorizationRuleConfigSubnets(rName, subnetCount, case1), Check: resource.ComposeTestCheckFunc( - testAccCheckClientVPNAuthorizationRuleExists(resource1Name, &v1), + testAccCheckClientVPNAuthorizationRuleExists(resource1Name, &v), resource.TestCheckResourceAttrPair(resource1Name, "target_network_cidr", fmt.Sprintf("aws_subnet.test.%d", subnetIndex1), "cidr_block"), resource.TestCheckResourceAttr(resource1Name, "authorize_all_groups", "true"), resource.TestCheckResourceAttr(resource1Name, "access_group_id", ""), - testAccCheckClientVPNAuthorizationRuleExists(resource2Name, &v2), + testAccCheckClientVPNAuthorizationRuleExists(resource2Name, &v), resource.TestCheckResourceAttrPair(resource2Name, "target_network_cidr", fmt.Sprintf("aws_subnet.test.%d", subnetIndex2), "cidr_block"), resource.TestCheckResourceAttr(resource2Name, "authorize_all_groups", "true"), resource.TestCheckResourceAttr(resource2Name, "access_group_id", ""), @@ -163,9 +209,9 @@ func testAccClientVPNAuthorizationRule_Subnets(t *testing.T) { ImportStateVerify: true, }, { - Config: testAccEc2ClientVpnAuthorizationRuleConfigSubnets(rStr, subnetCount, case2), + Config: testAccEc2ClientVpnAuthorizationRuleConfigSubnets(rName, subnetCount, case2), Check: resource.ComposeTestCheckFunc( - testAccCheckClientVPNAuthorizationRuleExists(resource2Name, &v3), + testAccCheckClientVPNAuthorizationRuleExists(resource2Name, &v), resource.TestCheckResourceAttrPair(resource2Name, "target_network_cidr", fmt.Sprintf("aws_subnet.test.%d", subnetIndex2), "cidr_block"), resource.TestCheckResourceAttr(resource2Name, "authorize_all_groups", "true"), resource.TestCheckResourceAttr(resource2Name, "access_group_id", ""), @@ -175,29 +221,6 @@ func testAccClientVPNAuthorizationRule_Subnets(t *testing.T) { }) } -func testAccClientVPNAuthorizationRule_disappears(t *testing.T) { - var v ec2.AuthorizationRule - rStr := sdkacctest.RandString(5) - resourceName := "aws_ec2_client_vpn_authorization_rule.test" - - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { testAccPreCheckClientVPNSyncronize(t); acctest.PreCheck(t) }, - ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID), - Providers: acctest.Providers, - CheckDestroy: testAccCheckClientVPNAuthorizationRuleDestroy, - Steps: []resource.TestStep{ - { - Config: testAccEc2ClientVpnAuthorizationRuleConfigBasic(rStr), - Check: resource.ComposeTestCheckFunc( - testAccCheckClientVPNAuthorizationRuleExists(resourceName, &v), - acctest.CheckResourceDisappears(acctest.Provider, tfec2.ResourceClientVPNAuthorizationRule(), resourceName), - ), - ExpectNonEmptyPlan: true, - }, - }, - }) -} - func testAccCheckClientVPNAuthorizationRuleDestroy(s *terraform.State) error { conn := acctest.Provider.Meta().(*conns.AWSClient).EC2Conn @@ -206,20 +229,29 @@ func testAccCheckClientVPNAuthorizationRuleDestroy(s *terraform.State) error { continue } - _, err := tfec2.FindClientVPNAuthorizationRuleByID(conn, rs.Primary.ID) - if err == nil { - return fmt.Errorf("Client VPN authorization rule (%s) still exists", rs.Primary.ID) + endpointID, targetNetworkCIDR, accessGroupID, err := tfec2.ClientVPNAuthorizationRuleParseResourceID(rs.Primary.ID) + + if err != nil { + return err } - if tfawserr.ErrMessageContains(err, tfec2.ErrCodeInvalidClientVpnAuthorizationRuleNotFound, "") || tfawserr.ErrMessageContains(err, tfec2.ErrCodeInvalidClientVpnEndpointIdNotFound, "") { + + _, err = tfec2.FindClientVPNAuthorizationRuleByEndpointIDTargetNetworkCIDRAndGroupID(conn, endpointID, targetNetworkCIDR, accessGroupID) + + if tfresource.NotFound(err) { continue } - return err + + if err != nil { + return err + } + + return fmt.Errorf("EC2 Client VPN Authorization Rule %s still exists", rs.Primary.ID) } return nil } -func testAccCheckClientVPNAuthorizationRuleExists(name string, assoc *ec2.AuthorizationRule) resource.TestCheckFunc { +func testAccCheckClientVPNAuthorizationRuleExists(name string, v *ec2.AuthorizationRule) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[name] if !ok { @@ -227,25 +259,65 @@ func testAccCheckClientVPNAuthorizationRuleExists(name string, assoc *ec2.Author } if rs.Primary.ID == "" { - return fmt.Errorf("No ID is set") + return fmt.Errorf("No EC2 Client VPN Authorization Rule ID is set") } - conn := acctest.Provider.Meta().(*conns.AWSClient).EC2Conn + endpointID, targetNetworkCIDR, accessGroupID, err := tfec2.ClientVPNAuthorizationRuleParseResourceID(rs.Primary.ID) - result, err := tfec2.FindClientVPNAuthorizationRuleByID(conn, rs.Primary.ID) if err != nil { - return fmt.Errorf("error reading Client VPN authorization rule (%s): %w", rs.Primary.ID, err) + return err } - if result != nil || len(result.AuthorizationRules) == 1 || result.AuthorizationRules[0] != nil { - *assoc = *result.AuthorizationRules[0] - return nil + conn := acctest.Provider.Meta().(*conns.AWSClient).EC2Conn + + output, err := tfec2.FindClientVPNAuthorizationRuleByEndpointIDTargetNetworkCIDRAndGroupID(conn, endpointID, targetNetworkCIDR, accessGroupID) + + if err != nil { + return err } - return fmt.Errorf("Client VPN network association (%s) not found", rs.Primary.ID) + *v = *output + + return nil } } +func testAccEc2ClientVpnAuthorizationRuleVpcBase(rName string, subnetCount int) string { + return acctest.ConfigCompose(acctest.ConfigAvailableAZsNoOptInDefaultExclude(), fmt.Sprintf(` +resource "aws_vpc" "test" { + cidr_block = "10.1.0.0/16" + + tags = { + Name = %[1]q + } +} + +resource "aws_subnet" "test" { + count = %[2]d + availability_zone = 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 + map_public_ip_on_launch = true + + tags = { + Name = %[1]q + } +} +`, rName, subnetCount)) +} + +func testAccEc2ClientVpnAuthorizationRuleAcmCertificateBase() string { + key := acctest.TLSRSAPrivateKeyPEM(2048) + certificate := acctest.TLSRSAX509SelfSignedCertificatePEM(key, "example.com") + + return fmt.Sprintf(` +resource "aws_acm_certificate" "test" { + certificate_body = "%[1]s" + private_key = "%[2]s" +} +`, acctest.TLSPEMEscapeNewlines(certificate), acctest.TLSPEMEscapeNewlines(key)) +} + func testAccEc2ClientVpnAuthorizationRuleConfigBasic(rName string) string { return acctest.ConfigCompose( testAccEc2ClientVpnAuthorizationRuleVpcBase(rName, 1), @@ -258,7 +330,6 @@ resource "aws_ec2_client_vpn_authorization_rule" "test" { } resource "aws_ec2_client_vpn_endpoint" "test" { - description = "terraform-testacc-clientvpn-%[1]s" server_certificate_arn = aws_acm_certificate.test.arn client_cidr_block = "10.0.0.0/16" @@ -270,6 +341,10 @@ resource "aws_ec2_client_vpn_endpoint" "test" { connection_log_options { enabled = false } + + tags = { + Name = %[1]q + } } `, rName)) } @@ -292,7 +367,6 @@ resource "aws_ec2_client_vpn_authorization_rule" %[1]q { b.String(), fmt.Sprintf(` resource "aws_ec2_client_vpn_endpoint" "test" { - description = "terraform-testacc-clientvpn-%[1]s" server_certificate_arn = aws_acm_certificate.test.arn client_cidr_block = "10.0.0.0/16" @@ -304,6 +378,10 @@ resource "aws_ec2_client_vpn_endpoint" "test" { connection_log_options { enabled = false } + + tags = { + Name = %[1]q + } }`, rName)) } @@ -325,7 +403,6 @@ resource "aws_ec2_client_vpn_authorization_rule" %[1]q { b.String(), fmt.Sprintf(` resource "aws_ec2_client_vpn_endpoint" "test" { - description = "terraform-testacc-clientvpn-%[1]s" server_certificate_arn = aws_acm_certificate.test.arn client_cidr_block = "10.0.0.0/16" @@ -337,41 +414,9 @@ resource "aws_ec2_client_vpn_endpoint" "test" { connection_log_options { enabled = false } -}`, rName)) -} - -func testAccEc2ClientVpnAuthorizationRuleVpcBase(rName string, subnetCount int) string { - return acctest.ConfigCompose(acctest.ConfigAvailableAZsNoOptInDefaultExclude(), fmt.Sprintf(` -resource "aws_vpc" "test" { - cidr_block = "10.1.0.0/16" tags = { - Name = "terraform-testacc-subnet-%[1]s" + Name = %[1]q } -} - -resource "aws_subnet" "test" { - count = %[2]d - availability_zone = 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 - map_public_ip_on_launch = true - - tags = { - Name = "tf-acc-subnet-%[1]s" - } -} -`, rName, subnetCount)) -} - -func testAccEc2ClientVpnAuthorizationRuleAcmCertificateBase() string { - key := acctest.TLSRSAPrivateKeyPEM(2048) - certificate := acctest.TLSRSAX509SelfSignedCertificatePEM(key, "example.com") - - return fmt.Sprintf(` -resource "aws_acm_certificate" "test" { - certificate_body = "%[1]s" - private_key = "%[2]s" -} -`, acctest.TLSPEMEscapeNewlines(certificate), acctest.TLSPEMEscapeNewlines(key)) +}`, rName)) } diff --git a/internal/service/ec2/client_vpn_endpoint_test.go b/internal/service/ec2/client_vpn_endpoint_test.go index 143234062d1..646c95df9b2 100644 --- a/internal/service/ec2/client_vpn_endpoint_test.go +++ b/internal/service/ec2/client_vpn_endpoint_test.go @@ -47,10 +47,11 @@ func TestAccEC2ClientVPNEndpoint_serial(t *testing.T) { "selfServicePortal": testAccClientVPNEndpoint_selfServicePortal, }, "AuthorizationRule": { - "basic": testAccClientVPNAuthorizationRule_basic, - "groups": testAccClientVPNAuthorizationRule_groups, - "Subnets": testAccClientVPNAuthorizationRule_Subnets, - "disappears": testAccClientVPNAuthorizationRule_disappears, + "basic": testAccClientVPNAuthorizationRule_basic, + "groups": testAccClientVPNAuthorizationRule_groups, + "subnets": testAccClientVPNAuthorizationRule_subnets, + "disappears": testAccClientVPNAuthorizationRule_disappears, + "disappearsEndpoint": testAccClientVPNAuthorizationRule_Disappears_endpoint, }, "NetworkAssociation": { "basic": testAccClientVPNNetworkAssociation_basic, diff --git a/internal/service/ec2/find.go b/internal/service/ec2/find.go index 179ad17e666..478368cc830 100644 --- a/internal/service/ec2/find.go +++ b/internal/service/ec2/find.go @@ -123,30 +123,71 @@ func FindClientVPNEndpointClientConnectResponseOptionsByID(conn *ec2.EC2, id str return output.ClientConnectOptions, nil } -func FindClientVPNAuthorizationRule(conn *ec2.EC2, endpointID, targetNetworkCidr, accessGroupID string) (*ec2.DescribeClientVpnAuthorizationRulesOutput, error) { - filters := map[string]string{ - "destination-cidr": targetNetworkCidr, - } - if accessGroupID != "" { - filters["group-id"] = accessGroupID +func FindClientVPNAuthorizationRule(conn *ec2.EC2, input *ec2.DescribeClientVpnAuthorizationRulesInput) (*ec2.AuthorizationRule, error) { + output, err := FindClientVPNAuthorizationRules(conn, input) + + if err != nil { + return nil, err } - input := &ec2.DescribeClientVpnAuthorizationRulesInput{ - ClientVpnEndpointId: aws.String(endpointID), - Filters: BuildAttributeFilterList(filters), + if len(output) == 0 || output[0] == nil || output[0].Status == nil { + return nil, tfresource.NewEmptyResultError(input) } - return conn.DescribeClientVpnAuthorizationRules(input) + if count := len(output); count > 1 { + return nil, tfresource.NewTooManyResultsError(count, input) + } + return output[0], nil } -func FindClientVPNAuthorizationRuleByID(conn *ec2.EC2, authorizationRuleID string) (*ec2.DescribeClientVpnAuthorizationRulesOutput, error) { - endpointID, targetNetworkCidr, accessGroupID, err := ClientVPNAuthorizationRuleParseID(authorizationRuleID) +func FindClientVPNAuthorizationRules(conn *ec2.EC2, input *ec2.DescribeClientVpnAuthorizationRulesInput) ([]*ec2.AuthorizationRule, error) { + var output []*ec2.AuthorizationRule + + err := conn.DescribeClientVpnAuthorizationRulesPages(input, func(page *ec2.DescribeClientVpnAuthorizationRulesOutput, lastPage bool) bool { + if page == nil { + return !lastPage + } + + for _, v := range page.AuthorizationRules { + if v == nil { + continue + } + + output = append(output, v) + } + + return !lastPage + }) + + if tfawserr.ErrCodeEquals(err, ErrCodeInvalidClientVpnEndpointIdNotFound) { + return nil, &resource.NotFoundError{ + LastError: err, + LastRequest: input, + } + } + if err != nil { return nil, err } - return FindClientVPNAuthorizationRule(conn, endpointID, targetNetworkCidr, accessGroupID) + return output, nil +} + +func FindClientVPNAuthorizationRuleByEndpointIDTargetNetworkCIDRAndGroupID(conn *ec2.EC2, endpointID, targetNetworkCIDR, accessGroupID string) (*ec2.AuthorizationRule, error) { + filters := map[string]string{ + "destination-cidr": targetNetworkCIDR, + } + if accessGroupID != "" { + filters["group-id"] = accessGroupID + } + input := &ec2.DescribeClientVpnAuthorizationRulesInput{ + ClientVpnEndpointId: aws.String(endpointID), + Filters: BuildAttributeFilterList(filters), + } + + return FindClientVPNAuthorizationRule(conn, input) + } func FindClientVPNRoute(conn *ec2.EC2, endpointID, targetSubnetID, destinationCidr string) (*ec2.DescribeClientVpnRoutesOutput, error) { diff --git a/internal/service/ec2/id.go b/internal/service/ec2/id.go index 0c844c4d7d3..6fcadf045eb 100644 --- a/internal/service/ec2/id.go +++ b/internal/service/ec2/id.go @@ -7,49 +7,24 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/create" ) -const clientVpnAuthorizationRuleIDSeparator = "," - -func ClientVPNAuthorizationRuleCreateID(endpointID, targetNetworkCidr, accessGroupID string) string { - parts := []string{endpointID, targetNetworkCidr} - if accessGroupID != "" { - parts = append(parts, accessGroupID) - } - id := strings.Join(parts, clientVpnAuthorizationRuleIDSeparator) - return id -} - -func ClientVPNAuthorizationRuleParseID(id string) (string, string, string, error) { - parts := strings.Split(id, clientVpnAuthorizationRuleIDSeparator) - if len(parts) == 2 && parts[0] != "" && parts[1] != "" { - return parts[0], parts[1], "", nil - } - if len(parts) == 3 && parts[0] != "" && parts[1] != "" && parts[2] != "" { - return parts[0], parts[1], parts[2], nil - } - - return "", "", "", - fmt.Errorf("unexpected format for ID (%q), expected endpoint-id"+clientVpnAuthorizationRuleIDSeparator+ - "target-network-cidr or endpoint-id"+clientVpnAuthorizationRuleIDSeparator+"target-network-cidr"+ - clientVpnAuthorizationRuleIDSeparator+"group-id", id) -} - -const clientVpnNetworkAssociationIDSeparator = "," +const clientVPNNetworkAssociationIDSeparator = "," func ClientVPNNetworkAssociationCreateID(endpointID, associationID string) string { parts := []string{endpointID, associationID} - id := strings.Join(parts, clientVpnNetworkAssociationIDSeparator) + id := strings.Join(parts, clientVPNNetworkAssociationIDSeparator) + return id } func ClientVPNNetworkAssociationParseID(id string) (string, string, error) { - parts := strings.Split(id, clientVpnNetworkAssociationIDSeparator) + parts := strings.Split(id, clientVPNNetworkAssociationIDSeparator) + if len(parts) == 2 && parts[0] != "" && parts[1] != "" { return parts[0], parts[1], nil } return "", "", - fmt.Errorf("unexpected format for ID (%q), expected endpoint-id"+clientVpnNetworkAssociationIDSeparator+ - "association-id", id) + fmt.Errorf("unexpected format for ID (%[1]s), expected EndpointID%[2]sAssociationID", id, clientVPNNetworkAssociationIDSeparator) } const clientVpnRouteIDSeparator = "," diff --git a/internal/service/ec2/status.go b/internal/service/ec2/status.go index c79acfc5a5b..e05f16d307d 100644 --- a/internal/service/ec2/status.go +++ b/internal/service/ec2/status.go @@ -109,37 +109,19 @@ func StatusClientVPNEndpointClientConnectResponseOptionsState(conn *ec2.EC2, id } } -const ( - ClientVPNAuthorizationRuleStatusNotFound = "NotFound" - - ClientVPNAuthorizationRuleStatusUnknown = "Unknown" -) - -// StatusClientVPNAuthorizationRule fetches the Client VPN authorization rule and its Status -func StatusClientVPNAuthorizationRule(conn *ec2.EC2, authorizationRuleID string) resource.StateRefreshFunc { +func StatusClientVPNAuthorizationRule(conn *ec2.EC2, endpointID, targetNetworkCIDR, accessGroupID string) resource.StateRefreshFunc { return func() (interface{}, string, error) { - result, err := FindClientVPNAuthorizationRuleByID(conn, authorizationRuleID) - if tfawserr.ErrCodeEquals(err, ErrCodeInvalidClientVpnAuthorizationRuleNotFound) { - return nil, ClientVPNAuthorizationRuleStatusNotFound, nil - } - if err != nil { - return nil, ClientVPNAuthorizationRuleStatusUnknown, err - } - - if result == nil || len(result.AuthorizationRules) == 0 || result.AuthorizationRules[0] == nil { - return nil, ClientVPNAuthorizationRuleStatusNotFound, nil - } + output, err := FindClientVPNAuthorizationRuleByEndpointIDTargetNetworkCIDRAndGroupID(conn, endpointID, targetNetworkCIDR, accessGroupID) - if len(result.AuthorizationRules) > 1 { - return nil, ClientVPNAuthorizationRuleStatusUnknown, fmt.Errorf("internal error: found %d results for Client VPN authorization rule (%s) status, need 1", len(result.AuthorizationRules), authorizationRuleID) + if tfresource.NotFound(err) { + return nil, "", nil } - rule := result.AuthorizationRules[0] - if rule.Status == nil || rule.Status.Code == nil { - return rule, ClientVPNAuthorizationRuleStatusUnknown, nil + if err != nil { + return nil, "", err } - return rule, aws.StringValue(rule.Status.Code), nil + return output, aws.StringValue(output.Status.Code), nil } } diff --git a/internal/service/ec2/wait.go b/internal/service/ec2/wait.go index 490b2c0798c..862efe48d10 100644 --- a/internal/service/ec2/wait.go +++ b/internal/service/ec2/wait.go @@ -157,39 +157,42 @@ func WaitClientVPNEndpointClientConnectResponseOptionsUpdated(conn *ec2.EC2, id } const ( - ClientVPNAuthorizationRuleActiveTimeout = 10 * time.Minute - - ClientVPNAuthorizationRuleRevokedTimeout = 10 * time.Minute + ClientVPNAuthorizationRuleCreatedTimeout = 10 * time.Minute + ClientVPNAuthorizationRuleDeletedTimeout = 10 * time.Minute ) -func WaitClientVPNAuthorizationRuleAuthorized(conn *ec2.EC2, authorizationRuleID string) (*ec2.AuthorizationRule, error) { +func WaitClientVPNAuthorizationRuleCreated(conn *ec2.EC2, endpointID, targetNetworkCIDR, accessGroupID string, timeout time.Duration) (*ec2.AuthorizationRule, error) { stateConf := &resource.StateChangeConf{ Pending: []string{ec2.ClientVpnAuthorizationRuleStatusCodeAuthorizing}, Target: []string{ec2.ClientVpnAuthorizationRuleStatusCodeActive}, - Refresh: StatusClientVPNAuthorizationRule(conn, authorizationRuleID), - Timeout: ClientVPNAuthorizationRuleActiveTimeout, + Refresh: StatusClientVPNAuthorizationRule(conn, endpointID, targetNetworkCIDR, accessGroupID), + Timeout: timeout, } outputRaw, err := stateConf.WaitForState() if output, ok := outputRaw.(*ec2.AuthorizationRule); ok { + tfresource.SetLastError(err, errors.New(aws.StringValue(output.Status.Message))) + return output, err } return nil, err } -func WaitClientVPNAuthorizationRuleRevoked(conn *ec2.EC2, authorizationRuleID string) (*ec2.AuthorizationRule, error) { +func WaitClientVPNAuthorizationRuleDeleted(conn *ec2.EC2, endpointID, targetNetworkCIDR, accessGroupID string, timeout time.Duration) (*ec2.AuthorizationRule, error) { stateConf := &resource.StateChangeConf{ Pending: []string{ec2.ClientVpnAuthorizationRuleStatusCodeRevoking}, Target: []string{}, - Refresh: StatusClientVPNAuthorizationRule(conn, authorizationRuleID), - Timeout: ClientVPNAuthorizationRuleRevokedTimeout, + Refresh: StatusClientVPNAuthorizationRule(conn, endpointID, targetNetworkCIDR, accessGroupID), + Timeout: timeout, } outputRaw, err := stateConf.WaitForState() if output, ok := outputRaw.(*ec2.AuthorizationRule); ok { + tfresource.SetLastError(err, errors.New(aws.StringValue(output.Status.Message))) + return output, err } diff --git a/website/docs/r/ec2_client_vpn_authorization_rule.html.markdown b/website/docs/r/ec2_client_vpn_authorization_rule.html.markdown index 9e9cf11d500..6ea1a383407 100644 --- a/website/docs/r/ec2_client_vpn_authorization_rule.html.markdown +++ b/website/docs/r/ec2_client_vpn_authorization_rule.html.markdown @@ -35,6 +35,13 @@ The following arguments are supported: No additional attributes are exported. +## Timeouts + +`aws_ec2_client_vpn_authorization_rule` provides the following [Timeouts](https://www.terraform.io/docs/configuration/blocks/resources/syntax.html#operation-timeouts) configuration options: + +- `create` - (Default `10 minutes`) Used for rule authorization +- `delete` - (Default `10 minutes`) Used for rule revocation + ## Import AWS Client VPN authorization rules can be imported using the endpoint ID and target network CIDR. If there is a specific group name that is included as well. All values are separated by a `,`.