-
Notifications
You must be signed in to change notification settings - Fork 9.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
r/aws_lb_listener: remove tcp_idle_timeout_seconds
default value
#40039
Conversation
Community NoteVoting for Prioritization
For Submitters
|
The `tcp_idle_timeout_seconds` default value of `350` has been removed in favor or marking this argument optional and computed. In practice, this means the default value of `350` is still read and stored in state for protocols which support this feature, but unless explicitly configured by the user, the `ModifyListenerAttributes` API will not be called during create or update operations. This change fixes a regression introduced by the addition of the `tcp_idle_timeout_seconds` argument with a default value, which caused any existing configuration using the `aws_lb_listener` resource to now require the calling principal to include the `elasticloadbalancing:ModifyLoadBalancerAttributes` IAM action in an attached IAM policy. By only sending this value when explicitly configured, existing configurations can continue to operate as expected with no modifications to the IAM permissions of the calling principal. To confirm the API is no longer being called when `tcp_idle_timeout_seconds` is not explicitly configured, I've grepped the debug logs of the same configuration applied with `v5.74.0` of the AWS provider and a locally built binary from this branch. The former includes a result with the API request body where the TCP idle timeout is set to the default of `350`. The latter includes no result indicating the `ModifyListenerAttributes` API was not called, as expected. __`v5.74.0`__: ```console % rg "Action=ModifyListenerAttributes" apply-old.out 2672: | Action=ModifyListenerAttributes&Attributes.member.1.Key=tcp.idle_timeout.seconds&Attributes.member.1.Value=350&ListenerArn=arn%3Aaws%3Aelasticloadbalancing%3Aus-west-2%3A<redacted>%3Alistener%2Fnet%2Fjb-test%2Fb0f926497af0c80f%2F819828f77724007f&Version=2015-12-01 ``` __This branch__: ```console % rg "Action=ModifyListenerAttributes" apply.out ``` ```console % make testacc PKG=elbv2 TESTARGS="-run=TestAccELBV2Listener_.*_basic" make: Verifying source code with gofmt... ==> Checking that code complies with gofmt requirements... TF_ACC=1 go1.23.2 test ./internal/service/elbv2/... -v -count 1 -parallel 20 -run=TestAccELBV2Listener_.*_basic -timeout 360m 2024/11/06 16:43:21 Initializing Terraform AWS Provider... --- PASS: TestAccELBV2Listener_Gateway_basic (191.64s) --- PASS: TestAccELBV2Listener_Network_basic (214.10s) --- PASS: TestAccELBV2Listener_Application_basic (229.81s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/elbv2 236.141s ```
Updates to the `tcp_idle_timeout_seconds` argument are now handled distinctly from changes to other non-tag related arguments. Additionally, the logic applied to appropriately infer the protocol for gateway load balancers during the create operation has been replicated during update. Together these changes prevent failures when updating the `tcp_idle_timeout_seconds` argument for gateway load balancers like the following: ``` Error: modifying ELBv2 Listener (arn:aws:elasticloadbalancing:us-west-2:<redacted>:listener/gwy/tf-acc-test-5332588832756138226/541226271e9ad4ec/2bbf23d4354df440): operation error Elastic Load Balancing v2: ModifyListener, https response error StatusCode: 400, RequestID: c6f51e60-0511-443d-a98e-6f51b48fce11, api error ValidationError: An action must be specified ``` Also updates the `_attributes` acceptances tests to fully exercise the update workflow: ```console % make testacc PKG=elbv2 TESTS=TestAccELBV2Listener_attributes make: Verifying source code with gofmt... ==> Checking that code complies with gofmt requirements... TF_ACC=1 go1.23.2 test ./internal/service/elbv2/... -v -count 1 -parallel 20 -run='TestAccELBV2Listener_attributes' -timeout 360m 2024/11/06 16:12:02 Initializing Terraform AWS Provider... --- PASS: TestAccELBV2Listener_attributes_gwlb_TCPIdleTimeoutSeconds (210.83s) --- PASS: TestAccELBV2Listener_attributes_nlb_TCPIdleTimeoutSeconds (238.02s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/elbv2 244.440s ```
7430ae9
to
5aa38a3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚀
> make testacc PKG=elbv2 TESTS=TestAccELBV2Listener_
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.23.2 test ./internal/service/elbv2/... -v -count 1 -parallel 20 -run='TestAccELBV2Listener_' -timeout 360m
2024/11/07 10:23:34 Initializing Terraform AWS Provider...
=== RUN TestAccELBV2Listener_tags
=== PAUSE TestAccELBV2Listener_tags
=== RUN TestAccELBV2Listener_tags_null
=== PAUSE TestAccELBV2Listener_tags_null
=== RUN TestAccELBV2Listener_tags_EmptyMap
....
--- PASS: TestAccELBV2Listener_Gateway_basic (205.84s)
=== CONT TestAccELBV2Listener_tags_DefaultTags_overlapping
--- PASS: TestAccELBV2Listener_tags_DefaultTags_emptyResourceTag (228.48s)
=== CONT TestAccELBV2Listener_tags_DefaultTags_nonOverlapping
--- PASS: TestAccELBV2Listener_Network_basic (231.58s)
=== CONT TestAccELBV2Listener_tags_EmptyTag_OnCreate
--- PASS: TestAccELBV2Listener_tags_DefaultTags_nullOverlappingResourceTag (241.37s)
=== CONT TestAccELBV2Listener_Protocol_https
--- PASS: TestAccELBV2Listener_tags_DefaultTags_emptyProviderOnlyTag (244.17s)
=== CONT TestAccELBV2Listener_redirectWithTargetGroupARN
--- PASS: TestAccELBV2Listener_tags_ComputedTag_OnCreate (254.01s)
=== CONT TestAccELBV2Listener_tags_EmptyTag_OnUpdate_Replace
--- PASS: TestAccELBV2Listener_Forward_tgARNAndForward (274.76s)
=== CONT TestAccELBV2Listener_DefaultAction_empty
=== RUN TestAccELBV2Listener_DefaultAction_empty/forward
=== PAUSE TestAccELBV2Listener_DefaultAction_empty/forward
=== RUN TestAccELBV2Listener_DefaultAction_empty/authenticate-oidc
=== PAUSE TestAccELBV2Listener_DefaultAction_empty/authenticate-oidc
=== RUN TestAccELBV2Listener_DefaultAction_empty/authenticate-cognito
=== PAUSE TestAccELBV2Listener_DefaultAction_empty/authenticate-cognito
=== RUN TestAccELBV2Listener_DefaultAction_empty/redirect
=== PAUSE TestAccELBV2Listener_DefaultAction_empty/redirect
=== RUN TestAccELBV2Listener_DefaultAction_empty/fixed-response
=== PAUSE TestAccELBV2Listener_DefaultAction_empty/fixed-response
=== CONT TestAccELBV2Listener_DefaultAction_actionDisappears
--- PASS: TestAccELBV2Listener_tags_ComputedTag_OnUpdate_Replace (277.07s)
=== CONT TestAccELBV2Listener_tags_EmptyTag_OnUpdate_Add
--- PASS: TestAccELBV2Listener_tags_ComputedTag_OnUpdate_Add (282.33s)
=== CONT TestAccELBV2Listener_DefaultAction_specifyOrder
--- PASS: TestAccELBV2Listener_tags_DefaultTags_updateToResourceOnly (285.59s)
=== CONT TestAccELBV2Listener_DefaultAction_defaultOrder
--- PASS: TestAccELBV2Listener_tags (289.47s)
=== CONT TestAccELBV2Listener_tags_DefaultTags_nullNonOverlappingResourceTag
--- PASS: TestAccELBV2Listener_tags_IgnoreTags_Overlap_ResourceTag (289.97s)
=== CONT TestAccELBV2Listener_oidc
--- PASS: TestAccELBV2Listener_Forward_ingest (302.31s)
=== CONT TestAccELBV2Listener_tags_EmptyMap
--- PASS: TestAccELBV2Listener_Forward_weighted (305.82s)
=== CONT TestAccELBV2Listener_cognito
--- PASS: TestAccELBV2Listener_disappears (310.81s)
=== CONT TestAccELBV2Listener_tags_AddOnUpdate
--- PASS: TestAccELBV2Listener_tags_DefaultTags_updateToProviderOnly (313.34s)
=== CONT TestAccELBV2Listener_fixedResponse
--- PASS: TestAccELBV2Listener_Application_basic (313.86s)
=== CONT TestAccELBV2Listener_Gateway_lbARN
--- PASS: TestAccELBV2Listener_Forward_update (318.86s)
=== CONT TestAccELBV2Listener_mutualAuthenticationPassthrough
--- PASS: TestAccELBV2Listener_tags_IgnoreTags_Overlap_DefaultTag (345.54s)
=== CONT TestAccELBV2Listener_redirect
--- PASS: TestAccELBV2Listener_tags_DefaultTags_providerOnly (402.14s)
=== CONT TestAccELBV2Listener_mutualAuthentication
--- PASS: TestAccELBV2Listener_tags_EmptyTag_OnCreate (246.05s)
=== CONT TestAccELBV2Listener_Protocol_tls
--- PASS: TestAccELBV2Listener_Protocol_https (241.31s)
=== CONT TestAccELBV2Listener_tags_null
--- PASS: TestAccELBV2Listener_Gateway_lbARN (183.53s)
=== CONT TestAccELBV2Listener_Forward_addAction
--- PASS: TestAccELBV2Listener_DefaultAction_actionDisappears (224.61s)
=== CONT TestAccELBV2Listener_attributes_nlb_TCPIdleTimeoutSeconds
--- PASS: TestAccELBV2Listener_DefaultAction_specifyOrder (217.16s)
=== CONT TestAccELBV2Listener_attributes_gwlb_TCPIdleTimeoutSeconds
=== CONT TestAccELBV2Listener_backwardsCompatibility
--- PASS: TestAccELBV2Listener_redirectWithTargetGroupARN (258.92s)
--- PASS: TestAccELBV2Listener_tags_EmptyTag_OnUpdate_Replace (251.13s)
=== CONT TestAccELBV2Listener_Forward_ignoreFields
--- PASS: TestAccELBV2Listener_oidc (238.61s)
=== CONT TestAccELBV2Listener_Protocol_upd
--- PASS: TestAccELBV2Listener_fixedResponse (216.21s)
=== CONT TestAccELBV2Listener_Forward_removeAction
=== CONT TestAccELBV2Listener_Forward_TGARNToForward_weightAndStickiness
--- PASS: TestAccELBV2Listener_tags_EmptyTag_OnUpdate_Add (256.15s)
--- PASS: TestAccELBV2Listener_tags_EmptyMap (234.00s)
=== CONT TestAccELBV2Listener_Forward_addStickiness
--- PASS: TestAccELBV2Listener_tags_DefaultTags_nullNonOverlappingResourceTag (247.09s)
=== CONT TestAccELBV2Listener_Forward_ToTGARN_weightStickiness
--- PASS: TestAccELBV2Listener_cognito (236.67s)
=== CONT TestAccELBV2Listener_Forward_removeStickiness
--- PASS: TestAccELBV2Listener_tags_DefaultTags_overlapping (346.79s)
=== CONT TestAccELBV2Listener_Forward_ToTGARN_noChanges
--- PASS: TestAccELBV2Listener_tags_AddOnUpdate (245.73s)
=== CONT TestAccELBV2Listener_Forward_TGARNToForward_noChanges
--- PASS: TestAccELBV2Listener_mutualAuthenticationPassthrough (244.29s)
=== CONT TestAccELBV2Listener_DefaultAction_empty/forward
--- PASS: TestAccELBV2Listener_tags_DefaultTags_nonOverlapping (335.86s)
=== CONT TestAccELBV2Listener_DefaultAction_empty/redirect
=== CONT TestAccELBV2Listener_DefaultAction_empty/fixed-response
=== CONT TestAccELBV2Listener_DefaultAction_empty/authenticate-cognito
=== CONT TestAccELBV2Listener_DefaultAction_empty/authenticate-oidc
--- PASS: TestAccELBV2Listener_DefaultAction_empty (0.00s)
--- PASS: TestAccELBV2Listener_DefaultAction_empty/forward (1.55s)
--- PASS: TestAccELBV2Listener_DefaultAction_empty/redirect (1.59s)
--- PASS: TestAccELBV2Listener_DefaultAction_empty/fixed-response (1.54s)
--- PASS: TestAccELBV2Listener_DefaultAction_empty/authenticate-cognito (1.62s)
--- PASS: TestAccELBV2Listener_DefaultAction_empty/authenticate-oidc (1.58s)
--- PASS: TestAccELBV2Listener_redirect (233.42s)
--- PASS: TestAccELBV2Listener_DefaultAction_defaultOrder (307.06s)
--- PASS: TestAccELBV2Listener_mutualAuthentication (245.06s)
--- PASS: TestAccELBV2Listener_attributes_gwlb_TCPIdleTimeoutSeconds (186.87s)
--- PASS: TestAccELBV2Listener_attributes_nlb_TCPIdleTimeoutSeconds (201.42s)
--- PASS: TestAccELBV2Listener_tags_null (239.98s)
--- PASS: TestAccELBV2Listener_backwardsCompatibility (225.79s)
--- PASS: TestAccELBV2Listener_Forward_addAction (239.55s)
--- PASS: TestAccELBV2Listener_Protocol_upd (209.02s)
--- PASS: TestAccELBV2Listener_Forward_ignoreFields (234.62s)
--- PASS: TestAccELBV2Listener_Forward_ToTGARN_weightStickiness (239.82s)
--- PASS: TestAccELBV2Listener_Forward_removeAction (250.27s)
--- PASS: TestAccELBV2Listener_Forward_TGARNToForward_weightAndStickiness (250.51s)
--- PASS: TestAccELBV2Listener_Forward_addStickiness (250.75s)
--- PASS: TestAccELBV2Listener_Forward_ToTGARN_noChanges (240.60s)
--- PASS: TestAccELBV2Listener_Forward_removeStickiness (250.81s)
--- PASS: TestAccELBV2Listener_Forward_TGARNToForward_noChanges (249.35s)
--- PASS: TestAccELBV2Listener_Protocol_tls (334.97s)
PASS
ok github.com/hashicorp/terraform-provider-aws/internal/service/elbv2 819.695s
This functionality has been released in v5.75.0 of the Terraform AWS Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you! |
Description
This change fixes two issues related to the newly added
tcp_idle_timeout_seconds
argument.Regression caused by a default value
Reported in #40000
The
tcp_idle_timeout_seconds
default value of350
has been removed in favor or marking this argument optional and computed. In practice, this means the default value of350
is still read and stored in state for protocols which support this feature, but unless explicitly configured by the user, theModifyListenerAttributes
API will not be called during create or update operations.This change fixes a regression introduced by the addition of the
tcp_idle_timeout_seconds
argument with a default value, which caused any existing configuration using theaws_lb_listener
resource to now require the calling principal to include theelasticloadbalancing:ModifyLoadBalancerAttributes
IAM action in an attached IAM policy. By only sending this value when explicitly configured, existing configurations can continue to operate as expected with no modifications to the IAM permissions of the calling principal.To confirm the API is no longer being called when
tcp_idle_timeout_seconds
is not explicitly configured, I've grepped the debug logs of the same configuration applied withv5.74.0
of the AWS provider and a locally built binary from this branch. The former includes a result with the API request body where the TCP idle timeout is set to the default of350
. The latter includes no result indicating theModifyListenerAttributes
API was not called, as expected.v5.74.0
:This branch:
% rg "Action=ModifyListenerAttributes" apply.out
Failed updates of gateway load balancers
Reported in #40022
Updates to the
tcp_idle_timeout_seconds
argument are now handled distinctly from changes to other non-tag related arguments. Additionally, the logic applied to appropriately infer the protocol for gateway load balancers during the create operation has been replicated during update. Together these changes prevent failures when updating thetcp_idle_timeout_seconds
argument for gateway load balancers like the following:Relations
Closes #40000
Closes #40022
Relates #39585
References
Output from Acceptance Testing