feat(aws): termination lifecycle worker config; cleaner IAM (#1495)

spinnaker · Mar 7, 2017 · d85deb4 · d85deb4
1 parent 95f6d4c
commit d85deb4
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 35 deletions.
diff --git a/...lix/spinnaker/clouddriver/aws/lifecycle/InstanceTerminationConfigurationProperties.groovy b/...lix/spinnaker/clouddriver/aws/lifecycle/InstanceTerminationConfigurationProperties.groovy
@@ -26,6 +26,7 @@ class InstanceTerminationConfigurationProperties {
 
   int visibilityTimeout = 30
   int waitTimeSeconds = 5
+  int sqsMessageRetentionPeriodSeconds = 120
 
   InstanceTerminationConfigurationProperties() {
     // default constructor
@@ -35,11 +36,13 @@ class InstanceTerminationConfigurationProperties {
                                              String queueARN,
                                              String topicARN,
                                              int visibilityTimeout,
-                                             int waitTimeSeconds) {
+                                             int waitTimeSeconds,
+                                             int sqsMessageRetentionPeriodSeconds) {
     this.accountName = accountName
     this.queueARN = queueARN
     this.topicARN = topicARN
     this.visibilityTimeout = visibilityTimeout
     this.waitTimeSeconds = waitTimeSeconds
+    this.sqsMessageRetentionPeriodSeconds = sqsMessageRetentionPeriodSeconds
   }
 }
diff --git a/...y/com/netflix/spinnaker/clouddriver/aws/lifecycle/InstanceTerminationLifecycleWorker.java b/...y/com/netflix/spinnaker/clouddriver/aws/lifecycle/InstanceTerminationLifecycleWorker.java
@@ -36,6 +36,7 @@
 import com.netflix.spinnaker.clouddriver.aws.deploy.description.EnableDisableInstanceDiscoveryDescription;
 import com.netflix.spinnaker.clouddriver.aws.deploy.ops.discovery.AwsEurekaSupport;
 import com.netflix.spinnaker.clouddriver.aws.security.AmazonClientProvider;
+import com.netflix.spinnaker.clouddriver.aws.security.AmazonCredentials.LifecycleHook;
 import com.netflix.spinnaker.clouddriver.aws.security.NetflixAmazonCredentials;
 import com.netflix.spinnaker.clouddriver.data.task.DefaultTask;
 import com.netflix.spinnaker.clouddriver.data.task.Task;
@@ -49,9 +50,10 @@
 import javax.inject.Provider;
 import java.io.IOException;
 import java.time.Duration;
-import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.Date;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Optional;
@@ -120,7 +122,9 @@ private void listenForMessages() {
     Set<? extends AccountCredentials> accountCredentials = accountCredentialsProvider.getAll();
     List<String> allAccountIds = getAllAccountIds(accountCredentials);
 
-    this.queueId = ensureQueueExists(amazonSQS, queueARN, topicARN, allAccountIds, getSourceRoleArns(accountCredentials));
+    this.queueId = ensureQueueExists(
+      amazonSQS, queueARN, topicARN, getSourceRoleArns(accountCredentials), properties.getSqsMessageRetentionPeriodSeconds()
+    );
     ensureTopicExists(amazonSNS, topicARN, allAccountIds, queueARN);
 
     while (true) {
@@ -257,10 +261,19 @@ private static Policy buildSNSPolicy(ARN topicARN, List<String> allAccountIds) {
     return new Policy("allow-remote-account-send", Collections.singletonList(statement));
   }
 
-  private static String ensureQueueExists(AmazonSQS amazonSQS, ARN queueARN, ARN topicARN, List<String> allAccounts, Set<String> terminatingRoleArns) {
+  private static String ensureQueueExists(AmazonSQS amazonSQS,
+                                          ARN queueARN,
+                                          ARN topicARN,
+                                          Set<String> terminatingRoleArns,
+                                          int sqsMessageRetentionPeriodSeconds) {
     String queueUrl = amazonSQS.createQueue(queueARN.name).getQueueUrl();
+
+    HashMap<String, String> attributes = new HashMap<>();
+    attributes.put("Policy", buildSQSPolicy(queueARN, topicARN, terminatingRoleArns).toJson());
+    attributes.put("MessageRetentionPeriod", Integer.toString(sqsMessageRetentionPeriodSeconds));
     amazonSQS.setQueueAttributes(
-      queueUrl, Collections.singletonMap("Policy", buildSQSPolicy(queueARN, topicARN, allAccounts, terminatingRoleArns).toJson())
+      queueUrl,
+      attributes
     );
 
     return queueUrl;
@@ -270,28 +283,19 @@ private static String ensureQueueExists(AmazonSQS amazonSQS, ARN queueARN, ARN t
    * This policy allows operators to choose whether or not to have lifecycle hooks to be sent via SNS for fanout, or
    * be sent directly to an SQS queue from the autoscaling group.
    */
-  private static Policy buildSQSPolicy(ARN queue, ARN topic, List<String> allAccounts, Set<String> terminatingRoleArns) {
+  private static Policy buildSQSPolicy(ARN queue, ARN topic, Set<String> terminatingRoleArns) {
     Statement snsStatement = new Statement(Effect.Allow).withActions(SQSActions.SendMessage);
     snsStatement.setPrincipals(Principal.All);
     snsStatement.setResources(Collections.singletonList(new Resource(queue.arn)));
     snsStatement.setConditions(Collections.singletonList(
       new Condition().withType("ArnEquals").withConditionKey("aws:SourceArn").withValues(topic.arn)
     ));
 
-    Set<Principal> allAccountPrincipals = allAccounts.stream().map(Principal::new).collect(Collectors.toSet());
-
-    List<Statement> statements = new ArrayList<>(Collections.singletonList(snsStatement));
-    for (String arnMatcher : terminatingRoleArns) {
-      Statement lifecycleStatement = new Statement(Effect.Allow).withActions(SQSActions.SendMessage, SQSActions.GetQueueUrl);
-      lifecycleStatement.setPrincipals(allAccountPrincipals);
-      lifecycleStatement.setResources(Collections.singletonList(new Resource(queue.arn)));
-      lifecycleStatement.setConditions(Collections.singletonList(
-        new Condition().withType("ArnLike").withConditionKey("aws:SourceArn").withValues(arnMatcher)
-      ));
-      statements.add(lifecycleStatement);
-    }
+    Statement sqsStatement = new Statement(Effect.Allow).withActions(SQSActions.SendMessage, SQSActions.GetQueueUrl);
+    sqsStatement.setPrincipals(terminatingRoleArns.stream().map(Principal::new).collect(Collectors.toList()));
+    sqsStatement.setResources(Collections.singletonList(new Resource(queue.arn)));
 
-    return new Policy("allow-sns-or-sqs-send", statements);
+    return new Policy("allow-sns-or-sqs-send", Arrays.asList(snsStatement, sqsStatement));
   }
 
   Id getLagMetricId(String region) {
@@ -323,16 +327,11 @@ private static <T extends AccountCredentials> Set<String> getSourceRoleArns(Set<
           sourceRoleArns.addAll(c.getLifecycleHooks()
             .stream()
             .filter(h -> "autoscaling:EC2_INSTANCE_TERMINATING".equals(h.getLifecycleTransition()))
-            .map(h -> convertRoleArnToIamConditionalMatcher(h.getRoleARN()))
+            .map(LifecycleHook::getRoleARN)
             .collect(Collectors.toSet()));
         }
       }
     }
     return sourceRoleArns;
   }
-
-  private static String convertRoleArnToIamConditionalMatcher(String roleArn) {
-    String[] roleName = roleArn.split(":");
-    return "arn:aws:iam::*:" + roleName[roleName.length-1];
-  }
 }
diff --git a/...tflix/spinnaker/clouddriver/aws/lifecycle/InstanceTerminationLifecycleWorkerProvider.java b/...tflix/spinnaker/clouddriver/aws/lifecycle/InstanceTerminationLifecycleWorkerProvider.java
@@ -86,7 +86,8 @@ public void start() {
             .replaceAll(REGION_TEMPLATE_PATTERN, region.getName())
             .replaceAll(ACCOUNT_ID_TEMPLATE_PATTERN, credentials.getAccountId()),
           properties.getVisibilityTimeout(),
-          properties.getWaitTimeSeconds()
+          properties.getWaitTimeSeconds(),
+          properties.getSqsMessageRetentionPeriodSeconds()
         ),
         discoverySupport,
         registry

diff --git a/...netflix/spinnaker/clouddriver/aws/lifecycle/InstanceTerminationLifecycleWorkerSpec.groovy b/...netflix/spinnaker/clouddriver/aws/lifecycle/InstanceTerminationLifecycleWorkerSpec.groovy
@@ -72,6 +72,7 @@ class InstanceTerminationLifecycleWorkerSpec extends Specification {
       queueARN.arn,
       topicARN.arn,
       -1,
+      -1,
       -1
     ),
     awsEurekaSupportProvider,
@@ -100,15 +101,16 @@ class InstanceTerminationLifecycleWorkerSpec extends Specification {
 
   def 'should create queue if it does not exist'() {
     when:
-    def queueId = InstanceTerminationLifecycleWorker.ensureQueueExists(amazonSQS, queueARN, topicARN, ['1234'], [] as Set<String>)
+    def queueId = InstanceTerminationLifecycleWorker.ensureQueueExists(amazonSQS, queueARN, topicARN, [] as Set<String>, 1)
 
     then:
     queueId == "my-queue-url"
 
     1 * amazonSQS.createQueue(queueARN.name) >> { new CreateQueueResult().withQueueUrl("my-queue-url") }
 
     1 * amazonSQS.setQueueAttributes("my-queue-url", [
-      "Policy": InstanceTerminationLifecycleWorker.buildSQSPolicy(queueARN, topicARN, ['1234'], [] as Set<String>).toJson()
+      "Policy": InstanceTerminationLifecycleWorker.buildSQSPolicy(queueARN, topicARN, [] as Set<String>).toJson(),
+      "MessageRetentionPeriod": "1"
     ])
     0 * _
   }
@@ -177,11 +179,10 @@ class InstanceTerminationLifecycleWorkerSpec extends Specification {
 
   def 'should build sqs policy supporting both sns and direct notifications'() {
     given:
-    def allAccountIds = ['100', '200']
-    Set<String> terminatingRoleArns = ['arn:aws:iam::*:role/terminatingRole']
+    Set<String> terminatingRoleArns = ['arn:aws:iam::100:role/terminatingRole', 'arn:aws:iam::200:role/terminatingRole']
 
     when:
-    def result = subject.buildSQSPolicy(queueARN, topicARN, allAccountIds, terminatingRoleArns)
+    def result = subject.buildSQSPolicy(queueARN, topicARN, terminatingRoleArns)
 
     then:
     result.statements.size() == 2
@@ -198,12 +199,12 @@ class InstanceTerminationLifecycleWorkerSpec extends Specification {
 
     // direct sqs
     result.statements[1].with {
-      it.principals*.id == ['100', '200']
+      it.principals*.id == [
+        'arn:aws:iam::100:role/terminatingRole',
+        'arn:aws:iam::200:role/terminatingRole'
+      ]
       it.actions*.actionName == ['SendMessage, GetQueueUrl']
       it.resources*.id == ['arn:aws:sqs:us-west-2:100:queueName']
-      it.conditions*.type == ['ArnLike']
-      it.conditions*.conditionKey == ['aws:SourceArn']
-      it.conditions*.values == [['arn:aws:iam::*:role/terminatingRole']]
     }
   }
 }