Skip to content
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.

Sign up for GitHub

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

Jump to bottom

elbv2: Prevent panics from unsafe * nil dereferences #5367

Merged
merged 2 commits into from
Jul 27, 2018
Merged

elbv2: Prevent panics from unsafe * nil dereferences #5367

merged 2 commits into from
Jul 27, 2018

Conversation

bflad
Copy link
Contributor

@bflad bflad commented Jul 27, 2018

Sorry for the cross-resource pull request but it seemed better to fix this across all the ELBv2 resources instead of just the specific case.

Fixes #5357

Changes proposed in this pull request:

  • Prevent all potential * nil dereference panics by using SDK provided functions (e.g. aws.StringValue())
  • Other minor refactoring while I'm touching all these files:

Output from acceptance testing:

57 tests passed (all tests)
=== RUN   TestAccAWSLBListenerRule_multipleConditionThrowsError
--- PASS: TestAccAWSLBListenerRule_multipleConditionThrowsError (2.57s)
=== RUN   TestAccAWSALBTargetGroup_generatedName
--- PASS: TestAccAWSALBTargetGroup_generatedName (22.68s)
=== RUN   TestAccAWSALBTargetGroup_namePrefix
--- PASS: TestAccAWSALBTargetGroup_namePrefix (23.96s)
=== RUN   TestAccAWSALBTargetGroup_setAndUpdateSlowStart
--- PASS: TestAccAWSALBTargetGroup_setAndUpdateSlowStart (36.79s)
=== RUN   TestAccAWSALBTargetGroup_changePortForceNew
--- PASS: TestAccAWSALBTargetGroup_changePortForceNew (36.93s)
=== RUN   TestAccAWSALBTargetGroup_changeVpcForceNew
--- PASS: TestAccAWSALBTargetGroup_changeVpcForceNew (38.41s)
=== RUN   TestAccAWSALBTargetGroup_tags
--- PASS: TestAccAWSALBTargetGroup_tags (43.37s)
=== RUN   TestAccAWSALBTargetGroup_updateHealthCheck
--- PASS: TestAccAWSALBTargetGroup_updateHealthCheck (43.52s)
=== RUN   TestAccAWSALBTargetGroup_changeProtocolForceNew
--- PASS: TestAccAWSALBTargetGroup_changeProtocolForceNew (44.37s)
=== RUN   TestAccAWSALBTargetGroup_basic
--- PASS: TestAccAWSALBTargetGroup_basic (49.61s)
=== RUN   TestAccAWSALBTargetGroup_changeNameForceNew
--- PASS: TestAccAWSALBTargetGroup_changeNameForceNew (50.33s)
=== RUN   TestAccAWSLBTargetGroup_basic
--- PASS: TestAccAWSLBTargetGroup_basic (14.25s)
=== RUN   TestAccAWSALBTargetGroup_updateSticknessEnabled
--- PASS: TestAccAWSALBTargetGroup_updateSticknessEnabled (53.55s)
=== RUN   TestAccAWSLBTargetGroup_namePrefix
--- PASS: TestAccAWSLBTargetGroup_namePrefix (9.16s)
=== RUN   TestAccAWSLBTargetGroup_generatedName
--- PASS: TestAccAWSLBTargetGroup_generatedName (16.34s)
=== RUN   TestAccAWSLBTargetGroup_networkLB_TargetGroupWithProxy
--- PASS: TestAccAWSLBTargetGroup_networkLB_TargetGroupWithProxy (26.92s)
=== RUN   TestAccAWSLBTargetGroup_networkLB_TargetGroup
--- PASS: TestAccAWSLBTargetGroup_networkLB_TargetGroup (33.78s)
=== RUN   TestAccAWSLBTargetGroup_changePortForceNew
--- PASS: TestAccAWSLBTargetGroup_changePortForceNew (22.40s)
=== RUN   TestAccAWSLBTargetGroup_TCP_HTTPHealthCheck
--- PASS: TestAccAWSLBTargetGroup_TCP_HTTPHealthCheck (33.90s)
=== RUN   TestAccAWSLBTargetGroup_changeNameForceNew
--- PASS: TestAccAWSLBTargetGroup_changeNameForceNew (28.23s)
=== RUN   TestAccAWSLBTargetGroup_changeProtocolForceNew
--- PASS: TestAccAWSLBTargetGroup_changeProtocolForceNew (30.84s)
=== RUN   TestAccAWSLBTargetGroup_stickinessWithTCPEnabledShouldError
--- PASS: TestAccAWSLBTargetGroup_stickinessWithTCPEnabledShouldError (1.51s)
=== RUN   TestAccAWSLBTargetGroup_changeVpcForceNew
--- PASS: TestAccAWSLBTargetGroup_changeVpcForceNew (35.95s)
=== RUN   TestAccAWSLBTargetGroup_stickinessWithTCPDisabled
--- PASS: TestAccAWSLBTargetGroup_stickinessWithTCPDisabled (17.33s)
=== RUN   TestAccAWSLBTargetGroup_defaults_application
--- PASS: TestAccAWSLBTargetGroup_defaults_application (22.19s)
=== RUN   TestAccAWSLBTargetGroup_tags
--- PASS: TestAccAWSLBTargetGroup_tags (32.60s)
=== RUN   TestAccAWSLBTargetGroup_updateHealthCheck
--- PASS: TestAccAWSLBTargetGroup_updateHealthCheck (36.71s)
=== RUN   TestAccAWSLBTargetGroup_defaults_network
--- PASS: TestAccAWSLBTargetGroup_defaults_network (30.11s)
=== RUN   TestAccAWSLBTargetGroup_updateSticknessEnabled
--- PASS: TestAccAWSLBTargetGroup_updateSticknessEnabled (41.00s)
=== RUN   TestAccAWSLBTargetGroupAttachment_basic
--- PASS: TestAccAWSLBTargetGroupAttachment_basic (102.24s)
=== RUN   TestAccAWSLBTargetGroupAttachment_withoutPort
--- PASS: TestAccAWSLBTargetGroupAttachment_withoutPort (120.71s)
=== RUN   TestAccDataSourceAWSLB_basic
--- PASS: TestAccDataSourceAWSLB_basic (191.93s)
=== RUN   TestAccAWSLBListenerRule_updateRulePriority
--- PASS: TestAccAWSLBListenerRule_updateRulePriority (205.13s)
=== RUN   TestAccDataSourceAWSLBListener_basic
--- PASS: TestAccDataSourceAWSLBListener_basic (208.61s)
=== RUN   TestAccAWSLBListener_basic
--- PASS: TestAccAWSLBListener_basic (209.01s)
=== RUN   TestAccAWSLBListener_https
--- PASS: TestAccAWSLBListener_https (199.02s)
=== RUN   TestAccDataSourceAWSLBListener_https
--- PASS: TestAccDataSourceAWSLBListener_https (232.94s)
=== RUN   TestAccAWSLBListenerRule_changeListenerRuleArnForcesNew
--- PASS: TestAccAWSLBListenerRule_changeListenerRuleArnForcesNew (234.05s)
=== RUN   TestAccDataSourceAWSALBTargetGroup_basic
--- PASS: TestAccDataSourceAWSALBTargetGroup_basic (255.68s)
=== RUN   TestAccAWSLBListenerRule_basic
--- PASS: TestAccAWSLBListenerRule_basic (266.43s)
=== RUN   TestAccAWSLB_generatedName
--- PASS: TestAccAWSLB_generatedName (190.17s)
=== RUN   TestAccAWSLB_generatesNameForZeroValue
--- PASS: TestAccAWSLB_generatesNameForZeroValue (193.02s)
=== RUN   TestAccAWSLBListenerRule_priority
--- PASS: TestAccAWSLBListenerRule_priority (293.95s)
=== RUN   TestAccAWSLB_basic
--- PASS: TestAccAWSLB_basic (213.61s)
=== RUN   TestAccAWSLB_namePrefix
--- PASS: TestAccAWSLB_namePrefix (200.51s)
=== RUN   TestAccAWSLB_networkLoadbalancerEIP
--- PASS: TestAccAWSLB_networkLoadbalancerEIP (215.04s)
=== RUN   TestAccAWSLB_networkLoadbalancerBasic
--- PASS: TestAccAWSLB_networkLoadbalancerBasic (236.63s)
=== RUN   TestAccAWSLB_tags
--- PASS: TestAccAWSLB_tags (245.96s)
=== RUN   TestAccAWSLB_applicationLoadBalancer_updateHttp2
--- PASS: TestAccAWSLB_applicationLoadBalancer_updateHttp2 (278.89s)
=== RUN   TestAccAWSLB_networkLoadbalancer_updateCrossZone
--- PASS: TestAccAWSLB_networkLoadbalancer_updateCrossZone (295.13s)
=== RUN   TestAccAWSLB_updatedSecurityGroups
--- PASS: TestAccAWSLB_updatedSecurityGroups (216.58s)
=== RUN   TestAccAWSLB_noSecurityGroup
--- PASS: TestAccAWSLB_noSecurityGroup (207.78s)
=== RUN   TestAccAWSLB_applicationLoadBalancer_updateDeletionProtection
--- PASS: TestAccAWSLB_applicationLoadBalancer_updateDeletionProtection (270.64s)
=== RUN   TestAccAWSLB_updatedSubnets
--- PASS: TestAccAWSLB_updatedSubnets (235.69s)
=== RUN   TestAccAWSLB_updatedIpAddressType
--- PASS: TestAccAWSLB_updatedIpAddressType (232.76s)
=== RUN   TestAccAWSLB_networkLoadbalancer_subnet_change
--- PASS: TestAccAWSLB_networkLoadbalancer_subnet_change (247.36s)
=== RUN   TestAccAWSLB_accesslogs
--- PASS: TestAccAWSLB_accesslogs (333.41s)

@bflad bflad added bug Addresses a defect in current functionality. crash Results from or addresses a Terraform crash or kernel panic. service/elbv2 Issues and PRs that pertain to the elbv2 service. labels Jul 27, 2018
@bflad bflad requested a review from a team July 27, 2018 14:21
@ghost ghost added the size/L Managed by automation to categorize the size of a PR. label Jul 27, 2018
This was referenced Jul 27, 2018
Closed
Closed
@bflad
resource/aws_lb_listener_rule: Ensure priority conversion error inclu…
cd39d3b 
…des priority

Fixes: aws/resource_aws_lb_listener_rule.go:190: missing argument for Errorf("%s"): format reads arg 2, have only 1 args
tombuildsstuff
tombuildsstuff approved these changes Jul 27, 2018
View reviewed changes
Copy link
Contributor

@tombuildsstuff tombuildsstuff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@appilon
Copy link
Contributor

appilon commented Jul 27, 2018

@tombuildsstuff is faster than lightning!

@bflad bflad added this to the v1.30.0 milestone Jul 27, 2018
@bflad bflad merged commit c449117 into master Jul 27, 2018
@bflad bflad deleted the b-elbv2-unsafe-dereferences branch July 27, 2018 18:54
bflad added a commit that referenced this pull request Jul 27, 2018
@bflad
Update CHANGELOG for #5367
65e13e2
@bflad bflad mentioned this pull request Jul 30, 2018
Closed
@bflad
Copy link
Contributor Author

bflad commented Aug 2, 2018

This has been released in version 1.30.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

@wking wking mentioned this pull request Dec 16, 2018
Merged
wking added a commit to wking/terraform-provider-aws that referenced this pull request Dec 16, 2018
@wking
website/docs/r/lb_target_group: Drop "TLS" from health-check protocols
e8709f4 
It was added in 10532ba (Be explicit about protocol options for
target group, 2018-07-19, hashicorp#5246), which quotes an error message that
includes "TLS".  But neither the AWS docs [1,2] nor our internal
validation (e.g. see d2e38fd, elbv2: Prevent panics from unsafe
dereferences, 2018-07-27, hashicorp#5367) include a TLS protocol.

[1]: https://docs.aws.amazon.com/elasticloadbalancing/latest/APIReference/API_CreateTargetGroup.html#API_CreateTargetGroup_RequestParameters
[2]: https://docs.aws.amazon.com/elasticloadbalancing/latest/network/target-group-health-checks.html#health-check-settings
@ghost
Copy link

ghost commented Apr 4, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

@ghost ghost locked and limited conversation to collaborators Apr 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@appilon appilon appilon approved these changes

@tombuildsstuff tombuildsstuff tombuildsstuff approved these changes

Assignees
No one assigned
Labels
bug Addresses a defect in current functionality. crash Results from or addresses a Terraform crash or kernel panic. service/elbv2 Issues and PRs that pertain to the elbv2 service. size/L Managed by automation to categorize the size of a PR.
Projects
None yet
Milestone
v1.30.0
Development

Successfully merging this pull request may close these issues.

EOF in aws_lb_listener when ALB is configured with the new "fixed response" option (via AWS console)
3 participants
@bflad @appilon @tombuildsstuff