Add options to merge command for controlling creation of system entry and type of relationships

[nightlark]

Adds several options to the merge subcommand for controlling if a system entry should be created, and what type should be used for the relationships created.

This enables workflow where relationships to a UUID should be created, but the system UUID was added to e.g. an SBOM repository as part of a separate file.

The more "standard" Contains relationship is used instead of the Includes relationship that apparently was unique to Surfactant merged SBOMs.

Changes

• Add option for disabling the creation of a system object
• Add option for specifying what type of relationship should be created for relationships to the system object; use "Contains" as the default type
• Fix a bug where a System SBOM object accessed UUID using subscript notation (["UUID"]) instead of as a member variable (.UUID)

Resolves #115
Resolves #116

[shaynakapadia]

If you want to clean this up a bit, you could use **kwargs. But it isn't neccessary

[nightlark]

I've been thinking about how to handle this pattern in click that seems to result in function definitions with a lot of arguments -- ideally I'd like to have a function that makes the feature usable when using Surfactant as a library, which will probably need at least one function with many args.

**kwargs could be a nice solution to avoid having two functions with duplicate function signatures that have many args and need to be kept in sync. I'll look into this some more and probably convert the generate subcommand at the same time.

[shaynakapadia]

Ah I see, I think you can achieve this by creating a custom decorator that adds options or args in the way you want. Similar to what you see in this stack overflow post: https://stackoverflow.com/questions/50061342/is-it-possible-to-reuse-python-click-option-decorators-for-multiple-commands

[shaynakapadia]

@nightlark I'm a bit concerned with the functionality of creating a contains relationship with a randomly generated uuid if no system uuid is present and the user specifies they don't want one added. This would create a relationship to a uuid that has no information attached to it, so a user looking at it wouldn't know if it was a system uuid, software uuid, or what. I think it would be better to require the user to provide a uuid to link to if no system uuid is present and a system object is not to be added. This would prevent floating UUIDs with no meaning attached to them. Thoughts?

[shaynakapadia]

@nightlark Also I typically use Includes for merges, what was the reason for changing the default to Contains?

[shaynakapadia]

If possible I would also add a few unit tests!

[nightlark]

@nightlark Also I typically use Includes for merges, what was the reason for changing the default to Contains?

Mostly consistency -- I was talking with Aaron about what relationships other labs were using in their SBOMs for tying software entries to the system object, and it seems everyone else is using "Contains".

@nightlark I'm a bit concerned with the functionality of creating a contains relationship with a randomly generated uuid if no system uuid is present and the user specifies they don't want one added. This would create a relationship to a uuid that has no information attached to it, so a user looking at it wouldn't know if it was a system uuid, software uuid, or what. I think it would be better to require the user to provide a uuid to link to if no system uuid is present and a system object is not to be added. This would prevent floating UUIDs with no meaning attached to them. Thoughts?

Oof... that would be fun to figure out why an SBOM fails validation when uploading to the repository. I'll add a command line option for providing a system_uuid, and checks to make sure a relationship to a random UUID is only added if the system was also added. I'll also make so that command line options take precedence over the config file like I'd originally planned.

[shaynakapadia]

Mostly consistency -- I was talking with Aaron about what relationships other labs were using in their SBOMs for tying software entries to the system object, and it seems everyone else is using "Contains".

Ah got it, maybe I'm using them wrong then :D

[shaynakapadia]

@nightlark The fix for the random uuid looks good! The logic if random system uuid is used, you must add the system object to the sbom makes sense to me and will prevent any floating uuids from getting generated.

Copilot reviewed 1 out of 1 changed files in this pull request and generated no suggestions.

Comments skipped due to low confidence (3)

surfactant/cmd/merge.py:205

• The docstring of the create_system_object function should be updated to reflect the new return type (System, bool).

def create_system_object(sbom: SBOM, config=None, system_uuid=None) -> Tuple[System, bool]:

surfactant/cmd/merge.py:121

• Ensure consistent use of system.UUID instead of system["UUID"].

Relationship(xUUID=system.UUID, yUUID=r, relationship=system_relationship)

surfactant/cmd/merge.py:47

• [nitpick] Improve the error message to provide more context, e.g., 'UUID xUUID or yUUID does not exist in the graph.'

logger.error("====ERROR xUUID or yUUID doesn't exist====")

[Copilot]

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 suggestion.