feat(provider/aws): Support specifying explicit subnet ids for deploy (…

…#2026)

Expectation is that a `subnetType` will still be provided and that
`subnetIds` will be a subnet of those valid for `subnetType`.

When explicit subnet ids are provided (or inherited), the newly created
server group will be tagged with:

`SPINNAKER_SUBNET_ID_OVERRIDE`: `"comma-separated list of subnet ids"`

clouddriver-aws/src/main/groovy/com/netflix/spinnaker/clouddriver/aws/AwsConfiguration.groovy

@@ -46,7 +46,9 @@ import com.netflix.spinnaker.clouddriver.aws.deploy.userdata.NullOpUserDataProvi
 import com.netflix.spinnaker.clouddriver.aws.deploy.userdata.UserDataProvider
 import com.netflix.spinnaker.clouddriver.aws.deploy.validators.BasicAmazonDeployDescriptionValidator
 import com.netflix.spinnaker.clouddriver.aws.model.AmazonBlockDevice
+import com.netflix.spinnaker.clouddriver.aws.model.AmazonServerGroup
 import com.netflix.spinnaker.clouddriver.aws.provider.AwsCleanupProvider
+import com.netflix.spinnaker.clouddriver.aws.provider.view.AmazonClusterProvider
 import com.netflix.spinnaker.clouddriver.aws.security.AWSProxy
 import com.netflix.spinnaker.clouddriver.aws.security.AmazonClientProvider
 import com.netflix.spinnaker.clouddriver.aws.security.AmazonCredentialsInitializer
@@ -63,6 +65,7 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty
 import org.springframework.boot.context.properties.ConfigurationProperties
 import org.springframework.boot.context.properties.EnableConfigurationProperties
+import org.springframework.context.ApplicationContext
 import org.springframework.context.annotation.Bean
 import org.springframework.context.annotation.ComponentScan
 import org.springframework.context.annotation.Configuration
@@ -215,8 +218,16 @@ class AwsConfiguration {
                                                     AccountCredentialsRepository accountCredentialsRepository,
                                                     DeployDefaults deployDefaults,
                                                     ScalingPolicyCopier scalingPolicyCopier,
-                                                    BlockDeviceConfig blockDeviceConfig) {
-    new BasicAmazonDeployHandler(regionScopedProviderFactory, accountCredentialsRepository, deployDefaults, scalingPolicyCopier, blockDeviceConfig)
+                                                    BlockDeviceConfig blockDeviceConfig,
+                                                    AmazonServerGroupProvider amazonServerGroupProvider) {
+    new BasicAmazonDeployHandler(
+      regionScopedProviderFactory,
+      accountCredentialsRepository,
+      amazonServerGroupProvider,
+      deployDefaults,
+      scalingPolicyCopier,
+      blockDeviceConfig
+    )
   }
 
   @Bean
@@ -289,4 +300,21 @@ class AwsConfiguration {
 
     new AwsCleanupProviderSynchronizer()
   }
+
+  @Bean
+  AmazonServerGroupProvider amazonServerGroupProvider(ApplicationContext applicationContext) {
+    return new AmazonServerGroupProvider(applicationContext)
+  }
+
+  class AmazonServerGroupProvider {
+    ApplicationContext applicationContext
+
+    AmazonServerGroupProvider(ApplicationContext applicationContext) {
+      this.applicationContext = applicationContext
+    }
+
+    AmazonServerGroup getServerGroup(String account, String region, String serverGroupName) {
+      return applicationContext.getBean(AmazonClusterProvider).getServerGroup(account, region, serverGroupName)
+    }
+  }
 }

clouddriver-aws/src/main/groovy/com/netflix/spinnaker/clouddriver/aws/deploy/AutoScalingWorker.groovy

@@ -60,6 +60,7 @@ class AutoScalingWorker {
   private Boolean startDisabled
   private Boolean associatePublicIpAddress
   private String subnetType
+  private List<String> subnetIds
   private Integer cooldown
   private Collection<String> enabledMetrics
   private Integer healthCheckGracePeriod
@@ -155,11 +156,24 @@ class AutoScalingWorker {
    *
    * @return list of subnet ids applicable to this deployment.
    */
-  List<String> getSubnetIds() {
-    subnetType ? getSubnets().subnetId : []
+  List<String> getSubnetIds(List<Subnet> allSubnetsForTypeAndAvailabilityZone) {
+    def subnetIds = allSubnetsForTypeAndAvailabilityZone*.subnetId
+
+    def invalidSubnetIds = (this.subnetIds ?: []).findAll { !subnetIds.contains(it) }
+    if (invalidSubnetIds) {
+      throw new IllegalStateException(
+        "One or more subnet ids are not valid (invalidSubnetIds: ${invalidSubnetIds.join(", ")}, subnetType: ${subnetType}, availabilityZones: ${availabilityZones})"
+      )
+    }
+
+    return this.subnetIds ?: subnetIds
   }
 
   private List<Subnet> getSubnets() {
+    if (!subnetType) {
+      return []
+    }
+
     DescribeSubnetsResult result = regionScopedProvider.amazonEC2.describeSubnets()
     List<Subnet> mySubnets = []
     for (subnet in result.subnets) {
@@ -203,11 +217,11 @@ class AutoScalingWorker {
     }
 
     // Favor subnetIds over availability zones
-    def subnetIds = subnetIds?.join(',')
+    def subnetIds = getSubnetIds(getSubnets())?.join(',')
     if (subnetIds) {
       task.updateStatus AWS_PHASE, " > Deploying to subnetIds: $subnetIds"
       request.withVPCZoneIdentifier(subnetIds)
-    } else if (subnetType && !subnets) {
+    } else if (subnetType && !getSubnets()) {
       throw new RuntimeException("No suitable subnet was found for internal subnet purpose '${subnetType}'!")
     } else {
       task.updateStatus AWS_PHASE, "Deploying to availabilityZones: $availabilityZones"

clouddriver-aws/src/main/groovy/com/netflix/spinnaker/clouddriver/aws/deploy/description/BasicAmazonDeployDescription.groovy

@@ -32,6 +32,7 @@ class BasicAmazonDeployDescription extends AbstractAmazonCredentialsDescription
   String freeFormDetails
   String instanceType
   String subnetType
+  List<String> subnetIds
   String iamRole
   String keyPair
   Boolean associatePublicIpAddress
@@ -65,6 +66,11 @@ class BasicAmazonDeployDescription extends AbstractAmazonCredentialsDescription
    */
   boolean copySourceCustomBlockDeviceMappings = true
 
+  /**
+   * If false, the newly created server group will not pick up overridden subnet ids from an ancestor group
+   */
+  boolean copySourceSubnetIdOverrides = true
+
   String classicLinkVpcId
   List<String> classicLinkVpcSecurityGroups
 

clouddriver-aws/src/main/groovy/com/netflix/spinnaker/clouddriver/aws/deploy/handlers/BasicAmazonDeployHandler.groovy

@@ -53,6 +53,8 @@ import java.util.regex.Pattern
 @Slf4j
 class BasicAmazonDeployHandler implements DeployHandler<BasicAmazonDeployDescription> {
   private static final String BASE_PHASE = "DEPLOY"
+  private static final String SUBNET_ID_OVERRIDE_TAG = "SPINNAKER_SUBNET_ID_OVERRIDE"
+
 
   private static final KNOWN_VIRTUALIZATION_FAMILIES = [
     paravirtual: ['c1', 'c3', 'hi1', 'hs1', 'm1', 'm2', 'm3', 't1'],
@@ -70,6 +72,7 @@ class BasicAmazonDeployHandler implements DeployHandler<BasicAmazonDeployDescrip
 
   private final RegionScopedProviderFactory regionScopedProviderFactory
   private final AccountCredentialsRepository accountCredentialsRepository
+  private final AwsConfiguration.AmazonServerGroupProvider amazonServerGroupProvider
   private final AwsConfiguration.DeployDefaults deployDefaults
   private final ScalingPolicyCopier scalingPolicyCopier
   private final BlockDeviceConfig blockDeviceConfig
@@ -78,11 +81,13 @@ class BasicAmazonDeployHandler implements DeployHandler<BasicAmazonDeployDescrip
 
   BasicAmazonDeployHandler(RegionScopedProviderFactory regionScopedProviderFactory,
                            AccountCredentialsRepository accountCredentialsRepository,
+                           AwsConfiguration.AmazonServerGroupProvider amazonServerGroupProvider,
                            AwsConfiguration.DeployDefaults deployDefaults,
                            ScalingPolicyCopier scalingPolicyCopier,
                            BlockDeviceConfig blockDeviceConfig) {
     this.regionScopedProviderFactory = regionScopedProviderFactory
     this.accountCredentialsRepository = accountCredentialsRepository
+    this.amazonServerGroupProvider = amazonServerGroupProvider
     this.deployDefaults = deployDefaults
     this.scalingPolicyCopier = scalingPolicyCopier
     this.blockDeviceConfig = blockDeviceConfig
@@ -269,6 +274,7 @@ class BasicAmazonDeployHandler implements DeployHandler<BasicAmazonDeployDescrip
         instanceType: description.instanceType,
         availabilityZones: availabilityZones,
         subnetType: subnetType,
+        subnetIds: description.subnetIds,
         classicLoadBalancers: loadBalancers.classicLoadBalancers,
         targetGroupArns: targetGroups.targetGroupARNs,
         cooldown: description.cooldown,
@@ -329,6 +335,35 @@ class BasicAmazonDeployHandler implements DeployHandler<BasicAmazonDeployDescrip
                                                     String sourceAsgName, Boolean useSourceCapacity,
                                                     BasicAmazonDeployDescription description) {
 
+    def copySourceSubnetIdOverrides = description.copySourceSubnetIdOverrides && !description.subnetIds
+    if (copySourceSubnetIdOverrides && sourceRegionScopedProvider && sourceAsgName) {
+      // avoid unnecessary AWS calls by fetching a cached copy of the source server group
+      def serverGroup = amazonServerGroupProvider.getServerGroup(
+        sourceRegionScopedProvider.amazonCredentials?.name,
+        sourceRegionScopedProvider.region,
+        sourceAsgName
+      )
+
+      if (serverGroup) {
+        String subnetIdTag = serverGroup.asg.tags.find { it.key == SUBNET_ID_OVERRIDE_TAG }?.value
+        if (subnetIdTag) {
+          // source server group had subnet id overrides, propagate them forward
+          description.subnetIds = subnetIdTag.split(",")
+        }
+      }
+    }
+
+    if (description.subnetIds) {
+      /*
+       * Ensure the new server group receives the subnet id override tag.
+       *
+       * These subnet ids will be validated against the region / availability zones and provided subnet type.
+       *
+       * The deploy will fail (and tag discarded!) if any of the ids are invalid.
+       */
+      description.tags[SUBNET_ID_OVERRIDE_TAG] = description.subnetIds.join(",")
+    }
+
     //skip a couple of AWS calls if we won't use any of the data
     if (!(useSourceCapacity || description.copySourceCustomBlockDeviceMappings)) {
       return description

clouddriver-aws/src/test/groovy/com/netflix/spinnaker/clouddriver/aws/deploy/AutoScalingWorkerUnitSpec.groovy

@@ -18,6 +18,8 @@ package com.netflix.spinnaker.clouddriver.aws.deploy
 
 import com.amazonaws.services.autoscaling.AmazonAutoScaling
 import com.amazonaws.services.autoscaling.model.AutoScalingGroup
+import com.amazonaws.services.ec2.AmazonEC2
+import com.amazonaws.services.ec2.model.Subnet
 import com.netflix.spinnaker.clouddriver.data.task.DefaultTask
 import com.netflix.spinnaker.clouddriver.data.task.Task
 import com.netflix.spinnaker.clouddriver.data.task.TaskRepository
@@ -40,13 +42,15 @@ class AutoScalingWorkerUnitSpec extends Specification {
   def asgService = Mock(AsgService)
   def autoScaling = Mock(AmazonAutoScaling)
   def clusterProvider = Mock(ClusterProvider)
+  def amazonEC2 = Mock(AmazonEC2)
   def awsServerGroupNameResolver = new AWSServerGroupNameResolver('test', 'us-east-1', asgService, [clusterProvider])
   def credential = TestCredential.named('foo')
   def regionScopedProvider = Stub(RegionScopedProviderFactory.RegionScopedProvider) {
     getAutoScaling() >> autoScaling
     getLaunchConfigurationBuilder() >> lcBuilder
     getAsgService() >> asgService
     getAWSServerGroupNameResolver() >> awsServerGroupNameResolver
+    getAmazonEC2() >> amazonEC2
   }
 
   def setup() {
@@ -87,10 +91,10 @@ class AutoScalingWorkerUnitSpec extends Specification {
   void "deploy derives name from ancestor asg and sets the ancestor asg name in the task result"() {
     setup:
     def autoScalingWorker = new AutoScalingWorker(
-      regionScopedProvider : regionScopedProvider,
+      regionScopedProvider: regionScopedProvider,
       credentials: credential,
-      application : "myasg",
-      region : "us-east-1"
+      application: "myasg",
+      region: "us-east-1"
     )
 
     when:
@@ -119,10 +123,10 @@ class AutoScalingWorkerUnitSpec extends Specification {
     def autoScalingWorker = new AutoScalingWorker(
       enabledMetrics: [],
       instanceMonitoring: true,
-      regionScopedProvider : regionScopedProvider,
+      regionScopedProvider: regionScopedProvider,
       credentials: credential,
-      application : "myasg",
-      region : "us-east-1"
+      application: "myasg",
+      region: "us-east-1"
     )
 
     when:
@@ -137,10 +141,10 @@ class AutoScalingWorkerUnitSpec extends Specification {
     def autoScalingWorker = new AutoScalingWorker(
       enabledMetrics: ['GroupMinSize', 'GroupMaxSize'],
       instanceMonitoring: false,
-      regionScopedProvider : regionScopedProvider,
+      regionScopedProvider: regionScopedProvider,
       credentials: credential,
-      application : "myasg",
-      region : "us-east-1"
+      application: "myasg",
+      region: "us-east-1"
     )
 
     when:
@@ -156,10 +160,10 @@ class AutoScalingWorkerUnitSpec extends Specification {
     def autoScalingWorker = new AutoScalingWorker(
       enabledMetrics: ['GroupMinSize', 'GroupMaxSize'],
       instanceMonitoring: true,
-      regionScopedProvider : regionScopedProvider,
+      regionScopedProvider: regionScopedProvider,
       credentials: credential,
-      application : "myasg",
-      region : "us-east-1"
+      application: "myasg",
+      region: "us-east-1"
     )
 
     when:
@@ -169,6 +173,39 @@ class AutoScalingWorkerUnitSpec extends Specification {
     1 * autoScaling.enableMetricsCollection({ it.metrics == ['GroupMinSize', 'GroupMaxSize'] })
   }
 
+  @Unroll
+  void "should validate provided subnet ids against those available for subnet type"() {
+    given:
+    def autoScalingWorker = new AutoScalingWorker(
+      subnetIds: subnetIds
+    )
+
+    when:
+    def filteredSubnetIds = autoScalingWorker.getSubnetIds(allSubnets)
+
+    then:
+    filteredSubnetIds == expectedSubnetIds
+
+    when:
+    autoScalingWorker = new AutoScalingWorker(
+      subnetIds: ["invalid-subnet-id"]
+    )
+    autoScalingWorker.getSubnetIds(allSubnets)
+
+    then:
+    def e = thrown(IllegalStateException)
+    e.message.startsWith("One or more subnet ids are not valid (invalidSubnetIds: invalid-subnet-id")
+
+    where:
+    subnetIds    | allSubnets                               || expectedSubnetIds
+    ["subnet-1"] | [subnet("subnet-1"), subnet("subnet-2")] || ["subnet-1"]
+    null         | [subnet("subnet-1"), subnet("subnet-2")] || ["subnet-1", "subnet-2"]
+  }
+
+  static Subnet subnet(String subnetId) {
+    return new Subnet().withSubnetId(subnetId)
+  }
+
   static ServerGroup sG(String name, Long createdTime, String region) {
     return new SimpleServerGroup(name: name, createdTime: createdTime, region: region)
   }