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

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")
+}