suggestion: convert command initial #33

houdini91 · 2024-04-20T20:17:18Z

Hello everyone,

As a contributor to the core Protobom library, I'm exploring the integration of a conversion command into bomctl, similar to the implementation found in https://github.com/bom-squad/sbom-convert. I hope this addition aligns with project guidelines and doesn't overstep any boundaries. Please feel free to review, suggest changes, or provide feedback on the proposed pull request.

The pull request introduces a Convert Command, which empowers users to seamlessly switch between SBOM formats.

To give it a try, execute the following command:

convert https://.../bom-1.5.cdx.json --output-file bom.spdx.json --format spdx

If --format is omitted, the command automatically inverts the detected format based on the provided SBOM input.

Here's the current command's usage:

  bomctl convert [flags] SBOM_URL...

Flags:
  -e, --encoding FILE      the output encoding [spdx: [text, json] cyclonedx: [json]
  -f, --format FILE        the output format [spdx, spdx-2.3, cyclonedx, cyclonedx-1.0, cyclonedx-1.1, cyclonedx-1.2, cyclonedx-1.3, cyclonedx-1.4, cyclonedx-1.5]
  -h, --help               help for convert
      --netrc              Use .netrc file for authentication to remote hosts
  -o, --output-file FILE   Path to output file (default )

Fixes # (issue)

Type of change

New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Currently not tested TBD

Checklist

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes
Any dependent changes have been merged and published in downstream modules
I have checked my code and corrected any misspellings

Signed-off-by: houdini91 <mdstrauss91@gmail.com>

jhoward-lm · 2024-04-20T21:18:31Z

@houdini91 thanks for the contribution! Looking forward to checking it out.

Just a couple of upcoming things I want to highlight because they might impact these changes:

very minor thing, there’s a pending PR to enforce conventional commit syntax for PR titles
the ent changes submitted to the new protobom/storage project might have a much larger impact here if/when they get merged. I’ll take a closer look when I can; @puerco is also working on design docs for the interfaces for translating between the current protobom types and their corresponding ent types, I believe as protobom reader and writer methods

Thanks again! We can discuss it further as a group in the SBOM tooling call on Monday

houdini91 · 2024-04-21T06:18:44Z

Sure, could you share the meeting schedule? I can't seem to find it in the OpenSSF calendar.

I'll review the lint PR to ensure I'm aligned with it.
Certainly, I wasn't aware of this storage repo. Let's gather more feedback.

In my opinion, the changes mainly affect how the protobuf document can be stored, whereas the convert command currently operates on the in-memory object. Could you explain how you see this change impacting the implementation?

jhoward-lm · 2024-04-21T20:04:30Z

Sure, could you share the meeting schedule? I can't seem to find it in the OpenSSF calendar.

@idunbarh can you invite Mikey to the bomctl working group meeting?

In my opinion, the changes mainly affect how the protobuf document can be stored, whereas the convert command currently operates on the in-memory object. Could you explain how you see this change impacting the implementation?

They also affect methods of interaction with the protoboms. One of the driving ideas for bomctl is to persist and save changes in the underlying database, and I see bomctl's interactions with the protobom types as being limited to parsing JSON data from remote URLs, translating between protobom and ent database representations, and writing a JSON file to disk.

I think the convert logic might fit in as something like:

user runs bomctl fetch to pull a remote document, then either
- specifies the --output-file option to fetch, with an additional option such as --format [cyclonedx|spdx] (doesn't exist yet)
- runs an additional command such as bomctl export or bomctl save (doesn't exist yet) that would either accept an SBOM_ID argument to pull from the database or accept piped input from bomctl fetch, with an additional option such as --format [cyclonedx|spdx]

I think it would be cool to have both options, personally. Although I'm not sure how it would work with a "tree" of SBOMs referenced by the SBOM specified.

idunbarh · 2024-04-22T00:32:36Z

@houdini91 it was great briefly meeting you in person at OSS NA last week. Puerco and you were talking about selection of the protobom migration date.

The meeting is the SBOM Tooling Working Meeting and is Monday at 5pm Eastern. We should probably go ahead and bite the bullet and get this project formally sandboxed within the OpenSSF. Right now its considered an "experiment" of the OpenSSF Security Tooling WG.

houdini91 · 2024-04-22T22:11:19Z

@jhoward-lm @idunbarh
Thanks the detailed reply.
Ok I think I starting to understand the vision of this project.

I try to amend this feature to reflect your suggestion to utilize the fetch command with output capabilities.

May I ask what where you thinking should be the identifier of a sbom after it consumed by the database in order to pull and export it? (Serial?, subject name? Some running number? User provided label? all of the above?)

jhoward-lm · 2024-04-22T22:48:18Z

May I ask what where you thinking should be the identifier of a sbom after it consumed by the database in order to pull and export it? (Serial?, subject name? Some running number? User provided label? all of the above?)

In ent, it's all about the ID field (at least by default, more complex unique indexes can be defined). It's either an auto-generated int, or whatever protobom defines. For a protobom Document, I guess it would be something like the Metadata.ID field.

The pieces aren't all in place yet, but it will probably end up looking something like

import (
    "github.com/protobom/storage/backends/ent"
    "github.com/protobom/storage/backends/ent/metadata"
)

func main() {
    client := ent.NewClient()

    document := client.Document.Query().
        QueryMetadata().
        Where(metadata.IDEQ("<serialNumber/unique ID>"))

    // convert ent Document to protobom Document

    // write file with protobom Writer/desired serializer
}

Not 100% sure on the final syntax there, but something like that.

Since it's probably a common operation, there would likely be a helper function written for it.

Signed-off-by: houdini91 <mdstrauss91@gmail.com>

houdini91 · 2024-04-23T16:34:26Z

@jhoward-lm

specifies the --output-file option to fetch, with an additional option such as --format [cyclonedx|spdx] (doesn't exist yet)

I removed the convert command entirely. Let me know if I should open a new branch and PR to reflect this.

I pushed an update to this PR to try and materialize this comment. Have a look and see if it matches what you were trying to convey

Regarding the usage of Fetch:

Fetch SBOM file(s) from HTTP(S), OCI, or Git URLs

Usage:
  bomctl fetch [flags] SBOM_URL...

Flags:
  -e, --encoding FILE      the output encoding [spdx: [text, json] cyclonedx: [json] (default json)
  -f, --format FILE        the output format [spdx, spdx-2.3, cyclonedx, cyclonedx-1.0, cyclonedx-1.1, cyclonedx-1.2, cyclonedx-1.3, cyclonedx-1.4, cyclonedx-1.5]
  -h, --help               help for fetch
      --netrc              Use .netrc file for authentication to remote hosts
  -o, --output-file FILE   Path to output file

I currently force explicitly setting the format flag when using the output-format flag. Although we can auto-detect the original format, automatically inverting the format seems less intuitive for the fetch command (IMHO), so I simply omitted it.

jhoward-lm · 2024-04-23T21:19:10Z

@houdini91

I just realized that if more than one SBOM URL positional argument can be provided, there isn't a way to correlate each of them to a particular --output-file value.

We can either:

limit fetch to a single SBOM URL argument, or
drop the --output-file option from bomctl fetch entirely, in favor of a new subcommand like export or save

@idunbarh Thoughts? My preference would probably be the latter, but it won't really be possible until the ent changes are merged into the new protobom storage project.

ashearin · 2024-04-23T21:41:47Z

drop the --output-file option from bomctl fetch entirely, in favor of a new subcommand like export or save

@jhoward-lm how do you envision this working? Would a user specify which to save/export based on the ID of the document? For example:

Initial cmd: bomctl fetch <url1> <url2> <url3>

Then bomctl save <bom1_ID> bom_1.spdx.json

If so, would it be helpful to have a list type cmd to show info about the sbom at glance? (similar to docker images)

I'm not sure if Protobom already covers this, but in case it doesn't

houdini91 · 2024-04-24T03:49:51Z

Initially, I thought the multi-file output support was planned to be achieved by adding an index before the file extension, based on what I saw in the code. However, there are actually 2 dimensions sbom lists: the URL list provided by the user and the sbom tree if found.

Another approach could be to allow exporting a zip/tar or simple directory. This could be implemented by supporting an "output-bundle" or "output directory" option, or simply using the output file name and adding a .tar or .zip extension.
I can try and imp this but we should agree on a directory hierarchy and naming I guess.

We could remove the output capabilities from the fetch command altogether and wait for the export command. Lets Close the pr in this case and wait for storage changes.

However, this would make operations like converting or pulling the bom tree a two-step process instead of one.
I guess once fetch and export are in place we can always imp a higher level third command that uses fetch and export internally to execute operations in a single step.

I also agree that adding a "list" command could be a valuable addition.

houdini91 · 2024-06-05T05:33:12Z

I closing the PR i don't think it relevant, as mention convert will be supported over the storage with store and retrieve basic flow and options.

houdini91 · 2024-06-05T05:33:16Z

I closing the PR i don't think it relevant, as mentioned above convert will be supported over the storage with store and retrieve basic flow and options.

8000

houdini91 added 4 commits April 20, 2024 22:55

convert command inital commit

d62e647

Signed-off-by: houdini91 <mdstrauss91@gmail.com>

typo comment

8bd42b0

Signed-off-by: houdini91 <mdstrauss91@gmail.com>

default encoding

06cce25

Signed-off-by: houdini91 <mdstrauss91@gmail.com>

typo comment

8922732

Signed-off-by: houdini91 <mdstrauss91@gmail.com>

houdini91 changed the title ~~convert command inital commit~~ suggestion: convert command initial Apr 20, 2024

revert convert command, fetch output format & encoding

9ecfb8f

Signed-off-by: houdini91 <mdstrauss91@gmail.com>

houdini91 closed this Jun 5, 2024

houdini91 mentioned this pull request Jun 11, 2024

feat: Export command - Export SBOM from storage #75

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

suggestion: convert command initial #33

suggestion: convert command initial #33

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

suggestion: convert command initial #33

suggestion: convert command initial #33

Uh oh!

Conversation

Uh oh!

Type of change

How Has This Been Tested?

Checklist

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!