From 436060fecb286612bb80b7ba27876e1d3e45b14e Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Tue, 10 May 2022 11:57:29 -0700 Subject: [PATCH] apache: Allow URL diagnostics to work with redirects Upon access of an app URL, it may redirect to another URL that is configured in app settings. This new URL could only be accessed on IPv4 or IPv6 only. When curl is invoked with the IP address version of a different kind, the access fails. In such cases, tell the diagnostics methods not the restrict to a particular address type. Tests: - Unit tests pass. - All of transmission's diagnostics tests pass. The URL tests show that they have been performed on a particular IP address type. Signed-off-by: Sunil Mohan Adapa Reviewed-by: James Valleroy --- plinth/modules/apache/components.py | 17 ++++++++++----- .../modules/apache/tests/test_components.py | 21 +++++++++++++------ 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/plinth/modules/apache/components.py b/plinth/modules/apache/components.py index c6ac23958..7737dce5e 100644 --- a/plinth/modules/apache/components.py +++ b/plinth/modules/apache/components.py @@ -15,7 +15,8 @@ from plinth import action_utils, actions, app class Webserver(app.LeaderComponent): """Component to enable/disable Apache configuration.""" - def __init__(self, component_id, web_name, kind='config', urls=None): + def __init__(self, component_id, web_name, kind='config', urls=None, + expect_redirects=False): """Initialize the web server component. component_id should be a unique ID across all components of an app and @@ -38,6 +39,7 @@ class Webserver(app.LeaderComponent): self.web_name = web_name self.kind = kind self.urls = urls or [] + self.expect_redirects = expect_redirects def is_enabled(self): """Return whether the Apache configuration is enabled.""" @@ -64,7 +66,9 @@ class Webserver(app.LeaderComponent): for url in self.urls: if '{host}' in url: results.extend( - diagnose_url_on_all(url, check_certificate=False)) + diagnose_url_on_all( + url, check_certificate=False, + expect_redirects=self.expect_redirects)) else: results.append(diagnose_url(url, check_certificate=False)) @@ -128,13 +132,16 @@ def diagnose_url(url, kind=None, env=None, check_certificate=True, return [testname, result] -def diagnose_url_on_all(url, **kwargs): +def diagnose_url_on_all(url, expect_redirects=False, **kwargs): """Run a diagnostic on whether a URL is accessible.""" results = [] for address in action_utils.get_addresses(): current_url = url.format(host=address['url_address']) - results.append( - diagnose_url(current_url, kind=address['kind'], **kwargs)) + diagnose_kwargs = dict(kwargs) + if not expect_redirects: + diagnose_kwargs.setdefault('kind', address['kind']) + + results.append(diagnose_url(current_url, **diagnose_kwargs)) return results diff --git a/plinth/modules/apache/tests/test_components.py b/plinth/modules/apache/tests/test_components.py index 61b71e3b5..7ceba4ca1 100644 --- a/plinth/modules/apache/tests/test_components.py +++ b/plinth/modules/apache/tests/test_components.py @@ -19,15 +19,17 @@ def test_webserver_init(): Webserver(None, None) webserver = Webserver('test-webserver', 'test-config', kind='module', - urls=['url1', 'url2']) + urls=['url1', 'url2'], expect_redirects=True) assert webserver.component_id == 'test-webserver' assert webserver.web_name == 'test-config' assert webserver.kind == 'module' assert webserver.urls == ['url1', 'url2'] + assert webserver.expect_redirects webserver = Webserver('test-webserver', None) assert webserver.kind == 'config' assert webserver.urls == [] + assert not webserver.expect_redirects @patch('plinth.action_utils.webserver_is_enabled') @@ -73,7 +75,7 @@ def test_webserver_disable(superuser_run): def test_webserver_diagnose(diagnose_url_on_all, diagnose_url): """Test running diagnostics.""" - def on_all_side_effect(url, check_certificate): + def on_all_side_effect(url, check_certificate, expect_redirects): return [('test-result-' + url, 'success')] def side_effect(url, check_certificate): @@ -81,15 +83,22 @@ def test_webserver_diagnose(diagnose_url_on_all, diagnose_url): diagnose_url_on_all.side_effect = on_all_side_effect diagnose_url.side_effect = side_effect - webserver = Webserver('test-webserver', 'test-config', - urls=['{host}url1', 'url2']) - results = webserver.diagnose() + webserver1 = Webserver('test-webserver', 'test-config', + urls=['{host}url1', 'url2'], expect_redirects=True) + results = webserver1.diagnose() assert results == [('test-result-{host}url1', 'success'), ('test-result-url2', 'success')] diagnose_url_on_all.assert_has_calls( - [call('{host}url1', check_certificate=False)]) + [call('{host}url1', check_certificate=False, expect_redirects=True)]) diagnose_url.assert_has_calls([call('url2', check_certificate=False)]) + 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( + [call('{host}url1', check_certificate=False, expect_redirects=False)]) + def test_uwsgi_init(): """Test that uWSGI component can be initialized."""