Skip to content

tests: Clarify approx match filter#8714

Merged
thalman merged 1 commit into
SSSD:masterfrom
justin-stephenson:approx_match_comment
May 25, 2026
Merged

tests: Clarify approx match filter#8714
thalman merged 1 commit into
SSSD:masterfrom
justin-stephenson:approx_match_comment

Conversation

@justin-stephenson

Copy link
Copy Markdown
Contributor

Debugging this test for an unrelated issue (new failover) lead to several questions about 'approximate match' filter, adding some comments to make it more clear after some investigation.

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

Copy link
Copy Markdown

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 adds explanatory comments to the LDAP access control system tests, detailing how different providers (AD, Samba, and 389ds) handle approximate match filters. A review comment identifies a potential logical inconsistency in the test logic: if Samba rejects the approximate match filter with an error as described in the new comments, the assertion that 'user1' can successfully log in may be incorrect, as the entire access check search would likely fail.

Comment thread src/tests/system/tests/test_access_control_ldap_filter.py Outdated

@thalman thalman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The comment is correct, thanks for update

@sssd-bot

Copy link
Copy Markdown
Contributor

The pull request was accepted by @thalman with the following PR CI status:


🟢 CodeQL (success)
🔴 rpm-build:centos-stream-10-x86_64:upstream (failure)
🔴 rpm-build:fedora-42-x86_64:upstream (failure)
🔴 rpm-build:fedora-43-x86_64:upstream (failure)
🔴 rpm-build:fedora-44-x86_64:upstream (failure)
🔴 rpm-build:fedora-rawhide-x86_64:upstream (failure)
🟢 Analyze (target) / cppcheck (success)
🟢 ci / intgcheck (centos-10) (success)
🟢 ci / intgcheck (fedora-42) (success)
🟢 ci / intgcheck (fedora-43) (success)
🟢 ci / intgcheck (fedora-44) (success)
🟢 ci / intgcheck (fedora-45) (success)
🟢 ci / prepare (success)
🟢 ci / system (centos-10) (success)
🟢 ci / system (fedora-42) (success)
🟢 ci / system (fedora-43) (success)
🟢 ci / system (fedora-44) (success)
🟢 ci / system (fedora-45) (success)
➖ 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.

@sssd-bot
sssd-bot force-pushed the approx_match_comment branch from a11db77 to 1af5130 Compare May 25, 2026 12:55
@thalman
thalman merged commit 21674dd into SSSD:master May 25, 2026
12 of 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-9 backport-to-sssd-2-13 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.

4 participants