Skip to content

Conversation

@aborah-sudo
Copy link
Contributor

Tests using nslcd fail under SELinux enforcing due to missing policies for test-only nss-pam-ldapd configuration. Add context manager to temporarily set permissive mode for affected tests.

@aborah-sudo aborah-sudo added Waiting for review Tests Trivial A single reviewer is sufficient to review the Pull Request labels Jan 12, 2026
Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a helper to temporarily disable SELinux for tests that require it. The implementation is sound, but can be improved for better maintainability and integration with pytest. I've suggested converting the new context manager into a pytest fixture. This will reduce code duplication across the test suite and make the tests cleaner. This also makes the newly added contextlib import unnecessary.

Copy link
Contributor

@jakub-vavra-cz jakub-vavra-cz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks nice and clean.
I suggest adding this to the framework with "host" instead of "client" so it can be more easily reused elsewhere.

@aborah-sudo
Copy link
Contributor Author

The code looks nice and clean. I suggest adding this to the framework with "host" instead of "client" so it can be more easily reused elsewhere.

SSSD/sssd-test-framework#227

@aborah-sudo
Copy link
Contributor Author

I will create a new ticket to rewrite the tests to not use nss-pam-ldapd but some other nss and pam modules.

@aborah-sudo aborah-sudo reopened this Jan 13, 2026
Copy link
Contributor

@jakub-vavra-cz jakub-vavra-cz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Copy link
Contributor

@jakub-vavra-cz jakub-vavra-cz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. The framework change SSSD/sssd-test-framework#227 needed is already merged just this ci did not register it.

Tests using nslcd fail under SELinux enforcing due to missing
policies for test-only nss-pam-ldapd configuration. Add context
manager to temporarily set permissive mode for affected tests.

Reviewed-by: Jakub Vávra <jvavra@redhat.com>
@sssd-bot
Copy link
Contributor

The pull request was accepted by @jakub-vavra-cz with the following PR CI status:


🟢 CodeFactor (success)
🟢 CodeQL (success)
NEUTRAL osh-diff-scan:fedora-rawhide-x86_64:upstream (neutral)
🟢 rpm-build:centos-stream-10-x86_64:upstream (success)
🟢 rpm-build:fedora-42-x86_64:upstream (success)
🟢 rpm-build:fedora-43-x86_64:upstream (success)
🟢 rpm-build:fedora-rawhide-x86_64:upstream (success)
🟢 Analyze (target) / cppcheck (success)
➖ ci / intgcheck (skipped)
🟢 ci / intgcheck (centos-10) (success)
🟢 ci / intgcheck (fedora-42) (success)
🟢 ci / intgcheck (fedora-43) (success)
🟢 ci / intgcheck (fedora-44) (success)
🔴 ci / prepare (failure)
➖ ci / system (skipped)
🟢 ci / system (centos-10) (success)
🟢 ci / system (fedora-42) (success)
🟢 ci / system (fedora-43) (success)
🔴 ci / system (fedora-44) (failure)
➖ Coverity scan / coverity (skipped)
🟢 Static code analysis / codeql (success)
🟢 Static code analysis / pre-commit (success)
🟢 Static code analysis / python-system-tests (success)


There are unsuccessful or unfinished checks. Make sure that the failures are not related to this pull request before merging.

@jakub-vavra-cz jakub-vavra-cz merged commit 8b0071c into SSSD:master Jan 16, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Accepted backport-to-sssd-2-10 backport-to-sssd-2-11 Ready to push Ready to push Tests Trivial A single reviewer is sufficient to review the Pull Request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants