New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pytest: Add test case for Expired sudo rule #680

Open
wants to merge 6 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@mrniranjan

mrniranjan commented Oct 16, 2018

No description provided.

restore_sssd = 'cp -f /etc/sssd/sssd.conf.orig /etc/sssd/sssd.conf'
session_multihost.master[0].run_command(restore_sssd)
session_multihost.master[0].service_sssd('restart')
time.sleep(5)

This comment has been minimized.

@jhrozek

jhrozek Oct 19, 2018

Contributor

Why do we need the sleep here? Doesn't systemctl block until the service starts?

assert ret == 'Success'
(ret, _) = ldap_inst.del_dn(sudo_ou)
assert ret == 'Success'
time.sleep(5)

This comment has been minimized.

@jhrozek

jhrozek Oct 19, 2018

Contributor

And why do we need the sleep after deleting the sudo rules?

@@ -220,6 +220,60 @@ def restore_sssd():
request.addfinalizer(restore_sssd)
@pytest.fixture
def set_entry_cache_timeout(session_multihost, request):
""" Set case_sensitive to false in sssd domain section """

This comment has been minimized.

@jhrozek

jhrozek Oct 19, 2018

Contributor

This comment seems to be copy-pasted from another test

""" Verify refreshing expired sudo rules doesn't crash sssd_sudo """
# pylint: disable=unused-argument
_pytest_fixtures = [enable_sss_sudo_nsswitch, generic_sudorule,
set_entry_cache_timeout]

This comment has been minimized.

@jhrozek

jhrozek Oct 19, 2018

Contributor

This is just a question for my own education, but what is the _pytest_fixtures variable good for? Is it for some linters?

This comment has been minimized.

@mrniranjan

mrniranjan Oct 19, 2018

pylint complains about fixtures , it treats these as function variables which are not used.

@jhrozek

Mostly looks good, I just wonder if we can remove the sleep commands during the fixtures

@jhrozek jhrozek self-assigned this Oct 19, 2018

@jhrozek

This comment has been minimized.

Contributor

jhrozek commented Oct 19, 2018

btw did you manage to reproduce the bug with this test and the buggy sssd version? I reverted the patches that fix the sssd bug but the tests still passed..

@mrniranjan

This comment has been minimized.

mrniranjan commented Oct 19, 2018

On the buggy version, i could see the sssd_sudo crashes.

Niranjan M.R added some commits Oct 16, 2018

Niranjan M.R
pytest/sudo: Modify fixture to restore sssd.conf
Modify set_case_sensitive_false fixture to restore sssd.conf
back to the original sssd.conf after running test_case_senitivity
test case

Signed-off-by: Niranjan M.R <mrniranjan@redhat.com>
Niranjan M.R
pytest/sudo: Rename create_sudorule to case_sensitive_sudorule
Add del_sudo_rule function to delete the sudo rules
after test_sensitivity completes.

Signed-off-by: Niranjan M.R <mrniranjan@redhat.com>
Niranjan M.R
pytest/sudo: call case_sensitive_sudorule fixture instead of create_s…
…udorule

Signed-off-by: Niranjan M.R <mrniranjan@redhat.com>
Niranjan M.R
pytest/sudo: Add 2 fixtures set_entry_cache_sudo_timeout and generic_…
…sudorule

set_entry_cache_sudo_timeout: this fixture adds entry_cache_sudo_timeout
to domain sections of sssd.conf
generic_sudorule: This is a generic sudo rule addding command /usr/bin/less
to be executed by posix user

generic_sudorule: Adds a generic sudo rule to access /usr/bin/less

Signed-off-by: Niranjan M.R <mrniranjan@redhat.com>
Niranjan M.R
pytest/sudo: Add Testcase: sssd crashes when refreshing expired sudo …
…rules.

Signed-off-by: Niranjan M.R <mrniranjan@redhat.com>
Niranjan M.R
pytest: use ConfigParser() instead of SafeConfigParser()
fix the warnings of SafeConfigParser being deprectated

Signed-off-by: Niranjan M.R <mrniranjan@redhat.com>
@mrniranjan

This comment has been minimized.

mrniranjan commented Oct 22, 2018

I have fixed the issues raised in the review comments, but for some reason github is not updating the PR with latest commits.

1 similar comment
@mrniranjan

This comment has been minimized.

mrniranjan commented Oct 22, 2018

I have fixed the issues raised in the review comments, but for some reason github is not updating the PR with latest commits.

@jhrozek

This comment has been minimized.

Contributor

jhrozek commented Oct 22, 2018

@mrniranjan

This comment has been minimized.

mrniranjan commented Nov 14, 2018

@jhrozek Can you review this ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment