AUTH-2813 adds back a single file support a cloudflared log file

logger/file_writer.go

@@ -33,7 +33,7 @@ func NewFileRollingWriter(directory, baseFileName string, maxFileSize int64, max
 // Write is an implementation of io.writer the rolls the file once it reaches its max size
 // It is expected the caller to Write is doing so in a thread safe manner (as WriteManager does).
 func (w *FileRollingWriter) Write(p []byte) (n int, err error) {
-	logFile := buildPath(w.directory, w.baseFileName)
+	logFile, isSingleFile := buildPath(w.directory, w.baseFileName)
 	if w.fileHandle == nil {
 		h, err := os.OpenFile(logFile, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0664)
 		if err != nil {
@@ -55,7 +55,7 @@ func (w *FileRollingWriter) Write(p []byte) (n int, err error) {
 	written, err := w.fileHandle.Write(p)
 
 	// check if the file needs to be rolled
-	if err == nil && info.Size()+int64(written) > w.maxFileSize {
+	if err == nil && info.Size()+int64(written) > w.maxFileSize && !isSingleFile {
 		// close the file handle than do the renaming. A new one will be opened on the next write
 		w.Close()
 		w.rename(logFile, 1)
@@ -77,7 +77,10 @@ func (w *FileRollingWriter) Close() {
 // but if cloudflared-1.log already exists, it is renamed to cloudflared-2.log,
 // then the other files move in to their postion
 func (w *FileRollingWriter) rename(sourcePath string, index uint) {
-	destinationPath := buildPath(w.directory, fmt.Sprintf("%s-%d", w.baseFileName, index))
+	destinationPath, isSingleFile := buildPath(w.directory, fmt.Sprintf("%s-%d", w.baseFileName, index))
+	if isSingleFile {
+		return //don't need to rename anything, it is a single file
+	}
 
 	// rolled to the max amount of files allowed on disk
 	if index >= w.maxFileCount {
@@ -93,8 +96,13 @@ func (w *FileRollingWriter) rename(sourcePath string, index uint) {
 	os.Rename(sourcePath, destinationPath)
 }
 
-func buildPath(directory, fileName string) string {
+// return the path to the log file and if it is a single file or not.
-	return filepath.Join(directory, fileName+".log")
+// true means a single file. false means a rolled file
+func buildPath(directory, fileName string) (string, bool) {
+	if !isDirectory(directory) { // not a directory, so try and treat it as a single file for backwards compatibility sake
+		return directory, true
+	}
+	return filepath.Join(directory, fileName+".log"), false
 }
 
 func exists(filePath string) bool {
@@ -103,3 +111,15 @@ func exists(filePath string) bool {
 	}
 	return true
 }
+
+func isDirectory(path string) bool {
+	if path == "" {
+		return true
+	}
+
+	fileInfo, err := os.Stat(path)
+	if err != nil {
+		return false
+	}
+	return fileInfo.IsDir()
+}

logger/file_writer_test.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"io/ioutil"
 	"os"
+	"path/filepath"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -31,18 +32,23 @@ func TestFileWrite(t *testing.T) {
 }
 
 func TestRolling(t *testing.T) {
+	dirName := "testdir"
+	err := os.Mkdir(dirName, 0755)
+	assert.NoError(t, err)
+
 	fileName := "test_file"
-	firstFile := fileName + ".log"
+	firstFile := filepath.Join(dirName, fileName+".log")
-	secondFile := fileName + "-1.log"
+	secondFile := filepath.Join(dirName, fileName+"-1.log")
-	thirdFile := fileName + "-2.log"
+	thirdFile := filepath.Join(dirName, fileName+"-2.log")
 
 	defer func() {
+		os.RemoveAll(dirName)
 		os.Remove(firstFile)
 		os.Remove(secondFile)
 		os.Remove(thirdFile)
 	}()
 
-	w := NewFileRollingWriter("", fileName, 1000, 2)
+	w := NewFileRollingWriter(dirName, fileName, 1000, 2)
 	defer w.Close()
 
 	for i := 99; i >= 1; i-- {
@@ -52,5 +58,51 @@ func TestRolling(t *testing.T) {
 	assert.FileExists(t, firstFile, "first file doesn't exist as expected")
 	assert.FileExists(t, secondFile, "second file doesn't exist as expected")
 	assert.FileExists(t, thirdFile, "third file doesn't exist as expected")
-	assert.False(t, exists(fileName+"-3.log"), "limited to two files and there is more")
+	assert.False(t, exists(filepath.Join(dirName, fileName+"-3.log")), "limited to two files and there is more")
+}
+
+func TestSingleFile(t *testing.T) {
+	fileName := "test_file"
+	testData := []byte(string("hello Dalton, how are you doing?"))
+	defer func() {
+		os.Remove(fileName)
+	}()
+
+	w := NewFileRollingWriter(fileName, fileName, 1000, 2)
+	defer w.Close()
+
+	l, err := w.Write(testData)
+
+	assert.NoError(t, err)
+	assert.Equal(t, l, len(testData), "expected write length and data length to match")
+
+	d, err := ioutil.ReadFile(fileName)
+	assert.FileExists(t, fileName, "file doesn't exist at expected path")
+	assert.Equal(t, d, testData, "expected data in file to match test data")
+}
+
+func TestSingleFileInDirectory(t *testing.T) {
+	dirName := "testdir"
+	err := os.Mkdir(dirName, 0755)
+	assert.NoError(t, err)
+
+	fileName := "test_file"
+	fullPath := filepath.Join(dirName, fileName+".log")
+	testData := []byte(string("hello Dalton, how are you doing?"))
+	defer func() {
+		os.Remove(fullPath)
+		os.RemoveAll(dirName)
+	}()
+
+	w := NewFileRollingWriter(fullPath, fileName, 1000, 2)
+	defer w.Close()
+
+	l, err := w.Write(testData)
+
+	assert.NoError(t, err)
+	assert.Equal(t, l, len(testData), "expected write length and data length to match")
+
+	d, err := ioutil.ReadFile(fullPath)
+	assert.FileExists(t, fullPath, "file doesn't exist at expected path")
+	assert.Equal(t, d, testData, "expected data in file to match test data")
 }

logger/formatter.go

@@ -2,6 +2,7 @@ package logger
 
 import (
 	"fmt"
+	"runtime"
 	"time"
 
 	"github.com/acmacalister/skittles"
@@ -58,14 +59,17 @@ func (f *DefaultFormatter) Content(l Level, c string) string {
 
 // TerminalFormatter is setup for colored output
 type TerminalFormatter struct {
-	format string
+	format        string
+	supportsColor bool
 }
 
 // NewTerminalFormatter creates a Terminal formatter for colored output
 // format is the time format to use for timestamp formatting
 func NewTerminalFormatter(format string) Formatter {
+	supportsColor := (runtime.GOOS != "windows")
 	return &TerminalFormatter{
-		format: format,
+		format:        format,
+		supportsColor: supportsColor,
 	}
 }
 
@@ -74,13 +78,13 @@ func (f *TerminalFormatter) Timestamp(l Level, d time.Time) string {
 	t := ""
 	switch l {
 	case InfoLevel:
-		t = skittles.Cyan("[INFO] ")
+		t = f.output("[INFO] ", skittles.Cyan)
 	case ErrorLevel:
-		t = skittles.Red("[ERROR] ")
+		t = f.output("[ERROR] ", skittles.Red)
 	case DebugLevel:
-		t = skittles.Yellow("[DEBUG] ")
+		t = f.output("[DEBUG] ", skittles.Yellow)
 	case FatalLevel:
-		t = skittles.Red("[FATAL] ")
+		t = f.output("[FATAL] ", skittles.Red)
 	}
 	return t
 }
@@ -89,3 +93,10 @@ func (f *TerminalFormatter) Timestamp(l Level, d time.Time) string {
 func (f *TerminalFormatter) Content(l Level, c string) string {
 	return c
 }
+
+func (f *TerminalFormatter) output(msg string, colorFunc func(interface{}) string) string {
+	if f.supportsColor {
+		return colorFunc(msg)
+	}
+	return msg
+}