From 8db37972d7f44c869c9686f3164081338d3f0d30 Mon Sep 17 00:00:00 2001 From: Andrej Luptak Date: Tue, 14 Jul 2026 15:53:57 +0200 Subject: [PATCH 1/7] feat: repo file fields sanitization Little sanitization for repo files during parsing process. Supporting test files taken and modified from vmaas-data repository. Part of RHINENG-13437 --- vmaas/reposcan/mnm.py | 8 + .../reposcan/repodata/metadata_validators.py | 131 +++++++++++++++ vmaas/reposcan/repodata/primary.py | 53 ++++-- vmaas/reposcan/repodata/primary_db.py | 42 +++-- .../reposcan/repodata/test/test_updateinfo.py | 40 +++++ .../reposcan/repodata/test/test_validators.py | 156 ++++++++++++++++++ vmaas/reposcan/repodata/updateinfo.py | 137 +++++++++------ .../repodata/primary_validation_test.xml | 75 +++++++++ .../update_validation_errata_test.xml | 83 ++++++++++ .../repodata/update_validation_test.xml | 23 +++ .../test_data/repodata/updateinfo.xml | 4 +- 11 files changed, 680 insertions(+), 72 deletions(-) create mode 100644 vmaas/reposcan/repodata/metadata_validators.py create mode 100644 vmaas/reposcan/repodata/test/test_validators.py create mode 100644 vmaas/reposcan/test_data/repodata/primary_validation_test.xml create mode 100644 vmaas/reposcan/test_data/repodata/update_validation_errata_test.xml create mode 100644 vmaas/reposcan/test_data/repodata/update_validation_test.xml diff --git a/vmaas/reposcan/mnm.py b/vmaas/reposcan/mnm.py index 0099c5396..229401b3f 100644 --- a/vmaas/reposcan/mnm.py +++ b/vmaas/reposcan/mnm.py @@ -46,3 +46,11 @@ REPOS_TO_CLEANUP = Gauge('vmaas_reposcan_repos_cleanup', '# of repos to cleanup from DB') CERT_EXPIRATION_WARNING = Gauge('vmaas_reposcan_certificate_expiration_days', 'Days until CDN certificate expiration', ['cert_name']) + +# Data integrity metrics +VALIDATION_FAILED_ITEMS = Counter('vmaas_reposcan_validation_failed_items', + 'Number of items that failed validation', + ['metadata_type', 'field']) +VALIDATION_TOTAL_ITEMS = Counter('vmaas_reposcan_validation_total_items', + 'Total number of items processed for validation', + ['metadata_type']) diff --git a/vmaas/reposcan/repodata/metadata_validators.py b/vmaas/reposcan/repodata/metadata_validators.py new file mode 100644 index 000000000..613cb55ca --- /dev/null +++ b/vmaas/reposcan/repodata/metadata_validators.py @@ -0,0 +1,131 @@ +""" +Input validation utilities for repository metadata. +""" +import re + +# Validation patterns +CVE_PATTERN = re.compile(r'^CVE-\d{4}-\d+$') +BUGZILLA_ID_PATTERN = re.compile(r'^\d+$') +PACKAGE_NAME_PATTERN = re.compile(r'^[a-zA-Z0-9._+\-]+$') +RELEASE_PATTERN = re.compile(r'^[a-zA-Z0-9._+\-~]+$') +VERSION_PATTERN = re.compile(r'^[a-zA-Z0-9._+\-~^]+$') + +# Architecture whitelist +VALID_ARCHITECTURES = { + 'noarch', + 'i386', + 'i486', + 'i586', + 'i686', + 'alpha', + 'alphaev6', + 'ia64', + 'sparc', + 'sparcv9', + 'sparc64', + 's390', + 'athlon', + 's390x', + 'ppc', + 'ppc64', + 'ppc64le', + 'pSeries', + 'iSeries', + 'x86_64', + 'ppc64iseries', + 'ppc64pseries', + 'ia32e', + 'amd64', + 'aarch64', + 'armv7hnl', + 'armv7hl', + 'armv7l', + 'armv6hl', + 'armv6l', + 'armv5tel', + 'src', +} + + +class ValidationError(Exception): + """Raised when validation fails.""" + + +def validate_cve_id(cve_id): + """Validate CVE ID format""" + cve_id = str(cve_id).strip() + + if not CVE_PATTERN.match(cve_id): + raise ValidationError(f"Invalid CVE ID format: {cve_id}") + + return cve_id + + +def validate_bugzilla_id(bugzilla_id): + """Validate Bugzilla ticket number format.""" + bugzilla_id = str(bugzilla_id).strip() + + if not BUGZILLA_ID_PATTERN.match(bugzilla_id): + raise ValidationError(f"Invalid Bugzilla ID format: {bugzilla_id}") + + return bugzilla_id + + +def validate_architecture(arch): + """Validate architecture name against whitelist""" + arch = str(arch).strip() + + if arch not in VALID_ARCHITECTURES: + raise ValidationError(f"Unknown architecture: {arch}") + + return arch + + +def validate_package_name(name): + """Validate package name format""" + name = str(name).strip() + + if not PACKAGE_NAME_PATTERN.match(name): + raise ValidationError(f"Invalid package name format: {name}") + + return name + + +def validate_version(version): + """Validate package version string""" + version = str(version).strip() + + if not VERSION_PATTERN.match(version): + raise ValidationError(f"Invalid version format: {version}") + + return version + + +def validate_release(release): + """Validate package release string""" + release = str(release).strip() + + if not RELEASE_PATTERN.match(release): + raise ValidationError(f"Invalid release format: {release}") + + return release + + +# Field type to validator function mapping +FIELD_VALIDATORS = { + 'name': validate_package_name, + 'arch': validate_architecture, + 'version': validate_version, + 'release': validate_release, + 'cve_id': validate_cve_id, + 'bugzilla_id': validate_bugzilla_id, +} + + +def validate_field(value, field_type): + """Unified validation function that validates a field based on its type""" + validator = FIELD_VALIDATORS.get(field_type) + if not validator: + raise ValueError(f"Unknown field type: {field_type}") + + return validator(value) diff --git a/vmaas/reposcan/repodata/primary.py b/vmaas/reposcan/repodata/primary.py index 8c00a02b1..5f8cb1052 100644 --- a/vmaas/reposcan/repodata/primary.py +++ b/vmaas/reposcan/repodata/primary.py @@ -5,6 +5,9 @@ import xml.etree.ElementTree as eT from vmaas.common.string import text_strip +from vmaas.common.logging_utils import get_logger +from vmaas.reposcan.repodata.metadata_validators import validate_field, ValidationError +from vmaas.reposcan.mnm import VALIDATION_FAILED_ITEMS, VALIDATION_TOTAL_ITEMS NS = {"primary": "http://linux.duke.edu/metadata/common", "rpm": "http://linux.duke.edu/metadata/rpm"} @@ -13,6 +16,7 @@ class PrimaryMD: """Class parsing Primary XML. Takes filename in the constructor.""" def __init__(self, filename): + self.logger = get_logger(__name__) self.package_count = 0 self.packages = [] root = None @@ -22,20 +26,47 @@ def __init__(self, filename): self.package_count = int(elem.get("packages")) elif elem.tag == "{%s}package" % NS["primary"] and event == "end": if elem.get("type") == "rpm": - package = {} - package["name"] = text_strip(elem.find("primary:name", NS)) - evr = elem.find("primary:version", NS) - package["epoch"] = evr.get("epoch") - package["ver"] = evr.get("ver") - package["rel"] = evr.get("rel") - package["arch"] = text_strip(elem.find("primary:arch", NS)) - package["summary"] = text_strip(elem.find("primary:summary", NS)) - package["description"] = text_strip(elem.find("primary:description", NS)) - package["srpm"] = elem.find("primary:format", NS).find("rpm:sourcerpm", NS).text - self.packages.append(package) + VALIDATION_TOTAL_ITEMS.labels(metadata_type='primary').inc() + try: + package = self._parse_package(elem) + self.packages.append(package) + except ValidationError as err: + self.logger.warning("Validation failed, skipped package: %s", str(err)) # Clear the XML tree continuously root.clear() + def _validate(self, value, field_type): + """Validate a field and track metrics on failure.""" + try: + return validate_field(value, field_type) + except ValidationError: + VALIDATION_FAILED_ITEMS.labels(metadata_type='primary', field=field_type).inc() + raise + + def _parse_package(self, elem): + """Parse and validate a single package element.""" + # Parse raw values + name_raw = text_strip(elem.find("primary:name", NS)) + evr = elem.find("primary:version", NS) + arch_raw = text_strip(elem.find("primary:arch", NS)) + summary_raw = text_strip(elem.find("primary:summary", NS)) + description_raw = text_strip(elem.find("primary:description", NS)) + srpm_raw = elem.find("primary:format", NS).find("rpm:sourcerpm", NS).text + + # Validate and build package dict + package = { + "name": self._validate(name_raw, 'name'), + "epoch": evr.get("epoch"), + "ver": self._validate(evr.get("ver"), 'version'), + "rel": self._validate(evr.get("rel"), 'release'), + "arch": self._validate(arch_raw, 'arch'), + "summary": summary_raw, + "description": description_raw, + "srpm": srpm_raw, + } + + return package + def get_package_count(self): """Returns count of packages in Primary file.""" return self.package_count diff --git a/vmaas/reposcan/repodata/primary_db.py b/vmaas/reposcan/repodata/primary_db.py index 05c451fdc..d60d2b932 100644 --- a/vmaas/reposcan/repodata/primary_db.py +++ b/vmaas/reposcan/repodata/primary_db.py @@ -3,11 +3,16 @@ """ import sqlite3 +from vmaas.common.logging_utils import get_logger +from vmaas.reposcan.repodata.metadata_validators import validate_field, ValidationError +from vmaas.reposcan.mnm import VALIDATION_FAILED_ITEMS, VALIDATION_TOTAL_ITEMS + class PrimaryDatabaseMD: """Class parsing Primary SQLite. Takes filename in the constructor.""" def __init__(self, filename): + self.logger = get_logger(__name__) self.packages = [] conn = sqlite3.connect(filename) conn.row_factory = sqlite3.Row @@ -17,18 +22,35 @@ def __init__(self, filename): summary, description, rpm_sourcerpm from packages""" for row in cur.execute(sql): - self.packages.append({ - "name": row["name"], - "epoch": row["epoch"], - "ver": row["version"], - "rel": row["release"], - "arch": row["arch"], - "summary": row["summary"], - "description": row["description"], - "srpm": row["rpm_sourcerpm"] - }) + VALIDATION_TOTAL_ITEMS.labels(metadata_type='primary_db').inc() + try: + package = self._parse_package(row) + self.packages.append(package) + except ValidationError as err: + self.logger.warning("Validation failed, skipped package: %s", str(err)) conn.close() + def _validate(self, value, field_type): + """Validate a field and track metrics on failure""" + try: + return validate_field(value, field_type) + except ValidationError: + VALIDATION_FAILED_ITEMS.labels(metadata_type='primary_db', field=field_type).inc() + raise + + def _parse_package(self, row): + """Parse and validate a single package row""" + return { + "name": self._validate(row["name"], 'name'), + "epoch": row["epoch"], + "ver": self._validate(row["version"], 'version'), + "rel": self._validate(row["release"], 'release'), + "arch": self._validate(row["arch"], 'arch'), + "summary": row["summary"], + "description": row["description"], + "srpm": row["rpm_sourcerpm"] + } + def get_package_count(self): """Returns count of packages in Primary SQLite file.""" return len(self.packages) diff --git a/vmaas/reposcan/repodata/test/test_updateinfo.py b/vmaas/reposcan/repodata/test/test_updateinfo.py index 350ee47f6..392d93ce0 100644 --- a/vmaas/reposcan/repodata/test/test_updateinfo.py +++ b/vmaas/reposcan/repodata/test/test_updateinfo.py @@ -4,6 +4,7 @@ from xml.etree.ElementTree import ParseError import pytest from vmaas.reposcan.repodata.updateinfo import UpdateInfoMD +from vmaas.reposcan.repodata.metadata_validators import ValidationError, validate_field KNOWN_UPDATE_TYPES = ("security", "bugfix", "enhancement", "newpackage") @@ -82,3 +83,42 @@ def test_updates(self, updateinfo): # Test fields of updates in list for update in updates: self._test_update(update) + + def test_valid_updates_pass(self): + """Test that valid errata are imported.""" + ui_inst = UpdateInfoMD("test_data/repodata/update_validation_errata_test.xml") + updates = ui_inst.list_updates() + imported_ids = {u["id"] for u in updates} + assert "vmaas_test_x86_64_1" in imported_ids + assert "vmaas_test_i386_1.1" in imported_ids + + def test_skip_invalid_errata(self, caplog): + """Test that errata with invalid fields are skipped.""" + ui_inst = UpdateInfoMD("test_data/repodata/update_validation_errata_test.xml") + updates = ui_inst.list_updates() + imported_ids = {u["id"] for u in updates} + # 4 errata total, 2 invalid (bad arch + bad CVE) should be skipped + assert len(updates) == 2 + assert "vmaas_test_x86_64_2" not in imported_ids + assert "vmaas_test_i386_2.1" not in imported_ids + assert caplog.text.count("Validation failed") == 2 + + def test_valid_bugzilla_ref(self): + """Test that type='bugzilla' with numeric id is validated and imported.""" + ui_inst = UpdateInfoMD("test_data/repodata/update_validation_errata_test.xml") + updates = ui_inst.list_updates() + update = next(u for u in updates if u["id"] == "vmaas_test_i386_1.1") + assert len(update["references"]) == 1 + assert update["references"][0]["id"] == "999999" + assert update["references"][0]["type"] == "bugzilla" + + def test_invalid_raises(self): + """Test that validate_field raises ValidationError for invalid input.""" + with pytest.raises(ValidationError): + validate_field("INVALID_ARCH", 'arch') + with pytest.raises(ValidationError): + validate_field("CVE-INVALID", 'cve_id') + with pytest.raises(ValidationError): + validate_field("not-a-number", 'bugzilla_id') + with pytest.raises(ValidationError): + validate_field("package with spaces", 'name') diff --git a/vmaas/reposcan/repodata/test/test_validators.py b/vmaas/reposcan/repodata/test/test_validators.py new file mode 100644 index 000000000..bd2b24fe6 --- /dev/null +++ b/vmaas/reposcan/repodata/test/test_validators.py @@ -0,0 +1,156 @@ +""" +Unit tests for input validation. +""" +import pytest +from vmaas.reposcan.repodata.metadata_validators import ( + validate_cve_id, + validate_bugzilla_id, + validate_architecture, + validate_package_name, + validate_version, + validate_release, + ValidationError, +) + + +class TestCVEValidation: + """Test CVE ID validation.""" + + def test_valid_cve(self): + """Test valid CVE IDs.""" + assert validate_cve_id("CVE-2024-1234") == "CVE-2024-1234" + assert validate_cve_id("CVE-2023-12345678") == "CVE-2023-12345678" + assert validate_cve_id(" CVE-2024-1234 ") == "CVE-2024-1234" # Strips whitespace + + def test_invalid_cve_format(self): + """Test invalid CVE ID formats.""" + with pytest.raises(ValidationError, match="Invalid CVE ID format"): + validate_cve_id("INVALID-2024-1234") + + with pytest.raises(ValidationError, match="Invalid CVE ID format"): + validate_cve_id("CVE-2024") # Missing ID number + + with pytest.raises(ValidationError, match="Invalid CVE ID format"): + validate_cve_id("CVE-24-1234") # Year too short + + with pytest.raises(ValidationError, match="Invalid CVE ID format"): + validate_cve_id("") + + with pytest.raises(ValidationError, match="Invalid CVE ID format"): + validate_cve_id(None) + + +class TestBugzillaValidation: + """Test Bugzilla ID validation.""" + + def test_valid_bugzilla_id(self): + """Test valid Bugzilla ticket numbers.""" + assert validate_bugzilla_id("1493960") == "1493960" + assert validate_bugzilla_id("999999") == "999999" + assert validate_bugzilla_id(" 12345 ") == "12345" + assert validate_bugzilla_id(12345) == "12345" # Casts non-str to str + + def test_invalid_bugzilla_format(self): + """Test invalid Bugzilla ID formats.""" + with pytest.raises(ValidationError, match="Invalid Bugzilla ID format"): + validate_bugzilla_id("CVE-2024-1234") + + with pytest.raises(ValidationError, match="Invalid Bugzilla ID format"): + validate_bugzilla_id("not-a-number") + + with pytest.raises(ValidationError, match="Invalid Bugzilla ID format"): + validate_bugzilla_id("") + + with pytest.raises(ValidationError, match="Invalid Bugzilla ID format"): + validate_bugzilla_id(None) + + +class TestArchitectureValidation: + """Test architecture validation.""" + + def test_valid_architectures(self): + """Test valid architectures.""" + assert validate_architecture("x86_64") == "x86_64" + assert validate_architecture("aarch64") == "aarch64" + assert validate_architecture("noarch") == "noarch" + assert validate_architecture(" x86_64 ") == "x86_64" + + def test_invalid_architecture(self): + """Test invalid/unknown architectures.""" + with pytest.raises(ValidationError, match="Unknown architecture"): + validate_architecture("invalid_arch") + + with pytest.raises(ValidationError, match="Unknown architecture"): + validate_architecture("ARM64") # Case sensitive + + with pytest.raises(ValidationError, match="Unknown architecture"): + validate_architecture("") + + +class TestPackageNameValidation: + """Test package name validation.""" + + def test_valid_package_names(self): + """Test valid package names.""" + assert validate_package_name("kernel") == "kernel" + assert validate_package_name("python3.11") == "python3.11" + assert validate_package_name("gcc-c++") == "gcc-c++" + assert validate_package_name("lib_test-1.0") == "lib_test-1.0" + + def test_invalid_package_names(self): + """Test invalid package names.""" + with pytest.raises(ValidationError, match="Invalid package name format"): + validate_package_name("package with spaces") + + with pytest.raises(ValidationError, match="Invalid package name format"): + validate_package_name("package@special") + + with pytest.raises(ValidationError, match="Invalid package name format"): + validate_package_name("") + + +class TestVersionValidation: + """Test version validation.""" + + def test_valid_versions(self): + """Test valid version strings.""" + assert validate_version("1.0.0") == "1.0.0" + assert validate_version("5.14.0") == "5.14.0" + assert validate_version("2.0~rc1") == "2.0~rc1" # Tilde for pre-releases + assert validate_version("1.el8_5") == "1.el8_5" + assert validate_version("1.22^20260313git904aa67") == "1.22^20260313git904aa67" + assert validate_version("0^20260319.75a7967") == "0^20260319.75a7967" + assert validate_version("1.0.0.101^git20260123.39e7bd0") == "1.0.0.101^git20260123.39e7bd0" + + def test_invalid_versions(self): + """Test invalid version strings.""" + with pytest.raises(ValidationError, match="Invalid"): + validate_version("version with spaces") + + with pytest.raises(ValidationError, match="Invalid"): + validate_version("1.0@invalid") + + with pytest.raises(ValidationError, match="Invalid"): + validate_version("") + + +class TestReleaseValidation: + """Test release validation.""" + + def test_valid_releases(self): + """Test valid release strings.""" + assert validate_release("1.el8") == "1.el8" + assert validate_release("2.fc27") == "2.fc27" + assert validate_release("1.module+el8+2517+b1471f1c") == "1.module+el8+2517+b1471f1c" + assert validate_release("1~bootstrap") == "1~bootstrap" + + def test_invalid_releases(self): + """Test invalid release strings.""" + with pytest.raises(ValidationError, match="Invalid"): + validate_release("1.22^20260313git904aa67") # ^ is version-only + + with pytest.raises(ValidationError, match="Invalid"): + validate_release("rel with spaces") + + with pytest.raises(ValidationError, match="Invalid"): + validate_release("") diff --git a/vmaas/reposcan/repodata/updateinfo.py b/vmaas/reposcan/repodata/updateinfo.py index 36939cb2a..7bb5356bd 100644 --- a/vmaas/reposcan/repodata/updateinfo.py +++ b/vmaas/reposcan/repodata/updateinfo.py @@ -10,6 +10,9 @@ from vmaas.common.string import text_strip from vmaas.common.utc import UTC from vmaas.common.strtobool import strtobool +from vmaas.common.logging_utils import get_logger +from vmaas.reposcan.repodata.metadata_validators import validate_field, ValidationError +from vmaas.reposcan.mnm import VALIDATION_FAILED_ITEMS, VALIDATION_TOTAL_ITEMS DATETIME_PATTERNS = { "%Y-%m-%d %H:%M:%S": re.compile(r"^\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}$"), @@ -22,58 +25,94 @@ class UpdateInfoMD: """Class parsing UpdateInfo XML. Takes filename in the constructor.""" def __init__(self, filename): + self.logger = get_logger(__name__) self.updates = [] root = None for event, elem in eT.iterparse(filename, events=("start", "end")): if elem.tag == "updates" and event == "start": root = elem elif elem.tag == "update" and event == "end": - update = {} - update["from"] = elem.get("from") - update["status"] = elem.get("status") - update["version"] = elem.get("version") - update["type"] = elem.get("type") - update["id"] = text_strip(elem.find("id")) - update["title"] = text_strip(elem.find("title")) - update["reboot"] = self._parse_reboot_suggested(elem) - - # Optional fields - text_elements = ["summary", "rights", "description", "release", "solution", "severity"] - date_elements = ["issued", "updated"] - for field in text_elements + date_elements: - found = elem.find(field) - if found is not None and field in text_elements: - content = text_strip(found) - update[field] = content if content else None - elif found is not None and field in date_elements: - update[field] = self._get_dt(found.get("date")) - else: - update[field] = None - - references = elem.find("references") - update["references"] = [] - for reference in references.findall("reference"): - update["references"].append({ - "href": reference.get("href"), - "id": reference.get("id"), - "type": reference.get("type"), - "title": reference.get("title") - }) - - pkglist = elem.find("pkglist") - update["pkglist"] = [] - if pkglist is not None: - for collection in pkglist.findall("collection"): - module = collection.find("module") - for pkg in collection.findall("package"): - rec = self._process_package(pkg, module) - if rec not in update["pkglist"]: - update["pkglist"].append(rec) - - self.updates.append(update) + VALIDATION_TOTAL_ITEMS.labels(metadata_type='updateinfo').inc() + try: + update = self._parse_update(elem) + self.updates.append(update) + except ValidationError as err: + self.logger.warning("Validation failed, skipped: %s", str(err)) # Clear the XML tree continuously root.clear() + def _validate(self, value, field_type): + """Validate a field and track metrics on failure.""" + try: + return validate_field(value, field_type) + except ValidationError: + VALIDATION_FAILED_ITEMS.labels(metadata_type='updateinfo', field=field_type).inc() + raise + + def _parse_update(self, elem): + """Parse and validate a single update/errata element. + + Errata header and text fields are stored as-is. Validation is applied to + pkglist NEVRA (package matching) and reference ids by type (cve, bugzilla). + ValidationError from those checks propagates to the caller. + """ + update = { + "from": elem.get("from"), + "status": elem.get("status"), + "version": elem.get("version"), + "type": elem.get("type"), + "id": text_strip(elem.find("id")), + "title": text_strip(elem.find("title")), + "reboot": self._parse_reboot_suggested(elem), + } + + # Optional fields - store as-is, no validation + text_elements = ["summary", "rights", "description", "release", "solution", "severity"] + date_elements = ["issued", "updated"] + for field in text_elements + date_elements: + found = elem.find(field) + if found is not None and field in text_elements: + content = text_strip(found) + update[field] = content if content else None + elif found is not None and field in date_elements: + update[field] = self._get_dt(found.get("date")) + else: + update[field] = None + + references = elem.find("references") + update["references"] = [] + for reference in references.findall("reference"): + update["references"].append(self._parse_reference(reference)) + + pkglist = elem.find("pkglist") + update["pkglist"] = [] + if pkglist is not None: + for collection in pkglist.findall("collection"): + module = collection.find("module") + for pkg in collection.findall("package"): + rec = self._process_package(pkg, module) + if rec not in update["pkglist"]: + update["pkglist"].append(rec) + + return update + + def _parse_reference(self, reference): + """Parse a reference; validate id by reference type.""" + ref_id = reference.get("id") + ref_type = reference.get("type") + + if ref_type == "cve": + ref_id = self._validate(ref_id, 'cve_id') + elif ref_type == "bugzilla": + ref_id = self._validate(ref_id, 'bugzilla_id') + + return { + "href": reference.get("href"), + "id": ref_id, + "type": ref_type, + "title": reference.get("title") + } + @staticmethod def _parse_reboot_suggested(elem) -> bool: """Try to parse bool from tag. Return False by default.""" @@ -84,14 +123,14 @@ def _parse_reboot_suggested(elem) -> bool: parsed_bool = bool(strtobool(parsed)) # strtobool returns 1 or 0, we need bool type return parsed_bool - @staticmethod - def _process_package(pkg, module): + def _process_package(self, pkg, module): + """Parse and validate pkglist entry NEVRA used for package matching.""" rec = { - "name": pkg.get("name"), + "name": self._validate(pkg.get("name"), 'name'), "epoch": pkg.get("epoch", "0"), - "ver": pkg.get("version"), - "rel": pkg.get("release"), - "arch": pkg.get("arch") + "ver": self._validate(pkg.get("version"), 'version'), + "rel": self._validate(pkg.get("release"), 'release'), + "arch": self._validate(pkg.get("arch"), 'arch') } if module is not None: rec["module_name"] = module.get("name") diff --git a/vmaas/reposcan/test_data/repodata/primary_validation_test.xml b/vmaas/reposcan/test_data/repodata/primary_validation_test.xml new file mode 100644 index 000000000..d59b1b895 --- /dev/null +++ b/vmaas/reposcan/test_data/repodata/primary_validation_test.xml @@ -0,0 +1,75 @@ + + + + valid-package + x86_64 + + abc123 + Valid test package + This package has all valid fields + + http://example.com + + + invalid-arch-package + INVALID_ARCH + + def456 + Package with invalid arch + This package should be skipped + + http://example.com + + + invalid name with spaces + x86_64 + + ghi789 + Package with invalid name + This package should also be skipped + + http://example.com + + diff --git a/vmaas/reposcan/test_data/repodata/update_validation_errata_test.xml b/vmaas/reposcan/test_data/repodata/update_validation_errata_test.xml new file mode 100644 index 000000000..24b275efd --- /dev/null +++ b/vmaas/reposcan/test_data/repodata/update_validation_errata_test.xml @@ -0,0 +1,83 @@ + + + +vmaas_test_x86_64_1 +VMaaS test errata for x86_64 package #1 +CentOS other + + +Moderate + + + +VMaaS test errata + + +CentOS other + +test-arch-vmaas-2-2.x86_64.rpm + + + + + +vmaas_test_x86_64_2 +VMaaS test errata for x86_64 package #2 +CentOS other + + +Critical + + + +VMaaS test errata + + +CentOS other + +test-arch-vmaas-3-3.INVALID_ARCH.rpm + + + + + +vmaas_test_i386_1.1 +VMaaS test errata for i386 package #1 from x86_64 +CentOS other + + +Moderate + + + +VMaaS test errata + + +CentOS other + +test-arch-vmaas-2-2.i386.rpm + + + + + +vmaas_test_i386_2.1 +VMaaS test errata for i386 package #2 from x86_64 repo +CentOS other + + +Critical + + + +VMaaS test errata + + +CentOS other + +test-arch-vmaas-3-3.i386.rpm + + + + + diff --git a/vmaas/reposcan/test_data/repodata/update_validation_test.xml b/vmaas/reposcan/test_data/repodata/update_validation_test.xml new file mode 100644 index 000000000..d0698b9d1 --- /dev/null +++ b/vmaas/reposcan/test_data/repodata/update_validation_test.xml @@ -0,0 +1,23 @@ + + + + vmaas_test_1 + VMaaS test errata + CentOS other + + + Critical + + + + VMaaS test errata + + + CentOS other + + test-vmaas-0.3-3.noarch.rpm + + + + + diff --git a/vmaas/reposcan/test_data/repodata/updateinfo.xml b/vmaas/reposcan/test_data/repodata/updateinfo.xml index f5e03d4b5..c0bf7acc7 100644 --- a/vmaas/reposcan/test_data/repodata/updateinfo.xml +++ b/vmaas/reposcan/test_data/repodata/updateinfo.xml @@ -10,7 +10,7 @@ Bump to upstream 6b67b3fab74d992bd07f72550006ab2c6907c416 - + @@ -94,7 +94,7 @@ [CHANGELOG](https://github.com/ploubser/JSON-Grep/blob/master/CHANGELOG.markdown) - + From e5eefee6b4b969b17f06de143894f7cab04d7f84 Mon Sep 17 00:00:00 2001 From: Andrej Luptak Date: Thu, 16 Jul 2026 10:51:41 +0200 Subject: [PATCH 2/7] feat: Grafana sanitization metrics Grafana metrics to keep track of potential sanitization failures at the begginning. Also added REGISTRY property so worker metrics can be exposed upon update withount preinitialization. --- ...oard-clouddot-insights-vmaas.configmap.yml | 238 +++++++++++++++--- vmaas/reposcan/main.py | 3 +- 2 files changed, 208 insertions(+), 33 deletions(-) diff --git a/monitoring/grafana/dashboards/grafana-dashboard-clouddot-insights-vmaas.configmap.yml b/monitoring/grafana/dashboards/grafana-dashboard-clouddot-insights-vmaas.configmap.yml index 56f81a1be..4d46b1e9c 100644 --- a/monitoring/grafana/dashboards/grafana-dashboard-clouddot-insights-vmaas.configmap.yml +++ b/monitoring/grafana/dashboards/grafana-dashboard-clouddot-insights-vmaas.configmap.yml @@ -27,7 +27,7 @@ data: "editable": true, "fiscalYearStartMonth": 0, "graphTooltip": 0, - "id": 1025985, + "id": 1, "links": [], "panels": [ { @@ -67,7 +67,8 @@ data: "mode": "absolute", "steps": [ { - "color": "#299c46" + "color": "#299c46", + "value": 0 }, { "color": "#ef843c", @@ -109,7 +110,7 @@ data: "textMode": "auto", "wideLayout": true }, - "pluginVersion": "11.6.3", + "pluginVersion": "12.3.2", "targets": [ { "datasource": { @@ -180,7 +181,8 @@ data: "mode": "absolute", "steps": [ { - "color": "#299c46" + "color": "#299c46", + "value": 0 }, { "color": "rgba(237, 129, 40, 0.89)", @@ -222,7 +224,7 @@ data: "textMode": "auto", "wideLayout": true }, - "pluginVersion": "11.6.3", + "pluginVersion": "12.3.2", "targets": [ { "datasource": { @@ -299,7 +301,8 @@ data: "mode": "absolute", "steps": [ { - "color": "#299c46" + "color": "#299c46", + "value": 0 }, { "color": "rgba(237, 129, 40, 0.89)", @@ -341,7 +344,7 @@ data: "textMode": "auto", "wideLayout": true }, - "pluginVersion": "11.6.3", + "pluginVersion": "12.3.2", "targets": [ { "datasource": { @@ -427,7 +430,7 @@ data: "steps": [ { "color": "#d44a3a", - "value": null + "value": 0 }, { "color": "#299c46", @@ -465,7 +468,7 @@ data: "textMode": "auto", "wideLayout": true }, - "pluginVersion": "11.6.3", + "pluginVersion": "12.3.2", "targets": [ { "datasource": { @@ -504,7 +507,8 @@ data: "mode": "absolute", "steps": [ { - "color": "#299c46" + "color": "#299c46", + "value": 0 }, { "color": "rgba(237, 129, 40, 0.89)", @@ -546,7 +550,7 @@ data: "textMode": "auto", "wideLayout": true }, - "pluginVersion": "11.6.3", + "pluginVersion": "12.3.2", "targets": [ { "datasource": { @@ -598,6 +602,156 @@ data: "title": "gabi-vmaas restarts", "type": "stat" }, + { + "datasource": { + "uid": "$datasource" + }, + "fieldConfig": { + "defaults": { + "color": { + "mode": "thresholds" + }, + "mappings": [ + { + "options": { + "match": "null", + "result": { + "text": "N/A" + } + }, + "type": "special" + } + ], + "thresholds": { + "mode": "absolute", + "steps": [ + { + "color": "#299c46", + "value": 0 + }, + { + "color": "#d44a3a", + "value": 1 + } + ] + }, + "unit": "none" + }, + "overrides": [] + }, + "gridPos": { + "h": 3, + "w": 3, + "x": 15, + "y": 1 + }, + "id": 150, + "maxDataPoints": 100, + "options": { + "colorMode": "background", + "graphMode": "none", + "justifyMode": "auto", + "orientation": "horizontal", + "percentChangeColorMode": "standard", + "reduceOptions": { + "calcs": [ + "lastNotNull" + ], + "fields": "", + "values": false + }, + "showPercentChange": false, + "text": {}, + "textMode": "auto", + "wideLayout": true + }, + "pluginVersion": "12.3.2", + "targets": [ + { + "datasource": { + "uid": "$datasource" + }, + "expr": "round(sum(increase(vmaas_reposcan_validation_failed_items_total[48h])))", + "format": "time_series", + "refId": "A" + } + ], + "title": "Validation failures [48h]", + "type": "stat" + }, + { + "datasource": { + "uid": "$datasource" + }, + "fieldConfig": { + "defaults": { + "color": { + "mode": "thresholds" + }, + "mappings": [ + { + "options": { + "match": "null", + "result": { + "text": "N/A" + } + }, + "type": "special" + } + ], + "thresholds": { + "mode": "absolute", + "steps": [ + { + "color": "#299c46", + "value": 0 + } + ] + }, + "unit": "none" + }, + "overrides": [] + }, + "gridPos": { + "h": 3, + "w": 3, + "x": 18, + "y": 1 + }, + "id": 151, + "maxDataPoints": 100, + "options": { + "colorMode": "background", + "graphMode": "none", + "justifyMode": "auto", + "orientation": "horizontal", + "percentChangeColorMode": "standard", + "reduceOptions": { + "calcs": [ + "lastNotNull" + ], + "fields": "", + "values": false + }, + "showPercentChange": false, + "text": {}, + "textMode": "auto", + "wideLayout": true + }, + "pluginVersion": "12.3.2", + "targets": [ + { + "datasource": { + "uid": "$datasource" + }, + "expr": "round(sum(increase(vmaas_reposcan_validation_total_items_total[48h])))", + "format": "time_series", + "refId": "A" + } + ], + "title": "Items validated [48h]", + "type": "stat" + }, { "collapsed": false, "gridPos": { @@ -644,6 +798,7 @@ data: "type": "linear" }, "showPoints": "never", + "showValues": false, "spanNulls": false, "stacking": { "group": "A", @@ -661,7 +816,8 @@ data: "mode": "absolute", "steps": [ { - "color": "green" + "color": "green", + "value": 0 }, { "color": "red", @@ -694,7 +850,7 @@ data: "sort": "none" } }, - "pluginVersion": "11.6.3", + "pluginVersion": "12.3.2", "targets": [ { "alias": "", @@ -753,6 +909,7 @@ data: "type": "linear" }, "showPoints": "never", + "showValues": false, "spanNulls": false, "stacking": { "group": "A", @@ -768,7 +925,8 @@ data: "mode": "absolute", "steps": [ { - "color": "green" + "color": "green", + "value": 0 }, { "color": "red", @@ -801,7 +959,7 @@ data: "sort": "none" } }, - "pluginVersion": "11.6.3", + "pluginVersion": "12.3.2", "targets": [ { "datasource": { @@ -862,6 +1020,7 @@ data: "type": "linear" }, "showPoints": "never", + "showValues": false, "spanNulls": false, "stacking": { "group": "A", @@ -878,7 +1037,8 @@ data: "mode": "absolute", "steps": [ { - "color": "green" + "color": "green", + "value": 0 }, { "color": "red", @@ -911,7 +1071,7 @@ data: "sort": "none" } }, - "pluginVersion": "11.6.3", + "pluginVersion": "12.3.2", "targets": [ { "datasource": { @@ -962,6 +1122,7 @@ data: "type": "linear" }, "showPoints": "never", + "showValues": false, "spanNulls": false, "stacking": { "group": "A", @@ -978,7 +1139,8 @@ data: "mode": "absolute", "steps": [ { - "color": "green" + "color": "green", + "value": 0 }, { "color": "red", @@ -1011,7 +1173,7 @@ data: "sort": "none" } }, - "pluginVersion": "11.6.3", + "pluginVersion": "12.3.2", "targets": [ { "datasource": { @@ -1135,6 +1297,7 @@ data: "type": "linear" }, "showPoints": "never", + "showValues": false, "spanNulls": false, "stacking": { "group": "A", @@ -1152,7 +1315,8 @@ data: "mode": "absolute", "steps": [ { - "color": "green" + "color": "green", + "value": 0 }, { "color": "red", @@ -1185,7 +1349,7 @@ data: "sort": "none" } }, - "pluginVersion": "11.6.3", + "pluginVersion": "12.3.2", "targets": [ { "datasource": { @@ -1238,6 +1402,7 @@ data: "type": "linear" }, "showPoints": "never", + "showValues": false, "spanNulls": false, "stacking": { "group": "A", @@ -1255,7 +1420,8 @@ data: "mode": "absolute", "steps": [ { - "color": "green" + "color": "green", + "value": 0 }, { "color": "red", @@ -1288,7 +1454,7 @@ data: "sort": "none" } }, - "pluginVersion": "11.6.3", + "pluginVersion": "12.3.2", "targets": [ { "datasource": { @@ -1340,6 +1506,7 @@ data: "type": "linear" }, "showPoints": "never", + "showValues": false, "spanNulls": false, "stacking": { "group": "A", @@ -1356,7 +1523,8 @@ data: "mode": "absolute", "steps": [ { - "color": "green" + "color": "green", + "value": 0 }, { "color": "red", @@ -1389,7 +1557,7 @@ data: "sort": "none" } }, - "pluginVersion": "11.6.3", + "pluginVersion": "12.3.2", "targets": [ { "datasource": { @@ -1443,6 +1611,7 @@ data: "type": "linear" }, "showPoints": "never", + "showValues": false, "spanNulls": false, "stacking": { "group": "A", @@ -1459,7 +1628,8 @@ data: "mode": "absolute", "steps": [ { - "color": "green" + "color": "green", + "value": 0 }, { "color": "red", @@ -1492,7 +1662,7 @@ data: "sort": "none" } }, - "pluginVersion": "11.6.3", + "pluginVersion": "12.3.2", "targets": [ { "datasource": { @@ -1546,6 +1716,7 @@ data: "type": "linear" }, "showPoints": "never", + "showValues": false, "spanNulls": false, "stacking": { "group": "A", @@ -1562,7 +1733,8 @@ data: "mode": "absolute", "steps": [ { - "color": "green" + "color": "green", + "value": 0 }, { "color": "red", @@ -1595,7 +1767,7 @@ data: "sort": "none" } }, - "pluginVersion": "11.6.3", + "pluginVersion": "12.3.2", "targets": [ { "datasource": { @@ -1649,6 +1821,7 @@ data: "type": "linear" }, "showPoints": "never", + "showValues": false, "spanNulls": false, "stacking": { "group": "A", @@ -1665,7 +1838,8 @@ data: "mode": "absolute", "steps": [ { - "color": "green" + "color": "green", + "value": 0 }, { "color": "red", @@ -1698,7 +1872,7 @@ data: "sort": "none" } }, - "pluginVersion": "11.6.3", + "pluginVersion": "12.3.2", "targets": [ { "datasource": { @@ -1721,7 +1895,7 @@ data: ], "preload": false, "refresh": "1m", - "schemaVersion": 41, + "schemaVersion": 42, "tags": [], "templating": { "list": [ diff --git a/vmaas/reposcan/main.py b/vmaas/reposcan/main.py index 9b7e0765d..f553dfa55 100644 --- a/vmaas/reposcan/main.py +++ b/vmaas/reposcan/main.py @@ -2,10 +2,11 @@ from prometheus_client import start_http_server from vmaas.common.config import Config +from vmaas.reposcan.mnm import REGISTRY from vmaas.reposcan.reposcan import create_app, DEFAULT_PATH, DEFAULT_PATH_API cfg = Config() -start_http_server(int(cfg.metrics_port)) +start_http_server(int(cfg.metrics_port), registry=REGISTRY) # pylint: disable=invalid-name app = create_app({DEFAULT_PATH + "/v1": "reposcan.spec.yaml", DEFAULT_PATH_API + "/v1": "reposcan.spec.yaml", From b0d5c38c2b69b33f1fe7d67be537e57b5e1744a5 Mon Sep 17 00:00:00 2001 From: Andrej Luptak Date: Mon, 20 Jul 2026 14:28:16 +0200 Subject: [PATCH 3/7] feat: repolist basearch allowed values Verify basearch field has one of allowed arch values. Part of RHINENG-13437 --- .../repodata/repository_controller.py | 20 +++++++++++- .../reposcan/repodata/test/test_repository.py | 32 +++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/vmaas/reposcan/repodata/repository_controller.py b/vmaas/reposcan/repodata/repository_controller.py index cad9b918c..2968d660f 100644 --- a/vmaas/reposcan/repodata/repository_controller.py +++ b/vmaas/reposcan/repodata/repository_controller.py @@ -14,10 +14,19 @@ from vmaas.reposcan.database.repository_store import RepositoryStore from vmaas.reposcan.download.downloader import FileDownloader, DownloadItem, VALID_HTTP_CODES from vmaas.reposcan.download.unpacker import FileUnpacker -from vmaas.reposcan.mnm import FAILED_REPOMD, FAILED_IMPORT_REPO, FAILED_REPO_WITH_HTTP_CODE, FAILED_METADATA_CHECKSUM from vmaas.reposcan.repodata.checksum import ChecksumError from vmaas.reposcan.repodata.checksum import verify_file_checksum +from vmaas.reposcan.mnm import ( + FAILED_REPOMD, + FAILED_IMPORT_REPO, + FAILED_REPO_WITH_HTTP_CODE, + FAILED_METADATA_CHECKSUM, + VALIDATION_FAILED_ITEMS, + VALIDATION_TOTAL_ITEMS, +) + +from vmaas.reposcan.repodata.metadata_validators import validate_architecture, ValidationError from vmaas.reposcan.repodata.repomd import RepoMD, RepoMDTypeNotFound from vmaas.reposcan.repodata.repository import Repository @@ -216,6 +225,15 @@ def add_db_repositories(self): def add_repository(self, repo_url, content_set, basearch, releasever, organization, *, # pylint: disable=too-many-positional-arguments cert_name=None, ca_cert=None, cert=None, key=None): """Queue repository to import/check updates.""" + if basearch: + VALIDATION_TOTAL_ITEMS.labels(metadata_type='repolist').inc() + try: + basearch = validate_architecture(basearch) + except ValidationError as err: + VALIDATION_FAILED_ITEMS.labels(metadata_type='repolist', field='basearch').inc() + self.logger.warning("Validation failed, skipped repository %s: %s", content_set, err) + return + repo_url = repo_url.strip() if not repo_url.endswith("/"): repo_url += "/" diff --git a/vmaas/reposcan/repodata/test/test_repository.py b/vmaas/reposcan/repodata/test/test_repository.py index f5788735f..b7fecd658 100644 --- a/vmaas/reposcan/repodata/test/test_repository.py +++ b/vmaas/reposcan/repodata/test/test_repository.py @@ -1,11 +1,14 @@ """ Unit test classes for repository module. """ +from unittest.mock import MagicMock + from vmaas.reposcan.repodata.primary import PrimaryMD from vmaas.reposcan.repodata.primary_db import PrimaryDatabaseMD from vmaas.reposcan.repodata.updateinfo import UpdateInfoMD from vmaas.reposcan.repodata.modules import ModuleMD from vmaas.reposcan.repodata.repository import Repository +from vmaas.reposcan.repodata.repository_controller import RepositoryController from vmaas.reposcan.repodata.test.test_updateinfo import KNOWN_UPDATE_TYPES from vmaas.reposcan.reposcan import DEFAULT_ORG_NAME @@ -88,3 +91,32 @@ def test_load_metadata(self): assert repo.primary is None assert repo.updateinfo is None assert repo.modules is None + + +class TestRepositoryControllerBasearch: + """Test basearch validation when queueing repositories.""" + + def _controller(self, monkeypatch): + monkeypatch.setattr( + "vmaas.reposcan.repodata.repository_controller.RepositoryStore", MagicMock + ) + return RepositoryController() + + def test_add_valid_basearch(self, monkeypatch): + """Valid basearch is queued.""" + ctrl = self._controller(monkeypatch) + ctrl.add_repository("http://example.com/", "cs", "x86_64", "9", DEFAULT_ORG_NAME) + assert len(ctrl.repositories) == 1 + + def test_skip_invalid_basearch(self, monkeypatch, caplog): + """Invalid basearch is skipped with a warning.""" + ctrl = self._controller(monkeypatch) + ctrl.add_repository("http://example.com/", "cs", "7Server", "9", DEFAULT_ORG_NAME) + assert len(ctrl.repositories) == 0 + assert "Validation failed" in caplog.text + + def test_add_without_basearch(self, monkeypatch): + """Repository without basearch is still queued.""" + ctrl = self._controller(monkeypatch) + ctrl.add_repository("http://example.com/", "cs", None, "9", DEFAULT_ORG_NAME) + assert len(ctrl.repositories) == 1 From 497d6da0f64cd460e1f680b694f5c2aeb4639cb9 Mon Sep 17 00:00:00 2001 From: Andrej Luptak Date: Mon, 20 Jul 2026 14:59:18 +0200 Subject: [PATCH 4/7] feat: unify reposync errors Grafana plot Instead of numeric counters reuse an existing vmaas reposcan errors plot to display validation failures. --- ...oard-clouddot-insights-vmaas.configmap.yml | 160 ++---------------- 1 file changed, 10 insertions(+), 150 deletions(-) diff --git a/monitoring/grafana/dashboards/grafana-dashboard-clouddot-insights-vmaas.configmap.yml b/monitoring/grafana/dashboards/grafana-dashboard-clouddot-insights-vmaas.configmap.yml index 4d46b1e9c..a572a8c54 100644 --- a/monitoring/grafana/dashboards/grafana-dashboard-clouddot-insights-vmaas.configmap.yml +++ b/monitoring/grafana/dashboards/grafana-dashboard-clouddot-insights-vmaas.configmap.yml @@ -602,156 +602,6 @@ data: "title": "gabi-vmaas restarts", "type": "stat" }, - { - "datasource": { - "uid": "$datasource" - }, - "fieldConfig": { - "defaults": { - "color": { - "mode": "thresholds" - }, - "mappings": [ - { - "options": { - "match": "null", - "result": { - "text": "N/A" - } - }, - "type": "special" - } - ], - "thresholds": { - "mode": "absolute", - "steps": [ - { - "color": "#299c46", - "value": 0 - }, - { - "color": "#d44a3a", - "value": 1 - } - ] - }, - "unit": "none" - }, - "overrides": [] - }, - "gridPos": { - "h": 3, - "w": 3, - "x": 15, - "y": 1 - }, - "id": 150, - "maxDataPoints": 100, - "options": { - "colorMode": "background", - "graphMode": "none", - "justifyMode": "auto", - "orientation": "horizontal", - "percentChangeColorMode": "standard", - "reduceOptions": { - "calcs": [ - "lastNotNull" - ], - "fields": "", - "values": false - }, - "showPercentChange": false, - "text": {}, - "textMode": "auto", - "wideLayout": true - }, - "pluginVersion": "12.3.2", - "targets": [ - { - "datasource": { - "uid": "$datasource" - }, - "expr": "round(sum(increase(vmaas_reposcan_validation_failed_items_total[48h])))", - "format": "time_series", - "refId": "A" - } - ], - "title": "Validation failures [48h]", - "type": "stat" - }, - { - "datasource": { - "uid": "$datasource" - }, - "fieldConfig": { - "defaults": { - "color": { - "mode": "thresholds" - }, - "mappings": [ - { - "options": { - "match": "null", - "result": { - "text": "N/A" - } - }, - "type": "special" - } - ], - "thresholds": { - "mode": "absolute", - "steps": [ - { - "color": "#299c46", - "value": 0 - } - ] - }, - "unit": "none" - }, - "overrides": [] - }, - "gridPos": { - "h": 3, - "w": 3, - "x": 18, - "y": 1 - }, - "id": 151, - "maxDataPoints": 100, - "options": { - "colorMode": "background", - "graphMode": "none", - "justifyMode": "auto", - "orientation": "horizontal", - "percentChangeColorMode": "standard", - "reduceOptions": { - "calcs": [ - "lastNotNull" - ], - "fields": "", - "values": false - }, - "showPercentChange": false, - "text": {}, - "textMode": "auto", - "wideLayout": true - }, - "pluginVersion": "12.3.2", - "targets": [ - { - "datasource": { - "uid": "$datasource" - }, - "expr": "round(sum(increase(vmaas_reposcan_validation_total_items_total[48h])))", - "format": "time_series", - "refId": "A" - } - ], - "title": "Items validated [48h]", - "type": "stat" - }, { "collapsed": false, "gridPos": { @@ -1245,6 +1095,16 @@ data: "format": "time_series", "intervalFactor": 1, "legendFormat": "metadata checksum failed", + "refId": "D" + }, + { + "datasource": { + "uid": "$datasource" + }, + "expr": "sum(vmaas_reposcan_validation_failed_items_total)", + "format": "time_series", + "intervalFactor": 1, + "legendFormat": "repo validation failed", "refId": "E" } ], From 21c51012ecf38a350d9eb12e47c7c13f92946abe Mon Sep 17 00:00:00 2001 From: Andrej Luptak Date: Mon, 20 Jul 2026 16:39:57 +0200 Subject: [PATCH 5/7] refactor: use database predefined arch values Use a single source of truth for arch names based on database values. As we sanitize the archnames, the insert is no longer relevant to us. --- vmaas/reposcan/conftest.py | 10 +++++ vmaas/reposcan/database/repository_store.py | 21 +++------- .../reposcan/repodata/metadata_validators.py | 42 ++++--------------- 3 files changed, 23 insertions(+), 50 deletions(-) diff --git a/vmaas/reposcan/conftest.py b/vmaas/reposcan/conftest.py index 6b2f28ed3..b292835d1 100644 --- a/vmaas/reposcan/conftest.py +++ b/vmaas/reposcan/conftest.py @@ -8,12 +8,22 @@ from vmaas.common.paths import DB_CREATE_SQL_PATH from vmaas.reposcan.database.database_handler import init_db +from vmaas.reposcan.repodata.metadata_validators import init_validator_architectures from vmaas.reposcan.redhatcsaf import modeling as csaf_model VMAAS_DIR = Path(__file__).resolve().parent.parent.parent VMAAS_DB_DATA = VMAAS_DIR.joinpath("vmaas", "reposcan", "test_data", "database", "test_data.sql") VMAAS_PG_OLD = VMAAS_DIR.joinpath("vmaas", "reposcan", "test_data", "database", "vmaas_db_postgresql_old.sql") +# Populate valid architectures before test modules import parsers at module level +init_validator_architectures([ + 'noarch', 'i386', 'i486', 'i586', 'i686', 'alpha', 'alphaev6', 'ia64', + 'sparc', 'sparcv9', 'sparc64', 's390', 'athlon', 's390x', 'ppc', 'ppc64', + 'ppc64le', 'pSeries', 'iSeries', 'x86_64', 'ppc64iseries', 'ppc64pseries', + 'ia32e', 'amd64', 'aarch64', 'armv7hnl', 'armv7hl', 'armv7l', 'armv6hl', + 'armv6l', 'armv5tel', 'src', +]) + EXPECTED_CSAF = ( ("cve-2023-0030.json", csaf_model.CsafCves({"CVE-2023-0030": csaf_model.CsafProducts()})), ( diff --git a/vmaas/reposcan/database/repository_store.py b/vmaas/reposcan/database/repository_store.py index 4b5f87e8f..cbcb0fe93 100644 --- a/vmaas/reposcan/database/repository_store.py +++ b/vmaas/reposcan/database/repository_store.py @@ -6,6 +6,7 @@ from vmaas.reposcan.database.modules_store import ModulesStore from vmaas.reposcan.database.package_store import PackageStore from vmaas.reposcan.database.update_store import UpdateStore +from vmaas.reposcan.repodata.metadata_validators import ValidationError, init_validator_architectures class RepositoryStore: @@ -19,6 +20,7 @@ def __init__(self): self.module_store = ModulesStore() self.package_store = PackageStore() self.update_store = UpdateStore() + init_validator_architectures(self.package_store.arch_map.keys()) self.content_set_to_db_id = self._prepare_content_set_map() self.organization_to_db_id = {} @@ -61,21 +63,10 @@ def list_repositories(self): return repos def _import_basearch(self, basearch): - cur = self.conn.cursor() - try: - cur.execute("select id from arch where name = %s", (basearch,)) - arch_id = cur.fetchone() - if not arch_id: - cur.execute("insert into arch (name) values(%s) returning id", (basearch,)) - arch_id = cur.fetchone() - self.conn.commit() - except Exception: - self.logger.exception("Failed to import basearch.") - self.conn.rollback() - raise - finally: - cur.close() - return arch_id[0] + arch_id = self.package_store.arch_map.get(basearch) + if not arch_id: + raise ValidationError(f"Invalid basearch: {basearch}") + return arch_id def _import_certificate(self, cert_name, ca_cert, cert, key): if not key: diff --git a/vmaas/reposcan/repodata/metadata_validators.py b/vmaas/reposcan/repodata/metadata_validators.py index 613cb55ca..ab4ddb85a 100644 --- a/vmaas/reposcan/repodata/metadata_validators.py +++ b/vmaas/reposcan/repodata/metadata_validators.py @@ -10,41 +10,13 @@ RELEASE_PATTERN = re.compile(r'^[a-zA-Z0-9._+\-~]+$') VERSION_PATTERN = re.compile(r'^[a-zA-Z0-9._+\-~^]+$') -# Architecture whitelist -VALID_ARCHITECTURES = { - 'noarch', - 'i386', - 'i486', - 'i586', - 'i686', - 'alpha', - 'alphaev6', - 'ia64', - 'sparc', - 'sparcv9', - 'sparc64', - 's390', - 'athlon', - 's390x', - 'ppc', - 'ppc64', - 'ppc64le', - 'pSeries', - 'iSeries', - 'x86_64', - 'ppc64iseries', - 'ppc64pseries', - 'ia32e', - 'amd64', - 'aarch64', - 'armv7hnl', - 'armv7hl', - 'armv7l', - 'armv6hl', - 'armv6l', - 'armv5tel', - 'src', -} +# Populated from DB arch table at startup via init_validator_architectures() +VALID_ARCHITECTURES = set() + + +def init_validator_architectures(arch_names): + """Populate valid architectures from DB values""" + VALID_ARCHITECTURES.update(arch_names) class ValidationError(Exception): From 3b170d3c058e3e7633bc4dda55a1349cd933ceb0 Mon Sep 17 00:00:00 2001 From: Andrej Luptak Date: Tue, 28 Jul 2026 10:29:19 +0200 Subject: [PATCH 6/7] perf: Validator per field cache and regex replacement Bypass regex validation by caching already validated values to improve the validator performance. Replace digit regex for python idigit which should perform better as well. --- vmaas/reposcan/database/repository_store.py | 11 ++----- .../reposcan/repodata/metadata_validators.py | 19 +++++++++-- .../reposcan/repodata/test/test_validators.py | 33 +++++++++++++++++++ 3 files changed, 52 insertions(+), 11 deletions(-) diff --git a/vmaas/reposcan/database/repository_store.py b/vmaas/reposcan/database/repository_store.py index cbcb0fe93..8d1c0d235 100644 --- a/vmaas/reposcan/database/repository_store.py +++ b/vmaas/reposcan/database/repository_store.py @@ -62,12 +62,6 @@ def list_repositories(self): cur.close() return repos - def _import_basearch(self, basearch): - arch_id = self.package_store.arch_map.get(basearch) - if not arch_id: - raise ValidationError(f"Invalid basearch: {basearch}") - return arch_id - def _import_certificate(self, cert_name, ca_cert, cert, key): if not key: key = None @@ -222,8 +216,9 @@ def import_repository(self, repo): cert_id = None if repo.basearch: - # will raise exception if db error occurs - basearch_id = self._import_basearch(repo.basearch) + basearch_id = self.package_store.arch_map.get(repo.basearch) + if not basearch_id: + raise ValidationError(f"Invalid basearch: {repo.basearch}") else: basearch_id = None diff --git a/vmaas/reposcan/repodata/metadata_validators.py b/vmaas/reposcan/repodata/metadata_validators.py index ab4ddb85a..3c5f065f9 100644 --- a/vmaas/reposcan/repodata/metadata_validators.py +++ b/vmaas/reposcan/repodata/metadata_validators.py @@ -5,7 +5,6 @@ # Validation patterns CVE_PATTERN = re.compile(r'^CVE-\d{4}-\d+$') -BUGZILLA_ID_PATTERN = re.compile(r'^\d+$') PACKAGE_NAME_PATTERN = re.compile(r'^[a-zA-Z0-9._+\-]+$') RELEASE_PATTERN = re.compile(r'^[a-zA-Z0-9._+\-~]+$') VERSION_PATTERN = re.compile(r'^[a-zA-Z0-9._+\-~^]+$') @@ -37,7 +36,7 @@ def validate_bugzilla_id(bugzilla_id): """Validate Bugzilla ticket number format.""" bugzilla_id = str(bugzilla_id).strip() - if not BUGZILLA_ID_PATTERN.match(bugzilla_id): + if not bugzilla_id.isdigit(): raise ValidationError(f"Invalid Bugzilla ID format: {bugzilla_id}") return bugzilla_id @@ -94,10 +93,24 @@ def validate_release(release): } +_validated_cache = {'name': {}, 'version': {}, 'release': {}} + + def validate_field(value, field_type): """Unified validation function that validates a field based on its type""" validator = FIELD_VALIDATORS.get(field_type) if not validator: raise ValueError(f"Unknown field type: {field_type}") - return validator(value) + cache = _validated_cache.get(field_type) + if cache is not None: + cached = cache.get(value) + if cached is not None: + return cached + + result = validator(value) + + if cache is not None: + cache[value] = result + + return result diff --git a/vmaas/reposcan/repodata/test/test_validators.py b/vmaas/reposcan/repodata/test/test_validators.py index bd2b24fe6..b27b1b6c9 100644 --- a/vmaas/reposcan/repodata/test/test_validators.py +++ b/vmaas/reposcan/repodata/test/test_validators.py @@ -9,6 +9,8 @@ validate_package_name, validate_version, validate_release, + validate_field, + _validated_cache, ValidationError, ) @@ -154,3 +156,34 @@ def test_invalid_releases(self): with pytest.raises(ValidationError, match="Invalid"): validate_release("") + + +class TestValidationCache: + """Test that validate_field caches results.""" + + def setup_method(self): + """Clear validation cache before each test.""" + for cache in _validated_cache.values(): + cache.clear() + + def test_cache_returns_same_result(self): + """Test that repeated calls return cached result.""" + first = validate_field("kernel", "name") + assert "kernel" in _validated_cache["name"] + second = validate_field("kernel", "name") + assert first == second + + def test_cache_skips_invalid(self): + """Test that invalid values are not cached.""" + with pytest.raises(ValidationError): + validate_field("bad package!", "name") + assert "bad package!" not in _validated_cache["name"] + + def test_cache_per_field_type(self): + """Test that cache is separate per field type.""" + validate_field("1.0.0", "version") + validate_field("1.el8", "release") + assert "1.0.0" in _validated_cache["version"] + assert "1.0.0" not in _validated_cache["release"] + assert "1.el8" in _validated_cache["release"] + assert "1.el8" not in _validated_cache["version"] From 1b35cf21f952f413bb51c639ce3aa3a5674ca871 Mon Sep 17 00:00:00 2001 From: Andrej Luptak Date: Tue, 28 Jul 2026 12:04:54 +0200 Subject: [PATCH 7/7] tests: valid arch value Arch is not a valid arch value for validation. We need valid arch so import repos can be tested, otherwise the sanitization will reject it. --- vmaas/reposcan/test/test_reposcan.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vmaas/reposcan/test/test_reposcan.py b/vmaas/reposcan/test/test_reposcan.py index 2d181f0a9..99ddbaac1 100644 --- a/vmaas/reposcan/test/test_reposcan.py +++ b/vmaas/reposcan/test/test_reposcan.py @@ -69,7 +69,7 @@ def test_add_repo_1(self): "name": "Testing repo desc", "baseurl": "http://localhost:8888/$releasever/$basearch/", "releasever": ["Server"], - "basearch": ["Arch"] + "basearch": ["x86_64"] } } } @@ -97,7 +97,7 @@ def test_add_repo_1_with_certs(self): "name": "Testing repo desc", "baseurl": "http://localhost:8888/$releasever/$basearch/", "releasever": ["Server"], - "basearch": ["Arch"] + "basearch": ["x86_64"] } } }