fix(ec2): instance may not have a name (#3639)

Problem
Currently the toolkit is unable to load EC2 instances that are not assigned a name. It causes it to fail to load all of the instances associated with a specific region.

Solution
displayed as (no name) in QuickPick.
displayed with instanceId in explorer.
behavior above captured in test suite.

Author: Hweinstock

src/ec2/explorer/ec2InstanceNode.ts

@@ -40,7 +40,7 @@ export class Ec2InstanceNode extends AWSTreeNodeBase implements AWSResourceNode
     }
 
     public get name(): string {
-        return getNameOfInstance(this.instance) ?? 'Unnamed instance'
+        return getNameOfInstance(this.instance) ?? `${this.InstanceId} (no name)`
     }
 
     public get InstanceId(): string {

src/ec2/explorer/ec2ParentNode.ts

@@ -39,8 +39,9 @@ export class Ec2ParentNode extends AWSTreeNodeBase {
     }
 
     public async updateChildren(): Promise<void> {
-        const ec2Instances = await (await this.ec2Client.getInstances()).toMap(instance => getNameOfInstance(instance))
-
+        const ec2Instances = await (
+            await this.ec2Client.getInstances()
+        ).toMap(instance => getNameOfInstance(instance) ?? instance.InstanceId)
         updateInPlace(
             this.ec2InstanceNodes,
             ec2Instances.keys(),

src/shared/clients/ec2Client.ts

@@ -43,7 +43,9 @@ export class Ec2Client {
             .flatten()
             .filter(instance => instance!.InstanceId !== undefined)
             .map(instance => {
-                return instance!.Tags ? { ...instance, name: lookupTagKey(instance!.Tags!, 'Name') } : instance!
+                return instanceHasName(instance!)
+                    ? { ...instance, name: lookupTagKey(instance!.Tags!, 'Name') }
+                    : instance!
             })
     }
 
@@ -109,9 +111,13 @@ export class Ec2Client {
 }
 
 export function getNameOfInstance(instance: EC2.Instance): string | undefined {
-    return instance.Tags ? lookupTagKey(instance.Tags, 'Name') : undefined
+    return instanceHasName(instance) ? lookupTagKey(instance.Tags!, 'Name')! : undefined
+}
+
+export function instanceHasName(instance: EC2.Instance): boolean {
+    return instance.Tags !== undefined && instance.Tags.filter(tag => tag.Key === 'Name').length != 0
 }
 
 function lookupTagKey(tags: EC2.Tag[], targetKey: string) {
-    return tags.filter(tag => tag.Key == targetKey)[0].Value
+    return tags.filter(tag => tag.Key === targetKey)[0].Value
 }

src/test/shared/clients/defaultEc2Client.test.ts

@@ -7,7 +7,7 @@ import * as assert from 'assert'
 import { AsyncCollection } from '../../../shared/utilities/asyncCollection'
 import { toCollection } from '../../../shared/utilities/asyncCollection'
 import { intoCollection } from '../../../shared/utilities/collectionUtils'
-import { Ec2Client } from '../../../shared/clients/ec2Client'
+import { Ec2Client, instanceHasName } from '../../../shared/clients/ec2Client'
 import { EC2 } from 'aws-sdk'
 
 describe('extractInstancesFromReservations', function () {
@@ -93,9 +93,49 @@ describe('extractInstancesFromReservations', function () {
                 actualResult
             )
         })
+
+    it('can process results without complete Tag field.', async function () {
+        const testReservationsList: EC2.ReservationList = [
+            {
+                Instances: [
+                    {
+                        InstanceId: 'id1',
+                        Tags: [{ Key: 'Name', Value: 'name1' }],
+                    },
+                    {
+                        InstanceId: 'id2',
+                    },
+                ],
+            },
+            {
+                Instances: [
+                    {
+                        InstanceId: 'id3',
+                        Tags: [{ Key: 'Name', Value: 'name3' }],
+                    },
+                    {
+                        InstanceId: 'id4',
+                        Tags: [],
+                    },
+                ],
+            },
+        ]
+
+        const actualResult = await client.getInstancesFromReservations(intoCollection([testReservationsList])).promise()
+
+        assert.deepStrictEqual(
+            [
+                { InstanceId: 'id1', name: 'name1', Tags: [{ Key: 'Name', Value: 'name1' }] },
+                { InstanceId: 'id2' },
+                { InstanceId: 'id3', name: 'name3', Tags: [{ Key: 'Name', Value: 'name3' }] },
+                { InstanceId: 'id4', Tags: [] },
+            ],
+            actualResult
+        )
+    })
 })
 
-describe('getSingleInstanceFilter', function () {
+describe('getInstancesFilter', function () {
     const client = new Ec2Client('')
 
     it('returns proper filter when given instanceId', function () {
@@ -122,3 +162,26 @@ describe('getSingleInstanceFilter', function () {
         assert.deepStrictEqual(expectedFilters2, actualFilters2)
     })
 })
+
+describe('instanceHasName', function () {
+    it('returns whether or not there is name attached to instance', function () {
+        const instances = [
+            { InstanceId: 'id1', Tags: [] },
+            { InstanceId: 'id2', name: 'name2', Tags: [{ Key: 'Name', Value: 'name2' }] },
+            { InstanceId: 'id3', Tags: [{ Key: 'NotName', Value: 'notAName' }] },
+            {
+                InstanceId: 'id4',
+                name: 'name4',
+                Tags: [
+                    { Key: 'Name', Value: 'name4' },
+                    { Key: 'anotherKey', Value: 'Another Key' },
+                ],
+            },
+        ]
+
+        assert.deepStrictEqual(false, instanceHasName(instances[0]))
+        assert.deepStrictEqual(true, instanceHasName(instances[1]))
+        assert.deepStrictEqual(false, instanceHasName(instances[2]))
+        assert.deepStrictEqual(true, instanceHasName(instances[3]))
+    })
+})