diagnostics: Add optional component_id to DiagnosticCheck

Signed-off-by: James Valleroy <jvalleroy@mailbox.org>
Reviewed-by: Sunil Mohan Adapa <sunil@medhas.org>

plinth/app.py

@@ -222,9 +222,9 @@ class App:
 
         Return value must be a list of results. Each result is a
         :class:`~plinth.diagnostic_check.DiagnosticCheck` with a
-        unique check_id, a user visible description of the test, and the
+        unique check_id, a user visible description of the test, the result,
-        result. The test result is a string enumeration from 'failed',
+        test parameters, and the component ID. The test result is a string
-        'passed', 'error', 'warning' and 'not_done'.
+        enumeration from 'failed', 'passed', 'error', 'warning' and 'not_done'.
 
         Results are typically collected by diagnosing each component of the app
         and then supplementing the results with any app level diagnostic tests.
@@ -314,9 +314,9 @@ class Component:
 
         Return value must be a list of results. Each result is a
         :class:`~plinth.diagnostic_check.DiagnosticCheck` with a
-        unique check_id, a user visible description of the test, and the
+        unique check_id, a user visible description of the test, the result,
-        result. The test result is a string enumeration from 'failed',
+        test parameters, and the component ID. The test result is a string
-        'passed', 'error', 'warning' and 'not_done'.
+        enumeration from 'failed', 'passed', 'error', 'warning' and 'not_done'.
 
         Also see :meth:`.has_diagnostics`.
 

plinth/config.py

@@ -104,7 +104,7 @@ class DropinConfigs(app_module.FollowerComponent):
             parameters: DiagnosticCheckParameters = {'etc_path': str(etc_path)}
             results.append(
                 DiagnosticCheck(check_id, description, result_string,
-                                parameters))
+                                parameters, self.component_id))
 
         return results
 

plinth/daemon.py

@@ -110,7 +110,9 @@ class Daemon(app.LeaderComponent):
         results = []
         results.append(self._diagnose_unit_is_running())
         for port in self.listen_ports:
-            results.append(diagnose_port_listening(port[0], port[1]))
+            results.append(
+                diagnose_port_listening(port[0], port[1], None,
+                                        self.component_id))
 
         return results
 
@@ -124,7 +126,8 @@ class Daemon(app.LeaderComponent):
             'service_name': str(self.unit)
         }
 
-        return DiagnosticCheck(check_id, description, result, parameters)
+        return DiagnosticCheck(check_id, description, result, parameters,
+                               self.component_id)
 
 
 class RelatedDaemon(app.FollowerComponent):
@@ -200,7 +203,8 @@ def app_is_running(app_):
 
 def diagnose_port_listening(
         port: int, kind: str = 'tcp',
-        listen_address: str | None = None) -> DiagnosticCheck:
+        listen_address: str | None = None,
+        component_id: str | None = None) -> DiagnosticCheck:
     """Run a diagnostic on whether a port is being listened on.
 
     Kind must be one of inet, inet4, inet6, tcp, tcp4, tcp6, udp,
@@ -222,7 +226,7 @@ def diagnose_port_listening(
 
     return DiagnosticCheck(check_id, description,
                            Result.PASSED if result else Result.FAILED,
-                           parameters)
+                           parameters, component_id)
 
 
 def _check_port(port: int, kind: str = 'tcp',
@@ -272,7 +276,8 @@ def _check_port(port: int, kind: str = 'tcp',
 
 
 def diagnose_netcat(host: str, port: int, remote_input: str = '',
-                    negate: bool = False) -> DiagnosticCheck:
+                    negate: bool = False,
+                    component_id: str | None = None) -> DiagnosticCheck:
     """Run a diagnostic using netcat."""
     try:
         process = subprocess.Popen(['nc', host, str(port)],
@@ -298,4 +303,5 @@ def diagnose_netcat(host: str, port: int, remote_input: str = '',
         check_id = f'daemon-netcat-negate-{host}-{port}'
         description = gettext_noop('Cannot connect to {host}:{port}')
 
-    return DiagnosticCheck(check_id, description, result, parameters)
+    return DiagnosticCheck(check_id, description, result, parameters,
+                           component_id)

plinth/diagnostic_check.py

@@ -30,6 +30,7 @@ class DiagnosticCheck:
     description: str
     result: Result = Result.NOT_DONE
     parameters: DiagnosticCheckParameters = field(default_factory=dict)
+    component_id: str | None = None
 
     @property
     def translated_description(self):
@@ -65,6 +66,7 @@ class CheckJSONDecoder(json.JSONDecoder):
         """Convert tagged data to DiagnosticCheck."""
         if data.get('__class__') == 'DiagnosticCheck':
             return DiagnosticCheck(data['check_id'], data['description'],
-                                   data['result'], data['parameters'])
+                                   data['result'], data['parameters'],
+                                   data.get('component_id'))
 
         return data

plinth/modules/apache/components.py

@@ -69,11 +69,13 @@ class Webserver(app.LeaderComponent):
         for url in self.urls:
             if '{host}' in url:
                 results.extend(
-                    diagnose_url_on_all(
+                    diagnose_url_on_all(url, check_certificate=False,
-                        url, check_certificate=False,
+                                        expect_redirects=self.expect_redirects,
-                        expect_redirects=self.expect_redirects))
+                                        component_id=self.component_id))
             else:
-                results.append(diagnose_url(url, check_certificate=False))
+                results.append(
+                    diagnose_url(url, check_certificate=False,
+                                 component_id=self.component_id))
 
         return results
 
@@ -141,7 +143,8 @@ def diagnose_url(url: str, kind: str | None = None,
                  check_certificate: bool = True,
                  extra_options: list[str] | None = None,
                  wrapper: str | None = None,
-                 expected_output: str | None = None) -> DiagnosticCheck:
+                 expected_output: str | None = None,
+                 component_id: str | None = None) -> DiagnosticCheck:
     """Run a diagnostic on whether a URL is accessible.
 
     Kind can be '4' for IPv4 or '6' for IPv6.
@@ -161,10 +164,12 @@ def diagnose_url(url: str, kind: str | None = None,
         check_id = f'apache-url-{url}'
         description = gettext_noop('Access URL {url}')
 
-    return DiagnosticCheck(check_id, description, result, parameters)
+    return DiagnosticCheck(check_id, description, result, parameters,
+                           component_id)
 
 
 def diagnose_url_on_all(url: str, expect_redirects: bool = False,
+                        component_id: str | None = None,
                         **kwargs) -> list[DiagnosticCheck]:
     """Run a diagnostic on whether a URL is accessible."""
     results = []
@@ -174,7 +179,9 @@ def diagnose_url_on_all(url: str, expect_redirects: bool = False,
         if not expect_redirects:
             diagnose_kwargs.setdefault('kind', address['kind'])
 
-        results.append(diagnose_url(current_url, **diagnose_kwargs))
+        results.append(
+            diagnose_url(current_url, component_id=component_id,
+                         **diagnose_kwargs))
 
     return results
 

plinth/modules/apache/tests/test_components.py

@@ -72,13 +72,16 @@ def test_webserver_disable(disable):
 def test_webserver_diagnose(diagnose_url_on_all, diagnose_url):
     """Test running diagnostics."""
 
-    def on_all_side_effect(url, check_certificate, expect_redirects):
+    def on_all_side_effect(url, check_certificate, expect_redirects,
+                           component_id):
         return [
-            DiagnosticCheck('test-all-id', 'test-result-' + url, 'success')
+            DiagnosticCheck('test-all-id', 'test-result-' + url, 'success', {},
+                            component_id)
         ]
 
-    def side_effect(url, check_certificate):
+    def side_effect(url, check_certificate, component_id):
-        return DiagnosticCheck('test-id', 'test-result-' + url, 'success')
+        return DiagnosticCheck('test-id', 'test-result-' + url, 'success', {},
+                               component_id)
 
     diagnose_url_on_all.side_effect = on_all_side_effect
     diagnose_url.side_effect = side_effect
@@ -86,19 +89,26 @@ def test_webserver_diagnose(diagnose_url_on_all, diagnose_url):
                            urls=['{host}url1', 'url2'], expect_redirects=True)
     results = webserver1.diagnose()
     assert results == [
-        DiagnosticCheck('test-all-id', 'test-result-{host}url1', 'success'),
+        DiagnosticCheck('test-all-id', 'test-result-{host}url1', 'success', {},
-        DiagnosticCheck('test-id', 'test-result-url2', 'success')
+                        'test-webserver'),
+        DiagnosticCheck('test-id', 'test-result-url2', 'success', {},
+                        'test-webserver')
     ]
-    diagnose_url_on_all.assert_has_calls(
+    diagnose_url_on_all.assert_has_calls([
-        [call('{host}url1', check_certificate=False, expect_redirects=True)])
+        call('{host}url1', check_certificate=False, expect_redirects=True,
-    diagnose_url.assert_has_calls([call('url2', check_certificate=False)])
+             component_id='test-webserver')
+    ])
+    diagnose_url.assert_has_calls(
+        [call('url2', check_certificate=False, component_id='test-webserver')])
 
     diagnose_url_on_all.reset_mock()
     webserver2 = Webserver('test-webserver', 'test-config',
                            urls=['{host}url1', 'url2'], expect_redirects=False)
     results = webserver2.diagnose()
-    diagnose_url_on_all.assert_has_calls(
+    diagnose_url_on_all.assert_has_calls([
-        [call('{host}url1', check_certificate=False, expect_redirects=False)])
+        call('{host}url1', check_certificate=False, expect_redirects=False,
+             component_id='test-webserver')
+    ])
 
 
 @patch('plinth.privileged.service.restart')
@@ -244,20 +254,23 @@ def test_diagnose_url(get_addresses, check):
             'test-1': 'value-1'
         },
         'wrapper': 'test-wrapper',
-        'expected_output': 'test-expected'
+        'expected_output': 'test-expected',
+        'component_id': 'test-component',
     }
     parameters = {key: args[key] for key in ['url', 'kind']}
     check.return_value = True
     result = diagnose_url(**args)
     assert result == DiagnosticCheck(
         'apache-url-kind-https://localhost/test-4',
-        'Access URL {url} on tcp{kind}', Result.PASSED, parameters)
+        'Access URL {url} on tcp{kind}', Result.PASSED, parameters,
+        'test-component')
 
     check.return_value = False
     result = diagnose_url(**args)
     assert result == DiagnosticCheck(
         'apache-url-kind-https://localhost/test-4',
-        'Access URL {url} on tcp{kind}', Result.FAILED, parameters)
+        'Access URL {url} on tcp{kind}', Result.FAILED, parameters,
+        'test-component')
 
     del args['kind']
     args['url'] = 'https://{host}/test'
@@ -287,10 +300,10 @@ def test_diagnose_url(get_addresses, check):
     assert results == [
         DiagnosticCheck('apache-url-kind-https://test-host-1/test-4',
                         'Access URL {url} on tcp{kind}', Result.PASSED,
-                        parameters[0]),
+                        parameters[0], 'test-component'),
         DiagnosticCheck('apache-url-kind-https://test-host-2/test-6',
                         'Access URL {url} on tcp{kind}', Result.PASSED,
-                        parameters[1]),
+                        parameters[1], 'test-component'),
     ]
 
 

plinth/modules/firewall/components.py

@@ -142,7 +142,8 @@ class Firewall(app.FollowerComponent):
                 'details': details
             }
             results.append(
-                DiagnosticCheck(check_id, description, result, parameters))
+                DiagnosticCheck(check_id, description, result, parameters,
+                                self.component_id))
 
             # External zone
             if self.is_external:
@@ -161,7 +162,8 @@ class Firewall(app.FollowerComponent):
 
             parameters = {'name': port, 'details': details}
             results.append(
-                DiagnosticCheck(check_id, description, result, parameters))
+                DiagnosticCheck(check_id, description, result, parameters,
+                                self.component_id))
 
         return results
 

plinth/modules/firewall/tests/test_components.py

@@ -161,28 +161,28 @@ def test_diagnose(get_enabled_services, get_port_details):
             'networks', Result.PASSED, {
                 'name': 'test-port1',
                 'details': '1234/tcp, 1234/udp'
-            }),
+            }, 'test-firewall-1'),
         DiagnosticCheck(
             'firewall-port-external-unavailable-test-port1',
             'Port {name} ({details}) unavailable for external '
             'networks', Result.PASSED, {
                 'name': 'test-port1',
                 'details': '1234/tcp, 1234/udp'
-            }),
+            }, 'test-firewall-1'),
         DiagnosticCheck(
             'firewall-port-internal-test-port2',
             'Port {name} ({details}) available for internal networks',
             Result.FAILED, {
                 'name': 'test-port2',
                 'details': '2345/udp'
-            }),
+            }, 'test-firewall-1'),
         DiagnosticCheck(
             'firewall-port-external-unavailable-test-port2',
             'Port {name} ({details}) unavailable for external networks',
             Result.FAILED, {
                 'name': 'test-port2',
                 'details': '2345/udp'
-            }),
+            }, 'test-firewall-1'),
     ]
 
     firewall = Firewall('test-firewall-1', ports=['test-port3', 'test-port4'],
@@ -195,28 +195,28 @@ def test_diagnose(get_enabled_services, get_port_details):
             Result.PASSED, {
                 'name': 'test-port3',
                 'details': '3456/tcp'
-            }),
+            }, 'test-firewall-1'),
         DiagnosticCheck(
             'firewall-port-external-available-test-port3',
             'Port {name} ({details}) available for external networks',
             Result.PASSED, {
                 'name': 'test-port3',
                 'details': '3456/tcp'
-            }),
+            }, 'test-firewall-1'),
         DiagnosticCheck(
             'firewall-port-internal-test-port4',
             'Port {name} ({details}) available for internal networks',
             Result.FAILED, {
                 'name': 'test-port4',
                 'details': '4567/udp'
-            }),
+            }, 'test-firewall-1'),
         DiagnosticCheck(
             'firewall-port-external-available-test-port4',
             'Port {name} ({details}) available for external networks',
             Result.FAILED, {
                 'name': 'test-port4',
                 'details': '4567/udp'
-            }),
+            }, 'test-firewall-1'),
     ]
 
 

plinth/package.py

@@ -210,7 +210,7 @@ class Packages(app_module.FollowerComponent):
                 }
                 results.append(
                     DiagnosticCheck(check_id, description, Result.FAILED,
-                                    parameters))
+                                    parameters, self.component_id))
                 continue
 
             result = Result.WARNING
@@ -230,7 +230,8 @@ class Packages(app_module.FollowerComponent):
                 'latest_version': str(latest_version)
             }
             results.append(
-                DiagnosticCheck(check_id, description, result, parameters))
+                DiagnosticCheck(check_id, description, result, parameters,
+                                self.component_id))
 
         return results
 

plinth/tests/test_config.py

@@ -165,11 +165,13 @@ def test_dropin_config_diagnose_symlinks(dropin_configs, tmp_path):
             DiagnosticCheck(
                 f'dropin-config-{tmp_path}/etc/test/path1',
                 'Static configuration {etc_path} is setup properly',
-                Result.FAILED, {'etc_path': f'{tmp_path}/etc/test/path1'}),
+                Result.FAILED, {'etc_path': f'{tmp_path}/etc/test/path1'},
+                'test-component'),
             DiagnosticCheck(
                 f'dropin-config-{tmp_path}/etc/path2',
                 'Static configuration {etc_path} is setup properly',
-                Result.FAILED, {'etc_path': f'{tmp_path}/etc/path2'}),
+                Result.FAILED, {'etc_path': f'{tmp_path}/etc/path2'},
+                'test-component'),
         ]
 
         # Proper symlinks exist

plinth/tests/test_daemon.py

@@ -181,9 +181,9 @@ def test_ensure_running(subprocess_call, subprocess_run, service_is_running,
 def test_diagnose(port_listening, service_is_running, daemon):
     """Test running diagnostics."""
 
-    def side_effect(port, kind):
+    def side_effect(port, kind, _listen_address, component_id):
         name = f'test-result-{port}-{kind}'
-        return DiagnosticCheck(name, name, Result.PASSED)
+        return DiagnosticCheck(name, name, Result.PASSED, {}, component_id)
 
     daemon = Daemon('test-daemon', 'test-unit', listen_ports=[(8273, 'tcp4'),
                                                               (345, 'udp')])
@@ -193,13 +193,16 @@ def test_diagnose(port_listening, service_is_running, daemon):
     assert results == [
         DiagnosticCheck('daemon-running-test-unit',
                         'Service {service_name} is running', Result.PASSED,
-                        {'service_name': 'test-unit'}),
+                        {'service_name': 'test-unit'}, 'test-daemon'),
         DiagnosticCheck('test-result-8273-tcp4', 'test-result-8273-tcp4',
-                        Result.PASSED),
+                        Result.PASSED, {}, 'test-daemon'),
         DiagnosticCheck('test-result-345-udp', 'test-result-345-udp',
-                        Result.PASSED)
+                        Result.PASSED, {}, 'test-daemon')
     ]
-    port_listening.assert_has_calls([call(8273, 'tcp4'), call(345, 'udp')])
+    port_listening.assert_has_calls([
+        call(8273, 'tcp4', None, 'test-daemon'),
+        call(345, 'udp', None, 'test-daemon')
+    ])
     service_is_running.assert_has_calls([call('test-unit')])
 
     service_is_running.return_value = False
@@ -342,12 +345,13 @@ def test_diagnose_netcat(popen):
     assert popen.mock_calls[2] == call().communicate(input=b'test-input')
 
     result = diagnose_netcat('test-host', 3300, remote_input='test-input',
-                             negate=True)
+                             negate=True, component_id='test-component')
     parameters2 = parameters.copy()
     parameters2['negate'] = True
     assert result == DiagnosticCheck('daemon-netcat-negate-test-host-3300',
                                      'Cannot connect to {host}:{port}',
-                                     Result.FAILED, parameters2)
+                                     Result.FAILED, parameters2,
+                                     'test-component')
 
     popen().returncode = 1
     result = diagnose_netcat('test-host', 3300, remote_input='test-input')

plinth/tests/test_diagnostic_check.py

@@ -23,13 +23,14 @@ def test_result():
 def test_diagnostic_check():
     """Test the diagnostic check data class."""
     with pytest.raises(TypeError):
-        DiagnosticCheck()
+        DiagnosticCheck()  # pylint: disable=E1120
 
     check = DiagnosticCheck('some-check-id', 'sample check')
     assert check.check_id == 'some-check-id'
     assert check.description == 'sample check'
     assert check.translated_description == 'sample check'
     assert check.result == Result.NOT_DONE
+    assert check.component_id is None
     assert not check.parameters
 
     check = DiagnosticCheck('some-check-id', 'sample check', Result.PASSED)
@@ -40,6 +41,10 @@ def test_diagnostic_check():
                             {'key': 'value'})
     assert check.parameters['key'] == 'value'
 
+    check = DiagnosticCheck('some-check-id', 'sample check', Result.FAILED, {},
+                            'some-component')
+    assert check.component_id == 'some-component'
+
 
 def test_translate():
     """Test formatting the translated description."""
@@ -58,7 +63,7 @@ def test_json_encoder_decoder():
     check_json = json.dumps(check, cls=CheckJSONEncoder)
     for string in [
             '"check_id": "some-check-id"', '"description": "sample check"',
-            '"result": "passed"', '"parameters": {}',
+            '"result": "passed"', '"parameters": {}', '"component_id": null',
             '"__class__": "DiagnosticCheck"'
     ]:
         assert string in check_json

plinth/tests/test_package.py

@@ -284,32 +284,34 @@ def test_diagnose(cache):
         DiagnosticCheck(
             'package-available-package1',
             'Package {package_expression} is not available for install',
-            Result.FAILED, {'package_expression': 'package1'}),
+            Result.FAILED, {'package_expression': 'package1'},
+            'test-component'),
         DiagnosticCheck(
             'package-latest-package2',
             'Package {package_name} is the latest version ({latest_version})',
             Result.PASSED, {
                 'package_name': 'package2',
                 'latest_version': '2.0'
-            }),
+            }, 'test-component'),
         DiagnosticCheck(
             'package-latest-package3',
             'Package {package_name} is the latest version ({latest_version})',
             Result.WARNING, {
                 'package_name': 'package3',
                 'latest_version': '3.0'
-            }),
+            }, 'test-component'),
         DiagnosticCheck(
             'package-available-package4 | package5',
             'Package {package_expression} is not available for install',
-            Result.FAILED, {'package_expression': 'package4 | package5'}),
+            Result.FAILED, {'package_expression': 'package4 | package5'},
+            'test-component'),
         DiagnosticCheck(
             'package-latest-package7',
             'Package {package_name} is the latest version ({latest_version})',
             Result.PASSED, {
                 'package_name': 'package7',
                 'latest_version': '4.0'
-            }),
+            }, 'test-component'),
     ]