Skip to content

[autobackport: sssd-2-13] Pkcs11 soft ocsp tests#8757

Open
sssd-bot wants to merge 1 commit into
SSSD:sssd-2-13from
sssd-bot:SSSD-sssd-backport-pr8557-to-sssd-2-13
Open

[autobackport: sssd-2-13] Pkcs11 soft ocsp tests#8757
sssd-bot wants to merge 1 commit into
SSSD:sssd-2-13from
sssd-bot:SSSD-sssd-backport-pr8557-to-sssd-2-13

Conversation

@sssd-bot

@sssd-bot sssd-bot commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

This is an automatic backport of PR#8557 Pkcs11 soft ocsp tests to branch sssd-2-13, created by @krishnavema.

Please make sure this backport is correct.

Note

The commits were cherry-picked without conflicts.

You can push changes to this pull request

git remote add sssd-bot git@github.com:sssd-bot/sssd.git
git fetch sssd-bot refs/heads/SSSD-sssd-backport-pr8557-to-sssd-2-13
git checkout SSSD-sssd-backport-pr8557-to-sssd-2-13
git push sssd-bot SSSD-sssd-backport-pr8557-to-sssd-2-13 --force

Original commits
47f67f0 - Add default timeout handling for soft_ocsp and regression tests for smart card authentication (resolves: RHEL-5043)

Backported commits

  • cd809c6 - Add default timeout handling for soft_ocsp and regression tests for smart card authentication (resolves: RHEL-5043)

Original Pull Request Body

Add optional label parameter for pkcs11 support

…mart card authentication (resolves: RHEL-5043)

Reviewed-by: Scott Poore <spoore@redhat.com>
Reviewed-by: Sumit Bose <sbose@redhat.com>
Reviewed-by: Tomáš Halman <thalman@redhat.com>
(cherry picked from commit 47f67f0)

@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 new system tests to verify SSSD smart card authentication with soft OCSP under different network conditions, along with helper functions to configure SSSD and redirect the OCSP responder. The review feedback identifies a critical test pollution issue where modifications to /etc/hosts persist across tests, and suggests using a pytest fixture to back up and restore the file for each test.

Comment on lines +76 to +80
def _redirect_ocsp_responder(client: Client, ipa: IPA, target_ip: str) -> None:
"""Point the IPA OCSP responder hostname to *target_ip* via ``/etc/hosts``."""
ipa_ca_hostname = f"ipa-ca.{ipa.domain}"
client.fs.append("/etc/hosts", f"\n{target_ip} {ipa_ca_hostname}\n")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

Modifying /etc/hosts by appending to it in _redirect_ocsp_responder persists across tests because the client VM/container is not recreated between individual test cases. This causes test pollution: for example, test_smartcard__soft_ocsp_with_reachable_responder (which runs after test_smartcard__soft_ocsp_with_unreachable_responder) will run with an unreachable responder, defeating the purpose of the test.

We should add an autouse pytest fixture to automatically back up and restore /etc/hosts for each test.

@pytest.fixture(autouse=True)
def restore_hosts(client: Client):
    """Backup and restore /etc/hosts to prevent test pollution."""
    client.host.conn.run("cp /etc/hosts /etc/hosts.bak")
    yield
    client.host.conn.run("mv /etc/hosts.bak /etc/hosts")


def _redirect_ocsp_responder(client: Client, ipa: IPA, target_ip: str) -> None:
    """Point the IPA OCSP responder hostname to *target_ip* via ``/etc/hosts``."""
    ipa_ca_hostname = f"ipa-ca.{ipa.domain}"
    client.fs.append("/etc/hosts", f"\n{target_ip}  {ipa_ca_hostname}\n")

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.

Isn't /etc/hosts automatically backed up and restored as a part of the topology controllers setup/teardown?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants