Skip to content

Commit 29df49a

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #1298 from yue9944882/cherry-pick-1223
Automated cherry pick of #1223: Remove potential nil ptr dereferences
2 parents 9c47b1a + 54e6c45 commit 29df49a

File tree

6 files changed

+84
-29
lines changed

6 files changed

+84
-29
lines changed

‎.ko.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
defaultBaseImage: registry.k8s.io/build-image/go-runner:v2.4.0-go1.24.9-bookworm.0
1+
defaultBaseImage: registry.k8s.io/build-image/go-runner:v2.4.0-go1.24.9-bookworm.0

‎go.mod‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
module k8s.io/cloud-provider-aws
22

3-
go 1.24
4-
5-
toolchain go1.24.9
3+
go 1.24.9
64

75
require (
86
github.com/aws/aws-sdk-go-v2 v1.36.5

‎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"
@@ -534,108 +535,114 @@ type FakeELB struct {
534535

535536
// CreateLoadBalancer is not implemented but is required for interface
536537
// conformance
537-
func (elb *FakeELB) CreateLoadBalancer(ctx context.Context, input *elb.CreateLoadBalancerInput, opts ...func(*elb.Options)) (*elb.CreateLoadBalancerOutput, error) {
538+
func (e *FakeELB) CreateLoadBalancer(ctx context.Context, input *elb.CreateLoadBalancerInput, opts ...func(*elb.Options)) (*elb.CreateLoadBalancerOutput, error) {
538539
panic("Not implemented")
539540
}
540541

541542
// DeleteLoadBalancer is not implemented but is required for interface
542543
// conformance
543-
func (elb *FakeELB) DeleteLoadBalancer(ctx context.Context, input *elb.DeleteLoadBalancerInput, opts ...func(*elb.Options)) (*elb.DeleteLoadBalancerOutput, error) {
544+
func (e *FakeELB) DeleteLoadBalancer(ctx context.Context, input *elb.DeleteLoadBalancerInput, opts ...func(*elb.Options)) (*elb.DeleteLoadBalancerOutput, error) {
544545
panic("Not implemented")
545546
}
546547

547548
// DescribeLoadBalancers is not implemented but is required for interface
548549
// conformance
549-
func (elb *FakeELB) DescribeLoadBalancers(ctx context.Context, input *elb.DescribeLoadBalancersInput, opts ...func(*elb.Options)) (*elb.DescribeLoadBalancersOutput, error) {
550+
func (e *FakeELB) DescribeLoadBalancers(ctx context.Context, input *elb.DescribeLoadBalancersInput, opts ...func(*elb.Options)) (*elb.DescribeLoadBalancersOutput, error) {
550551
panic("Not implemented")
551552
}
552553

553554
// AddTags is not implemented but is required for interface conformance
554-
func (elb *FakeELB) AddTags(ctx context.Context, input *elb.AddTagsInput, opts ...func(*elb.Options)) (*elb.AddTagsOutput, error) {
555+
func (e *FakeELB) AddTags(ctx context.Context, input *elb.AddTagsInput, opts ...func(*elb.Options)) (*elb.AddTagsOutput, error) {
555556
panic("Not implemented")
556557
}
557558

558559
// RegisterInstancesWithLoadBalancer is not implemented but is required for
559560
// interface conformance
560-
func (elb *FakeELB) RegisterInstancesWithLoadBalancer(ctx context.Context, input *elb.RegisterInstancesWithLoadBalancerInput, opts ...func(*elb.Options)) (*elb.RegisterInstancesWithLoadBalancerOutput, error) {
561+
func (e *FakeELB) RegisterInstancesWithLoadBalancer(ctx context.Context, input *elb.RegisterInstancesWithLoadBalancerInput, opts ...func(*elb.Options)) (*elb.RegisterInstancesWithLoadBalancerOutput, error) {
561562
panic("Not implemented")
562563
}
563564

564565
// DeregisterInstancesFromLoadBalancer is not implemented but is required for
565566
// interface conformance
566-
func (elb *FakeELB) DeregisterInstancesFromLoadBalancer(ctx context.Context, input *elb.DeregisterInstancesFromLoadBalancerInput, opts ...func(*elb.Options)) (*elb.DeregisterInstancesFromLoadBalancerOutput, error) {
567+
func (e *FakeELB) DeregisterInstancesFromLoadBalancer(ctx context.Context, input *elb.DeregisterInstancesFromLoadBalancerInput, opts ...func(*elb.Options)) (*elb.DeregisterInstancesFromLoadBalancerOutput, error) {
567568
panic("Not implemented")
568569
}
569570

570571
// DetachLoadBalancerFromSubnets is not implemented but is required for
571572
// interface conformance
572-
func (elb *FakeELB) DetachLoadBalancerFromSubnets(ctx context.Context, input *elb.DetachLoadBalancerFromSubnetsInput, opts ...func(*elb.Options)) (*elb.DetachLoadBalancerFromSubnetsOutput, error) {
573+
func (e *FakeELB) DetachLoadBalancerFromSubnets(ctx context.Context, input *elb.DetachLoadBalancerFromSubnetsInput, opts ...func(*elb.Options)) (*elb.DetachLoadBalancerFromSubnetsOutput, error) {
573574
panic("Not implemented")
574575
}
575576

576577
// AttachLoadBalancerToSubnets is not implemented but is required for interface
577578
// conformance
578-
func (elb *FakeELB) AttachLoadBalancerToSubnets(ctx context.Context, input *elb.AttachLoadBalancerToSubnetsInput, opts ...func(*elb.Options)) (*elb.AttachLoadBalancerToSubnetsOutput, error) {
579+
func (e *FakeELB) AttachLoadBalancerToSubnets(ctx context.Context, input *elb.AttachLoadBalancerToSubnetsInput, opts ...func(*elb.Options)) (*elb.AttachLoadBalancerToSubnetsOutput, error) {
579580
panic("Not implemented")
580581
}
581582

582583
// CreateLoadBalancerListeners is not implemented but is required for interface
583584
// conformance
584-
func (elb *FakeELB) CreateLoadBalancerListeners(ctx context.Context, input *elb.CreateLoadBalancerListenersInput, opts ...func(*elb.Options)) (*elb.CreateLoadBalancerListenersOutput, error) {
585+
func (e *FakeELB) CreateLoadBalancerListeners(ctx context.Context, input *elb.CreateLoadBalancerListenersInput, opts ...func(*elb.Options)) (*elb.CreateLoadBalancerListenersOutput, error) {
585586
panic("Not implemented")
586587
}
587588

588589
// DeleteLoadBalancerListeners is not implemented but is required for interface
589590
// conformance
590-
func (elb *FakeELB) DeleteLoadBalancerListeners(ctx context.Context, input *elb.DeleteLoadBalancerListenersInput, opts ...func(*elb.Options)) (*elb.DeleteLoadBalancerListenersOutput, error) {
591+
func (e *FakeELB) DeleteLoadBalancerListeners(ctx context.Context, input *elb.DeleteLoadBalancerListenersInput, opts ...func(*elb.Options)) (*elb.DeleteLoadBalancerListenersOutput, error) {
591592
panic("Not implemented")
592593
}
593594

594595
// ApplySecurityGroupsToLoadBalancer is not implemented but is required for
595596
// interface conformance
596-
func (elb *FakeELB) ApplySecurityGroupsToLoadBalancer(ctx context.Context, input *elb.ApplySecurityGroupsToLoadBalancerInput, opts ...func(*elb.Options)) (*elb.ApplySecurityGroupsToLoadBalancerOutput, error) {
597+
func (e *FakeELB) ApplySecurityGroupsToLoadBalancer(ctx context.Context, input *elb.ApplySecurityGroupsToLoadBalancerInput, opts ...func(*elb.Options)) (*elb.ApplySecurityGroupsToLoadBalancerOutput, error) {
597598
panic("Not implemented")
598599
}
599600

600601
// ConfigureHealthCheck is not implemented but is required for interface
601602
// conformance
602-
func (elb *FakeELB) ConfigureHealthCheck(ctx context.Context, input *elb.ConfigureHealthCheckInput, opts ...func(*elb.Options)) (*elb.ConfigureHealthCheckOutput, error) {
603+
func (e *FakeELB) ConfigureHealthCheck(ctx context.Context, input *elb.ConfigureHealthCheckInput, opts ...func(*elb.Options)) (*elb.ConfigureHealthCheckOutput, error) {
603604
panic("Not implemented")
604605
}
605606

606607
// CreateLoadBalancerPolicy is not implemented but is required for interface
607608
// conformance
608-
func (elb *FakeELB) CreateLoadBalancerPolicy(ctx context.Context, input *elb.CreateLoadBalancerPolicyInput, opts ...func(*elb.Options)) (*elb.CreateLoadBalancerPolicyOutput, error) {
609-
panic("Not implemented")
609+
func (e *FakeELB) CreateLoadBalancerPolicy(ctx context.Context, input *elb.CreateLoadBalancerPolicyInput, opts ...func(*elb.Options)) (*elb.CreateLoadBalancerPolicyOutput, error) {
610+
return &elb.CreateLoadBalancerPolicyOutput{}, nil
610611
}
611612

612613
// SetLoadBalancerPoliciesForBackendServer is not implemented but is required
613614
// for interface conformance
614-
func (elb *FakeELB) SetLoadBalancerPoliciesForBackendServer(ctx context.Context, input *elb.SetLoadBalancerPoliciesForBackendServerInput, opts ...func(*elb.Options)) (*elb.SetLoadBalancerPoliciesForBackendServerOutput, error) {
615+
func (e *FakeELB) SetLoadBalancerPoliciesForBackendServer(ctx context.Context, input *elb.SetLoadBalancerPoliciesForBackendServerInput, opts ...func(*elb.Options)) (*elb.SetLoadBalancerPoliciesForBackendServerOutput, error) {
615616
panic("Not implemented")
616617
}
617618

618619
// SetLoadBalancerPoliciesOfListener is not implemented but is required for
619620
// interface conformance
620-
func (elb *FakeELB) SetLoadBalancerPoliciesOfListener(ctx context.Context, input *elb.SetLoadBalancerPoliciesOfListenerInput, opts ...func(*elb.Options)) (*elb.SetLoadBalancerPoliciesOfListenerOutput, error) {
621+
func (e *FakeELB) SetLoadBalancerPoliciesOfListener(ctx context.Context, input *elb.SetLoadBalancerPoliciesOfListenerInput, opts ...func(*elb.Options)) (*elb.SetLoadBalancerPoliciesOfListenerOutput, error) {
621622
panic("Not implemented")
622623
}
623624

624625
// DescribeLoadBalancerPolicies is not implemented but is required for
625626
// interface conformance
626-
func (elb *FakeELB) DescribeLoadBalancerPolicies(ctx context.Context, input *elb.DescribeLoadBalancerPoliciesInput, opts ...func(*elb.Options)) (*elb.DescribeLoadBalancerPoliciesOutput, error) {
627-
panic("Not implemented")
627+
func (e *FakeELB) DescribeLoadBalancerPolicies(ctx context.Context, input *elb.DescribeLoadBalancerPoliciesInput, opts ...func(*elb.Options)) (*elb.DescribeLoadBalancerPoliciesOutput, error) {
628+
if aws.ToString(input.LoadBalancerName) == "" {
629+
return nil, &elbtypes.LoadBalancerAttributeNotFoundException{}
630+
}
631+
if len(input.PolicyNames) == 0 || input.PolicyNames[0] == "k8s-SSLNegotiationPolicy-" {
632+
return nil, &elbtypes.PolicyNotFoundException{}
633+
}
634+
return &elb.DescribeLoadBalancerPoliciesOutput{}, nil
628635
}
629636

630637
// DescribeLoadBalancerAttributes is not implemented but is required for
631638
// interface conformance
632-
func (elb *FakeELB) DescribeLoadBalancerAttributes(ctx context.Context, input *elb.DescribeLoadBalancerAttributesInput, opts ...func(*elb.Options)) (*elb.DescribeLoadBalancerAttributesOutput, error) {
639+
func (e *FakeELB) DescribeLoadBalancerAttributes(ctx context.Context, input *elb.DescribeLoadBalancerAttributesInput, opts ...func(*elb.Options)) (*elb.DescribeLoadBalancerAttributesOutput, error) {
633640
panic("Not implemented")
634641
}
635642

636643
// ModifyLoadBalancerAttributes is not implemented but is required for
637644
// interface conformance
638-
func (elb *FakeELB) ModifyLoadBalancerAttributes(ctx context.Context, input *elb.ModifyLoadBalancerAttributesInput, opts ...func(*elb.Options)) (*elb.ModifyLoadBalancerAttributesOutput, error) {
645+
func (e *FakeELB) ModifyLoadBalancerAttributes(ctx context.Context, input *elb.ModifyLoadBalancerAttributesInput, opts ...func(*elb.Options)) (*elb.ModifyLoadBalancerAttributesOutput, error) {
639646
panic("Not implemented")
640647
}
641648

‎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+
}

‎tests/e2e/go.mod‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,6 @@
11
module k8s.io/cloud-provider-aws/tests/e2e
22

3-
go 1.24
4-
5-
toolchain go1.24.9
6-
3+
go 1.24.9
74

85
require (
96
github.com/onsi/ginkgo/v2 v2.9.4

0 commit comments

Comments
 (0)