Optimized filter functions

This commit is contained in:
Kevin Veen-Birkenbach 2025-07-13 14:20:22 +02:00
parent c8669e19cf
commit 5340d580ce
No known key found for this signature in database
GPG Key ID: 44D8F11FD62F878E
7 changed files with 69 additions and 22 deletions

View File

@ -124,7 +124,7 @@ class FilterModule(object):
# Enable loading via ancestors # Enable loading via ancestors
if ( if (
self.is_feature_enabled(applications, 'portfolio_iframe', application_id) self.is_feature_enabled(applications, 'port-ui-desktop', application_id)
and directive == 'frame-ancestors' and directive == 'frame-ancestors'
): ):
domain = domains.get('web-app-port-ui')[0] domain = domains.get('web-app-port-ui')[0]

View File

@ -72,14 +72,12 @@ def get_app_conf(applications, application_id, config_path, strict=True):
try: try:
obj = applications[application_id] obj = applications[application_id]
except KeyError: except KeyError:
if strict: raise AppConfigKeyError(
raise AppConfigKeyError( f"Application ID '{application_id}' not found in applications dict.\n"
f"Application ID '{application_id}' not found in applications dict.\n" f"path_trace: {path_trace}\n"
f"path_trace: {path_trace}\n" f"applications keys: {list(applications.keys())}\n"
f"applications keys: {list(applications.keys())}\n" f"config_path: {config_path}"
f"config_path: {config_path}" )
)
return False
for part in config_path.split("."): for part in config_path.split("."):
path_trace.append(part) path_trace.append(part)

View File

@ -0,0 +1,47 @@
# tests/integration/test_no_applications_variable_usage.py
import os
import re
import unittest
class TestNoApplicationsVariableUsage(unittest.TestCase):
"""
This test ensures that the pattern `applications[some_variable]` is not used anywhere
under the roles/ directory. Instead, the usage of get_app_conf should be preferred.
"""
APPLICATIONS_VARIABLE_PATTERN = re.compile(r"applications\[\s*[a-zA-Z_][a-zA-Z0-9_]*\s*\]")
def test_no_applications_variable_usage(self):
roles_dir = os.path.abspath(os.path.join(os.path.dirname(__file__), '..', '..', 'roles'))
found = []
for root, dirs, files in os.walk(roles_dir):
# Skip __pycache__ folders
dirs[:] = [d for d in dirs if d != '__pycache__']
for file in files:
if file.endswith('.pyc'):
continue
file_path = os.path.join(root, file)
# Skip this test file itself (so it can contain the pattern in docstrings)
if os.path.abspath(file_path) == os.path.abspath(__file__):
continue
try:
with open(file_path, 'r', encoding='utf-8') as f:
for lineno, line in enumerate(f, 1):
match = self.APPLICATIONS_VARIABLE_PATTERN.search(line)
if match:
found.append(f"{file_path}:{lineno}: {line.strip()}")
except Exception:
# Binary or unreadable file, skip
continue
if found:
self.fail(
"Found illegal usages of 'applications[variable]' in the following locations:\n"
+ "\n".join(found)
+ "\n\nPlease use get_app_conf instead."
)
if __name__ == '__main__':
unittest.main()

View File

@ -1,5 +1,3 @@
# tests/unit/test_configuration_filters.py
import unittest import unittest
import sys import sys
import os import os
@ -15,6 +13,8 @@ from filter_plugins.configuration_filters import (
is_feature_enabled, is_feature_enabled,
) )
from filter_plugins.get_app_conf import AppConfigKeyError
class TestConfigurationFilters(unittest.TestCase): class TestConfigurationFilters(unittest.TestCase):
def setUp(self): def setUp(self):
@ -38,7 +38,8 @@ class TestConfigurationFilters(unittest.TestCase):
self.assertFalse(is_feature_enabled(self.applications, 'nonexistent', 'app1')) self.assertFalse(is_feature_enabled(self.applications, 'nonexistent', 'app1'))
def test_is_feature_enabled_false_missing_app(self): def test_is_feature_enabled_false_missing_app(self):
self.assertFalse(is_feature_enabled(self.applications, 'oauth2', 'unknown_app')) with self.assertRaises(Exception):
is_feature_enabled(self.applications, 'oauth2', 'unknown_app')
if __name__ == '__main__': if __name__ == '__main__':
unittest.main() unittest.main()

View File

@ -174,10 +174,10 @@ class TestCspFilters(unittest.TestCase):
def test_build_csp_header_frame_ancestors(self): def test_build_csp_header_frame_ancestors(self):
""" """
frame-ancestors should include the wildcarded SLD+TLD when frame-ancestors should include the wildcarded SLD+TLD when
'portfolio_iframe' is enabled, and omit it when disabled. 'port-ui-desktop' is enabled, and omit it when disabled.
""" """
# Ensure feature enabled and domain set # Ensure feature enabled and domain set
self.apps['app1']['features']['portfolio_iframe'] = True self.apps['app1']['features']['port-ui-desktop'] = True
# simulate a subdomain for the application # simulate a subdomain for the application
self.domains['web-app-port-ui'] = ['domain-example.com'] self.domains['web-app-port-ui'] = ['domain-example.com']
@ -189,7 +189,7 @@ class TestCspFilters(unittest.TestCase):
) )
# Now disable the feature and rebuild # Now disable the feature and rebuild
self.apps['app1']['features']['portfolio_iframe'] = False self.apps['app1']['features']['port-ui-desktop'] = False
header_no = self.filter.build_csp_header(self.apps, 'app1', self.domains, web_protocol='https') header_no = self.filter.build_csp_header(self.apps, 'app1', self.domains, web_protocol='https')
# Should no longer contain the wildcarded sld.tld # Should no longer contain the wildcarded sld.tld
self.assertNotIn("*.domain-example.com", header_no) self.assertNotIn("*.domain-example.com", header_no)

View File

@ -4,7 +4,7 @@ import os
sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), '../../../filter_plugins'))) sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), '../../../filter_plugins')))
from get_app_conf import get_app_conf from get_app_conf import get_app_conf, AppConfigKeyError
from ansible.errors import AnsibleFilterError from ansible.errors import AnsibleFilterError
class TestGetAppConf(unittest.TestCase): class TestGetAppConf(unittest.TestCase):
@ -99,11 +99,12 @@ class TestGetAppConf(unittest.TestCase):
get_app_conf(apps, "app", "foo[1].bar", strict=True) get_app_conf(apps, "app", "foo[1].bar", strict=True)
def test_application_id_not_found(self): def test_application_id_not_found(self):
"""Test with unknown application_id: should raise error in strict mode.""" """Test with unknown application_id: should always raise error now."""
with self.assertRaises(AnsibleFilterError): with self.assertRaises(AppConfigKeyError):
get_app_conf(self.applications, "unknown", "features.foo", strict=True) get_app_conf(self.applications, "unknown", "features.foo", strict=True)
# Non-strict: returns False with self.assertRaises(AppConfigKeyError):
self.assertFalse(get_app_conf(self.applications, "unknown", "features.foo", strict=False)) get_app_conf(self.applications, "unknown", "features.foo", strict=False)
if __name__ == '__main__': if __name__ == '__main__':
unittest.main() unittest.main()

View File

@ -57,7 +57,7 @@ galaxy_info:
"applications": { "applications": {
"portfolio": { "portfolio": {
"features": { "features": {
"portfolio_iframe": True "port-ui-desktop": True
} }
} }
}, },
@ -90,7 +90,7 @@ galaxy_info:
"applications": { "applications": {
"portfolio": { "portfolio": {
"features": { "features": {
"portfolio_iframe": True "port-ui-desktop": True
} }
} }
}, },