fix(aws): Only autocreate app elb security group on Create (#2025)

- Updated to only auto create <appname>-elb group for new ELBs
spinnaker · Oct 21, 2017 · 2c6f325 · 2c6f325
1 parent c7611e0
commit 2c6f325
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 67 deletions.
diff --git a/...er/clouddriver/aws/deploy/ops/loadbalancer/UpsertAmazonLoadBalancerAtomicOperation.groovy b/...er/clouddriver/aws/deploy/ops/loadbalancer/UpsertAmazonLoadBalancerAtomicOperation.groovy
@@ -127,24 +127,6 @@ class UpsertAmazonLoadBalancerAtomicOperation implements AtomicOperation<UpsertA
       }
 
       def securityGroupNamesToIds = regionScopedProvider.securityGroupService.getSecurityGroupIds(description.securityGroups, description.vpcId)
-      String application = null
-      try {
-        application = Names.parseName(description.name).getApp() ?: Names.parseName(description.clusterName).getApp()
-        IngressLoadBalancerGroupResult ingressLoadBalancerResult = ingressApplicationLoadBalancerGroup(
-          description,
-          application,
-          region,
-          listeners,
-          securityGroupLookupFactory
-        )
-
-        securityGroupNamesToIds.put(ingressLoadBalancerResult.groupName, ingressLoadBalancerResult.groupId)
-        task.updateStatus BASE_PHASE, "Authorized app ELB Security Group ${ingressLoadBalancerResult}"
-      } catch (Exception e) {
-        log.error("Failed to authorize app ELB security group {}-elb on application security group", application,  e)
-        task.updateStatus BASE_PHASE, "Failed to authorize app ELB security group ${application}-elb on application security group"
-      }
-
       def securityGroups = securityGroupNamesToIds.values()
       log.info("security groups on {} {}", description.name, securityGroups)
       String dnsName
@@ -155,6 +137,25 @@ class UpsertAmazonLoadBalancerAtomicOperation implements AtomicOperation<UpsertA
           subnetIds = regionScopedProvider.subnetAnalyzer.getSubnetIdsForZones(availabilityZones,
                   description.subnetType, SubnetTarget.ELB, 1)
         }
+
+        String application = null
+        try {
+          application = Names.parseName(description.name).getApp() ?: Names.parseName(description.clusterName).getApp()
+          IngressLoadBalancerGroupResult ingressLoadBalancerResult = ingressApplicationLoadBalancerGroup(
+            description,
+            application,
+            region,
+            listeners,
+            securityGroupLookupFactory
+          )
+
+          securityGroupNamesToIds.put(ingressLoadBalancerResult.groupName, ingressLoadBalancerResult.groupId)
+          task.updateStatus BASE_PHASE, "Authorized app ELB Security Group ${ingressLoadBalancerResult}"
+        } catch (Exception e) {
+          log.error("Failed to authorize app ELB security group {}-elb on application security group", application,  e)
+          task.updateStatus BASE_PHASE, "Failed to authorize app ELB security group ${application}-elb on application security group"
+        }
+
         dnsName = LoadBalancerUpsertHandler.createLoadBalancer(loadBalancing, loadBalancerName, isInternal, availabilityZones, subnetIds, listeners, securityGroups)
       } else {
         dnsName = loadBalancer.DNSName

diff --git a/...ver/aws/deploy/ops/loadbalancer/UpsertAmazonLoadBalancerClassicAtomicOperationSpec.groovy b/...ver/aws/deploy/ops/loadbalancer/UpsertAmazonLoadBalancerClassicAtomicOperationSpec.groovy
@@ -231,26 +231,11 @@ class UpsertAmazonLoadBalancerClassicAtomicOperationSpec extends Specification {
         internalPort: 8080
       ))
 
-    description.vpcId = "vpcId"
-
     when:
     operation.operate([])
 
-    then:
-    1 * securityGroupLookup.getSecurityGroupByName('bar', 'kato-elb', 'vpcId') >> Optional.of(elbSecurityGroupUpdater)
-    1 * securityGroupLookup.getSecurityGroupByName('bar', 'kato', 'vpcId') >> Optional.of(appSecurityGroupUpdater)
-    1 * elbSecurityGroupUpdater.getSecurityGroup() >> elbSecurityGroup
-    1 * appSecurityGroupUpdater.getSecurityGroup() >> applicationSecurityGroup
-    1 * appSecurityGroupUpdater.addIngress(_) >> {
-      def permissions = it[0] as List<IpPermission>
-      assert permissions.size() == 3
-      assert 7001 in permissions*.fromPort && 8501 in permissions*.fromPort
-      assert 7001 in permissions*.toPort && 8501 in permissions*.toPort
-      assert 8080 in permissions*.fromPort && 8080 in permissions*.fromPort
-      assert elbSecurityGroup.groupId in permissions[0].userIdGroupPairs*.groupId
-      assert elbSecurityGroup.groupId in permissions[1].userIdGroupPairs*.groupId
-      assert elbSecurityGroup.groupId in permissions[2].userIdGroupPairs*.groupId
-    }
+    then: 'should not auto create elb sg on update'
+    0 * appSecurityGroupUpdater.addIngress(_)
 
     and:
     1 * loadBalancing.describeLoadBalancers(new DescribeLoadBalancersRequest(loadBalancerNames: ["kato-main-frontend"])) >>
@@ -292,7 +277,6 @@ class UpsertAmazonLoadBalancerClassicAtomicOperationSpec extends Specification {
       )
     ]
     description.listeners.clear()
-    description.vpcId = "vpcId"
     description.listeners.add(
       new UpsertAmazonLoadBalancerClassicDescription.Listener(
         externalProtocol: UpsertAmazonLoadBalancerClassicDescription.Listener.ListenerType.TCP,
@@ -310,16 +294,6 @@ class UpsertAmazonLoadBalancerClassicAtomicOperationSpec extends Specification {
     operation.operate([])
 
     then:
-    1 * securityGroupLookup.getSecurityGroupByName('bar', 'kato-elb', 'vpcId') >> Optional.of(elbSecurityGroupUpdater)
-    1 * securityGroupLookup.getSecurityGroupByName('bar', 'kato', 'vpcId') >> Optional.of(appSecurityGroupUpdater)
-    1 * elbSecurityGroupUpdater.getSecurityGroup() >> elbSecurityGroup
-    1 * appSecurityGroupUpdater.getSecurityGroup() >> applicationSecurityGroup
-    1 * appSecurityGroupUpdater.addIngress(_) >> {
-      def permissions = it[0] as List<IpPermission>
-      assert permissions.size() == 3
-    }
-
-    and:
     thrown(AtomicOperationException)
 
     1 * loadBalancing.describeLoadBalancers(new DescribeLoadBalancersRequest(loadBalancerNames: ["kato-main-frontend"])) >>
@@ -357,13 +331,6 @@ class UpsertAmazonLoadBalancerClassicAtomicOperationSpec extends Specification {
     operation.operate([])
 
     then:
-    1 * securityGroupLookup.getSecurityGroupByName('bar', 'kato-elb', 'vpcId') >> Optional.of(elbSecurityGroupUpdater)
-    1 * securityGroupLookup.getSecurityGroupByName('bar', 'kato', 'vpcId') >> Optional.of(appSecurityGroupUpdater)
-    1 * elbSecurityGroupUpdater.getSecurityGroup() >> elbSecurityGroup
-    1 * appSecurityGroupUpdater.getSecurityGroup() >> applicationSecurityGroup
-    1 * appSecurityGroupUpdater.addIngress(_)
-
-    and:
     1 * loadBalancing.describeLoadBalancers(new DescribeLoadBalancersRequest(loadBalancerNames: ["kato-main-frontend"])) >>
             new DescribeLoadBalancersResult(loadBalancerDescriptions: [loadBalancer])
     1 * loadBalancing.modifyLoadBalancerAttributes(_) >> {  ModifyLoadBalancerAttributesRequest request ->
@@ -467,13 +434,6 @@ class UpsertAmazonLoadBalancerClassicAtomicOperationSpec extends Specification {
     operation.operate([])
 
     then:
-    1 * securityGroupLookup.getSecurityGroupByName('bar', 'kato-elb', 'vpcId') >> Optional.of(elbSecurityGroupUpdater)
-    1 * securityGroupLookup.getSecurityGroupByName('bar', 'kato', 'vpcId') >> Optional.of(appSecurityGroupUpdater)
-    1 * elbSecurityGroupUpdater.getSecurityGroup() >> elbSecurityGroup
-    1 * appSecurityGroupUpdater.getSecurityGroup() >> applicationSecurityGroup
-    1 * appSecurityGroupUpdater.addIngress(_)
-
-    and:
     1 * loadBalancing.describeLoadBalancers(_) >> new DescribeLoadBalancersResult(loadBalancerDescriptions: [loadBalancer])
     1 * loadBalancing.deleteLoadBalancerListeners(new DeleteLoadBalancerListenersRequest(loadBalancerPorts: [111]))
     1 * loadBalancing.createLoadBalancerListeners(new CreateLoadBalancerListenersRequest(
@@ -492,13 +452,6 @@ class UpsertAmazonLoadBalancerClassicAtomicOperationSpec extends Specification {
     operation.operate([])
 
     then:
-    1 * securityGroupLookup.getSecurityGroupByName('bar', 'kato-elb', 'vpcId') >> Optional.of(elbSecurityGroupUpdater)
-    1 * securityGroupLookup.getSecurityGroupByName('bar', 'kato', 'vpcId') >> Optional.of(appSecurityGroupUpdater)
-    1 * elbSecurityGroupUpdater.getSecurityGroup() >> elbSecurityGroup
-    1 * appSecurityGroupUpdater.getSecurityGroup() >> applicationSecurityGroup
-    1 * appSecurityGroupUpdater.addIngress(_)
-
-    and:
     1 * loadBalancing.describeLoadBalancers(_) >> new DescribeLoadBalancersResult(loadBalancerDescriptions: [loadBalancer])
     1 * loadBalancing.deleteLoadBalancerListeners(new DeleteLoadBalancerListenersRequest(loadBalancerPorts: [111]))
     0 * loadBalancing.deleteLoadBalancerListeners(_)