From a0f806886fa50851154766c8900f9272b9867ec2 Mon Sep 17 00:00:00 2001
From: Sergey Orlov <sorlov@redhat.com>
Date: Wed, 20 Jan 2021 18:21:22 +0100
Subject: [PATCH 1/4] ipatests: add a tests-oriented wrapper for pexpect module

The pexpect module can be used for controlling and testing interactive
command-line programs. The wrapper adds testing-oriented features like
logging and automatic process termination and default check for process
exit status.

Related to: https://pagure.io/freeipa/issue/8690
---
 freeipa.spec.in                              |   2 +
 ipatests/pytest_ipa/integration/expect.py    | 153 +++++++++++++++++++
 ipatests/pytest_ipa/integration/host.py      |   3 +
 ipatests/pytest_ipa/integration/transport.py |   9 ++
 ipatests/setup.py                            |   1 +
 pylint_plugins.py                            |   1 +
 6 files changed, 169 insertions(+)
 create mode 100644 ipatests/pytest_ipa/integration/expect.py

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 29960cfc657..0d8b1dd068a 100755
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -331,6 +331,7 @@ BuildRequires:  python3-lxml
 BuildRequires:  python3-netaddr >= %{python_netaddr_version}
 BuildRequires:  python3-netifaces
 BuildRequires:  python3-paste
+BuildRequires:  python3-pexpect
 BuildRequires:  python3-pki >= %{pki_version}
 BuildRequires:  python3-polib
 BuildRequires:  python3-pyasn1
@@ -845,6 +846,7 @@ Requires: python3-ipaserver = %{version}-%{release}
 Requires: iptables
 Requires: python3-coverage
 Requires: python3-cryptography >= 1.6
+Requires: python3-pexpect
 %if 0%{?fedora}
 # These packages do not exist on RHEL and for ipatests use
 # they are installed on the controller through other means
diff --git a/ipatests/pytest_ipa/integration/expect.py b/ipatests/pytest_ipa/integration/expect.py
new file mode 100644
index 00000000000..85350a10638
--- /dev/null
+++ b/ipatests/pytest_ipa/integration/expect.py
@@ -0,0 +1,153 @@
+import time
+import logging
+
+import pexpect
+from pexpect.exceptions import ExceptionPexpect, TIMEOUT
+
+
+logger = logging.getLogger(__name__)
+logger.setLevel(logging.DEBUG)
+
+
+class IpaTestExpect(pexpect.spawn):
+    """A wrapper class around pexpect.spawn for easier usage in automated tests
+
+    Please see pexpect documentation at
+    https://pexpect.readthedocs.io/en/stable/api/index.html for general usage
+    instructions. Note that usage of "+", "*" and '?' at the end of regular
+    expressions arguments to .expect() is meaningless.
+
+    This wrapper adds ability to use the class as a context manager, which
+    will take care of verifying process return status and terminating
+    the process if it did not do it normally. The context manager is the
+    recommended way of using the class in tests.
+    Basic usage example:
+
+    ```
+        with IpaTestExpect('some_command') as e:
+            e.expect_exact('yes or no?')
+            e.sendline('yes')
+    ```
+
+    At exit from context manager the following checks are performed by default:
+    1. there is nothing in output since last call to .expect()
+    2. the process has terminated
+    3. return code is 0
+
+    If any check fails, an exceptio is raised. If you want to override checks
+    1 and 3 you can call .expect_exit() explicitly:
+
+    ```
+    with IpaTestExpect('some_command') as e:
+        ...
+        e.expect_exit(ok_returncode=1, ignore_remaining_output=True)
+    ```
+
+    All .expect* methods are strict, meaning that if they do not find the
+    pattern in the output during given amount of time, the exception is raised.
+    So they can directly be used to verify output for presence of specific
+    strings.
+
+    Another addition is .get_last_output() method which can be used get process
+    output from penultimate up to the last call to .expect(). The result can
+    be used for more complex checks which can not be expressed as simple
+    regexes, for example we can check for absence of string in output:
+
+    ```
+    with IpaTestExpect('some_command') as e:
+        ...
+        e.expect('All done')
+        output = e.get_last_output()
+    assert 'WARNING' not in output
+    ```
+    """
+    def __init__(self, argv, default_timeout=10, encoding='utf-8'):
+        if isinstance(argv, str):
+            command = argv
+            args = []
+        else:
+            command = argv[0]
+            args = argv[1:]
+        super().__init__(
+            command, args, timeout=default_timeout, encoding=encoding,
+            echo=False
+        )
+
+    def expect_exit(self, timeout=-1, ok_returncode=0, raiseonerr=True,
+                    ignore_remaining_output=False):
+        if timeout == -1:
+            timeout = self.timeout
+        wait_to_exit_until = time.time() + timeout
+        if not self.eof():
+            self.expect(pexpect.EOF, timeout)
+        errors = []
+        if not ignore_remaining_output and self.before.strip():
+            errors.append('Unexpected output at program exit: {!r}'
+                          .format(self.before))
+
+        while time.time() < wait_to_exit_until:
+            if not self.isalive():
+                break
+            time.sleep(0.1)
+        else:
+            errors.append('Program did not exit after waiting for {} seconds'
+                          .format(self.timeout))
+        if (not self.isalive() and raiseonerr
+                and self.exitstatus != ok_returncode):
+            errors.append('Program exited with unexpected status {}'
+                          .format(self.exitstatus))
+        self.exit_checked = True
+        if errors:
+            raise ExceptionPexpect(
+                'Program exited with an unexpected state:\n'
+                + '\n'.join(errors))
+
+    def send(self, s):
+        """Wrapper to provide logging input string"""
+        logger.debug('Sending %r', s)
+        return super().send(s)
+
+    def expect_list(self, pattern_list, *args, **kwargs):
+        """Wrapper to provide logging output string and expected patterns"""
+        try:
+            result = super().expect_list(pattern_list, *args, **kwargs)
+        finally:
+            self._log_output(pattern_list)
+        return result
+
+    def expect_exact(self, pattern_list, *args, **kwargs):
+        """Wrapper to provide logging output string and expected patterns"""
+        try:
+            result = super().expect_exact(pattern_list, *args, **kwargs)
+        finally:
+            self._log_output(pattern_list)
+        return result
+
+    def get_last_output(self):
+        """Return output consumed by last call to .expect*()"""
+        output = self.before
+        if isinstance(self.after, str):
+            output += self.after
+        return output
+
+    def _log_output(self, expected):
+        logger.debug('Output received: %r, expected: "%s", ',
+                     self.get_last_output(), expected)
+
+    def __enter__(self):
+        self.exit_checked = False
+        return self
+
+    def __exit__(self, exc_type, exc_val, exc_tb):
+        exception_occurred = bool(exc_type)
+        try:
+            if not self.exit_checked:
+                self.expect_exit(raiseonerr=not exception_occurred,
+                                 ignore_remaining_output=exception_occurred)
+        except TIMEOUT:
+            if not exception_occurred:
+                raise
+        finally:
+            if self.isalive():
+                logger.error('Command still active, terminating.')
+                self.terminate(True)
diff --git a/ipatests/pytest_ipa/integration/host.py b/ipatests/pytest_ipa/integration/host.py
index d1df228b103..bcadcbae2c4 100644
--- a/ipatests/pytest_ipa/integration/host.py
+++ b/ipatests/pytest_ipa/integration/host.py
@@ -204,6 +204,9 @@ def run_command(self, argv, set_env=True, stdin_text=None,
         else:
             return result
 
+    def spawn_expect(self, argv, default_timeout=10, encoding='utf-8'):
+        """Run command on host using IpaTestExpect"""
+        return self.transport.spawn_expect(argv, default_timeout, encoding)
 
 class WinHost(pytest_multihost.host.WinHost):
     """
diff --git a/ipatests/pytest_ipa/integration/transport.py b/ipatests/pytest_ipa/integration/transport.py
index d55e72ddb8c..f12df122542 100644
--- a/ipatests/pytest_ipa/integration/transport.py
+++ b/ipatests/pytest_ipa/integration/transport.py
@@ -7,6 +7,8 @@
 """
 import os
 
+from .expect import IpaTestExpect
+
 from pytest_multihost.transport import OpenSSHTransport
 
 
@@ -46,3 +48,10 @@ def _get_ssh_argv(self):
         self.log.debug("SSH invocation: %s", argv)
 
         return argv
+
+    def spawn_expect(self, argv, default_timeout, encoding):
+        self.log.debug('Starting pexpect ssh session')
+        if isinstance(argv, str):
+            argv = [argv]
+        argv = self._get_ssh_argv() + ['-t', '-q'] + argv
+        return IpaTestExpect(argv, default_timeout, encoding)
diff --git a/ipatests/setup.py b/ipatests/setup.py
index 8b89f90ca8f..46fb0995801 100644
--- a/ipatests/setup.py
+++ b/ipatests/setup.py
@@ -73,6 +73,7 @@
             "pytest_multihost",
             "python-ldap",
             "six",
+            "pexpect",
         ],
         extras_require={
             "integration": ["dbus-python", "pyyaml", "ipaserver"],
diff --git a/pylint_plugins.py b/pylint_plugins.py
index 82096055f55..7e1b934e0c5 100644
--- a/pylint_plugins.py
+++ b/pylint_plugins.py
@@ -217,6 +217,7 @@ def fake_class(name_or_class_obj, members=()):
             'put_file_contents',
             'get_file_contents',
             'ldap_connect',
+            {'spawn_expect': ['__enter__', '__exit__']},
         ]},
         'replicas',
         'clients',

From 4ff14c4114a596314877551119cd999acd8f3d4a Mon Sep 17 00:00:00 2001
From: Sergey Orlov <sorlov@redhat.com>
Date: Wed, 20 Jan 2021 18:24:57 +0100
Subject: [PATCH 2/4] ipatests: use pexpect to invoke ktutil

`ktutil` is a REPL-style utility that can be controlled only interactively.
The common approach of sending commands to stdin does not work with it on
systems where `readline` library has version less then 8.0 due to a bug
in that version.
With `pexpect` we avoid this bug because it emulates the terminal
when interacting with spawned process instead of simply sending all input
to stdin.

Related to: https://pagure.io/freeipa/issue/8690
---
 ipatests/pytest_ipa/integration/tasks.py | 34 +++++++++++++-----------
 1 file changed, 19 insertions(+), 15 deletions(-)

diff --git a/ipatests/pytest_ipa/integration/tasks.py b/ipatests/pytest_ipa/integration/tasks.py
index 70ec1a3ec43..b91859816dd 100755
--- a/ipatests/pytest_ipa/integration/tasks.py
+++ b/ipatests/pytest_ipa/integration/tasks.py
@@ -2184,19 +2184,6 @@ def extract_key_refs(self, keytab, princ=None):
         return keys_to_sync
 
     def copy_key(self, keytab, keyentry):
-        # keyentry.key is a hex value of the actual key
-        # prefixed with 0x, as produced by klist -K -k.
-        # However, ktutil accepts hex value without 0x, so
-        # we should strip first two characters.
-        stdin = textwrap.dedent("""\
-        rkt {keytab}
-        addent -key -p {principal} -k {kvno} -e {etype}
-        {key}
-        wkt {keytab}
-        """).format(keytab=keytab, principal=keyentry.principal,
-                    kvno=keyentry.kvno, etype=keyentry.etype,
-                    key=keyentry.key[2:])
-
         def get_keytab_mtime():
             """Get keytab file mtime.
 
@@ -2211,8 +2198,25 @@ def get_keytab_mtime():
 
         mtime_before = get_keytab_mtime()
 
-        self.host.run_command([paths.KTUTIL], stdin_text=stdin,
-                              log_stdout=False)
+        with self.host.spawn_expect(paths.KTUTIL, default_timeout=5) as e:
+            e.expect_exact('ktutil:')
+            e.sendline('rkt {}'.format(keytab))
+            e.expect_exact('ktutil:')
+            e.sendline(
+                'addent -key -p {principal} -k {kvno} -e {etype}'
+                .format(principal=keyentry.principal, kvno=keyentry.kvno,
+                        etype=keyentry.etype,))
+            e.expect('Key for.+:')
+            # keyentry.key is a hex value of the actual key
+            # prefixed with 0x, as produced by klist -K -k.
+            # However, ktutil accepts hex value without 0x, so
+            # we should strip first two characters.
+            e.sendline(keyentry.key[2:])
+            e.expect_exact('ktutil:')
+            e.sendline('wkt {}'.format(keytab))
+            e.expect_exact('ktutil:')
+            e.sendline('q')
+
         if mtime_before == get_keytab_mtime():
             raise Exception('{} did not update keytab file "{}"'.format(
                 paths.KTUTIL, keytab))

From a1471799ab8386bbd140449099c32508dc89135e Mon Sep 17 00:00:00 2001
From: Sergey Orlov <sorlov@redhat.com>
Date: Thu, 21 Jan 2021 09:05:42 +0100
Subject: [PATCH 3/4] ipatests: use pexpect to control inetractive session of
 ipa-adtrust-install

During interactive session of `ipa-adtrust-install` the user needs to
answer several questions. This was done by sending all answers to
the processes stdin without analyzing the questions.

If the installation scenario changes at some point we can get on of the
following results:
* the test fails in the end and the root cause is not obvious
* if a new question was added
* test does not fail but answers are provided for wrong questions -
  in this case scope of test case changes without being noticed

If we use `pexpect` for controlling the session, the test will fail
immediately when it encounters unexpected question.

Related to: https://pagure.io/freeipa/issue/8690
---
 .../test_integration/test_adtrust_install.py  | 125 +++++++++++-------
 1 file changed, 75 insertions(+), 50 deletions(-)

diff --git a/ipatests/test_integration/test_adtrust_install.py b/ipatests/test_integration/test_adtrust_install.py
index e50f05c6821..bbbb385a58e 100644
--- a/ipatests/test_integration/test_adtrust_install.py
+++ b/ipatests/test_integration/test_adtrust_install.py
@@ -103,31 +103,40 @@ def test_add_agent_on_stopped_replica(self):
         self.unconfigure_replica_as_agent(self.replicas[0])
         self.replicas[0].run_command(['ipactl', 'stop'])
 
-        cmd_input = (
-            # admin password:
-            self.master.config.admin_password + '\n' +
-            # WARNING: The smb.conf already exists. Running ipa-adtrust-install
-            # will break your existing samba configuration.
-            # Do you wish to continue? [no]:
-            'yes\n'
-            # Enable trusted domains support in slapi-nis? [no]:
-            '\n' +
-            # WARNING: 1 IPA masters are not yet able to serve information
-            # about users from trusted forests.
-            # Installer can add them to the list of IPA masters allowed to
-            # access information about trusts.
-            # If you choose to do so, you also need to restart LDAP service on
-            # those masters.
-            # Refer to ipa-adtrust-install(1) man page for details.
-            # IPA master[replica1.testrelm.test]?[no]:
-            'yes\n'
-        )
         try:
-            res = self.master.run_command(['ipa-adtrust-install',
-                                           '--add-agents'],
-                                          stdin_text=cmd_input)
-            expected_re = '"ipactl restart".+"systemctl restart sssd"'
-            assert re.search(expected_re, res.stdout_text, re.DOTALL)
+            cmd = ['ipa-adtrust-install', '--add-agents']
+            with self.master.spawn_expect(cmd) as e:
+                e.expect('admin password:')
+                e.sendline(self.master.config.admin_password)
+                # WARNING: The smb.conf already exists.
+                # Running ipa-adtrust-install
+                # will break your existing samba configuration.
+                # Do you wish to continue? [no]:
+                e.expect([
+                    'smb\\.conf detected.+Overwrite smb\\.conf\\?',
+                    'smb\\.conf already exists.+Do you wish to continue\\?'])
+                e.sendline('yes')
+                e.expect_exact('Enable trusted domains support in slapi-nis?')
+                e.sendline('no')
+                # WARNING: 1 IPA masters are not yet able to serve information
+                # about users from trusted forests.
+                # Installer can add them to the list of IPA masters allowed to
+                # access information about trusts.
+                # If you choose to do so, you also need to restart LDAP
+                # service on
+                # those masters.
+                # Refer to ipa-adtrust-install(1) man page for details.
+                # IPA master[replica1.testrelm.test]?[no]:
+                e.expect('Installer can add them to the list of IPA masters '
+                         'allowed to access information about trusts.+'
+                         'IPA master \\[{}\\]'
+                         .format(re.escape(self.replicas[0].hostname)),
+                         timeout=120)
+                e.sendline('yes')
+                e.expect('"ipactl restart".+"systemctl restart sssd".+'
+                         + re.escape(self.replicas[0].hostname),
+                         timeout=60)
+                e.expect_exit(ignore_remaining_output=True)
         finally:
             self.replicas[0].run_command(['ipactl', 'start'])
 
@@ -141,15 +150,20 @@ def test_add_agent_on_running_replica_without_compat(self):
         replica.
         """
         self.unconfigure_replica_as_agent(self.replicas[0])
-        cmd_input = (
-            # admin password:
-            self.master.config.admin_password + '\n' +
-            # WARNING: The smb.conf already exists. Running ipa-adtrust-install
+        cmd = ['ipa-adtrust-install', '--add-agents']
+        with self.master.spawn_expect(cmd) as e:
+            e.expect_exact('admin password:')
+            e.sendline(self.master.config.admin_password)
+            # WARNING: The smb.conf already exists.
+            # Running ipa-adtrust-install
             # will break your existing samba configuration.
             # Do you wish to continue? [no]:
-            'yes\n'
-            # Enable trusted domains support in slapi-nis? [no]:
-            '\n' +
+            e.expect([
+                'smb\\.conf detected.+Overwrite smb\\.conf\\?',
+                'smb\\.conf already exists.+Do you wish to continue\\?'])
+            e.sendline('yes')
+            e.expect_exact('Enable trusted domains support in slapi-nis?')
+            e.sendline('no')
             # WARNING: 1 IPA masters are not yet able to serve information
             # about users from trusted forests.
             # Installer can add them to the list of IPA masters allowed to
@@ -158,13 +172,17 @@ def test_add_agent_on_running_replica_without_compat(self):
             # those masters.
             # Refer to ipa-adtrust-install(1) man page for details.
             # IPA master[replica1.testrelm.test]?[no]:
-            'yes\n'
-        )
-        expected = '"ipactl restart"'
-        res = self.master.run_command(['ipa-adtrust-install', '--add-agents'],
-                                      stdin_text=cmd_input)
+            e.expect('Installer can add them to the list of IPA masters '
+                     'allowed to access information about trusts.+'
+                     'IPA master \\[{}\\]'
+                     .format(re.escape(self.replicas[0].hostname)),
+                     timeout=120)
+            e.sendline('yes')
+            e.expect_exit(ignore_remaining_output=True, timeout=60)
+            output = e.get_last_output()
+        assert 'Setup complete' in output
         # The replica must have been restarted automatically, no msg required
-        assert expected not in res.stdout_text
+        assert 'ipactl restart' not in output
 
     def test_add_agent_on_running_replica_with_compat(self):
         """ Check ipa-addtrust-install --add-agents when the replica is running
@@ -177,15 +195,18 @@ def test_add_agent_on_running_replica_with_compat(self):
         """
         self.unconfigure_replica_as_agent(self.replicas[0])
 
-        cmd_input = (
-            # admin password:
-            self.master.config.admin_password + '\n' +
-            # WARNING: The smb.conf already exists. Running ipa-adtrust-install
+        cmd = ['ipa-adtrust-install', '--add-agents', '--enable-compat']
+        with self.master.spawn_expect(cmd) as e:
+            e.expect_exact('admin password:')
+            e.sendline(self.master.config.admin_password)
+            # WARNING: The smb.conf already exists.
+            # Running ipa-adtrust-install
             # will break your existing samba configuration.
             # Do you wish to continue? [no]:
-            'yes\n'
-            # Enable trusted domains support in slapi-nis? [no]:
-            'yes\n' +
+            e.expect([
+                'smb\\.conf detected.+Overwrite smb\\.conf\\?',
+                'smb\\.conf already exists.+Do you wish to continue\\?'])
+            e.sendline('yes')
             # WARNING: 1 IPA masters are not yet able to serve information
             # about users from trusted forests.
             # Installer can add them to the list of IPA masters allowed to
@@ -194,13 +215,17 @@ def test_add_agent_on_running_replica_with_compat(self):
             # those masters.
             # Refer to ipa-adtrust-install(1) man page for details.
             # IPA master[replica1.testrelm.test]?[no]:
-            'yes\n'
-        )
-        expected = '"ipactl restart"'
-        res = self.master.run_command(['ipa-adtrust-install', '--add-agents'],
-                                      stdin_text=cmd_input)
+            e.expect('Installer can add them to the list of IPA masters '
+                     'allowed to access information about trusts.+'
+                     'IPA master \\[{}\\]'
+                     .format(re.escape(self.replicas[0].hostname)),
+                     timeout=120)
+            e.sendline('yes')
+            e.expect_exit(ignore_remaining_output=True, timeout=60)
+            output = e.get_last_output()
+        assert 'Setup complete' in output
         # The replica must have been restarted automatically, no msg required
-        assert expected not in res.stdout_text
+        assert 'ipactl restart' not in output
 
         # Ensure that the schema compat plugin is configured:
         conn = self.replicas[0].ldap_connect()

From ef1caafd82be3d353cfb9e7035d865162d80794e Mon Sep 17 00:00:00 2001
From: Sergey Orlov <sorlov@redhat.com>
Date: Wed, 3 Feb 2021 11:35:22 +0100
Subject: [PATCH 4/4] temp commit

---
 .freeipa-pr-ci.yaml                        |  2 +-
 ipatests/prci_definitions/temp_commit.yaml | 18 +++++++++++++++---
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/.freeipa-pr-ci.yaml b/.freeipa-pr-ci.yaml
index abcf8c5b634..80656690080 120000
--- a/.freeipa-pr-ci.yaml
+++ b/.freeipa-pr-ci.yaml
@@ -1 +1 @@
-ipatests/prci_definitions/gating.yaml
\ No newline at end of file
+ipatests/prci_definitions/temp_commit.yaml
\ No newline at end of file
diff --git a/ipatests/prci_definitions/temp_commit.yaml b/ipatests/prci_definitions/temp_commit.yaml
index 58560d8c49f..a0334cb4317 100644
--- a/ipatests/prci_definitions/temp_commit.yaml
+++ b/ipatests/prci_definitions/temp_commit.yaml
@@ -61,14 +61,26 @@ jobs:
         timeout: 1800
         topology: *build
 
-  fedora-latest-ipa-4-9/temp_commit:
+  fedora-latest-ipa-4-9/test_smb:
+    requires: [fedora-latest-ipa-4-9/build]
+    priority: 50
+    job:
+      class: RunADTests
+      args:
+        build_url: '{fedora-latest-ipa-4-9/build_url}'
+        test_suite: test_integration/test_smb.py
+        template: *ci-ipa-4-9-latest
+        timeout: 7200
+        topology: *ad_master_2client
+
+  fedora-latest-ipa-4-9/test_adtrust_install:
     requires: [fedora-latest-ipa-4-9/build]
     priority: 50
     job:
       class: RunPytest
       args:
         build_url: '{fedora-latest-ipa-4-9/build_url}'
-        test_suite: test_integration/test_REPLACEME.py
+        test_suite: test_integration/test_adtrust_install.py
         template: *ci-ipa-4-9-latest
         timeout: 3600
-        topology: *master_1repl_1client
+        topology: *master_1repl
