Skip to content

Modular RPM support fix - #89

Open
atomicturtle wants to merge 1 commit into
resf:mainfrom
atomicturtle:fix/72-module-fields
Open

Modular RPM support fix#89
atomicturtle wants to merge 1 commit into
resf:mainfrom
atomicturtle:fix/72-module-fields

Conversation

@atomicturtle

Copy link
Copy Markdown
Contributor

We were discarding ::nodejs:16 from RH CSAF at ingest, then trying to rediscover it via exact NEVRA match in Rocky’s modules.yaml. That breaks when Rocky’s build ID (+1760+903d54b9) doesn’t match Red Hat’s (+21536+8fdee1fb). Fix is keep the stream from CSAF and match without requiring identical build IDs.

a specific example, the redhat data looks like this:

nodejs-1:16.20.2-4.module+el8.9.0+21536+8fdee1fb.x86_64::nodejs:16

and apollo was discarding "::nodejs:16", so it just contained the bare NERVA. Then later when apollo tries to reconstruct the module data, its looking for the exact redhat NERVA, which is never going to be true, since we build the module with a different build ID. So the lookup misses, and the RLSA packages end up with module_name / stream null even though Red Hat told us the stream in the CSAF.

Fix: keep the module identity from CSAF at ingest, and when yaml doesn’t match, fall back using a cleaned NEVRA (ignoring the build-id suffix) so Rocky’s rebuild still maps to nodejs:16.

Closes issue #72

We were discarding ::nodejs:16 from RH CSAF at ingest, then trying to rediscover it via exact NEVRA match in Rocky’s modules.yaml. That breaks when Rocky’s build ID (+1760+903d54b9) doesn’t match Red Hat’s (+21536+8fdee1fb). Fix is keep the stream from CSAF and match without requiring identical build IDs.

a specific example, the redhat data looks like this:

nodejs-1:16.20.2-4.module+el8.9.0+21536+8fdee1fb.x86_64::nodejs:16

and apollo was discarding "::nodejs:16", so it just contained the
bare NERVA. Then later when apollo tries to reconstruct the module data,
its looking for the exact redhat NERVA, which is never going to be true, since
we build the module with a different build ID. So the lookup misses, and the RLSA
packages end up with module_name / stream null even though Red Hat told us the
stream in the CSAF.

Fix: keep the module identity from CSAF at ingest, and when yaml doesn’t match, fall back using a cleaned NEVRA (ignoring the build-id suffix) so Rocky’s rebuild still maps to nodejs:16.

Closes issue resf#72

Signed-off-by: Scott R. Shinn <scott@atomicorp.com>

@rockythorn rockythorn left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Review: Request changes (one blocking issue)

Tests pass (test_rh_matcher_activities, test_rhcsaf), no merge conflicts with main. Code is well-structured.

What works

CSAF parsing: _strip_csaf_product_prefix(), _module_fields_from_purl(), and _parse_csaf_package_product_id() correctly extract module metadata from CSAF product IDs and purls. The AppStream prefix stripping and ::module:stream suffix handling are both tested.

Module resolution fallback: _resolve_modular_package_fields has a clean multi-step fallback: modules.yaml -> RH CSAF advisory data -> enrichment from yaml for missing version/context. The .rocky suffix normalization in _normalize_module_nevra_key handles the Rocky rebuild NEVRA mismatch.

Worker integration: create_or_update_red_hat_advisory_packages correctly handles both new packages (with module fields in bulk_create) and existing packages (selective update only when module fields are missing).

Datetime formats: Adding microsecond format (%f) to parse_datetime prevents crashes on fractional-second timestamps from CSAF.

Blocking issue

Missing database migration. The PR adds 4 columns to RedHatAdvisoryPackage:

module_context = fields.TextField(null=True)
module_name = fields.TextField(null=True)
module_stream = fields.TextField(null=True)
module_version = fields.TextField(null=True)

But these columns don't exist in the actual database:

\d red_hat_advisory_packages
 id                  | bigint
 red_hat_advisory_id | bigint
 nevra               | text

The worker will crash on any INSERT/UPDATE that references these columns. Needs a migration:

ALTER TABLE red_hat_advisory_packages
  ADD COLUMN IF NOT EXISTS module_context text,
  ADD COLUMN IF NOT EXISTS module_name text,
  ADD COLUMN IF NOT EXISTS module_stream text,
  ADD COLUMN IF NOT EXISTS module_version text;

Either include this in the PR or document that it must be applied before deployment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants