From ba858d67cc1f52d758c48a89038a7712de599016 Mon Sep 17 00:00:00 2001
From: Anbraten <6918444+anbraten@users.noreply.github.com>
Date: Mon, 1 Jul 2024 19:03:14 +0200
Subject: [PATCH] Adjust logger types (#3859)

---
 agent/logger.go                  |  4 +++-
 cli/exec/exec.go                 |  2 +-
 pipeline/log/line_writer.go      |  6 +-----
 pipeline/log/line_writer_test.go |  1 -
 pipeline/log/utils.go            |  5 ++---
 pipeline/log/utils_test.go       | 15 +++++++++++++++
 pipeline/logger.go               |  2 +-
 7 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/agent/logger.go b/agent/logger.go
index 508cc5ff5..7847999a7 100644
--- a/agent/logger.go
+++ b/agent/logger.go
@@ -33,7 +33,9 @@ const (
 )
 
 func (r *Runner) createLogger(_logger zerolog.Logger, uploads *sync.WaitGroup, workflow *rpc.Workflow) pipeline.Logger {
-	return func(step *backend.Step, rc io.Reader) error {
+	return func(step *backend.Step, rc io.ReadCloser) error {
+		defer rc.Close()
+
 		logger := _logger.With().
 			Str("image", step.Image).
 			Str("workflow_id", workflow.ID).
diff --git a/cli/exec/exec.go b/cli/exec/exec.go
index 7b5659c15..2f588ee72 100644
--- a/cli/exec/exec.go
+++ b/cli/exec/exec.go
@@ -277,7 +277,7 @@ func convertPathForWindows(path string) string {
 }
 
 const maxLogLineLength = 1024 * 1024 // 1mb
-var defaultLogger = pipeline.Logger(func(step *backendTypes.Step, rc io.Reader) error {
+var defaultLogger = pipeline.Logger(func(step *backendTypes.Step, rc io.ReadCloser) error {
 	logWriter := NewLineWriter(step.Name, step.UUID)
 	return pipelineLog.CopyLineByLine(logWriter, rc, maxLogLineLength)
 })
diff --git a/pipeline/log/line_writer.go b/pipeline/log/line_writer.go
index e2a6fd2e4..0c5058f45 100644
--- a/pipeline/log/line_writer.go
+++ b/pipeline/log/line_writer.go
@@ -40,7 +40,7 @@ type LineWriter struct {
 }
 
 // NewLineWriter returns a new line reader.
-func NewLineWriter(peer rpc.Peer, stepUUID string, secret ...string) io.WriteCloser {
+func NewLineWriter(peer rpc.Peer, stepUUID string, secret ...string) io.Writer {
 	lw := &LineWriter{
 		peer:      peer,
 		stepUUID:  stepUUID,
@@ -73,7 +73,3 @@ func (w *LineWriter) Write(p []byte) (n int, err error) {
 
 	return len(data), nil
 }
-
-func (w *LineWriter) Close() error {
-	return nil
-}
diff --git a/pipeline/log/line_writer_test.go b/pipeline/log/line_writer_test.go
index 669f75973..8bf8f6251 100644
--- a/pipeline/log/line_writer_test.go
+++ b/pipeline/log/line_writer_test.go
@@ -31,7 +31,6 @@ func TestLineWriter(t *testing.T) {
 
 	secrets := []string{"world"}
 	lw := log.NewLineWriter(peer, "e9ea76a5-44a1-4059-9c4a-6956c478b26d", secrets...)
-	defer lw.Close()
 
 	_, err := lw.Write([]byte("hello world\n"))
 	assert.NoError(t, err)
diff --git a/pipeline/log/utils.go b/pipeline/log/utils.go
index ade772c82..6c3e2dfe7 100644
--- a/pipeline/log/utils.go
+++ b/pipeline/log/utils.go
@@ -20,7 +20,7 @@ import (
 	"io"
 )
 
-func writeChunks(dst io.WriteCloser, data []byte, size int) error {
+func writeChunks(dst io.Writer, data []byte, size int) error {
 	if len(data) <= size {
 		_, err := dst.Write(data)
 		return err
@@ -41,9 +41,8 @@ func writeChunks(dst io.WriteCloser, data []byte, size int) error {
 	return nil
 }
 
-func CopyLineByLine(dst io.WriteCloser, src io.Reader, maxSize int) error {
+func CopyLineByLine(dst io.Writer, src io.Reader, maxSize int) error {
 	r := bufio.NewReader(src)
-	defer dst.Close()
 
 	for {
 		// TODO: read til newline or maxSize directly
diff --git a/pipeline/log/utils_test.go b/pipeline/log/utils_test.go
index 2e21b01ef..9861b4aef 100644
--- a/pipeline/log/utils_test.go
+++ b/pipeline/log/utils_test.go
@@ -144,3 +144,18 @@ func TestCopyLineByLineSizeLimit(t *testing.T) {
 
 	wg.Wait()
 }
+
+func TestStringReader(t *testing.T) {
+	r := io.NopCloser(strings.NewReader("123\n4567\n890"))
+
+	testWriter := &testWriter{
+		Mutex:  &sync.Mutex{},
+		writes: make([]string, 0),
+	}
+
+	err := log.CopyLineByLine(testWriter, r, 1024)
+	assert.NoError(t, err)
+
+	writes := testWriter.GetWrites()
+	assert.Lenf(t, writes, 3, "expected 3 writes, got: %v", writes)
+}
diff --git a/pipeline/logger.go b/pipeline/logger.go
index c557e0e94..6eea180d1 100644
--- a/pipeline/logger.go
+++ b/pipeline/logger.go
@@ -21,4 +21,4 @@ import (
 )
 
 // Logger handles the process logging.
-type Logger func(*backend.Step, io.Reader) error
+type Logger func(*backend.Step, io.ReadCloser) error