From ce1accedbce1d841d2fa88676fabbdaa1487f92f Mon Sep 17 00:00:00 2001 From: Alex Willmer Date: Fri, 30 Aug 2024 16:00:52 +0100 Subject: [PATCH 1/3] tests: Refactor Ansible copy integration tests to be loop driven This is in anticipation of #1110, which only exhibits inside a with_items: loop. For this refactor `loop:` is used, to confirm the refactored tests are still correct. A subsequent commit will change them to with_items. The content of the files and their SHA1 checksums are unchanged. --- tests/ansible/integration/action/copy.yml | 125 +++++++++++----------- 1 file changed, 62 insertions(+), 63 deletions(-) diff --git a/tests/ansible/integration/action/copy.yml b/tests/ansible/integration/action/copy.yml index 73f3bd1e..cbebe3ab 100644 --- a/tests/ansible/integration/action/copy.yml +++ b/tests/ansible/integration/action/copy.yml @@ -2,91 +2,90 @@ - name: integration/action/copy.yml hosts: test-targets - tasks: - - name: Create tiny file - copy: - dest: /tmp/copy-tiny-file - content: - this is a tiny file. - delegate_to: localhost - run_once: true + vars: + sourced_files: + - src: /tmp/copy-tiny-file + dest: /tmp/copy-tiny-file.out + content: this is a tiny file. + expected_checksum: f29faa9a6f19a700a941bf2aa5b281643c4ec8a0 + - src: /tmp/copy-large-file + dest: /tmp/copy-large-file.out + content: "{{ 'x' * 200000 }}" + expected_checksum: 62951f943c41cdd326e5ce2b53a779e7916a820d + + inline_files: + - dest: /tmp/copy-tiny-inline-file.out + content: tiny inline content + expected_checksum: b26dd6444595e2bdb342aa0a91721b57478b5029 + - dest: /tmp/copy-large-inline-file.out + content: | + {{ 'y' * 200000 }} + expected_checksum: d675f47e467eae19e49032a2cc39118e12a6ee72 - - name: Create large file + files: "{{ sourced_files + inline_files }}" + tasks: + - name: Create sourced files copy: - dest: /tmp/copy-large-file - # Must be larger than Connection.SMALL_SIZE_LIMIT. - content: "{% for x in range(200000) %}x{% endfor %}" + dest: "{{ item.src }}" + content: "{{ item.content }}" + mode: u=rw,go=r + loop: "{{ sourced_files }}" + loop_control: + label: "{{ item.src }}" delegate_to: localhost run_once: true - - name: Cleanup copied files + - name: Cleanup lingering destination files file: + path: "{{ item.dest }}" state: absent - path: "{{item}}" - with_items: - - /tmp/copy-tiny-file.out - - /tmp/copy-large-file.out - - /tmp/copy-tiny-inline-file.out - - /tmp/copy-large-inline-file.out + loop: "{{ files }}" + loop_control: + label: "{{ item.dest }}" - - name: Copy large file + - name: Copy sourced files copy: - dest: /tmp/copy-large-file.out - src: /tmp/copy-large-file - - - name: Copy tiny file - copy: - dest: /tmp/copy-tiny-file.out - src: /tmp/copy-tiny-file + src: "{{ item.src }}" + dest: "{{ item.dest }}" + mode: u=rw,go=r + loop: "{{ sourced_files }}" + loop_control: + label: "{{ item.dest }}" - - name: Copy tiny inline file + - name: Copy inline files copy: - dest: /tmp/copy-tiny-inline-file.out - content: "tiny inline content" - - - name: Copy large inline file - copy: - dest: /tmp/copy-large-inline-file.out - content: | - {% for x in range(200000) %}y{% endfor %} + dest: "{{ item.dest }}" + content: "{{ item.content }}" + mode: u=rw,go=r + loop: "{{ inline_files }}" + loop_control: + label: "{{ item.dest }}" # stat results - name: Stat copied files stat: - path: "{{item}}" - with_items: - - /tmp/copy-tiny-file.out - - /tmp/copy-large-file.out - - /tmp/copy-tiny-inline-file.out - - /tmp/copy-large-inline-file.out + path: "{{ item.dest }}" + loop: "{{ files }}" + loop_control: + label: "{{ item.dest }}" register: stat - assert: that: - - stat.results[0].stat.checksum == "f29faa9a6f19a700a941bf2aa5b281643c4ec8a0" - - stat.results[1].stat.checksum == "62951f943c41cdd326e5ce2b53a779e7916a820d" - - stat.results[2].stat.checksum == "b26dd6444595e2bdb342aa0a91721b57478b5029" - - stat.results[3].stat.checksum == "d675f47e467eae19e49032a2cc39118e12a6ee72" - fail_msg: stat={{stat}} + - item.stat.checksum == item.item.expected_checksum + quiet: true # Avoid spamming stdout with 400 kB of item.item.content + fail_msg: item={{ item }} + loop: "{{ stat.results }}" + loop_control: + label: "{{ item.stat.path }}" - - name: Cleanup files + - name: Cleanup destination files file: + path: "{{ item.dest }}" state: absent - path: "{{item}}" - with_items: - - /tmp/copy-tiny-file - - /tmp/copy-tiny-file.out - - /tmp/copy-no-mode - - /tmp/copy-no-mode.out - - /tmp/copy-with-mode - - /tmp/copy-with-mode.out - - /tmp/copy-large-file - - /tmp/copy-large-file.out - - /tmp/copy-tiny-inline-file.out - - /tmp/copy-large-inline-file - - /tmp/copy-large-inline-file.out - - # end of cleaning out files (again) + loop: "{{ files }}" + loop_control: + label: "{{ item.dest }}" tags: - copy From 0bd3c6cba50bf9e607aa13432586af8af4fe51ba Mon Sep 17 00:00:00 2001 From: Jonathan Rosser Date: Wed, 28 Aug 2024 08:24:35 +0100 Subject: [PATCH 2/3] Fix AnsibleUnsafeText when copying files larger than SMALL_FILE_LIMIT Small files are carried in-band in the communication between controller and remote, with larger files being copied by falling back to a more traditional ansible put_file mechanism. This large file code path was missed in b822f20. --- ansible_mitogen/connection.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ansible_mitogen/connection.py b/ansible_mitogen/connection.py index 6bdf11ba..a3f66eac 100644 --- a/ansible_mitogen/connection.py +++ b/ansible_mitogen/connection.py @@ -1129,6 +1129,6 @@ class Connection(ansible.plugins.connection.ConnectionBase): self.get_chain().call( ansible_mitogen.target.transfer_file, context=self.binding.get_child_service_context(), - in_path=in_path, - out_path=out_path + in_path=ansible_mitogen.utils.unsafe.cast(in_path), + out_path=ansible_mitogen.utils.unsafe.cast(out_path) ) From 5af6534a70eb03a71c5a181bd2abcf77e2be903e Mon Sep 17 00:00:00 2001 From: Alex Willmer Date: Fri, 30 Aug 2024 17:19:20 +0100 Subject: [PATCH 3/3] tests: Test AnsibleUnsafeText when copying files larger SMALL_FILE_LIMIT The bug was fixed in a previous commit by Jonathan Rosser. This adds testing. The bug is only triggered when the copy module is used inside a `with_items:` loop and the destination filename has an extension. A `loop:` loop is not sufficient. refs #1110 --- docs/changelog.rst | 3 +++ tests/ansible/integration/action/copy.yml | 17 ++++++++++------- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 63b4da12..4fc177c1 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -23,6 +23,9 @@ Unreleased * :gh:issue:`1087` Fix :exc:`mitogen.core.StreamError` when Ansible template module is called with a ``dest:`` filename that has an extension +* :gh:issue:`1110` Fix :exc:`mitogen.core.StreamError` when Ansible copy + module is called with a file larger than 124 kibibytes + (:data:`ansible_mitogen.connection.Connection.SMALL_FILE_LIMIT`) v0.3.9 (2024-08-13) diff --git a/tests/ansible/integration/action/copy.yml b/tests/ansible/integration/action/copy.yml index cbebe3ab..edaa3e49 100644 --- a/tests/ansible/integration/action/copy.yml +++ b/tests/ansible/integration/action/copy.yml @@ -1,4 +1,6 @@ # Verify copy module for small and large files, and inline content. +# To exercise https://github.com/mitogen-hq/mitogen/pull/1110 destination +# files must have extensions and loops must use `with_items:`. - name: integration/action/copy.yml hosts: test-targets @@ -29,7 +31,7 @@ dest: "{{ item.src }}" content: "{{ item.content }}" mode: u=rw,go=r - loop: "{{ sourced_files }}" + with_items: "{{ sourced_files }}" loop_control: label: "{{ item.src }}" delegate_to: localhost @@ -39,7 +41,7 @@ file: path: "{{ item.dest }}" state: absent - loop: "{{ files }}" + with_items: "{{ files }}" loop_control: label: "{{ item.dest }}" @@ -48,7 +50,7 @@ src: "{{ item.src }}" dest: "{{ item.dest }}" mode: u=rw,go=r - loop: "{{ sourced_files }}" + with_items: "{{ sourced_files }}" loop_control: label: "{{ item.dest }}" @@ -57,7 +59,7 @@ dest: "{{ item.dest }}" content: "{{ item.content }}" mode: u=rw,go=r - loop: "{{ inline_files }}" + with_items: "{{ inline_files }}" loop_control: label: "{{ item.dest }}" @@ -66,7 +68,7 @@ - name: Stat copied files stat: path: "{{ item.dest }}" - loop: "{{ files }}" + with_items: "{{ files }}" loop_control: label: "{{ item.dest }}" register: stat @@ -76,7 +78,7 @@ - item.stat.checksum == item.item.expected_checksum quiet: true # Avoid spamming stdout with 400 kB of item.item.content fail_msg: item={{ item }} - loop: "{{ stat.results }}" + with_items: "{{ stat.results }}" loop_control: label: "{{ item.stat.path }}" @@ -84,8 +86,9 @@ file: path: "{{ item.dest }}" state: absent - loop: "{{ files }}" + with_items: "{{ files }}" loop_control: label: "{{ item.dest }}" tags: - copy + - issue_1110