From c2ac18fa199dab63ce491016a763b02cffe8cfc9 Mon Sep 17 00:00:00 2001 From: Andrew Pantuso Date: Fri, 25 Jun 2021 18:22:54 -0400 Subject: [PATCH 1/7] Initial commit --- plugins/modules/docker_container.py | 11 +++- .../docker_container/tasks/tests/ports.yml | 56 +++++++++++++++++++ 2 files changed, 65 insertions(+), 2 deletions(-) diff --git a/plugins/modules/docker_container.py b/plugins/modules/docker_container.py index fe0380152..23badc567 100644 --- a/plugins/modules/docker_container.py +++ b/plugins/modules/docker_container.py @@ -663,6 +663,13 @@ - If I(container_default_behavior) is set to C(compatiblity) (the default value), this option has a default of C(no). type: bool + publish_all_ports: + description: + - Publish all ports to the host. + - Any specified port bindings from I(published_ports) will remain intact when C(true). + type: bool + default: false + version_added: 1.8.0 published_ports: description: - List of ports to publish from the container to the host. @@ -677,7 +684,7 @@ is different from the C(docker) command line utility. Use the L(dig lookup,../lookup/dig.html) to resolve hostnames." - A value of C(all) will publish all exposed container ports to random host ports, ignoring - any other mappings. + any other mappings. Use I(publish_all_ports) instead as the use of C(all) will be deprecated in version 2.0.0. - If I(networks) parameter is provided, will inspect each network to see if there exists a bridge network with optional parameter C(com.docker.network.bridge.host_binding_ipv4). If such a network is found, then published ports where no host IP address is specified @@ -1424,7 +1431,6 @@ def __init__(self, client): except ValueError as exc: self.fail("Failed to convert %s to bytes: %s" % (param_name, to_native(exc))) - self.publish_all_ports = False self.published_ports = self._parse_publish_ports() if self.published_ports == 'all': self.publish_all_ports = True @@ -3558,6 +3564,7 @@ def main(): pid_mode=dict(type='str'), pids_limit=dict(type='int'), privileged=dict(type='bool'), + publish_all_ports=dict(type='bool', default=False), published_ports=dict(type='list', elements='str', aliases=['ports']), pull=dict(type='bool', default=False), purge_networks=dict(type='bool', default=False), diff --git a/tests/integration/targets/docker_container/tasks/tests/ports.yml b/tests/integration/targets/docker_container/tasks/tests/ports.yml index 895cd236f..c19301e39 100644 --- a/tests/integration/targets/docker_container/tasks/tests/ports.yml +++ b/tests/integration/targets/docker_container/tasks/tests/ports.yml @@ -284,3 +284,59 @@ - published_ports_2 is not changed - published_ports_3 is changed - published_ports_4 is failed + +#################################################################### +## publish_all_ports ############################################### +#################################################################### + +- name: publish_all_ports + docker_container: + image: "{{ docker_test_image_alpine }}" + command: '/bin/sh -c "sleep 10m"' + name: "{{ cname }}" + state: started + publish_all_ports: true + published_ports: + - "9001:9001" + - "9010-9050:9010-9050" + force_kill: yes + register: publish_all_ports_1 + +- name: publish_all_ports (idempotency) + docker_container: + image: "{{ docker_test_image_alpine }}" + command: '/bin/sh -c "sleep 10m"' + name: "{{ cname }}" + state: started + publish_all_ports: true + published_ports: + - "9001:9001" + - "9010-9050:9010-9050" + force_kill: yes + register: publish_all_ports_2 + +- name: publish_all_ports true -> false + docker_container: + image: "{{ docker_test_image_alpine }}" + command: '/bin/sh -c "sleep 10m"' + name: "{{ cname }}" + state: started + publish_all_ports: false + published_ports: + - "9001:9001" + - "9010-9050:9010-9050" + force_kill: yes + register: publish_all_ports_3 + +- name: cleanup + docker_container: + name: "{{ cname }}" + state: absent + force_kill: yes + diff: no + +- assert: + that: + - publish_all_ports_1 is changed + - publish_all_ports_2 is not changed + - publish_all_ports_3 is changed From 18e6bd20d26382ddc94680aecdb3f51df36180a1 Mon Sep 17 00:00:00 2001 From: Andrew Pantuso Date: Fri, 25 Jun 2021 18:57:55 -0400 Subject: [PATCH 2/7] Adding changelog fragment --- 162-docker_container_publish_all_option.yml | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 162-docker_container_publish_all_option.yml diff --git a/162-docker_container_publish_all_option.yml b/162-docker_container_publish_all_option.yml new file mode 100644 index 000000000..b8d8ff9ef --- /dev/null +++ b/162-docker_container_publish_all_option.yml @@ -0,0 +1,4 @@ +--- +minor_changes: + - docker_container - added ``publish_all_ports`` option to publish all exposed ports to random ports except those + explicitly bound with ``published_ports`` (https://github.com/ansible-collections/community.docker/pull/162). From cf90e54aa540cb1e92313ba94c160cb9a2210bdf Mon Sep 17 00:00:00 2001 From: Andrew Pantuso Date: Sat, 26 Jun 2021 08:30:15 -0400 Subject: [PATCH 3/7] Updating deprecation notice --- plugins/modules/docker_container.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/plugins/modules/docker_container.py b/plugins/modules/docker_container.py index 23badc567..51a8c9633 100644 --- a/plugins/modules/docker_container.py +++ b/plugins/modules/docker_container.py @@ -1432,9 +1432,13 @@ def __init__(self, client): self.fail("Failed to convert %s to bytes: %s" % (param_name, to_native(exc))) self.published_ports = self._parse_publish_ports() + if self.published_ports == 'all': - self.publish_all_ports = True - self.published_ports = None + if not self.publish_all_ports: + self.publish_all_ports = True + self.published_ports = None + else: + self.fail('"all" is not a valid value for "published_ports" when "publish_all_ports" is "true"') self.ports = self._parse_exposed_ports(self.published_ports) self.log("expose ports:") @@ -1756,10 +1760,9 @@ def _parse_publish_ports(self): if len(self.published_ports) > 1: self.client.module.deprecate( 'Specifying "all" in published_ports together with port mappings is not properly ' - 'supported by the module. The port mappings are currently ignored. Please specify ' - 'only port mappings, or the value "all". The behavior for mixed usage will either ' - 'be forbidden in version 2.0.0, or properly handled. In any case, the way you ' - 'currently use the module will change in a breaking way', + 'supported by the module. The port mappings are currently ignored. Set publish_all_ports ' + 'to "true" to randomly assign port mappings for those not specified by published_ports. ' + 'The use of "all" in published_ports will be removed in version 2.0.0.', collection_name='community.docker', version='2.0.0') return 'all' From 7956550b20ddb5ed28debe8ebcd881be4209f7a4 Mon Sep 17 00:00:00 2001 From: Andrew Pantuso Date: Sat, 26 Jun 2021 09:07:43 -0400 Subject: [PATCH 4/7] Adding integration test --- .../targets/docker_container/tasks/tests/ports.yml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/integration/targets/docker_container/tasks/tests/ports.yml b/tests/integration/targets/docker_container/tasks/tests/ports.yml index c19301e39..0a210c2b8 100644 --- a/tests/integration/targets/docker_container/tasks/tests/ports.yml +++ b/tests/integration/targets/docker_container/tasks/tests/ports.yml @@ -83,6 +83,19 @@ force_kill: yes register: published_ports_5 +- name: published_ports -- all equivalence with publish_all_ports + docker_container: + image: "{{ docker_test_image_alpine }}" + command: '/bin/sh -c "sleep 10m"' + name: "{{ cname }}" + state: started + exposed_ports: + - "9001" + - "9002" + publish_all_ports: true + force_kill: yes + register: published_ports_6 + - name: cleanup docker_container: name: "{{ cname }}" @@ -97,6 +110,7 @@ - published_ports_3 is changed - published_ports_4 is not changed - published_ports_5 is changed + - published_ports_6 is not changed #################################################################### ## published_ports: port range ##################################### From 823207a7f9ce5b56b55cdc19bd7143f535a4f88e Mon Sep 17 00:00:00 2001 From: Andrew Pantuso Date: Sat, 26 Jun 2021 11:17:05 -0400 Subject: [PATCH 5/7] Applying second round of review suggestions --- plugins/modules/docker_container.py | 7 +- .../docker_container/tasks/tests/ports.yml | 107 +++++++++++++++++- 2 files changed, 104 insertions(+), 10 deletions(-) diff --git a/plugins/modules/docker_container.py b/plugins/modules/docker_container.py index 51a8c9633..839e51b19 100644 --- a/plugins/modules/docker_container.py +++ b/plugins/modules/docker_container.py @@ -668,7 +668,6 @@ - Publish all ports to the host. - Any specified port bindings from I(published_ports) will remain intact when C(true). type: bool - default: false version_added: 1.8.0 published_ports: description: @@ -1434,11 +1433,11 @@ def __init__(self, client): self.published_ports = self._parse_publish_ports() if self.published_ports == 'all': - if not self.publish_all_ports: + if self.publish_all_ports is None: self.publish_all_ports = True self.published_ports = None else: - self.fail('"all" is not a valid value for "published_ports" when "publish_all_ports" is "true"') + self.fail('"all" is not a valid value for "published_ports" when "publish_all_ports" is specified') self.ports = self._parse_exposed_ports(self.published_ports) self.log("expose ports:") @@ -3567,7 +3566,7 @@ def main(): pid_mode=dict(type='str'), pids_limit=dict(type='int'), privileged=dict(type='bool'), - publish_all_ports=dict(type='bool', default=False), + publish_all_ports=dict(type='bool'), published_ports=dict(type='list', elements='str', aliases=['ports']), pull=dict(type='bool', default=False), purge_networks=dict(type='bool', default=False), diff --git a/tests/integration/targets/docker_container/tasks/tests/ports.yml b/tests/integration/targets/docker_container/tasks/tests/ports.yml index 0a210c2b8..241e4a1bc 100644 --- a/tests/integration/targets/docker_container/tasks/tests/ports.yml +++ b/tests/integration/targets/docker_container/tasks/tests/ports.yml @@ -303,20 +303,68 @@ ## publish_all_ports ############################################### #################################################################### -- name: publish_all_ports +- name: publish_all_ports (true) docker_container: image: "{{ docker_test_image_alpine }}" command: '/bin/sh -c "sleep 10m"' name: "{{ cname }}" state: started publish_all_ports: true + force_kill: yes + register: publish_all_ports_1 + +- name: publish_all_ports (Idempotency) + docker_container: + image: "{{ docker_test_image_alpine }}" + command: '/bin/sh -c "sleep 10m"' + name: "{{ cname }}" + state: started + publish_all_ports: true + force_kill: yes + register: publish_all_ports_2 + +- name: publish_all_ports (false) + docker_container: + image: "{{ docker_test_image_alpine }}" + command: '/bin/sh -c "sleep 10m"' + name: "{{ cname }}" + state: started + publish_all_ports: false + force_kill: yes + register: publish_all_ports_3 + +- name: publish_all_ports (Idempotency 2) + docker_container: + image: "{{ docker_test_image_alpine }}" + command: '/bin/sh -c "sleep 10m"' + name: "{{ cname }}" + state: started + publish_all_ports: false + force_kill: yes + register: publish_all_ports_4 + +- name: publish_all_ports (null) + docker_container: + image: "{{ docker_test_image_alpine }}" + command: '/bin/sh -c "sleep 10m"' + name: "{{ cname }}" + state: started + force_kill: yes + register: publish_all_ports_5 + +- name: publish_all_ports (null with published_ports) + docker_container: + image: "{{ docker_test_image_alpine }}" + command: '/bin/sh -c "sleep 10m"' + name: "{{ cname }}" + state: started published_ports: - "9001:9001" - "9010-9050:9010-9050" force_kill: yes - register: publish_all_ports_1 + register: publish_all_ports_6 -- name: publish_all_ports (idempotency) +- name: publish_all_ports (null -> true with published_ports) docker_container: image: "{{ docker_test_image_alpine }}" command: '/bin/sh -c "sleep 10m"' @@ -327,9 +375,22 @@ - "9001:9001" - "9010-9050:9010-9050" force_kill: yes - register: publish_all_ports_2 + register: publish_all_ports_7 -- name: publish_all_ports true -> false +- name: publish_all_ports (Idempotency with published_ports) + docker_container: + image: "{{ docker_test_image_alpine }}" + command: '/bin/sh -c "sleep 10m"' + name: "{{ cname }}" + state: started + publish_all_ports: true + published_ports: + - "9001:9001" + - "9010-9050:9010-9050" + force_kill: yes + register: publish_all_ports_8 + +- name: publish_all_ports (true -> false with published_ports) docker_container: image: "{{ docker_test_image_alpine }}" command: '/bin/sh -c "sleep 10m"' @@ -340,7 +401,33 @@ - "9001:9001" - "9010-9050:9010-9050" force_kill: yes - register: publish_all_ports_3 + register: publish_all_ports_9 + +- name: publish_all_ports (Idempotency2 with published_ports) + docker_container: + image: "{{ docker_test_image_alpine }}" + command: '/bin/sh -c "sleep 10m"' + name: "{{ cname }}" + state: started + publish_all_ports: false + published_ports: + - "9001:9001" + - "9010-9050:9010-9050" + force_kill: yes + register: publish_all_ports_10 + +- name: publish_all_ports (false -> null with published_ports) + docker_container: + image: "{{ docker_test_image_alpine }}" + command: '/bin/sh -c "sleep 10m"' + name: "{{ cname }}" + state: started + publish_all_ports: false + published_ports: + - "9001:9001" + - "9010-9050:9010-9050" + force_kill: yes + register: publish_all_ports_11 - name: cleanup docker_container: @@ -354,3 +441,11 @@ - publish_all_ports_1 is changed - publish_all_ports_2 is not changed - publish_all_ports_3 is changed + - publish_all_ports_4 is not changed + - publish_all_ports_5 is not changed + - publish_all_ports_6 is changed + - publish_all_ports_7 is changed + - publish_all_ports_8 is not changed + - publish_all_ports_9 is changed + - publish_all_ports_10 is not changed + - publish_all_ports_11 is not changed From 9d5ec3c0cfe1eaa0161293b919b2510ee1921f11 Mon Sep 17 00:00:00 2001 From: Andrew Pantuso Date: Sun, 27 Jun 2021 10:01:32 -0400 Subject: [PATCH 6/7] Updating docs and cleaning up integration tests --- plugins/modules/docker_container.py | 2 +- .../docker_container/tasks/tests/ports.yml | 203 ++++++------------ 2 files changed, 67 insertions(+), 138 deletions(-) diff --git a/plugins/modules/docker_container.py b/plugins/modules/docker_container.py index 839e51b19..b99a9c930 100644 --- a/plugins/modules/docker_container.py +++ b/plugins/modules/docker_container.py @@ -1761,7 +1761,7 @@ def _parse_publish_ports(self): 'Specifying "all" in published_ports together with port mappings is not properly ' 'supported by the module. The port mappings are currently ignored. Set publish_all_ports ' 'to "true" to randomly assign port mappings for those not specified by published_ports. ' - 'The use of "all" in published_ports will be removed in version 2.0.0.', + 'The use of "all" in published_ports next to other values will be removed in version 2.0.0.', collection_name='community.docker', version='2.0.0') return 'all' diff --git a/tests/integration/targets/docker_container/tasks/tests/ports.yml b/tests/integration/targets/docker_container/tasks/tests/ports.yml index 241e4a1bc..b836fe979 100644 --- a/tests/integration/targets/docker_container/tasks/tests/ports.yml +++ b/tests/integration/targets/docker_container/tasks/tests/ports.yml @@ -303,149 +303,78 @@ ## publish_all_ports ############################################### #################################################################### -- name: publish_all_ports (true) +- set_fact: + publish_all_ports_test_cases: + - test_name: no_options + changed: true + - test_name: null_to_true + publish_all_ports_value: true + changed: true + - test_name: true_idempotency + publish_all_ports_value: true + changed: false + - test_name: true_to_null + changed: false + - test_name: null_to_true_2 + publish_all_ports_value: true + changed: false + - test_name: true_to_false + publish_all_ports_value: false + changed: true + - test_name: false_idempotency + publish_all_ports_value: false + changed: false + - test_name: false_to_null + changed: false + - test_name: null_with_published_ports + published_ports_value: &ports + - "9001:9001" + - "9010-9050:9010-9050" + changed: true + - test_name: null_to_true_with_published_ports + publish_all_ports_value: true + published_ports_value: *ports + changed: true + - test_name: true_idempotency_with_published_ports + publish_all_ports_value: true + published_ports_value: *ports + changed: false + - test_name: true_to_null_with_published_ports + published_ports_value: *ports + changed: false + - test_name: null_to_true_2_with_published_ports + publish_all_ports_value: true + published_ports_value: *ports + changed: false + - test_name: true_to_false_with_published_ports + publish_all_ports_value: false + published_ports_value: *ports + changed: true + - test_name: false_idempotency_with_published_ports + publish_all_ports_value: false + published_ports_value: *ports + changed: false + - test_name: false_to_null_with_published_ports + published_ports_value: *ports + changed: false + +- name: publish_all_ports ({{ test_case.test_name }}) docker_container: image: "{{ docker_test_image_alpine }}" command: '/bin/sh -c "sleep 10m"' name: "{{ cname }}" state: started - publish_all_ports: true - force_kill: yes - register: publish_all_ports_1 - -- name: publish_all_ports (Idempotency) - docker_container: - image: "{{ docker_test_image_alpine }}" - command: '/bin/sh -c "sleep 10m"' - name: "{{ cname }}" - state: started - publish_all_ports: true - force_kill: yes - register: publish_all_ports_2 - -- name: publish_all_ports (false) - docker_container: - image: "{{ docker_test_image_alpine }}" - command: '/bin/sh -c "sleep 10m"' - name: "{{ cname }}" - state: started - publish_all_ports: false - force_kill: yes - register: publish_all_ports_3 - -- name: publish_all_ports (Idempotency 2) - docker_container: - image: "{{ docker_test_image_alpine }}" - command: '/bin/sh -c "sleep 10m"' - name: "{{ cname }}" - state: started - publish_all_ports: false - force_kill: yes - register: publish_all_ports_4 - -- name: publish_all_ports (null) - docker_container: - image: "{{ docker_test_image_alpine }}" - command: '/bin/sh -c "sleep 10m"' - name: "{{ cname }}" - state: started - force_kill: yes - register: publish_all_ports_5 - -- name: publish_all_ports (null with published_ports) - docker_container: - image: "{{ docker_test_image_alpine }}" - command: '/bin/sh -c "sleep 10m"' - name: "{{ cname }}" - state: started - published_ports: - - "9001:9001" - - "9010-9050:9010-9050" + publish_all_ports: "{{ test_case.publish_all_ports_value | default(omit) }}" + published_ports: "{{ test_case.published_ports_value | default(omit) }}" force_kill: yes - register: publish_all_ports_6 - -- name: publish_all_ports (null -> true with published_ports) - docker_container: - image: "{{ docker_test_image_alpine }}" - command: '/bin/sh -c "sleep 10m"' - name: "{{ cname }}" - state: started - publish_all_ports: true - published_ports: - - "9001:9001" - - "9010-9050:9010-9050" - force_kill: yes - register: publish_all_ports_7 - -- name: publish_all_ports (Idempotency with published_ports) - docker_container: - image: "{{ docker_test_image_alpine }}" - command: '/bin/sh -c "sleep 10m"' - name: "{{ cname }}" - state: started - publish_all_ports: true - published_ports: - - "9001:9001" - - "9010-9050:9010-9050" - force_kill: yes - register: publish_all_ports_8 - -- name: publish_all_ports (true -> false with published_ports) - docker_container: - image: "{{ docker_test_image_alpine }}" - command: '/bin/sh -c "sleep 10m"' - name: "{{ cname }}" - state: started - publish_all_ports: false - published_ports: - - "9001:9001" - - "9010-9050:9010-9050" - force_kill: yes - register: publish_all_ports_9 - -- name: publish_all_ports (Idempotency2 with published_ports) - docker_container: - image: "{{ docker_test_image_alpine }}" - command: '/bin/sh -c "sleep 10m"' - name: "{{ cname }}" - state: started - publish_all_ports: false - published_ports: - - "9001:9001" - - "9010-9050:9010-9050" - force_kill: yes - register: publish_all_ports_10 - -- name: publish_all_ports (false -> null with published_ports) - docker_container: - image: "{{ docker_test_image_alpine }}" - command: '/bin/sh -c "sleep 10m"' - name: "{{ cname }}" - state: started - publish_all_ports: false - published_ports: - - "9001:9001" - - "9010-9050:9010-9050" - force_kill: yes - register: publish_all_ports_11 - -- name: cleanup - docker_container: - name: "{{ cname }}" - state: absent - force_kill: yes - diff: no + register: publish_all_ports + loop_control: + loop_var: test_case + loop: "{{ publish_all_ports_test_cases }}" - assert: that: - - publish_all_ports_1 is changed - - publish_all_ports_2 is not changed - - publish_all_ports_3 is changed - - publish_all_ports_4 is not changed - - publish_all_ports_5 is not changed - - publish_all_ports_6 is changed - - publish_all_ports_7 is changed - - publish_all_ports_8 is not changed - - publish_all_ports_9 is changed - - publish_all_ports_10 is not changed - - publish_all_ports_11 is not changed + - publish_all_ports.results[index].changed == publish_all_ports_test_cases[index].changed + loop: "{{ range(0, publish_all_ports_test_cases | length) | list }}" + loop_control: + loop_var: index \ No newline at end of file From db3486e2d0c048a55e8f77329b3818221bc5c978 Mon Sep 17 00:00:00 2001 From: Andrew Pantuso Date: Sun, 27 Jun 2021 10:25:34 -0400 Subject: [PATCH 7/7] Updating test loop logic --- .../targets/docker_container/tasks/tests/ports.yml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/integration/targets/docker_container/tasks/tests/ports.yml b/tests/integration/targets/docker_container/tasks/tests/ports.yml index b836fe979..370d038ea 100644 --- a/tests/integration/targets/docker_container/tasks/tests/ports.yml +++ b/tests/integration/targets/docker_container/tasks/tests/ports.yml @@ -374,7 +374,8 @@ - assert: that: - - publish_all_ports.results[index].changed == publish_all_ports_test_cases[index].changed - loop: "{{ range(0, publish_all_ports_test_cases | length) | list }}" + - publish_all_ports.results[index].changed == test_case.changed + loop: "{{ publish_all_ports_test_cases }}" loop_control: - loop_var: index \ No newline at end of file + index_var: index + loop_var: test_case \ No newline at end of file