-
-
Notifications
You must be signed in to change notification settings - Fork 50
feat: add support for component's evidences according to spec #810
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
base: main
Are you sure you want to change the base?
Conversation
Thank you for providing this feature, @OxPirates could you
|
@jkowalleck Thank you for your time, Will work on it and submit the changes. |
@jkowalleck Thank you for the detailed review. All comments have been addressed except the one regarding ToolReference |
cyclonedx/model/component.py
Outdated
@tools.setter | ||
def tools(self, tools: Iterable[Union[str, BomRef]]) -> None: | ||
"""Convert all inputs to BomRef for consistent storage""" | ||
self._tools = SortedSet(BomRef(str(t)) for t in tools) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hm, per specs, this is a list of (string or BomRef).
https://cyclonedx.org/docs/1.6/json/#components_items_evidence_identity_oneOf_i0_items_tools
so it seams like forcing everything to be a BomRef
is not the intended solution, or is it?
FYI: We already have some code that might help validate BomLinks.
I need to think about this a bit more.
lets leave the "tools" topic open for a while :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! I've pushed a minor update based on the feedback. Let me know if you have any final thoughts—happy to make further adjustments if needed.
tests/_data/models.py
Outdated
@@ -1485,4 +1554,5 @@ def get_bom_for_issue540_duplicate_components() -> Bom: | |||
get_bom_with_lifecycles, | |||
get_bom_with_definitions_standards, | |||
get_bom_with_definitions_and_detailed_standards, | |||
get_bom_with_component_evidence, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please dont modify this existing model.
- craft a new own data model, instead of reusing an existing one! Create an own function called
get_bom_with_component_evidence()
- it will be found and used automatically.
tests/_data/models.py
Outdated
) | ||
], | ||
tools=[ | ||
BomRef('ref0'), # BomRef reference |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is not how this should work.
instead of rafting a new BomRef
for this single use,
the spec clearly says to reference an already defined one that was used in metadata.tools
.
https://cyclonedx.org/docs/1.6/json/#components_items_evidence_identity_oneOf_i0_items_tools
The object in the BOM identified by its bom-ref. This is often a component or service but may be any object type supporting bom-refs. Tools used for analysis should already be defined in the BOM, either in the metadata/tools, components, or formulation.
tests/_data/models.py
Outdated
offset=16, | ||
symbol='exampleSymbol', | ||
additional_context='Found in source code', | ||
bom_ref=bom_ref, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is the bom_ref
used here? it seams to have no purpose - can it be removed?
bom_ref=bom_ref, |
tests/_data/models.py
Outdated
@@ -737,6 +773,55 @@ def get_component_setuptools_complete(include_pedigree: bool = True) -> Componen | |||
return component | |||
|
|||
|
|||
def get_component_evidence_basic(tool: BomRef, bom_ref: str) -> ComponentEvidence: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
def get_component_evidence_basic(tool: BomRef, bom_ref: str) -> ComponentEvidence: | |
def get_component_evidence_basic(tools: Iterable[Tool]) -> ComponentEvidence: |
tests/_data/models.py
Outdated
confidence=Decimal('0.8'), value='analysis-tool' | ||
) | ||
], | ||
tools=[tool] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tools=[tool] | |
tools=[tool.bom_ref for tool in tools] |
tests/_data/models.py
Outdated
licenses=[DisjunctiveLicense(id='MIT')], | ||
author='Test Author' | ||
) | ||
component.evidence = get_component_evidence_basic(tool=tool_component.bom_ref, bom_ref='ref:setuptools') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
component.evidence = get_component_evidence_basic(tool=tool_component.bom_ref, bom_ref='ref:setuptools') | |
component.evidence = get_component_evidence_basic(tools=[tool_component]) |
Signed-off-by: Arun <[email protected]>
fixes #737