From bcde62f47525d628826f8b86f62fc2229888c520 Mon Sep 17 00:00:00 2001 From: razzle Date: Thu, 23 May 2024 12:55:47 -0400 Subject: [PATCH 1/3] much cleaner log file creation Signed-off-by: razzle --- src/cmd/common/setup.go | 14 +++++++++++--- src/pkg/message/credentials.go | 8 ++++---- src/pkg/message/message.go | 29 +++++------------------------ src/pkg/message/pausable.go | 19 ++++++++++++------- 4 files changed, 32 insertions(+), 38 deletions(-) diff --git a/src/cmd/common/setup.go b/src/cmd/common/setup.go index ae2052b882..72fd0d6722 100644 --- a/src/cmd/common/setup.go +++ b/src/cmd/common/setup.go @@ -5,8 +5,10 @@ package common import ( + "fmt" "io" "os" + "time" "github.com/defenseunicorns/zarf/src/config" "github.com/defenseunicorns/zarf/src/config/lang" @@ -51,14 +53,20 @@ func SetupCLI() { } if !config.SkipLogFile { - logFile, err := message.UseLogFile("") + ts := time.Now().Format("2006-01-02-15-04-05") + + f, err := os.CreateTemp("", fmt.Sprintf("zarf-%s-*.log", ts)) + if err != nil { + message.WarnErr(err, "Error creating a log file in a temporary directory") + return + } + logFile, err := message.UseLogFile(f) if err != nil { message.WarnErr(err, "Error saving a log file to a temporary directory") return } pterm.SetDefaultOutput(io.MultiWriter(os.Stderr, logFile)) - location := message.LogFileLocation() - message.Notef("Saving log file to %s", location) + message.Notef("Saving log file to %s", f.Name()) } } diff --git a/src/pkg/message/credentials.go b/src/pkg/message/credentials.go index 60ca971119..07b85525bc 100644 --- a/src/pkg/message/credentials.go +++ b/src/pkg/message/credentials.go @@ -32,8 +32,8 @@ func PrintCredentialTable(state *types.ZarfState, componentsToDeploy []types.Dep // Pause the logfile's output to avoid credentials being printed to the log file if logFile != nil { - logFile.pause() - defer logFile.resume() + logFile.Pause() + defer logFile.Resume() } loginData := [][]string{} @@ -95,8 +95,8 @@ func PrintComponentCredential(state *types.ZarfState, componentName string) { func PrintCredentialUpdates(oldState *types.ZarfState, newState *types.ZarfState, services []string) { // Pause the logfile's output to avoid credentials being printed to the log file if logFile != nil { - logFile.pause() - defer logFile.resume() + logFile.Pause() + defer logFile.Resume() } for _, service := range services { diff --git a/src/pkg/message/message.go b/src/pkg/message/message.go index 1801c7b9f5..919daf5a40 100644 --- a/src/pkg/message/message.go +++ b/src/pkg/message/message.go @@ -7,7 +7,6 @@ package message import ( "encoding/json" "fmt" - "io" "net/http" "os" "runtime/debug" @@ -49,7 +48,7 @@ var RuleLine = strings.Repeat("━", TermWidth) var logLevel = InfoLevel // logFile acts as a buffer for logFile generation -var logFile *pausableLogFile +var logFile *PausableLogFile // DebugWriter represents a writer interface that writes to message.Debug type DebugWriter struct{} @@ -77,32 +76,14 @@ func init() { pterm.SetDefaultOutput(os.Stderr) } -// UseLogFile writes output to stderr and a logFile. -func UseLogFile(dir string) (io.Writer, error) { - // Prepend the log filename with a timestamp. - ts := time.Now().Format("2006-01-02-15-04-05") - - f, err := os.CreateTemp(dir, fmt.Sprintf("zarf-%s-*.log", ts)) - if err != nil { - return nil, err - } - - logFile = &pausableLogFile{ - wr: f, - f: f, - } +// UseLogFile wraps a given file in a PausableLogFile +// and sets it as the log file used by the message package. +func UseLogFile(f *os.File) (*PausableLogFile, error) { + logFile = NewPausableLogFile(f) return logFile, nil } -// LogFileLocation returns the location of the log file. -func LogFileLocation() string { - if logFile == nil { - return "" - } - return logFile.f.Name() -} - // SetLogLevel sets the log level. func SetLogLevel(lvl LogLevel) { logLevel = lvl diff --git a/src/pkg/message/pausable.go b/src/pkg/message/pausable.go index ee8d7b6a67..bee72e2a50 100644 --- a/src/pkg/message/pausable.go +++ b/src/pkg/message/pausable.go @@ -9,23 +9,28 @@ import ( "os" ) -// pausableLogFile is a pausable log file -type pausableLogFile struct { +// PausableLogFile is a pausable log file +type PausableLogFile struct { wr io.Writer f *os.File } -// pause the log file -func (l *pausableLogFile) pause() { +// NewPausableLogFile creates a new pausable log file +func NewPausableLogFile(f *os.File) *PausableLogFile { + return &PausableLogFile{wr: f, f: f} +} + +// Pause the log file +func (l *PausableLogFile) Pause() { l.wr = io.Discard } -// resume the log file -func (l *pausableLogFile) resume() { +// Resume the log file +func (l *PausableLogFile) Resume() { l.wr = l.f } // Write writes the data to the log file -func (l *pausableLogFile) Write(p []byte) (n int, err error) { +func (l *PausableLogFile) Write(p []byte) (n int, err error) { return l.wr.Write(p) } From 94304c57951eb1caa841ec290c5a9a3f0943b740 Mon Sep 17 00:00:00 2001 From: razzle Date: Thu, 23 May 2024 13:37:41 -0400 Subject: [PATCH 2/3] tests Signed-off-by: razzle --- src/pkg/message/message.go | 8 ++++---- src/pkg/message/pausable.go | 32 +++++++++++++++---------------- src/pkg/message/pausable_test.go | 33 ++++++++++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 21 deletions(-) create mode 100644 src/pkg/message/pausable_test.go diff --git a/src/pkg/message/message.go b/src/pkg/message/message.go index 919daf5a40..713bb90cbe 100644 --- a/src/pkg/message/message.go +++ b/src/pkg/message/message.go @@ -48,7 +48,7 @@ var RuleLine = strings.Repeat("━", TermWidth) var logLevel = InfoLevel // logFile acts as a buffer for logFile generation -var logFile *PausableLogFile +var logFile *PausableWriter // DebugWriter represents a writer interface that writes to message.Debug type DebugWriter struct{} @@ -76,10 +76,10 @@ func init() { pterm.SetDefaultOutput(os.Stderr) } -// UseLogFile wraps a given file in a PausableLogFile +// UseLogFile wraps a given file in a PausableWriter // and sets it as the log file used by the message package. -func UseLogFile(f *os.File) (*PausableLogFile, error) { - logFile = NewPausableLogFile(f) +func UseLogFile(f *os.File) (*PausableWriter, error) { + logFile = NewPausableWriter(f) return logFile, nil } diff --git a/src/pkg/message/pausable.go b/src/pkg/message/pausable.go index bee72e2a50..b9e8fae1c7 100644 --- a/src/pkg/message/pausable.go +++ b/src/pkg/message/pausable.go @@ -6,31 +6,29 @@ package message import ( "io" - "os" ) -// PausableLogFile is a pausable log file -type PausableLogFile struct { - wr io.Writer - f *os.File +// PausableWriter is a pausable writer +type PausableWriter struct { + out, wr io.Writer } -// NewPausableLogFile creates a new pausable log file -func NewPausableLogFile(f *os.File) *PausableLogFile { - return &PausableLogFile{wr: f, f: f} +// NewPausableWriter creates a new pausable writer +func NewPausableWriter(wr io.Writer) *PausableWriter { + return &PausableWriter{out: wr, wr: wr} } -// Pause the log file -func (l *PausableLogFile) Pause() { - l.wr = io.Discard +// Pause sets the output writer to io.Discard +func (pw *PausableWriter) Pause() { + pw.out = io.Discard } -// Resume the log file -func (l *PausableLogFile) Resume() { - l.wr = l.f +// Resume sets the output writer back to the original writer +func (pw *PausableWriter) Resume() { + pw.out = pw.wr } -// Write writes the data to the log file -func (l *PausableLogFile) Write(p []byte) (n int, err error) { - return l.wr.Write(p) +// Write writes the data to the underlying output writer +func (pw *PausableWriter) Write(p []byte) (n int, err error) { + return pw.out.Write(p) } diff --git a/src/pkg/message/pausable_test.go b/src/pkg/message/pausable_test.go new file mode 100644 index 0000000000..34d2a1e88f --- /dev/null +++ b/src/pkg/message/pausable_test.go @@ -0,0 +1,33 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: 2021-Present The Zarf Authors + +package message + +import ( + "bytes" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestPausableWriter(t *testing.T) { + var buf bytes.Buffer + + pw := NewPausableWriter(&buf) + + pw.Write([]byte("foo")) + + require.Equal(t, "foo", buf.String()) + + pw.Pause() + + pw.Write([]byte("bar")) + + require.Equal(t, "foo", buf.String()) + + pw.Resume() + + pw.Write([]byte("baz")) + + require.Equal(t, "foobaz", buf.String()) +} From d38e690ae8d856b3904d04ec8dc68219a5927a1a Mon Sep 17 00:00:00 2001 From: razzle Date: Thu, 23 May 2024 15:09:43 -0400 Subject: [PATCH 3/3] ensure writes succeed Signed-off-by: razzle --- src/pkg/message/pausable_test.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/pkg/message/pausable_test.go b/src/pkg/message/pausable_test.go index 34d2a1e88f..2cfeb2c827 100644 --- a/src/pkg/message/pausable_test.go +++ b/src/pkg/message/pausable_test.go @@ -15,19 +15,25 @@ func TestPausableWriter(t *testing.T) { pw := NewPausableWriter(&buf) - pw.Write([]byte("foo")) + n, err := pw.Write([]byte("foo")) + require.NoError(t, err) + require.Equal(t, 3, n) require.Equal(t, "foo", buf.String()) pw.Pause() - pw.Write([]byte("bar")) + n, err = pw.Write([]byte("bar")) + require.NoError(t, err) + require.Equal(t, 3, n) require.Equal(t, "foo", buf.String()) pw.Resume() - pw.Write([]byte("baz")) + n, err = pw.Write([]byte("baz")) + require.NoError(t, err) + require.Equal(t, 3, n) require.Equal(t, "foobaz", buf.String()) }