Skip to content
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

CONFDB: Files domain if activated without .conf #825

Closed
wants to merge 2 commits into from

Conversation

thalman
Copy link
Contributor

@thalman thalman commented Jun 3, 2019

Implicit files domain gets activated when no sssd.conf present
and sssd is started. This does not respect --disable-files-domain
configure option

Resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=1713352

@thalman thalman requested review from jhrozek and mzidek-gh June 3, 2019 12:56
@mzidek-gh mzidek-gh self-assigned this Jun 27, 2019
@alexey-tikhonov alexey-tikhonov self-assigned this Jul 3, 2019
@alexey-tikhonov
Copy link
Member

retest this please

@alexey-tikhonov
Copy link
Member

centos-ci fails constantly with:

make-intgcheck:         failure  00:21:03 ci-build-debug/ci-make-intgcheck.log

Unfortunately there is no way to retrieve log.

@thalman, do you have an idea what is the cause?

@thalman
Copy link
Contributor Author

thalman commented Jul 4, 2019

Thanks @alexey-tikhonov for pointing this out. I managed to get this error locally.
I will investigate it more but I think that it is connected with this change and test
needs to be updated.

Here is the error from log:

==================================== ERRORS ====================================
_____________________ ERROR at setup of test_no_sssd_conf ______________________
Traceback (most recent call last):
  File "/home/thalman/sssd/src/tests/intg/test_files_provider.py", line 293, in no_sssd_conf
    create_sssd_fixture(request)
  File "/home/thalman/sssd/src/tests/intg/test_files_provider.py", line 128, in create_sssd_fixture
    start_sssd()
  File "/home/thalman/sssd/src/tests/intg/test_files_provider.py", line 98, in start_sssd
    raise Exception("sssd start failed")
Exception: sssd start failed
=================================== FAILURES ===================================

@alexey-tikhonov
Copy link
Member

Well, this PR is quite controversial in the context of #255

I hope to recv some comments in the BZ...

@alexey-tikhonov
Copy link
Member

@thalman, please, look comments inline (especially second comment)

Implicit files domain gets activated when no sssd.conf present
and sssd is started. This does not respect --disable-files-domain
configure option

Resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=1713352
Some tests expect that SSSD is compiled with --enable-files-domain
option (test_no_sssd_conf). But having this enabled by default
breaks some other tests.

This patch adds --enable-files-domain to test build and explicitly
disables the domain in configuration of some tests (ldap, enumeration).

Resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=1713352
@alexey-tikhonov
Copy link
Member

In regards of test adaptation: why is it required to turn enable_files_domain off in config of those two specific tests - test_enumeration.py and test_ldap.py?

I worry that inclusion of --enable-files-domain in intgcheck build affects all tests with sssd.conf available...

@thalman
Copy link
Contributor Author

thalman commented Jul 18, 2019

In regards of test adaptation: why is it required to turn enable_files_domain off in config of those two specific tests - test_enumeration.py and test_ldap.py?

Those tests expect particular response from provider but incidentally response is extended of
files provider. For example

Traceback (most recent call last):
  File "/home/thalman/sssd/src/tests/intg/test_enumeration.py", line 306, in test_sanity_rfc2307
    pwd.getpwuid(1)
  File "/usr/lib/python2.7/site-packages/_pytest/python_api.py", line 627, in __exit__
    fail(self.message)
  File "/usr/lib/python2.7/site-packages/_pytest/outcomes.py", line 92, in fail
    raise Failed(msg=msg, pytrace=pytrace)
Failed: DID NOT RAISE <type 'exceptions.KeyError'>

expected result here is that id 1 does not exist, but it does in /etc/passwd
similar problems are in other failing tests

I worry that inclusion of --enable-files-domain in intgcheck build affects all tests with sssd.conf
available...

Well there are tests (I am aware of test_files_provider.py/test_no_sssd_conf) that expect
SSSD compiled with --enable-files-domain.

Having now --enable-files-domain in fedora/rhel8 I would like to see this option used when testing.

Tests that relay on single provider source should be fixed so they explicitly have enable_files_domain = false in produced sssd.conf

@alexey-tikhonov
Copy link
Member

Ok, ack.

@jhrozek
Copy link
Contributor

jhrozek commented Jul 22, 2019

@jhrozek
Copy link
Contributor

jhrozek commented Jul 22, 2019

Looks like the patches don't apply cleanly atop sssd-1-16. @thalman would you like to open a separate backport PR?

@jhrozek
Copy link
Contributor

jhrozek commented Jul 22, 2019

Ah, sorry, there is already PR#824. So I can close this one.

@jhrozek jhrozek closed this Jul 22, 2019
@jhrozek jhrozek added the Pushed label Jul 22, 2019
@thalman thalman deleted the files-domain branch November 10, 2020 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants