Add new command sbom:cyclonedx

[hedtke]

This replaces the proposal conan-io/conan#14405

It closes conan-io/conan#9787

It is a possible replacement for https://github.com/CycloneDX/cyclonedx-conan

[memsharded]

If this could be a replacement for https://github.com/CycloneDX/cyclonedx-conan, it would be great to ping them and make them aware explicitly, maybe a ticket in their repo, to potentially centralize efforts somewhere.

As the proposal is a new custom command, it is really not critical where is it, because it can be integrated easily with conan config install <url>, so it could be either centralized in the CycloneDX org, or here in the Conan org, but as it seems they already put a great effort there, it makes sense to align with them.

[memsharded]

Ok, I see that you already did in CycloneDX/cyclonedx-conan#84. Commenting there.

[jkowalleck]

Hi, I am the maintainer of https://github.com/CycloneDX/cyclonedx-conan,
and a member of OWASP's CycloneDX Core Working Group

for the following reasons, i would invite you to chat and discuss in CycloneDX slack channel #conan.
Join via invite link https://cyclonedx.org/slack/invite.

I see some misconceptions and missing spots in this PR:

• missing metadata.component describing the thing the SBOM was built for
• dependency graph has no root, not connections, just nodes
• optional component's namespace appears to be missing
• invalid component license values might render the whole JSON invalid

[jkowalleck]

re: #66 (comment)

[...] it could be either centralized in the CycloneDX org, or here in the Conan org, but as it seems they already put a great effort there, it makes sense to align with them.

I will discuss with the CycloneDX Core team a possible adoption of a Coann2 extension.
I will keep you updated here: CycloneDX/cyclonedx-conan#84 (comment)

PS: see the response here CycloneDX/cyclonedx-conan#84 (comment)

[andreas-hilti]

It would also be very helpful to add at least one or two tests.

[coderpatros]

If this could be a replacement for https://github.com/CycloneDX/cyclonedx-conan, it would be great to ping them and make them aware explicitly, maybe a ticket in their repo, to potentially centralize efforts somewhere.

As the proposal is a new custom command, it is really not critical where is it, because it can be integrated easily with conan config install <url>, so it could be either centralized in the CycloneDX org, or here in the Conan org, but as it seems they already put a great effort there, it makes sense to align with them.

IMHO it would be better for this functionality to be maintained in the Conan ecosystem rather than the CycloneDX project. Package managers and build systems are complicated. And they all behave a bit differently. IMO it's much more likely to attract the required expertise and experience here. The CycloneDX team are happy to provide support, feedback, answer questions, etc.

[hedtke]

It would also be very helpful to add at least one or two tests.

Test added

[hedtke]

Ready for next review round. I think I addressed everything. Thank you all for the helpful comments!

[hedtke]

I added support for cyclonedx-python-lib 3 which is the latest major version supported for python 3.6

[hedtke]

Not easy to get everything right. I found a way to differentiate between cyclonedx v3 (for Python 3.6) and v4 (for later) without using importlib.metadata (since Python 3.8) and without cyclonedx.__version__, see CycloneDX/cyclonedx-python-lib#416

there are lots of changes since the tick

[prince-chrismc]

Unfortunately I do not have the knowledge to review this at a technical level. At a high level this looks really good with my background using them in the past. I like the CLI for this is the most intuitive so far, I think it's a great start 👏 I also like the pattern for using extern deps + the error message - very well done 👍

As for the code, and getting this merged we'll need to wait for someone of the team to make that decision :)

[jkowalleck]

I also like the pattern for using extern deps + the error message - very well done

Thanks. :D
I think this is a pattern that extensions could use.
I encapsulated almost all the code into the extension's runner function, including the imports. This way, nothing is imported or created when the extension is loaded.
Only when the extension is executed, then the needed externals are probed.

[memsharded]

I think this is ready to be merged from your side, it just needs a first quick round of internal testing and feedback from our side.

We will play a little bit with it, probably try to refactor a bit the code to be more aligned with some of our style, and consider and think about some changes like the "layer" ("recipe") name. Sorry we haven't managed to do this yet, we have been swamped, but we haven't forgot about this PR. Thanks!

[memsharded]

Why --sbom_format and not the generic --format common to all Conan commands?

Maybe to be able to embed the imports in the command?

[jkowalleck]

actually there was a pythonic and pretty implementation for this,
at state 4ce29c0

conan-extensions/extensions/commands/recipe/cmd_create_sbom.py

Lines 16 to 29 in 4ce29c0

	def format_cyclonedx_14_json(bom: 'Bom') -> None:
	from cyclonedx.output.json import JsonV1Dot4
	serialized_json = JsonV1Dot4(bom).output_as_string()
	cli_out_write(serialized_json)
	def format_text(_: Any) -> None:
	raise ConanException("Format 'text' not supported")
	@conan_command(group="Recipe", formatters={
	"text": format_text, # added by default in BaseConanCommand.__init__
	"cyclonedx_1.4_json": format_cyclonedx_14_json})
	def create_sbom(conan_api: ConanAPI, parser, *args) -> 'Bom':

... but it got changed to what it is now ... and not for the better, i would say.

[hedtke]

Changed back to use the formatters

[memsharded]

I think we still need to look for a better "layer" name than "recipe"

[jkowalleck]

i'd suggest conan sbom:cyclonedx --format 1.4_xml as stated here: #66 (comment)

[hedtke]

OK, I changed the name of the command to conan sbom:cyclonedx --format 1.4_xml

[hedtke]

Ready for the next review round

[memsharded]

Ok, so this PR is merged, thanks very much all @hedtke for the contributions and all the reviewers!

This is a great first step. We still have some work ahead, while the APIs stabilize, there will probably be breakages that we will need to fix. We would like also take the chance of future fixes and other tasks to probably go over the code and propose some refactors or style changes to align a bit better with the rest of the Conan codebases, but that shouldn't be very relevant.

[memsharded]

For new features, command args changes, etc (anything beyond bugfixes), I'd recommend submitting issues, discussing there the design, etc, and then move to PRs after alignement.