From ba80e490c2de588246d5d10594b90002e137888e Mon Sep 17 00:00:00 2001 From: Christopher Angelo Phillips <32073428+spiffcs@users.noreply.github.com> Date: Fri, 17 Nov 2023 14:14:13 -0500 Subject: [PATCH] feat: allow for stdout to be buffered on each command (#2335) * feat: add preRun func to version to restore stdout Signed-off-by: Christopher Phillips * test: add test to capture version in output Signed-off-by: Christopher Phillips * change stdout buffering to log to be opt-in per command Signed-off-by: Alex Goodman * fix tests Signed-off-by: Alex Goodman --------- Signed-off-by: Christopher Phillips Signed-off-by: Alex Goodman Co-authored-by: Alex Goodman --- cmd/syft/cli/cli.go | 4 +- cmd/syft/cli/commands/attest.go | 4 ++ cmd/syft/cli/commands/convert.go | 4 ++ cmd/syft/cli/commands/packages.go | 4 ++ cmd/syft/cli/commands/poweruser.go | 4 ++ cmd/syft/internal/ui/capture.go | 16 +++-- cmd/syft/internal/ui/log_writer.go | 2 +- cmd/syft/internal/ui/log_writer_test.go | 4 +- .../internal/ui/stdout_logging_application.go | 63 ------------------- go.mod | 3 +- test/cli/version_cmd_test.go | 29 +++++++++ 11 files changed, 62 insertions(+), 75 deletions(-) delete mode 100644 cmd/syft/internal/ui/stdout_logging_application.go create mode 100644 test/cli/version_cmd_test.go diff --git a/cmd/syft/cli/cli.go b/cmd/syft/cli/cli.go index 23984adc3..d95c7ecd2 100644 --- a/cmd/syft/cli/cli.go +++ b/cmd/syft/cli/cli.go @@ -23,8 +23,8 @@ import ( // It also constructs the syft attest command and the syft version command. // `RunE` is the earliest that the complete application configuration can be loaded. func Application(id clio.Identification) clio.Application { - app, rootCmd := create(id, os.Stdout) - return ui.StdoutLoggingApplication(app, rootCmd) + app, _ := create(id, os.Stdout) + return app } // Command returns the root command for the syft CLI application. This is useful for embedding the entire syft CLI diff --git a/cmd/syft/cli/commands/attest.go b/cmd/syft/cli/commands/attest.go index 1c8341542..69dfe4317 100644 --- a/cmd/syft/cli/commands/attest.go +++ b/cmd/syft/cli/commands/attest.go @@ -13,6 +13,7 @@ import ( "github.com/anchore/clio" "github.com/anchore/stereoscope/pkg/image" "github.com/anchore/syft/cmd/syft/cli/options" + "github.com/anchore/syft/cmd/syft/internal/ui" "github.com/anchore/syft/internal" "github.com/anchore/syft/internal/bus" "github.com/anchore/syft/internal/file" @@ -78,6 +79,9 @@ func Attest(app clio.Application) *cobra.Command { Args: validatePackagesArgs, PreRunE: applicationUpdateCheck(id, &opts.UpdateCheck), RunE: func(cmd *cobra.Command, args []string) error { + restoreStdout := ui.CaptureStdoutToTraceLog() + defer restoreStdout() + return runAttest(id, opts, args[0]) }, }, opts) diff --git a/cmd/syft/cli/commands/convert.go b/cmd/syft/cli/commands/convert.go index 604c408e5..3a3687811 100644 --- a/cmd/syft/cli/commands/convert.go +++ b/cmd/syft/cli/commands/convert.go @@ -9,6 +9,7 @@ import ( "github.com/anchore/clio" "github.com/anchore/syft/cmd/syft/cli/options" + "github.com/anchore/syft/cmd/syft/internal/ui" "github.com/anchore/syft/internal" "github.com/anchore/syft/internal/log" "github.com/anchore/syft/syft/format" @@ -47,6 +48,9 @@ func Convert(app clio.Application) *cobra.Command { Args: validateConvertArgs, PreRunE: applicationUpdateCheck(id, &opts.UpdateCheck), RunE: func(cmd *cobra.Command, args []string) error { + restoreStdout := ui.CaptureStdoutToTraceLog() + defer restoreStdout() + return RunConvert(opts, args[0]) }, }, opts) diff --git a/cmd/syft/cli/commands/packages.go b/cmd/syft/cli/commands/packages.go index 45e7eef14..adc291c32 100644 --- a/cmd/syft/cli/commands/packages.go +++ b/cmd/syft/cli/commands/packages.go @@ -10,6 +10,7 @@ import ( "github.com/anchore/stereoscope/pkg/image" "github.com/anchore/syft/cmd/syft/cli/eventloop" "github.com/anchore/syft/cmd/syft/cli/options" + "github.com/anchore/syft/cmd/syft/internal/ui" "github.com/anchore/syft/internal" "github.com/anchore/syft/internal/file" "github.com/anchore/syft/internal/log" @@ -84,6 +85,9 @@ func Packages(app clio.Application) *cobra.Command { Args: validatePackagesArgs, PreRunE: applicationUpdateCheck(id, &opts.UpdateCheck), RunE: func(cmd *cobra.Command, args []string) error { + restoreStdout := ui.CaptureStdoutToTraceLog() + defer restoreStdout() + return runPackages(id, opts, args[0]) }, }, opts) diff --git a/cmd/syft/cli/commands/poweruser.go b/cmd/syft/cli/commands/poweruser.go index fe8496b01..6c3718497 100644 --- a/cmd/syft/cli/commands/poweruser.go +++ b/cmd/syft/cli/commands/poweruser.go @@ -12,6 +12,7 @@ import ( "github.com/anchore/stereoscope/pkg/image" "github.com/anchore/syft/cmd/syft/cli/eventloop" "github.com/anchore/syft/cmd/syft/cli/options" + "github.com/anchore/syft/cmd/syft/internal/ui" "github.com/anchore/syft/internal" "github.com/anchore/syft/syft/artifact" "github.com/anchore/syft/syft/format/syftjson" @@ -58,6 +59,9 @@ func PowerUser(app clio.Application) *cobra.Command { Hidden: true, PreRunE: applicationUpdateCheck(id, &opts.UpdateCheck), RunE: func(cmd *cobra.Command, args []string) error { + restoreStdout := ui.CaptureStdoutToTraceLog() + defer restoreStdout() + return runPowerUser(id, opts, args[0]) }, }, opts) diff --git a/cmd/syft/internal/ui/capture.go b/cmd/syft/internal/ui/capture.go index 8148fddfd..0fb659244 100644 --- a/cmd/syft/internal/ui/capture.go +++ b/cmd/syft/internal/ui/capture.go @@ -8,13 +8,19 @@ import ( "github.com/anchore/syft/internal/log" ) -// capture replaces the provided *os.File and redirects output to the provided writer. The return value is a function, -// which is used to stop the current capturing of output and restore the original file. +const defaultStdoutLogBufferSize = 1024 + +// CaptureStdoutToTraceLog replaces stdout and redirects output to the log as trace lines. The return value is a +// function, which is used to stop the current capturing of output and restore the original file. // Example: // -// restore := capture(&os.Stderr, writer) -// // here, stderr will be captured and redirected to the provided writer -// restore() // block until the output has all been sent to the writer and restore the original stderr +// restore := CaptureStdoutToTraceLog() +// // here, stdout will be captured and redirected to the provided writer +// restore() // block until the output has all been sent to the writer and restore the original stdout +func CaptureStdoutToTraceLog() (close func()) { + return capture(&os.Stdout, newLogWriter(), defaultStdoutLogBufferSize) +} + func capture(target **os.File, writer io.Writer, bufSize int) (close func()) { original := *target diff --git a/cmd/syft/internal/ui/log_writer.go b/cmd/syft/internal/ui/log_writer.go index ebe8381d3..0e07cf67c 100644 --- a/cmd/syft/internal/ui/log_writer.go +++ b/cmd/syft/internal/ui/log_writer.go @@ -25,7 +25,7 @@ func (l *logWriter) Write(data []byte) (n int, err error) { s, err := l.r.ReadString('\n') s = strings.TrimRight(s, "\n") for s != "" { - log.Trace(s) + log.Trace("[unexpected stdout] " + s) n += len(s) if err != nil { break diff --git a/cmd/syft/internal/ui/log_writer_test.go b/cmd/syft/internal/ui/log_writer_test.go index ad8a0d230..3ed48f8b5 100644 --- a/cmd/syft/internal/ui/log_writer_test.go +++ b/cmd/syft/internal/ui/log_writer_test.go @@ -23,14 +23,14 @@ func Test_logWriter(t *testing.T) { _, _ = w.Write([]byte("a\nvalue")) - expected := []any{"a", "value"} + expected := []any{"[unexpected stdout] a", "[unexpected stdout] value"} require.Equal(t, expected, bl.values) bl.values = nil _, _ = w.Write([]byte("some")) _, _ = w.Write([]byte("thing")) - expected = []any{"some", "thing"} + expected = []any{"[unexpected stdout] some", "[unexpected stdout] thing"} require.Equal(t, expected, bl.values) } diff --git a/cmd/syft/internal/ui/stdout_logging_application.go b/cmd/syft/internal/ui/stdout_logging_application.go deleted file mode 100644 index b807a4d4c..000000000 --- a/cmd/syft/internal/ui/stdout_logging_application.go +++ /dev/null @@ -1,63 +0,0 @@ -package ui - -import ( - "os" - - "github.com/spf13/cobra" - "github.com/spf13/pflag" - - "github.com/anchore/clio" -) - -const defaultStdoutLogBufferSize = 1024 - -// StdoutLoggingApplication wraps the provided app in a clio.Application, which captures non-report data -// written to os.Stdout and instead logs it to the internal logger. It also modifies the rootCmd help function -// to restore os.Stdout in order for Cobra to properly print help to stdout -func StdoutLoggingApplication(app clio.Application, rootCmd *cobra.Command) clio.Application { - return &stdoutLoggingApplication{ - delegate: app, - rootCmd: rootCmd, - } -} - -// stdoutLoggingApplication is a clio.Application, which captures data written to os.Stdout prior to the report -// being output and instead sends non-report output to the internal logger -type stdoutLoggingApplication struct { - delegate clio.Application - rootCmd *cobra.Command -} - -func (s *stdoutLoggingApplication) ID() clio.Identification { - return s.delegate.ID() -} - -func (s *stdoutLoggingApplication) AddFlags(flags *pflag.FlagSet, cfgs ...any) { - s.delegate.AddFlags(flags, cfgs...) -} - -func (s *stdoutLoggingApplication) SetupCommand(cmd *cobra.Command, cfgs ...any) *cobra.Command { - return s.delegate.SetupCommand(cmd, cfgs...) -} - -func (s *stdoutLoggingApplication) SetupRootCommand(cmd *cobra.Command, cfgs ...any) *cobra.Command { - return s.delegate.SetupRootCommand(cmd, cfgs...) -} - -func (s *stdoutLoggingApplication) Run() { - // capture everything written to stdout that is not report output - restoreStdout := capture(&os.Stdout, newLogWriter(), defaultStdoutLogBufferSize) - defer restoreStdout() - - // need to restore stdout for cobra to properly output help text to the user on stdout - baseHelpFunc := s.rootCmd.HelpFunc() - defer s.rootCmd.SetHelpFunc(baseHelpFunc) - s.rootCmd.SetHelpFunc(func(command *cobra.Command, strings []string) { - restoreStdout() - baseHelpFunc(command, strings) - }) - - s.delegate.Run() -} - -var _ clio.Application = (*stdoutLoggingApplication)(nil) diff --git a/go.mod b/go.mod index 92cc3f91d..57b8deeb6 100644 --- a/go.mod +++ b/go.mod @@ -76,8 +76,6 @@ require ( modernc.org/sqlite v1.27.0 ) -require github.com/spf13/pflag v1.0.5 - require ( dario.cat/mergo v1.0.0 // indirect github.com/AdaLogics/go-fuzz-headers v0.0.0-20230811130428-ced1acdcaa24 // indirect @@ -185,6 +183,7 @@ require ( github.com/skeema/knownhosts v1.2.0 // indirect github.com/spf13/cast v1.5.1 // indirect github.com/spf13/jwalterweatherman v1.1.0 // indirect + github.com/spf13/pflag v1.0.5 // indirect github.com/spf13/viper v1.16.0 // indirect github.com/subosito/gotenv v1.4.2 // indirect github.com/sylabs/sif/v2 v2.11.5 // indirect diff --git a/test/cli/version_cmd_test.go b/test/cli/version_cmd_test.go new file mode 100644 index 000000000..188d0f815 --- /dev/null +++ b/test/cli/version_cmd_test.go @@ -0,0 +1,29 @@ +package cli + +import ( + "testing" +) + +func TestVersionCmdPrintsToStdout(t *testing.T) { + tests := []struct { + name string + env map[string]string + assertions []traitAssertion + }{ + { + name: "version command prints to stdout", + assertions: []traitAssertion{ + assertInOutput("Version:"), + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + pkgCmd, pkgsStdout, pkgsStderr := runSyft(t, test.env, "version") + for _, traitFn := range test.assertions { + traitFn(t, pkgsStdout, pkgsStderr, pkgCmd.ProcessState.ExitCode()) + } + }) + } +}