Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding optional RPM summary to SBOMs #762

Merged
Merged
Show file tree
Hide file tree
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
1 change: 1 addition & 0 deletions cachi2/core/models/input.py
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ class RpmPackageInput(_PackageInputBase):
"""Accepted input for a rpm package."""

type: Literal["rpm"]
include_summary_in_sbom: bool = False
options: Optional[ExtraOptions] = None


Expand Down
8 changes: 8 additions & 0 deletions cachi2/core/models/property_semantics.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ class PropertySet:
npm_development: bool = False
pip_package_binary: bool = False
bundler_package_binary: bool = False
rpm_summary: str = ""

@classmethod
def from_properties(cls, props: Iterable[Property]) -> "Self":
Expand All @@ -44,6 +45,7 @@ def from_properties(cls, props: Iterable[Property]) -> "Self":
npm_development = False
pip_package_binary = False
bundler_package_binary = False
rpm_summary = ""

for prop in props:
if prop.name == "cachi2:found_by":
Expand All @@ -58,6 +60,8 @@ def from_properties(cls, props: Iterable[Property]) -> "Self":
pip_package_binary = True
elif prop.name == "cachi2:bundler:package:binary":
bundler_package_binary = True
elif prop.name == "cachi2:rpm_summary":
rpm_summary = prop.value
else:
assert_never(prop.name)

Expand All @@ -68,6 +72,7 @@ def from_properties(cls, props: Iterable[Property]) -> "Self":
npm_development,
pip_package_binary,
bundler_package_binary,
rpm_summary,
)

def to_properties(self) -> list[Property]:
Expand All @@ -87,6 +92,8 @@ def to_properties(self) -> list[Property]:
props.append(Property(name="cachi2:pip:package:binary", value="true"))
if self.bundler_package_binary:
props.append(Property(name="cachi2:bundler:package:binary", value="true"))
if self.rpm_summary:
props.append(Property(name="cachi2:rpm_summary", value=self.rpm_summary))

return sorted(props, key=lambda p: (p.name, p.value))

Expand All @@ -100,4 +107,5 @@ def merge(self, other: "Self") -> "Self":
npm_development=self.npm_development and other.npm_development,
pip_package_binary=self.pip_package_binary or other.pip_package_binary,
bundler_package_binary=self.bundler_package_binary or other.bundler_package_binary,
rpm_summary=self.rpm_summary or other.rpm_summary,
)
1 change: 1 addition & 0 deletions cachi2/core/models/sbom.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
PropertyName = Literal[
"cachi2:bundler:package:binary",
"cachi2:found_by",
"cachi2:rpm_summary",
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

FWIW properties can be straightforwardly converted to annotations.

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member

@eskultety eskultety Jan 9, 2025

Choose a reason for hiding this comment

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

@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:missing_hash:in_file",
"cachi2:pip:package:binary",
"cdx:npm:package:bundled",
Expand Down
24 changes: 20 additions & 4 deletions cachi2/core/package_managers/rpm/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ class Package:
vendor: Optional[str] = None
checksum: Optional[str] = None
repository_id: Optional[str] = None
summary: Optional[str] = None

@classmethod
def from_filepath(cls, rpm_filepath: Path, rpm_download_metadata: dict[str, Any]) -> "Package":
Expand Down Expand Up @@ -85,6 +86,7 @@ def _query_rpm_fields(file_path: Path) -> dict[str, str]:
"version=%{VERSION}\n"
"release=%{RELEASE}\n"
"arch=%{ARCH}\n"
"summary=%{SUMMARY}\n"
# vendor and epoch are optional RPM tags; return "" if not set instead of "(None)"
"vendor=%|VENDOR?{%{VENDOR}}:{}|\n"
"epoch=%|EPOCH?{%{EPOCH}}:{}|\n"
Expand Down Expand Up @@ -205,7 +207,14 @@ def fetch_rpm_source(request: Request) -> RequestOutput:

for package in request.rpm_packages:
path = request.source_dir.join_within_root(package.path)
components.extend(_resolve_rpm_project(path, request.output_dir, options=package.options))
components.extend(
_resolve_rpm_project(
path,
request.output_dir,
options=package.options,
include_summary_in_sbom=package.include_summary_in_sbom,
)
)

# FIXME: this is only ever good enough for a PoC, but needs to be handled properly in the
# future.
Expand Down Expand Up @@ -240,6 +249,7 @@ def _resolve_rpm_project(
source_dir: RootedPath,
output_dir: RootedPath,
options: Optional[ExtraOptions] = None,
include_summary_in_sbom: bool = False,
) -> list[Component]:
"""
Process a request for a single RPM source directory.
Expand Down Expand Up @@ -288,7 +298,7 @@ def _resolve_rpm_project(
_verify_downloaded(metadata)

lockfile_relative_path = source_dir.subpath_from_root / DEFAULT_LOCKFILE_NAME
return _generate_sbom_components(metadata, lockfile_relative_path)
return _generate_sbom_components(metadata, lockfile_relative_path, include_summary_in_sbom)


def _download(
Expand Down Expand Up @@ -383,14 +393,20 @@ def _is_rpm_file(file_path: Path) -> bool:


def _generate_sbom_components(
files_metadata: dict[Path, Any], lockfile_path: Path
files_metadata: dict[Path, Any],
lockfile_path: Path,
include_summary_in_sbom: bool = False,
) -> list[Component]:
components = []
for file_path, file_metadata in files_metadata.items():
if not _is_rpm_file(file_path):
continue
package = Package.from_filepath(file_path, file_metadata)
components.append(package.to_component(lockfile_path))
component = package.to_component(lockfile_path)
if include_summary_in_sbom:
summary = Property(name="cachi2:rpm_summary", value=str(package.summary))
component.properties.append(summary)
components.append(component)
return components


Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
environment_variables: []
project_files: []
91 changes: 91 additions & 0 deletions tests/integration/test_data/rpm_summary_is_reported/bom.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
{
"bomFormat": "CycloneDX",
"components": [
{
"name": "glibc-common",
"properties": [
{
"name": "cachi2:found_by",
"value": "cachi2"
},
{
"name": "cachi2:rpm_summary",
"value": "Common binaries and locale data for glibc"
}
],
"purl": "pkg:rpm/fedora/[email protected]?arch=x86_64&checksum=sha256:45fe79ffea9358fc7a4f233e2358b08678bdec476680d0655063b4a4058e8789&repository_id=releases",
"type": "library",
"version": "2.39"
},
{
"name": "glibc-common",
"properties": [
{
"name": "cachi2:found_by",
"value": "cachi2"
},
{
"name": "cachi2:missing_hash:in_file",
"value": "another-project/rpms.lock.yaml"
}
],
"purl": "pkg:rpm/fedora/[email protected]?arch=x86_64&repository_id=releases",
"type": "library",
"version": "2.39"
},
{
"name": "glibc-minimal-langpack",
"properties": [
{
"name": "cachi2:found_by",
"value": "cachi2"
}
],
"purl": "pkg:rpm/fedora/[email protected]?arch=x86_64&checksum=sha256:6f9b45618d3b46fbbfb44407e05dd7054f3bd6a4df4f7291b576858b88deadc5&repository_id=releases",
"type": "library",
"version": "2.38"
},
{
"name": "glibc-minimal-langpack",
"properties": [
{
"name": "cachi2:found_by",
"value": "cachi2"
},
{
"name": "cachi2:rpm_summary",
"value": "Minimal language packs for glibc."
}
],
"purl": "pkg:rpm/fedora/[email protected]?arch=x86_64&checksum=sha256:9982f68ddcc3e972a2f3f220f29d56b0e9dde0e409bdecfe0bc559fe39013dde&repository_id=releases",
"type": "library",
"version": "2.39"
},
{
"name": "gzip",
"properties": [
{
"name": "cachi2:found_by",
"value": "cachi2"
},
{
"name": "cachi2:rpm_summary",
"value": "GNU data compression program"
}
],
"purl": "pkg:rpm/fedora/[email protected]?arch=x86_64&checksum=sha256:6dcc2f8885135fc873c8ab94a6c7df05883060c5b25287956bebb3aa15a84e71&repository_id=releases",
"type": "library",
"version": "1.13"
}
],
"metadata": {
"tools": [
{
"name": "cachi2",
"vendor": "red hat"
}
]
},
"specVersion": "1.4",
"version": 1
}
16 changes: 16 additions & 0 deletions tests/integration/test_rpm.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,22 @@
reason="CACHI2_TEST_LOCAL_DNF_SERVER!=true",
),
),
pytest.param(
utils.TestParameters(
repo="https://github.com/cachito-testing/cachi2-rpm",
ref="multiple-packages",
packages=(
{"path": "this-project", "type": "rpm", "include_summary_in_sbom": "true"},
{"path": "another-project", "type": "rpm"},
),
flags=["--dev-package-managers"],
check_output=True,
check_deps_checksums=False,
check_vendor_checksums=False,
expected_exit_code=0,
),
id="rpm_summary_is_reported",
),
],
)
def test_rpm_packages(
Expand Down
5 changes: 5 additions & 0 deletions tests/unit/models/test_input.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ class TestPackageInput:
"type": "rpm",
"path": Path("."),
"options": None,
"include_summary_in_sbom": False,
},
),
(
Expand All @@ -80,6 +81,7 @@ class TestPackageInput:
"foorepo": {"arch": "x86_64", "enabled": True},
}
},
"include_summary_in_sbom": False,
},
{
"type": "rpm",
Expand All @@ -91,6 +93,7 @@ class TestPackageInput:
},
"ssl": None,
},
"include_summary_in_sbom": False,
},
),
(
Expand All @@ -110,6 +113,7 @@ class TestPackageInput:
"ssl_verify": False,
},
},
"include_summary_in_sbom": False,
},
),
(
Expand Down Expand Up @@ -138,6 +142,7 @@ class TestPackageInput:
"ssl_verify": False,
},
},
"include_summary_in_sbom": False,
},
),
],
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/package_managers/test_rpm.py
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ def test_resolve_rpm_project(
mock_model_validate.return_value, mock_package_dir_path, None
)
mock_verify_downloaded.assert_called_once_with({})
mock_generate_sbom_components.assert_called_once_with({}, Path("rpms.lock.yaml"))
mock_generate_sbom_components.assert_called_once_with({}, Path("rpms.lock.yaml"), False)


@mock.patch("cachi2.core.package_managers.rpm.main.run_cmd")
Expand Down
Loading