Tests:port rfc2307 username begin with a space #6110

shridhargadekar · 2022-04-13T19:45:25Z

Porting and rewriting of a rfc2307 testcase which tests
a username having a space char at it's beginning

sumit-bose · 2022-04-19T11:30:49Z

src/tests/multihost/alltests/test_rfc2307.py

+        :id: 6923436c-d4e4-4a0d-a8f3-1e94ecb1dee3
+        :description: user with a white space at the beginning in it's name
+         should be able to log in
+        :bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1362023


Hi,

this bugzilla ticket does not corresponds to what is tested, can you check if it is the right one?

bye,
Sumit

Corrected the bugzilla link.

sumit-bose · 2022-04-22T10:22:44Z

Hi,

thanks for fixing the bugzilla link. I would suggest to add https://bugzilla.redhat.com/show_bug.cgi?id=1065534 as well since it describes the original issue.

I had to add the following changes to make the test working for me locally:

diff --git a/src/tests/multihost/alltests/test_rfc2307.py b/src/tests/multihost/alltests/test_rfc2307.py
index 0f5f5d357..813134369 100644
--- a/src/tests/multihost/alltests/test_rfc2307.py
+++ b/src/tests/multihost/alltests/test_rfc2307.py
@@ -12,7 +12,7 @@ from sssd.testlib.common.utils import sssdTools
 from sssd.testlib.common.libkrb5 import krb5srv
 from sssd.testlib.common.utils import SSHClient
 from sssd.testlib.common.utils import sssdTools, LdapOperations
-from constants import ds_instance_name, ds_suffix, ds_rootdn, ds_rootpw
+from constants import ds_instance_name, ds_suffix
 from sssd.testlib.common.exceptions import SSHLoginException
 from sssd.testlib.common.expect import pexpect_ssh
 
@@ -35,7 +35,6 @@ def usr_grp(multihost, user_info, type):
         try:
             ldap_inst.posix_group("ou=Groups", ds_suffix, user_info,
                                   memberUid=user_info.get('memberUid'))
-        return True
         except LdapException:
             print(f"Unable to add ldap group {user_info}")
             assert False
@@ -80,7 +79,7 @@ class Testrfc2307(object):
         domain_name = tools.get_domain_section_name()
         multihost.client[0].service_sssd('start')
         tools.clear_sssd_cache()
-        user = f'\ tuser@{domain_name}'
+        user = f'\\ tuser@{domain_name}'
         client = pexpect_ssh(multihost.client[0].sys_hostname, user,
                              'Secret123', debug=False)
         try:
diff --git a/src/tests/multihost/sssd/testlib/common/expect.py b/src/tests/multihost/sssd/testlib/common/expect.py
index 35a790e25..29febf0e4 100644
--- a/src/tests/multihost/sssd/testlib/common/expect.py
+++ b/src/tests/multihost/sssd/testlib/common/expect.py
@@ -48,7 +48,10 @@ class pexpect_ssh(object):
         self.ssh.sendline("echo $?")
         self.ssh.prompt()
         returncode = self.ssh.before
-        ret = returncode.decode('utf-8').split('\r')[1].strip('\n')
+        # I guess due to the end-of-line encodings the second array element
+        # after the split is empty and the return code is in the third element,
+        # aka [2]
+        ret = returncode.decode('utf-8').split('\r')[2].strip('\n')
         output_str = output_utf8.decode('utf-8')
         if raiseonerr:
             if (int(ret)) != 0:

Additionally there is the warning:

src/tests/multihost/alltests/test_rfc2307.py:45
  /home/sbose/wip/sssd/src/tests/multihost/alltests/test_rfc2307.py:45: PytestUnknownMarkWarning: Unknown pytest.mark.rfc2307 - is this a typo?  You can register custom marks to avoid this warning - for details, see https://docs.pytest.org/en/stable/mark.html
    @pytest.mark.rfc2307

Finally I would suggest to not only check that tuser@domain with a white-space at the beginning can be found but that tuser@domain without the while-space is not found. Additionally I would check that a different user with a "normal" name can be found and if a white-space is added at the beginning of the name it cannot be found anymore.

bye,
Sumit

madhuriupadhye · 2022-04-25T07:29:03Z

Also could you please add the link of successful job link from CI

shridhargadekar · 2022-04-29T19:15:19Z

Hi,
I've added the check scenarios for:

Existing user with a username having a space at the beginning,
a non existing user having similar username without space at beginning.
Fetching normal existing user information without adding space at beginning
Fetching normal existing user information after adding a preceding whitespace as first character

sumit-bose · 2022-05-02T06:25:05Z

src/tests/multihost/alltests/test_rfc2307.py

+            assert ret == '0'
+            client.logout()
+        user = f'foo1@{domain_name}'
+        multihost.client[0].run_command(f'id {user}', raiseonerr=False)


Hi,

I think the two lines above can be removed, the proper test for foo1@{domain_name} starts at line 96.

bye,
Sumit

sumit-bose · 2022-05-02T06:29:47Z

Hi,

thanks for the new tests, I think it would be good to mention them in the initial comment in the test as well.

I still have to change the array index in pexpect_ssh() to make the test work.

diff --git a/src/tests/multihost/sssd/testlib/common/expect.py b/src/tests/multihost/sssd/testlib/common/expect.py
index 35a790e25..c6c8fa218 100644
--- a/src/tests/multihost/sssd/testlib/common/expect.py
+++ b/src/tests/multihost/sssd/testlib/common/expect.py
@@ -48,7 +48,7 @@ class pexpect_ssh(object):
         self.ssh.sendline("echo $?")
         self.ssh.prompt()
         returncode = self.ssh.before
-        ret = returncode.decode('utf-8').split('\r')[1].strip('\n')
+        ret = returncode.decode('utf-8').split('\r')[2].strip('\n')
         output_str = output_utf8.decode('utf-8')
         if raiseonerr:
             if (int(ret)) != 0:

is the test passing for you without this change? If yes this might need some further investigation.

bye,
Sumit

sumit-bose · 2022-05-02T13:33:13Z

Hi,

thanks for the update. Finally, if @pytest.mark.rfc2307 is intended then I guess you should add rfc2307 to markers in src/tests/multihost/alltests/pytest.ini as well.

bye,
Sumit

sumit-bose

Hi,

thanks for adding the marker, ACK.

bye,
Sumit

shridhargadekar · 2022-05-03T12:52:33Z

Hi Sumit,
Modifying the a/src/tests/multihost/sssd/testlib/common/expect.py is causing failure in fetching the 'ret' (return value) of command executed with client.command()
+ ret = returncode.decode('utf-8').split('\r')[2].strip('\n')

sumit-bose · 2022-05-04T07:43:01Z

Hi,

it looks like there is an issue with a new feature of recent version of libreadline, see
e.g. pexpect/pexpect#669. The following patch works for me and should also work with older versions of libreadline which do not know about the enable-bracketed-paste variable. I also replace split('\r') with splitlines()
to let python handle the dirty details of line-breaks.

HTH

bye,
Sumit

diff --git a/src/tests/multihost/sssd/testlib/common/expect.py b/src/tests/multihost/sssd/testlib/common/expect.py
index c6c8fa218..7085f6708 100644
--- a/src/tests/multihost/sssd/testlib/common/expect.py
+++ b/src/tests/multihost/sssd/testlib/common/expect.py
@@ -42,13 +42,17 @@ class pexpect_ssh(object):
 
     def command(self, command, raiseonerr=False):
         """ Run Non interactive Commands """
+        # Disable bracketed-paste mode in libreadline,
+        # see https://github.com/pexpect/pexpect/issues/669 for details
+        self.ssh.sendline("bind 'set enable-bracketed-paste off' 2> /dev/null")
+        self.ssh.prompt()
         self.ssh.sendline(command)
         self.ssh.prompt()
         output_utf8 = self.ssh.before
         self.ssh.sendline("echo $?")
         self.ssh.prompt()
         returncode = self.ssh.before
-        ret = returncode.decode('utf-8').split('\r')[2].strip('\n')
+        ret = returncode.decode('utf-8').splitlines()[1].strip('\n')
         output_str = output_utf8.decode('utf-8')
         if raiseonerr:
             if (int(ret)) != 0:

shridhargadekar · 2022-05-04T10:39:35Z

Hi,
After adding splitlines()[1], got the code working.

sumit-bose · 2022-05-04T11:11:28Z

Hi, After adding splitlines()[1], got the code working.

Hi,

I guess you are running the test against RHEL-8. If you have the chance to run the tests with recent Fedora or RHEL-9 I would expect that you will need the set enable-bracketed-paste off part as well.

bye,
Sumit

src/tests/multihost/alltests/test_rfc2307.py

madhuriupadhye · 2022-05-05T13:26:06Z

src/tests/multihost/alltests/test_rfc2307.py

+    @pytest.mark.tier2
+    def test_0001_bz1362023(self, multihost, backupsssdconf):
+        """
+        :title: IDM-SSSD-TC: rfc2307: user with spaces at beginning


This is porting, so are we using the same title and id from the polarion?
because we do have the same test case in polarion.

I've used different ID and different title from original bash testcases.

okay, so we are keeping the bash test also? Correct me, if I am wrong.

Minor string-formatter change (added f-string)

madhuriupadhye · 2022-05-06T08:38:40Z

As of now, we keep different test case titles and IDs,
LGTM.

sumit-bose · 2022-05-06T09:42:32Z

Hi,

@madhuriupadhye, @shridhargadekar, did one of you run the test with RHEL-9? I still think some fix for bracketed-paste mode should be added.

bye,
Sumit

shridhargadekar · 2022-05-06T09:52:17Z

@sumit-bose, this patch tested against rhel8. I'm testing it against the rhel9 as well. I'll update with it's results

pbrezina · 2022-05-06T10:29:32Z

Pushed PR: #6110

master
- 0c35ed5 - Tests:port rfc2307 username begin with a space
sssd-2-7
- 21052b9 - Tests:port rfc2307 username begin with a space

shridhargadekar added Waiting for review Tests labels Apr 13, 2022

shridhargadekar force-pushed the rfc2307_porting branch from 15c6875 to 3e4ba40 Compare April 13, 2022 19:46

andreboscatto assigned sumit-bose Apr 14, 2022

sumit-bose reviewed Apr 19, 2022

View reviewed changes

shridhargadekar force-pushed the rfc2307_porting branch 2 times, most recently from f2f618c to 25ce325 Compare April 19, 2022 17:45

shridhargadekar requested a review from madhuriupadhye April 20, 2022 12:28

madhuriupadhye added Changes requested and removed Waiting for review labels Apr 27, 2022

shridhargadekar force-pushed the rfc2307_porting branch from 25ce325 to 5d96b62 Compare April 29, 2022 19:07

shridhargadekar added Waiting for review and removed Changes requested labels Apr 29, 2022

shridhargadekar force-pushed the rfc2307_porting branch from 5d96b62 to 125ea7f Compare April 29, 2022 19:17

sumit-bose reviewed May 2, 2022

View reviewed changes

shridhargadekar force-pushed the rfc2307_porting branch 2 times, most recently from ecbf99e to 24da9fe Compare May 2, 2022 11:45

andreboscatto assigned madhuriupadhye May 2, 2022

shridhargadekar force-pushed the rfc2307_porting branch from 24da9fe to b4b371c Compare May 2, 2022 14:10

sumit-bose approved these changes May 3, 2022

View reviewed changes

shridhargadekar force-pushed the rfc2307_porting branch from b4b371c to bda0537 Compare May 4, 2022 10:38

madhuriupadhye requested changes May 5, 2022

View reviewed changes

src/tests/multihost/alltests/test_rfc2307.py Outdated Show resolved Hide resolved

src/tests/multihost/alltests/test_rfc2307.py Outdated Show resolved Hide resolved

src/tests/multihost/alltests/test_rfc2307.py Outdated Show resolved Hide resolved

shridhargadekar force-pushed the rfc2307_porting branch 2 times, most recently from 3a37c75 to edd7335 Compare May 5, 2022 13:19

madhuriupadhye reviewed May 5, 2022

View reviewed changes

Tests:port rfc2307 username begin with a space

3041724

Minor string-formatter change (added f-string)

shridhargadekar force-pushed the rfc2307_porting branch from edd7335 to 3041724 Compare May 5, 2022 13:33

sgoveas added the backport-to-stable Targets also latest stable branch label May 5, 2022

madhuriupadhye approved these changes May 6, 2022

View reviewed changes

madhuriupadhye added Accepted and removed Waiting for review labels May 6, 2022

pbrezina added the Ready to push Ready to push label May 6, 2022

pbrezina added Pushed and removed Accepted Ready to push Ready to push labels May 6, 2022

pbrezina closed this May 6, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tests:port rfc2307 username begin with a space #6110

Tests:port rfc2307 username begin with a space #6110

shridhargadekar commented Apr 13, 2022

sumit-bose Apr 19, 2022

shridhargadekar Apr 19, 2022

sumit-bose commented Apr 22, 2022

madhuriupadhye commented Apr 25, 2022

shridhargadekar commented Apr 29, 2022

sumit-bose May 2, 2022

sumit-bose commented May 2, 2022

sumit-bose commented May 2, 2022

sumit-bose left a comment

shridhargadekar commented May 3, 2022

sumit-bose commented May 4, 2022

shridhargadekar commented May 4, 2022

sumit-bose commented May 4, 2022

madhuriupadhye May 5, 2022

shridhargadekar May 5, 2022

madhuriupadhye May 5, 2022 •

edited

Loading

madhuriupadhye commented May 6, 2022 •

edited

Loading

sumit-bose commented May 6, 2022

shridhargadekar commented May 6, 2022

pbrezina commented May 6, 2022

Tests:port rfc2307 username begin with a space #6110

Tests:port rfc2307 username begin with a space #6110

Conversation

shridhargadekar commented Apr 13, 2022

sumit-bose Apr 19, 2022

Choose a reason for hiding this comment

shridhargadekar Apr 19, 2022

Choose a reason for hiding this comment

sumit-bose commented Apr 22, 2022

madhuriupadhye commented Apr 25, 2022

shridhargadekar commented Apr 29, 2022

sumit-bose May 2, 2022

Choose a reason for hiding this comment

sumit-bose commented May 2, 2022

sumit-bose commented May 2, 2022

sumit-bose left a comment

Choose a reason for hiding this comment

shridhargadekar commented May 3, 2022

sumit-bose commented May 4, 2022

shridhargadekar commented May 4, 2022

sumit-bose commented May 4, 2022

madhuriupadhye May 5, 2022

Choose a reason for hiding this comment

shridhargadekar May 5, 2022

Choose a reason for hiding this comment

madhuriupadhye May 5, 2022 • edited Loading

Choose a reason for hiding this comment

madhuriupadhye commented May 6, 2022 • edited Loading

sumit-bose commented May 6, 2022

shridhargadekar commented May 6, 2022

pbrezina commented May 6, 2022

madhuriupadhye May 5, 2022 •

edited

Loading

madhuriupadhye commented May 6, 2022 •

edited

Loading