From 4e42cc09b47f33bc356cb75f23e5f57461dc4cfc Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcritten@redhat.com>
Date: Wed, 18 Jun 2025 14:34:38 -0400
Subject: [PATCH] Don't rely on order in trust agent/controller role check

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: https://github.com/freeipa/freeipa-healthcheck/issues/356

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
---
 src/ipahealthcheck/ipa/plugin.py | 15 ++++---
 tests/test_ipa_trust.py          | 71 +++++++++++++++++++++++++++++++-
 2 files changed, 79 insertions(+), 7 deletions(-)

diff --git a/src/ipahealthcheck/ipa/plugin.py b/src/ipahealthcheck/ipa/plugin.py
index f1a325c3..efaa9477 100644
--- a/src/ipahealthcheck/ipa/plugin.py
+++ b/src/ipahealthcheck/ipa/plugin.py
@@ -35,6 +35,13 @@ def __init__(self):
         self.trust_controller = False
         self.ca_configured = False
 
+    def has_role(self, roles):
+        for role in roles:
+            if role.get('server_server') == api.env.host:
+                if role.get('status') == 'enabled':
+                    return True
+        return False
+
     def initialize(self, framework, config, options=None):
         super().initialize(framework, config)
         # deferred import for mock
@@ -81,12 +88,8 @@ def initialize(self, framework, config, options=None):
                 component_services=['ADTRUST']
             ),
         )
-        role = roles[0].status(api)[0]
-        if role.get('status') == 'enabled':
-            self.trust_agent = True
-        role = roles[1].status(api)[0]
-        if role.get('status') == 'enabled':
-            self.trust_controller = True
+        self.trust_agent = self.has_role(roles[0].status(api))
+        self.trust_controller = self.has_role(roles[1].status(api))
 
 
 registry = IPARegistry()
diff --git a/tests/test_ipa_trust.py b/tests/test_ipa_trust.py
index 39ad6ec5..3d050c3b 100644
--- a/tests/test_ipa_trust.py
+++ b/tests/test_ipa_trust.py
@@ -11,7 +11,8 @@
 from util import m_api
 
 from ipahealthcheck.core import config, constants
-from ipahealthcheck.ipa.plugin import registry
+from ipahealthcheck.core.plugin import Results
+from ipahealthcheck.ipa.plugin import registry, IPARegistry
 from ipahealthcheck.ipa.trust import (IPATrustAgentCheck,
                                       IPATrustDomainsCheck,
                                       IPADomainCheck,
@@ -1329,3 +1330,71 @@ def test_ipakrbauthzdata_negative(self):
         assert result.result == constants.ERROR
         assert result.source == 'ipahealthcheck.ipa.trust'
         assert result.check == 'IPAauthzdatapacCheck'
+
+
+class TestHasRole(BaseTest):
+    """Verify that the output of server-role-find which is used to
+       determine whether a host is a trust agent or controller
+      (or neither) isn't dependent upon the order the hosts are
+      returned.
+
+      Only trust agent is tested here but there is no difference
+      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):
+        self.results = Results()
+        reg = IPARegistry()
+
+        roles = [
+            {
+                "role_servrole": "AD trust agent",
+                "server_server": "server.ipa.example",
+                "status": "enabled",
+            },
+            {
+                "role_servrole": "AD trust agent",
+                "server_server": "replica.ipa.example",
+                "status": "absent",
+            },
+        ]
+
+        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