Phase 1: Enhance recipe generator with semantic validation
- Add pykickstart integration for semantic validation - Support version-specific validation (F42 for Fedora 43, DEVEL for rawhide) - Track deprecated/removed commands (authconfig, keyboard, langsupport, nfs, parted) - Add warning for deprecated commands, error for removed commands - Provide migration suggestions for each deprecated command - Add --strict flag for CI mode (warnings treated as errors) - Upgrade validate_recipe_semantic() with enhanced error reporting - Add _check_deprecated_commands() for command-level validation - Improve version extraction regex to handle _ and - separators - Add 5 new test cases for deprecated command detection and version extraction - Update tests/test_recipe_generator.py with comprehensive validation tests All 36 tests pass with semantic validation working for generated recipes.
This commit is contained in:
Lukas Greve
+192
-4
@@ -12,6 +12,59 @@ from pathlib import Path
|
||||
from typing import Dict, List, Optional, Any
|
||||
|
||||
|
||||
# Deprecated/removed command mappings for Fedora 43 (F42) and rawhide
|
||||
DEPRECATED_COMMANDS: Dict[str, Dict[str, str]] = {
|
||||
'authconfig': {
|
||||
'status': 'removed',
|
||||
'removed_in': 'F34',
|
||||
'alternative': 'authselect',
|
||||
'message': 'authconfig was removed in Fedora 34. Use authselect instead.'
|
||||
},
|
||||
'keyboard': {
|
||||
'status': 'deprecated',
|
||||
'deprecated_in': 'F18',
|
||||
'alternative': 'keyboard --vckeymap',
|
||||
'message': 'keyboard command is deprecated. Use keyboard --vckeymap instead.'
|
||||
},
|
||||
'langsupport': {
|
||||
'status': 'deprecated',
|
||||
'deprecated_in': 'F21',
|
||||
'alternative': 'lang',
|
||||
'message': 'langsupport is deprecated. Use lang command instead.'
|
||||
},
|
||||
'nfs': {
|
||||
'status': 'deprecated',
|
||||
'deprecated_in': 'F23',
|
||||
'alternative': 'repo --name=nfs',
|
||||
'message': 'nfs command is deprecated. Use repo command instead.'
|
||||
},
|
||||
'parted': {
|
||||
'status': 'deprecated',
|
||||
'deprecated_in': 'F13',
|
||||
'alternative': 'part',
|
||||
'message': 'parted command is deprecated. Use part command instead.'
|
||||
},
|
||||
}
|
||||
|
||||
|
||||
def _import_pykickstart():
|
||||
"""Import pykickstart modules, returns None if not available."""
|
||||
try:
|
||||
from pykickstart.parser import KickstartParser
|
||||
from pykickstart.version import makeVersion
|
||||
from pykickstart.version import DEVEL
|
||||
from pykickstart.errors import KickstartParseError, KickstartError
|
||||
return {
|
||||
'parser': KickstartParser,
|
||||
'makeVersion': makeVersion,
|
||||
'DEVEL': DEVEL,
|
||||
'KickstartParseError': KickstartParseError,
|
||||
'KickstartError': KickstartError
|
||||
}
|
||||
except ImportError:
|
||||
return None
|
||||
|
||||
|
||||
class RecipeGenerator:
|
||||
"""Generate kickstart recipes from templates and modifiers."""
|
||||
|
||||
@@ -21,6 +74,13 @@ class RecipeGenerator:
|
||||
self.ingredients_dir = self.project_root / ingredients_dir
|
||||
self.templates = self.load_templates(templates_file)
|
||||
|
||||
def get_ksversion(self, version: str) -> Optional[str]:
|
||||
"""Map Phyllome OS version to pykickstart version string."""
|
||||
if version == 'rawhide':
|
||||
return None
|
||||
else:
|
||||
return f'F{int(version) - 1}'
|
||||
|
||||
def load_templates(self, path: Path) -> Dict:
|
||||
"""Load recipe templates from YAML file."""
|
||||
try:
|
||||
@@ -217,6 +277,88 @@ class RecipeGenerator:
|
||||
|
||||
return issues
|
||||
|
||||
def validate_recipe_semantic(self, content: str, version: str) -> List[str]:
|
||||
"""Validate recipe using pykickstart parser with version-specific checks."""
|
||||
issues = []
|
||||
|
||||
modules = _import_pykickstart()
|
||||
if modules is None:
|
||||
issues.append("Warning: pykickstart not installed, skipping semantic validation")
|
||||
return issues
|
||||
|
||||
KickstartParser = modules['parser']
|
||||
makeVersion = modules['makeVersion']
|
||||
KickstartParseError = modules['KickstartParseError']
|
||||
KickstartError = modules['KickstartError']
|
||||
|
||||
ks_version_str = self.get_ksversion(version)
|
||||
if ks_version_str:
|
||||
ks_version = makeVersion(ks_version_str)
|
||||
else:
|
||||
ks_version = makeVersion(modules['DEVEL'])
|
||||
|
||||
try:
|
||||
parser = KickstartParser(ks_version)
|
||||
parser.readKickstartFromString(content)
|
||||
except KickstartParseError as e:
|
||||
issues.append(f"Syntax error line {e.lineno}: {e.message}")
|
||||
except KickstartError as e:
|
||||
issues.append(f"Validation error: {str(e)}")
|
||||
except Exception as e:
|
||||
issues.append(f"Unexpected error during parsing: {str(e)}")
|
||||
|
||||
# Check for deprecated commands in the content
|
||||
issues.extend(self._check_deprecated_commands(content))
|
||||
|
||||
return issues
|
||||
|
||||
def _check_deprecated_commands(self, content: str) -> List[str]:
|
||||
"""Check for deprecated and removed commands with suggestions."""
|
||||
issues = []
|
||||
|
||||
for line_num, line in enumerate(content.split('\n'), start=1):
|
||||
# Skip comments and empty lines
|
||||
stripped = line.strip()
|
||||
if not stripped or stripped.startswith('#'):
|
||||
continue
|
||||
|
||||
# Extract command (first word after % if in section, or just the first word)
|
||||
if stripped.startswith('%'):
|
||||
continue # Skip section headers
|
||||
|
||||
parts = stripped.split()
|
||||
if not parts:
|
||||
continue
|
||||
|
||||
cmd = parts[0]
|
||||
|
||||
if cmd in DEPRECATED_COMMANDS:
|
||||
cmd_info = DEPRECATED_COMMANDS[cmd]
|
||||
status = cmd_info['status']
|
||||
msg = cmd_info['message']
|
||||
|
||||
if status == 'removed':
|
||||
issues.append(f"ERROR: Line {line_num}: {msg}")
|
||||
else:
|
||||
issues.append(f"Warning: Line {line_num}: {msg}")
|
||||
|
||||
return issues
|
||||
|
||||
def extract_version(self, content: str, filename: str) -> Optional[str]:
|
||||
"""Extract Fedora version from recipe content or filename."""
|
||||
import re
|
||||
filename_match = re.search(r'(?:_|-)(43|rawhide)(?:_|-|.cfg|.yaml|$)', filename)
|
||||
if filename_match:
|
||||
return filename_match.group(1)
|
||||
|
||||
for line in content.split('\n'):
|
||||
if 'core-fedora-repo-43' in line:
|
||||
return '43'
|
||||
elif 'core-fedora-repo-rawhide' in line:
|
||||
return 'rawhide'
|
||||
|
||||
return None
|
||||
|
||||
def generate_filename(self, recipe_type: str, version: str, **modifiers) -> str:
|
||||
"""Generate recipe filename from parameters."""
|
||||
# Map modifiers to filename components
|
||||
@@ -310,6 +452,11 @@ def main():
|
||||
nargs='+',
|
||||
help='Validate recipe files')
|
||||
|
||||
# Strict mode for CI
|
||||
parser.add_argument('--strict',
|
||||
action='store_true',
|
||||
help='Treat warnings as errors (CI mode)')
|
||||
|
||||
args = parser.parse_args()
|
||||
|
||||
# Initialize generator
|
||||
@@ -323,6 +470,16 @@ def main():
|
||||
with open(recipe_path) as f:
|
||||
content = f.read()
|
||||
issues = generator.validate_recipe(content)
|
||||
|
||||
# Extract version and perform semantic validation
|
||||
filename = Path(recipe_path).stem
|
||||
version = generator.extract_version(content, filename)
|
||||
if version:
|
||||
semantic_issues = generator.validate_recipe_semantic(content, version)
|
||||
issues.extend(semantic_issues)
|
||||
else:
|
||||
issues.append("Warning: Could not determine version, skipping semantic validation")
|
||||
|
||||
if issues:
|
||||
all_issues.append((recipe_path, issues))
|
||||
except FileNotFoundError:
|
||||
@@ -330,11 +487,38 @@ def main():
|
||||
sys.exit(2)
|
||||
|
||||
if all_issues:
|
||||
print("Validation issues found:", file=sys.stderr)
|
||||
print("=== Recipe Validation Report ===", file=sys.stderr)
|
||||
for path, issues in all_issues:
|
||||
print(f"\n{path}:", file=sys.stderr)
|
||||
error_count = sum(1 for i in issues if 'ERROR' in i)
|
||||
warning_count = sum(1 for i in issues if 'Warning:' in i)
|
||||
|
||||
if error_count > 0:
|
||||
for issue in issues:
|
||||
print(f" - {issue}", file=sys.stderr)
|
||||
if 'ERROR' in issue:
|
||||
print(f" {issue}", file=sys.stderr)
|
||||
|
||||
if warning_count > 0:
|
||||
for issue in issues:
|
||||
if 'Warning:' in issue:
|
||||
print(f" {issue}", file=sys.stderr)
|
||||
|
||||
if error_count == 0 and warning_count == 0:
|
||||
print(f" No issues found (file exists)", file=sys.stderr)
|
||||
|
||||
print(f"\nSummary:", file=sys.stderr)
|
||||
print(f" - {len(all_issues)} recipe(s) checked", file=sys.stderr)
|
||||
|
||||
total_errors = sum(len([i for i in issues if 'ERROR' in i]) for _, issues in all_issues)
|
||||
total_warnings = sum(len([i for i in issues if 'Warning:' in i]) for _, issues in all_issues)
|
||||
print(f" - {total_errors} error(s), {total_warnings} warning(s)", file=sys.stderr)
|
||||
|
||||
# Strict mode: treat warnings as errors
|
||||
if args.strict and total_warnings > 0:
|
||||
print("\nStrict mode: Warnings treated as errors", file=sys.stderr)
|
||||
sys.exit(1)
|
||||
|
||||
if total_errors > 0:
|
||||
sys.exit(1)
|
||||
else:
|
||||
print("All recipes validated successfully")
|
||||
@@ -375,7 +559,9 @@ def main():
|
||||
|
||||
if args.validate and not args.dry_run:
|
||||
issues = generator.validate_recipe(content)
|
||||
if issues:
|
||||
semantic_issues = generator.validate_recipe_semantic(content, version)
|
||||
all_issues = issues + semantic_issues
|
||||
if all_issues:
|
||||
print(f"Validation issues for {recipe_type} {version}:", file=sys.stderr)
|
||||
for issue in issues:
|
||||
print(f" - {issue}", file=sys.stderr)
|
||||
@@ -409,7 +595,9 @@ def main():
|
||||
|
||||
if args.validate:
|
||||
issues = generator.validate_recipe(content)
|
||||
if issues:
|
||||
semantic_issues = generator.validate_recipe_semantic(content, args.version)
|
||||
all_issues = issues + semantic_issues
|
||||
if all_issues:
|
||||
print("Validation issues:", file=sys.stderr)
|
||||
for issue in issues:
|
||||
print(f" - {issue}", file=sys.stderr)
|
||||
|
||||
@@ -225,3 +225,111 @@ class TestRecipeGenerator:
|
||||
manifest = {'recipes': [{'name': 'test', 'variants': [{}]}]}
|
||||
errors = self.generator.validate_manifest(manifest)
|
||||
assert any('version' in error for error in errors)
|
||||
|
||||
def test_get_ksversion_43(self):
|
||||
"""Test version mapping for Fedora 43."""
|
||||
version = self.generator.get_ksversion('43')
|
||||
assert version == 'F42'
|
||||
|
||||
def test_get_ksversion_rawhide(self):
|
||||
"""Test version mapping for rawhide."""
|
||||
version = self.generator.get_ksversion('rawhide')
|
||||
assert version is None
|
||||
|
||||
def test_validate_recipe_semantic_f42(self):
|
||||
"""Test semantic validation with valid recipe for F42."""
|
||||
content = """text
|
||||
poweroff
|
||||
zerombr
|
||||
clearpart --all --initlabel
|
||||
part /boot/efi --fstype="efi" --size=512
|
||||
part / --fstype="ext4" --grow
|
||||
%packages
|
||||
@base-graphical
|
||||
%end
|
||||
"""
|
||||
issues = self.generator.validate_recipe_semantic(content, '43')
|
||||
# Should have no syntax errors
|
||||
assert not any('Syntax' in issue for issue in issues)
|
||||
assert not any('Validation' in issue for issue in issues)
|
||||
|
||||
def test_validate_recipe_semantic_empty(self):
|
||||
"""Test semantic validation with empty content."""
|
||||
content = ""
|
||||
issues = self.generator.validate_recipe_semantic(content, '43')
|
||||
# Empty content should fail parsing but not crash
|
||||
assert len(issues) >= 0 # At least info about empty content
|
||||
|
||||
def test_validate_recipe_semantic_invalid_syntax(self):
|
||||
"""Test semantic validation detects invalid syntax."""
|
||||
content = """text
|
||||
invalidcmd --option=value
|
||||
%packages
|
||||
@base-graphical
|
||||
%end
|
||||
"""
|
||||
issues = self.generator.validate_recipe_semantic(content, '43')
|
||||
# Should detect invalid command
|
||||
has_syntax_error = any('Syntax' in issue or 'invalidcmd' in issue.lower()
|
||||
for issue in issues)
|
||||
# Or pykickstart warning if not available
|
||||
has_warning = any('Warning' in issue or 'pykickstart' in issue.lower()
|
||||
for issue in issues)
|
||||
assert has_syntax_error or has_warning
|
||||
|
||||
|
||||
def test_validate_recipe_full_validation(self):
|
||||
"""Test full validation combines file and semantic checks."""
|
||||
content = self.generator.generate_recipe('virtual-desktop', '43',
|
||||
desktop='gnome',
|
||||
storage='standard',
|
||||
security='secure')
|
||||
issues = self.generator.validate_recipe(content)
|
||||
# Check ingredient existence
|
||||
assert len([i for i in issues if 'Missing' in i]) == 0, \
|
||||
f"Found missing ingredients: {issues}"
|
||||
|
||||
def test_check_deprecated_removed_command(self):
|
||||
"""Test detection of removed deprecated command."""
|
||||
content = """text
|
||||
authconfig --enableshadow
|
||||
%packages
|
||||
@core
|
||||
%end
|
||||
"""
|
||||
issues = self.generator.validate_recipe_semantic(content, '43')
|
||||
# Should detect authconfig as removed
|
||||
has_removed = any('ERROR' in i and 'authconfig' in i for i in issues)
|
||||
assert has_removed
|
||||
|
||||
def test_check_deprecated_warning_command(self):
|
||||
"""Test detection of deprecated command as warning."""
|
||||
content = """text
|
||||
keyboard --evgrd
|
||||
%packages
|
||||
@core
|
||||
%end
|
||||
"""
|
||||
issues = self.generator.validate_recipe_semantic(content, '43')
|
||||
# Should detect keyboard as deprecated (warning)
|
||||
has_warning = any('Warning' in i and 'keyboard' in i for i in issues)
|
||||
assert has_warning
|
||||
|
||||
def test_extract_version_with_dash(self):
|
||||
"""Test version extraction from filename with dash."""
|
||||
filename = 'test-43.cfg'
|
||||
version = self.generator.extract_version('', filename)
|
||||
assert version == '43'
|
||||
|
||||
def test_extract_version_with_underscore(self):
|
||||
"""Test version extraction from filename with underscore."""
|
||||
filename = 'test_43.cfg'
|
||||
version = self.generator.extract_version('', filename)
|
||||
assert version == '43'
|
||||
|
||||
def test_extract_version_from_content(self):
|
||||
"""Test version extraction from content."""
|
||||
content = "%include ../ingredients/core-fedora-repo-43.cfg"
|
||||
version = self.generator.extract_version(content, 'test.cfg')
|
||||
assert version == '43'
|
||||
|
||||
|
||||
Reference in New Issue
Block a user