From 85f322fe977d424094b09368b8e07a621c236d7b Mon Sep 17 00:00:00 2001
From: Keepers <ryanfkeepers@gmail.com>
Date: Wed, 26 Apr 2023 18:54:15 -0600
Subject: [PATCH] don't give log-file prepopulate flag a default (#3237)

Passing the default logging file into flag pre-population causes the logger to assume the flag is always passed in and never falls back to the ENV setting.  This ensures that if no flag is provided, the env log file setting is used.

---

#### Does this PR need a docs update or release note?

- [x] :white_check_mark: Yes, it's included

#### Type of change

- [x] :bug: Bugfix

#### Test Plan

- [x] :muscle: Manual
- [x] :zap: Unit test
- [x] :green_heart: E2E
---
 .github/workflows/sanity-test.yaml | 41 +++++++++++++++---------------
 CHANGELOG.md                       |  1 +
 src/pkg/logger/logger.go           | 10 +++++---
 3 files changed, 28 insertions(+), 24 deletions(-)

diff --git a/.github/workflows/sanity-test.yaml b/.github/workflows/sanity-test.yaml
index e669394df..b2a479104 100644
--- a/.github/workflows/sanity-test.yaml
+++ b/.github/workflows/sanity-test.yaml
@@ -123,10 +123,10 @@ jobs:
           ./corso backup create exchange \
           --no-stats \
           --mailbox "${CORSO_M365_TEST_USER_ID}" \
-           --hide-progress \
-           --data 'email' \
-            --json \
-            2>&1 | tee $TEST_RESULT/backup_exchange.txt 
+          --hide-progress \
+          --data 'email' \
+          --json \
+          2>&1 | tee $TEST_RESULT/backup_exchange.txt 
 
           resultjson=$(sed -e '1,/Completed Backups/d' $TEST_RESULT/backup_exchange.txt ) 
 
@@ -235,8 +235,7 @@ jobs:
           set -euo pipefail
           ./sanityCheck
 
-        
- # Onedrive test
+# Onedrive test
 
       # run the tests
       - name: Backup onedrive test
@@ -372,11 +371,11 @@ jobs:
         id: sha-info
         if: failure()
         run: |  
-                echo ${GITHUB_REF#refs/heads/}-${GITHUB_SHA}
-                echo SHA=${GITHUB_REF#refs/heads/}-${GITHUB_SHA} >> $GITHUB_OUTPUT
-                echo RUN_URL=${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}  >> $GITHUB_OUTPUT
-                echo COMMIT_URL=${{ github.server_url }}/${{ github.repository }}/commit/${GITHUB_SHA} >> $GITHUB_OUTPUT
-               
+          echo ${GITHUB_REF#refs/heads/}-${GITHUB_SHA}
+          echo SHA=${GITHUB_REF#refs/heads/}-${GITHUB_SHA} >> $GITHUB_OUTPUT
+          echo RUN_URL=${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}  >> $GITHUB_OUTPUT
+          echo COMMIT_URL=${{ github.server_url }}/${{ github.repository }}/commit/${GITHUB_SHA} >> $GITHUB_OUTPUT
+
       
       - name: Send Github Action failure to Slack
         id: slack-notification
@@ -387,16 +386,16 @@ jobs:
             {
               "text": "GitHub Action build result: ${{ job.status }} on SHA: ${{  steps.sha-info.outputs.SHA }}",
               "blocks": [
-                  {
-                      "type": "header",
-                      "text": {
-                        "type": "plain_text",
-                        "text": "Failure in Sanity Test"
-                      }
-                 },
-                   {
-                      "type": "divider"
-                    },
+                {
+                  "type": "header",
+                  "text": {
+                    "type": "plain_text",
+                    "text": "Failure in Sanity Test"
+                  }
+                },
+                {
+                  "type": "divider"
+                },
                 {
                   "type": "section",
                   "text": {
diff --git a/CHANGELOG.md b/CHANGELOG.md
index a840f7587..a9479588c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -29,6 +29,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Fixed edge case in incremental backups where moving a subfolder, deleting and recreating the subfolder's original parent folder, and moving the subfolder back to where it started would skip backing up unchanged items in the subfolder.
 - SharePoint now correctly displays site urls on `backup list`, instead of the site id.
 - Drives with a directory containing a folder named 'folder' will now restore without error.
+- The CORSO_LOG_FILE env is appropriately utilized if no --log-file flag is provided.
 
 ### Known Issues
 - Restoring a OneDrive or SharePoint file with the same name as a file with that name as its M365 ID may restore both items.
diff --git a/src/pkg/logger/logger.go b/src/pkg/logger/logger.go
index ad02751c4..fde379430 100644
--- a/src/pkg/logger/logger.go
+++ b/src/pkg/logger/logger.go
@@ -126,16 +126,15 @@ type Settings struct {
 // AddLogLevelFlag() and AddLogFileFlag() ensures the flags are
 // displayed as part of the help/usage output.
 func PreloadLoggingFlags(args []string) Settings {
-	dlf := defaultLogLocation()
 	fs := pflag.NewFlagSet("seed-logger", pflag.ContinueOnError)
 	fs.ParseErrorsWhitelist.UnknownFlags = true
-	addFlags(fs, dlf)
+	addFlags(fs, "")
 
 	// prevents overriding the corso/cobra help processor
 	fs.BoolP("help", "h", false, "")
 
 	ls := Settings{
-		File:        dlf,
+		File:        "",
 		Level:       LogLevelFV,
 		PIIHandling: SensitiveInfoFV,
 	}
@@ -186,6 +185,11 @@ func GetLogFile(logFileFlagVal string) string {
 		r = os.Getenv("CORSO_LOG_FILE")
 	}
 
+	// if no flag or env is specified, fall back to the default
+	if len(r) == 0 {
+		r = defaultLogLocation()
+	}
+
 	if r == "-" {
 		r = Stdout
 	}