sbp commented on code in PR #942:
URL:
https://github.com/apache/tooling-trusted-releases/pull/942#discussion_r2990755812
##########
atr/sbom/utilities.py:
##########
@@ -138,10 +140,35 @@ def patch_to_data(patch_ops: models.patch.Patch) ->
list[dict[str, Any]]:
return [op.model_dump(by_alias=True, exclude_none=True) for op in
patch_ops]
-def path_to_bundle(path: pathlib.Path) -> models.bundle.Bundle:
+def path_to_bundle(path: pathlib.Path) -> models.bundle.Bundle | None:
text = path.read_text(encoding="utf-8")
- bom = models.bom.Bom.model_validate_json(text)
- return models.bundle.Bundle(doc=yyjson.Document(text), bom=bom, path=path,
text=text)
+ bom: Bom | None = None
+ source_type: Literal["json", "xml"] | None = None
+ # Default to latest schema version
+ version_str: str | None = None
+ spec_version: SchemaVersion | None = None
+ if path.name.endswith(".json"):
+ bom_json: dict[str, Any] = json.loads(text)
+ bom = Bom.from_json(data=bom_json)
+ source_type = "json"
+ version_str = bom_json.get('specVersion', '1.7')
+ spec_version = SchemaVersion.from_version(version_str)
+ elif path.name.endswith(".xml"):
+ bom_xml = ElementTree.fromstring(text)
+ bom = Bom.from_xml(bom_xml)
+ tag = re.match(r"\{http://cyclonedx.org/schema/bom/(.+)}", bom_xml.tag)
Review Comment:
Should probably have an `if tag is None` guard after this for a better error
message.
##########
atr/sbom/osv.py:
##########
@@ -165,29 +166,6 @@ def _assemble_vulnerabilities(doc: yyjson.Document,
patch_ops: models.patch.Patc
)
-def _component_purl_with_version(component: models.bom.Component) -> str |
None:
Review Comment:
Why was this deleted? I thought that this was quite useful. Were we finding
that there was a problem with it?
##########
atr/sbom/conformance.py:
##########
@@ -299,19 +304,17 @@ def ntia_2021_issues(
else:
errors.append(models.conformance.MissingProperty(property=models.conformance.Property.METADATA_COMPONENT))
- if bom_value.metadata.author is None:
+ if len(bom_value.metadata.authors) < 1:
errors.append(models.conformance.MissingProperty(property=models.conformance.Property.METADATA_AUTHOR))
-
- if bom_value.metadata.timestamp is None:
Review Comment:
We still need a check for this somehow. The library now backfills this if
it's missing, which I assume was causing the type checker to say this is always
`False`. We need to know if it was there or not before the backfill. This is a
drawback of using the library.
##########
atr/sbom/models/bom.py:
##########
@@ -1,100 +0,0 @@
-# Licensed to the Apache Software Foundation (ASF) under one
Review Comment:
This file was deleted, but `__init__.py` still imports it:
```python
from . import base, bom, bundle, conformance, licenses, osv, patch, sbomqs,
tool
__all__ = ["base", "bom", "bundle", "conformance", "licenses", "osv",
"patch", "sbomqs", "tool"]
```
##########
atr/sbom/utilities.py:
##########
@@ -138,10 +140,35 @@ def patch_to_data(patch_ops: models.patch.Patch) ->
list[dict[str, Any]]:
return [op.model_dump(by_alias=True, exclude_none=True) for op in
patch_ops]
-def path_to_bundle(path: pathlib.Path) -> models.bundle.Bundle:
+def path_to_bundle(path: pathlib.Path) -> models.bundle.Bundle | None:
Review Comment:
I don't think this ever returns `None`. There are some callers that do a
`None` guard, but they don't need to.
##########
atr/sbom/utilities.py:
##########
@@ -138,10 +140,35 @@ def patch_to_data(patch_ops: models.patch.Patch) ->
list[dict[str, Any]]:
return [op.model_dump(by_alias=True, exclude_none=True) for op in
patch_ops]
-def path_to_bundle(path: pathlib.Path) -> models.bundle.Bundle:
+def path_to_bundle(path: pathlib.Path) -> models.bundle.Bundle | None:
text = path.read_text(encoding="utf-8")
- bom = models.bom.Bom.model_validate_json(text)
- return models.bundle.Bundle(doc=yyjson.Document(text), bom=bom, path=path,
text=text)
+ bom: Bom | None = None
+ source_type: Literal["json", "xml"] | None = None
+ # Default to latest schema version
+ version_str: str | None = None
+ spec_version: SchemaVersion | None = None
+ if path.name.endswith(".json"):
+ bom_json: dict[str, Any] = json.loads(text)
+ bom = Bom.from_json(data=bom_json)
+ source_type = "json"
+ version_str = bom_json.get('specVersion', '1.7')
Review Comment:
Single quotes.
##########
atr/sbom/cli.py:
##########
@@ -20,6 +20,10 @@
import sys
import yyjson
+from cyclonedx.output import make_outputter
+from cyclonedx.schema import OutputFormat
+from cyclonedx.schema.schema import SchemaVersion1Dot7, SCHEMA_VERSIONS
Review Comment:
`SCHEMA_VERSIONS` is unused.
##########
atr/sbom/cli.py:
##########
@@ -57,10 +61,15 @@ def command_merge(bundle: models.bundle.Bundle) -> None:
patch_ops = asyncio.run(bundle_to_ntia_patch(bundle))
if patch_ops:
patch_data = patch_to_data(patch_ops)
- merged = bundle.doc.patch(yyjson.Document(patch_data))
- print(merged.dumps())
+ output = bundle.doc.patch(yyjson.Document(patch_data))
+ else:
+ output = bundle.doc
+ if bundle.source_type == "json":
+ print(output.dumps())
else:
- print(bundle.doc.dumps())
+ print(make_outputter(
+ Bom.from_json(data=output.as_obj), OutputFormat.XML,
bundle.spec_version
Review Comment:
Probably a good idea to construct the `Bom.from_json` outside and then check
whether it's `None`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
