From 26f349837d3bcac6631ce596273f1a844e07c8cd Mon Sep 17 00:00:00 2001 From: Dan Guido Date: Thu, 7 Aug 2025 11:04:36 -0700 Subject: [PATCH] Fix linter issues in test_yaml_jinja2_expressions.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fixed trailing whitespace issues (W293) - Applied ruff formatting for consistent code style - All tests still pass after formatting changes 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- tests/unit/test_yaml_jinja2_expressions.py | 258 ++++++++++----------- 1 file changed, 126 insertions(+), 132 deletions(-) diff --git a/tests/unit/test_yaml_jinja2_expressions.py b/tests/unit/test_yaml_jinja2_expressions.py index c9bfae9b..946b7054 100644 --- a/tests/unit/test_yaml_jinja2_expressions.py +++ b/tests/unit/test_yaml_jinja2_expressions.py @@ -3,6 +3,7 @@ Test that Jinja2 expressions within YAML files are valid. This catches issues like inline comments in Jinja2 expressions within YAML task files. """ + import re from pathlib import Path @@ -16,18 +17,12 @@ def find_yaml_files_with_jinja2(): yaml_files = [] # Look for YAML files in roles that are likely to have Jinja2 - patterns = [ - 'roles/**/tasks/*.yml', - 'roles/**/defaults/*.yml', - 'roles/**/vars/*.yml', - 'playbooks/*.yml', - '*.yml' - ] + patterns = ["roles/**/tasks/*.yml", "roles/**/defaults/*.yml", "roles/**/vars/*.yml", "playbooks/*.yml", "*.yml"] - skip_dirs = {'.git', '.venv', 'venv', '.env', 'configs'} + skip_dirs = {".git", ".venv", "venv", ".env", "configs"} for pattern in patterns: - for path in Path('.').glob(pattern): + for path in Path(".").glob(pattern): if not any(skip_dir in path.parts for skip_dir in skip_dirs): yaml_files.append(path) @@ -39,31 +34,35 @@ def extract_jinja2_expressions(content): expressions = [] # Find {{ ... }} expressions (variable interpolations) - for match in re.finditer(r'\{\{(.+?)\}\}', content, re.DOTALL): - expressions.append({ - 'type': 'variable', - 'content': match.group(1), - 'full': match.group(0), - 'start': match.start(), - 'end': match.end() - }) + for match in re.finditer(r"\{\{(.+?)\}\}", content, re.DOTALL): + expressions.append( + { + "type": "variable", + "content": match.group(1), + "full": match.group(0), + "start": match.start(), + "end": match.end(), + } + ) # Find {% ... %} expressions (control structures) - for match in re.finditer(r'\{%(.+?)%\}', content, re.DOTALL): - expressions.append({ - 'type': 'control', - 'content': match.group(1), - 'full': match.group(0), - 'start': match.start(), - 'end': match.end() - }) + for match in re.finditer(r"\{%(.+?)%\}", content, re.DOTALL): + expressions.append( + { + "type": "control", + "content": match.group(1), + "full": match.group(0), + "start": match.start(), + "end": match.end(), + } + ) return expressions def find_line_number(content, position): """Find the line number for a given position in content.""" - return content[:position].count('\n') + 1 + return content[:position].count("\n") + 1 def validate_jinja2_expression(expression, context_vars=None): @@ -75,9 +74,9 @@ def validate_jinja2_expression(expression, context_vars=None): context_vars = get_test_variables() # First check for inline comments - this is the main issue we want to catch - if '#' in expression['content']: + if "#" in expression["content"]: # Check if the # is within a list or dict literal - content = expression['content'] + content = expression["content"] # Remove strings to avoid false positives cleaned = re.sub(r'"[^"]*"', '""', content) cleaned = re.sub(r"'[^']*'", "''", cleaned) @@ -85,43 +84,46 @@ def validate_jinja2_expression(expression, context_vars=None): # Look for # that appears to be a comment # The # should have something before it (not at start) and something after (the comment text) # Also check for # at the start of a line within the expression - if '#' in cleaned: + if "#" in cleaned: # Check each line in the cleaned expression - for line in cleaned.split('\n'): + for line in cleaned.split("\n"): line = line.strip() - if '#' in line: + if "#" in line: # If # appears and it's not escaped (\#) - hash_idx = line.find('#') + hash_idx = line.find("#") if hash_idx >= 0: # Check if it's escaped - if hash_idx == 0 or line[hash_idx-1] != '\\': + if hash_idx == 0 or line[hash_idx - 1] != "\\": # This looks like an inline comment - return False, "Inline comment (#) found in Jinja2 expression - comments must be outside expressions" + return ( + False, + "Inline comment (#) found in Jinja2 expression - comments must be outside expressions", + ) try: env = Environment(undefined=StrictUndefined) # Add common Ansible filters (expanded list) - env.filters['bool'] = lambda x: bool(x) - env.filters['default'] = lambda x, d='': x if x else d - env.filters['to_uuid'] = lambda x: 'mock-uuid' - env.filters['b64encode'] = lambda x: 'mock-base64' - env.filters['b64decode'] = lambda x: 'mock-decoded' - env.filters['version'] = lambda x, op: True - env.filters['ternary'] = lambda x, y, z=None: y if x else (z if z is not None else '') - env.filters['regex_replace'] = lambda x, p, r: x - env.filters['difference'] = lambda x, y: list(set(x) - set(y)) - env.filters['strftime'] = lambda fmt, ts: 'mock-timestamp' - env.filters['int'] = lambda x: int(x) if x else 0 - env.filters['list'] = lambda x: list(x) - env.filters['map'] = lambda x, *args: x - env.tests['version'] = lambda x, op: True + env.filters["bool"] = lambda x: bool(x) + env.filters["default"] = lambda x, d="": x if x else d + env.filters["to_uuid"] = lambda x: "mock-uuid" + env.filters["b64encode"] = lambda x: "mock-base64" + env.filters["b64decode"] = lambda x: "mock-decoded" + env.filters["version"] = lambda x, op: True + env.filters["ternary"] = lambda x, y, z=None: y if x else (z if z is not None else "") + env.filters["regex_replace"] = lambda x, p, r: x + env.filters["difference"] = lambda x, y: list(set(x) - set(y)) + env.filters["strftime"] = lambda fmt, ts: "mock-timestamp" + env.filters["int"] = lambda x: int(x) if x else 0 + env.filters["list"] = lambda x: list(x) + env.filters["map"] = lambda x, *args: x + env.tests["version"] = lambda x, op: True # Wrap the expression in appropriate delimiters for parsing - if expression['type'] == 'variable': - template_str = '{{' + expression['content'] + '}}' + if expression["type"] == "variable": + template_str = "{{" + expression["content"] + "}}" else: - template_str = '{%' + expression['content'] + '%}' + template_str = "{%" + expression["content"] + "%}" # Try to compile the template template = env.from_string(template_str) @@ -134,15 +136,15 @@ def validate_jinja2_expression(expression, context_vars=None): except TemplateSyntaxError as e: # Check for the specific inline comment issue - if '#' in expression['content']: + if "#" in expression["content"]: # Check if the # is within a list or dict literal - content = expression['content'] + content = expression["content"] # Remove strings to avoid false positives cleaned = re.sub(r'"[^"]*"', '""', content) cleaned = re.sub(r"'[^']*'", "''", cleaned) # Look for # that appears to be a comment (not in string, not escaped) - if re.search(r'[^\\\n]#[^\}]', cleaned): + if re.search(r"[^\\\n]#[^\}]", cleaned): return False, "Inline comment (#) found in Jinja2 expression - comments must be outside expressions" return False, f"Syntax error: {e.message}" @@ -151,7 +153,7 @@ def validate_jinja2_expression(expression, context_vars=None): # Be lenient - we mainly care about inline comments and basic syntax # Ignore runtime errors (undefined vars, missing attributes, etc.) error_str = str(e).lower() - if any(ignore in error_str for ignore in ['undefined', 'has no attribute', 'no filter']): + if any(ignore in error_str for ignore in ["undefined", "has no attribute", "no filter"]): return True, None # These are runtime issues, not syntax issues return False, f"Error: {str(e)}" @@ -160,54 +162,47 @@ def get_test_variables(): """Get a comprehensive set of test variables for expression validation.""" return { # Network configuration - 'IP_subject_alt_name': '10.0.0.1', - 'server_name': 'algo-vpn', - 'wireguard_port': 51820, - 'wireguard_network': '10.19.49.0/24', - 'wireguard_network_ipv6': 'fd9d:bc11:4021::/64', - 'strongswan_network': '10.19.48.0/24', - 'strongswan_network_ipv6': 'fd9d:bc11:4020::/64', - + "IP_subject_alt_name": "10.0.0.1", + "server_name": "algo-vpn", + "wireguard_port": 51820, + "wireguard_network": "10.19.49.0/24", + "wireguard_network_ipv6": "fd9d:bc11:4021::/64", + "strongswan_network": "10.19.48.0/24", + "strongswan_network_ipv6": "fd9d:bc11:4020::/64", # Feature flags - 'ipv6_support': True, - 'dns_encryption': True, - 'dns_adblocking': True, - 'wireguard_enabled': True, - 'ipsec_enabled': True, - + "ipv6_support": True, + "dns_encryption": True, + "dns_adblocking": True, + "wireguard_enabled": True, + "ipsec_enabled": True, # OpenSSL/PKI - 'openssl_constraint_random_id': 'test-uuid-12345', - 'CA_password': 'test-password', - 'p12_export_password': 'test-p12-password', - 'ipsec_pki_path': '/etc/ipsec.d', - 'ipsec_config_path': '/etc/ipsec.d', - 'subjectAltName': 'IP:10.0.0.1,DNS:vpn.example.com', - 'subjectAltName_type': 'IP', - + "openssl_constraint_random_id": "test-uuid-12345", + "CA_password": "test-password", + "p12_export_password": "test-p12-password", + "ipsec_pki_path": "/etc/ipsec.d", + "ipsec_config_path": "/etc/ipsec.d", + "subjectAltName": "IP:10.0.0.1,DNS:vpn.example.com", + "subjectAltName_type": "IP", # Ansible variables - 'ansible_default_ipv4': {'address': '10.0.0.1'}, - 'ansible_default_ipv6': {'address': '2600:3c01::f03c:91ff:fedf:3b2a'}, - 'ansible_distribution': 'Ubuntu', - 'ansible_distribution_version': '22.04', - 'ansible_date_time': {'epoch': '1234567890'}, - + "ansible_default_ipv4": {"address": "10.0.0.1"}, + "ansible_default_ipv6": {"address": "2600:3c01::f03c:91ff:fedf:3b2a"}, + "ansible_distribution": "Ubuntu", + "ansible_distribution_version": "22.04", + "ansible_date_time": {"epoch": "1234567890"}, # User management - 'users': ['alice', 'bob', 'charlie'], - 'all_users': ['alice', 'bob', 'charlie', 'david'], - + "users": ["alice", "bob", "charlie"], + "all_users": ["alice", "bob", "charlie", "david"], # Common variables - 'item': 'test-item', - 'algo_provider': 'local', - 'algo_server_name': 'algo-vpn', - 'dns_servers': ['1.1.1.1', '1.0.0.1'], - + "item": "test-item", + "algo_provider": "local", + "algo_server_name": "algo-vpn", + "dns_servers": ["1.1.1.1", "1.0.0.1"], # OpenSSL version for conditionals - 'openssl_version': '3.0.0', - + "openssl_version": "3.0.0", # IPsec configuration - 'certificate_validity_days': 3650, - 'ike_cipher': 'aes128gcm16-prfsha512-ecp256', - 'esp_cipher': 'aes128gcm16-ecp256', + "certificate_validity_days": 3650, + "ike_cipher": "aes128gcm16-prfsha512-ecp256", + "esp_cipher": "aes128gcm16-ecp256", } @@ -241,14 +236,14 @@ def validate_yaml_file(yaml_path, check_inline_comments_only=False): is_valid, error = validate_jinja2_expression(expr) if not is_valid: - line_num = find_line_number(content, expr['start']) + line_num = find_line_number(content, expr["start"]) error_msg = f"{yaml_path}:{line_num}: {error}" # Separate inline comment errors from other errors - if error and 'inline comment' in error.lower(): + if error and "inline comment" in error.lower(): inline_comment_errors.append(error_msg) # Show context for inline comment errors - if len(expr['full']) < 200: + if len(expr["full"]) < 200: inline_comment_errors.append(f" Expression: {expr['full'][:100]}...") elif not check_inline_comments_only: other_errors.append(error_msg) @@ -285,9 +280,9 @@ def test_regression_openssl_inline_comments(): # Test the problematic expression - should fail expr_with_comments = { - 'type': 'variable', - 'content': problematic_expr[2:-2], # Remove {{ }} - 'full': problematic_expr + "type": "variable", + "content": problematic_expr[2:-2], # Remove {{ }} + "full": problematic_expr, } is_valid, error = validate_jinja2_expression(expr_with_comments) assert not is_valid, "Should have detected inline comments in problematic expression" @@ -295,9 +290,9 @@ def test_regression_openssl_inline_comments(): # Test the fixed expression - should pass expr_fixed = { - 'type': 'variable', - 'content': fixed_expr[2:-2], # Remove {{ }} - 'full': fixed_expr + "type": "variable", + "content": fixed_expr[2:-2], # Remove {{ }} + "full": fixed_expr, } is_valid, error = validate_jinja2_expression(expr_fixed) assert is_valid, f"Fixed expression should pass but got error: {error}" @@ -321,39 +316,39 @@ def test_edge_cases_inline_comments(): ("{{ url + '/#anchor' }}", True, "URL fragment should pass"), ("{{ '#FF0000' }}", True, "Hex color code should pass"), ("{{ var }} # comment outside", True, "Comment outside expression should pass"), - ("""{{ [ + ( + """{{ [ 'item1', # comment here 'item2' - ] }}""", False, "Multi-line with inline comment should fail"), + ] }}""", + False, + "Multi-line with inline comment should fail", + ), ] for expr_str, should_pass, description in test_cases: # For the "comment outside" case, extract just the Jinja2 expression - if '{{' in expr_str and '#' in expr_str and expr_str.index('#') > expr_str.index('}}'): + if "{{" in expr_str and "#" in expr_str and expr_str.index("#") > expr_str.index("}}"): # Comment is outside the expression - extract just the expression part - match = re.search(r'(\{\{.+?\}\})', expr_str) + match = re.search(r"(\{\{.+?\}\})", expr_str) if match: actual_expr = match.group(1) - expr_type = 'variable' + expr_type = "variable" content = actual_expr[2:-2].strip() else: continue - elif expr_str.strip().startswith('{{'): - expr_type = 'variable' + elif expr_str.strip().startswith("{{"): + expr_type = "variable" content = expr_str.strip()[2:-2] actual_expr = expr_str.strip() - elif expr_str.strip().startswith('{%'): - expr_type = 'control' + elif expr_str.strip().startswith("{%"): + expr_type = "control" content = expr_str.strip()[2:-2] actual_expr = expr_str.strip() else: continue - expr = { - 'type': expr_type, - 'content': content, - 'full': actual_expr - } + expr = {"type": expr_type, "content": content, "full": actual_expr} is_valid, error = validate_jinja2_expression(expr) @@ -361,7 +356,9 @@ def test_edge_cases_inline_comments(): assert is_valid, f"{description}: {error}" else: assert not is_valid, f"{description}: Should have failed but passed" - assert "inline comment" in (error or "").lower(), f"{description}: Expected inline comment error, got: {error}" + assert "inline comment" in (error or "").lower(), ( + f"{description}: Expected inline comment error, got: {error}" + ) def test_yaml_files_no_inline_comments(): @@ -369,21 +366,21 @@ def test_yaml_files_no_inline_comments(): Test that all YAML files in the project don't contain inline comments in Jinja2 expressions. """ yaml_files = find_yaml_files_with_jinja2() - + all_inline_comment_errors = [] files_with_inline_comments = [] for yaml_file in yaml_files: has_inline_comments, inline_errors, _ = validate_yaml_file(yaml_file, check_inline_comments_only=True) - + if has_inline_comments: files_with_inline_comments.append(str(yaml_file)) all_inline_comment_errors.extend(inline_errors) # Assert no inline comments found assert not all_inline_comment_errors, ( - f"Found inline comments in {len(files_with_inline_comments)} files:\n" + - "\n".join(all_inline_comment_errors[:10]) # Show first 10 errors + f"Found inline comments in {len(files_with_inline_comments)} files:\n" + + "\n".join(all_inline_comment_errors[:10]) # Show first 10 errors ) @@ -391,14 +388,11 @@ def test_openssl_file_specifically(): """ Specifically test the OpenSSL file that had the original bug. """ - openssl_file = Path('roles/strongswan/tasks/openssl.yml') - + openssl_file = Path("roles/strongswan/tasks/openssl.yml") + if not openssl_file.exists(): pytest.skip(f"{openssl_file} not found") - + has_inline_comments, inline_errors, _ = validate_yaml_file(openssl_file) - - assert not has_inline_comments, ( - f"Found inline comments in {openssl_file}:\n" + - "\n".join(inline_errors) - ) + + assert not has_inline_comments, f"Found inline comments in {openssl_file}:\n" + "\n".join(inline_errors)