Repository: fabfuel/ecs-deploy

Issue:
Remove omitted ENV variables on new deployments
We are using ecs deploy to specify environment (and secrets) via command line for new TaskDefinitions. If an older TaskDefinition has some ENV variables that we need to remove from future deployments (by removing them from the command line) they do still show up in the new TaskDefinition - seems the existing ones don't get deleted if removing them from the command line.

Discussion:

Patch:
diff --git a/README.rst b/README.rst
index a5e3451..c6602be 100644
--- a/README.rst
+++ b/README.rst
@@ -162,6 +162,16 @@ This will modify the definition **of two containers**.
 The **webserver**'s environment variable `SOME_VARIABLE` will be set to "SOME_VALUE" and the variable `OTHER_VARIABLE` to "OTHER_VALUE".
 The **app**'s environment variable `APP_VARIABLE` will be set to "APP_VALUE".
 
+
+Set environment variables exclusively, remove all other pre-existing environment variables
+==========================================================================================
+To reset all existing environment variables of a task definition, use the flag ``--exclusive-env`` ::
+
+    $ ecs deploy my-cluster my-service -e webserver SOME_VARIABLE SOME_VALUE --exclusive-env
+
+This will remove **all other** existing environment variables of **all containers** of the task definition, except for the variable `SOME_VARIABLE` with the value "SOME_VALUE" in the webserver container.
+
+
 Set a secret environment variable from the AWS Parameter Store
 ==============================================================
 
@@ -178,6 +188,16 @@ You can also specify the full arn of the parameter::
 
 This will modify the **webserver** container definition and add or overwrite the environment variable `SOME_SECRET` with the value of the `KEY_OF_SECRET_IN_PARAMETER_STORE` in the AWS Parameter Store of the AWS Systems Manager.
 
+
+Set secrets exclusively, remove all other pre-existing secret environment variables
+===================================================================================
+To reset all existing secrets (secret environment variables) of a task definition, use the flag ``--exclusive-secrets`` ::
+
+    $ ecs deploy my-cluster my-service -s webserver NEW_SECRET KEY_OF_SECRET_IN_PARAMETER_STORE --exclusive-secret
+
+This will remove **all other** existing secret environment variables of **all containers** of the task definition, except for the new secret variable `NEW_SECRET` with the value coming from the AWS Parameter Store with the name "KEY_OF_SECRET_IN_PARAMETER_STORE" in the webserver container.
+
+
 Modify a command
 ================
 To change the command of a specific container, run the following command::
diff --git a/ecs_deploy/cli.py b/ecs_deploy/cli.py
index 3b9e9d9..45cdfee 100644
--- a/ecs_deploy/cli.py
+++ b/ecs_deploy/cli.py
@@ -46,7 +46,9 @@ def get_client(access_key_id, secret_access_key, region, profile):
 @click.option('--diff/--no-diff', default=True, help='Print which values were changed in the task definition (default: --diff)')
 @click.option('--deregister/--no-deregister', default=True, help='Deregister or keep the old task definition (default: --deregister)')
 @click.option('--rollback/--no-rollback', default=False, help='Rollback to previous revision, if deployment failed (default: --no-rollback)')
-def deploy(cluster, service, tag, image, command, env, secret, role, task, region, access_key_id, secret_access_key, profile, timeout, newrelic_apikey, newrelic_appid, comment, user, ignore_warnings, diff, deregister, rollback):
+@click.option('--exclusive-env', is_flag=True, default=False, help='Set the given environment variables exclusively and remove all other pre-existing env variables from all containers')
+@click.option('--exclusive-secrets', is_flag=True, default=False, help='Set the given secrets exclusively and remove all other pre-existing secrets from all containers')
+def deploy(cluster, service, tag, image, command, env, secret, role, task, region, access_key_id, secret_access_key, profile, timeout, newrelic_apikey, newrelic_appid, comment, user, ignore_warnings, diff, deregister, rollback, exclusive_env, exclusive_secrets):
     """
     Redeploy or modify a service.
 
@@ -66,8 +68,8 @@ def deploy(cluster, service, tag, image, command, env, secret, role, task, regio
         td = get_task_definition(deployment, task)
         td.set_images(tag, **{key: value for (key, value) in image})
         td.set_commands(**{key: value for (key, value) in command})
-        td.set_environment(env)
-        td.set_secrets(secret)
+        td.set_environment(env, exclusive_env)
+        td.set_secrets(secret, exclusive_secrets)
         td.set_role_arn(role)
 
         if diff:
diff --git a/ecs_deploy/ecs.py b/ecs_deploy/ecs.py
index 74cf19a..9136bdd 100644
--- a/ecs_deploy/ecs.py
+++ b/ecs_deploy/ecs.py
@@ -241,7 +241,7 @@ class EcsTaskDefinition(object):
                 self._diff.append(diff)
                 container[u'command'] = new_command.split(" ")
 
-    def set_environment(self, environment_list):
+    def set_environment(self, environment_list, exclusive=False):
         environment = {}
 
         for env in environment_list:
@@ -253,14 +253,25 @@ class EcsTaskDefinition(object):
             if container[u'name'] in environment:
                 self.apply_container_environment(
                     container=container,
-                    new_environment=environment[container[u'name']]
+                    new_environment=environment[container[u'name']],
+                    exclusive=exclusive,
+                )
+            elif exclusive is True:
+                self.apply_container_environment(
+                    container=container,
+                    new_environment={},
+                    exclusive=exclusive,
                 )
 
-    def apply_container_environment(self, container, new_environment):
+    def apply_container_environment(self, container, new_environment, exclusive=False):
         environment = container.get('environment', {})
         old_environment = {env['name']: env['value'] for env in environment}
-        merged = old_environment.copy()
-        merged.update(new_environment)
+
+        if exclusive is True:
+            merged = new_environment
+        else:
+            merged = old_environment.copy()
+            merged.update(new_environment)
 
         if old_environment == merged:
             return
@@ -277,7 +288,7 @@ class EcsTaskDefinition(object):
             {"name": e, "value": merged[e]} for e in merged
         ]
 
-    def set_secrets(self, secrets_list):
+    def set_secrets(self, secrets_list, exclusive=False):
         secrets = {}
 
         for secret in secrets_list:
@@ -289,14 +300,25 @@ class EcsTaskDefinition(object):
             if container[u'name'] in secrets:
                 self.apply_container_secrets(
                     container=container,
-                    new_secrets=secrets[container[u'name']]
+                    new_secrets=secrets[container[u'name']],
+                    exclusive=exclusive,
+                )
+            elif exclusive is True:
+                self.apply_container_secrets(
+                    container=container,
+                    new_secrets={},
+                    exclusive=exclusive,
                 )
 
-    def apply_container_secrets(self, container, new_secrets):
+    def apply_container_secrets(self, container, new_secrets, exclusive=False):
         secrets = container.get('secrets', {})
         old_secrets = {secret['name']: secret['valueFrom'] for secret in secrets}
-        merged = old_secrets.copy()
-        merged.update(new_secrets)
+
+        if exclusive is True:
+            merged = new_secrets
+        else:
+            merged = old_secrets.copy()
+            merged.update(new_secrets)
 
         if old_secrets == merged:
             return
@@ -369,23 +391,33 @@ class EcsTaskDefinitionDiff(object):
     @staticmethod
     def _get_environment_diffs(container, env, old_env):
         msg = u'Changed environment "%s" of container "%s" to: "%s"'
+        msg_removed = u'Removed environment "%s" of container "%s"'
         diffs = []
         for name, value in env.items():
             old_value = old_env.get(name)
             if value != old_value or not old_value:
                 message = msg % (name, container, value)
                 diffs.append(message)
+        for old_name in old_env.keys():
+            if old_name not in env.keys():
+                message = msg_removed % (old_name, container)
+                diffs.append(message)
         return diffs
 
     @staticmethod
     def _get_secrets_diffs(container, secrets, old_secrets):
         msg = u'Changed secret "%s" of container "%s" to: "%s"'
+        msg_removed = u'Removed secret "%s" of container "%s"'
         diffs = []
         for name, value in secrets.items():
             old_value = old_secrets.get(name)
             if value != old_value or not old_value:
                 message = msg % (name, container, value)
                 diffs.append(message)
+        for old_name in old_secrets.keys():
+            if old_name not in secrets.keys():
+                message = msg_removed % (old_name, container)
+                diffs.append(message)
         return diffs
 
 


Test patch:
diff --git a/tests/test_cli.py b/tests/test_cli.py
index 8922286..79150b1 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -253,6 +253,52 @@ def test_deploy_empty_environment_variable_again(get_client, runner):
     assert u'Deployment successful' in result.output
 
 
+@patch('ecs_deploy.cli.get_client')
+def test_deploy_exclusive_environment(get_client, runner):
+    get_client.return_value = EcsTestClient('acces_key', 'secret_key')
+    result = runner.invoke(cli.deploy, (CLUSTER_NAME, SERVICE_NAME, '-e', 'webserver', 'new-env', 'new-value', '--exclusive-env'))
+
+    assert result.exit_code == 0
+    assert not result.exception
+
+    assert u"Deploying based on task definition: test-task:1" in result.output
+    assert u"Updating task definition" in result.output
+    assert u'Changed environment "new-env" of container "webserver" to: "new-value"' in result.output
+
+    assert u'Removed environment "foo" of container "webserver"' in result.output
+    assert u'Removed environment "lorem" of container "webserver"' in result.output
+
+    assert u'Removed secret' not in result.output
+
+    assert u'Successfully created revision: 2' in result.output
+    assert u'Successfully deregistered revision: 1' in result.output
+    assert u'Successfully changed task definition to: test-task:2' in result.output
+    assert u'Deployment successful' in result.output
+
+
+@patch('ecs_deploy.cli.get_client')
+def test_deploy_exclusive_secret(get_client, runner):
+    get_client.return_value = EcsTestClient('acces_key', 'secret_key')
+    result = runner.invoke(cli.deploy, (CLUSTER_NAME, SERVICE_NAME, '-s', 'webserver', 'new-secret', 'new-place', '--exclusive-secrets'))
+
+    assert result.exit_code == 0
+    assert not result.exception
+
+    assert u"Deploying based on task definition: test-task:1" in result.output
+    assert u"Updating task definition" in result.output
+    assert u'Changed secret "new-secret" of container "webserver" to: "new-place"' in result.output
+
+    assert u'Removed secret "baz" of container "webserver"' in result.output
+    assert u'Removed secret "dolor" of container "webserver"' in result.output
+
+    assert u'Removed environment' not in result.output
+
+    assert u'Successfully created revision: 2' in result.output
+    assert u'Successfully deregistered revision: 1' in result.output
+    assert u'Successfully changed task definition to: test-task:2' in result.output
+    assert u'Deployment successful' in result.output
+
+
 @patch('ecs_deploy.cli.get_client')
 def test_deploy_one_new_secret_variable(get_client, runner):
     get_client.return_value = EcsTestClient('acces_key', 'secret_key')
diff --git a/tests/test_ecs.py b/tests/test_ecs.py
index 6634188..c93372b 100644
--- a/tests/test_ecs.py
+++ b/tests/test_ecs.py
@@ -28,7 +28,7 @@ TASK_DEFINITION_CONTAINERS_1 = [
     {u'name': u'webserver', u'image': u'webserver:123', u'command': u'run',
      u'environment': ({"name": "foo", "value": "bar"}, {"name": "lorem", "value": "ipsum"}, {"name": "empty", "value": ""}),
      u'secrets': ({"name": "baz", "valueFrom": "qux"}, {"name": "dolor", "valueFrom": "sit"})},
-    {u'name': u'application', u'image': u'application:123', u'command': u'run'}
+    {u'name': u'application', u'image': u'application:123', u'command': u'run', u'environment': ()}
 ]
 TASK_DEFINITION_FAMILY_2 = u'test-task'
 TASK_DEFINITION_REVISION_2 = 2
@@ -39,7 +39,7 @@ TASK_DEFINITION_CONTAINERS_2 = [
     {u'name': u'webserver', u'image': u'webserver:123', u'command': u'run',
      u'environment': ({"name": "foo", "value": "bar"}, {"name": "lorem", "value": "ipsum"}, {"name": "empty", "value": ""}),
      u'secrets': ({"name": "baz", "valueFrom": "qux"}, {"name": "dolor", "valueFrom": "sit"})},
-    {u'name': u'application', u'image': u'application:123', u'command': u'run'}
+    {u'name': u'application', u'image': u'application:123', u'command': u'run', u'environment': ()}
 ]
 
 PAYLOAD_TASK_DEFINITION_1 = {
@@ -310,12 +310,40 @@ def test_task_set_image(task_definition):
 
 
 def test_task_set_environment(task_definition):
+    assert len(task_definition.containers[0]['environment']) == 3
+
     task_definition.set_environment(((u'webserver', u'foo', u'baz'), (u'webserver', u'some-name', u'some-value')))
 
+    assert len(task_definition.containers[0]['environment']) == 4
+
     assert {'name': 'lorem', 'value': 'ipsum'} in task_definition.containers[0]['environment']
     assert {'name': 'foo', 'value': 'baz'} in task_definition.containers[0]['environment']
     assert {'name': 'some-name', 'value': 'some-value'} in task_definition.containers[0]['environment']
 
+
+def test_task_set_environment_exclusively(task_definition):
+    assert len(task_definition.containers[0]['environment']) == 3
+    assert len(task_definition.containers[1]['environment']) == 0
+
+    task_definition.set_environment(((u'application', u'foo', u'baz'), (u'application', u'new-var', u'new-value')), exclusive=True)
+
+    assert len(task_definition.containers[0]['environment']) == 0
+    assert len(task_definition.containers[1]['environment']) == 2
+
+    assert task_definition.containers[0]['environment'] == []
+    assert {'name': 'foo', 'value': 'baz'} in task_definition.containers[1]['environment']
+    assert {'name': 'new-var', 'value': 'new-value'} in task_definition.containers[1]['environment']
+
+
+def test_task_set_secrets_exclusively(task_definition):
+    assert len(task_definition.containers[0]['secrets']) == 2
+
+    task_definition.set_secrets(((u'webserver', u'new-secret', u'another-place'), ), exclusive=True)
+
+    assert len(task_definition.containers[0]['secrets']) == 1
+    assert {'name': 'new-secret', 'valueFrom': 'another-place'} in task_definition.containers[0]['secrets']
+
+
 def test_task_set_secrets(task_definition):
     task_definition.set_secrets(((u'webserver', u'foo', u'baz'), (u'webserver', u'some-name', u'some-value')))
 
@@ -323,6 +351,7 @@ def test_task_set_secrets(task_definition):
     assert {'name': 'foo', 'valueFrom': 'baz'} in task_definition.containers[0]['secrets']
     assert {'name': 'some-name', 'valueFrom': 'some-value'} in task_definition.containers[0]['secrets']
 
+
 def test_task_set_image_for_unknown_container(task_definition):
     with pytest.raises(UnknownContainerError):
         task_definition.set_images(foobar=u'new-image:123')
@@ -336,6 +365,7 @@ def test_task_set_command(task_definition):
         if container[u'name'] == u'application':
             assert container[u'command'] == [u'run-application']
 
+
 def test_task_set_command_with_multiple_arguments(task_definition):
     task_definition.set_commands(webserver=u'run-webserver arg1 arg2', application=u'run-application arg1 arg2')
     for container in task_definition.containers: