Merge pull request #22911 from hashicorp/b-aws_ec2_client_vpn_network_association-security-group-mixup

r/aws_ec2_client_vpn_network_association: Deprecate `security_groups`; r/aws_ec2_client_vpn_endpoint: Add `security_group_ids` and `vpc_id`

Author: ewbankkit

.changelog/22911.txt

@@ -0,0 +1,15 @@
+```release-note:note
+resource/aws_ec2_client_vpn_network_association: The `security_groups` argument has been deprecated. Use the `security_group_ids` argument of the `aws_ec2_client_vpn_endpoint` resource instead
+```
+
+```release-note:enhancement
+resource/aws_ec2_client_vpn_endpoint: Add `security_group_ids` and `vpc_id` arguments
+```
+
+```release-note:enhancement
+data-source/aws_ec2_client_vpn_endpoint: Add `security_group_ids` and `vpc_id` attributes
+```
+
+```release-note:note
+resource/aws_ec2_client_vpn_route: Add [custom `timeouts`](https://www.terraform.io/docs/language/resources/syntax.html#operation-timeouts) block
+```

internal/conns/conns.go

@@ -1718,26 +1718,29 @@ func (c *Config) Client() (interface{}, error) {
 	})
 
 	client.EC2Conn.Handlers.Retry.PushBack(func(r *request.Request) {
-		if r.Operation.Name == "CreateClientVpnEndpoint" {
-			if tfawserr.ErrMessageContains(r.Error, "OperationNotPermitted", "Endpoint cannot be created while another endpoint is being created") {
+		switch err := r.Error; r.Operation.Name {
+		case "AttachVpnGateway", "DetachVpnGateway":
+			if tfawserr.ErrMessageContains(err, "InvalidParameterValue", "This call cannot be completed because there are pending VPNs or Virtual Interfaces") {
 				r.Retryable = aws.Bool(true)
 			}
-		}
 
-		if r.Operation.Name == "CreateVpnConnection" {
-			if tfawserr.ErrMessageContains(r.Error, "VpnConnectionLimitExceeded", "maximum number of mutating objects has been reached") {
+		case "CreateClientVpnEndpoint":
+			if tfawserr.ErrMessageContains(err, "OperationNotPermitted", "Endpoint cannot be created while another endpoint is being created") {
 				r.Retryable = aws.Bool(true)
 			}
-		}
 
-		if r.Operation.Name == "CreateVpnGateway" {
-			if tfawserr.ErrMessageContains(r.Error, "VpnGatewayLimitExceeded", "maximum number of mutating objects has been reached") {
+		case "CreateClientVpnRoute", "DeleteClientVpnRoute":
+			if tfawserr.ErrMessageContains(err, "ConcurrentMutationLimitExceeded", "Cannot initiate another change for this endpoint at this time") {
+				r.Retryable = aws.Bool(true)
+			}
+
+		case "CreateVpnConnection":
+			if tfawserr.ErrMessageContains(err, "VpnConnectionLimitExceeded", "maximum number of mutating objects has been reached") {
 				r.Retryable = aws.Bool(true)
 			}
-		}
 
-		if r.Operation.Name == "AttachVpnGateway" || r.Operation.Name == "DetachVpnGateway" {
-			if tfawserr.ErrMessageContains(r.Error, "InvalidParameterValue", "This call cannot be completed because there are pending VPNs or Virtual Interfaces") {
+		case "CreateVpnGateway":
+			if tfawserr.ErrMessageContains(err, "VpnGatewayLimitExceeded", "maximum number of mutating objects has been reached") {
 				r.Retryable = aws.Bool(true)
 			}
 		}

internal/service/ec2/client_vpn_authorization_rule.go

@@ -9,6 +9,7 @@ import (
 	"github.com/aws/aws-sdk-go/service/ec2"
 	"github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
 	"github.com/hashicorp/terraform-provider-aws/internal/conns"
 	"github.com/hashicorp/terraform-provider-aws/internal/tfresource"
 	"github.com/hashicorp/terraform-provider-aws/internal/verify"
@@ -33,6 +34,7 @@ func ResourceClientVPNAuthorizationRule() *schema.Resource {
 				Type:         schema.TypeString,
 				Optional:     true,
 				ForceNew:     true,
+				ValidateFunc: validation.StringDoesNotContainAny(","),
 				ExactlyOneOf: []string{"access_group_id", "authorize_all_groups"},
 			},
 			"authorize_all_groups": {

internal/service/ec2/client_vpn_endpoint.go

@@ -156,6 +156,14 @@ func ResourceClientVPNEndpoint() *schema.Resource {
 				Optional: true,
 				Elem:     &schema.Schema{Type: schema.TypeString},
 			},
+			"security_group_ids": {
+				Type:     schema.TypeSet,
+				MinItems: 1,
+				MaxItems: 5,
+				Optional: true,
+				Computed: true,
+				Elem:     &schema.Schema{Type: schema.TypeString},
+			},
 			"self_service_portal": {
 				Type:         schema.TypeString,
 				Optional:     true,
@@ -192,6 +200,11 @@ func ResourceClientVPNEndpoint() *schema.Resource {
 				Default:      ec2.TransportProtocolUdp,
 				ValidateFunc: validation.StringInSlice(ec2.TransportProtocol_Values(), false),
 			},
+			"vpc_id": {
+				Type:     schema.TypeString,
+				Optional: true,
+				Computed: true,
+			},
 			"vpn_port": {
 				Type:     schema.TypeInt,
 				Optional: true,
@@ -243,6 +256,10 @@ func resourceClientVPNEndpointCreate(d *schema.ResourceData, meta interface{}) e
 		input.DnsServers = flex.ExpandStringList(v.([]interface{}))
 	}
 
+	if v, ok := d.GetOk("security_group_ids"); ok {
+		input.SecurityGroupIds = flex.ExpandStringSet(v.(*schema.Set))
+	}
+
 	if v, ok := d.GetOk("self_service_portal"); ok {
 		input.SelfServicePortal = aws.String(v.(string))
 	}
@@ -251,6 +268,10 @@ func resourceClientVPNEndpointCreate(d *schema.ResourceData, meta interface{}) e
 		input.SessionTimeoutHours = aws.Int64(int64(v.(int)))
 	}
 
+	if v, ok := d.GetOk("vpc_id"); ok {
+		input.VpcId = aws.String(v.(string))
+	}
+
 	log.Printf("[DEBUG] Creating EC2 Client VPN Endpoint: %s", input)
 	output, err := conn.CreateClientVpnEndpoint(input)
 
@@ -316,6 +337,7 @@ func resourceClientVPNEndpointRead(d *schema.ResourceData, meta interface{}) err
 	d.Set("description", ep.Description)
 	d.Set("dns_name", ep.DnsName)
 	d.Set("dns_servers", aws.StringValueSlice(ep.DnsServers))
+	d.Set("security_group_ids", aws.StringValueSlice(ep.SecurityGroupIds))
 	if aws.StringValue(ep.SelfServicePortalUrl) != "" {
 		d.Set("self_service_portal", ec2.SelfServicePortalEnabled)
 	} else {
@@ -326,6 +348,7 @@ func resourceClientVPNEndpointRead(d *schema.ResourceData, meta interface{}) err
 	d.Set("split_tunnel", ep.SplitTunnel)
 	d.Set("status", ep.Status.Code)
 	d.Set("transport_protocol", ep.TransportProtocol)
+	d.Set("vpc_id", ep.VpcId)
 	d.Set("vpn_port", ep.VpnPort)
 
 	tags := KeyValueTags(ep.Tags).IgnoreAWS().IgnoreConfig(ignoreTagsConfig)
@@ -387,6 +410,12 @@ func resourceClientVPNEndpointUpdate(d *schema.ResourceData, meta interface{}) e
 			}
 		}
 
+		if d.HasChange("security_group_ids") {
+			input.SecurityGroupIds = flex.ExpandStringSet(d.Get("security_group_ids").(*schema.Set))
+			// "InvalidParameterValue: Security Groups cannot be modified without specifying Vpc Id"
+			input.VpcId = aws.String(d.Get("vpc_id").(string))
+		}
+
 		if d.HasChange("self_service_portal") {
 			input.SelfServicePortal = aws.String(d.Get("self_service_portal").(string))
 		}
@@ -407,6 +436,10 @@ func resourceClientVPNEndpointUpdate(d *schema.ResourceData, meta interface{}) e
 			input.VpnPort = aws.Int64(int64(d.Get("vpn_port").(int)))
 		}
 
+		if d.HasChange("vpc_id") {
+			input.VpcId = aws.String(d.Get("vpc_id").(string))
+		}
+
 		if _, err := conn.ModifyClientVpnEndpoint(input); err != nil {
 			return fmt.Errorf("error modifying EC2 Client VPN Endpoint (%s): %w", d.Id(), err)
 		}

internal/service/ec2/client_vpn_endpoint_data_source.go

@@ -119,11 +119,16 @@ func DataSourceClientVPNEndpoint() *schema.Resource {
 				Computed: true,
 			},
 			"dns_servers": {
-				Type:     schema.TypeSet,
+				Type:     schema.TypeList,
 				Computed: true,
 				Elem:     &schema.Schema{Type: schema.TypeString},
 			},
 			"filter": DataSourceFiltersSchema(),
+			"security_group_ids": {
+				Type:     schema.TypeList,
+				Computed: true,
+				Elem:     &schema.Schema{Type: schema.TypeString},
+			},
 			"self_service_portal": {
 				Type:     schema.TypeString,
 				Computed: true,
@@ -145,6 +150,10 @@ func DataSourceClientVPNEndpoint() *schema.Resource {
 				Type:     schema.TypeString,
 				Computed: true,
 			},
+			"vpc_id": {
+				Type:     schema.TypeString,
+				Computed: true,
+			},
 			"vpn_port": {
 				Type:     schema.TypeInt,
 				Computed: true,
@@ -219,6 +228,7 @@ func dataSourceClientVPNEndpointRead(d *schema.ResourceData, meta interface{}) e
 	d.Set("description", ep.Description)
 	d.Set("dns_name", ep.DnsName)
 	d.Set("dns_servers", aws.StringValueSlice(ep.DnsServers))
+	d.Set("security_group_ids", aws.StringValueSlice(ep.SecurityGroupIds))
 	if aws.StringValue(ep.SelfServicePortalUrl) != "" {
 		d.Set("self_service_portal", ec2.SelfServicePortalEnabled)
 	} else {
@@ -228,6 +238,7 @@ func dataSourceClientVPNEndpointRead(d *schema.ResourceData, meta interface{}) e
 	d.Set("session_timeout_hours", ep.SessionTimeoutHours)
 	d.Set("split_tunnel", ep.SplitTunnel)
 	d.Set("transport_protocol", ep.TransportProtocol)
+	d.Set("vpc_id", ep.VpcId)
 	d.Set("vpn_port", ep.VpnPort)
 
 	if err := d.Set("tags", KeyValueTags(ep.Tags).IgnoreAWS().IgnoreConfig(ignoreTagsConfig).Map()); err != nil {

internal/service/ec2/client_vpn_endpoint_data_source_test.go

@@ -35,12 +35,14 @@ func testAccClientVPNEndpointDataSource_basic(t *testing.T) {
 					resource.TestCheckResourceAttrPair(datasource1Name, "description", resourceName, "description"),
 					resource.TestCheckResourceAttrPair(datasource1Name, "dns_name", resourceName, "dns_name"),
 					resource.TestCheckResourceAttrPair(datasource1Name, "dns_servers.#", resourceName, "dns_servers.#"),
+					resource.TestCheckResourceAttrPair(datasource1Name, "security_group_ids.#", resourceName, "security_group_ids.#"),
 					resource.TestCheckResourceAttrPair(datasource1Name, "self_service_portal", resourceName, "self_service_portal"),
 					resource.TestCheckResourceAttrPair(datasource1Name, "server_certificate_arn", resourceName, "server_certificate_arn"),
 					resource.TestCheckResourceAttrPair(datasource1Name, "session_timeout_hours", resourceName, "session_timeout_hours"),
 					resource.TestCheckResourceAttrPair(datasource1Name, "split_tunnel", resourceName, "split_tunnel"),
 					resource.TestCheckResourceAttrPair(datasource1Name, "tags.%", resourceName, "tags.%"),
 					resource.TestCheckResourceAttrPair(datasource1Name, "transport_protocol", resourceName, "transport_protocol"),
+					resource.TestCheckResourceAttrPair(datasource1Name, "vpc_id", resourceName, "vpc_id"),
 					resource.TestCheckResourceAttrPair(datasource1Name, "vpn_port", resourceName, "vpn_port"),
 
 					resource.TestCheckResourceAttrPair(datasource2Name, "arn", resourceName, "arn"),
@@ -53,12 +55,14 @@ func testAccClientVPNEndpointDataSource_basic(t *testing.T) {
 					resource.TestCheckResourceAttrPair(datasource2Name, "description", resourceName, "description"),
 					resource.TestCheckResourceAttrPair(datasource2Name, "dns_name", resourceName, "dns_name"),
 					resource.TestCheckResourceAttrPair(datasource2Name, "dns_servers.#", resourceName, "dns_servers.#"),
+					resource.TestCheckResourceAttrPair(datasource2Name, "security_group_ids.#", resourceName, "security_group_ids.#"),
 					resource.TestCheckResourceAttrPair(datasource2Name, "self_service_portal", resourceName, "self_service_portal"),
 					resource.TestCheckResourceAttrPair(datasource2Name, "server_certificate_arn", resourceName, "server_certificate_arn"),
 					resource.TestCheckResourceAttrPair(datasource2Name, "session_timeout_hours", resourceName, "session_timeout_hours"),
 					resource.TestCheckResourceAttrPair(datasource2Name, "split_tunnel", resourceName, "split_tunnel"),
 					resource.TestCheckResourceAttrPair(datasource2Name, "tags.%", resourceName, "tags.%"),
 					resource.TestCheckResourceAttrPair(datasource2Name, "transport_protocol", resourceName, "transport_protocol"),
+					resource.TestCheckResourceAttrPair(datasource2Name, "vpc_id", resourceName, "vpc_id"),
 					resource.TestCheckResourceAttrPair(datasource2Name, "vpn_port", resourceName, "vpn_port"),
 
 					resource.TestCheckResourceAttrPair(datasource3Name, "arn", resourceName, "arn"),
@@ -71,12 +75,14 @@ func testAccClientVPNEndpointDataSource_basic(t *testing.T) {
 					resource.TestCheckResourceAttrPair(datasource3Name, "description", resourceName, "description"),
 					resource.TestCheckResourceAttrPair(datasource3Name, "dns_name", resourceName, "dns_name"),
 					resource.TestCheckResourceAttrPair(datasource3Name, "dns_servers.#", resourceName, "dns_servers.#"),
+					resource.TestCheckResourceAttrPair(datasource3Name, "security_group_ids.#", resourceName, "security_group_ids.#"),
 					resource.TestCheckResourceAttrPair(datasource3Name, "self_service_portal", resourceName, "self_service_portal"),
 					resource.TestCheckResourceAttrPair(datasource3Name, "server_certificate_arn", resourceName, "server_certificate_arn"),
 					resource.TestCheckResourceAttrPair(datasource3Name, "session_timeout_hours", resourceName, "session_timeout_hours"),
 					resource.TestCheckResourceAttrPair(datasource3Name, "split_tunnel", resourceName, "split_tunnel"),
 					resource.TestCheckResourceAttrPair(datasource3Name, "tags.%", resourceName, "tags.%"),
 					resource.TestCheckResourceAttrPair(datasource3Name, "transport_protocol", resourceName, "transport_protocol"),
+					resource.TestCheckResourceAttrPair(datasource3Name, "vpc_id", resourceName, "vpc_id"),
 					resource.TestCheckResourceAttrPair(datasource3Name, "vpn_port", resourceName, "vpn_port"),
 				),
 			},