-
Notifications
You must be signed in to change notification settings - Fork 95
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
Add SBOM files for applicable components #212
Conversation
Please see more details at: https://github.com/espressif/esp-idf-sbom
@fhrbata PTAL |
@@ -1,4 +1,5 @@ | |||
version: "2.5.0" | |||
# Note: Please keep this version consistent with sbom.yml file | |||
version: "2.5.0~1" |
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.
Hi @mahavirj , I'm a little bit confused and probably missing something. Why are we changing the versions in idf_component.yml
? For example here from 2.5.0
to 2.5.0~1
. Also just a note, if the version is not found in sbom.yml
it will be take from idf_component.yml
if presented. The same goes for description. So maybe we can avoid the duplication. Now I'm starting to think that maybe we should ask @kumekay if it would be possible to add the sbom fields support into idf_component.yml
so we do not have to keep the info on two places for the managed components. Just a thought. Thank you.
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.
Since we are adding a new file to this component, we must bump up its version so that it gets pushed to the upstream registry.
Also just a note, if the version is not found in sbom.yml it will be take from idf_component.yml if presented.
Okay, in that case, I can remove version
field from the sbom.yml
file. Probably we can remove description field as well?
it would be possible to add the sbom fields support into idf_component.yml so we do not have to keep the info on two places for the managed components
That makes sense.
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.
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.
It would also make the implementation of automatic version updates (#174) a lot easier if there are less files which have to be modified for each version bump.
For now, espressif/github-actions#40 has added support for updating the submodule itself and the version in idf_component.yml. Having to update also sbom-version
and sbom-hash
in .gitmodules, plus the new sbom.yml file, definitely makes the implementation more complex...
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.
By the way, do I understand it right that after the SBOM information is moved into sbom.yml, we will revert the .gitmodules changes done in #199? I'm trying to understand whether I have to implement automatic updates to sbom-version and sbom-hash entries from update_submodule_versions action.
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.
@mahavirj , @kumekay I quickly tried, with idf-component-manager 1.3.2, some local test managed component and I can confirm that the root variables are used and not reported. With e.g. 1.2.3
I get this
Invalid manifest format
Unknown keys: cpe, originator, supplier
SUGGESTION: This component may be using a newer version of the component manager.
I just wanted to point this out, because if we update idf_component.yml
files, people with older manager will run into this. I'm not sure if this is(could be) a problem.
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.
have a special sbom namespace
This could be an override switch as well. If the version is present under sbom namespace then pick that one, else use the component version field itself.
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 could be an override switch as well. If the version is present under sbom namespace then pick that one, else use the component version field itself.
Don't we actually have two libraries, from SBOM point of view, here?
flowchart TD
app["Application"]
espressif/libpng["espressif/libpng\n1.6.39~2"]
libpng["libpng\n1.6.39"]
app -- depends --> espressif/libpng
espressif/libpng -- depends --> libpng
And in some cases, there could be more than one library inside a component. For example, console
component in IDF:
flowchart TD
app["Application"]
console["$IDF_PATH/console"]
argtable3["argtable3\n3.2.2"]
linenoise["linenoise\n1.0.1"]
app -- depends --> console
console -- depends --> argtable3
console -- depends --> linenoise
Not saying we should handle such cases in this MR, just thought it's relevant to the discussion.
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.
Hi @igrr , thank you for the graphs!
IIUC the problem we are trying to solve is that even though we have the sbom information for submodules in .gitmodules
, it's of course not propagated through component manager. So if I do
idf.py add-dependency "espressif/libpng^1.6.39"
the libpng component added to my project has no info that the libpng
subdir in managed_components/espressif__libpng/
is actually a submodule in the idf-extra-components
repo with sbom info for it in .gitmodules
.
I agree with your point above, that from the sbom POV the libpng component should depend on libpng library, but this relationship is currently not possible to express in the tool :(. It works like presented in your graph if we have submodule entry for the libpng library available in .gitmodules
and .gitmodules
available while the sbom is generated.
For the first libpng
component example, I think we have to extend the sbom.yml to allow to express this relationship. Maybe something like this added in idf_component.yml
sbom:
contains:
- path: libpng
name: libpng
version: 1.6.39
cpe: cpe:2.3:a:libpng:libpng:{}:*:*:*:*:*:*:*
supplier: 'Organization: libpng'
description: Portable Network Graphics support, official PNG reference library
This would actually replace the sbom data and component->submodule relationship from .gitmodules
.
For the second console
component example, I think we can use the same approach as for libpng
. We would just add two libs: argtable3
and linenoise
. Also IIUC argtable3
and linenoise
are actually not submodules, so maybe we can added their own sbom.yml into their dirs.
components/console/
├── argtable3
│ └── sbom.yml
├── linenoise
│ └── sbom.yml
└── sbom.yml
Anyway both approaches require changes in the tool. I'm open and grateful for any ideas WRT this. Thank you very much!
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.
@fhrbata Proposal sounds good to me. Please let me know once you have tool side changes in place, alternatively please feel free to take over this PR. Thanks.
Hi @mahavirj , Here #239 is a PR, which adds the sbom information using referenced manifest files. It a draft, because we also need to change the checking in CI and probably remove the sbom information from Thank you |
Closing in favor or #239 |
Please see more details at:
https://github.com/espressif/esp-idf-sbom
Checklist
Change description
Please describe your change here