Merge pull request #1417 from anotherchrisberry/lfh-cleanse

provider/amazon: clean lifecycle hook names
spinnaker · Feb 7, 2017 · 19e5841 · 19e5841
2 parents 5f3725c + 6ab3bed
commit 19e5841
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 2 deletions.
diff --git a/...rc/main/groovy/com/netflix/spinnaker/clouddriver/aws/deploy/AsgLifecycleHookWorker.groovy b/...rc/main/groovy/com/netflix/spinnaker/clouddriver/aws/deploy/AsgLifecycleHookWorker.groovy
@@ -55,7 +55,7 @@ class AsgLifecycleHookWorker {
         case AmazonAsgLifecycleHook.TransitionType.LIFECYCLE:
           def request = new PutLifecycleHookRequest(
             autoScalingGroupName: targetAsgName,
-            lifecycleHookName: lifecycleHookName,
+            lifecycleHookName: cleanLifecycleHookName(lifecycleHookName),
             roleARN: arnTemplater(lifecycleHook.roleARN, targetRegion, targetAccountId),
             notificationTargetARN: arnTemplater(lifecycleHook.notificationTargetARN, targetRegion, targetAccountId),
             notificationMetadata: lifecycleHook.notificationMetadata,
@@ -84,4 +84,8 @@ class AsgLifecycleHookWorker {
   private static String arnTemplater(String arnTemplate, String region, String accountId) {
     arnTemplate.replaceAll(REGION_TEMPLATE_PATTERN, region).replaceAll(ACCOUNT_ID_TEMPLATE_PATTERN, accountId)
   }
+
+  public static String cleanLifecycleHookName(String name) {
+    return name.replaceAll("[^A-Za-z0-9\\-_/]", "_")
+  }
 }
diff --git a/...netflix/spinnaker/clouddriver/aws/deploy/ops/UpsertAsgLifecycleHookAtomicOperation.groovy b/...netflix/spinnaker/clouddriver/aws/deploy/ops/UpsertAsgLifecycleHookAtomicOperation.groovy
@@ -17,6 +17,7 @@
 package com.netflix.spinnaker.clouddriver.aws.deploy.ops
 
 import com.amazonaws.services.autoscaling.model.PutLifecycleHookRequest
+import com.netflix.spinnaker.clouddriver.aws.deploy.AsgLifecycleHookWorker
 import com.netflix.spinnaker.clouddriver.aws.deploy.description.UpsertAsgLifecycleHookDescription
 import com.netflix.spinnaker.clouddriver.aws.security.AmazonClientProvider
 import com.netflix.spinnaker.clouddriver.aws.services.IdGenerator
@@ -42,7 +43,7 @@ class UpsertAsgLifecycleHookAtomicOperation implements AtomicOperation<Void> {
   Void operate(List priorOutputs) {
     final lifecycleHookName = description.name ?: "${description.serverGroupName}-lifecycle-${idGenerator.nextId()}"
     final request = new PutLifecycleHookRequest(
-      lifecycleHookName: lifecycleHookName,
+      lifecycleHookName: AsgLifecycleHookWorker.cleanLifecycleHookName(lifecycleHookName),
       autoScalingGroupName: description.serverGroupName,
       lifecycleTransition: description.lifecycleTransition.toString(),
       roleARN: description.roleARN,

diff --git a/...est/groovy/com/netflix/spinnaker/clouddriver/aws/deploy/AsgLifecycleHookWorkerSpec.groovy b/...est/groovy/com/netflix/spinnaker/clouddriver/aws/deploy/AsgLifecycleHookWorkerSpec.groovy
@@ -54,6 +54,31 @@ class AsgLifecycleHookWorkerSpec extends Specification {
     0 * autoScaling.putLifecycleHook(_)
   }
 
+  void 'should create clean lifecycle hook name'() {
+    given:
+    def hook = new AmazonAsgLifecycleHook(
+        roleARN: 'arn:aws:iam::123456789012:role/my-notification-role',
+        notificationTargetARN: 'arn:aws:sns:us-east-1:123456789012:my-sns-topic',
+        lifecycleTransition: AmazonAsgLifecycleHook.Transition.EC2InstanceTerminating,
+        heartbeatTimeout: 3600,
+        defaultResult: AmazonAsgLifecycleHook.DefaultResult.ABANDON
+      )
+
+    when:
+    asgLifecycleHookWorker.attach(Mock(Task), [hook], 'asg-foo.bar.baz-v001')
+
+    then:
+    1 * autoScaling.putLifecycleHook(new PutLifecycleHookRequest(
+      lifecycleHookName: 'asg-foo_bar_baz-v001-lifecycle-1',
+      autoScalingGroupName: 'asg-foo.bar.baz-v001',
+      lifecycleTransition: 'autoscaling:EC2_INSTANCE_TERMINATING',
+      notificationTargetARN: 'arn:aws:sns:us-east-1:123456789012:my-sns-topic',
+      roleARN: 'arn:aws:iam::123456789012:role/my-notification-role',
+      heartbeatTimeout: 3600,
+      defaultResult: 'ABANDON'
+    ))
+  }
+
   void 'should create defined lifecycle hooks'() {
     given:
     def lifecycleHooks = [

diff --git a/...spinnaker/clouddriver/aws/deploy/ops/UpsertAsgLifecycleHookAtomicOperationUnitSpec.groovy b/...spinnaker/clouddriver/aws/deploy/ops/UpsertAsgLifecycleHookAtomicOperationUnitSpec.groovy
@@ -72,6 +72,24 @@ class UpsertAsgLifecycleHookAtomicOperationUnitSpec extends Specification {
     0 * _
   }
 
+  def 'removes invalid characters from lifecycle hook name'() {
+    when:
+    description.serverGroupName = 'a+b=c!d@e#f$g%h&i*j(k)l_m-n{o}p[q]r\\s<t>u,v.w?x/y|z-v001' // this is a valid ASG name
+    op.operate([])
+
+    then:
+    1 * autoScaling.putLifecycleHook(new PutLifecycleHookRequest(
+      lifecycleHookName: 'a_b_c_d_e_f_g_h_i_j_k_l_m-n_o_p_q_r_s_t_u_v_w_x/y_z-v001-lifecycle-1',
+      autoScalingGroupName: 'a+b=c!d@e#f$g%h&i*j(k)l_m-n{o}p[q]r\\s<t>u,v.w?x/y|z-v001',
+      lifecycleTransition: 'autoscaling:EC2_INSTANCE_TERMINATING',
+      roleARN: 'arn:aws:iam::123456789012:role/my-notification-role',
+      notificationTargetARN: 'arn:aws:sns:us-west-1:123456789012:my-sns-topic',
+      heartbeatTimeout: 3600,
+      defaultResult: 'ABANDON'
+    ))
+    0 * _
+  }
+
   def 'creates named lifecycle hook'() {
     given:
     description.name = 'fancyHook'