SNOW-990111: Add easy logging improvements

Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs

@@ -4,10 +4,12 @@
 
 using System;
 using System.IO;
+using System.Runtime.InteropServices;
 using Moq;
 using NUnit.Framework;
 using Snowflake.Data.Configuration;
 using Snowflake.Data.Core.Tools;
+using static Snowflake.Data.Tests.UnitTests.Configuration.EasyLoggingConfigGenerator;
 
 namespace Snowflake.Data.Tests.UnitTests.Configuration
 {
@@ -16,7 +18,7 @@ public class EasyLoggingConfigFinderTest
     {
         private const string InputConfigFilePath = "input_config.json";
         private const string EnvironmentalConfigFilePath = "environmental_config.json";
-        private const string HomeDirectory = "/home/user";
+        private static readonly string HomeDirectory = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile);
         private static readonly string s_driverConfigFilePath = Path.Combine(".", EasyLoggingConfigFinder.ClientConfigFileName);
         private static readonly string s_homeConfigFilePath = Path.Combine(HomeDirectory, EasyLoggingConfigFinder.ClientConfigFileName);
         private static readonly string s_tempConfigFilePath = Path.Combine(Path.GetTempPath(), EasyLoggingConfigFinder.ClientConfigFileName);
@@ -104,7 +106,42 @@ public void TestThatTakesFilePathFromHomeLocationWhenNoInputParamEnvironmentVarN
         }
 
         [Test]
-        public void TestThatTakesFilePathFromTempDirectoryWhenNoOtherWaysPossible()
+        public void TestThatTakesFilePathFromHomeDirectoryWhenNoOtherWaysPossible()
+        {
+            // arrange
+            MockFileOnHomePath();
+
+            // act
+            var filePath = t_finder.FindConfigFilePath(null);
+
+            // assert
+            Assert.AreEqual(s_homeConfigFilePath, filePath);
+        }
+
+        [Test]
+        [Ignore("This test requires manual interaction and therefore cannot be run in CI")]
+        public void TestThatConfigFileIsNotUsedIfOthersCanModifyTheConfigFile()
+        {
+            // Note: In order to trigger the error, the config file permissions should allow other users to modify the file
+            if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
+            {
+                // arrange
+                var configFilePath = CreateConfigTempFile(Config(EasyLoggingLogLevel.Warn.ToString(), InputConfigFilePath));
+
+                // act
+                var thrown = Assert.Throws<Exception>(() => t_finder.FindConfigFilePath(configFilePath));
+
+                // assert
+                Assert.IsNotNull(thrown);
+                Assert.AreEqual(thrown.Message, "Error due to other users having permission to modify the config file");
+
+                // cleanup
+                File.Delete(configFilePath);
+            }
+        }
+
+        [Test]
+        public void TestThatDoesNotTakeFilePathFromTempDirectoryWhenNoOtherWaysPossible()
         {
             // arrange
             MockFileOnTempPath();
@@ -113,7 +150,7 @@ public void TestThatTakesFilePathFromTempDirectoryWhenNoOtherWaysPossible()
             var filePath = t_finder.FindConfigFilePath(null);
 
             // assert
-            Assert.AreEqual(s_tempConfigFilePath, filePath);
+            Assert.Null(filePath);
         }
 
         [Test]

Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerManagerTest.cs

@@ -5,6 +5,7 @@
 using System;
 using System.IO;
 using System.Linq;
+using System.Runtime.InteropServices;
 using NUnit.Framework;
 using Snowflake.Data.Configuration;
 using Snowflake.Data.Core;
@@ -21,7 +22,7 @@ public class EasyLoggerManagerTest
         private const string WarnMessage = "Easy logging Warn message";
         private const string ErrorMessage = "Easy logging Error message";
         private const string FatalMessage = "Easy logging Fatal message";
-        private static readonly string s_logsDirectory = Path.GetTempPath();
+        private static readonly string s_logsDirectory = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile);
 
         [ThreadStatic]
         private static string t_directoryLogPath;
@@ -92,6 +93,37 @@ public void TestThatLogsToProperFileWithProperLogLevelOnly()
             Assert.That(logLines, Has.Exactly(1).Matches<string>(s => s.Contains(FatalMessage)));
         }
 
+        [Test]
+        public void TestThatUnknownFieldsAreLogged()
+        {
+            // arrange
+            string ConfigWithUnknownFields = @"{
+                    ""common"": {
+                        ""log_level"": ""warn"",
+                        ""log_path"": ""path"",
+                        ""fake_log_field_1"": ""abc"",
+                        ""fake_log_field_2"": ""123""
+                    }
+                }";
+            var configFilePath = Guid.NewGuid().ToString() + ".json";
+            using (var writer = File.CreateText(configFilePath))
+            {
+                writer.Write(ConfigWithUnknownFields.Replace("\\", "\\\\"));
+            }
+            EasyLoggerManager.Instance.ReconfigureEasyLogging(EasyLoggingLogLevel.Warn, t_directoryLogPath);
+
+            // act
+            var parser = new EasyLoggingConfigParser();
+            var config = parser.Parse(configFilePath);
+
+            // assert
+            var logLines = File.ReadLines(FindLogFilePath(t_directoryLogPath));
+            Assert.That(logLines, Has.Exactly(2).Matches<string>(s => s.Contains($"Unknown field from config: ")));
+
+            // cleanup
+            File.Delete(configFilePath);
+        }
+
         private static string RandomLogsDirectoryPath()
         {
             var randomName = Path.GetRandomFileName();

Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs

@@ -4,6 +4,7 @@
 
 using System;
 using System.IO;
+using System.Runtime.InteropServices;
 using Moq;
 using NUnit.Framework;
 using Snowflake.Data.Configuration;
@@ -16,8 +17,8 @@ namespace Snowflake.Data.Tests.UnitTests.Session
     [TestFixture]
     public class EasyLoggingStarterTest
     {
-
-        private const string LogPath = "/some-logs-path/some-folder";
+        private static readonly string HomeDirectory = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile);
+        private static readonly string LogPath = Path.Combine(HomeDirectory, "some-logs-path/some-folder");
         private const string ConfigPath = "/some-path/config.json";
         private const string AnotherConfigPath = "/another/path";
         private static readonly string s_expectedLogPath = Path.Combine(LogPath, "dotnet");
@@ -39,7 +40,15 @@ public class EasyLoggingStarterTest
                 LogPath = LogPath
             }
         };
-
+
+        private static readonly ClientConfig s_configWithNoLogPath = new ClientConfig
+        {
+            CommonProps = new ClientConfigCommonProps
+            {
+                LogLevel = "Info"
+            }
+        };
+
         [ThreadStatic]
         private static Mock<EasyLoggingConfigProvider> t_easyLoggingProvider;
 
@@ -49,6 +58,9 @@ public class EasyLoggingStarterTest
         [ThreadStatic]
         private static Mock<DirectoryOperations> t_directoryOperations;
 
+        [ThreadStatic]
+        private static Mock<EnvironmentOperations> t_environmentOperations;
+
         [ThreadStatic]
         private static EasyLoggingStarter t_easyLoggerStarter;
 
@@ -58,9 +70,60 @@ public void BeforeEach()
             t_easyLoggingProvider = new Mock<EasyLoggingConfigProvider>();
             t_easyLoggerManager = new Mock<EasyLoggerManager>();
             t_directoryOperations = new Mock<DirectoryOperations>();
-            t_easyLoggerStarter = new EasyLoggingStarter(t_easyLoggingProvider.Object, t_easyLoggerManager.Object, t_directoryOperations.Object);
+            t_environmentOperations = new Mock<EnvironmentOperations>();
+            t_easyLoggerStarter = new EasyLoggingStarter(t_easyLoggingProvider.Object, t_easyLoggerManager.Object, t_directoryOperations.Object, t_environmentOperations.Object);
         }
-
+
+        [Test]
+        public void TestThatCreatedDirectoryPermissionsFollowUmask()
+        {
+            // Note: To test with a different value than the default umask, it will have to be set before running this test
+            if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
+            {
+                // arrange
+                t_easyLoggingProvider
+                    .Setup(provider => provider.ProvideConfig(ConfigPath))
+                    .Returns(s_configWithInfoLevel);
+                t_directoryOperations
+                    .Setup(provider => provider.Exists(ConfigPath))
+                    .Returns(Directory.Exists(ConfigPath));
+                t_directoryOperations
+                    .Setup(provider => provider.CreateDirectory(s_expectedLogPath))
+                    .Returns(Directory.CreateDirectory(s_expectedLogPath));
+
+                // act
+                t_easyLoggerStarter.Init(ConfigPath);
+                var umask = EasyLoggerUtil.AllPermissions - int.Parse(EasyLoggerUtil.CallBash("umask"));
+                string commandParameters = RuntimeInformation.IsOSPlatform(OSPlatform.Linux) ? "-c '%a'" : "-f %A";
+                var dirPermissions = EasyLoggerUtil.CallBash($"stat {commandParameters} {s_expectedLogPath}");
+
+                // assert
+                Assert.IsTrue(umask >= int.Parse(dirPermissions));
+
+                // cleanup
+                Directory.Delete(s_expectedLogPath);
+            }
+        }
+
+        [Test]
+        public void TestThatThrowsErrorWhenLogPathAndHomeDirectoryIsNotSet()
+        {
+            // arrange
+            t_easyLoggingProvider
+                .Setup(provider => provider.ProvideConfig(ConfigPath))
+                .Returns(s_configWithNoLogPath);
+            t_environmentOperations
+                .Setup(provider => provider.GetFolderPath(Environment.SpecialFolder.UserProfile))
+                .Returns("");
+
+            // act
+            var thrown = Assert.Throws<Exception>(() => t_easyLoggerStarter.Init(ConfigPath));
+
+            // assert
+            Assert.IsNotNull(thrown);
+            Assert.AreEqual(thrown.Message, "No log path found for easy logging. Home directory is not configured and log path is not provided");
+        }
+
         [Test]
         public void TestThatConfiguresEasyLoggingOnlyOnceWhenInitializedWithConfigPath()
         {

Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs

@@ -4,6 +4,8 @@
 
 using System;
 using System.IO;
+using System.Runtime.InteropServices;
+using Mono.Unix;
 using Snowflake.Data.Core.Tools;
 using Snowflake.Data.Log;
 
@@ -33,24 +35,34 @@
 
         public virtual string FindConfigFilePath(string configFilePathFromConnectionString)
         {
-            return GetFilePathFromInputParameter(configFilePathFromConnectionString)
-                    ?? GetFilePathEnvironmentVariable()
-                    ?? GetFilePathFromDriverLocation()
-                    ?? GetFilePathFromHomeDirectory()
-                    ?? GetFilePathFromTempDirectory();
+            var configFilePath = GetFilePathFromInputParameter(configFilePathFromConnectionString, "connection string")
+                              ?? GetFilePathEnvironmentVariable()
+                              ?? GetFilePathFromDriverLocation()
+                              ?? GetFilePathFromHomeDirectory();
+            if (configFilePath != null)
+            {
+                CheckIfValidPermissions(configFilePath);
+            }
+            return configFilePath;
         }
 
         private string GetFilePathEnvironmentVariable()
         {
             var filePath = _environmentOperations.GetEnvironmentVariable(ClientConfigEnvironmentName);
-            return GetFilePathFromInputParameter(filePath);
+            return GetFilePathFromInputParameter(filePath, "environment variable");
         }
-
-        private string GetFilePathFromTempDirectory() => SearchForConfigInDirectory(Path.GetTempPath, "temp");
 
         private string GetFilePathFromHomeDirectory() => SearchForConfigInDirectory(GetHomeDirectory, "home");
-
-        private string GetFilePathFromInputParameter(string filePath) => string.IsNullOrEmpty(filePath) ? null : filePath;
+
+        private string GetFilePathFromInputParameter(string filePath, string inputDescription)
+        {
+            if (string.IsNullOrEmpty(filePath))
+            {
+                return null;
+            }
+            s_logger.Info($"Using config file specified from {inputDescription}");
+            return filePath;
+        }
 
         private string GetHomeDirectory() =>_environmentOperations.GetFolderPath(Environment.SpecialFolder.UserProfile);
 
@@ -61,13 +73,13 @@
             try
             {
                 var directory = directoryProvider.Invoke();
-                if (string.IsNullOrEmpty(directory))
+                if (string.IsNullOrEmpty(directory) || !Directory.Exists(directory))
                 {
                     return null;
                 }
 
                 var filePath = Path.Combine(directory, ClientConfigFileName);
-                return OnlyIfFileExists(filePath);
+                return OnlyIfFileExists(filePath, directoryDescription);
             }
             catch (Exception e)
             {
@@ -76,6 +88,31 @@
             }
         }
 
-        private string OnlyIfFileExists(string filePath) => _fileOperations.Exists(filePath) ? filePath : null;
+        private string OnlyIfFileExists(string filePath, string directoryDescription)
+        {
+            if (_fileOperations.Exists(filePath))
+            {
+                s_logger.Info($"Using config file specified from {directoryDescription} directory");
+                return filePath;
+            }
+            return null;
+        }
+
+        private void CheckIfValidPermissions(string filePath)
+        {
+            if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
+                return;
+
+            // Check if others have permissions to modify the file and fail if so
+            var fileInfo = new UnixFileInfo(filePath);
+            if (fileInfo.Exists &&
+                ((fileInfo.FileAccessPermissions & FileAccessPermissions.GroupWrite) != 0 ||
+                (fileInfo.FileAccessPermissions & FileAccessPermissions.OtherWrite) != 0))
+            {
+                var errorMessage = "Error due to other users having permission to modify the config file";
+                s_logger.Error(errorMessage);
+                throw new Exception(errorMessage);
+            }
+        }
     }
 }

Snowflake.Data/Configuration/EasyLoggingConfigParser.cs

@@ -3,8 +3,12 @@
  */
 
 using System;
+using System.Collections.Generic;
 using System.IO;
+using System.Linq;
+using System.Reflection;
 using Newtonsoft.Json;
+using Newtonsoft.Json.Linq;
 using Snowflake.Data.Log;
 
 namespace Snowflake.Data.Configuration
@@ -44,6 +48,7 @@ private ClientConfig TryToParseFile(string fileContent)
             try {
                 var config = JsonConvert.DeserializeObject<ClientConfig>(fileContent);
                 Validate(config);
+                CheckForUnknownFields(fileContent);
                 return config;
             }
             catch (Exception e)
@@ -61,5 +66,22 @@ private void Validate(ClientConfig config)
                 EasyLoggingLogLevelExtensions.From(config.CommonProps.LogLevel);
             }
         }
+
+        private void CheckForUnknownFields(string fileContent)
+        {
+            // Parse the specified config file and get the key-value pairs from the "common" section
+            List<string> knownProperties = new List<string>();
+            foreach (var property in typeof(ClientConfigCommonProps).GetProperties())
+            {
+                var jsonPropertyAttribute = property.GetCustomAttribute<JsonPropertyAttribute>();
+                knownProperties.Add(jsonPropertyAttribute.PropertyName);
+            }
+
+            JObject.Parse(fileContent).GetValue("common")
+                .Cast<JProperty>()
+                .Where(property => knownProperties.Contains(property.Name))
+                .ToList()
+                .ForEach(unknownKey => s_logger.Warn($"Unknown field from config: {unknownKey.Name}"));
+        }
     }
 }