Tests: Add support for IPA IPA Trust #7517

justin-stephenson · 2024-08-01T13:35:09Z

Add tests for new IPA Trust topologies in SSSD test framework.

Linked PRs:

SSSD/sssd-ci-containers#106
SSSD/sssd-test-framework#119

src/tests/system/tests/test_ipa_trusts.py

pbrezina · 2024-08-14T11:01:40Z

Hi, I added blocked label since I think we should not merge it until new IPA is released. We should also add conditional skip based on feature detection.

justin-stephenson · 2024-08-14T20:09:30Z

Updated mhc.yaml based on SSSD/sssd-test-framework#119 (comment)

danlavu

Passing,

============================= test session starts ==============================
collecting ... 

Selected tests will use the following hosts:
  client: client.test
  ad: dc.ad.test
  samba: dc.samba.test
  ipa: master.ipa.test
  ipa: master2.ipa2.test

collected 586 items / 580 deselected / 6 selected

tests/test_ipa_trusts.py::test_ipa_trusts__lookup_group_without_sid (ipa-trust-ad) 
tests/test_ipa_trusts.py::test_ipa_trusts__ipa_master_lookup_trusted_user (ipa-trust-ad) 
tests/test_ipa_trusts.py::test_ipa_trusts__ipa_master_lookup_trusted_user (ipa-trust-ipa) 
tests/test_ipa_trusts.py::test_ipa_trusts__lookup_trusted_user (ipa-trust-ipa) 
tests/test_ipa_trusts.py::test_ipa_trusts__lookup_group_without_sid (ipa-trust-samba) 
tests/test_ipa_trusts.py::test_ipa_trusts__ipa_master_lookup_trusted_user (ipa-trust-samba) PASSED [ 16%]PASSED [ 33%]PASSED [ 50%]PASSED [ 66%]PASSED [ 83%]PASSED [100%]

================ 6 passed, 580 deselected in 574.83s (0:09:34) =================

Process finished with exit code 0

Some minor changes but it looks good, thank you Justin

danlavu · 2024-12-09T15:07:49Z

src/tests/system/tests/test_ipa_trusts.py

+    :customerscenario: True
+    """
+    client.sssd.clear(db=True, memcache=True, logs=True)
+    client.sssd.restart()


We have updated the method, you can merge 105-106 with client.sssd.restart(clean=True) , applies to line 79-80 as well.

danlavu · 2024-12-09T15:08:53Z

src/tests/system/tests/test_ipa_trusts.py

+    username = trusted.admin_fqn
+
+    id_user = client.tools.id(username)
+    assert id_user is not None


Can you provide an error message please, assert id_user is not None, "User is not found!"

danlavu · 2024-12-09T15:09:19Z

src/tests/system/tests/test_ipa_trusts.py

+
+    id_user = client.tools.id(username)
+    assert id_user is not None
+    assert id_user.user.name == username


Same, Username does not match!

All the assertions, line, 86,87,119 and 120

jakub-vavra-cz · 2024-12-18T11:42:39Z

src/tests/system/mhc.yaml

+    config:
+      client:
+        ipa_domain: ipa2.test
+        krb5_keytab: /enrollment/ipa2.test.keytab


This is most probably wrong as there is no code to enroll client to ipa2 in SSSD/sssd-ci-containers#106

I don't know if we plan to add any enrollment of systems into IPA2 domain. If not, can this whole part be removed?

config: client: ipa_domain: ipa2.test krb5_keytab: /enrollment/ipa2.test.keytab ldap_krb5_keytab: /enrollment/ipa2.test.keytab

No, I don't think we need to enroll tests into ipa2.

jakub-vavra-cz · 2024-12-18T11:45:09Z

src/tests/system/tests/test_ipa_trusts.py

@@ -60,3 +61,58 @@ def test_ipa_trusts__lookup_group_without_sid(ipa: IPA, trusted: GenericADProvid
    status = ipa.sssctl.domain_status(trusted.domain, online=True)
    assert "online status: offline" not in status.stdout.lower(), "AD domain went offline!"
    assert "online status: online" in status.stdout.lower(), "AD domain was not online!"
+
+
+@pytest.mark.importance("low")


We should have at least on test that will cover ipa-ipa trust feature that is high or critical to land in the gating.

I would like to ask @danlavu to take care of this.

I'm writing a test for ipa - adtrust, we should be able to parameterize it. https://github.com/SSSD/sssd/pull/7779/files , basic auth.

jakub-vavra-cz · 2024-12-18T11:47:05Z

src/tests/system/tests/test_ipa_trusts.py

+    """
+    :title: Basic IPA-IPA Trust lookup on IPA server
+    :setup:
+        1. Restart SSSD and clear cache on IPA server


These tests should have requirement matching the RHEL-XXX jira name to link the test to requirement that will be created in polarion.

Added @pytest.mark.ticket(jira="RHEL-14752") to the 2 newly added tests.

jakub-vavra-cz · 2024-12-18T11:47:54Z

src/tests/system/tests/test_ipa_trusts.py

+
+
+@pytest.mark.importance("low")
+@pytest.mark.topology(KnownTopologyGroup.AnyIPATrust)


There should be a pytest.ticket linking to relevant jiras.

Added @pytest.mark.ticket(jira="RHEL-14752") to the 2 newly added tests.

jakub-vavra-cz · 2024-12-18T11:48:36Z

src/tests/system/tests/test_ipa_trusts.py

+    assert id_user.user.name == username, "Username does not match"
+
+
+@pytest.mark.importance("low")


This looks like importance medium to me.

jakub-vavra-cz · 2024-12-18T11:50:59Z

src/tests/system/tests/test_ipa_trusts.py

+    # Resolve group
+    groupname = trusted.fqn("admins")
+
+    getent_group = client.tools.getent.group(groupname)


I would move this before all of the asserts so we can see if group resolution works when something fails for user resolution.

jakub-vavra-cz · 2024-12-18T11:53:48Z

src/tests/system/tests/test_ipa_trusts.py

+
+    getent_group = client.tools.getent.group(groupname)
+    assert getent_group is not None
+    assert getent_group.name == groupname


I am missing a testcase that will authenticate (su, ssh) as a normal user from the trusted domain.
(possibly parametrized for short username and fqn.)

I would like to ask @danlavu to take care of this.

jakub-vavra-cz · 2024-12-18T11:54:18Z

src/tests/system/tests/test_ipa_trusts.py

+
+    id_user = client.tools.id(username)
+    assert id_user is not None, "Trusted admin user not found"
+    assert id_user.user.name == username, "Username does not match"


We should check group membership for the user.

jakub-vavra-cz

See inline.

This was referenced Aug 1, 2024

Add IPA IPA Trust Topology Controller SSSD/sssd-test-framework#119

Open

Support IPA IPA Trust with additional IPA server SSSD/sssd-ci-containers#106

Open

justin-stephenson force-pushed the ipa_ipa_trust_tests branch from 596880a to fd8ef2f Compare August 1, 2024 13:42

justin-stephenson marked this pull request as ready for review August 1, 2024 13:43

justin-stephenson force-pushed the ipa_ipa_trust_tests branch 2 times, most recently from 8abd754 to 644678a Compare August 2, 2024 20:02

danlavu reviewed Aug 6, 2024

View reviewed changes

src/tests/system/tests/test_ipa_trusts.py Outdated Show resolved Hide resolved

src/tests/system/tests/test_ipa_trusts.py Outdated Show resolved Hide resolved

andreboscatto assigned danlavu and pbrezina Aug 6, 2024

andreboscatto requested review from pbrezina and jakub-vavra-cz August 6, 2024 12:35

andreboscatto assigned jakub-vavra-cz Aug 6, 2024

justin-stephenson force-pushed the ipa_ipa_trust_tests branch 4 times, most recently from d92d5a9 to c2ef19c Compare August 8, 2024 15:41

pbrezina added the Blocked label Aug 14, 2024

justin-stephenson force-pushed the ipa_ipa_trust_tests branch from c2ef19c to 38cf052 Compare August 14, 2024 20:08

justin-stephenson force-pushed the ipa_ipa_trust_tests branch from 38cf052 to f32ac30 Compare August 15, 2024 17:54

justin-stephenson force-pushed the ipa_ipa_trust_tests branch from f32ac30 to 7a3115d Compare September 3, 2024 18:51

alexey-tikhonov added the Tests label Sep 11, 2024

justin-stephenson force-pushed the ipa_ipa_trust_tests branch from 7a3115d to d9f952d Compare September 20, 2024 15:44

danlavu requested changes Dec 9, 2024

View reviewed changes

justin-stephenson force-pushed the ipa_ipa_trust_tests branch from d9f952d to 8d0e39f Compare December 9, 2024 20:13

jakub-vavra-cz reviewed Dec 18, 2024

View reviewed changes

jakub-vavra-cz requested changes Dec 18, 2024

View reviewed changes

justin-stephenson force-pushed the ipa_ipa_trust_tests branch from 8d0e39f to af145e8 Compare December 18, 2024 15:16

Tests: Add support for IPA IPA Trust

a55729e

justin-stephenson force-pushed the ipa_ipa_trust_tests branch from af145e8 to a55729e Compare December 18, 2024 15:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tests: Add support for IPA IPA Trust #7517

Tests: Add support for IPA IPA Trust #7517

justin-stephenson commented Aug 1, 2024

pbrezina commented Aug 14, 2024

justin-stephenson commented Aug 14, 2024

danlavu left a comment

danlavu Dec 9, 2024

justin-stephenson Dec 9, 2024

danlavu Dec 9, 2024

justin-stephenson Dec 9, 2024

danlavu Dec 9, 2024

justin-stephenson Dec 9, 2024

jakub-vavra-cz Dec 18, 2024

justin-stephenson Dec 18, 2024

danlavu Dec 20, 2024

jakub-vavra-cz Dec 18, 2024

justin-stephenson Dec 18, 2024

danlavu Dec 20, 2024

jakub-vavra-cz Dec 18, 2024

justin-stephenson Dec 18, 2024

jakub-vavra-cz Dec 18, 2024

justin-stephenson Dec 18, 2024

jakub-vavra-cz Dec 18, 2024

justin-stephenson Dec 18, 2024

jakub-vavra-cz Dec 18, 2024

justin-stephenson Dec 18, 2024

jakub-vavra-cz Dec 18, 2024 •

edited

Loading

justin-stephenson Dec 18, 2024

jakub-vavra-cz Dec 18, 2024

justin-stephenson Dec 18, 2024

jakub-vavra-cz left a comment



		@pytest.mark.importance("low")
		@pytest.mark.topology(KnownTopologyGroup.AnyIPATrust)

		assert id_user.user.name == username, "Username does not match"


		@pytest.mark.importance("low")

Tests: Add support for IPA IPA Trust #7517

Are you sure you want to change the base?

Tests: Add support for IPA IPA Trust #7517

Conversation

justin-stephenson commented Aug 1, 2024

pbrezina commented Aug 14, 2024

justin-stephenson commented Aug 14, 2024

danlavu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakub-vavra-cz Dec 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakub-vavra-cz left a comment

Choose a reason for hiding this comment

jakub-vavra-cz Dec 18, 2024 •

edited

Loading