fix(ecs): Additional null checks around cached EC2 instance data (#3901…

…) (#3908)

Fixes spinnaker/spinnaker#4657

clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/provider/view/EcsServerClusterProvider.java

@@ -343,11 +343,15 @@ private EcsServerGroup buildEcsServerGroup(
             com.amazonaws.services.ec2.model.Instance ec2Instance =
                 containerInformationService.getEc2Instance(account, region, task);
             if (ec2Instance != null) {
-              vpcId = ec2Instance.getVpcId();
-              securityGroups =
-                  ec2Instance.getSecurityGroups().stream()
-                      .map(GroupIdentifier::getGroupId)
-                      .collect(Collectors.toSet());
+              if (ec2Instance.getVpcId() != null && !ec2Instance.getVpcId().isEmpty()) {
+                vpcId = ec2Instance.getVpcId();
+              }
+              if (ec2Instance.getSecurityGroups() != null) {
+                securityGroups =
+                    ec2Instance.getSecurityGroups().stream()
+                        .map(GroupIdentifier::getGroupId)
+                        .collect(Collectors.toSet());
+              }
               break;
             }
           }

clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/services/ContainerInformationService.java

@@ -168,12 +168,16 @@ public String getTaskPrivateAddress(String accountName, String region, Task task
     }
 
     String hostPrivateIpAddress = instance.getPrivateIpAddress();
+    if (hostPrivateIpAddress == null || hostPrivateIpAddress.isEmpty()) {
+      return null;
+    }
+
     return String.format("%s:%s", hostPrivateIpAddress, hostPort);
   }
 
   public String getTaskZone(String accountName, String region, Task task) {
     Instance ec2Instance = getEc2Instance(accountName, region, task);
-    if (ec2Instance != null) {
+    if (ec2Instance != null && ec2Instance.getPlacement() != null) {
       return ec2Instance.getPlacement().getAvailabilityZone();
     }
 

clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/provider/view/EcsServerClusterProviderSpec.groovy

@@ -321,7 +321,7 @@ class EcsServerClusterProviderSpec extends Specification {
     retrievedCluster == expectedCluster
   }
 
-  def 'should produce an ecs cluster with unknown VPC'() {
+  def 'should produce an ecs cluster with unknown VPC for missing EC2 instance'() {
     given:
     for (serverGroup in expectedCluster.serverGroups) {
       EcsServerGroup ecsServerGroup = serverGroup
@@ -337,6 +337,22 @@ class EcsServerClusterProviderSpec extends Specification {
     retrievedCluster == expectedCluster
   }
 
+  def 'should produce an ecs cluster with unknown VPC for EC2 instance missing its network config'() {
+    given:
+    for (serverGroup in expectedCluster.serverGroups) {
+      EcsServerGroup ecsServerGroup = serverGroup
+      ecsServerGroup.vpcId = 'None'
+      ecsServerGroup.securityGroups = []
+    }
+
+    when:
+    def retrievedCluster = provider.getCluster("myapp", CREDS_NAME, FAMILY_NAME)
+
+    then:
+    containerInformationService.getEc2Instance(_, _, _) >> new Instance()
+    retrievedCluster == expectedCluster
+  }
+
   def 'should produce ecs clusters'() {
     when:
     def retrievedClusters = provider.getClusterDetails("myapp").values().flatten()

clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/services/ContainerInformationServiceSpec.groovy

@@ -268,6 +268,48 @@ class ContainerInformationServiceSpec extends Specification {
     retrievedIp == null
   }
 
+  def 'should return a null when container instance IP address is not yet cached'() {
+    given:
+    def account = 'test-account'
+    def region = 'us-west-1'
+    def containerInstanceArn = 'container-instance-arn'
+    def port = 1337
+
+    def ecsAccount = new ECSCredentialsConfig.Account(
+      name: account,
+      awsAccount: 'aws-' + account
+    )
+
+    def task = new Task(
+      containerInstanceArn: containerInstanceArn,
+      containers: [
+        new Container(
+          networkBindings: [
+            new NetworkBinding(
+              hostPort: port
+            )
+          ]
+        )
+      ]
+    )
+
+    def containerInstance = new ContainerInstance(
+      ec2InstanceId: 'i-deadbeef'
+    )
+
+    def instance = new Instance()
+
+    containerInstanceCacheClient.get(_) >> containerInstance
+    ecsInstanceCacheClient.find(_, _, _) >> [instance]
+    ecsCredentialsConfig.getAccounts() >> [ecsAccount]
+
+    when:
+    def retrievedIp = service.getTaskPrivateAddress(account, region, task)
+
+    then:
+    retrievedIp == null
+  }
+
   def 'should return a unknown when there is no container instance for the task'() {
     given:
     def task = new Task(
@@ -502,6 +544,31 @@ class ContainerInformationServiceSpec extends Specification {
     retrievedZone == null
   }
 
+  def 'should return null when container instance zone is not yet cached'() {
+    given:
+    def task = new Task(containerInstanceArn: 'container-instance-arn')
+    def containerInstance = new ContainerInstance(ec2InstanceId: 'i-deadbeef')
+    def ecsAccount = new ECSCredentialsConfig.Account(
+      name: 'ecs-account',
+      awsAccount: 'aws-test-account'
+    )
+    def givenInstance = new Instance(
+      instanceId: 'i-deadbeef',
+      privateIpAddress: '0.0.0.0',
+      publicIpAddress: '127.0.0.1'
+    )
+
+    containerInstanceCacheClient.get(_) >> containerInstance
+    ecsCredentialsConfig.getAccounts() >> [ecsAccount]
+    ecsInstanceCacheClient.find(_, _, _) >> [givenInstance]
+
+    when:
+    def retrievedZone = service.getTaskZone('ecs-account', 'us-west-1', task)
+
+    then:
+    retrievedZone == null
+  }
+
   def 'should return a cluster name'() {
     given:
     def originalClusterName = 'test-cluster'