Validate cloud provider id and return error

Author: bianchi2

pkg/cloudprovider/aws/aws.go

@@ -40,8 +40,15 @@ func instanceToProviderID(instance *autoscaling.Instance) string {
 	return fmt.Sprintf("aws:///%s/%s", *instance.AvailabilityZone, *instance.InstanceId)
 }
 
-func providerIDToInstanceID(providerID string) string {
-	return strings.Split(providerID, "/")[4]
+func providerIDToInstanceID(providerID string) (string, error) {
+	if providerID == "" {
+		return "", fmt.Errorf("empty providerID, it may be set later by cloud controller")
+	}
+	parts := strings.Split(providerID, "/")
+	if len(parts) < 5 {
+		return "", fmt.Errorf("malformed providerID %s: expected at least 4 slashes", providerID)
+	}
+	return strings.Split(providerID, "/")[4], nil
 }
 
 // CloudProvider providers an aws cloud provider implementation
@@ -136,8 +143,10 @@ type Instance struct {
 func (c *CloudProvider) GetInstance(node *v1.Node) (cloudprovider.Instance, error) {
 	var instance *Instance
 
-	id := providerIDToInstanceID(node.Spec.ProviderID)
-
+	id, err := providerIDToInstanceID(node.Spec.ProviderID)
+	if err != nil {
+		return instance, errors.New("no cloud provider id found, it may be set later by cloud controller")
+	}
 	input := &ec2.DescribeInstancesInput{
 		InstanceIds: []*string{&id},
 	}

pkg/cloudprovider/aws/aws_test.go

@@ -70,7 +70,21 @@ func TestInstanceToProviderId(t *testing.T) {
 }
 
 func TestProviderIdToInstanceId(t *testing.T) {
-	assert.Equal(t, "abc123", providerIDToInstanceID("aws:///us-east-1b/abc123"))
+	id, err := providerIDToInstanceID("aws:///us-east-1b/abc123")
+	assert.Nil(t, err)
+	assert.Equal(t, "abc123", id)
+}
+
+func TestProviderIdToInstanceIdEmpty(t *testing.T) {
+	_, err := providerIDToInstanceID("")
+	assert.NotNil(t, err)
+	assert.Contains(t, err.Error(), "empty providerID, it may be set later by cloud controller")
+}
+
+func TestProviderIdToInstanceIdMalformed(t *testing.T) {
+	_, err := providerIDToInstanceID("fake://provider/id")
+	assert.NotNil(t, err)
+	assert.Contains(t, err.Error(), "malformed providerID fake://provider/id: expected at least 4 slashes")
 }
 
 func newMockCloudProvider(nodeGroups []string, service *test.MockAutoscalingService, ec2Service *test.MockEc2Service) (*CloudProvider, error) {

pkg/cloudprovider/aws/cloud_provider_test.go

@@ -308,3 +308,14 @@ func TestCloudProvider_GetInstance(t *testing.T) {
 		})
 	}
 }
+
+func TestCloudProvider_GetInstance_No_Provider_ID(t *testing.T) {
+	nodeGroups := []string{"1"}
+	node := &v1.Node{
+		Spec: v1.NodeSpec{},
+	}
+	ec2Service := &test.MockEc2Service{}
+	awsCloudProvider, err := newMockCloudProvider(nodeGroups, nil, ec2Service)
+	_, err = awsCloudProvider.GetInstance(node)
+	assert.NotNil(t, err)
+}