Skip to content
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

Runner package and generator object skeletons #294

Merged
merged 3 commits into from
Aug 11, 2023

Conversation

puerco
Copy link
Member

@puerco puerco commented Jul 21, 2023

This PR adds the skeleton of a mockable runner package.

The package has a generator object that executes all the SBOM generation
logic. It relies on two underlying objects: the implementation and the
document format handler.

  • The implementation handles all the generator main functions.
  • All the format-specific code lies in thG format handler which can be
    swapped out for each format supported by the generator.

This is a skeleton only, the logic needs to be implemented still.

/cc @nishakm @ba11b0y

Signed-off-by: Adolfo García Veytia (Puerco) puerco@chainguard.dev

This commit adds the skeleton of a mockable runner package.

The package has a generator object that executes all the SBOM generation
logic. It relies on two underlying objects: the implementation and the
document format handler.

- The implementation handles all the generator main functions.
- All the format-specific code lies in the format handler which can be
swapped out for each format supported by the generator.

This is a skeleton only, the logic needs to be implemented still.

Signed-off-by: Adolfo García Veytia (Puerco) <puerco@chainguard.dev>
Signed-off-by: Adolfo García Veytia (Puerco) <puerco@chainguard.dev>
@puerco puerco requested a review from nishakm July 21, 2023 21:07
Copy link
Member Author

@puerco puerco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ba11boy:

Here is the skeleton to plug the parser functionality and redraw the flow of the generator.

At its core, the CLI would simply create a new runner.Generator() and create the sbom:

// Create a new generator
generator := runner.NewWithOptions(options.Options{})

// Write the SBOM to STDOOUT:
generator.CreateSBOM("path/to/source", os.Stdout)

I've added some comments hinting where to plug the logic you wrote in #293

Once we have a working prototype we can generate the mocks and write integration tests.

Let me know if you have any questions!

}
}

func (di *defaultGeneratorImplementation) GetCodeParsers(*options.Options) ([]plugin.Plugin, error) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return nil, nil
}

func (di *defaultGeneratorImplementation) RunParser(*options.Options, plugin.Plugin) ([]meta.Package, error) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines +41 to +44
// TODO: Create the new SPDX packages
// TODO: Assign the meta package data
// TODO: Add to document
// TODO: Relate the package to the top level directory
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ba11b0y:
This function captures the main conversion of metapackage to spdx 2.3 package. It should implement each step, including your convertToPackage() function in your original PR.

Comment on lines +26 to +27
// TODO: Create top-level package
// TODO: Relate top-level package to document
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ba11b0y:

Thew SPDX document needs to have a top-level package representing the source code base. All the packages coming from the parsers should be related to that top-level element (not directly to the document).

Signed-off-by: Adolfo García Veytia (Puerco) <puerco@chainguard.dev>

type DocumentFormatHandler interface {
CreateDocument(*options.Options) (interface{}, error)
AddDocumentPackages(*options.Options, interface{}, []meta.Package) error
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not combine these together? I remember you had a reason, but I can't recall what it was.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I split them in preparation for a more complete generation process. If we add more steps, for example splitting the licensing bits or registering files, then it is easier to move the stages around.

Since it is is internal, we can combine them now and split them later if we need it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on conversations we've been having in the SBOM community (I'm thinking about our current issue with CreationInfo) it seems to me we will probably want to generate a document with its elements all in one go. In which case, maybe the underlying functionality (make each element one by one and assemble them into a document) maybe makes more sense?

Copy link
Contributor

@ba11b0y ba11b0y left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I then create a new PR off of this or make changes in this PR itself?

g.docHandler = newDocHandler

// Check the codebase and return the applicable parsers
parsers, err := g.implementation.GetCodeParsers(&g.Options)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't think of a case where there would be more than 1 applicable parsers, care to explain?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Multiple languages used in one project maybe?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahh yeah!

@@ -2,49 +2,58 @@

module github.com/spdx/spdx-sbom-generator

go 1.17
go 1.20
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

@nishakm
Copy link
Collaborator

nishakm commented Jul 24, 2023

Should I then create a new PR off of this or make changes in this PR itself?

Sure if it's easier for you.

@nishakm nishakm merged commit 4b2ec8f into opensbom-generator:main Aug 11, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants