Fix ctrl+c in pause module and add tests (#40134)

* Fix all cases with pause and ctrl+c
 - naked:
	- pause:
 - with prompt
	- pause: prompt=hi
 - time wait
	- pause: seconds=60
 - time wait with prompt
	- pause: seconds=60 prompt=hi


Fixes #35372

* Use curses to control stdout
* Use curses to clear lines on interactive input
* Validate input for echo parameter and fail nicely if invalid
* Add integration tests for pause module using pexpect
* Use try except when trying to determine erase sequence to account for lack of TTY in containers in tests
* Improve output validation for regular paus test
* Accept two digit precision for pause length in test
* Check for seconds when seconds is specificed, minutes when minutes is specified
* Add test for no TTY mode

Co-authored by: Toshio Kuratomi <a.badger@gmail.com>
Co-authored by: Brian Coca <brian.coca+git@gmail.com>

12 files changed, 475 insertions, 28 deletions

diff --git a/changelogs/fragments/pause-ctrl-c.yaml b/changelogs/fragments/pause-ctrl-c.yaml
new file mode 100644
index 0000000000..c7e3bf418f
--- /dev/null
+++ b/changelogs/fragments/pause-ctrl-c.yaml
@@ -0,0 +1,2 @@
+bugfixes:
+  - pause - ensure ctrl+c interrupt works in all cases (https://github.com/ansible/ansible/issues/35372)
diff --git a/lib/ansible/plugins/action/pause.py b/lib/ansible/plugins/action/pause.py
index 0813aecad2..bdf9c9a51c 100644
--- a/lib/ansible/plugins/action/pause.py
+++ b/lib/ansible/plugins/action/pause.py
@@ -19,14 +19,16 @@ __metaclass__ = type
 
 import datetime
 import signal
+import sys
 import termios
 import time
 import tty
 
 from os import isatty
 from ansible.errors import AnsibleError
+from ansible.module_utils._text import to_text, to_native
+from ansible.module_utils.parsing.convert_bool import boolean
 from ansible.module_utils.six import PY3
-from ansible.module_utils._text import to_text
 from ansible.plugins.action import ActionBase
 
 try:
@@ -35,6 +37,20 @@ except ImportError:
     from ansible.utils.display import Display
     display = Display()
 
+try:
+    import curses
+    curses.setupterm()
+    HAS_CURSES = True
+except (ImportError, curses.error):
+    HAS_CURSES = False
+
+if HAS_CURSES:
+    MOVE_TO_BOL = curses.tigetstr('cr')
+    CLEAR_TO_EOL = curses.tigetstr('el')
+else:
+    MOVE_TO_BOL = b'\r'
+    CLEAR_TO_EOL = b'\x1b[K'
+
 
 class AnsibleTimeoutExceeded(Exception):
     pass
@@ -44,6 +60,11 @@ def timeout_handler(signum, frame):
     raise AnsibleTimeoutExceeded
 
 
+def clear_line(stdout):
+    stdout.write(b'\x1b[%s' % MOVE_TO_BOL)
+    stdout.write(b'\x1b[%s' % CLEAR_TO_EOL)
+
+
 class ActionModule(ActionBase):
     ''' pauses execution for a length or time, or until input is received '''
 
@@ -81,10 +102,11 @@ class ActionModule(ActionBase):
 
         # Should keystrokes be echoed to stdout?
         if 'echo' in self._task.args:
-            echo = self._task.args['echo']
-            if not type(echo) == bool:
+            try:
+                echo = boolean(self._task.args['echo'])
+            except TypeError as e:
                 result['failed'] = True
-                result['msg'] = "'%s' is not a valid setting for 'echo'." % self._task.args['echo']
+                result['msg'] = to_native(e)
                 return result
 
             # Add a note saying the output is hidden if echo is disabled
@@ -96,7 +118,7 @@ class ActionModule(ActionBase):
             prompt = "[%s]\n%s%s:" % (self._task.get_name().strip(), self._task.args['prompt'], echo_prompt)
         else:
             # If no custom prompt is specified, set a default prompt
-            prompt = "[%s]\n%s%s:" % (self._task.get_name().strip(), 'Press enter to continue', echo_prompt)
+            prompt = "[%s]\n%s%s:" % (self._task.get_name().strip(), 'Press enter to continue, Ctrl+C to interrupt', echo_prompt)
 
         # Are 'minutes' or 'seconds' keys that exist in 'args'?
         if 'minutes' in self._task.args or 'seconds' in self._task.args:
@@ -149,65 +171,83 @@ class ActionModule(ActionBase):
             try:
                 if PY3:
                     stdin = self._connection._new_stdin.buffer
+                    stdout = sys.stdout.buffer
                 else:
                     stdin = self._connection._new_stdin
+                    stdout = sys.stdout
                 fd = stdin.fileno()
             except (ValueError, AttributeError):
                 # ValueError: someone is using a closed file descriptor as stdin
                 # AttributeError: someone is using a null file descriptor as stdin on windoez
                 stdin = None
+
             if fd is not None:
                 if isatty(fd):
+
+                    # grab actual Ctrl+C sequence
+                    try:
+                        intr = termios.tcgetattr(fd)[6][termios.VINTR]
+                    except Exception:
+                        # unsupported/not present, use default
+                        intr = b'\x03'  # value for Ctrl+C
+
+                    # get backspace sequences
+                    try:
+                        backspace = termios.tcgetattr(fd)[6][termios.VERASE]
+                    except Exception:
+                        backspace = [b'\x7f', b'\x08']
+
                     old_settings = termios.tcgetattr(fd)
                     tty.setraw(fd)
+                    tty.setraw(stdout.fileno())
 
-                    # Enable a few things turned off by tty.setraw()
-                    # ICANON -> Allows characters to be deleted and hides things like ^M.
-                    # ICRNL -> Makes the return key work when ICANON is enabled, otherwise
-                    #          you get stuck at the prompt with no way to get out of it.
-                    # See man termios for details on these flags
-                    if not seconds:
+                    # Only echo input if no timeout is specified
+                    if not seconds and echo:
                         new_settings = termios.tcgetattr(fd)
-                        new_settings[0] = new_settings[0] | termios.ICRNL
-                        new_settings[3] = new_settings[3] | termios.ICANON
+                        new_settings[3] = new_settings[3] | termios.ECHO
                         termios.tcsetattr(fd, termios.TCSANOW, new_settings)
 
-                        if echo:
-                            # Enable ECHO since tty.setraw() disables it
-                            new_settings = termios.tcgetattr(fd)
-                            new_settings[3] = new_settings[3] | termios.ECHO
-                            termios.tcsetattr(fd, termios.TCSANOW, new_settings)
-
                     # flush the buffer to make sure no previous key presses
                     # are read in below
                     termios.tcflush(stdin, termios.TCIFLUSH)
+
             while True:
                 try:
                     if fd is not None:
                         key_pressed = stdin.read(1)
-
-                        if seconds:
-                            if key_pressed == b'\x03':
-                                raise KeyboardInterrupt
+                        if key_pressed == intr:  # value for Ctrl+C
+                            clear_line(stdout)
+                            raise KeyboardInterrupt
 
                     if not seconds:
                         if fd is None or not isatty(fd):
-                            display.warning("Not waiting from prompt as stdin is not interactive")
+                            display.warning("Not waiting for response to prompt as stdin is not interactive")
                             break
+
                         # read key presses and act accordingly
                         if key_pressed in (b'\r', b'\n'):
+                            clear_line(stdout)
                             break
+                        elif key_pressed in backspace:
+                            # delete a character if backspace is pressed
+                            result['user_input'] = result['user_input'][:-1]
+                            clear_line(stdout)
+                            if echo:
+                                stdout.write(result['user_input'])
+                            stdout.flush()
                         else:
                             result['user_input'] += key_pressed
 
                 except KeyboardInterrupt:
-                    if seconds is not None:
-                        signal.alarm(0)
+                    signal.alarm(0)
                     display.display("Press 'C' to continue the play or 'A' to abort \r"),
                     if self._c_or_a(stdin):
+                        clear_line(stdout)
                         break
-                    else:
-                        raise AnsibleError('user requested abort!')
+
+                    clear_line(stdout)
+
+                    raise AnsibleError('user requested abort!')
 
         except AnsibleTimeoutExceeded:
             # this is the exception we expect when the alarm signal
diff --git a/test/integration/targets/pause/aliases b/test/integration/targets/pause/aliases
new file mode 100644
index 0000000000..7af8b7f05b
--- /dev/null
+++ b/test/integration/targets/pause/aliases
@@ -0,0 +1 @@
+posix/ci/group2
diff --git a/test/integration/targets/pause/pause-1.yml b/test/integration/targets/pause/pause-1.yml
new file mode 100644
index 0000000000..493036895f
--- /dev/null
+++ b/test/integration/targets/pause/pause-1.yml
@@ -0,0 +1,11 @@
+- name: Test pause module in default state
+  hosts: testhost
+  become: no
+  gather_facts: no
+
+  tasks:
+    - name: EXPECTED FAILURE
+      pause:
+
+    - debug:
+        msg: Task after pause
diff --git a/test/integration/targets/pause/pause-2.yml b/test/integration/targets/pause/pause-2.yml
new file mode 100644
index 0000000000..e3d900ec47
--- /dev/null
+++ b/test/integration/targets/pause/pause-2.yml
@@ -0,0 +1,12 @@
+- name: Test pause module with custom prompt
+  hosts: testhost
+  become: no
+  gather_facts: no
+
+  tasks:
+    - name: EXPECTED FAILURE
+      pause:
+        prompt: Custom prompt
+
+    - debug:
+        msg: Task after pause
diff --git a/test/integration/targets/pause/pause-3.yml b/test/integration/targets/pause/pause-3.yml
new file mode 100644
index 0000000000..8daf061054
--- /dev/null
+++ b/test/integration/targets/pause/pause-3.yml
@@ -0,0 +1,12 @@
+- name: Test pause module with pause
+  hosts: testhost
+  become: no
+  gather_facts: no
+
+  tasks:
+    - name: EXPECTED FAILURE
+      pause:
+        seconds: 2
+
+    - debug:
+        msg: Task after pause
diff --git a/test/integration/targets/pause/pause-4.yml b/test/integration/targets/pause/pause-4.yml
new file mode 100644
index 0000000000..01bd06a20c
--- /dev/null
+++ b/test/integration/targets/pause/pause-4.yml
@@ -0,0 +1,13 @@
+- name: Test pause module with pause and custom prompt
+  hosts: testhost
+  become: no
+  gather_facts: no
+
+  tasks:
+    - name: EXPECTED FAILURE
+      pause:
+        seconds: 2
+        prompt: Waiting for two seconds
+
+    - debug:
+        msg: Task after pause
diff --git a/test/integration/targets/pause/pause-5.yml b/test/integration/targets/pause/pause-5.yml
new file mode 100644
index 0000000000..2a2b154820
--- /dev/null
+++ b/test/integration/targets/pause/pause-5.yml
@@ -0,0 +1,35 @@
+- name: Test pause module echo output
+  hosts: testhost
+  become: no
+  gather_facts: no
+
+  tasks:
+    - pause:
+        echo: yes
+        prompt: Enter some text
+      register: results
+
+    - name: Ensure that input was captured
+      assert:
+        that:
+          - results.user_input == 'hello there'
+
+    - pause:
+        echo: yes
+        prompt: Enter some text to edit
+      register: result
+
+    - name: Ensure edited input was captured
+      assert:
+        that:
+          - result.user_input == 'hello tommy boy'
+
+    - pause:
+        echo: no
+        prompt: Enter some text
+      register: result
+
+    - name: Ensure secret input was caputered
+      assert:
+        that:
+          - result.user_input == 'supersecretpancakes'
diff --git a/test/integration/targets/pause/runme.sh b/test/integration/targets/pause/runme.sh
new file mode 100755
index 0000000000..aeda654bd8
--- /dev/null
+++ b/test/integration/targets/pause/runme.sh
@@ -0,0 +1,23 @@
+#!/usr/bin/env bash
+
+set -eux
+
+# Test pause module when no tty and non-interactive. This is to prevent playbooks
+# from hanging in cron and Tower jobs.
+/usr/bin/env bash << EOF
+ansible-playbook test-pause-no-tty.yml -i ../../inventory 2>&1 | \
+    grep '\[WARNING\]: Not waiting for response to prompt as stdin is not interactive' && {
+        echo 'Successfully skipped pause in no TTY mode' >&2
+        exit 0
+    } || {
+        echo 'Failed to skip pause module' >&2
+        exit 1
+    }
+EOF
+
+# Test pause with seconds and minutes specified
+ansible-playbook test-pause.yml -i ../../inventory "$@"
+
+# Interactively test pause
+pip install pexpect
+python test-pause.py -i ../../inventory "$@"
diff --git a/test/integration/targets/pause/test-pause-no-tty.yml b/test/integration/targets/pause/test-pause-no-tty.yml
new file mode 100644
index 0000000000..f7a2b36cbc
--- /dev/null
+++ b/test/integration/targets/pause/test-pause-no-tty.yml
@@ -0,0 +1,7 @@
+- name: Test pause
+  hosts: testhost
+  gather_facts: no
+  become: no
+
+  tasks:
+    - pause:
diff --git a/test/integration/targets/pause/test-pause.py b/test/integration/targets/pause/test-pause.py
new file mode 100755
index 0000000000..20afef98b1
--- /dev/null
+++ b/test/integration/targets/pause/test-pause.py
@@ -0,0 +1,270 @@
+#!/usr/bin/env python
+
+import os
+import pexpect
+import sys
+import termios
+
+from ansible.module_utils.six import PY2
+
+args = sys.argv[1:]
+
+env_vars = {
+    'ANSIBLE_ROLES_PATH': './roles',
+    'ANSIBLE_NOCOLOR': 'True',
+    'ANSIBLE_RETRY_FILES_ENABLED': 'False'
+}
+
+try:
+    backspace = termios.tcgetattr(sys.stdin.fileno())[6][termios.VERASE]
+except Exception:
+    backspace = b'\x7f'
+
+if PY2:
+    log_buffer = sys.stdout
+else:
+    log_buffer = sys.stdout.buffer
+
+os.environ.update(env_vars)
+
+# -- Plain pause -- #
+playbook = 'pause-1.yml'
+
+# Case 1 - Contiune with enter
+pause_test = pexpect.spawn(
+    'ansible-playbook',
+    args=[playbook] + args,
+    timeout=10,
+    env=os.environ
+)
+
+pause_test.logfile = log_buffer
+pause_test.expect(r'Press enter to continue, Ctrl\+C to interrupt:')
+pause_test.send('\r')
+pause_test.expect('Task after pause')
+pause_test.expect(pexpect.EOF)
+pause_test.close()
+
+
+# Case 2 - Continue with C
+pause_test = pexpect.spawn(
+    'ansible-playbook',
+    args=[playbook] + args,
+    timeout=10,
+    env=os.environ
+)
+
+pause_test.logfile = log_buffer
+pause_test.expect(r'Press enter to continue, Ctrl\+C to interrupt:')
+pause_test.send('\x03')
+pause_test.expect("Press 'C' to continue the play or 'A' to abort")
+pause_test.send('C')
+pause_test.expect('Task after pause')
+pause_test.expect(pexpect.EOF)
+pause_test.close()
+
+
+# Case 3 - Abort with A
+pause_test = pexpect.spawn(
+    'ansible-playbook',
+    args=[playbook] + args,
+    timeout=10,
+    env=os.environ
+)
+
+pause_test.logfile = log_buffer
+pause_test.expect(r'Press enter to continue, Ctrl\+C to interrupt:')
+pause_test.send('\x03')
+pause_test.expect("Press 'C' to continue the play or 'A' to abort")
+pause_test.send('A')
+pause_test.expect('user requested abort!')
+pause_test.expect(pexpect.EOF)
+pause_test.close()
+
+# -- Custom Prompt -- #
+playbook = 'pause-2.yml'
+
+# Case 1 - Contiune with enter
+pause_test = pexpect.spawn(
+    'ansible-playbook',
+    args=[playbook] + args,
+    timeout=10,
+    env=os.environ
+)
+
+pause_test.logfile = log_buffer
+pause_test.expect(r'Custom prompt:')
+pause_test.send('\r')
+pause_test.expect('Task after pause')
+pause_test.expect(pexpect.EOF)
+pause_test.close()
+
+
+# Case 2 - Contiune with C
+pause_test = pexpect.spawn(
+    'ansible-playbook',
+    args=[playbook] + args,
+    timeout=10,
+    env=os.environ
+)
+
+pause_test.logfile = log_buffer
+pause_test.expect(r'Custom prompt:')
+pause_test.send('\x03')
+pause_test.expect("Press 'C' to continue the play or 'A' to abort")
+pause_test.send('C')
+pause_test.expect('Task after pause')
+pause_test.expect(pexpect.EOF)
+pause_test.close()
+
+
+# Case 3 - Abort with A
+pause_test = pexpect.spawn(
+    'ansible-playbook',
+    args=[playbook] + args,
+    timeout=10,
+    env=os.environ
+)
+
+pause_test.logfile = log_buffer
+pause_test.expect(r'Custom prompt:')
+pause_test.send('\x03')
+pause_test.expect("Press 'C' to continue the play or 'A' to abort")
+pause_test.send('A')
+pause_test.expect('user requested abort!')
+pause_test.expect(pexpect.EOF)
+pause_test.close()
+
+# -- Pause for N seconds -- #
+
+playbook = 'pause-3.yml'
+
+# Case 1 - Wait for task to continue after timeout
+pause_test = pexpect.spawn(
+    'ansible-playbook',
+    args=[playbook] + args,
+    timeout=10,
+    env=os.environ
+)
+
+pause_test.logfile = log_buffer
+pause_test.expect(r'Pausing for \d+ seconds')
+pause_test.expect(r"\(ctrl\+C then 'C' = continue early, ctrl\+C then 'A' = abort\)")
+pause_test.expect('Task after pause')
+pause_test.expect(pexpect.EOF)
+pause_test.close()
+
+# Case 2 - Contiune with Ctrl + C, C
+pause_test = pexpect.spawn(
+    'ansible-playbook',
+    args=[playbook] + args,
+    timeout=10,
+    env=os.environ
+)
+
+pause_test.logfile = log_buffer
+pause_test.expect(r'Pausing for \d+ seconds')
+pause_test.expect(r"\(ctrl\+C then 'C' = continue early, ctrl\+C then 'A' = abort\)")
+pause_test.send('\x03')
+pause_test.send('C')
+pause_test.expect('Task after pause')
+pause_test.expect(pexpect.EOF)
+pause_test.close()
+
+
+# Case 3 - Abort with Ctrl + C, A
+pause_test = pexpect.spawn(
+    'ansible-playbook',
+    args=[playbook] + args,
+    timeout=10,
+    env=os.environ
+)
+
+pause_test.logfile = log_buffer
+pause_test.expect(r'Pausing for \d+ seconds')
+pause_test.expect(r"\(ctrl\+C then 'C' = continue early, ctrl\+C then 'A' = abort\)")
+pause_test.send('\x03')
+pause_test.send('A')
+pause_test.expect('user requested abort!')
+pause_test.expect(pexpect.EOF)
+pause_test.close()
+
+# -- Pause for N seconds with custom prompt -- #
+
+playbook = 'pause-4.yml'
+
+# Case 1 - Wait for task to continue after timeout
+pause_test = pexpect.spawn(
+    'ansible-playbook',
+    args=[playbook] + args,
+    timeout=10,
+    env=os.environ
+)
+
+pause_test.logfile = log_buffer
+pause_test.expect(r'Pausing for \d+ seconds')
+pause_test.expect(r"\(ctrl\+C then 'C' = continue early, ctrl\+C then 'A' = abort\)")
+pause_test.expect(r"Waiting for two seconds:")
+pause_test.expect('Task after pause')
+pause_test.expect(pexpect.EOF)
+pause_test.close()
+
+# Case 2 - Contiune with Ctrl + C, C
+pause_test = pexpect.spawn(
+    'ansible-playbook',
+    args=[playbook] + args,
+    timeout=10,
+    env=os.environ
+)
+
+pause_test.logfile = log_buffer
+pause_test.expect(r'Pausing for \d+ seconds')
+pause_test.expect(r"\(ctrl\+C then 'C' = continue early, ctrl\+C then 'A' = abort\)")
+pause_test.expect(r"Waiting for two seconds:")
+pause_test.send('\x03')
+pause_test.send('C')
+pause_test.expect('Task after pause')
+pause_test.expect(pexpect.EOF)
+pause_test.close()
+
+
+# Case 3 - Abort with Ctrl + C, A
+pause_test = pexpect.spawn(
+    'ansible-playbook',
+    args=[playbook] + args,
+    timeout=10,
+    env=os.environ
+)
+
+pause_test.logfile = log_buffer
+pause_test.expect(r'Pausing for \d+ seconds')
+pause_test.expect(r"\(ctrl\+C then 'C' = continue early, ctrl\+C then 'A' = abort\)")
+pause_test.expect(r"Waiting for two seconds:")
+pause_test.send('\x03')
+pause_test.send('A')
+pause_test.expect('user requested abort!')
+pause_test.expect(pexpect.EOF)
+pause_test.close()
+
+# -- Enter input and ensure it's caputered, echoed, and can be edited -- #
+
+playbook = 'pause-5.yml'
+
+pause_test = pexpect.spawn(
+    'ansible-playbook',
+    args=[playbook] + args,
+    timeout=10,
+    env=os.environ
+)
+
+pause_test.logfile = log_buffer
+pause_test.expect(r'Enter some text:')
+pause_test.sendline('hello there')
+pause_test.expect(r'Enter some text to edit:')
+pause_test.send('hello there')
+pause_test.send(backspace * 4)
+pause_test.send('ommy boy\r')
+pause_test.expect(r'Enter some text \(output is hidden\):')
+pause_test.sendline('supersecretpancakes')
+pause_test.expect(pexpect.EOF)
+pause_test.close()
diff --git a/test/integration/targets/pause/test-pause.yml b/test/integration/targets/pause/test-pause.yml
new file mode 100644
index 0000000000..cfe9f32427
--- /dev/null
+++ b/test/integration/targets/pause/test-pause.yml
@@ -0,0 +1,21 @@
+- name: Test pause
+  hosts: testhost
+  gather_facts: no
+  become: no
+
+  tasks:
+    - pause:
+        seconds: 1
+      register: results
+
+    - assert:
+        that:
+          - results.stdout is search('Paused for \d+\.\d+ seconds')
+
+    - pause:
+        minutes: 1
+      register: results
+
+    - assert:
+        that:
+          - results.stdout is search('Paused for \d+\.\d+ minutes')