-
Notifications
You must be signed in to change notification settings - Fork 593
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 template output #1051
add template output #1051
Conversation
Signed-off-by: Jonas Xavier <jonasx@anchore.com>
Signed-off-by: Jonas Xavier <jonasx@anchore.com>
Signed-off-by: Jonas Xavier <jonasx@anchore.com>
Signed-off-by: Jonas Xavier <jonasx@anchore.com>
Signed-off-by: Jonas Xavier <jonasx@anchore.com>
Signed-off-by: Jonas Xavier <jonasx@anchore.com>
syft/sbom/format.go
Outdated
@@ -23,6 +25,7 @@ type Format interface { | |||
Encode(io.Writer, SBOM) error | |||
Decode(io.Reader) (*SBOM, error) | |||
Validate(io.Reader) error | |||
WithOptions(options.Format) Format |
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.
we shouldn't be changing this interface reactively
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.
I see, I dropped it in favor of type casting to template format.
cmd/syft/cli/options/writer.go
Outdated
@@ -55,6 +56,7 @@ func parseOutputs(outputs []string, defaultFile string) (out []sbom.WriterOption | |||
errs = multierror.Append(errs, fmt.Errorf("bad output format: '%s'", name)) | |||
continue | |||
} | |||
format = format.WithOptions(options.Format{TemplateFilePath: templateFilePath}) |
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.
another way to achieve this without changing the Format interface is to type assert to template.Format
, and if the assertion is ok, then add on the additional information or create a new instance of the format with the new information injected via a constructor.
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.
yes, I like that idea. I used it and that helped drop options.Format
.
syft/format-options/options.go
Outdated
package options | ||
|
||
// Format has options to be used by SBOM formats | ||
type Format struct { |
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.
I see why this is here, however, it is a little awkward (it's only purpose is for the state of template and isn't clear how to use this for more formats in the future). Is there a better way to do this?
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.
Right, I removed it. Now the template output receives the param directly.
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.
I left a couple of comments around the format-options
and downstream effects. The ultimate problem with the code (on main
) is that there isn't a path to inject dependencies, which is the thing this PR is contending with.
I think there is a compromise that can be made without needing to change the Format
interface, which is a pattern that affects all formats. That compromise can be made around https://github.com/anchore/syft/pull/1051/files#r900169470 . In the future we can pay down the improvement of injecting all formats from the application config (which solves this whole problem a lot better, but is out of scope for this PR).
README.md
Outdated
@@ -230,6 +230,40 @@ Where the `formats` available are: | |||
- `spdx-json`: A JSON report conforming to the [SPDX 2.2 JSON Schema](https://github.com/spdx/spdx-spec/blob/v2.2/schemas/spdx-schema.json). | |||
- `github`: A JSON report conforming to GitHub's dependency snapshot format. | |||
- `table`: A columnar summary (default). | |||
- `template`: Lets the user specify the output format. See "Using templates" below. |
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.
We should make this section name be a hyperlink to the section
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.
I added the link 👍🏼
Signed-off-by: Jonas Xavier <jonasx@anchore.com>
internal/formats/template/format.go
Outdated
} | ||
|
||
func (f OutputFormat) WithTemplate(filePath string) sbom.Format { | ||
return OutputFormat{templateFilePath: filePath} |
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's a little odd that this is worded like a functional option, but creates a new object like a builder pattern.
I think this fits a little better as a setter for a field on the instance it was called on, something like
f.SetTemplatePath(path)
(side steps the need to create a new instance of the format). The main reason why I'm mentioning this is that it would be easier to read and follow (setters are easier, the naming wouldn't conflict with other patterns, etc).
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.
I see now. I've changed the func signature to that.
Signed-off-by: Jonas Xavier <jonasx@anchore.com>
cmd/syft/cli/options/writer.go
Outdated
tmpl, ok := format.(template.OutputFormat) | ||
if ok { |
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.
nit: This is often done on the same line in Go
Signed-off-by: Jonas Xavier <jonasx@anchore.com>
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.
Looks good!
* main: (70 commits) fix: add php catalogers to all catalogers (anchore#1065) feat: add use-all-catalogers flag (anchore#1050) Updates parsing of `yarn.lock` to use `resolved` URLs that are pulled from yarn and npm registries (anchore#926) remove OSS Meetup message (anchore#1057) add pom.xml cataloger (anchore#1055) Add support for CBL-Mariner distroless images (anchore#1045) Add catalogers configuration (anchore#1038) add template output (anchore#1051) update stereoscope to latest version (anchore#1052) update zip_read_closer to incorporate zip64 support (anchore#1041) Add pacman (alpm) parser support (anchore#943) Update of README.md (anchore#1027) bump cosign to v1.9.0 to resolve reporting of GHSA-66x3-6cw3-v5gj (anchore#1025) add workflows to test new project automation (anchore#1023) improve LanguageByName and add unit tests (anchore#1034) Read Description from dpkg status files (anchore#996) Add announcement for Anchore OSS Virtual Meetup (anchore#1033) add main module field to go bin metadata (anchore#1026) Add filters to package cataloger (anchore#1021) change draft to false for release process (anchore#1016) ... Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
* add template output Signed-off-by: Jonas Xavier <jonasx@anchore.com> * remove dead code Signed-off-by: Jonas Xavier <jonasx@anchore.com> * fix template cli flag Signed-off-by: Jonas Xavier <jonasx@anchore.com> * implement template's own format type Signed-off-by: Jonas Xavier <jonasx@anchore.com> * simpler code Signed-off-by: Jonas Xavier <jonasx@anchore.com> * fix readme link to Go template Signed-off-by: Jonas Xavier <jonasx@anchore.com> * feedback changes Signed-off-by: Jonas Xavier <jonasx@anchore.com> * simpler func signature patter Signed-off-by: Jonas Xavier <jonasx@anchore.com> * nit Signed-off-by: Jonas Xavier <jonasx@anchore.com> * fix linter error Signed-off-by: Jonas Xavier <jonasx@anchore.com>
* add template output Signed-off-by: Jonas Xavier <jonasx@anchore.com> * remove dead code Signed-off-by: Jonas Xavier <jonasx@anchore.com> * fix template cli flag Signed-off-by: Jonas Xavier <jonasx@anchore.com> * implement template's own format type Signed-off-by: Jonas Xavier <jonasx@anchore.com> * simpler code Signed-off-by: Jonas Xavier <jonasx@anchore.com> * fix readme link to Go template Signed-off-by: Jonas Xavier <jonasx@anchore.com> * feedback changes Signed-off-by: Jonas Xavier <jonasx@anchore.com> * simpler func signature patter Signed-off-by: Jonas Xavier <jonasx@anchore.com> * nit Signed-off-by: Jonas Xavier <jonasx@anchore.com> * fix linter error Signed-off-by: Jonas Xavier <jonasx@anchore.com>
Allow syft to output an SBOM based on a given Go template like Grype currently does. I added a new output format called
template
that receives as param the template file path. I extended thesbom.Format
to accept format options that formats can use if they implement it the interface themselves.Closes: #152
Signed-off-by: Jonas Xavier jonasx@anchore.com