remove nil ptr dereference

Author: gargipanatula

pkg/providers/v1/aws_fakes.go

@@ -32,6 +32,7 @@ import (
 	"github.com/aws/aws-sdk-go-v2/service/ec2"
 	ec2types "github.com/aws/aws-sdk-go-v2/service/ec2/types"
 	elb "github.com/aws/aws-sdk-go-v2/service/elasticloadbalancing"
+	elbtypes "github.com/aws/aws-sdk-go-v2/service/elasticloadbalancing/types"
 	elbv2 "github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2"
 	"github.com/aws/aws-sdk-go-v2/service/kms"
 	"k8s.io/klog/v2"
@@ -592,8 +593,8 @@ func (elb *FakeELB) ConfigureHealthCheck(ctx context.Context, input *elb.Configu
 
 // CreateLoadBalancerPolicy is not implemented but is required for interface
 // conformance
-func (elb *FakeELB) CreateLoadBalancerPolicy(ctx context.Context, input *elb.CreateLoadBalancerPolicyInput, opts ...func(*elb.Options)) (*elb.CreateLoadBalancerPolicyOutput, error) {
-	panic("Not implemented")
+func (e *FakeELB) CreateLoadBalancerPolicy(ctx context.Context, input *elb.CreateLoadBalancerPolicyInput, opts ...func(*elb.Options)) (*elb.CreateLoadBalancerPolicyOutput, error) {
+	return &elb.CreateLoadBalancerPolicyOutput{}, nil
 }
 
 // SetLoadBalancerPoliciesForBackendServer is not implemented but is required
@@ -610,8 +611,14 @@ func (elb *FakeELB) SetLoadBalancerPoliciesOfListener(ctx context.Context, input
 
 // DescribeLoadBalancerPolicies is not implemented but is required for
 // interface conformance
-func (elb *FakeELB) DescribeLoadBalancerPolicies(ctx context.Context, input *elb.DescribeLoadBalancerPoliciesInput, opts ...func(*elb.Options)) (*elb.DescribeLoadBalancerPoliciesOutput, error) {
-	panic("Not implemented")
+func (e *FakeELB) DescribeLoadBalancerPolicies(ctx context.Context, input *elb.DescribeLoadBalancerPoliciesInput, opts ...func(*elb.Options)) (*elb.DescribeLoadBalancerPoliciesOutput, error) {
+	if aws.ToString(input.LoadBalancerName) == "" {
+		return nil, &elbtypes.LoadBalancerAttributeNotFoundException{}
+	}
+	if len(input.PolicyNames) == 0 || input.PolicyNames[0] == "k8s-SSLNegotiationPolicy-" {
+		return nil, &elbtypes.PolicyNotFoundException{}
+	}
+	return &elb.DescribeLoadBalancerPoliciesOutput{}, nil
 }
 
 // DescribeLoadBalancerAttributes is not implemented but is required for

pkg/providers/v1/aws_loadbalancer.go

@@ -816,6 +816,9 @@ func (c *Cloud) updateInstanceSecurityGroupsForNLB(ctx context.Context, lbName s
 			if err != nil {
 				return fmt.Errorf("error finding instance group: %q", err)
 			}
+			if sg == nil {
+				return fmt.Errorf("error finding security group: %s", sgID)
+			}
 			clusterSGs[sgID] = sg
 		}
 	}
@@ -1505,13 +1508,16 @@ func (c *Cloud) ensureSSLNegotiationPolicy(ctx context.Context, loadBalancer *el
 		},
 	})
 	if err != nil {
+		// If DescribeLoadBalancerPolicies returns a PolicyNotFoundException, we must proceed and create the policy.
 		var notFoundErr *elbtypes.PolicyNotFoundException
 		if !errors.As(err, &notFoundErr) {
 			return fmt.Errorf("error describing security policies on load balancer: %q", err)
 		}
 	}
 
-	if len(result.PolicyDescriptions) > 0 {
+	// If DescribeLoadBalancerPolicies yielded a PolicyNotFoundException, result will be nil,
+	// so we must check before dereferencing
+	if result != nil && len(result.PolicyDescriptions) > 0 {
 		return nil
 	}
 

pkg/providers/v1/aws_loadbalancer_test.go

@@ -1071,3 +1071,50 @@ func TestCloud_computeTargetGroupExpectedTargets(t *testing.T) {
 		})
 	}
 }
+
+// Make sure that errors returned by DescribeLoadBalancerPolicies are
+// handled gracefully, and don't progress further into the function
+func TestEnsureSSLNegotiationPolicyErrorHandling(t *testing.T) {
+	awsServices := NewFakeAWSServices(TestClusterID)
+	c, err := newAWSCloud(config.CloudConfig{}, awsServices)
+	if err != nil {
+		t.Errorf("Error building aws cloud: %v", err)
+		return
+	}
+
+	tests := []struct {
+		name         string
+		loadBalancer *elbtypes.LoadBalancerDescription
+		policyName   string
+		expectError  bool
+	}{
+		{
+			name: "Expect LoadBalancerAttributeNotFoundException, error",
+			loadBalancer: &elbtypes.LoadBalancerDescription{
+				LoadBalancerName: aws.String(""),
+			},
+			policyName:  "",
+			expectError: true,
+		},
+		{
+			name: "Expect PolicyNotFoundException, nil error",
+			loadBalancer: &elbtypes.LoadBalancerDescription{
+				LoadBalancerName: aws.String("test-lb"),
+			},
+			policyName:  "",
+			expectError: false,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			err := c.ensureSSLNegotiationPolicy(context.TODO(), test.loadBalancer, test.policyName)
+			if test.expectError && err == nil {
+				t.Errorf("Expected error but got none")
+			}
+			if !test.expectError && err != nil {
+				t.Errorf("Expected no error but got: %v", err)
+			}
+		})
+	}
+}