From 688c7e89d9a6e7853b1aadab91288ed1c14c7cac Mon Sep 17 00:00:00 2001
From: Oleksandr Novak <novak.as@i.ua>
Date: Mon, 30 May 2022 11:51:33 +0200
Subject: [PATCH] ecs_service - fix validation for `placementConstraints`
 (#1170)

ecs_service - fix validation for `placementConstraints`

SUMMARY
Fixes #1058
ISSUE TYPE
Bugfix Pull Request
COMPONENT NAME
ecs_service

Reviewed-by: Markus Bergholz <git@osuv.de>
Reviewed-by: Oleksandr Novak <novak.as@i.ua>
Reviewed-by: Alina Buzachis <None>
---
 .../1300-ecs_service-placementConstraints.yml        |  2 ++
 plugins/modules/ecs_service.py                       | 11 +++++++++--
 tests/integration/targets/ecs_cluster/tasks/main.yml | 12 ++++++++++++
 3 files changed, 23 insertions(+), 2 deletions(-)
 create mode 100644 changelogs/fragments/1300-ecs_service-placementConstraints.yml

diff --git a/changelogs/fragments/1300-ecs_service-placementConstraints.yml b/changelogs/fragments/1300-ecs_service-placementConstraints.yml
new file mode 100644
index 00000000000..85e3d36de4d
--- /dev/null
+++ b/changelogs/fragments/1300-ecs_service-placementConstraints.yml
@@ -0,0 +1,2 @@
+bugfixes:
+  - ecs_service - fix validation for ``placement_constraints``. It's possible to use ``distinctInstance`` placement constraint now (https://github.com/ansible-collections/community.aws/issues/1058)
diff --git a/plugins/modules/ecs_service.py b/plugins/modules/ecs_service.py
index 66f20b63d81..4f94a452ce9 100644
--- a/plugins/modules/ecs_service.py
+++ b/plugins/modules/ecs_service.py
@@ -113,6 +113,7 @@
             type: str
           expression:
             description: A cluster query language expression to apply to the constraint.
+            required: false
             type: str
     placement_strategy:
         description:
@@ -584,7 +585,6 @@ def create_service(self, service_name, cluster_name, task_definition, load_balan
             clientToken=client_token,
             role=role,
             deploymentConfiguration=deployment_configuration,
-            placementConstraints=placement_constraints,
             placementStrategy=placement_strategy
         )
         if network_configuration:
@@ -597,6 +597,13 @@ def create_service(self, service_name, cluster_name, task_definition, load_balan
             params['healthCheckGracePeriodSeconds'] = health_check_grace_period_seconds
         if service_registries:
             params['serviceRegistries'] = service_registries
+
+        # filter placement_constraint and left only those where value is not None
+        # use-case: `distinctInstance` type should never contain `expression`, but None will fail `str` type validation
+        if placement_constraints:
+            params['placementConstraints'] = [{key: value for key, value in constraint.items() if value is not None}
+                                              for constraint in placement_constraints]
+
         # desired count is not required if scheduling strategy is daemon
         if desired_count is not None:
             params['desiredCount'] = desired_count
@@ -674,7 +681,7 @@ def main():
             elements='dict',
             options=dict(
                 type=dict(type='str'),
-                expression=dict(type='str')
+                expression=dict(required=False, type='str')
             )
         ),
         placement_strategy=dict(
diff --git a/tests/integration/targets/ecs_cluster/tasks/main.yml b/tests/integration/targets/ecs_cluster/tasks/main.yml
index 1a84b163265..7f49374b532 100644
--- a/tests/integration/targets/ecs_cluster/tasks/main.yml
+++ b/tests/integration/targets/ecs_cluster/tasks/main.yml
@@ -215,6 +215,8 @@
         desired_count: 1
         deployment_configuration: "{{ ecs_service_deployment_configuration }}"
         placement_strategy: "{{ ecs_service_placement_strategy }}"
+        placement_constraints:
+          - type: distinctInstance
         health_check_grace_period_seconds: "{{ ecs_service_health_check_grace_period }}"
         load_balancers:
           - targetGroupArn: "{{ elb_target_group_instance.target_group_arn }}"
@@ -223,6 +225,12 @@
         role: "ecsServiceRole"
       register: ecs_service
 
+    - name: check that placement constraint has been applied
+      assert:
+        that:
+          - ecs_service.changed
+          - "ecs_service.service.placementConstraints[0].type == 'distinctInstance'"
+
     - name: check that ECS service creation changed
       assert:
         that:
@@ -237,6 +245,8 @@
         desired_count: 1
         deployment_configuration: "{{ ecs_service_deployment_configuration }}"
         placement_strategy: "{{ ecs_service_placement_strategy }}"
+        placement_constraints:
+          - type: distinctInstance
         health_check_grace_period_seconds: "{{ ecs_service_health_check_grace_period }}"
         load_balancers:
           - targetGroupArn: "{{ elb_target_group_instance.target_group_arn }}"
@@ -260,6 +270,8 @@
         desired_count: 1
         deployment_configuration: "{{ ecs_service_deployment_configuration }}"
         placement_strategy: "{{ ecs_service_placement_strategy }}"
+        placement_constraints:
+          - type: distinctInstance
         health_check_grace_period_seconds: "{{ ecs_service_health_check_grace_period }}"
         load_balancers:
           - targetGroupArn: "{{ elb_target_group_instance.target_group_arn }}"