Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 74 additions & 13 deletions doozer/doozerlib/cli/release_gen_payload.py
Original file line number Diff line number Diff line change
Expand Up @@ -2392,22 +2392,83 @@ def validate_pkg_consistency_req(
rhcos_build_id: str,
package_rpm_finder=None,
) -> Optional[AssemblyIssue]:
"""check that the specified package in the member is consistent with the RHCOS build"""
"""
Check that the specified package in the member is consistent with the RHCOS build.

For Konflux builds, this also detects when multiple versions of the same package
are installed (e.g., two different kernel versions), which should never happen.
"""
logger = bri.runtime.logger
payload_tag_nvr: str = bri.get_nvr()
logger.debug(f"Checking consistency of {pkg} for {payload_tag_nvr} against {rhcos_build_id}")
member_nvrs: Dict[str, Dict] = bri.get_all_installed_package_build_dicts(
package_rpm_finder or self.package_rpm_finder
) # by name
try:
build = member_nvrs[pkg]
except KeyError:
return AssemblyIssue(
f"RHCOS consistency configuration specifies that payload tag '{payload_tag}' "
f"should install package '{pkg}', but it does not",
payload_tag,
AssemblyIssueCode.FAILED_CONSISTENCY_REQUIREMENT,
)

# For Konflux, get ALL installed package NVRs to detect duplicates
# For Brew, use the old method (only one version per package is expected)
if self.runtime.build_system == 'konflux':
# Access the build record directly to get all installed packages
build_record = bri.get_build_obj() # Returns KonfluxBuildRecord
installed_packages = build_record.installed_packages # List of package NVRs

# Filter for packages matching the name we're checking
matching_packages = [pkg_nvr for pkg_nvr in installed_packages if parse_nvr(pkg_nvr)['name'] == pkg]

if not matching_packages:
return AssemblyIssue(
f"RHCOS consistency configuration specifies that payload tag '{payload_tag}' "
f"should install package '{pkg}', but it does not",
payload_tag,
AssemblyIssueCode.FAILED_CONSISTENCY_REQUIREMENT,
)

# Check if there are multiple versions of the same package installed
if len(matching_packages) > 1:
return AssemblyIssue(
f"Payload tag '{payload_tag}' has multiple versions of package '{pkg}' installed: "
f"{', '.join(matching_packages)}. Only one version should be present.",
payload_tag,
AssemblyIssueCode.FAILED_CONSISTENCY_REQUIREMENT,
)

# Get the single package build dict
pkg_finder = package_rpm_finder or self.package_rpm_finder
pkg_finder._cache_packages(matching_packages)
build = pkg_finder._packages_build_dicts[matching_packages[0]]

else: # Brew
# First, check for duplicate packages in Brew builds
# Although OSBS should enforce single versions, check to be safe
all_rpm_dicts = bri.get_all_installed_rpm_dicts()
matching_package_nvrs = set()
for rpm_dict in all_rpm_dicts:
rpm_nvr = rpm_dict['nvr']
parsed = parse_nvr(rpm_nvr)
if parsed['name'] == pkg:
Comment on lines +2443 to +2445
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Same parse_nvr ValueError risk in Brew path.

The parse_nvr call here has the same risk of raising ValueError on malformed NVRs as noted in the Konflux path.

🛡️ Proposed defensive handling
             for rpm_dict in all_rpm_dicts:
                 rpm_nvr = rpm_dict['nvr']
-                parsed = parse_nvr(rpm_nvr)
-                if parsed['name'] == pkg:
+                try:
+                    parsed = parse_nvr(rpm_nvr)
+                except ValueError:
+                    logger.warning(f"Skipping malformed RPM NVR: {rpm_nvr}")
+                    continue
+                if parsed['name'] == pkg:
                     # Extract package NVR from RPM NVR (RPMs have arch suffix)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
rpm_nvr = rpm_dict['nvr']
parsed = parse_nvr(rpm_nvr)
if parsed['name'] == pkg:
rpm_nvr = rpm_dict['nvr']
try:
parsed = parse_nvr(rpm_nvr)
except ValueError:
logger.warning(f"Skipping malformed RPM NVR: {rpm_nvr}")
continue
if parsed['name'] == pkg:
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@doozer/doozerlib/cli/release_gen_payload.py` around lines 2446 - 2448, The
parse_nvr call on rpm_nvr (from rpm_dict) can raise ValueError for malformed
NVRs; wrap the call to parse_nvr(rpm_nvr) in a try/except ValueError block
(around the code using rpm_dict['nvr'] and parsed) and on exception log a
warning (including rpm_nvr and pkg for context) and continue/skip that entry so
the loop does not crash; ensure you reference rpm_dict, rpm_nvr, parse_nvr,
parsed and pkg when adding the handling.

# Extract package NVR from RPM NVR (RPMs have arch suffix)
# e.g., kernel-5.14.0-427.116.1.el9_4.x86_64 -> kernel-5.14.0-427.116.1.el9_4
package_nvr = f"{parsed['name']}-{parsed['version']}-{parsed['release']}"
matching_package_nvrs.add(package_nvr)

if not matching_package_nvrs:
return AssemblyIssue(
f"RHCOS consistency configuration specifies that payload tag '{payload_tag}' "
f"should install package '{pkg}', but it does not",
payload_tag,
AssemblyIssueCode.FAILED_CONSISTENCY_REQUIREMENT,
)

if len(matching_package_nvrs) > 1:
return AssemblyIssue(
f"Payload tag '{payload_tag}' has multiple versions of package '{pkg}' installed: "
f"{', '.join(sorted(matching_package_nvrs))}. Only one version should be present.",
payload_tag,
AssemblyIssueCode.FAILED_CONSISTENCY_REQUIREMENT,
)

# Get the build dict using the old method (now we know there's only one)
member_nvrs: Dict[str, Dict] = bri.get_all_installed_package_build_dicts(
package_rpm_finder or self.package_rpm_finder
) # by name
build = member_nvrs[pkg] # We know this exists now
Comment on lines +2468 to +2471
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Potential KeyError if methods return inconsistent data.

The comment states "We know this exists now" based on finding the package via get_all_installed_rpm_dicts(), but get_all_installed_package_build_dicts() is a separate method that queries different data. If these methods have any filtering differences, member_nvrs[pkg] could raise KeyError.

🛡️ Proposed defensive check
             # Get the build dict using the old method (now we know there's only one)
             member_nvrs: Dict[str, Dict] = bri.get_all_installed_package_build_dicts(
                 package_rpm_finder or self.package_rpm_finder
             )  # by name
-            build = member_nvrs[pkg]  # We know this exists now
+            build = member_nvrs.get(pkg)
+            if not build:
+                return AssemblyIssue(
+                    f"Package '{pkg}' found in RPM list but not in package build dicts for '{payload_tag}'",
+                    payload_tag,
+                    AssemblyIssueCode.FAILED_CONSISTENCY_REQUIREMENT,
+                )
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
member_nvrs: Dict[str, Dict] = bri.get_all_installed_package_build_dicts(
package_rpm_finder or self.package_rpm_finder
) # by name
build = member_nvrs[pkg] # We know this exists now
member_nvrs: Dict[str, Dict] = bri.get_all_installed_package_build_dicts(
package_rpm_finder or self.package_rpm_finder
) # by name
build = member_nvrs.get(pkg)
if not build:
return AssemblyIssue(
f"Package '{pkg}' found in RPM list but not in package build dicts for '{payload_tag}'",
payload_tag,
AssemblyIssueCode.FAILED_CONSISTENCY_REQUIREMENT,
)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@doozer/doozerlib/cli/release_gen_payload.py` around lines 2471 - 2474, The
code assumes member_nvrs[pkg] always exists but
get_all_installed_package_build_dicts() can return a different set than the
earlier finder, risking a KeyError; change the access to defensively check for
the key (use member_nvrs.get(pkg)) and if missing either raise a clear,
informative exception (include pkg, package_rpm_finder/self.package_rpm_finder
and that get_all_installed_package_build_dicts() did not return an entry) or
handle it gracefully (log the missing pkg and skip/continue), updating any
callers to expect the new error/flow; refer to
get_all_installed_package_build_dicts, member_nvrs, pkg and
package_rpm_finder/self.package_rpm_finder to locate the change.


# get names of all the actual RPMs included in this package build, because that's what we
# have for comparison in the RHCOS metadata (not the package name).
Expand Down
Loading