From 3910a332e2d410ea977bbbc4e8bf8f039bee97cb Mon Sep 17 00:00:00 2001 From: lysimon Date: Fri, 14 Sep 2018 20:21:34 +0200 Subject: [PATCH] feat(provider/ecs): Added support for environment variables (#2961) --- .../CreateServerGroupDescription.java | 2 + .../ops/CreateServerGroupAtomicOperation.java | 9 +++ ...CreateServerGroupDescriptionValidator.java | 16 ++++ ...reateServerGroupAtomicOperationSpec.groovy | 79 ++++++++++++++++++- ...ServergroupDescriptionValidatorSpec.groovy | 26 ++++++ 5 files changed, 131 insertions(+), 1 deletion(-) diff --git a/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/deploy/description/CreateServerGroupDescription.java b/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/deploy/description/CreateServerGroupDescription.java index c6f2c8385ee..a543cfaca2c 100644 --- a/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/deploy/description/CreateServerGroupDescription.java +++ b/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/deploy/description/CreateServerGroupDescription.java @@ -39,6 +39,8 @@ public class CreateServerGroupDescription extends AbstractECSDescription { Integer computeUnits; Integer reservedMemory; + Map environmentVariables; + String dockerImageAddress; ServerGroup.Capacity capacity; diff --git a/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/deploy/ops/CreateServerGroupAtomicOperation.java b/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/deploy/ops/CreateServerGroupAtomicOperation.java index 51ff26c3e0c..5f9057827e1 100644 --- a/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/deploy/ops/CreateServerGroupAtomicOperation.java +++ b/clouddriver-ecs/src/main/java/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 containerEnvironment = new LinkedList<>(); + + // Set all user defined environment variables + final Map environmentVariables = description.getEnvironmentVariables(); + if(environmentVariables != null) { + for (Map.Entry 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/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/deploy/validators/EcsCreateServerGroupDescriptionValidator.java b/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/deploy/validators/EcsCreateServerGroupDescriptionValidator.java index b732d439360..dfb161acbd9 100644 --- a/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/deploy/validators/EcsCreateServerGroupDescriptionValidator.java +++ b/clouddriver-ecs/src/main/java/com/netflix/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 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/clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/deploy/ops/CreateServerGroupAtomicOperationSpec.groovy b/clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/deploy/ops/CreateServerGroupAtomicOperationSpec.groovy index 4e43d2a2291..4171afc4e07 100644 --- a/clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/deploy/ops/CreateServerGroupAtomicOperationSpec.groovy +++ b/clouddriver-ecs/src/test/groovy/com/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/clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/deploy/validators/EcsCreateServergroupDescriptionValidatorSpec.groovy b/clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/deploy/validators/EcsCreateServergroupDescriptionValidatorSpec.groovy index f6684373e1e..9a162c4667d 100644 --- a/clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/deploy/validators/EcsCreateServergroupDescriptionValidatorSpec.groovy +++ b/clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/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() {