feat(provider/ecs): Added support for environment variables (#2961)

spinnaker · Sep 14, 2018 · 3910a33 · 3910a33
1 parent d7254d1
commit 3910a33
Show file tree

Hide file tree

Showing 5 changed files with 131 additions and 1 deletion.
diff --git a/...om/netflix/spinnaker/clouddriver/ecs/deploy/description/CreateServerGroupDescription.java b/...om/netflix/spinnaker/clouddriver/ecs/deploy/description/CreateServerGroupDescription.java
@@ -39,6 +39,8 @@ public class CreateServerGroupDescription extends AbstractECSDescription {
   Integer computeUnits;
   Integer reservedMemory;
 
+  Map<String, String> environmentVariables;
+
   String dockerImageAddress;
 
   ServerGroup.Capacity capacity;

diff --git a/...va/com/netflix/spinnaker/clouddriver/ecs/deploy/ops/CreateServerGroupAtomicOperation.java b/...va/com/netflix/spinnaker/clouddriver/ecs/deploy/ops/CreateServerGroupAtomicOperation.java
@@ -135,6 +135,15 @@ protected TaskDefinition registerTaskDefinition(AmazonECS ecs, String ecsService
 
   protected RegisterTaskDefinitionRequest makeTaskDefinitionRequest(String ecsServiceRole, String newServerGroupVersion) {
     Collection<KeyValuePair> containerEnvironment = new LinkedList<>();
+
+    // Set all user defined environment variables
+    final Map<String, String> environmentVariables = description.getEnvironmentVariables();
+    if(environmentVariables != null) {
+      for (Map.Entry<String, String> entry : environmentVariables.entrySet()) {
+        containerEnvironment.add(new KeyValuePair().withName(entry.getKey()).withValue(entry.getValue()));
+      }
+    }
+
     containerEnvironment.add(new KeyValuePair().withName("SERVER_GROUP").withValue(newServerGroupVersion));
     containerEnvironment.add(new KeyValuePair().withName("CLOUD_STACK").withValue(description.getStack()));
     containerEnvironment.add(new KeyValuePair().withName("CLOUD_DETAIL").withValue(description.getFreeFormDetails()));

diff --git a/...spinnaker/clouddriver/ecs/deploy/validators/EcsCreateServerGroupDescriptionValidator.java b/...spinnaker/clouddriver/ecs/deploy/validators/EcsCreateServerGroupDescriptionValidator.java
@@ -25,7 +25,9 @@
 import org.springframework.stereotype.Component;
 import org.springframework.validation.Errors;
 
+import java.util.Collections;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 
 @EcsOperation(AtomicOperations.CREATE_SERVER_GROUP)
@@ -41,6 +43,12 @@ public class EcsCreateServerGroupDescriptionValidator extends CommonValidator {
     "attribute:ecs.ami-id"
   );
 
+  private static final Set<String> RESERVED_ENVIRONMENT_VARIABLES = Sets.newHashSet(
+    "SERVER_GROUP",
+    "CLOUD_STACK",
+    "CLOUD_DETAIL"
+  );
+
   public EcsCreateServerGroupDescriptionValidator() {
     super("createServerGroupDescription");
   }
@@ -130,5 +138,13 @@ public void validate(List priorDescriptions, Object description, Errors errors)
       rejectValue(errors, "reservedMemory", "not.nullable");
     }
 
+    // Verify that the environment variables set by the user do not contain reserved values
+    if (createServerGroupDescription.getEnvironmentVariables() != null) {
+      if(!Collections.disjoint(createServerGroupDescription.getEnvironmentVariables().keySet(),
+        RESERVED_ENVIRONMENT_VARIABLES)) {
+        rejectValue(errors, "environmentVariables", "invalid");
+      }
+    }
+
   }
 }
diff --git a/.../netflix/spinnaker/clouddriver/ecs/deploy/ops/CreateServerGroupAtomicOperationSpec.groovy b/.../netflix/spinnaker/clouddriver/ecs/deploy/ops/CreateServerGroupAtomicOperationSpec.groovy
@@ -67,7 +67,6 @@ class CreateServerGroupAtomicOperationSpec extends CommonAtomicOperation {
 
   def service = new Service(serviceName: "${serviceName}")
 
-
   def 'should create a service'() {
     given:
     def description = new CreateServerGroupDescription(
@@ -336,4 +335,82 @@ class CreateServerGroupAtomicOperationSpec extends CommonAtomicOperation {
     then:
     request.getContainerDefinitions().get(0).getLogConfiguration().getOptions() == logOptions
   }
+
+  def 'should generate a RegisterTaskDefinitionRequest object'() {
+    given:
+    def description = Mock(CreateServerGroupDescription)
+    description.getApplication() >> 'v1'
+    description.getStack() >> 'kcats'
+    description.getFreeFormDetails() >> 'liated'
+    description.ecsClusterName = 'test-cluster'
+    description.iamRole = 'None (No IAM role)'
+    description.getContainerPort() >> 1337
+    description.targetGroup = 'target-group-arn'
+    description.getPortProtocol() >> 'tcp'
+    description.getComputeUnits() >> 9001
+    description.getReservedMemory() >> 9001
+    description.getDockerImageAddress() >> 'docker-image-url'
+    description.capacity = new ServerGroup.Capacity(1, 1, 1)
+    description.availabilityZones = ['us-west-1': ['us-west-1a', 'us-west-1b', 'us-west-1c']]
+    description.autoscalingPolicies = []
+    description.placementStrategySequence = []
+
+    def operation = new CreateServerGroupAtomicOperation(description)
+
+    when:
+    RegisterTaskDefinitionRequest result = operation.makeTaskDefinitionRequest("test-role", "v1")
+
+    then:
+    result.getTaskRoleArn() == null
+    result.getFamily() == "v1-kcats-liated"
+
+    result.getContainerDefinitions().size() == 1
+    def containerDefinition = result.getContainerDefinitions().first()
+    containerDefinition.name == 'v1'
+    containerDefinition.image == 'docker-image-url'
+    containerDefinition.cpu == 9001
+    containerDefinition.memoryReservation == 9001
+
+    containerDefinition.portMappings.size() == 1
+    def portMapping = containerDefinition.portMappings.first()
+    portMapping.getHostPort() == 0
+    portMapping.getContainerPort() == 1337
+    portMapping.getProtocol() == 'tcp'
+
+    containerDefinition.environment.size() == 3
+    def environments = [:]
+    for(elem in containerDefinition.environment){
+      environments.put(elem.getName(), elem.getValue())
+    }
+    environments.get("SERVER_GROUP") == "v1"
+    environments.get("CLOUD_STACK") == "kcats"
+    environments.get("CLOUD_DETAIL") == "liated"
+  }
+
+  def 'should set additional environment variables'() {
+    given:
+    def description = Mock(CreateServerGroupDescription)
+    description.getApplication() >> 'v1'
+    description.getStack() >> 'kcats'
+    description.getFreeFormDetails() >> 'liated'
+    description.getEnvironmentVariables() >> ["ENVIRONMENT_1" : "test1", "ENVIRONMENT_2" : "test2"]
+    def operation = new CreateServerGroupAtomicOperation(description)
+
+    when:
+    RegisterTaskDefinitionRequest result = operation.makeTaskDefinitionRequest("test-role", "v1")
+
+    then:
+    result.getContainerDefinitions().size() == 1
+    def containerDefinition = result.getContainerDefinitions().first()
+    containerDefinition.environment.size() == 5
+    def environments = [:]
+    for(elem in containerDefinition.environment){
+      environments.put(elem.getName(), elem.getValue())
+    }
+    environments.get("SERVER_GROUP") == "v1"
+    environments.get("CLOUD_STACK") == "kcats"
+    environments.get("CLOUD_DETAIL") == "liated"
+    environments.get("ENVIRONMENT_1") == "test1"
+    environments.get("ENVIRONMENT_2") == "test2"
+  }
 }
diff --git a/...ker/clouddriver/ecs/deploy/validators/EcsCreateServergroupDescriptionValidatorSpec.groovy b/...ker/clouddriver/ecs/deploy/validators/EcsCreateServergroupDescriptionValidatorSpec.groovy
@@ -79,6 +79,32 @@ class EcsCreateServergroupDescriptionValidatorSpec extends AbstractValidatorSpec
     1 * errors.rejectValue('availabilityZones', "${getDescriptionName()}.availabilityZones.must.have.only.one")
   }
 
+  void 'should fail when environment variables contain reserved key'() {
+    given:
+    def description = (CreateServerGroupDescription) getDescription()
+    description.environmentVariables = ['SERVER_GROUP':'invalid', 'tag_1':'valid_tag']
+    def errors = Mock(Errors)
+
+    when:
+    validator.validate([], description, errors)
+
+    then:
+    1 * errors.rejectValue('environmentVariables', "${getDescriptionName()}.environmentVariables.invalid")
+  }
+
+  void 'should pass with correct environment variables'() {
+    given:
+    def description = getDescription()
+    description.environmentVariables = ['TAG_1':'valid_tag_1', 'TAG_2':'valid_tag_2']
+    def errors = Mock(Errors)
+
+    when:
+    validator.validate([], description, errors)
+
+    then:
+    0 * errors.rejectValue(_, _)
+  }
+
 
   @Override
   AbstractECSDescription getNulledDescription() {