Simplify the SBOM writer interface (anchore#1892)

* remove sbom.writer bytes call and consolidate helpers to options pkg Signed-off-by: Alex Goodman <alex.goodman@anchore.com> * dont close stdout Signed-off-by: Alex Goodman <alex.goodman@anchore.com> * remove close operation from multiwriter Signed-off-by: Alex Goodman <alex.goodman@anchore.com> --------- Signed-off-by: Alex Goodman <alex.goodman@anchore.com>
GijsCalis · Jun 23, 2023 · 5308831 · 5308831
1 parent 939c954
commit 5308831
Show file tree

Hide file tree

Showing 11 changed files with 443 additions and 537 deletions.
diff --git a/cmd/syft/cli/attest/attest.go b/cmd/syft/cli/attest/attest.go
@@ -17,6 +17,7 @@ import (
 	"github.com/anchore/syft/cmd/syft/cli/packages"
 	"github.com/anchore/syft/internal/bus"
 	"github.com/anchore/syft/internal/config"
+	"github.com/anchore/syft/internal/log"
 	"github.com/anchore/syft/internal/ui"
 	"github.com/anchore/syft/syft"
 	"github.com/anchore/syft/syft/event"
@@ -33,17 +34,6 @@ func Run(_ context.Context, app *config.Application, args []string) error {
 		return err
 	}
 
-	writer, err := options.MakeWriter(app.Outputs, app.File, app.OutputTemplatePath)
-	if err != nil {
-		return fmt.Errorf("unable to write to report destination: %w", err)
-	}
-
-	defer func() {
-		if err := writer.Close(); err != nil {
-			fmt.Printf("unable to close report destination: %+v", err)
-		}
-	}()
-
 	// could be an image or a directory, with or without a scheme
 	// TODO: validate that source is image
 	userInput := args[0]
@@ -62,15 +52,15 @@ func Run(_ context.Context, app *config.Application, args []string) error {
 	subscription := eventBus.Subscribe()
 
 	return eventloop.EventLoop(
-		execWorker(app, *si, writer),
+		execWorker(app, *si),
 		eventloop.SetupSignals(),
 		subscription,
 		stereoscope.Cleanup,
 		ui.Select(options.IsVerbose(app), app.Quiet)...,
 	)
 }
 
-func buildSBOM(app *config.Application, si source.Input, writer sbom.Writer, errs chan error) ([]byte, error) {
+func buildSBOM(app *config.Application, si source.Input, errs chan error) (*sbom.SBOM, error) {
 	src, cleanup, err := source.New(si, app.Registry.ToOptions(), app.Exclusions)
 	if cleanup != nil {
 		defer cleanup()
@@ -88,135 +78,90 @@ func buildSBOM(app *config.Application, si source.Input, writer sbom.Writer, err
 		return nil, fmt.Errorf("no SBOM produced for %q", si.UserInput)
 	}
 
-	// note: only works for single format no multi writer support
-	sBytes, err := writer.Bytes(*s)
-	if err != nil {
-		return nil, fmt.Errorf("unable to build SBOM bytes: %w", err)
-	}
-
-	return sBytes, nil
+	return s, nil
 }
 
 //nolint:funlen
-func execWorker(app *config.Application, si source.Input, writer sbom.Writer) <-chan error {
+func execWorker(app *config.Application, si source.Input) <-chan error {
 	errs := make(chan error)
 	go func() {
 		defer close(errs)
-		sBytes, err := buildSBOM(app, si, writer, errs)
+		defer bus.Publish(partybus.Event{Type: event.Exit})
+
+		s, err := buildSBOM(app, si, errs)
 		if err != nil {
 			errs <- fmt.Errorf("unable to build SBOM: %w", err)
 			return
 		}
 
-		// TODO: add multi writer support
-		for _, o := range app.Outputs {
-			f, err := os.CreateTemp("", o)
-			if err != nil {
-				errs <- fmt.Errorf("unable to create temp file: %w", err)
-				return
-			}
-
-			defer f.Close()
-			defer os.Remove(f.Name())
-
-			if _, err := f.Write(sBytes); err != nil {
-				errs <- fmt.Errorf("unable to write SBOM to temp file: %w", err)
-				return
-			}
-
-			// TODO: what other validation here besides binary name?
-			cmd := "cosign"
-			if !commandExists(cmd) {
-				errs <- fmt.Errorf("unable to find cosign in PATH; make sure you have it installed")
-				return
-			}
-
-			// Select Cosign predicate type based on defined output type
-			// As orientation, check: https://github.com/sigstore/cosign/blob/main/pkg/cosign/attestation/attestation.go
-			var predicateType string
-			switch strings.ToLower(o) {
-			case "cyclonedx-json":
-				predicateType = "cyclonedx"
-			case "spdx-tag-value":
-				predicateType = "spdx"
-			case "spdx-json":
-				predicateType = "spdxjson"
-			default:
-				predicateType = "custom"
-			}
-
-			args := []string{"attest", si.UserInput, "--predicate", f.Name(), "--type", predicateType}
-			if app.Attest.Key != "" {
-				args = append(args, "--key", app.Attest.Key)
-			}
-
-			execCmd := exec.Command(cmd, args...)
-			execCmd.Env = os.Environ()
-			if app.Attest.Key != "" {
-				execCmd.Env = append(execCmd.Env, fmt.Sprintf("COSIGN_PASSWORD=%s", app.Attest.Password))
-			} else {
-				// no key provided, use cosign's keyless mode
-				execCmd.Env = append(execCmd.Env, "COSIGN_EXPERIMENTAL=1")
-			}
-
-			// bus adapter for ui to hook into stdout via an os pipe
-			r, w, err := os.Pipe()
-			if err != nil {
-				errs <- fmt.Errorf("unable to create os pipe: %w", err)
-				return
-			}
-			defer w.Close()
-
-			b := &busWriter{r: r, w: w, mon: progress.NewManual(-1)}
-			execCmd.Stdout = b
-			execCmd.Stderr = b
-			defer b.mon.SetCompleted()
-
-			// attest the SBOM
-			err = execCmd.Run()
-			if err != nil {
-				b.mon.SetError(err)
-				errs <- fmt.Errorf("unable to attest SBOM: %w", err)
-				return
-			}
+		// note: ValidateOutputOptions ensures that there is no more than one output type
+		o := app.Outputs[0]
+
+		f, err := os.CreateTemp("", o)
+		if err != nil {
+			errs <- fmt.Errorf("unable to create temp file: %w", err)
+			return
 		}
+		defer os.Remove(f.Name())
 
-		bus.Publish(partybus.Event{
-			Type:  event.Exit,
-			Value: func() error { return nil },
-		})
-	}()
-	return errs
-}
+		writer, err := options.MakeSBOMWriter(app.Outputs, f.Name(), app.OutputTemplatePath)
+		if err != nil {
+			errs <- fmt.Errorf("unable to create SBOM writer: %w", err)
+			return
+		}
 
-func ValidateOutputOptions(app *config.Application) error {
-	err := packages.ValidateOutputOptions(app)
-	if err != nil {
-		return err
-	}
+		if err := writer.Write(*s); err != nil {
+			errs <- fmt.Errorf("unable to write SBOM to temp file: %w", err)
+			return
+		}
 
-	if len(app.Outputs) > 1 {
-		return fmt.Errorf("multiple SBOM format is not supported for attest at this time")
-	}
+		// TODO: what other validation here besides binary name?
+		cmd := "cosign"
+		if !commandExists(cmd) {
+			errs <- fmt.Errorf("unable to find cosign in PATH; make sure you have it installed")
+			return
+		}
 
-	// cannot use table as default output format when using template output
-	if slices.Contains(app.Outputs, table.ID.String()) {
-		app.Outputs = []string{syftjson.ID.String()}
-	}
+		// Select Cosign predicate type based on defined output type
+		// As orientation, check: https://github.com/sigstore/cosign/blob/main/pkg/cosign/attestation/attestation.go
+		var predicateType string
+		switch strings.ToLower(o) {
+		case "cyclonedx-json":
+			predicateType = "cyclonedx"
+		case "spdx-tag-value", "spdx-tv":
+			predicateType = "spdx"
+		case "spdx-json", "json":
+			predicateType = "spdxjson"
+		default:
+			predicateType = "custom"
+		}
 
-	return nil
-}
+		args := []string{"attest", si.UserInput, "--predicate", f.Name(), "--type", predicateType}
+		if app.Attest.Key != "" {
+			args = append(args, "--key", app.Attest.Key)
+		}
 
-type busWriter struct {
-	w          *os.File
-	r          *os.File
-	hasWritten bool
-	mon        *progress.Manual
-}
+		execCmd := exec.Command(cmd, args...)
+		execCmd.Env = os.Environ()
+		if app.Attest.Key != "" {
+			execCmd.Env = append(execCmd.Env, fmt.Sprintf("COSIGN_PASSWORD=%s", app.Attest.Password))
+		} else {
+			// no key provided, use cosign's keyless mode
+			execCmd.Env = append(execCmd.Env, "COSIGN_EXPERIMENTAL=1")
+		}
+
+		log.WithFields("cmd", strings.Join(execCmd.Args, " ")).Trace("creating attestation")
+
+		// bus adapter for ui to hook into stdout via an os pipe
+		r, w, err := os.Pipe()
+		if err != nil {
+			errs <- fmt.Errorf("unable to create os pipe: %w", err)
+			return
+		}
+		defer w.Close()
+
+		mon := progress.NewManual(-1)
 
-func (b *busWriter) Write(p []byte) (n int, err error) {
-	if !b.hasWritten {
-		b.hasWritten = true
 		bus.Publish(
 			partybus.Event{
 				Type: event.AttestationStarted,
@@ -229,13 +174,44 @@ func (b *busWriter) Write(p []byte) (n int, err error) {
 					Context: "cosign",
 				},
 				Value: &monitor.ShellProgress{
-					Reader: b.r,
-					Manual: b.mon,
+					Reader: r,
+					Manual: mon,
 				},
 			},
 		)
+
+		execCmd.Stdout = w
+		execCmd.Stderr = w
+
+		// attest the SBOM
+		err = execCmd.Run()
+		if err != nil {
+			mon.SetError(err)
+			errs <- fmt.Errorf("unable to attest SBOM: %w", err)
+			return
+		}
+
+		mon.SetCompleted()
+	}()
+	return errs
+}
+
+func ValidateOutputOptions(app *config.Application) error {
+	err := packages.ValidateOutputOptions(app)
+	if err != nil {
+		return err
+	}
+
+	if len(app.Outputs) > 1 {
+		return fmt.Errorf("multiple SBOM format is not supported for attest at this time")
 	}
-	return b.w.Write(p)
+
+	// cannot use table as default output format when using template output
+	if slices.Contains(app.Outputs, table.ID.String()) {
+		app.Outputs = []string{syftjson.ID.String()}
+	}
+
+	return nil
 }
 
 func commandExists(cmd string) bool {

diff --git a/cmd/syft/cli/convert/convert.go b/cmd/syft/cli/convert/convert.go
@@ -14,17 +14,11 @@ import (
 
 func Run(_ context.Context, app *config.Application, args []string) error {
 	log.Warn("convert is an experimental feature, run `syft convert -h` for help")
-	writer, err := options.MakeWriter(app.Outputs, app.File, app.OutputTemplatePath)
+	writer, err := options.MakeSBOMWriter(app.Outputs, app.File, app.OutputTemplatePath)
 	if err != nil {
 		return err
 	}
 
-	defer func() {
-		if err := writer.Close(); err != nil {
-			log.Warnf("unable to write to report destination: %w", err)
-		}
-	}()
-
 	// this can only be a SBOM file
 	userInput := args[0]