diff options
author | Stan Hu <stanhu@gmail.com> | 2021-02-25 09:56:23 -0800 |
---|---|---|
committer | Stan Hu <stanhu@gmail.com> | 2021-02-26 09:51:32 -0800 |
commit | 48ec7c61dcb0a5ba763cda89f369a23b9c27b037 (patch) | |
tree | fd25711a460c18dd7ad6c416d53d64cd5fef5f83 | |
parent | c0a75dae6ec448c1c82eb94b58488cdd8f6759be (diff) | |
download | gitlab-shell-48ec7c61dcb0a5ba763cda89f369a23b9c27b037.tar.gz |
Fix gitlab-shell panic when log file not writable
Previously when the gitlab-shell log was not writable, gitlab-shell
would attempt to fall back to the syslog to log an error. However,
if the syslog logger creation succeeded, gitlab-shell would panic
since `err` was `nil`.
Relates to https://gitlab.com/gitlab-org/gitlab-shell/-/issues/510
-rw-r--r-- | internal/logger/logger.go | 11 | ||||
-rw-r--r-- | internal/logger/logger_test.go | 14 |
2 files changed, 23 insertions, 2 deletions
diff --git a/internal/logger/logger.go b/internal/logger/logger.go index f836555..651aa08 100644 --- a/internal/logger/logger.go +++ b/internal/logger/logger.go @@ -1,6 +1,7 @@ package logger import ( + "fmt" "io/ioutil" "log/syslog" "os" @@ -22,8 +23,14 @@ func Configure(cfg *config.Config) { logFile, err := os.OpenFile(cfg.LogFile, os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0644) if err != nil { progName, _ := os.Executable() - syslogLogger, err := syslog.NewLogger(syslog.LOG_ERR|syslog.LOG_USER, 0) - syslogLogger.Print(progName + ": Unable to configure logging: " + err.Error()) + syslogLogger, syslogLoggerErr := syslog.NewLogger(syslog.LOG_ERR|syslog.LOG_USER, 0) + if syslogLoggerErr == nil { + msg := fmt.Sprintf("%s: Unable to configure logging: %v\n", progName, err.Error()) + syslogLogger.Print(msg) + } else { + msg := fmt.Sprintf("%s: Unable to configure logging: %v, %v\n", progName, err.Error(), syslogLoggerErr.Error()) + fmt.Fprintf(os.Stderr, msg) + } // Discard logs since a log file was specified but couldn't be opened log.SetOutput(ioutil.Discard) diff --git a/internal/logger/logger_test.go b/internal/logger/logger_test.go index 8b28b37..9bffad2 100644 --- a/internal/logger/logger_test.go +++ b/internal/logger/logger_test.go @@ -30,3 +30,17 @@ func TestConfigure(t *testing.T) { require.NoError(t, err) require.True(t, strings.Contains(string(data), `msg":"this is a test"`)) } + +func TestConfigureWithPermissionError(t *testing.T) { + tmpPath, err := ioutil.TempDir(os.TempDir(), "logtest-") + require.NoError(t, err) + defer os.RemoveAll(tmpPath) + + config := config.Config{ + LogFile: tmpPath, + LogFormat: "json", + } + + Configure(&config) + log.Info("this is a test") +} |