From 5d15f0637a6c73ff7c00ff2dcfc86b2d53d74c5f Mon Sep 17 00:00:00 2001
From: Keerthan Reddy Mala <kremala@amazon.com>
Date: Fri, 12 May 2023 17:56:22 -0700
Subject: [PATCH] ignore invalid providerid for cache indexer

---
 pkg/providers/v1/aws.go      | 10 ++++---
 pkg/providers/v1/aws_test.go | 56 +++++++++++++++++++++++++++++++++++-
 2 files changed, 61 insertions(+), 5 deletions(-)

diff --git a/pkg/providers/v1/aws.go b/pkg/providers/v1/aws.go
index b56d70f019..b77944d126 100644
--- a/pkg/providers/v1/aws.go
+++ b/pkg/providers/v1/aws.go
@@ -45,10 +45,8 @@ import (
 	"github.com/aws/aws-sdk-go/service/kms"
 	"github.com/aws/aws-sdk-go/service/sts"
 	"gopkg.in/gcfg.v1"
-	v1 "k8s.io/api/core/v1"
-	"k8s.io/klog/v2"
-	netutils "k8s.io/utils/net"
 
+	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
@@ -68,6 +66,8 @@ import (
 	cloudvolume "k8s.io/cloud-provider/volume"
 	volerr "k8s.io/cloud-provider/volume/errors"
 	volumehelpers "k8s.io/cloud-provider/volume/helpers"
+	"k8s.io/klog/v2"
+	netutils "k8s.io/utils/net"
 )
 
 // NLBHealthCheckRuleDescription is the comment used on a security group rule to
@@ -841,7 +841,9 @@ func InstanceIDIndexFunc(obj interface{}) ([]string, error) {
 	}
 	instanceID, err := KubernetesInstanceID(node.Spec.ProviderID).MapToAWSInstanceID()
 	if err != nil {
-		return []string{""}, fmt.Errorf("error mapping node %q's provider ID %q to instance ID: %v", node.Name, node.Spec.ProviderID, err)
+		//logging the error as warning as Informer.AddIndexers would panic if there is an error
+		klog.Warningf("error mapping node %q's provider ID %q to instance ID: %v", node.Name, node.Spec.ProviderID, err)
+		return []string{""}, nil
 	}
 	return []string{string(instanceID)}, nil
 }
diff --git a/pkg/providers/v1/aws_test.go b/pkg/providers/v1/aws_test.go
index 5f9f1dd869..88a2e965d0 100644
--- a/pkg/providers/v1/aws_test.go
+++ b/pkg/providers/v1/aws_test.go
@@ -36,7 +36,6 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
-
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
@@ -4007,3 +4006,58 @@ func TestDescribeInstances(t *testing.T) {
 		})
 	}
 }
+
+func TestInstanceIDIndexFunc(t *testing.T) {
+	type args struct {
+		obj interface{}
+	}
+	tests := []struct {
+		name    string
+		args    args
+		want    []string
+		wantErr bool
+	}{
+		{
+			name: "returns empty on invalid provider id",
+			args: args{
+				obj: &v1.Node{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "test-node",
+					},
+					Spec: v1.NodeSpec{
+						ProviderID: "foo://com-2351",
+					},
+				},
+			},
+			want:    []string{""},
+			wantErr: false,
+		},
+		{
+			name: "returns correct instance id on valid provider id",
+			args: args{
+				obj: &v1.Node{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "test-valid-node",
+					},
+					Spec: v1.NodeSpec{
+						ProviderID: "aws:////i-12345678abcdef01",
+					},
+				},
+			},
+			want:    []string{"i-12345678abcdef01"},
+			wantErr: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := InstanceIDIndexFunc(tt.args.obj)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("InstanceIDIndexFunc() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("InstanceIDIndexFunc() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}