Skip to content

Commit

Permalink
Merge pull request #40039 from hashicorp/b-lb_listener-tcp-timeout-re…
Browse files Browse the repository at this point in the history
…gression

r/aws_lb_listener: remove `tcp_idle_timeout_seconds` default value
  • Loading branch information
jar-b authored Nov 7, 2024
2 parents c903e7c + 5aa38a3 commit 0d5d9e5
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 47 deletions.
6 changes: 6 additions & 0 deletions .changelog/40039.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
```release-note:bug
resource/aws_lb_listener: Remove the default `tcp_idle_timeout_seconds` value, preventing `ModifyListenerAttributes` API calls when a value is not explicitly configured
```
```release-note:bug
resource/aws_lb_listener: Fix errors when updating the `tcp_idle_timeout_seconds` argument for gateway load balancers
```
48 changes: 27 additions & 21 deletions internal/service/elbv2/listener.go
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ func resourceListener() *schema.Resource {
"tcp_idle_timeout_seconds": {
Type: schema.TypeInt,
Optional: true,
Default: 350,
Computed: true,
ValidateFunc: validation.IntBetween(60, 6000),
// Attribute only valid for TCP (NLB) and GENEVE (GWLB) listeners
DiffSuppressFunc: suppressIfListenerProtocolNot(awstypes.ProtocolEnumGeneve, awstypes.ProtocolEnumTcp),
Expand Down Expand Up @@ -536,9 +536,7 @@ func resourceListenerCreate(ctx context.Context, d *schema.ResourceData, meta in
}

// Listener attributes like TCP idle timeout are not supported on create
var attributes []awstypes.ListenerAttribute

attributes = append(attributes, listenerAttributes.expand(d, listenerProtocolType, false)...)
attributes := listenerAttributes.expand(d, listenerProtocolType, false)

if len(attributes) > 0 {
if err := modifyListenerAttributes(ctx, conn, d.Id(), attributes); err != nil {
Expand Down Expand Up @@ -604,9 +602,7 @@ func resourceListenerUpdate(ctx context.Context, d *schema.ResourceData, meta in
var diags diag.Diagnostics
conn := meta.(*conns.AWSClient).ELBV2Client(ctx)

var attributes []awstypes.ListenerAttribute

if d.HasChangesExcept(names.AttrTags, names.AttrTagsAll) {
if d.HasChangesExcept(names.AttrTags, names.AttrTagsAll, "tcp_idle_timeout_seconds") {
input := &elasticloadbalancingv2.ModifyListenerInput{
ListenerArn: aws.String(d.Id()),
}
Expand Down Expand Up @@ -644,14 +640,6 @@ func resourceListenerUpdate(ctx context.Context, d *schema.ResourceData, meta in
input.SslPolicy = aws.String(v.(string))
}

attributes = append(attributes, listenerAttributes.expand(d, awstypes.ProtocolEnum(d.Get(names.AttrProtocol).(string)), true)...)

if len(attributes) > 0 {
if err := modifyListenerAttributes(ctx, conn, d.Id(), attributes); err != nil {
return sdkdiag.AppendFromErr(diags, err)
}
}

_, err := tfresource.RetryWhenIsA[*awstypes.CertificateNotFoundException](ctx, d.Timeout(schema.TimeoutUpdate), func() (interface{}, error) {
return conn.ModifyListener(ctx, input)
})
Expand All @@ -661,6 +649,23 @@ func resourceListenerUpdate(ctx context.Context, d *schema.ResourceData, meta in
}
}

if d.HasChanges("tcp_idle_timeout_seconds") {
lbARN := d.Get("load_balancer_arn").(string)
listenerProtocolType := awstypes.ProtocolEnum(d.Get(names.AttrProtocol).(string))
// Protocol does not need to be explicitly set with GWLB listeners, nor is it returned by the API
// If protocol is not set, use the load balancer ARN to determine if listener is gateway type and set protocol appropriately
if listenerProtocolType == awstypes.ProtocolEnum("") && strings.Contains(lbARN, "loadbalancer/gwy/") {
listenerProtocolType = awstypes.ProtocolEnumGeneve
}

attributes := listenerAttributes.expand(d, listenerProtocolType, true)
if len(attributes) > 0 {
if err := modifyListenerAttributes(ctx, conn, d.Id(), attributes); err != nil {
return sdkdiag.AppendFromErr(diags, err)
}
}
}

return append(diags, resourceListenerRead(ctx, d, meta)...)
}

Expand Down Expand Up @@ -750,12 +755,13 @@ func (m listenerAttributeMap) expand(d *schema.ResourceData, listenerType awstyp
continue
}

if attributeInfo.tfType == schema.TypeInt {
v := (d.Get(tfAttributeName)).(int)
attributes = append(attributes, awstypes.ListenerAttribute{
Key: aws.String(attributeInfo.apiAttributeKey),
Value: flex.IntValueToString(v),
})
if v, ok := d.GetOk(tfAttributeName); ok {
if attributeInfo.tfType == schema.TypeInt {
attributes = append(attributes, awstypes.ListenerAttribute{
Key: aws.String(attributeInfo.apiAttributeKey),
Value: flex.IntValueToString(v.(int)),
})
}
}
}

Expand Down
60 changes: 34 additions & 26 deletions internal/service/elbv2/listener_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"fmt"
"regexp"
"slices"
"strconv"
"testing"

"github.com/YakDriver/regexache"
Expand Down Expand Up @@ -62,6 +63,7 @@ func TestAccELBV2Listener_Application_basic(t *testing.T) {
resource.TestCheckResourceAttr(resourceName, "ssl_policy", ""),
resource.TestCheckResourceAttr(resourceName, acctest.CtTagsPercent, "0"),
resource.TestCheckResourceAttr(resourceName, acctest.CtTagsAllPercent, "0"),
resource.TestCheckNoResourceAttr(resourceName, "tcp_idle_timeout_seconds"),
),
},
{
Expand Down Expand Up @@ -114,6 +116,7 @@ func TestAccELBV2Listener_Network_basic(t *testing.T) {
resource.TestCheckResourceAttr(resourceName, "ssl_policy", ""),
resource.TestCheckResourceAttr(resourceName, acctest.CtTagsPercent, "0"),
resource.TestCheckResourceAttr(resourceName, acctest.CtTagsAllPercent, "0"),
resource.TestCheckResourceAttr(resourceName, "tcp_idle_timeout_seconds", "350"),
),
},
{
Expand Down Expand Up @@ -165,6 +168,7 @@ func TestAccELBV2Listener_Gateway_basic(t *testing.T) {
resource.TestCheckResourceAttr(resourceName, "ssl_policy", ""),
resource.TestCheckResourceAttr(resourceName, acctest.CtTagsPercent, "0"),
resource.TestCheckResourceAttr(resourceName, acctest.CtTagsAllPercent, "0"),
resource.TestCheckResourceAttr(resourceName, "tcp_idle_timeout_seconds", "350"),
),
},
{
Expand Down Expand Up @@ -1013,7 +1017,7 @@ func TestAccELBV2Listener_attributes_gwlb_TCPIdleTimeoutSeconds(t *testing.T) {
lbResourceName := "aws_lb.test"
resourceName := "aws_lb_listener.test"
tcpTimeout1 := 60
// tcpTimeout2 := 6000
tcpTimeout2 := 6000

if testing.Short() {
t.Skip("skipping long-running test in short mode")
Expand All @@ -1032,8 +1036,7 @@ func TestAccELBV2Listener_attributes_gwlb_TCPIdleTimeoutSeconds(t *testing.T) {
resource.TestCheckResourceAttrPair(resourceName, "load_balancer_arn", lbResourceName, names.AttrARN),
resource.TestCheckResourceAttr(resourceName, names.AttrProtocol, ""),
resource.TestCheckResourceAttr(resourceName, names.AttrPort, "0"),
resource.TestCheckResourceAttr(resourceName, "tcp_idle_timeout_seconds", "60"),
resource.TestCheckResourceAttr(resourceName, "tags.Name", rName),
resource.TestCheckResourceAttr(resourceName, "tcp_idle_timeout_seconds", strconv.Itoa(tcpTimeout1)),
),
},
{
Expand All @@ -1044,6 +1047,16 @@ func TestAccELBV2Listener_attributes_gwlb_TCPIdleTimeoutSeconds(t *testing.T) {
"default_action.0.forward",
},
},
{
Config: testAccListenerConfig_attributes_gwlbTCPIdleTimeoutSeconds(rName, tcpTimeout2),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckListenerExists(ctx, resourceName, &conf),
resource.TestCheckResourceAttrPair(resourceName, "load_balancer_arn", lbResourceName, names.AttrARN),
resource.TestCheckResourceAttr(resourceName, names.AttrProtocol, ""),
resource.TestCheckResourceAttr(resourceName, names.AttrPort, "0"),
resource.TestCheckResourceAttr(resourceName, "tcp_idle_timeout_seconds", strconv.Itoa(tcpTimeout2)),
),
},
},
})
}
Expand All @@ -1053,6 +1066,8 @@ func TestAccELBV2Listener_attributes_nlb_TCPIdleTimeoutSeconds(t *testing.T) {
var conf awstypes.Listener
resourceName := "aws_lb_listener.test"
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
tcpTimeout1 := 60
tcpTimeout2 := 6000

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(ctx, t) },
Expand All @@ -1061,31 +1076,13 @@ func TestAccELBV2Listener_attributes_nlb_TCPIdleTimeoutSeconds(t *testing.T) {
CheckDestroy: testAccCheckListenerDestroy(ctx),
Steps: []resource.TestStep{
{
Config: testAccListenerConfig_attributes_nlbTCPIdleTimeoutSeconds(rName),
Config: testAccListenerConfig_attributes_nlbTCPIdleTimeoutSeconds(rName, tcpTimeout1),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckListenerExists(ctx, resourceName, &conf),
resource.TestCheckResourceAttr("aws_lb.test", "load_balancer_type", "network"),
resource.TestCheckResourceAttrPair(resourceName, "load_balancer_arn", "aws_lb.test", names.AttrARN),
acctest.MatchResourceAttrRegionalARN(resourceName, names.AttrARN, "elasticloadbalancing", regexache.MustCompile("listener/.+$")),
resource.TestCheckNoResourceAttr(resourceName, "alpn_policy"),
resource.TestCheckNoResourceAttr(resourceName, names.AttrCertificateARN),
resource.TestCheckResourceAttr(resourceName, "default_action.#", "1"),
resource.TestCheckResourceAttr(resourceName, "default_action.0.order", "1"),
resource.TestCheckResourceAttr(resourceName, "default_action.0.type", "forward"),
resource.TestCheckResourceAttrPair(resourceName, "default_action.0.target_group_arn", "aws_lb_target_group.test", names.AttrARN),
resource.TestCheckResourceAttr(resourceName, "default_action.0.authenticate_cognito.#", "0"),
resource.TestCheckResourceAttr(resourceName, "default_action.0.authenticate_oidc.#", "0"),
resource.TestCheckResourceAttr(resourceName, "default_action.0.fixed_response.#", "0"),
resource.TestCheckResourceAttr(resourceName, "default_action.0.forward.#", "0"),
resource.TestCheckResourceAttr(resourceName, "default_action.0.redirect.#", "0"),
resource.TestCheckResourceAttrPair(resourceName, names.AttrID, resourceName, names.AttrARN),
resource.TestCheckResourceAttr(resourceName, "mutual_authentication.#", "0"),
resource.TestCheckResourceAttr(resourceName, names.AttrProtocol, "TCP"),
resource.TestCheckResourceAttr(resourceName, names.AttrPort, "80"),
resource.TestCheckResourceAttr(resourceName, "ssl_policy", ""),
resource.TestCheckResourceAttr(resourceName, "tcp_idle_timeout_seconds", "60"),
resource.TestCheckResourceAttr(resourceName, acctest.CtTagsPercent, "0"),
resource.TestCheckResourceAttr(resourceName, acctest.CtTagsAllPercent, "0"),
resource.TestCheckResourceAttr(resourceName, "tcp_idle_timeout_seconds", strconv.Itoa(tcpTimeout1)),
),
},
{
Expand All @@ -1096,6 +1093,16 @@ func TestAccELBV2Listener_attributes_nlb_TCPIdleTimeoutSeconds(t *testing.T) {
"default_action.0.forward",
},
},
{
Config: testAccListenerConfig_attributes_nlbTCPIdleTimeoutSeconds(rName, tcpTimeout2),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckListenerExists(ctx, resourceName, &conf),
resource.TestCheckResourceAttrPair(resourceName, "load_balancer_arn", "aws_lb.test", names.AttrARN),
resource.TestCheckResourceAttr(resourceName, names.AttrProtocol, "TCP"),
resource.TestCheckResourceAttr(resourceName, names.AttrPort, "80"),
resource.TestCheckResourceAttr(resourceName, "tcp_idle_timeout_seconds", strconv.Itoa(tcpTimeout2)),
),
},
},
})
}
Expand Down Expand Up @@ -2964,7 +2971,7 @@ resource "aws_lb_listener" "test" {
`, rName, seconds))
}

func testAccListenerConfig_attributes_nlbTCPIdleTimeoutSeconds(rName string) string {
func testAccListenerConfig_attributes_nlbTCPIdleTimeoutSeconds(rName string, seconds int) string {
return acctest.ConfigCompose(
testAccListenerConfig_base(rName), fmt.Sprintf(`
resource "aws_lb_listener" "test" {
Expand All @@ -2976,7 +2983,8 @@ resource "aws_lb_listener" "test" {
target_group_arn = aws_lb_target_group.test.id
type = "forward"
}
tcp_idle_timeout_seconds = 60
tcp_idle_timeout_seconds = %[2]d
}
resource "aws_lb" "test" {
Expand Down Expand Up @@ -3012,7 +3020,7 @@ resource "aws_lb_target_group" "test" {
Name = %[1]q
}
}
`, rName))
`, rName, seconds))
}

func testAccListenerConfig_Forward_changeWeightedToBasic(rName, rName2 string) string {
Expand Down

0 comments on commit 0d5d9e5

Please sign in to comment.