Skip to content

Commit e19ca0b

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

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"
@@ -521,108 +522,114 @@ type FakeELB struct {
521522

522523
// CreateLoadBalancer is not implemented but is required for interface
523524
// conformance
524-
func (elb *FakeELB) CreateLoadBalancer(ctx context.Context, input *elb.CreateLoadBalancerInput, opts ...func(*elb.Options)) (*elb.CreateLoadBalancerOutput, error) {
525+
func (e *FakeELB) CreateLoadBalancer(ctx context.Context, input *elb.CreateLoadBalancerInput, opts ...func(*elb.Options)) (*elb.CreateLoadBalancerOutput, error) {
525526
panic("Not implemented")
526527
}
527528

528529
// DeleteLoadBalancer is not implemented but is required for interface
529530
// conformance
530-
func (elb *FakeELB) DeleteLoadBalancer(ctx context.Context, input *elb.DeleteLoadBalancerInput, opts ...func(*elb.Options)) (*elb.DeleteLoadBalancerOutput, error) {
531+
func (e *FakeELB) DeleteLoadBalancer(ctx context.Context, input *elb.DeleteLoadBalancerInput, opts ...func(*elb.Options)) (*elb.DeleteLoadBalancerOutput, error) {
531532
panic("Not implemented")
532533
}
533534

534535
// DescribeLoadBalancers is not implemented but is required for interface
535536
// conformance
536-
func (elb *FakeELB) DescribeLoadBalancers(ctx context.Context, input *elb.DescribeLoadBalancersInput, opts ...func(*elb.Options)) (*elb.DescribeLoadBalancersOutput, error) {
537+
func (e *FakeELB) DescribeLoadBalancers(ctx context.Context, input *elb.DescribeLoadBalancersInput, opts ...func(*elb.Options)) (*elb.DescribeLoadBalancersOutput, error) {
537538
panic("Not implemented")
538539
}
539540

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

545546
// RegisterInstancesWithLoadBalancer is not implemented but is required for
546547
// interface conformance
547-
func (elb *FakeELB) RegisterInstancesWithLoadBalancer(ctx context.Context, input *elb.RegisterInstancesWithLoadBalancerInput, opts ...func(*elb.Options)) (*elb.RegisterInstancesWithLoadBalancerOutput, error) {
548+
func (e *FakeELB) RegisterInstancesWithLoadBalancer(ctx context.Context, input *elb.RegisterInstancesWithLoadBalancerInput, opts ...func(*elb.Options)) (*elb.RegisterInstancesWithLoadBalancerOutput, error) {
548549
panic("Not implemented")
549550
}
550551

551552
// DeregisterInstancesFromLoadBalancer is not implemented but is required for
552553
// interface conformance
553-
func (elb *FakeELB) DeregisterInstancesFromLoadBalancer(ctx context.Context, input *elb.DeregisterInstancesFromLoadBalancerInput, opts ...func(*elb.Options)) (*elb.DeregisterInstancesFromLoadBalancerOutput, error) {
554+
func (e *FakeELB) DeregisterInstancesFromLoadBalancer(ctx context.Context, input *elb.DeregisterInstancesFromLoadBalancerInput, opts ...func(*elb.Options)) (*elb.DeregisterInstancesFromLoadBalancerOutput, error) {
554555
panic("Not implemented")
555556
}
556557

557558
// DetachLoadBalancerFromSubnets is not implemented but is required for
558559
// interface conformance
559-
func (elb *FakeELB) DetachLoadBalancerFromSubnets(ctx context.Context, input *elb.DetachLoadBalancerFromSubnetsInput, opts ...func(*elb.Options)) (*elb.DetachLoadBalancerFromSubnetsOutput, error) {
560+
func (e *FakeELB) DetachLoadBalancerFromSubnets(ctx context.Context, input *elb.DetachLoadBalancerFromSubnetsInput, opts ...func(*elb.Options)) (*elb.DetachLoadBalancerFromSubnetsOutput, error) {
560561
panic("Not implemented")
561562
}
562563

563564
// AttachLoadBalancerToSubnets is not implemented but is required for interface
564565
// conformance
565-
func (elb *FakeELB) AttachLoadBalancerToSubnets(ctx context.Context, input *elb.AttachLoadBalancerToSubnetsInput, opts ...func(*elb.Options)) (*elb.AttachLoadBalancerToSubnetsOutput, error) {
566+
func (e *FakeELB) AttachLoadBalancerToSubnets(ctx context.Context, input *elb.AttachLoadBalancerToSubnetsInput, opts ...func(*elb.Options)) (*elb.AttachLoadBalancerToSubnetsOutput, error) {
566567
panic("Not implemented")
567568
}
568569

569570
// CreateLoadBalancerListeners is not implemented but is required for interface
570571
// conformance
571-
func (elb *FakeELB) CreateLoadBalancerListeners(ctx context.Context, input *elb.CreateLoadBalancerListenersInput, opts ...func(*elb.Options)) (*elb.CreateLoadBalancerListenersOutput, error) {
572+
func (e *FakeELB) CreateLoadBalancerListeners(ctx context.Context, input *elb.CreateLoadBalancerListenersInput, opts ...func(*elb.Options)) (*elb.CreateLoadBalancerListenersOutput, error) {
572573
panic("Not implemented")
573574
}
574575

575576
// DeleteLoadBalancerListeners is not implemented but is required for interface
576577
// conformance
577-
func (elb *FakeELB) DeleteLoadBalancerListeners(ctx context.Context, input *elb.DeleteLoadBalancerListenersInput, opts ...func(*elb.Options)) (*elb.DeleteLoadBalancerListenersOutput, error) {
578+
func (e *FakeELB) DeleteLoadBalancerListeners(ctx context.Context, input *elb.DeleteLoadBalancerListenersInput, opts ...func(*elb.Options)) (*elb.DeleteLoadBalancerListenersOutput, error) {
578579
panic("Not implemented")
579580
}
580581

581582
// ApplySecurityGroupsToLoadBalancer is not implemented but is required for
582583
// interface conformance
583-
func (elb *FakeELB) ApplySecurityGroupsToLoadBalancer(ctx context.Context, input *elb.ApplySecurityGroupsToLoadBalancerInput, opts ...func(*elb.Options)) (*elb.ApplySecurityGroupsToLoadBalancerOutput, error) {
584+
func (e *FakeELB) ApplySecurityGroupsToLoadBalancer(ctx context.Context, input *elb.ApplySecurityGroupsToLoadBalancerInput, opts ...func(*elb.Options)) (*elb.ApplySecurityGroupsToLoadBalancerOutput, error) {
584585
panic("Not implemented")
585586
}
586587

587588
// ConfigureHealthCheck is not implemented but is required for interface
588589
// conformance
589-
func (elb *FakeELB) ConfigureHealthCheck(ctx context.Context, input *elb.ConfigureHealthCheckInput, opts ...func(*elb.Options)) (*elb.ConfigureHealthCheckOutput, error) {
590+
func (e *FakeELB) ConfigureHealthCheck(ctx context.Context, input *elb.ConfigureHealthCheckInput, opts ...func(*elb.Options)) (*elb.ConfigureHealthCheckOutput, error) {
590591
panic("Not implemented")
591592
}
592593

593594
// CreateLoadBalancerPolicy is not implemented but is required for interface
594595
// conformance
595-
func (elb *FakeELB) CreateLoadBalancerPolicy(ctx context.Context, input *elb.CreateLoadBalancerPolicyInput, opts ...func(*elb.Options)) (*elb.CreateLoadBalancerPolicyOutput, error) {
596-
panic("Not implemented")
596+
func (e *FakeELB) CreateLoadBalancerPolicy(ctx context.Context, input *elb.CreateLoadBalancerPolicyInput, opts ...func(*elb.Options)) (*elb.CreateLoadBalancerPolicyOutput, error) {
597+
return &elb.CreateLoadBalancerPolicyOutput{}, nil
597598
}
598599

599600
// SetLoadBalancerPoliciesForBackendServer is not implemented but is required
600601
// for interface conformance
601-
func (elb *FakeELB) SetLoadBalancerPoliciesForBackendServer(ctx context.Context, input *elb.SetLoadBalancerPoliciesForBackendServerInput, opts ...func(*elb.Options)) (*elb.SetLoadBalancerPoliciesForBackendServerOutput, error) {
602+
func (e *FakeELB) SetLoadBalancerPoliciesForBackendServer(ctx context.Context, input *elb.SetLoadBalancerPoliciesForBackendServerInput, opts ...func(*elb.Options)) (*elb.SetLoadBalancerPoliciesForBackendServerOutput, error) {
602603
panic("Not implemented")
603604
}
604605

605606
// SetLoadBalancerPoliciesOfListener is not implemented but is required for
606607
// interface conformance
607-
func (elb *FakeELB) SetLoadBalancerPoliciesOfListener(ctx context.Context, input *elb.SetLoadBalancerPoliciesOfListenerInput, opts ...func(*elb.Options)) (*elb.SetLoadBalancerPoliciesOfListenerOutput, error) {
608+
func (e *FakeELB) SetLoadBalancerPoliciesOfListener(ctx context.Context, input *elb.SetLoadBalancerPoliciesOfListenerInput, opts ...func(*elb.Options)) (*elb.SetLoadBalancerPoliciesOfListenerOutput, error) {
608609
panic("Not implemented")
609610
}
610611

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

617624
// DescribeLoadBalancerAttributes is not implemented but is required for
618625
// interface conformance
619-
func (elb *FakeELB) DescribeLoadBalancerAttributes(ctx context.Context, input *elb.DescribeLoadBalancerAttributesInput, opts ...func(*elb.Options)) (*elb.DescribeLoadBalancerAttributesOutput, error) {
626+
func (e *FakeELB) DescribeLoadBalancerAttributes(ctx context.Context, input *elb.DescribeLoadBalancerAttributesInput, opts ...func(*elb.Options)) (*elb.DescribeLoadBalancerAttributesOutput, error) {
620627
panic("Not implemented")
621628
}
622629

623630
// ModifyLoadBalancerAttributes is not implemented but is required for
624631
// interface conformance
625-
func (elb *FakeELB) ModifyLoadBalancerAttributes(ctx context.Context, input *elb.ModifyLoadBalancerAttributesInput, opts ...func(*elb.Options)) (*elb.ModifyLoadBalancerAttributesOutput, error) {
632+
func (e *FakeELB) ModifyLoadBalancerAttributes(ctx context.Context, input *elb.ModifyLoadBalancerAttributesInput, opts ...func(*elb.Options)) (*elb.ModifyLoadBalancerAttributesOutput, error) {
626633
panic("Not implemented")
627634
}
628635

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

0 commit comments

Comments
 (0)