Skip to content

Commit bfdd869

Browse files
gargipanatulayue9944882
gargipanatula
authored and
yue9944882
committed
remove nil ptr dereference
Signed-off-by: Min Jin <[email protected]>
1 parent 8a26e11 commit bfdd869

File tree

3 files changed

+81
-21
lines changed

3 files changed

+81
-21
lines changed

‎pkg/providers/v1/aws_fakes.go‎

Lines changed: 27 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import (
3232
"github.com/aws/aws-sdk-go-v2/service/ec2"
3333
ec2types "github.com/aws/aws-sdk-go-v2/service/ec2/types"
3434
elb "github.com/aws/aws-sdk-go-v2/service/elasticloadbalancing"
35+
elbtypes "github.com/aws/aws-sdk-go-v2/service/elasticloadbalancing/types"
3536
elbv2 "github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2"
3637
"github.com/aws/aws-sdk-go-v2/service/kms"
3738
"k8s.io/klog/v2"
@@ -519,108 +520,114 @@ type FakeELB struct {
519520

520521
// CreateLoadBalancer is not implemented but is required for interface
521522
// conformance
522-
func (elb *FakeELB) CreateLoadBalancer(ctx context.Context, input *elb.CreateLoadBalancerInput, opts ...func(*elb.Options)) (*elb.CreateLoadBalancerOutput, error) {
523+
func (e *FakeELB) CreateLoadBalancer(ctx context.Context, input *elb.CreateLoadBalancerInput, opts ...func(*elb.Options)) (*elb.CreateLoadBalancerOutput, error) {
523524
panic("Not implemented")
524525
}
525526

526527
// DeleteLoadBalancer is not implemented but is required for interface
527528
// conformance
528-
func (elb *FakeELB) DeleteLoadBalancer(ctx context.Context, input *elb.DeleteLoadBalancerInput, opts ...func(*elb.Options)) (*elb.DeleteLoadBalancerOutput, error) {
529+
func (e *FakeELB) DeleteLoadBalancer(ctx context.Context, input *elb.DeleteLoadBalancerInput, opts ...func(*elb.Options)) (*elb.DeleteLoadBalancerOutput, error) {
529530
panic("Not implemented")
530531
}
531532

532533
// DescribeLoadBalancers is not implemented but is required for interface
533534
// conformance
534-
func (elb *FakeELB) DescribeLoadBalancers(ctx context.Context, input *elb.DescribeLoadBalancersInput, opts ...func(*elb.Options)) (*elb.DescribeLoadBalancersOutput, error) {
535+
func (e *FakeELB) DescribeLoadBalancers(ctx context.Context, input *elb.DescribeLoadBalancersInput, opts ...func(*elb.Options)) (*elb.DescribeLoadBalancersOutput, error) {
535536
panic("Not implemented")
536537
}
537538

538539
// AddTags is not implemented but is required for interface conformance
539-
func (elb *FakeELB) AddTags(ctx context.Context, input *elb.AddTagsInput, opts ...func(*elb.Options)) (*elb.AddTagsOutput, error) {
540+
func (e *FakeELB) AddTags(ctx context.Context, input *elb.AddTagsInput, opts ...func(*elb.Options)) (*elb.AddTagsOutput, error) {
540541
panic("Not implemented")
541542
}
542543

543544
// RegisterInstancesWithLoadBalancer is not implemented but is required for
544545
// interface conformance
545-
func (elb *FakeELB) RegisterInstancesWithLoadBalancer(ctx context.Context, input *elb.RegisterInstancesWithLoadBalancerInput, opts ...func(*elb.Options)) (*elb.RegisterInstancesWithLoadBalancerOutput, error) {
546+
func (e *FakeELB) RegisterInstancesWithLoadBalancer(ctx context.Context, input *elb.RegisterInstancesWithLoadBalancerInput, opts ...func(*elb.Options)) (*elb.RegisterInstancesWithLoadBalancerOutput, error) {
546547
panic("Not implemented")
547548
}
548549

549550
// DeregisterInstancesFromLoadBalancer is not implemented but is required for
550551
// interface conformance
551-
func (elb *FakeELB) DeregisterInstancesFromLoadBalancer(ctx context.Context, input *elb.DeregisterInstancesFromLoadBalancerInput, opts ...func(*elb.Options)) (*elb.DeregisterInstancesFromLoadBalancerOutput, error) {
552+
func (e *FakeELB) DeregisterInstancesFromLoadBalancer(ctx context.Context, input *elb.DeregisterInstancesFromLoadBalancerInput, opts ...func(*elb.Options)) (*elb.DeregisterInstancesFromLoadBalancerOutput, error) {
552553
panic("Not implemented")
553554
}
554555

555556
// DetachLoadBalancerFromSubnets is not implemented but is required for
556557
// interface conformance
557-
func (elb *FakeELB) DetachLoadBalancerFromSubnets(ctx context.Context, input *elb.DetachLoadBalancerFromSubnetsInput, opts ...func(*elb.Options)) (*elb.DetachLoadBalancerFromSubnetsOutput, error) {
558+
func (e *FakeELB) DetachLoadBalancerFromSubnets(ctx context.Context, input *elb.DetachLoadBalancerFromSubnetsInput, opts ...func(*elb.Options)) (*elb.DetachLoadBalancerFromSubnetsOutput, error) {
558559
panic("Not implemented")
559560
}
560561

561562
// AttachLoadBalancerToSubnets is not implemented but is required for interface
562563
// conformance
563-
func (elb *FakeELB) AttachLoadBalancerToSubnets(ctx context.Context, input *elb.AttachLoadBalancerToSubnetsInput, opts ...func(*elb.Options)) (*elb.AttachLoadBalancerToSubnetsOutput, error) {
564+
func (e *FakeELB) AttachLoadBalancerToSubnets(ctx context.Context, input *elb.AttachLoadBalancerToSubnetsInput, opts ...func(*elb.Options)) (*elb.AttachLoadBalancerToSubnetsOutput, error) {
564565
panic("Not implemented")
565566
}
566567

567568
// CreateLoadBalancerListeners is not implemented but is required for interface
568569
// conformance
569-
func (elb *FakeELB) CreateLoadBalancerListeners(ctx context.Context, input *elb.CreateLoadBalancerListenersInput, opts ...func(*elb.Options)) (*elb.CreateLoadBalancerListenersOutput, error) {
570+
func (e *FakeELB) CreateLoadBalancerListeners(ctx context.Context, input *elb.CreateLoadBalancerListenersInput, opts ...func(*elb.Options)) (*elb.CreateLoadBalancerListenersOutput, error) {
570571
panic("Not implemented")
571572
}
572573

573574
// DeleteLoadBalancerListeners is not implemented but is required for interface
574575
// conformance
575-
func (elb *FakeELB) DeleteLoadBalancerListeners(ctx context.Context, input *elb.DeleteLoadBalancerListenersInput, opts ...func(*elb.Options)) (*elb.DeleteLoadBalancerListenersOutput, error) {
576+
func (e *FakeELB) DeleteLoadBalancerListeners(ctx context.Context, input *elb.DeleteLoadBalancerListenersInput, opts ...func(*elb.Options)) (*elb.DeleteLoadBalancerListenersOutput, error) {
576577
panic("Not implemented")
577578
}
578579

579580
// ApplySecurityGroupsToLoadBalancer is not implemented but is required for
580581
// interface conformance
581-
func (elb *FakeELB) ApplySecurityGroupsToLoadBalancer(ctx context.Context, input *elb.ApplySecurityGroupsToLoadBalancerInput, opts ...func(*elb.Options)) (*elb.ApplySecurityGroupsToLoadBalancerOutput, error) {
582+
func (e *FakeELB) ApplySecurityGroupsToLoadBalancer(ctx context.Context, input *elb.ApplySecurityGroupsToLoadBalancerInput, opts ...func(*elb.Options)) (*elb.ApplySecurityGroupsToLoadBalancerOutput, error) {
582583
panic("Not implemented")
583584
}
584585

585586
// ConfigureHealthCheck is not implemented but is required for interface
586587
// conformance
587-
func (elb *FakeELB) ConfigureHealthCheck(ctx context.Context, input *elb.ConfigureHealthCheckInput, opts ...func(*elb.Options)) (*elb.ConfigureHealthCheckOutput, error) {
588+
func (e *FakeELB) ConfigureHealthCheck(ctx context.Context, input *elb.ConfigureHealthCheckInput, opts ...func(*elb.Options)) (*elb.ConfigureHealthCheckOutput, error) {
588589
panic("Not implemented")
589590
}
590591

591592
// CreateLoadBalancerPolicy is not implemented but is required for interface
592593
// conformance
593-
func (elb *FakeELB) CreateLoadBalancerPolicy(ctx context.Context, input *elb.CreateLoadBalancerPolicyInput, opts ...func(*elb.Options)) (*elb.CreateLoadBalancerPolicyOutput, error) {
594-
panic("Not implemented")
594+
func (e *FakeELB) CreateLoadBalancerPolicy(ctx context.Context, input *elb.CreateLoadBalancerPolicyInput, opts ...func(*elb.Options)) (*elb.CreateLoadBalancerPolicyOutput, error) {
595+
return &elb.CreateLoadBalancerPolicyOutput{}, nil
595596
}
596597

597598
// SetLoadBalancerPoliciesForBackendServer is not implemented but is required
598599
// for interface conformance
599-
func (elb *FakeELB) SetLoadBalancerPoliciesForBackendServer(ctx context.Context, input *elb.SetLoadBalancerPoliciesForBackendServerInput, opts ...func(*elb.Options)) (*elb.SetLoadBalancerPoliciesForBackendServerOutput, error) {
600+
func (e *FakeELB) SetLoadBalancerPoliciesForBackendServer(ctx context.Context, input *elb.SetLoadBalancerPoliciesForBackendServerInput, opts ...func(*elb.Options)) (*elb.SetLoadBalancerPoliciesForBackendServerOutput, error) {
600601
panic("Not implemented")
601602
}
602603

603604
// SetLoadBalancerPoliciesOfListener is not implemented but is required for
604605
// interface conformance
605-
func (elb *FakeELB) SetLoadBalancerPoliciesOfListener(ctx context.Context, input *elb.SetLoadBalancerPoliciesOfListenerInput, opts ...func(*elb.Options)) (*elb.SetLoadBalancerPoliciesOfListenerOutput, error) {
606+
func (e *FakeELB) SetLoadBalancerPoliciesOfListener(ctx context.Context, input *elb.SetLoadBalancerPoliciesOfListenerInput, opts ...func(*elb.Options)) (*elb.SetLoadBalancerPoliciesOfListenerOutput, error) {
606607
panic("Not implemented")
607608
}
608609

609610
// DescribeLoadBalancerPolicies is not implemented but is required for
610611
// interface conformance
611-
func (elb *FakeELB) DescribeLoadBalancerPolicies(ctx context.Context, input *elb.DescribeLoadBalancerPoliciesInput, opts ...func(*elb.Options)) (*elb.DescribeLoadBalancerPoliciesOutput, error) {
612-
panic("Not implemented")
612+
func (e *FakeELB) DescribeLoadBalancerPolicies(ctx context.Context, input *elb.DescribeLoadBalancerPoliciesInput, opts ...func(*elb.Options)) (*elb.DescribeLoadBalancerPoliciesOutput, error) {
613+
if aws.ToString(input.LoadBalancerName) == "" {
614+
return nil, &elbtypes.LoadBalancerAttributeNotFoundException{}
615+
}
616+
if len(input.PolicyNames) == 0 || input.PolicyNames[0] == "k8s-SSLNegotiationPolicy-" {
617+
return nil, &elbtypes.PolicyNotFoundException{}
618+
}
619+
return &elb.DescribeLoadBalancerPoliciesOutput{}, nil
613620
}
614621

615622
// DescribeLoadBalancerAttributes is not implemented but is required for
616623
// interface conformance
617-
func (elb *FakeELB) DescribeLoadBalancerAttributes(ctx context.Context, input *elb.DescribeLoadBalancerAttributesInput, opts ...func(*elb.Options)) (*elb.DescribeLoadBalancerAttributesOutput, error) {
624+
func (e *FakeELB) DescribeLoadBalancerAttributes(ctx context.Context, input *elb.DescribeLoadBalancerAttributesInput, opts ...func(*elb.Options)) (*elb.DescribeLoadBalancerAttributesOutput, error) {
618625
panic("Not implemented")
619626
}
620627

621628
// ModifyLoadBalancerAttributes is not implemented but is required for
622629
// interface conformance
623-
func (elb *FakeELB) ModifyLoadBalancerAttributes(ctx context.Context, input *elb.ModifyLoadBalancerAttributesInput, opts ...func(*elb.Options)) (*elb.ModifyLoadBalancerAttributesOutput, error) {
630+
func (e *FakeELB) ModifyLoadBalancerAttributes(ctx context.Context, input *elb.ModifyLoadBalancerAttributesInput, opts ...func(*elb.Options)) (*elb.ModifyLoadBalancerAttributesOutput, error) {
624631
panic("Not implemented")
625632
}
626633

‎pkg/providers/v1/aws_loadbalancer.go‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -816,6 +816,9 @@ func (c *Cloud) updateInstanceSecurityGroupsForNLB(ctx context.Context, lbName s
816816
if err != nil {
817817
return fmt.Errorf("error finding instance group: %q", err)
818818
}
819+
if sg == nil {
820+
return fmt.Errorf("error finding security group: %s", sgID)
821+
}
819822
clusterSGs[sgID] = sg
820823
}
821824
}
@@ -1505,13 +1508,16 @@ func (c *Cloud) ensureSSLNegotiationPolicy(ctx context.Context, loadBalancer *el
15051508
},
15061509
})
15071510
if err != nil {
1511+
// If DescribeLoadBalancerPolicies returns a PolicyNotFoundException, we must proceed and create the policy.
15081512
var notFoundErr *elbtypes.PolicyNotFoundException
15091513
if !errors.As(err, &notFoundErr) {
15101514
return fmt.Errorf("error describing security policies on load balancer: %q", err)
15111515
}
15121516
}
15131517

1514-
if len(result.PolicyDescriptions) > 0 {
1518+
// If DescribeLoadBalancerPolicies yielded a PolicyNotFoundException, result will be nil,
1519+
// so we must check before dereferencing
1520+
if result != nil && len(result.PolicyDescriptions) > 0 {
15151521
return nil
15161522
}
15171523

‎pkg/providers/v1/aws_loadbalancer_test.go‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1073,3 +1073,50 @@ func TestCloud_computeTargetGroupExpectedTargets(t *testing.T) {
10731073
})
10741074
}
10751075
}
1076+
1077+
// Make sure that errors returned by DescribeLoadBalancerPolicies are
1078+
// handled gracefully, and don't progress further into the function
1079+
func TestEnsureSSLNegotiationPolicyErrorHandling(t *testing.T) {
1080+
awsServices := NewFakeAWSServices(TestClusterID)
1081+
c, err := newAWSCloud(config.CloudConfig{}, awsServices)
1082+
if err != nil {
1083+
t.Errorf("Error building aws cloud: %v", err)
1084+
return
1085+
}
1086+
1087+
tests := []struct {
1088+
name string
1089+
loadBalancer *elbtypes.LoadBalancerDescription
1090+
policyName string
1091+
expectError bool
1092+
}{
1093+
{
1094+
name: "Expect LoadBalancerAttributeNotFoundException, error",
1095+
loadBalancer: &elbtypes.LoadBalancerDescription{
1096+
LoadBalancerName: aws.String(""),
1097+
},
1098+
policyName: "",
1099+
expectError: true,
1100+
},
1101+
{
1102+
name: "Expect PolicyNotFoundException, nil error",
1103+
loadBalancer: &elbtypes.LoadBalancerDescription{
1104+
LoadBalancerName: aws.String("test-lb"),
1105+
},
1106+
policyName: "",
1107+
expectError: false,
1108+
},
1109+
}
1110+
1111+
for _, test := range tests {
1112+
t.Run(test.name, func(t *testing.T) {
1113+
err := c.ensureSSLNegotiationPolicy(context.TODO(), test.loadBalancer, test.policyName)
1114+
if test.expectError && err == nil {
1115+
t.Errorf("Expected error but got none")
1116+
}
1117+
if !test.expectError && err != nil {
1118+
t.Errorf("Expected no error but got: %v", err)
1119+
}
1120+
})
1121+
}
1122+
}

0 commit comments

Comments
 (0)