Don't rely on order in trust agent/controller role check

[rcritten]

The code expected that the local server would always be the first one returned. Instead loop through the returned list to find the current server and set the state based on that.

Fixes: #356

Summary by Sourcery

Loop through server-role entries to detect trust agent and controller roles by matching the local host instead of assuming fixed positions in the list.

Bug Fixes:

• Avoid relying on list order when checking trust agent and controller roles by matching the current server and status.

Enhancements:

• Introduce has_role helper in IPARegistry to abstract role detection logic.
• Use has_role to set trust_agent and trust_controller flags dynamically.

Tests:

• Add TestHasRole with scenarios covering enabled/absent statuses in different list orders.

[sourcery-ai]

Hey @rcritten - I've reviewed your changes - here's some feedback:

• Add a test case covering the trust_controller scenario so you validate has_role works correctly for both agent and controller roles.
• Consider renaming has_role to something like is_current_host_enabled_role to make its intent clearer and easier to maintain.

Prompt for AI Agents

Please address the comments from this code review:
## Overall Comments
- Add a test case covering the trust_controller scenario so you validate has_role works correctly for both agent and controller roles.
- Consider renaming has_role to something like is_current_host_enabled_role to make its intent clearer and easier to maintain.

## Individual Comments

### Comment 1
<location> `tests/test_ipa_trust.py:1383` </location>
<code_context>
+
+        assert reg.has_role(roles) is True
+
+    def test_no_role(self):
+        self.results = Results()
+        reg = IPARegistry()
+
+        roles = [
+            {
+                "role_servrole": "AD trust agent",
+                "server_server": "server.ipa.example",
+                "status": "absent",
+            },
+            {
+                "role_servrole": "AD trust agent",
+                "server_server": "replica.ipa.example",
+                "status": "enabled",
+            },
+        ]
+
+        assert reg.has_role(roles) is False
</code_context>

<issue_to_address>
Consider adding a test for an empty roles list.

Adding a test with an empty roles list will verify that has_role correctly returns False and handles this case without errors.
</issue_to_address>

### Comment 2
<location> `tests/test_ipa_trust.py:1345` </location>
<code_context>
+      between an agent and a trust in the way they are stored in
+      a server role.
+    """
+    def test_role_last(self):
+        self.results = Results()
+        reg = IPARegistry()
+
+        roles = [
+            {
+                "role_servrole": "AD trust agent",
+                "server_server": "replica.ipa.example",
+                "status": "absent",
+            },
+            {
+                "role_servrole": "AD trust agent",
+                "server_server": "server.ipa.example",
+                "status": "enabled",
+            },
+        ]
+
+        assert reg.has_role(roles) is True
+
+    def test_role_first(self):
</code_context>

<issue_to_address>
Tests assume api.env.host is 'server.ipa.example', which may not always be true.

Mock or patch api.env.host in the test setup to avoid environment-dependent failures and ensure consistent results.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[flo-renaud]

@rcritten
thanks for the PR, works for me. Tested on an installation with master = controller, replica = agent:

• with the current version of ipa-healthcheck:

# ipa-healthcheck --source ipahealthcheck.ipa.trust
[
  {
    "source": "ipahealthcheck.ipa.trust",
    "check": "IPATrustControllerPrincipalCheck",
    "result": "ERROR",
    "uuid": "ba7a9972-2231-4c10-b49e-5a72234b81aa",
    "when": "20250620125938Z",
    "duration": "0.000814",
    "kw": {
      "key": "krbprincipalname=cifs/replica.testrelm.test@TESTRELM.TEST,cn=services,cn=accounts,dc=testrelm,dc=test",
      "error": "no such entry",
      "msg": "Error retrieving ldap entry {key}: {error}"
    }
  },
  {
    "source": "ipahealthcheck.ipa.trust",
    "check": "IPATrustControllerServiceCheck",
    "result": "ERROR",
    "uuid": "66d376ba-6c91-416d-b494-fc71e23a6541",
    "when": "20250620125938Z",
    "duration": "0.000064",
    "kw": {
      "key": "cn=ADTRUST,cn=replica.testrelm.test,cn=masters,cn=ipa,cn=etc,dc=testrelm,dc=test",
      "error": "no such entry",
      "msg": "Error retrieving ldap entry {key}: {error}"
    }
  },
  {
    "source": "ipahealthcheck.ipa.trust",
    "check": "IPATrustControllerConfCheck",
    "result": "ERROR",
    "uuid": "25012c86-b38e-442e-bf1d-23735a6f97a4",
    "when": "20250620125938Z",
    "duration": "0.045160",
    "kw": {
      "key": "net conf list",
      "error": "No section: 'global'",
      "section": "global",
      "option": "passdb backend",
      "msg": "Unable to read '{option}' in section {section} in {key} output: {error}"
    }
  }
]

• with the version from the PR:

# git clone http://github.com/rcritten/freeipa-healthcheck
# cd freeipa-healthcheck/
# git checkout issue_356
#  python3 -m venv --system-site-packages venv
# venv/bin/pip install -e .
# source venv/bin/activate
(venv) [root@replica freeipa-healthcheck]# ipa-healthcheck --source ipahealthcheck.ipa.trust
[]

The new test is also green.