diff --git a/cmd/cloudflared/tunnel/subcommand_context_test.go b/cmd/cloudflared/tunnel/subcommand_context_test.go index 11b714be..d2744366 100644 --- a/cmd/cloudflared/tunnel/subcommand_context_test.go +++ b/cmd/cloudflared/tunnel/subcommand_context_test.go @@ -271,7 +271,7 @@ func (d *deleteMockTunnelStore) CleanupConnections(tunnelID uuid.UUID) error { func Test_subcommandContext_Delete(t *testing.T) { type fields struct { c *cli.Context - logger logger.Service + log *zerolog.Logger isUIEnabled bool fs fileSystem tunnelstoreClient *deleteMockTunnelStore @@ -283,8 +283,7 @@ func Test_subcommandContext_Delete(t *testing.T) { newCertPath := "new_cert.json" tunnelID1 := uuid.MustParse("df5ed608-b8b4-4109-89f3-9f2cf199df64") tunnelID2 := uuid.MustParse("af5ed608-b8b4-4109-89f3-9f2cf199df64") - logger, err := logger.New() - require.NoError(t, err) + log := zerolog.Nop() var tests = []struct { name string @@ -296,7 +295,7 @@ func Test_subcommandContext_Delete(t *testing.T) { { name: "clean up continues if credentials are not found", fields: fields{ - logger: logger, + log: &log, fs: mockFileSystem{ rf: func(filePath string) ([]byte, error) { return nil, errors.New("file not found") @@ -307,7 +306,7 @@ func Test_subcommandContext_Delete(t *testing.T) { flagSet := flag.NewFlagSet("test0", flag.PanicOnError) flagSet.String(CredFileFlag, newCertPath, "") c := cli.NewContext(cli.NewApp(), flagSet, nil) - err = c.Set(CredFileFlag, newCertPath) + _ = c.Set(CredFileFlag, newCertPath) return c }(), tunnelstoreClient: newDeleteMockTunnelStore( @@ -331,7 +330,7 @@ func Test_subcommandContext_Delete(t *testing.T) { t.Run(tt.name, func(t *testing.T) { sc := &subcommandContext{ c: tt.fields.c, - logger: tt.fields.logger, + log: tt.fields.log, isUIEnabled: tt.fields.isUIEnabled, fs: tt.fields.fs, tunnelstoreClient: tt.fields.tunnelstoreClient, diff --git a/logger/configuration.go b/logger/configuration.go index ed969ec7..b4a011b1 100644 --- a/logger/configuration.go +++ b/logger/configuration.go @@ -1,5 +1,7 @@ package logger +import "path/filepath" + var defaultConfig = createDefaultConfig() // Logging configuration @@ -16,12 +18,17 @@ type ConsoleConfig struct { } type FileConfig struct { - Filepath string + Dirname string + Filename string +} + +func (fc *FileConfig) Fullpath() string { + return filepath.Join(fc.Dirname, fc.Filename) } type RollingConfig struct { - Directory string - Filename string + Dirname string + Filename string maxSize int // megabytes maxBackups int // files @@ -34,18 +41,19 @@ func createDefaultConfig() Config { const RollingMaxSize = 1 // Mb const RollingMaxBackups = 5 // files const RollingMaxAge = 0 // Keep forever - const rollingLogFilename = "cloudflared.log" + const defaultLogFilename = "cloudflared.log" return Config{ ConsoleConfig: &ConsoleConfig{ noColor: false, }, FileConfig: &FileConfig{ - Filepath: "", + Dirname: "", + Filename: defaultLogFilename, }, RollingConfig: &RollingConfig{ - Directory: "", - Filename: rollingLogFilename, + Dirname: "", + Filename: defaultLogFilename, maxSize: RollingMaxSize, maxBackups: RollingMaxBackups, maxAge: RollingMaxAge, @@ -57,7 +65,7 @@ func createDefaultConfig() Config { func CreateConfig( minLevel string, disableTerminal bool, - rollingLogPath, rollingLogFilename, nonRollingLogFilePath string, + rollingLogPath, nonRollingLogFilePath string, ) *Config { var console *ConsoleConfig if !disableTerminal { @@ -65,13 +73,11 @@ func CreateConfig( } var file *FileConfig - if nonRollingLogFilePath != "" { - file = createFileConfig(nonRollingLogFilePath) - } - var rolling *RollingConfig if rollingLogPath != "" { - rolling = createRollingConfig(rollingLogPath, rollingLogFilename) + rolling = createRollingConfig(rollingLogPath) + } else if nonRollingLogFilePath != "" { + file = createFileConfig(nonRollingLogFilePath) } if minLevel == "" { @@ -93,24 +99,27 @@ func createConsoleConfig() *ConsoleConfig { } } -func createFileConfig(filepath string) *FileConfig { - if filepath == "" { - filepath = defaultConfig.FileConfig.Filepath +func createFileConfig(fullpath string) *FileConfig { + if fullpath == "" { + return defaultConfig.FileConfig } + dirname, filename := filepath.Split(fullpath) + return &FileConfig{ - Filepath: filepath, + Dirname: dirname, + Filename: filename, } } -func createRollingConfig(directory, filename string) *RollingConfig { +func createRollingConfig(directory string) *RollingConfig { if directory == "" { - directory = defaultConfig.RollingConfig.Directory + directory = defaultConfig.RollingConfig.Dirname } return &RollingConfig{ - Directory: directory, - Filename: filename, + Dirname: directory, + Filename: defaultConfig.RollingConfig.Filename, maxSize: defaultConfig.RollingConfig.maxSize, maxBackups: defaultConfig.RollingConfig.maxBackups, maxAge: defaultConfig.RollingConfig.maxAge, diff --git a/logger/create.go b/logger/create.go index 6cf4cfd7..493bbe2c 100644 --- a/logger/create.go +++ b/logger/create.go @@ -1,6 +1,7 @@ package logger import ( + "fmt" "io" "os" "path" @@ -22,6 +23,9 @@ const ( LogSSHDirectoryFlag = "log-directory" LogSSHLevelFlag = "log-level" + + dirPermMode = 0744 // rwxr--r-- + filePermMode = 0644 // rw-r--r-- ) func fallbackLogger(err error) *zerolog.Logger { @@ -38,6 +42,15 @@ func newZerolog(loggerConfig *Config) *zerolog.Logger { writers = append(writers, createConsoleLogger(*loggerConfig.ConsoleConfig)) } + if loggerConfig.FileConfig != nil { + fileLogger, err := createFileLogger(*loggerConfig.FileConfig) + if err != nil { + return fallbackLogger(err) + } + + writers = append(writers, fileLogger) + } + if loggerConfig.RollingConfig != nil { rollingLogger, err := createRollingLogger(*loggerConfig.RollingConfig) if err != nil { @@ -84,11 +97,14 @@ func createFromContext( logLevel, disableTerminal, logDirectory, - defaultConfig.RollingConfig.Filename, logFile, ) - return newZerolog(loggerConfig) + log := newZerolog(loggerConfig) + if incompatibleFlagsSet := logFile != "" && logDirectory != ""; incompatibleFlagsSet { + log.Error().Msgf("Your config includes values for both %s and %s, but they are incompatible. %s takes precedence.", LogFileFlag, logDirectoryFlagName, LogFileFlag) + } + return log } func Create(loggerConfig *Config) *zerolog.Logger { @@ -106,13 +122,53 @@ func createConsoleLogger(config ConsoleConfig) io.Writer { } } +func createFileLogger(config FileConfig) (io.Writer, error) { + var logFile io.Writer + fullpath := config.Fullpath() + + // Try to open the existing file + logFile, err := os.OpenFile(fullpath, os.O_APPEND|os.O_WRONLY, filePermMode) + if err != nil { + // If the existing file wasn't found, or couldn't be opened, just ignore + // it and recreate a new one. + logFile, err = createLogFile(config) + // If creating a new logfile fails, then we have no choice but to error out. + if err != nil { + return nil, err + } + } + + fileLogger := zerolog.New(logFile).With().Logger() + + return fileLogger, nil +} + +func createLogFile(config FileConfig) (io.Writer, error) { + if config.Dirname != "" { + err := os.MkdirAll(config.Dirname, dirPermMode) + + if err != nil { + return nil, fmt.Errorf("unable to create directories for new logfile: %s", err) + } + } + + mode := os.FileMode(filePermMode) + + logFile, err := os.OpenFile(config.Filename, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, mode) + if err != nil { + return nil, fmt.Errorf("unable to create a new logfile: %s", err) + } + + return logFile, nil +} + func createRollingLogger(config RollingConfig) (io.Writer, error) { - if err := os.MkdirAll(config.Directory, 0744); err != nil { + if err := os.MkdirAll(config.Dirname, dirPermMode); err != nil { return nil, err } return &lumberjack.Logger{ - Filename: path.Join(config.Directory, config.Filename), + Filename: path.Join(config.Dirname, config.Filename), MaxBackups: config.maxBackups, MaxSize: config.maxSize, MaxAge: config.maxAge,