remove nil ptr dereferences

Author: gargipanatula

pkg/providers/v1/aws.go

@@ -1004,7 +1004,7 @@ func (c *Cloud) GetZoneByProviderID(ctx context.Context, providerID string) (clo
 	}
 
 	instance, err := c.getInstanceByID(ctx, string(instanceID))
-	if err != nil {
+	if err != nil || instance == nil {
 		return cloudprovider.Zone{}, err
 	}
 	return c.getInstanceZone(instance), nil
@@ -1078,7 +1078,7 @@ func (c *Cloud) buildSelfAWSInstance(ctx context.Context) (*awsInstance, error)
 	defer instanceIDMetadata.Content.Close()
 
 	instance, err := c.getInstanceByID(ctx, string(instanceIDBytes))
-	if err != nil {
+	if err != nil || instance == nil {
 		return nil, fmt.Errorf("error finding instance %s: %q", string(instanceIDBytes), err)
 	}
 	return newAWSInstance(c.ec2, instance), nil
@@ -3146,7 +3146,7 @@ func (c *Cloud) UpdateLoadBalancer(ctx context.Context, clusterName string, serv
 // Returns the instance with the specified ID
 func (c *Cloud) getInstanceByID(ctx context.Context, instanceID string) (*ec2types.Instance, error) {
 	instances, err := c.getInstancesByIDs(ctx, []string{instanceID})
-	if err != nil {
+	if err != nil || instances == nil {
 		return nil, err
 	}
 
@@ -3347,13 +3347,6 @@ func (c *Cloud) getFullInstance(ctx context.Context, nodeName types.NodeName) (*
 	return awsInstance, instance, err
 }
 
-// extract private ip address from node name
-func nodeNameToIPAddress(nodeName string) string {
-	nodeName = strings.TrimPrefix(nodeName, privateDNSNamePrefix)
-	nodeName = strings.Split(nodeName, ".")[0]
-	return strings.ReplaceAll(nodeName, "-", ".")
-}
-
 func (c *Cloud) nodeNameToInstanceID(nodeName types.NodeName) (InstanceID, error) {
 	if strings.HasPrefix(string(nodeName), rbnNamePrefix) {
 		// depending on if you use a RHEL (e.g. AL2) or Debian (e.g. standard Ubuntu) based distribution, the

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"
@@ -622,7 +623,13 @@ func (e *FakeELB) SetLoadBalancerPoliciesOfListener(ctx context.Context, input *
 // DescribeLoadBalancerPolicies is not implemented but is required for
 // interface conformance
 func (e *FakeELB) DescribeLoadBalancerPolicies(ctx context.Context, input *elb.DescribeLoadBalancerPoliciesInput, opts ...func(*elb.Options)) (*elb.DescribeLoadBalancerPoliciesOutput, error) {
-	panic("Not implemented")
+	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,7 +816,7 @@ func (c *Cloud) updateInstanceSecurityGroupsForNLB(ctx context.Context, lbName s
 	{
 		for sgID := range desiredSGIDs.Difference(sets.StringKeySet(clusterSGs)) {
 			sg, err := c.findSecurityGroup(ctx, sgID)
-			if err != nil {
+			if err != nil || sg == nil {
 				return fmt.Errorf("error finding instance group: %q", err)
 			}
 			clusterSGs[sgID] = sg
@@ -1512,6 +1512,7 @@ func (c *Cloud) ensureSSLNegotiationPolicy(ctx context.Context, loadBalancer *el
 		if !errors.As(err, &notFoundErr) {
 			return fmt.Errorf("error describing security policies on load balancer: %q", err)
 		}
+		return nil
 	}
 
 	if len(result.PolicyDescriptions) > 0 {

pkg/providers/v1/aws_loadbalancer_test.go

@@ -1073,3 +1073,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)
+			}
+		})
+	}
+}

pkg/providers/v1/instances_v2.go

@@ -22,6 +22,7 @@ package aws
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"k8s.io/cloud-provider-aws/pkg/providers/v1/variant"
 	"strconv"
@@ -33,6 +34,9 @@ import (
 )
 
 func (c *Cloud) getProviderID(ctx context.Context, node *v1.Node) (string, error) {
+	if node == nil {
+		return "", errors.New("error getting provider id, node is nil")
+	}
 	if node.Spec.ProviderID != "" {
 		return node.Spec.ProviderID, nil
 	}
@@ -147,7 +151,7 @@ func (c *Cloud) InstanceMetadata(ctx context.Context, node *v1.Node) (*cloudprov
 		}
 	} else {
 		instance, err := c.getInstanceByID(ctx, string(instanceID))
-		if err != nil {
+		if err != nil || instance == nil {
 			return nil, fmt.Errorf("failed to get instance by ID %s: %w", instanceID, err)
 		}