Adding optional RPM summary to SBOMs #762

a-ovchinnikov · 2024-12-04T03:43:44Z

This change adds an option to include RPM summary in a SBOM.

Maintainers will complete the following section

Commit messages are descriptive enough
Code coverage from testing does not decrease and new code is covered
Docs updated (if applicable)
Docs links in the code are still valid (if docs were updated)

Note: if the contribution is external (not from an organization member), the CI
pipeline will not run automatically. After verifying that the CI is safe to run:

approve GitHub Actions workflows by clicking a button
approve the Red Hat Trusted App Pipeline container build by commenting /ok-to-test
(as is the standard for Pipelines as Code)

cachi2/core/models/property_semantics.py

taylormadore · 2024-12-11T20:40:38Z

cachi2/core/models/sbom.py

@@ -7,6 +7,7 @@
 PropertyName = Literal[
    "cachi2:bundler:package:binary",
    "cachi2:found_by",
+    "cachi2:rpm_summary",


Should we use a custom cachi2 property or perhaps the description field for a CycloneDX Component instead?

Maybe. I'll need to think about it more. A Property looks the least invasive way of doing this.

If we decide to go the component.description path, we might be able to map it to SPDX similarly:
https://spdx.github.io/spdx-spec/v2.3/package-information/#718-package-summary-description-field

FWIW properties can be straightforwardly converted to annotations.

Should we use a custom cachi2 property or perhaps the description field for a CycloneDX Component instead?

Description feels more natural, but then we need to sit down and define what description is expected to represent a particular PM backend's component to remain consistent across all backends. In that case, also for consistency reasons, we should also introduce the same functionality to the rest of the backends in a short timeline span in order not to leave the rest of PMs behind; note that a custom property IMO feels like less of a problem for consistency than making use of an existing CycloneDX field suited directly for this purpose.

@taylormadore would you care enough to create an issue on investigating the possibility of adopting a more general approach covering all backends via the built-in description field? I like the idea in general and I feel we should invest some time into exploring the possibility before ruling it out completely.

cachi2/core/models/input.py

cachi2/core/package_managers/rpm/main.py

brunoapimentel · 2025-01-08T19:58:00Z

/retest

eskultety

One major comment, otherwise we're pretty much set for merging, one last respin will be needed.

cachi2/core/models/property_semantics.py

cachi2/core/package_managers/rpm/main.py

eskultety · 2025-01-09T12:56:00Z

cachi2/core/models/input.py

@@ -216,6 +216,7 @@ class RpmPackageInput(_PackageInputBase):
    """Accepted input for a rpm package."""

    type: Literal["rpm"]
+    add_rpm_summary: bool = False


nitpick - it's part of RpmPackageInput already so the 'rpm' part of the option name is redundant, but at the same time the name itself doesn't indicate this is an SBOM only flag - what about sbom_inlude_summary OR include_summary_in_sbom? What I'm after is some kind of naming pattern that could be reasonably reusable for other such flags we might be required to add, IOW include_XYZ_in_sbom.

same naming nitpick - I apologize for not having been clearer in my initial response, but I having "sbom" somehow mentioned in the option would be beneficial. If this were an internal variable I would have resolved the thread already, but sadly this is an input JSON option which isn't properly documented (I mean you can't really run a help on the input json like you would on other CLI options) and so having a clear descriptive name on the option so that users immediately know what to expect from the flag is a huge plus IMO.

Good point, done.

brunoapimentel · 2025-01-15T23:10:24Z

/retest

eskultety

Approve, but please try to address the input JSON option naming nitpick I raised I think it's justified for users sake.

eskultety · 2025-01-16T09:11:12Z

tests/integration/test_data/rpm_summary_is_reported/bom.json

+          "value": "cachi2"
+        },
+        {
+            'name': 'cachi2:rpm_summary',


nitpick: single quotes instead of double quotes (multiple occurrences). I would not even notice if GH UI wouldn't flag this in red for me, that's why I'm mentioning it (it's also the only bom.json with this pattern so it would be inconsistent :) ).

eskultety · 2025-01-16T09:15:16Z

cachi2/core/models/input.py

@@ -216,6 +216,7 @@ class RpmPackageInput(_PackageInputBase):
    """Accepted input for a rpm package."""

    type: Literal["rpm"]
+    add_rpm_summary: bool = False


same naming nitpick - I apologize for not having been clearer in my initial response, but I having "sbom" somehow mentioned in the option would be beneficial. If this were an internal variable I would have resolved the thread already, but sadly this is an input JSON option which isn't properly documented (I mean you can't really run a help on the input json like you would on other CLI options) and so having a clear descriptive name on the option so that users immediately know what to expect from the flag is a huge plus IMO.

This change adds an option to include RPM summary in a SBOM. Signed-off-by: Alexey Ovchinnikov <[email protected]>

a-ovchinnikov requested review from eskultety and brunoapimentel and removed request for eskultety December 4, 2024 10:57

taylormadore reviewed Dec 11, 2024

View reviewed changes

a-ovchinnikov force-pushed the rpm_adding_summary branch from 660aada to 4f7631b Compare December 12, 2024 18:33

eskultety reviewed Dec 13, 2024

View reviewed changes

cachi2/core/models/input.py Outdated Show resolved Hide resolved

eskultety reviewed Jan 2, 2025

View reviewed changes

cachi2/core/package_managers/rpm/main.py Outdated Show resolved Hide resolved

a-ovchinnikov marked this pull request as ready for review January 2, 2025 18:41

a-ovchinnikov changed the title ~~WIP: Adding optional RPM summary to SBOMs~~ Adding optional RPM summary to SBOMs Jan 2, 2025

a-ovchinnikov force-pushed the rpm_adding_summary branch from 4f7631b to bdc850d Compare January 2, 2025 21:33

eskultety reviewed Jan 9, 2025

View reviewed changes

a-ovchinnikov force-pushed the rpm_adding_summary branch from bdc850d to 376c6f2 Compare January 14, 2025 23:30

brunoapimentel approved these changes Jan 15, 2025

View reviewed changes

eskultety approved these changes Jan 16, 2025

View reviewed changes

Adding optional RPM summary to SBOMs

30b574d

This change adds an option to include RPM summary in a SBOM. Signed-off-by: Alexey Ovchinnikov <[email protected]>

a-ovchinnikov force-pushed the rpm_adding_summary branch from 376c6f2 to 30b574d Compare January 16, 2025 10:47

a-ovchinnikov added this pull request to the merge queue Jan 16, 2025

Merged via the queue into hermetoproject:main with commit 38f8cd9 Jan 16, 2025
14 of 15 checks passed

a-ovchinnikov deleted the rpm_adding_summary branch January 16, 2025 11:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding optional RPM summary to SBOMs #762

Adding optional RPM summary to SBOMs #762

a-ovchinnikov commented Dec 4, 2024 •

edited

Loading

taylormadore Dec 11, 2024

a-ovchinnikov Dec 12, 2024

brunoapimentel Dec 12, 2024

a-ovchinnikov Dec 12, 2024

eskultety Jan 2, 2025

eskultety Jan 9, 2025 •

edited

Loading

brunoapimentel commented Jan 8, 2025

eskultety left a comment

eskultety Jan 9, 2025

eskultety Jan 16, 2025

a-ovchinnikov Jan 16, 2025

brunoapimentel commented Jan 15, 2025

eskultety left a comment

eskultety Jan 16, 2025

a-ovchinnikov Jan 16, 2025

eskultety Jan 16, 2025

Adding optional RPM summary to SBOMs #762

Adding optional RPM summary to SBOMs #762

Conversation

a-ovchinnikov commented Dec 4, 2024 • edited Loading

Maintainers will complete the following section

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eskultety Jan 9, 2025 • edited Loading

Choose a reason for hiding this comment

brunoapimentel commented Jan 8, 2025

eskultety left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brunoapimentel commented Jan 15, 2025

eskultety left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

a-ovchinnikov commented Dec 4, 2024 •

edited

Loading

eskultety Jan 9, 2025 •

edited

Loading