From 5bd6504e51a9f328c442e801ba0f6016be5893e3 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Fri, 12 Jan 2024 18:01:56 -0800 Subject: [PATCH 01/45] SNOW-990111: Add easy logging improvements --- .../EasyLoggingConfigFinderTest.cs | 19 +++++- .../UnitTests/Logger/EasyLoggerManagerTest.cs | 20 ++++++ .../Configuration/EasyLoggingConfigFinder.cs | 65 +++++++++++++++---- .../Configuration/EasyLoggingConfigParser.cs | 28 ++++++++ .../Core/Session/EasyLoggingStarter.cs | 40 +++++++++++- Snowflake.Data/Logger/EasyLoggerUtil.cs | 30 +++++++++ 6 files changed, 186 insertions(+), 16 deletions(-) create mode 100644 Snowflake.Data/Logger/EasyLoggerUtil.cs diff --git a/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs b/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs index c73feab9a..105c4a575 100644 --- a/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs @@ -16,7 +16,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 +104,20 @@ 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] + public void TestThatDoesNotTakeFilePathFromTempDirectoryWhenNoOtherWaysPossible() { // arrange MockFileOnTempPath(); @@ -113,7 +126,7 @@ public void TestThatTakesFilePathFromTempDirectoryWhenNoOtherWaysPossible() var filePath = t_finder.FindConfigFilePath(null); // assert - Assert.AreEqual(s_tempConfigFilePath, filePath); + Assert.Null(filePath); } [Test] diff --git a/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerManagerTest.cs b/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerManagerTest.cs index feeb728b9..35f6c731e 100644 --- a/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerManagerTest.cs +++ b/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; @@ -92,6 +93,25 @@ public void TestThatLogsToProperFileWithProperLogLevelOnly() Assert.That(logLines, Has.Exactly(1).Matches(s => s.Contains(FatalMessage))); } + [Test] + //[Ignore("This test requires manual interaction and therefore cannot be run in CI")] + public void TestThatPermissionsFollowUmask() + { + // 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 + EasyLoggerManager.Instance.ReconfigureEasyLogging(EasyLoggingLogLevel.Debug, t_directoryLogPath); + + // act + var umask = EasyLoggerUtil.AllPermissions - int.Parse(EasyLoggerUtil.CallBash("umask")); + var dirPermissions = EasyLoggerUtil.CallBash($"stat -c '%a' {t_directoryLogPath}"); + + // assert + Assert.IsTrue(umask >= int.Parse(dirPermissions)); + } + } + private static string RandomLogsDirectoryPath() { var randomName = Path.GetRandomFileName(); diff --git a/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs b/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs index 1d0c375b3..af5d5ba8c 100644 --- a/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs +++ b/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs @@ -4,6 +4,9 @@ using System; using System.IO; +using System.Runtime.InteropServices; +using Snowflake.Data.Client; +using Snowflake.Data.Core; using Snowflake.Data.Core.Tools; using Snowflake.Data.Log; @@ -33,24 +36,36 @@ internal EasyLoggingConfigFinder() public virtual string FindConfigFilePath(string configFilePathFromConnectionString) { - return GetFilePathFromInputParameter(configFilePathFromConnectionString) + return GetFilePathFromInputParameter(configFilePathFromConnectionString, "connection string") ?? GetFilePathEnvironmentVariable() ?? GetFilePathFromDriverLocation() - ?? GetFilePathFromHomeDirectory() - ?? GetFilePathFromTempDirectory(); + ?? GetFilePathFromHomeDirectory(); } 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)) + { + if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + CheckIfValidPermissions(filePath); + } + s_logger.Info($"Using config file specified from {inputDescription}"); + return filePath; + } + else + { + return null; + } + } private string GetHomeDirectory() =>_environmentOperations.GetFolderPath(Environment.SpecialFolder.UserProfile); @@ -61,13 +76,13 @@ private string SearchForConfigInDirectory(Func directoryProvider, string 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 +91,34 @@ private string SearchForConfigInDirectory(Func directoryProvider, string } } - private string OnlyIfFileExists(string filePath) => _fileOperations.Exists(filePath) ? filePath : null; + private string OnlyIfFileExists(string filePath, string directoryDescription) + { + if (_fileOperations.Exists(filePath)) + { + if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + CheckIfValidPermissions(filePath); + } + s_logger.Info($"Using config file specified from {directoryDescription} directory"); + return filePath; + } + else + { + return null; + } + } + + private void CheckIfValidPermissions(string filePath) + { + // Check if others have permissions to modify the file and fail if so + string filePermissions = EasyLoggerUtil.CallBash($"stat -c '%a' {filePath}"); + if (int.Parse(filePermissions) > EasyLoggerUtil.OnlyUserHasPermissionToWrite) + { + s_logger.Error($"Error due to other users having permission to modify the config file"); + throw new SnowflakeDbException( + SFError.INTERNAL_ERROR, + "The config file is modifiable by other users and will not be used."); + } + } } } diff --git a/Snowflake.Data/Configuration/EasyLoggingConfigParser.cs b/Snowflake.Data/Configuration/EasyLoggingConfigParser.cs index f21af2302..fc45e685b 100644 --- a/Snowflake.Data/Configuration/EasyLoggingConfigParser.cs +++ b/Snowflake.Data/Configuration/EasyLoggingConfigParser.cs @@ -4,7 +4,9 @@ using System; using System.IO; +using System.Reflection; using Newtonsoft.Json; +using Newtonsoft.Json.Linq; using Snowflake.Data.Log; namespace Snowflake.Data.Configuration @@ -44,6 +46,7 @@ private ClientConfig TryToParseFile(string fileContent) try { var config = JsonConvert.DeserializeObject(fileContent); Validate(config); + CheckForUnknownFields(fileContent, config); return config; } catch (Exception e) @@ -61,5 +64,30 @@ private void Validate(ClientConfig config) EasyLoggingLogLevelExtensions.From(config.CommonProps.LogLevel); } } + + private void CheckForUnknownFields(string fileContent, ClientConfig config) + { + // Parse the specified config file and get the key-value pairs from the "common" section + JObject obj = (JObject)(JObject.Parse(fileContent).First.First); + bool isUnknownField = true; + foreach (var keyValuePair in obj) + { + foreach(var property in config.CommonProps.GetType().GetProperties()) + { + var jsonPropertyAttribute = property.GetCustomAttribute(); + if (keyValuePair.Key.Equals(jsonPropertyAttribute.PropertyName)) + { + isUnknownField = false; + break; + } + } + if (isUnknownField) + { + s_logger.Warn($"Unknown field from config: {keyValuePair.Key}"); + } + + isUnknownField = true; + } + } } } diff --git a/Snowflake.Data/Core/Session/EasyLoggingStarter.cs b/Snowflake.Data/Core/Session/EasyLoggingStarter.cs index 9800e5186..9fe282789 100644 --- a/Snowflake.Data/Core/Session/EasyLoggingStarter.cs +++ b/Snowflake.Data/Core/Session/EasyLoggingStarter.cs @@ -2,7 +2,12 @@ * Copyright (c) 2023 Snowflake Computing Inc. All rights reserved. */ +using System; using System.IO; +using System.Runtime.InteropServices; +using System.Security.AccessControl; +using System.Security.Principal; +using Snowflake.Data.Client; using Snowflake.Data.Configuration; using Snowflake.Data.Core.Tools; using Snowflake.Data.Log; @@ -42,6 +47,15 @@ internal EasyLoggingStarter() public virtual void Init(string configFilePathFromConnectionString) { + if (string.IsNullOrEmpty(configFilePathFromConnectionString)) + { + s_logger.Info($"Attempting to enable easy logging without a config file specified from connection string"); + } + else + { + s_logger.Info($"Attempting to enable easy logging using config file specified from connection string: {configFilePathFromConnectionString}"); + } + lock (_lockForExclusiveInit) { if (!AllowedToInitialize(configFilePathFromConnectionString)) @@ -56,6 +70,8 @@ public virtual void Init(string configFilePathFromConnectionString) } var logLevel = GetLogLevel(config.CommonProps.LogLevel); var logPath = GetLogPath(config.CommonProps.LogPath); + s_logger.Info($"LogLevel set to {logLevel}"); + s_logger.Info($"LogPath set to {logPath}"); _easyLoggerManager.ReconfigureEasyLogging(logLevel, logPath); _initTrialParameters = new EasyLoggingInitTrialParameters(configFilePathFromConnectionString); } @@ -90,13 +106,33 @@ private string GetLogPath(string logPath) var logPathOrDefault = logPath; if (string.IsNullOrEmpty(logPath)) { - s_logger.Warn("LogPath in client config not found. Using temporary directory as a default value"); - logPathOrDefault = Path.GetTempPath(); + s_logger.Warn("LogPath in client config not found. Using home directory as a default value"); + logPathOrDefault = EnvironmentOperations.Instance.GetFolderPath(Environment.SpecialFolder.UserProfile); + if (string.IsNullOrEmpty(logPathOrDefault)) + { + throw new SnowflakeDbException( + SFError.INTERNAL_ERROR, + "No log path found for easy logging. Home directory is not configured and log path is not provided."); + } } var pathWithDotnetSubdirectory = Path.Combine(logPathOrDefault, "dotnet"); if (!_directoryOperations.Exists(pathWithDotnetSubdirectory)) { _directoryOperations.CreateDirectory(pathWithDotnetSubdirectory); + + if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + var umask = EasyLoggerUtil.AllPermissions - int.Parse(EasyLoggerUtil.CallBash("umask")); + string dirPermissions = EasyLoggerUtil.CallBash($"stat -c '%a' {pathWithDotnetSubdirectory}"); + if (int.Parse(dirPermissions) > umask) + { + EasyLoggerUtil.CallBash($"chmod -R {EasyLoggerUtil.AllUserPermissions} {pathWithDotnetSubdirectory}"); + } + if (int.Parse(dirPermissions) != EasyLoggerUtil.AllUserPermissions) + { + s_logger.Warn($"Access permission for the logs directory is {dirPermissions}"); + } + } } return pathWithDotnetSubdirectory; diff --git a/Snowflake.Data/Logger/EasyLoggerUtil.cs b/Snowflake.Data/Logger/EasyLoggerUtil.cs new file mode 100644 index 000000000..259027433 --- /dev/null +++ b/Snowflake.Data/Logger/EasyLoggerUtil.cs @@ -0,0 +1,30 @@ +/* + * Copyright (c) 2024 Snowflake Computing Inc. All rights reserved. + */ + +using System.Diagnostics; + +namespace Snowflake.Data.Log +{ + internal class EasyLoggerUtil + { + internal static int AllPermissions = 777; + + internal static int AllUserPermissions = 700; + + internal static int OnlyUserHasPermissionToWrite = 644; + + internal static string CallBash(string command) + { + using (Process process = new Process()) + { + process.StartInfo.FileName = "/bin/bash"; + process.StartInfo.Arguments = $"-c \"{command}\""; + process.StartInfo.UseShellExecute = false; + process.StartInfo.RedirectStandardOutput = true; + process.Start(); + return process.StandardOutput.ReadToEnd().Replace("\n", string.Empty); + } + } + } +} From e0dbec7cc2e4ee86da83bd7c801df873c179a35c Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Mon, 15 Jan 2024 13:49:44 -0800 Subject: [PATCH 02/45] SNOW-990111: Add easy logging improvements --- Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs | 5 +++-- Snowflake.Data/Core/Session/EasyLoggingStarter.cs | 9 ++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs b/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs index af5d5ba8c..b08316993 100644 --- a/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs +++ b/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs @@ -111,8 +111,9 @@ private string OnlyIfFileExists(string filePath, string directoryDescription) private void CheckIfValidPermissions(string filePath) { // Check if others have permissions to modify the file and fail if so - string filePermissions = EasyLoggerUtil.CallBash($"stat -c '%a' {filePath}"); - if (int.Parse(filePermissions) > EasyLoggerUtil.OnlyUserHasPermissionToWrite) + int filePermissions; + bool isParsed = int.TryParse(EasyLoggerUtil.CallBash($"stat -c '%a' {filePath}"), out filePermissions); + if (isParsed && filePermissions > EasyLoggerUtil.OnlyUserHasPermissionToWrite) { s_logger.Error($"Error due to other users having permission to modify the config file"); throw new SnowflakeDbException( diff --git a/Snowflake.Data/Core/Session/EasyLoggingStarter.cs b/Snowflake.Data/Core/Session/EasyLoggingStarter.cs index 9fe282789..580246586 100644 --- a/Snowflake.Data/Core/Session/EasyLoggingStarter.cs +++ b/Snowflake.Data/Core/Session/EasyLoggingStarter.cs @@ -5,8 +5,6 @@ using System; using System.IO; using System.Runtime.InteropServices; -using System.Security.AccessControl; -using System.Security.Principal; using Snowflake.Data.Client; using Snowflake.Data.Configuration; using Snowflake.Data.Core.Tools; @@ -123,12 +121,13 @@ private string GetLogPath(string logPath) if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { var umask = EasyLoggerUtil.AllPermissions - int.Parse(EasyLoggerUtil.CallBash("umask")); - string dirPermissions = EasyLoggerUtil.CallBash($"stat -c '%a' {pathWithDotnetSubdirectory}"); - if (int.Parse(dirPermissions) > umask) + int dirPermissions; + bool isParsed = int.TryParse(EasyLoggerUtil.CallBash($"stat -c '%a' {pathWithDotnetSubdirectory}"), out dirPermissions); + if (isParsed && dirPermissions > umask) { EasyLoggerUtil.CallBash($"chmod -R {EasyLoggerUtil.AllUserPermissions} {pathWithDotnetSubdirectory}"); } - if (int.Parse(dirPermissions) != EasyLoggerUtil.AllUserPermissions) + if (isParsed && dirPermissions != EasyLoggerUtil.AllUserPermissions) { s_logger.Warn($"Access permission for the logs directory is {dirPermissions}"); } From 7da9e4fb2ac27d8cba292263a86874f9911e0e53 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Mon, 15 Jan 2024 14:26:33 -0800 Subject: [PATCH 03/45] SNOW-990111: Add easy logging improvements --- .../UnitTests/Logger/EasyLoggerManagerTest.cs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerManagerTest.cs b/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerManagerTest.cs index 35f6c731e..e69c7f895 100644 --- a/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerManagerTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerManagerTest.cs @@ -22,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; @@ -105,10 +105,14 @@ public void TestThatPermissionsFollowUmask() // act var umask = EasyLoggerUtil.AllPermissions - int.Parse(EasyLoggerUtil.CallBash("umask")); - var dirPermissions = EasyLoggerUtil.CallBash($"stat -c '%a' {t_directoryLogPath}"); + int dirPermissions; + bool isParsed = int.TryParse(EasyLoggerUtil.CallBash($"stat -c '%a' {t_directoryLogPath}"), out dirPermissions); // assert - Assert.IsTrue(umask >= int.Parse(dirPermissions)); + if (isParsed) + { + Assert.IsTrue(umask >= dirPermissions); + } } } From 8a9e74a8fec0449abb14c3c9697123fa561fe521 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Mon, 15 Jan 2024 19:13:04 -0800 Subject: [PATCH 04/45] SNOW-990111: Add easy logging improvements --- .../EasyLoggingConfigFinderTest.cs | 22 ++++++++++ .../UnitTests/Logger/EasyLoggerManagerTest.cs | 41 +++++++++++++++---- .../Configuration/EasyLoggingConfigFinder.cs | 7 ++-- 3 files changed, 59 insertions(+), 11 deletions(-) diff --git a/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs b/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs index 105c4a575..21b7d91f8 100644 --- a/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs +++ b/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 { @@ -116,6 +118,26 @@ public void TestThatTakesFilePathFromHomeDirectoryWhenNoOtherWaysPossible() Assert.AreEqual(s_homeConfigFilePath, filePath); } + [Test] + public void TestThatConfigFileIsNotUsedIfOthersCanModifyTheConfigFile() + { + if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + // arrange + var configFilePath = CreateConfigTempFile(Config(EasyLoggingLogLevel.Warn.ToString(), InputConfigFilePath)); + + // act + var thrown = Assert.Throws(() => 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() { diff --git a/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerManagerTest.cs b/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerManagerTest.cs index e69c7f895..3fbba44eb 100644 --- a/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerManagerTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerManagerTest.cs @@ -93,9 +93,40 @@ public void TestThatLogsToProperFileWithProperLogLevelOnly() Assert.That(logLines, Has.Exactly(1).Matches(s => s.Contains(FatalMessage))); } + [Test] + public void TestThatUnknownFieldsAreLogged() + { + // arrange + string ConfigWithUnknownFields = @"{ + ""common"": { + ""log_level"": """ + EasyLoggingLogLevel.Warn.ToString() + @""", + ""log_path"": """ + t_directoryLogPath + @""", + ""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(s => s.Contains($"Unknown field from config: "))); + + // cleanup + File.Delete(configFilePath); + } + [Test] //[Ignore("This test requires manual interaction and therefore cannot be run in CI")] - public void TestThatPermissionsFollowUmask() + public void TestThatDirectoryPermissionsFollowUmask() { // 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)) @@ -105,14 +136,10 @@ public void TestThatPermissionsFollowUmask() // act var umask = EasyLoggerUtil.AllPermissions - int.Parse(EasyLoggerUtil.CallBash("umask")); - int dirPermissions; - bool isParsed = int.TryParse(EasyLoggerUtil.CallBash($"stat -c '%a' {t_directoryLogPath}"), out dirPermissions); + var dirPermissions = EasyLoggerUtil.CallBash($"stat -c '%a' {t_directoryLogPath}"); // assert - if (isParsed) - { - Assert.IsTrue(umask >= dirPermissions); - } + Assert.IsTrue(umask >= int.Parse(dirPermissions)); } } diff --git a/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs b/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs index b08316993..bd9758f76 100644 --- a/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs +++ b/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs @@ -115,10 +115,9 @@ private void CheckIfValidPermissions(string filePath) bool isParsed = int.TryParse(EasyLoggerUtil.CallBash($"stat -c '%a' {filePath}"), out filePermissions); if (isParsed && filePermissions > EasyLoggerUtil.OnlyUserHasPermissionToWrite) { - s_logger.Error($"Error due to other users having permission to modify the config file"); - throw new SnowflakeDbException( - SFError.INTERNAL_ERROR, - "The config file is modifiable by other users and will not be used."); + var errorMessage = "Error due to other users having permission to modify the config file"; + s_logger.Error(errorMessage); + throw new Exception(errorMessage); } } } From e592ce443fc541fcffa01b0988d2e948ea022df4 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Mon, 15 Jan 2024 19:39:56 -0800 Subject: [PATCH 05/45] SNOW-990111: Add easy logging improvements --- .../Configuration/EasyLoggingConfigFinderTest.cs | 2 ++ .../UnitTests/Logger/EasyLoggerManagerTest.cs | 2 +- Snowflake.Data/Core/Session/EasyLoggingStarter.cs | 9 +++++---- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs b/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs index 21b7d91f8..5faf6f079 100644 --- a/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs @@ -119,8 +119,10 @@ public void TestThatTakesFilePathFromHomeDirectoryWhenNoOtherWaysPossible() } [Test] + [Ignore("This test requires manual interaction and therefore cannot be run in CI")] public void TestThatConfigFileIsNotUsedIfOthersCanModifyTheConfigFile() { + // Note: The umask needs to manually be set to allow other users to modify the created config file if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { // arrange diff --git a/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerManagerTest.cs b/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerManagerTest.cs index 3fbba44eb..db219dd09 100644 --- a/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerManagerTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerManagerTest.cs @@ -125,7 +125,7 @@ public void TestThatUnknownFieldsAreLogged() } [Test] - //[Ignore("This test requires manual interaction and therefore cannot be run in CI")] + [Ignore("This test requires manual interaction and therefore cannot be run in CI")] public void TestThatDirectoryPermissionsFollowUmask() { // Note: To test with a different value than the default umask, it will have to be set before running this test diff --git a/Snowflake.Data/Core/Session/EasyLoggingStarter.cs b/Snowflake.Data/Core/Session/EasyLoggingStarter.cs index 580246586..4ae4e8699 100644 --- a/Snowflake.Data/Core/Session/EasyLoggingStarter.cs +++ b/Snowflake.Data/Core/Session/EasyLoggingStarter.cs @@ -123,14 +123,15 @@ private string GetLogPath(string logPath) var umask = EasyLoggerUtil.AllPermissions - int.Parse(EasyLoggerUtil.CallBash("umask")); int dirPermissions; bool isParsed = int.TryParse(EasyLoggerUtil.CallBash($"stat -c '%a' {pathWithDotnetSubdirectory}"), out dirPermissions); - if (isParsed && dirPermissions > umask) - { - EasyLoggerUtil.CallBash($"chmod -R {EasyLoggerUtil.AllUserPermissions} {pathWithDotnetSubdirectory}"); - } if (isParsed && dirPermissions != EasyLoggerUtil.AllUserPermissions) { s_logger.Warn($"Access permission for the logs directory is {dirPermissions}"); } + if (isParsed && dirPermissions != umask) + { + s_logger.Warn($"Setting access permission for the logs directory from {dirPermissions} to {EasyLoggerUtil.AllUserPermissions}"); + EasyLoggerUtil.CallBash($"chmod -R {EasyLoggerUtil.AllUserPermissions} {pathWithDotnetSubdirectory}"); + } } } From 55b0694228846e6140e22300a7437fdafb669630 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Tue, 16 Jan 2024 00:09:03 -0800 Subject: [PATCH 06/45] SNOW-990111: Add easy logging improvements --- .../UnitTests/Logger/EasyLoggerManagerTest.cs | 19 ---------- .../Session/EasyLoggingStarterTest.cs | 36 +++++++++++++++++-- .../Core/Session/EasyLoggingStarter.cs | 11 +++--- 3 files changed, 40 insertions(+), 26 deletions(-) diff --git a/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerManagerTest.cs b/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerManagerTest.cs index db219dd09..d4b504a11 100644 --- a/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerManagerTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerManagerTest.cs @@ -124,25 +124,6 @@ public void TestThatUnknownFieldsAreLogged() File.Delete(configFilePath); } - [Test] - [Ignore("This test requires manual interaction and therefore cannot be run in CI")] - public void TestThatDirectoryPermissionsFollowUmask() - { - // 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 - EasyLoggerManager.Instance.ReconfigureEasyLogging(EasyLoggingLogLevel.Debug, t_directoryLogPath); - - // act - var umask = EasyLoggerUtil.AllPermissions - int.Parse(EasyLoggerUtil.CallBash("umask")); - var dirPermissions = EasyLoggerUtil.CallBash($"stat -c '%a' {t_directoryLogPath}"); - - // assert - Assert.IsTrue(umask >= int.Parse(dirPermissions)); - } - } - private static string RandomLogsDirectoryPath() { var randomName = Path.GetRandomFileName(); diff --git a/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs b/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs index 00a2ed9b7..de48a6385 100644 --- a/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs +++ b/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; @@ -39,7 +40,7 @@ public class EasyLoggingStarterTest LogPath = LogPath } }; - + [ThreadStatic] private static Mock t_easyLoggingProvider; @@ -60,7 +61,38 @@ public void BeforeEach() t_directoryOperations = new Mock(); t_easyLoggerStarter = new EasyLoggingStarter(t_easyLoggingProvider.Object, t_easyLoggerManager.Object, t_directoryOperations.Object); } - + + [Test] + //[Ignore("This test requires manual interaction and therefore cannot be run in CI")] + 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")); + var dirPermissions = EasyLoggerUtil.CallBash($"stat -c '%a' {s_expectedLogPath}"); + + // assert + Assert.IsTrue(umask >= int.Parse(dirPermissions)); + + // cleanup + Directory.Delete(s_expectedLogPath); + } + } + [Test] public void TestThatConfiguresEasyLoggingOnlyOnceWhenInitializedWithConfigPath() { diff --git a/Snowflake.Data/Core/Session/EasyLoggingStarter.cs b/Snowflake.Data/Core/Session/EasyLoggingStarter.cs index 4ae4e8699..0879e826b 100644 --- a/Snowflake.Data/Core/Session/EasyLoggingStarter.cs +++ b/Snowflake.Data/Core/Session/EasyLoggingStarter.cs @@ -126,11 +126,12 @@ private string GetLogPath(string logPath) if (isParsed && dirPermissions != EasyLoggerUtil.AllUserPermissions) { s_logger.Warn($"Access permission for the logs directory is {dirPermissions}"); - } - if (isParsed && dirPermissions != umask) - { - s_logger.Warn($"Setting access permission for the logs directory from {dirPermissions} to {EasyLoggerUtil.AllUserPermissions}"); - EasyLoggerUtil.CallBash($"chmod -R {EasyLoggerUtil.AllUserPermissions} {pathWithDotnetSubdirectory}"); + + if (dirPermissions != umask) + { + s_logger.Warn($"Setting access permission for the logs directory from {dirPermissions} to {EasyLoggerUtil.AllUserPermissions}"); + EasyLoggerUtil.CallBash($"chmod -R {EasyLoggerUtil.AllUserPermissions} {pathWithDotnetSubdirectory}"); + } } } } From 387ab555b9448b080c6509e34f10b51de95ab408 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Tue, 16 Jan 2024 00:25:31 -0800 Subject: [PATCH 07/45] SNOW-990111: Add easy logging improvements --- .../UnitTests/Session/EasyLoggingStarterTest.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs b/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs index de48a6385..2f0312110 100644 --- a/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs @@ -17,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"); From 9383efcae88d274ecfe6632d77441d06e703edba Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Tue, 16 Jan 2024 01:10:45 -0800 Subject: [PATCH 08/45] SNOW-990111: Add easy logging improvements --- .../Session/EasyLoggingStarterTest.cs | 30 ++++++++++++++++++- .../Core/Session/EasyLoggingStarter.cs | 4 +-- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs b/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs index 2f0312110..b79810bfe 100644 --- a/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs @@ -41,6 +41,14 @@ public class EasyLoggingStarterTest } }; + private static readonly ClientConfig s_configWithNoLogPath = new ClientConfig + { + CommonProps = new ClientConfigCommonProps + { + LogLevel = "Info" + } + }; + [ThreadStatic] private static Mock t_easyLoggingProvider; @@ -63,7 +71,7 @@ public void BeforeEach() } [Test] - //[Ignore("This test requires manual interaction and therefore cannot be run in CI")] + [Ignore("This test requires manual interaction and therefore cannot be run in CI")] public void TestThatCreatedDirectoryPermissionsFollowUmask() { // Note: To test with a different value than the default umask, it will have to be set before running this test @@ -93,6 +101,26 @@ public void TestThatCreatedDirectoryPermissionsFollowUmask() } } + [Test] + //[Ignore("This test requires manual interaction and therefore cannot be run in CI")] + public void TestThatThrowsErrorWhenLogPathAndHomeDirectoryIsNotSet() + { + if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + // arrange + t_easyLoggingProvider + .Setup(provider => provider.ProvideConfig(ConfigPath)) + .Returns(s_configWithNoLogPath); + + // act + var thrown = Assert.Throws(() => 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() { diff --git a/Snowflake.Data/Core/Session/EasyLoggingStarter.cs b/Snowflake.Data/Core/Session/EasyLoggingStarter.cs index 0879e826b..765501c6d 100644 --- a/Snowflake.Data/Core/Session/EasyLoggingStarter.cs +++ b/Snowflake.Data/Core/Session/EasyLoggingStarter.cs @@ -108,9 +108,7 @@ private string GetLogPath(string logPath) logPathOrDefault = EnvironmentOperations.Instance.GetFolderPath(Environment.SpecialFolder.UserProfile); if (string.IsNullOrEmpty(logPathOrDefault)) { - throw new SnowflakeDbException( - SFError.INTERNAL_ERROR, - "No log path found for easy logging. Home directory is not configured and log path is not provided."); + throw new Exception("No log path found for easy logging. Home directory is not configured and log path is not provided"); } } var pathWithDotnetSubdirectory = Path.Combine(logPathOrDefault, "dotnet"); From dcf951e262c9e4d1742796aa8d38f6ad47f4b757 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Tue, 16 Jan 2024 01:31:33 -0800 Subject: [PATCH 09/45] SNOW-990111: Add easy logging improvements --- .../UnitTests/Session/EasyLoggingStarterTest.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs b/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs index b79810bfe..0c4a33dfb 100644 --- a/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs @@ -71,9 +71,11 @@ public void BeforeEach() } [Test] - [Ignore("This test requires manual interaction and therefore cannot be run in CI")] + //[Ignore("This test requires manual interaction and therefore cannot be run in CI")] public void TestThatCreatedDirectoryPermissionsFollowUmask() { + Console.WriteLine("HomeDirectory: " + HomeDirectory); + Console.WriteLine("LogPath: " + LogPath); // 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)) { @@ -102,9 +104,10 @@ public void TestThatCreatedDirectoryPermissionsFollowUmask() } [Test] - //[Ignore("This test requires manual interaction and therefore cannot be run in CI")] + [Ignore("This test requires manual interaction and therefore cannot be run in CI")] public void TestThatThrowsErrorWhenLogPathAndHomeDirectoryIsNotSet() { + // Note: The user home directory needs to be reconfigured to null for the test to pass if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { // arrange From 2e7c648ceb1eca4b0a75547f14961f78fb2023e5 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Tue, 16 Jan 2024 01:35:44 -0800 Subject: [PATCH 10/45] SNOW-990111: Add easy logging improvements --- .../UnitTests/Session/EasyLoggingStarterTest.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs b/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs index 0c4a33dfb..f79b72717 100644 --- a/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs @@ -18,7 +18,7 @@ namespace Snowflake.Data.Tests.UnitTests.Session public class EasyLoggingStarterTest { private static readonly string HomeDirectory = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile); - private static readonly string LogPath = Path.Combine(HomeDirectory, "/some-logs-path/some-folder"); + 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"); @@ -74,8 +74,6 @@ public void BeforeEach() //[Ignore("This test requires manual interaction and therefore cannot be run in CI")] public void TestThatCreatedDirectoryPermissionsFollowUmask() { - Console.WriteLine("HomeDirectory: " + HomeDirectory); - Console.WriteLine("LogPath: " + LogPath); // 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)) { From 1d208cf59b2b561af298187383d60f266bf26d8b Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Tue, 16 Jan 2024 09:13:01 -0800 Subject: [PATCH 11/45] SNOW-990111: Add easy logging improvements --- .../UnitTests/Session/EasyLoggingStarterTest.cs | 3 +++ Snowflake.Data/Core/Session/EasyLoggingStarter.cs | 2 ++ 2 files changed, 5 insertions(+) diff --git a/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs b/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs index f79b72717..28d5002d6 100644 --- a/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs @@ -91,6 +91,9 @@ public void TestThatCreatedDirectoryPermissionsFollowUmask() // act t_easyLoggerStarter.Init(ConfigPath); var umask = EasyLoggerUtil.AllPermissions - int.Parse(EasyLoggerUtil.CallBash("umask")); + + Console.WriteLine("s_expectedLogPath:" + s_expectedLogPath); + var dirPermissions = EasyLoggerUtil.CallBash($"stat -c '%a' {s_expectedLogPath}"); // assert diff --git a/Snowflake.Data/Core/Session/EasyLoggingStarter.cs b/Snowflake.Data/Core/Session/EasyLoggingStarter.cs index 765501c6d..b6c23984c 100644 --- a/Snowflake.Data/Core/Session/EasyLoggingStarter.cs +++ b/Snowflake.Data/Core/Session/EasyLoggingStarter.cs @@ -121,6 +121,8 @@ private string GetLogPath(string logPath) var umask = EasyLoggerUtil.AllPermissions - int.Parse(EasyLoggerUtil.CallBash("umask")); int dirPermissions; bool isParsed = int.TryParse(EasyLoggerUtil.CallBash($"stat -c '%a' {pathWithDotnetSubdirectory}"), out dirPermissions); + Console.WriteLine("pathWithDotnetSubdirectory:" + pathWithDotnetSubdirectory); + Console.WriteLine("dirPermissions:" + dirPermissions); if (isParsed && dirPermissions != EasyLoggerUtil.AllUserPermissions) { s_logger.Warn($"Access permission for the logs directory is {dirPermissions}"); From b2d3c106153238614d084d0606e13956710f2d5d Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Tue, 16 Jan 2024 09:13:59 -0800 Subject: [PATCH 12/45] SNOW-990111: Add easy logging improvements --- .../UnitTests/Session/EasyLoggingStarterTest.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs b/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs index 28d5002d6..d29c4b9e5 100644 --- a/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs @@ -91,10 +91,10 @@ public void TestThatCreatedDirectoryPermissionsFollowUmask() // act t_easyLoggerStarter.Init(ConfigPath); var umask = EasyLoggerUtil.AllPermissions - int.Parse(EasyLoggerUtil.CallBash("umask")); + var dirPermissions = EasyLoggerUtil.CallBash($"stat -c '%a' {s_expectedLogPath}"); Console.WriteLine("s_expectedLogPath:" + s_expectedLogPath); - - var dirPermissions = EasyLoggerUtil.CallBash($"stat -c '%a' {s_expectedLogPath}"); + Console.WriteLine("dirPermissions:" + dirPermissions); // assert Assert.IsTrue(umask >= int.Parse(dirPermissions)); From 98b716d98b69e0ccc33f5a4da3ff9ace41607f88 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Tue, 16 Jan 2024 09:40:40 -0800 Subject: [PATCH 13/45] SNOW-990111: Add easy logging improvements --- .../UnitTests/Session/EasyLoggingStarterTest.cs | 4 +++- Snowflake.Data/Core/Session/EasyLoggingStarter.cs | 5 ++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs b/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs index d29c4b9e5..427fa0501 100644 --- a/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs @@ -91,8 +91,10 @@ public void TestThatCreatedDirectoryPermissionsFollowUmask() // act t_easyLoggerStarter.Init(ConfigPath); var umask = EasyLoggerUtil.AllPermissions - int.Parse(EasyLoggerUtil.CallBash("umask")); - var dirPermissions = EasyLoggerUtil.CallBash($"stat -c '%a' {s_expectedLogPath}"); + string commandParameters = RuntimeInformation.IsOSPlatform(OSPlatform.Linux) ? "-c '%a'" : "-f %A"; + var dirPermissions = EasyLoggerUtil.CallBash($"stat {commandParameters} {s_expectedLogPath}"); + Console.WriteLine("umask:" + umask); Console.WriteLine("s_expectedLogPath:" + s_expectedLogPath); Console.WriteLine("dirPermissions:" + dirPermissions); diff --git a/Snowflake.Data/Core/Session/EasyLoggingStarter.cs b/Snowflake.Data/Core/Session/EasyLoggingStarter.cs index b6c23984c..73db008e0 100644 --- a/Snowflake.Data/Core/Session/EasyLoggingStarter.cs +++ b/Snowflake.Data/Core/Session/EasyLoggingStarter.cs @@ -120,9 +120,8 @@ private string GetLogPath(string logPath) { var umask = EasyLoggerUtil.AllPermissions - int.Parse(EasyLoggerUtil.CallBash("umask")); int dirPermissions; - bool isParsed = int.TryParse(EasyLoggerUtil.CallBash($"stat -c '%a' {pathWithDotnetSubdirectory}"), out dirPermissions); - Console.WriteLine("pathWithDotnetSubdirectory:" + pathWithDotnetSubdirectory); - Console.WriteLine("dirPermissions:" + dirPermissions); + string commandParameters = RuntimeInformation.IsOSPlatform(OSPlatform.Linux) ? "-c '%a'" : "-f %A"; + bool isParsed = int.TryParse(EasyLoggerUtil.CallBash($"stat {commandParameters} {pathWithDotnetSubdirectory}"), out dirPermissions); if (isParsed && dirPermissions != EasyLoggerUtil.AllUserPermissions) { s_logger.Warn($"Access permission for the logs directory is {dirPermissions}"); From 764636065a453596fa9a64a7bb1dd7d441f965ef Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Tue, 16 Jan 2024 09:54:04 -0800 Subject: [PATCH 14/45] SNOW-990111: Add easy logging improvements --- .../UnitTests/Session/EasyLoggingStarterTest.cs | 4 ---- Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs | 3 ++- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs b/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs index 427fa0501..464be6b6a 100644 --- a/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs @@ -94,10 +94,6 @@ public void TestThatCreatedDirectoryPermissionsFollowUmask() string commandParameters = RuntimeInformation.IsOSPlatform(OSPlatform.Linux) ? "-c '%a'" : "-f %A"; var dirPermissions = EasyLoggerUtil.CallBash($"stat {commandParameters} {s_expectedLogPath}"); - Console.WriteLine("umask:" + umask); - Console.WriteLine("s_expectedLogPath:" + s_expectedLogPath); - Console.WriteLine("dirPermissions:" + dirPermissions); - // assert Assert.IsTrue(umask >= int.Parse(dirPermissions)); diff --git a/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs b/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs index bd9758f76..f02e7443d 100644 --- a/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs +++ b/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs @@ -112,7 +112,8 @@ private void CheckIfValidPermissions(string filePath) { // Check if others have permissions to modify the file and fail if so int filePermissions; - bool isParsed = int.TryParse(EasyLoggerUtil.CallBash($"stat -c '%a' {filePath}"), out filePermissions); + string commandParameters = RuntimeInformation.IsOSPlatform(OSPlatform.Linux) ? "-c '%a'" : "-f %A"; + bool isParsed = int.TryParse(EasyLoggerUtil.CallBash($"stat {commandParameters} {filePath}"), out filePermissions); if (isParsed && filePermissions > EasyLoggerUtil.OnlyUserHasPermissionToWrite) { var errorMessage = "Error due to other users having permission to modify the config file"; From 5911f7ab1d0146bb7509bcc2dbceaa3a565443a3 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Tue, 16 Jan 2024 10:16:28 -0800 Subject: [PATCH 15/45] SNOW-990111: Add easy logging improvements --- .../EasyLoggingConfigFinderTest.cs | 2 +- .../UnitTests/Logger/EasyLoggerManagerTest.cs | 4 +-- .../Session/EasyLoggingStarterTest.cs | 33 ++++++++++--------- .../Core/Session/EasyLoggingStarter.cs | 10 ++++-- 4 files changed, 27 insertions(+), 22 deletions(-) diff --git a/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs b/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs index 5faf6f079..caa9cc260 100644 --- a/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs @@ -122,7 +122,7 @@ public void TestThatTakesFilePathFromHomeDirectoryWhenNoOtherWaysPossible() [Ignore("This test requires manual interaction and therefore cannot be run in CI")] public void TestThatConfigFileIsNotUsedIfOthersCanModifyTheConfigFile() { - // Note: The umask needs to manually be set to allow other users to modify the created config file + // 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 diff --git a/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerManagerTest.cs b/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerManagerTest.cs index d4b504a11..4ff35fe4a 100644 --- a/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerManagerTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerManagerTest.cs @@ -99,8 +99,8 @@ public void TestThatUnknownFieldsAreLogged() // arrange string ConfigWithUnknownFields = @"{ ""common"": { - ""log_level"": """ + EasyLoggingLogLevel.Warn.ToString() + @""", - ""log_path"": """ + t_directoryLogPath + @""", + ""log_level"": ""log_level"", + ""log_path"": ""log_path"", ""fake_log_field_1"": ""abc"", ""fake_log_field_2"": ""123"" } diff --git a/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs b/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs index 464be6b6a..24c2e09d3 100644 --- a/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs @@ -58,6 +58,9 @@ public class EasyLoggingStarterTest [ThreadStatic] private static Mock t_directoryOperations; + [ThreadStatic] + private static Mock t_environmentOperations; + [ThreadStatic] private static EasyLoggingStarter t_easyLoggerStarter; @@ -67,11 +70,11 @@ public void BeforeEach() t_easyLoggingProvider = new Mock(); t_easyLoggerManager = new Mock(); t_directoryOperations = new Mock(); - t_easyLoggerStarter = new EasyLoggingStarter(t_easyLoggingProvider.Object, t_easyLoggerManager.Object, t_directoryOperations.Object); + t_environmentOperations = new Mock(); + t_easyLoggerStarter = new EasyLoggingStarter(t_easyLoggingProvider.Object, t_easyLoggerManager.Object, t_directoryOperations.Object, t_environmentOperations.Object); } [Test] - //[Ignore("This test requires manual interaction and therefore cannot be run in CI")] public void TestThatCreatedDirectoryPermissionsFollowUmask() { // Note: To test with a different value than the default umask, it will have to be set before running this test @@ -103,24 +106,22 @@ public void TestThatCreatedDirectoryPermissionsFollowUmask() } [Test] - [Ignore("This test requires manual interaction and therefore cannot be run in CI")] public void TestThatThrowsErrorWhenLogPathAndHomeDirectoryIsNotSet() { - // Note: The user home directory needs to be reconfigured to null for the test to pass - if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) - { - // arrange - t_easyLoggingProvider - .Setup(provider => provider.ProvideConfig(ConfigPath)) - .Returns(s_configWithNoLogPath); + // 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(() => t_easyLoggerStarter.Init(ConfigPath)); + // act + var thrown = Assert.Throws(() => 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"); - } + // 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] diff --git a/Snowflake.Data/Core/Session/EasyLoggingStarter.cs b/Snowflake.Data/Core/Session/EasyLoggingStarter.cs index 73db008e0..7bea6e323 100644 --- a/Snowflake.Data/Core/Session/EasyLoggingStarter.cs +++ b/Snowflake.Data/Core/Session/EasyLoggingStarter.cs @@ -22,21 +22,25 @@ internal class EasyLoggingStarter private readonly DirectoryOperations _directoryOperations; + private readonly EnvironmentOperations _environmentOperations; + private readonly object _lockForExclusiveInit = new object(); private EasyLoggingInitTrialParameters _initTrialParameters = null; public static readonly EasyLoggingStarter Instance = new EasyLoggingStarter(EasyLoggingConfigProvider.Instance, - EasyLoggerManager.Instance, DirectoryOperations.Instance); + EasyLoggerManager.Instance, DirectoryOperations.Instance, EnvironmentOperations.Instance); internal EasyLoggingStarter( EasyLoggingConfigProvider easyLoggingConfigProvider, EasyLoggerManager easyLoggerManager, - DirectoryOperations directoryOperations) + DirectoryOperations directoryOperations, + EnvironmentOperations environmentOperations) { _easyLoggingConfigProvider = easyLoggingConfigProvider; _easyLoggerManager = easyLoggerManager; _directoryOperations = directoryOperations; + _environmentOperations = environmentOperations; } internal EasyLoggingStarter() @@ -105,7 +109,7 @@ private string GetLogPath(string logPath) if (string.IsNullOrEmpty(logPath)) { s_logger.Warn("LogPath in client config not found. Using home directory as a default value"); - logPathOrDefault = EnvironmentOperations.Instance.GetFolderPath(Environment.SpecialFolder.UserProfile); + logPathOrDefault = _environmentOperations.GetFolderPath(Environment.SpecialFolder.UserProfile); if (string.IsNullOrEmpty(logPathOrDefault)) { throw new Exception("No log path found for easy logging. Home directory is not configured and log path is not provided"); From 9d2596cc06f33ad76f21ff4e00d481ce96e4a1d2 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Tue, 16 Jan 2024 10:38:49 -0800 Subject: [PATCH 16/45] SNOW-990111: Add easy logging improvements --- .../UnitTests/Logger/EasyLoggerManagerTest.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerManagerTest.cs b/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerManagerTest.cs index 4ff35fe4a..a6b13e59b 100644 --- a/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerManagerTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerManagerTest.cs @@ -99,8 +99,8 @@ public void TestThatUnknownFieldsAreLogged() // arrange string ConfigWithUnknownFields = @"{ ""common"": { - ""log_level"": ""log_level"", - ""log_path"": ""log_path"", + ""log_level"": ""warn"", + ""log_path"": ""path"", ""fake_log_field_1"": ""abc"", ""fake_log_field_2"": ""123"" } From 8d316729e7dbbafd4b4ed8cd11d824515010b559 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Wed, 17 Jan 2024 15:08:01 -0800 Subject: [PATCH 17/45] SNOW-990111: Refactor easy logging improvements --- .../Configuration/EasyLoggingConfigFinder.cs | 40 ++++++++----------- .../Configuration/EasyLoggingConfigParser.cs | 34 +++++++--------- 2 files changed, 30 insertions(+), 44 deletions(-) diff --git a/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs b/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs index f02e7443d..12ac2b59c 100644 --- a/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs +++ b/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs @@ -5,8 +5,6 @@ using System; using System.IO; using System.Runtime.InteropServices; -using Snowflake.Data.Client; -using Snowflake.Data.Core; using Snowflake.Data.Core.Tools; using Snowflake.Data.Log; @@ -36,10 +34,15 @@ internal EasyLoggingConfigFinder() public virtual string FindConfigFilePath(string configFilePathFromConnectionString) { - return GetFilePathFromInputParameter(configFilePathFromConnectionString, "connection string") - ?? GetFilePathEnvironmentVariable() - ?? GetFilePathFromDriverLocation() - ?? GetFilePathFromHomeDirectory(); + var configFilePath = GetFilePathFromInputParameter(configFilePathFromConnectionString, "connection string") + ?? GetFilePathEnvironmentVariable() + ?? GetFilePathFromDriverLocation() + ?? GetFilePathFromHomeDirectory(); + if (configFilePath != null) + { + CheckIfValidPermissions(configFilePath); + } + return configFilePath; } private string GetFilePathEnvironmentVariable() @@ -52,19 +55,12 @@ private string GetFilePathEnvironmentVariable() private string GetFilePathFromInputParameter(string filePath, string inputDescription) { - if (!string.IsNullOrEmpty(filePath)) - { - if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) - { - CheckIfValidPermissions(filePath); - } - s_logger.Info($"Using config file specified from {inputDescription}"); - return filePath; - } - else + 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); @@ -95,21 +91,17 @@ private string OnlyIfFileExists(string filePath, string directoryDescription) { if (_fileOperations.Exists(filePath)) { - if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) - { - CheckIfValidPermissions(filePath); - } s_logger.Info($"Using config file specified from {directoryDescription} directory"); return filePath; } - else - { - return null; - } + 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 int filePermissions; string commandParameters = RuntimeInformation.IsOSPlatform(OSPlatform.Linux) ? "-c '%a'" : "-f %A"; diff --git a/Snowflake.Data/Configuration/EasyLoggingConfigParser.cs b/Snowflake.Data/Configuration/EasyLoggingConfigParser.cs index fc45e685b..2ba28c787 100644 --- a/Snowflake.Data/Configuration/EasyLoggingConfigParser.cs +++ b/Snowflake.Data/Configuration/EasyLoggingConfigParser.cs @@ -3,7 +3,9 @@ */ using System; +using System.Collections.Generic; using System.IO; +using System.Linq; using System.Reflection; using Newtonsoft.Json; using Newtonsoft.Json.Linq; @@ -46,7 +48,7 @@ private ClientConfig TryToParseFile(string fileContent) try { var config = JsonConvert.DeserializeObject(fileContent); Validate(config); - CheckForUnknownFields(fileContent, config); + CheckForUnknownFields(fileContent); return config; } catch (Exception e) @@ -65,29 +67,21 @@ private void Validate(ClientConfig config) } } - private void CheckForUnknownFields(string fileContent, ClientConfig config) + private void CheckForUnknownFields(string fileContent) { // Parse the specified config file and get the key-value pairs from the "common" section - JObject obj = (JObject)(JObject.Parse(fileContent).First.First); - bool isUnknownField = true; - foreach (var keyValuePair in obj) + List knownProperties = new List(); + foreach (var property in typeof(ClientConfigCommonProps).GetProperties()) { - foreach(var property in config.CommonProps.GetType().GetProperties()) - { - var jsonPropertyAttribute = property.GetCustomAttribute(); - if (keyValuePair.Key.Equals(jsonPropertyAttribute.PropertyName)) - { - isUnknownField = false; - break; - } - } - if (isUnknownField) - { - s_logger.Warn($"Unknown field from config: {keyValuePair.Key}"); - } - - isUnknownField = true; + var jsonPropertyAttribute = property.GetCustomAttribute(); + knownProperties.Add(jsonPropertyAttribute.PropertyName); } + + JObject.Parse(fileContent).GetValue("common") + .Cast() + .Where(property => knownProperties.Contains(property.Name)) + .ToList() + .ForEach(unknownKey => s_logger.Warn($"Unknown field from config: {unknownKey}")); } } } From e185fbd744d1c7c9fb84842e4076caa8d72a3480 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Wed, 17 Jan 2024 15:53:37 -0800 Subject: [PATCH 18/45] SNOW-990111: Refactor easy logging improvements --- Snowflake.Data/Configuration/EasyLoggingConfigParser.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Snowflake.Data/Configuration/EasyLoggingConfigParser.cs b/Snowflake.Data/Configuration/EasyLoggingConfigParser.cs index 2ba28c787..b00c2c5ec 100644 --- a/Snowflake.Data/Configuration/EasyLoggingConfigParser.cs +++ b/Snowflake.Data/Configuration/EasyLoggingConfigParser.cs @@ -81,7 +81,7 @@ private void CheckForUnknownFields(string fileContent) .Cast() .Where(property => knownProperties.Contains(property.Name)) .ToList() - .ForEach(unknownKey => s_logger.Warn($"Unknown field from config: {unknownKey}")); + .ForEach(unknownKey => s_logger.Warn($"Unknown field from config: {unknownKey.Name}")); } } } From bcd0936ab9c4ccef9f1f2f2a270f494c0b3efadc Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Thu, 18 Jan 2024 18:52:05 -0800 Subject: [PATCH 19/45] SNOW-990111: Refactor easy logging improvements --- .../Configuration/EasyLoggingConfigFinder.cs | 9 ++-- .../Core/Session/EasyLoggingStarter.cs | 13 +++-- Snowflake.Data/Logger/EasyLoggerUtil.cs | 51 +++++++++++++++++++ Snowflake.Data/Snowflake.Data.csproj | 1 + 4 files changed, 63 insertions(+), 11 deletions(-) diff --git a/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs b/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs index 12ac2b59c..d99f1d95d 100644 --- a/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs +++ b/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs @@ -5,6 +5,7 @@ using System; using System.IO; using System.Runtime.InteropServices; +using Mono.Unix; using Snowflake.Data.Core.Tools; using Snowflake.Data.Log; @@ -103,10 +104,10 @@ private void CheckIfValidPermissions(string filePath) return; // Check if others have permissions to modify the file and fail if so - int filePermissions; - string commandParameters = RuntimeInformation.IsOSPlatform(OSPlatform.Linux) ? "-c '%a'" : "-f %A"; - bool isParsed = int.TryParse(EasyLoggerUtil.CallBash($"stat {commandParameters} {filePath}"), out filePermissions); - if (isParsed && filePermissions > EasyLoggerUtil.OnlyUserHasPermissionToWrite) + 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); diff --git a/Snowflake.Data/Core/Session/EasyLoggingStarter.cs b/Snowflake.Data/Core/Session/EasyLoggingStarter.cs index 7bea6e323..964c339fe 100644 --- a/Snowflake.Data/Core/Session/EasyLoggingStarter.cs +++ b/Snowflake.Data/Core/Session/EasyLoggingStarter.cs @@ -5,7 +5,7 @@ using System; using System.IO; using System.Runtime.InteropServices; -using Snowflake.Data.Client; +using Mono.Unix; using Snowflake.Data.Configuration; using Snowflake.Data.Core.Tools; using Snowflake.Data.Log; @@ -123,17 +123,16 @@ private string GetLogPath(string logPath) if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { var umask = EasyLoggerUtil.AllPermissions - int.Parse(EasyLoggerUtil.CallBash("umask")); - int dirPermissions; - string commandParameters = RuntimeInformation.IsOSPlatform(OSPlatform.Linux) ? "-c '%a'" : "-f %A"; - bool isParsed = int.TryParse(EasyLoggerUtil.CallBash($"stat {commandParameters} {pathWithDotnetSubdirectory}"), out dirPermissions); - if (isParsed && dirPermissions != EasyLoggerUtil.AllUserPermissions) + var dirInfo = new UnixDirectoryInfo(pathWithDotnetSubdirectory); + if (dirInfo.Exists && dirInfo.FileAccessPermissions != FileAccessPermissions.UserReadWriteExecute) { - s_logger.Warn($"Access permission for the logs directory is {dirPermissions}"); + var dirPermissions = EasyLoggerUtil.ConvertFileAccessPermissionsToInt(dirInfo.FileAccessPermissions); + s_logger.Warn($"Access permission for the logs directory is {dirPermissions}"); if (dirPermissions != umask) { s_logger.Warn($"Setting access permission for the logs directory from {dirPermissions} to {EasyLoggerUtil.AllUserPermissions}"); - EasyLoggerUtil.CallBash($"chmod -R {EasyLoggerUtil.AllUserPermissions} {pathWithDotnetSubdirectory}"); + dirInfo.FileAccessPermissions = Mono.Unix.FileAccessPermissions.UserReadWriteExecute; } } } diff --git a/Snowflake.Data/Logger/EasyLoggerUtil.cs b/Snowflake.Data/Logger/EasyLoggerUtil.cs index 259027433..342658f53 100644 --- a/Snowflake.Data/Logger/EasyLoggerUtil.cs +++ b/Snowflake.Data/Logger/EasyLoggerUtil.cs @@ -2,7 +2,9 @@ * Copyright (c) 2024 Snowflake Computing Inc. All rights reserved. */ +using System; using System.Diagnostics; +using Mono.Unix; namespace Snowflake.Data.Log { @@ -14,6 +16,55 @@ internal class EasyLoggerUtil internal static int OnlyUserHasPermissionToWrite = 644; + internal static int ConvertFileAccessPermissionsToInt(FileAccessPermissions permissions) + { + int userPermission = 0; + int groupPermission = 0; + int otherPermission = 0; + + foreach (FileAccessPermissions perm in Enum.GetValues(typeof(FileAccessPermissions))) + { + // Check if enum exists in the directory permission + if ((permissions & perm) != 0) + { + string permName = perm.ToString(); + if (!permName.Contains("ReadWriteExecute")) + { + if (permName.Contains("User")) + { + userPermission += (int)perm; + } + else if (permName.Contains("Group")) + { + groupPermission += (int)perm; + } + else if (permName.Contains("Other")) + { + otherPermission += (int)perm; + } + } + } + } + + // Divide by 2^6 + userPermission /= 64; + + // Divide by 2^3 + groupPermission /= 8; + + return Convert.ToInt32(string.Format("{0}{1}{2}", userPermission, groupPermission, otherPermission)); + } + + internal static bool OnlyUserHasAllPermissions(FileAccessPermissions permissions) + { + return permissions == FileAccessPermissions.UserReadWriteExecute; + } + + internal static bool OnlyUserCanWrite(FileAccessPermissions permissions) + { + return permissions == FileAccessPermissions.UserWrite; + } + internal static string CallBash(string command) { using (Process process = new Process()) diff --git a/Snowflake.Data/Snowflake.Data.csproj b/Snowflake.Data/Snowflake.Data.csproj index 9649f18f7..bb18b0a41 100644 --- a/Snowflake.Data/Snowflake.Data.csproj +++ b/Snowflake.Data/Snowflake.Data.csproj @@ -24,6 +24,7 @@ + From 08e378599cfc05c2b30d6615c7b8d021c2e04d62 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Thu, 18 Jan 2024 19:49:37 -0800 Subject: [PATCH 20/45] SNOW-990111: Refactor easy logging improvements --- Snowflake.Data/Snowflake.Data.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Snowflake.Data/Snowflake.Data.csproj b/Snowflake.Data/Snowflake.Data.csproj index bb18b0a41..6eb4c2263 100644 --- a/Snowflake.Data/Snowflake.Data.csproj +++ b/Snowflake.Data/Snowflake.Data.csproj @@ -24,7 +24,7 @@ - + From e40116acd54da8f11540aa33ebabd62597a346b7 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Mon, 22 Jan 2024 16:19:43 -0800 Subject: [PATCH 21/45] SNOW-990111: Refactor easy logging changes --- .../EasyLoggingConfigFinderTest.cs | 89 ++++++++++--------- .../UnitTests/Logger/EasyLoggerManagerTest.cs | 21 ++--- .../Session/EasyLoggingStarterTest.cs | 85 +++++++++++++----- .../Configuration/EasyLoggingConfigFinder.cs | 18 ++-- .../Configuration/EasyLoggingConfigParser.cs | 18 ++-- .../Core/Session/EasyLoggingStarter.cs | 45 ++++++---- Snowflake.Data/Logger/EasyLoggerUtil.cs | 29 ------ 7 files changed, 169 insertions(+), 136 deletions(-) diff --git a/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs b/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs index caa9cc260..c55ec9154 100644 --- a/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs @@ -18,14 +18,17 @@ public class EasyLoggingConfigFinderTest { private const string InputConfigFilePath = "input_config.json"; private const string EnvironmentalConfigFilePath = "environmental_config.json"; - private static readonly string HomeDirectory = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile); - private static readonly string s_driverConfigFilePath = Path.Combine(".", EasyLoggingConfigFinder.ClientConfigFileName); + private const string HomeDirectory = "/home/user"; + private const string DriverDirectory = "."; + private static readonly string s_driverConfigFilePath = Path.Combine(DriverDirectory, 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); [ThreadStatic] private static Mock t_fileOperations; + [ThreadStatic] + private static Mock t_directoryOperations; + [ThreadStatic] private static Mock t_environmentOperations; @@ -36,8 +39,10 @@ public class EasyLoggingConfigFinderTest public void Setup() { t_fileOperations = new Mock(); + t_directoryOperations = new Mock(); t_environmentOperations = new Mock(); - t_finder = new EasyLoggingConfigFinder(t_fileOperations.Object, t_environmentOperations.Object); + t_finder = new EasyLoggingConfigFinder(t_fileOperations.Object, t_directoryOperations.Object, t_environmentOperations.Object); + MockDirectoriesExist(); MockHomeDirectory(); } @@ -48,7 +53,6 @@ public void TestThatTakesFilePathFromTheInput() MockFileFromEnvironmentalVariable(); MockFileOnDriverPath(); MockFileOnHomePath(); - MockFileOnTempPath(); // act var filePath = t_finder.FindConfigFilePath(InputConfigFilePath); @@ -67,7 +71,6 @@ public void TestThatTakesFilePathFromEnvironmentVariableIfInputNotPresent( MockFileFromEnvironmentalVariable(); MockFileOnDriverPath(); MockFileOnHomePath(); - MockFileOnTempPath(); // act var filePath = t_finder.FindConfigFilePath(inputFilePath); @@ -82,7 +85,6 @@ public void TestThatTakesFilePathFromDriverLocationWhenNoInputParameterNorEnviro // arrange MockFileOnDriverPath(); MockFileOnHomePath(); - MockFileOnTempPath(); // act var filePath = t_finder.FindConfigFilePath(null); @@ -96,7 +98,6 @@ public void TestThatTakesFilePathFromHomeLocationWhenNoInputParamEnvironmentVarN { // arrange MockFileOnHomePath(); - MockFileOnTempPath(); // act var filePath = t_finder.FindConfigFilePath(null); @@ -119,38 +120,26 @@ public void TestThatTakesFilePathFromHomeDirectoryWhenNoOtherWaysPossible() } [Test] - [Ignore("This test requires manual interaction and therefore cannot be run in CI")] + [Ignore("TODO: modify the test and remove Ignore")] 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)) + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { - // arrange - var configFilePath = CreateConfigTempFile(Config(EasyLoggingLogLevel.Warn.ToString(), InputConfigFilePath)); - - // act - var thrown = Assert.Throws(() => 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); + Assert.Ignore("skip test on Windows"); } - } - [Test] - public void TestThatDoesNotTakeFilePathFromTempDirectoryWhenNoOtherWaysPossible() - { // arrange - MockFileOnTempPath(); - + var configFilePath = CreateConfigTempFile(Config(EasyLoggingLogLevel.Warn.ToString(), InputConfigFilePath)); + // act - var filePath = t_finder.FindConfigFilePath(null); - + var thrown = Assert.Throws(() => t_finder.FindConfigFilePath(configFilePath)); + // assert - Assert.Null(filePath); + Assert.IsNotNull(thrown); + Assert.AreEqual(thrown.Message, "Error due to other users having permission to modify the config file"); + + // cleanup + File.Delete(configFilePath); } [Test] @@ -176,12 +165,14 @@ public void TestThatDoesNotFailWhenSearchForOneOfDirectoriesFails() Assert.IsNull(filePath); t_environmentOperations.Verify(e => e.GetFolderPath(Environment.SpecialFolder.UserProfile), Times.Once); } - + [Test] - public void TestThatDoesNotFailWhenOneOfDirectoriesNotDefined() + public void TestThatDoesNotFailWhenHomeDirectoryDoesNotExist() { // arrange - MockHomeDirectoryReturnsNull(); + MockFileOnHomePath(); + MockHomeDirectoryDoesNotExist(); + MockFileOnHomePathDoesNotExist(); // act var filePath = t_finder.FindConfigFilePath(null); @@ -191,6 +182,13 @@ public void TestThatDoesNotFailWhenOneOfDirectoriesNotDefined() t_environmentOperations.Verify(e => e.GetFolderPath(Environment.SpecialFolder.UserProfile), Times.Once); } + private static void MockDirectoriesExist() + { + t_directoryOperations + .Setup(d => d.Exists(It.IsAny())) + .Returns(true); + } + private static void MockHomeDirectory() { t_environmentOperations @@ -205,6 +203,20 @@ private static void MockHomeDirectoryFails() .Throws(() => new Exception("No home directory")); } + private static void MockHomeDirectoryDoesNotExist() + { + t_directoryOperations + .Setup(d => d.Exists(HomeDirectory)) + .Returns(false); + } + + private static void MockFileOnHomePathDoesNotExist() + { + t_fileOperations + .Setup(f => f.Exists(s_homeConfigFilePath)) + .Returns(false); + } + private static void MockHomeDirectoryReturnsNull() { t_environmentOperations @@ -232,12 +244,5 @@ private static void MockFileOnHomePath() .Setup(f => f.Exists(s_homeConfigFilePath)) .Returns(true); } - - private static void MockFileOnTempPath() - { - t_fileOperations - .Setup(f => f.Exists(s_tempConfigFilePath)) - .Returns(true); - } } } diff --git a/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerManagerTest.cs b/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerManagerTest.cs index a6b13e59b..34e2d4f96 100644 --- a/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerManagerTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerManagerTest.cs @@ -94,17 +94,18 @@ public void TestThatLogsToProperFileWithProperLogLevelOnly() } [Test] - public void TestThatUnknownFieldsAreLogged() + public void TestThatOnlyUnknownFieldsAreLogged() { // arrange - string ConfigWithUnknownFields = @"{ - ""common"": { - ""log_level"": ""warn"", - ""log_path"": ""path"", - ""fake_log_field_1"": ""abc"", - ""fake_log_field_2"": ""123"" - } - }"; + string expectedFakeLogField = "fake_log_field"; + string ConfigWithUnknownFields = $@"{{ + ""common"": {{ + ""LOG_LEVEL"": ""warn"", + ""lOg_PaTh"": ""path"", + ""{expectedFakeLogField}_1"": ""abc"", + ""{expectedFakeLogField}_2"": ""123"" + }} + }}"; var configFilePath = Guid.NewGuid().ToString() + ".json"; using (var writer = File.CreateText(configFilePath)) { @@ -118,7 +119,7 @@ public void TestThatUnknownFieldsAreLogged() // assert var logLines = File.ReadLines(FindLogFilePath(t_directoryLogPath)); - Assert.That(logLines, Has.Exactly(2).Matches(s => s.Contains($"Unknown field from config: "))); + Assert.That(logLines, Has.Exactly(2).Matches(s => s.Contains($"Unknown field from config: {expectedFakeLogField}"))); // cleanup File.Delete(configFilePath); diff --git a/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs b/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs index 24c2e09d3..3519dff8f 100644 --- a/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs @@ -3,6 +3,7 @@ */ using System; +using System.Diagnostics; using System.IO; using System.Runtime.InteropServices; using Moq; @@ -17,6 +18,7 @@ namespace Snowflake.Data.Tests.UnitTests.Session [TestFixture] public class EasyLoggingStarterTest { + private static readonly int AllPermissions = 777; 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"; @@ -78,31 +80,33 @@ public void BeforeEach() 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)) + 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); + Assert.Ignore("skip test on 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 expectedDirPermissions = AllPermissions - int.Parse(CallBash("umask")); + string commandParameters = RuntimeInformation.IsOSPlatform(OSPlatform.Linux) ? "-c '%a'" : "-f %A"; + var dirPermissions = int.Parse(CallBash($"stat {commandParameters} {s_expectedLogPath}")); + + // assert + Assert.AreEqual(expectedDirPermissions, dirPermissions); + + // cleanup + Directory.Delete(s_expectedLogPath); } [Test] @@ -121,7 +125,27 @@ public void TestThatThrowsErrorWhenLogPathAndHomeDirectoryIsNotSet() // 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"); + Assert.AreEqual(thrown.InnerException.Message, "No log path found for easy logging. Home directory is not configured and log path is not provided"); + } + + [Test] + public void TestThatThrowsErrorWhenLogPathIsNotSetAndHomeDirectoryThrowsAnException() + { + // arrange + var ex = new Exception("No home directory"); + t_easyLoggingProvider + .Setup(provider => provider.ProvideConfig(ConfigPath)) + .Returns(s_configWithNoLogPath); + t_environmentOperations + .Setup(provider => provider.GetFolderPath(Environment.SpecialFolder.UserProfile)) + .Throws(() => ex); + + // act + var thrown = Assert.Throws(() => t_easyLoggerStarter.Init(ConfigPath)); + + // assert + Assert.IsNotNull(thrown); + Assert.AreEqual(thrown.Message, $"Error while trying to retrieve the home directory: {ex}"); } [Test] @@ -196,5 +220,18 @@ public void TestThatReconfiguresEasyLoggingWithConfigPathIfNotGivenForTheFirstTi t_easyLoggerManager.Verify(manager => manager.ReconfigureEasyLogging(EasyLoggingLogLevel.Info, s_expectedLogPath), Times.Once); t_easyLoggerManager.VerifyNoOtherCalls(); } + + private static string CallBash(string command) + { + using (Process process = new Process()) + { + process.StartInfo.FileName = "/bin/bash"; + process.StartInfo.Arguments = $"-c \"{command}\""; + process.StartInfo.UseShellExecute = false; + process.StartInfo.RedirectStandardOutput = true; + process.Start(); + return process.StandardOutput.ReadToEnd().Replace("\n", string.Empty); + } + } } } diff --git a/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs b/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs index d99f1d95d..1d66e7bf1 100644 --- a/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs +++ b/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs @@ -19,13 +19,15 @@ internal class EasyLoggingConfigFinder internal const string ClientConfigEnvironmentName = "SF_CLIENT_CONFIG_FILE"; private readonly FileOperations _fileOperations; + private readonly DirectoryOperations _directoryOperations; private readonly EnvironmentOperations _environmentOperations; - public static readonly EasyLoggingConfigFinder Instance = new EasyLoggingConfigFinder(FileOperations.Instance, EnvironmentOperations.Instance); + public static readonly EasyLoggingConfigFinder Instance = new EasyLoggingConfigFinder(FileOperations.Instance, DirectoryOperations.Instance, EnvironmentOperations.Instance); - internal EasyLoggingConfigFinder(FileOperations fileOperations, EnvironmentOperations environmentOperations) + internal EasyLoggingConfigFinder(FileOperations fileOperations, DirectoryOperations directoryOperations, EnvironmentOperations environmentOperations) { _fileOperations = fileOperations; + _directoryOperations = directoryOperations; _environmentOperations = environmentOperations; } @@ -60,7 +62,7 @@ private string GetFilePathFromInputParameter(string filePath, string inputDescri { return null; } - s_logger.Info($"Using config file specified from {inputDescription}"); + s_logger.Info($"Using config file specified from {inputDescription}: {filePath}"); return filePath; } @@ -73,10 +75,14 @@ private string SearchForConfigInDirectory(Func directoryProvider, string try { var directory = directoryProvider.Invoke(); - if (string.IsNullOrEmpty(directory) || !Directory.Exists(directory)) + if (string.IsNullOrEmpty(directory)) { return null; } + if (!_directoryOperations.Exists(directory)) + { + s_logger.Warn($"Searching for a config file in the {directoryDescription} directory that does not exist: {directory}"); + } var filePath = Path.Combine(directory, ClientConfigFileName); return OnlyIfFileExists(filePath, directoryDescription); @@ -92,7 +98,7 @@ private string OnlyIfFileExists(string filePath, string directoryDescription) { if (_fileOperations.Exists(filePath)) { - s_logger.Info($"Using config file specified from {directoryDescription} directory"); + s_logger.Info($"Using config file specified from {directoryDescription} directory: {filePath}"); return filePath; } return null; @@ -109,7 +115,7 @@ private void CheckIfValidPermissions(string filePath) ((fileInfo.FileAccessPermissions & FileAccessPermissions.GroupWrite) != 0 || (fileInfo.FileAccessPermissions & FileAccessPermissions.OtherWrite) != 0)) { - var errorMessage = "Error due to other users having permission to modify the config file"; + var errorMessage = $"Error due to other users having permission to modify the config file: {filePath}"; s_logger.Error(errorMessage); throw new Exception(errorMessage); } diff --git a/Snowflake.Data/Configuration/EasyLoggingConfigParser.cs b/Snowflake.Data/Configuration/EasyLoggingConfigParser.cs index b00c2c5ec..7b59afa21 100644 --- a/Snowflake.Data/Configuration/EasyLoggingConfigParser.cs +++ b/Snowflake.Data/Configuration/EasyLoggingConfigParser.cs @@ -70,16 +70,22 @@ private void Validate(ClientConfig config) private void CheckForUnknownFields(string fileContent) { // Parse the specified config file and get the key-value pairs from the "common" section - List knownProperties = new List(); - foreach (var property in typeof(ClientConfigCommonProps).GetProperties()) - { + HashSet knownProperties = new HashSet(); +#if NET471 + foreach (var property in typeof(ClientConfigCommonProps).GetProperties()) + { var jsonPropertyAttribute = property.GetCustomAttribute(); knownProperties.Add(jsonPropertyAttribute.PropertyName); - } + } +#else + knownProperties = typeof(ClientConfigCommonProps).GetProperties() + .Select(property => property.GetCustomAttribute().PropertyName) + .ToHashSet(); +#endif - JObject.Parse(fileContent).GetValue("common") + JObject.Parse(fileContent).GetValue("common", StringComparison.OrdinalIgnoreCase)? .Cast() - .Where(property => knownProperties.Contains(property.Name)) + .Where(property => !knownProperties.Contains(property.Name, StringComparer.OrdinalIgnoreCase)) .ToList() .ForEach(unknownKey => s_logger.Warn($"Unknown field from config: {unknownKey.Name}")); } diff --git a/Snowflake.Data/Core/Session/EasyLoggingStarter.cs b/Snowflake.Data/Core/Session/EasyLoggingStarter.cs index 964c339fe..b6950e511 100644 --- a/Snowflake.Data/Core/Session/EasyLoggingStarter.cs +++ b/Snowflake.Data/Core/Session/EasyLoggingStarter.cs @@ -109,10 +109,19 @@ private string GetLogPath(string logPath) if (string.IsNullOrEmpty(logPath)) { s_logger.Warn("LogPath in client config not found. Using home directory as a default value"); - logPathOrDefault = _environmentOperations.GetFolderPath(Environment.SpecialFolder.UserProfile); - if (string.IsNullOrEmpty(logPathOrDefault)) + try { - throw new Exception("No log path found for easy logging. Home directory is not configured and log path is not provided"); + logPathOrDefault = _environmentOperations.GetFolderPath(Environment.SpecialFolder.UserProfile); + if (string.IsNullOrEmpty(logPathOrDefault)) + { + throw new Exception("No log path found for easy logging. Home directory is not configured and log path is not provided"); + } + } + catch(Exception e) + { + var errorMessage = $"Error while trying to retrieve the home directory: {e}"; + s_logger.Error(errorMessage); + throw new Exception(errorMessage, e); } } var pathWithDotnetSubdirectory = Path.Combine(logPathOrDefault, "dotnet"); @@ -120,26 +129,24 @@ private string GetLogPath(string logPath) { _directoryOperations.CreateDirectory(pathWithDotnetSubdirectory); - if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) - { - var umask = EasyLoggerUtil.AllPermissions - int.Parse(EasyLoggerUtil.CallBash("umask")); - var dirInfo = new UnixDirectoryInfo(pathWithDotnetSubdirectory); - if (dirInfo.Exists && dirInfo.FileAccessPermissions != FileAccessPermissions.UserReadWriteExecute) - { - var dirPermissions = EasyLoggerUtil.ConvertFileAccessPermissionsToInt(dirInfo.FileAccessPermissions); - - s_logger.Warn($"Access permission for the logs directory is {dirPermissions}"); - if (dirPermissions != umask) - { - s_logger.Warn($"Setting access permission for the logs directory from {dirPermissions} to {EasyLoggerUtil.AllUserPermissions}"); - dirInfo.FileAccessPermissions = Mono.Unix.FileAccessPermissions.UserReadWriteExecute; - } - } - } + CheckDirPermissionsOnlyAllowUser(pathWithDotnetSubdirectory); } return pathWithDotnetSubdirectory; } + + private void CheckDirPermissionsOnlyAllowUser(string dirPath) + { + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + return; + + var dirInfo = new UnixDirectoryInfo(dirPath); + if (dirInfo.Exists && dirInfo.FileAccessPermissions != FileAccessPermissions.UserReadWriteExecute) + { + var dirPermissions = EasyLoggerUtil.ConvertFileAccessPermissionsToInt(dirInfo.FileAccessPermissions); + s_logger.Warn($"Access permission for the logs directory is currently {dirPermissions}"); + } + } } internal class EasyLoggingInitTrialParameters diff --git a/Snowflake.Data/Logger/EasyLoggerUtil.cs b/Snowflake.Data/Logger/EasyLoggerUtil.cs index 342658f53..2ae70b24f 100644 --- a/Snowflake.Data/Logger/EasyLoggerUtil.cs +++ b/Snowflake.Data/Logger/EasyLoggerUtil.cs @@ -10,12 +10,6 @@ namespace Snowflake.Data.Log { internal class EasyLoggerUtil { - internal static int AllPermissions = 777; - - internal static int AllUserPermissions = 700; - - internal static int OnlyUserHasPermissionToWrite = 644; - internal static int ConvertFileAccessPermissionsToInt(FileAccessPermissions permissions) { int userPermission = 0; @@ -54,28 +48,5 @@ internal static int ConvertFileAccessPermissionsToInt(FileAccessPermissions perm return Convert.ToInt32(string.Format("{0}{1}{2}", userPermission, groupPermission, otherPermission)); } - - internal static bool OnlyUserHasAllPermissions(FileAccessPermissions permissions) - { - return permissions == FileAccessPermissions.UserReadWriteExecute; - } - - internal static bool OnlyUserCanWrite(FileAccessPermissions permissions) - { - return permissions == FileAccessPermissions.UserWrite; - } - - internal static string CallBash(string command) - { - using (Process process = new Process()) - { - process.StartInfo.FileName = "/bin/bash"; - process.StartInfo.Arguments = $"-c \"{command}\""; - process.StartInfo.UseShellExecute = false; - process.StartInfo.RedirectStandardOutput = true; - process.Start(); - return process.StandardOutput.ReadToEnd().Replace("\n", string.Empty); - } - } } } From 47d07bf9775c9f242c410cbea58b9f66a565d95f Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Mon, 22 Jan 2024 16:49:03 -0800 Subject: [PATCH 22/45] SNOW-990111: Refactor easy logging changes --- .../UnitTests/Session/EasyLoggingStarterTest.cs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs b/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs index 3519dff8f..162d90639 100644 --- a/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs @@ -6,6 +6,7 @@ using System.Diagnostics; using System.IO; using System.Runtime.InteropServices; +using Mono.Unix; using Moq; using NUnit.Framework; using Snowflake.Data.Configuration; @@ -96,11 +97,12 @@ public void TestThatCreatedDirectoryPermissionsFollowUmask() .Setup(provider => provider.CreateDirectory(s_expectedLogPath)) .Returns(Directory.CreateDirectory(s_expectedLogPath)); + var expectedDirPermissions = AllPermissions - int.Parse(CallBash("umask")); + // act t_easyLoggerStarter.Init(ConfigPath); - var expectedDirPermissions = AllPermissions - int.Parse(CallBash("umask")); - string commandParameters = RuntimeInformation.IsOSPlatform(OSPlatform.Linux) ? "-c '%a'" : "-f %A"; - var dirPermissions = int.Parse(CallBash($"stat {commandParameters} {s_expectedLogPath}")); + var dirInfo = new UnixDirectoryInfo(s_expectedLogPath); + var dirPermissions = EasyLoggerUtil.ConvertFileAccessPermissionsToInt(dirInfo.FileAccessPermissions); // assert Assert.AreEqual(expectedDirPermissions, dirPermissions); From 1c43705c82ef01b13c2b396d79d6461aff795ae1 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Mon, 22 Jan 2024 17:16:37 -0800 Subject: [PATCH 23/45] SNOW-990111: Refactor easy logging changes --- .../UnitTests/Configuration/EasyLoggingConfigFinderTest.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs b/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs index c55ec9154..2b32b6bd5 100644 --- a/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs @@ -185,7 +185,8 @@ public void TestThatDoesNotFailWhenHomeDirectoryDoesNotExist() private static void MockDirectoriesExist() { t_directoryOperations - .Setup(d => d.Exists(It.IsAny())) + .Setup(d => d.Exists( + It.Is(dir => dir.Equals(DriverDirectory) || dir.Equals(HomeDirectory)))) .Returns(true); } From eff94253ed0bba2e2dc68de170f5cf93bb1844c2 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Tue, 23 Jan 2024 17:15:43 -0800 Subject: [PATCH 24/45] SNOW-990111: Add test for EasyLoggerUtil --- .../UnitTests/Logger/EasyLoggerUtilTest.cs | 225 ++++++++++++++++++ Snowflake.Data/Logger/EasyLoggerUtil.cs | 1 - 2 files changed, 225 insertions(+), 1 deletion(-) create mode 100644 Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerUtilTest.cs diff --git a/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerUtilTest.cs b/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerUtilTest.cs new file mode 100644 index 000000000..debd7e244 --- /dev/null +++ b/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerUtilTest.cs @@ -0,0 +1,225 @@ +/* + * Copyright (c) 2023 Snowflake Computing Inc. All rights reserved. + */ + +using Mono.Unix; +using NUnit.Framework; +using Snowflake.Data.Log; +using System; +using System.Collections.Generic; + +namespace Snowflake.Data.Tests.UnitTests.Logger +{ + [TestFixture] + public class EasyLoggerUtilTest + { + [Test] + public void TestConversionForAllPermissionCombinations( + [ValueSource(nameof(UserPermissionTestCases))] PermissionTestCase userTestCase, + [ValueSource(nameof(GroupPermissionTestCases))] PermissionTestCase groupTestCase, + [ValueSource(nameof(OtherPermissionTestCases))] PermissionTestCase otherTestCase) + { + // arrange + var permissions = userTestCase.permissions | groupTestCase.permissions | otherTestCase.permissions; + var expectedPermissions = userTestCase.expectedPermissions + groupTestCase.expectedPermissions + otherTestCase.expectedPermissions; + + // act + var convertedPermissions = EasyLoggerUtil.ConvertFileAccessPermissionsToInt(permissions); + + // assert + Assert.AreEqual(expectedPermissions, convertedPermissions); + } + + public static IEnumerable UserPermissionTestCases() + { + var noPermissionTestCase = new PermissionTestCase() + { + expectedPermissions = 000 + }; + + var readPermissionsTestCase = new PermissionTestCase() + { + permissions = FileAccessPermissions.UserRead, + expectedPermissions = 400 + }; + + var writePermissionsTestCase = new PermissionTestCase() + { + permissions = FileAccessPermissions.UserWrite, + expectedPermissions = 200 + }; + + var executePermissionsTestCase = new PermissionTestCase() + { + permissions = FileAccessPermissions.UserExecute, + expectedPermissions = 100 + }; + + var readWritePermissionsTestCase = new PermissionTestCase() + { + permissions = FileAccessPermissions.UserRead | FileAccessPermissions.UserWrite, + expectedPermissions = 600 + }; + + var readExecutePermissionsTestCase = new PermissionTestCase() + { + permissions = FileAccessPermissions.UserRead | FileAccessPermissions.UserExecute, + expectedPermissions = 500 + }; + + var writeExecutePermissionsTestCase = new PermissionTestCase() + { + permissions = FileAccessPermissions.UserWrite | FileAccessPermissions.UserExecute, + expectedPermissions = 300 + }; + + var allPermissionsTestCase = new PermissionTestCase() + { + permissions = FileAccessPermissions.UserReadWriteExecute, + expectedPermissions = 700 + }; + + return new[] + { + noPermissionTestCase, + readPermissionsTestCase, + writePermissionsTestCase, + executePermissionsTestCase, + readWritePermissionsTestCase, + readExecutePermissionsTestCase, + writeExecutePermissionsTestCase, + allPermissionsTestCase + }; + } + + public static IEnumerable GroupPermissionTestCases() + { + var noPermissionTestCase = new PermissionTestCase() + { + expectedPermissions = 000 + }; + + var readPermissionsTestCase = new PermissionTestCase() + { + permissions = FileAccessPermissions.GroupRead, + expectedPermissions = 040 + }; + + var writePermissionsTestCase = new PermissionTestCase() + { + permissions = FileAccessPermissions.GroupWrite, + expectedPermissions = 020 + }; + + var executePermissionsTestCase = new PermissionTestCase() + { + permissions = FileAccessPermissions.GroupExecute, + expectedPermissions = 010 + }; + + var readWritePermissionsTestCase = new PermissionTestCase() + { + permissions = FileAccessPermissions.GroupRead | FileAccessPermissions.GroupWrite, + expectedPermissions = 060 + }; + + var readExecutePermissionsTestCase = new PermissionTestCase() + { + permissions = FileAccessPermissions.GroupRead | FileAccessPermissions.GroupExecute, + expectedPermissions = 050 + }; + + var writeExecutePermissionsTestCase = new PermissionTestCase() + { + permissions = FileAccessPermissions.GroupWrite | FileAccessPermissions.GroupExecute, + expectedPermissions = 030 + }; + + var allPermissionsTestCase = new PermissionTestCase() + { + permissions = FileAccessPermissions.GroupReadWriteExecute, + expectedPermissions = 070 + }; + + return new[] + { + noPermissionTestCase, + readPermissionsTestCase, + writePermissionsTestCase, + executePermissionsTestCase, + readWritePermissionsTestCase, + readExecutePermissionsTestCase, + writeExecutePermissionsTestCase, + allPermissionsTestCase + }; + } + + public static IEnumerable OtherPermissionTestCases() + { + var noPermissionTestCase = new PermissionTestCase() + { + expectedPermissions = 000 + }; + + var readPermissionsTestCase = new PermissionTestCase() + { + permissions = FileAccessPermissions.OtherRead, + expectedPermissions = 004 + }; + + var writePermissionsTestCase = new PermissionTestCase() + { + permissions = FileAccessPermissions.OtherWrite, + expectedPermissions = 002 + }; + + var executePermissionsTestCase = new PermissionTestCase() + { + permissions = FileAccessPermissions.OtherExecute, + expectedPermissions = 001 + }; + + var readWritePermissionsTestCase = new PermissionTestCase() + { + permissions = FileAccessPermissions.OtherRead | FileAccessPermissions.OtherWrite, + expectedPermissions = 006 + }; + + var readExecutePermissionsTestCase = new PermissionTestCase() + { + permissions = FileAccessPermissions.OtherRead | FileAccessPermissions.OtherExecute, + expectedPermissions = 005 + }; + + var writeExecutePermissionsTestCase = new PermissionTestCase() + { + permissions = FileAccessPermissions.OtherWrite | FileAccessPermissions.OtherExecute, + expectedPermissions = 003 + }; + + var allPermissionsTestCase = new PermissionTestCase() + { + permissions = FileAccessPermissions.OtherReadWriteExecute, + expectedPermissions = 007 + }; + + return new[] + { + noPermissionTestCase, + readPermissionsTestCase, + writePermissionsTestCase, + executePermissionsTestCase, + readWritePermissionsTestCase, + readExecutePermissionsTestCase, + writeExecutePermissionsTestCase, + allPermissionsTestCase + }; + } + + public class PermissionTestCase + { + internal FileAccessPermissions permissions; + internal int expectedPermissions; + } + } +} diff --git a/Snowflake.Data/Logger/EasyLoggerUtil.cs b/Snowflake.Data/Logger/EasyLoggerUtil.cs index 2ae70b24f..823819c76 100644 --- a/Snowflake.Data/Logger/EasyLoggerUtil.cs +++ b/Snowflake.Data/Logger/EasyLoggerUtil.cs @@ -3,7 +3,6 @@ */ using System; -using System.Diagnostics; using Mono.Unix; namespace Snowflake.Data.Log From f70802100cb314a88bcb0345936aae130cdad950 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Tue, 23 Jan 2024 19:14:32 -0800 Subject: [PATCH 25/45] SNOW-990111: Modify config finder tests --- .../EasyLoggingConfigFinderTest.cs | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs b/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs index 2b32b6bd5..1d0a1162a 100644 --- a/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs @@ -120,7 +120,6 @@ public void TestThatTakesFilePathFromHomeDirectoryWhenNoOtherWaysPossible() } [Test] - [Ignore("TODO: modify the test and remove Ignore")] public void TestThatConfigFileIsNotUsedIfOthersCanModifyTheConfigFile() { if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) @@ -130,6 +129,8 @@ public void TestThatConfigFileIsNotUsedIfOthersCanModifyTheConfigFile() // arrange var configFilePath = CreateConfigTempFile(Config(EasyLoggingLogLevel.Warn.ToString(), InputConfigFilePath)); + var fileInfo = new Mono.Unix.UnixFileInfo(InputConfigFilePath); + fileInfo.Create(Mono.Unix.FileAccessPermissions.AllPermissions); // act var thrown = Assert.Throws(() => t_finder.FindConfigFilePath(configFilePath)); @@ -166,6 +167,20 @@ public void TestThatDoesNotFailWhenSearchForOneOfDirectoriesFails() t_environmentOperations.Verify(e => e.GetFolderPath(Environment.SpecialFolder.UserProfile), Times.Once); } + [Test] + public void TestThatDoesNotFailWhenHomeDirectoryReturnsNull() + { + // arrange + MockHomeDirectoryReturnsNull(); + + // act + var filePath = t_finder.FindConfigFilePath(null); + + // assert + Assert.IsNull(filePath); + t_environmentOperations.Verify(e => e.GetFolderPath(Environment.SpecialFolder.UserProfile), Times.Once); + } + [Test] public void TestThatDoesNotFailWhenHomeDirectoryDoesNotExist() { From 8837fe7c896cc02d544188e8ff6a0a741ba042c3 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Tue, 23 Jan 2024 19:35:34 -0800 Subject: [PATCH 26/45] SNOW-990111: Modify config finder tests --- .../UnitTests/Configuration/EasyLoggingConfigFinderTest.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs b/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs index 1d0a1162a..f99318750 100644 --- a/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs @@ -120,6 +120,7 @@ public void TestThatTakesFilePathFromHomeDirectoryWhenNoOtherWaysPossible() } [Test] + [Ignore("TODO: modify the test and remove Ignore")] public void TestThatConfigFileIsNotUsedIfOthersCanModifyTheConfigFile() { if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) @@ -130,7 +131,7 @@ public void TestThatConfigFileIsNotUsedIfOthersCanModifyTheConfigFile() // arrange var configFilePath = CreateConfigTempFile(Config(EasyLoggingLogLevel.Warn.ToString(), InputConfigFilePath)); var fileInfo = new Mono.Unix.UnixFileInfo(InputConfigFilePath); - fileInfo.Create(Mono.Unix.FileAccessPermissions.AllPermissions); + fileInfo.Create(Mono.Unix.FileAccessPermissions.GroupWrite | Mono.Unix.FileAccessPermissions.OtherWrite); // act var thrown = Assert.Throws(() => t_finder.FindConfigFilePath(configFilePath)); From 16a99799ad35af2d56b174f23b7af92441b381b1 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Tue, 23 Jan 2024 19:48:36 -0800 Subject: [PATCH 27/45] SNOW-990111: Modify config finder tests --- .../UnitTests/Configuration/EasyLoggingConfigFinderTest.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs b/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs index f99318750..f46108545 100644 --- a/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs @@ -120,7 +120,6 @@ public void TestThatTakesFilePathFromHomeDirectoryWhenNoOtherWaysPossible() } [Test] - [Ignore("TODO: modify the test and remove Ignore")] public void TestThatConfigFileIsNotUsedIfOthersCanModifyTheConfigFile() { if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) @@ -130,8 +129,8 @@ public void TestThatConfigFileIsNotUsedIfOthersCanModifyTheConfigFile() // arrange var configFilePath = CreateConfigTempFile(Config(EasyLoggingLogLevel.Warn.ToString(), InputConfigFilePath)); - var fileInfo = new Mono.Unix.UnixFileInfo(InputConfigFilePath); - fileInfo.Create(Mono.Unix.FileAccessPermissions.GroupWrite | Mono.Unix.FileAccessPermissions.OtherWrite); + var fileInfo = new Mono.Unix.UnixFileInfo(configFilePath); + fileInfo.Create(Mono.Unix.FileAccessPermissions.AllPermissions); // act var thrown = Assert.Throws(() => t_finder.FindConfigFilePath(configFilePath)); From 190308d703e9d049efe6496c07b0b59d5c647678 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Tue, 23 Jan 2024 21:00:12 -0800 Subject: [PATCH 28/45] SNOW-990111: Modify config finder tests --- .../UnitTests/Configuration/EasyLoggingConfigFinderTest.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs b/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs index f46108545..86f0f739d 100644 --- a/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs @@ -120,6 +120,7 @@ public void TestThatTakesFilePathFromHomeDirectoryWhenNoOtherWaysPossible() } [Test] + [Ignore("TODO: modify the test and remove Ignore")] public void TestThatConfigFileIsNotUsedIfOthersCanModifyTheConfigFile() { if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) From 49832a809478032f16c7f80c5d8bf12160d0efec Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Tue, 23 Jan 2024 21:46:25 -0800 Subject: [PATCH 29/45] SNOW-990111: Modify config finder tests --- .../EasyLoggingConfigFinderTest.cs | 21 +++++++++++-------- .../Configuration/EasyLoggingConfigFinder.cs | 7 +++---- Snowflake.Data/Core/Tools/FileOperations.cs | 13 ++++++++++++ 3 files changed, 28 insertions(+), 13 deletions(-) diff --git a/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs b/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs index 86f0f739d..c07dab397 100644 --- a/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs @@ -5,6 +5,7 @@ using System; using System.IO; using System.Runtime.InteropServices; +using Mono.Unix; using Moq; using NUnit.Framework; using Snowflake.Data.Configuration; @@ -120,7 +121,6 @@ public void TestThatTakesFilePathFromHomeDirectoryWhenNoOtherWaysPossible() } [Test] - [Ignore("TODO: modify the test and remove Ignore")] public void TestThatConfigFileIsNotUsedIfOthersCanModifyTheConfigFile() { if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) @@ -129,19 +129,15 @@ public void TestThatConfigFileIsNotUsedIfOthersCanModifyTheConfigFile() } // arrange - var configFilePath = CreateConfigTempFile(Config(EasyLoggingLogLevel.Warn.ToString(), InputConfigFilePath)); - var fileInfo = new Mono.Unix.UnixFileInfo(configFilePath); - fileInfo.Create(Mono.Unix.FileAccessPermissions.AllPermissions); + MockFileOnHomePath(); + MockHasFlagReturnsTrue(); // act - var thrown = Assert.Throws(() => t_finder.FindConfigFilePath(configFilePath)); + var thrown = Assert.Throws(() => t_finder.FindConfigFilePath(null)); // assert Assert.IsNotNull(thrown); - Assert.AreEqual(thrown.Message, "Error due to other users having permission to modify the config file"); - - // cleanup - File.Delete(configFilePath); + Assert.AreEqual(thrown.Message, $"Error due to other users having permission to modify the config file: {s_homeConfigFilePath}"); } [Test] @@ -206,6 +202,13 @@ private static void MockDirectoriesExist() .Returns(true); } + private static void MockHasFlagReturnsTrue() + { + t_fileOperations + .Setup(f => f.HasFlag(It.IsAny())) + .Returns(true); + } + private static void MockHomeDirectory() { t_environmentOperations diff --git a/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs b/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs index 1d66e7bf1..06b6a8f2f 100644 --- a/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs +++ b/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs @@ -110,10 +110,9 @@ private void CheckIfValidPermissions(string filePath) 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)) + _fileOperations.SetUnixFileInfo(filePath); + if (_fileOperations.HasFlag(FileAccessPermissions.GroupWrite) || + _fileOperations.HasFlag(FileAccessPermissions.OtherWrite)) { var errorMessage = $"Error due to other users having permission to modify the config file: {filePath}"; s_logger.Error(errorMessage); diff --git a/Snowflake.Data/Core/Tools/FileOperations.cs b/Snowflake.Data/Core/Tools/FileOperations.cs index 9efe481bd..d290087dc 100644 --- a/Snowflake.Data/Core/Tools/FileOperations.cs +++ b/Snowflake.Data/Core/Tools/FileOperations.cs @@ -2,6 +2,7 @@ * Copyright (c) 2023 Snowflake Computing Inc. All rights reserved. */ +using Mono.Unix; using System.IO; namespace Snowflake.Data.Core.Tools @@ -10,9 +11,21 @@ internal class FileOperations { public static readonly FileOperations Instance = new FileOperations(); + private UnixFileInfo _unixFileInfo; + public virtual bool Exists(string path) { return File.Exists(path); } + + public virtual void SetUnixFileInfo(string path) + { + _unixFileInfo = new UnixFileInfo(path); + } + + public virtual bool HasFlag(FileAccessPermissions permissions) + { + return _unixFileInfo.FileAccessPermissions.HasFlag(permissions); + } } } From e17dcf3eb840d28ec66455228df345a6a540fb38 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Tue, 23 Jan 2024 23:01:36 -0800 Subject: [PATCH 30/45] SNOW-990111: Modify config finder tests --- .../EasyLoggingConfigFinderTest.cs | 14 ++++++++--- .../Configuration/EasyLoggingConfigFinder.cs | 12 +++++---- Snowflake.Data/Core/Tools/FileOperations.cs | 13 ---------- .../Core/Tools/UnixFileOperations.cs | 25 +++++++++++++++++++ 4 files changed, 42 insertions(+), 22 deletions(-) create mode 100644 Snowflake.Data/Core/Tools/UnixFileOperations.cs diff --git a/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs b/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs index c07dab397..6f6e32fff 100644 --- a/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs @@ -10,7 +10,6 @@ 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 { @@ -27,6 +26,9 @@ public class EasyLoggingConfigFinderTest [ThreadStatic] private static Mock t_fileOperations; + [ThreadStatic] + private static Mock t_unixFileOperations; + [ThreadStatic] private static Mock t_directoryOperations; @@ -40,9 +42,10 @@ public class EasyLoggingConfigFinderTest public void Setup() { t_fileOperations = new Mock(); + t_unixFileOperations = new Mock(); t_directoryOperations = new Mock(); t_environmentOperations = new Mock(); - t_finder = new EasyLoggingConfigFinder(t_fileOperations.Object, t_directoryOperations.Object, t_environmentOperations.Object); + t_finder = new EasyLoggingConfigFinder(t_fileOperations.Object, t_unixFileOperations.Object, t_directoryOperations.Object, t_environmentOperations.Object); MockDirectoriesExist(); MockHomeDirectory(); } @@ -204,8 +207,11 @@ private static void MockDirectoriesExist() private static void MockHasFlagReturnsTrue() { - t_fileOperations - .Setup(f => f.HasFlag(It.IsAny())) + t_unixFileOperations + .Setup(f => f.HasFlag( + It.Is(p => + p.Equals(FileAccessPermissions.GroupWrite) || + p.Equals(FileAccessPermissions.OtherWrite)))) .Returns(true); } diff --git a/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs b/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs index 06b6a8f2f..4c26568fc 100644 --- a/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs +++ b/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs @@ -19,14 +19,16 @@ internal class EasyLoggingConfigFinder internal const string ClientConfigEnvironmentName = "SF_CLIENT_CONFIG_FILE"; private readonly FileOperations _fileOperations; + private readonly UnixFileOperations _unixFileOperations; private readonly DirectoryOperations _directoryOperations; private readonly EnvironmentOperations _environmentOperations; - public static readonly EasyLoggingConfigFinder Instance = new EasyLoggingConfigFinder(FileOperations.Instance, DirectoryOperations.Instance, EnvironmentOperations.Instance); + public static readonly EasyLoggingConfigFinder Instance = new EasyLoggingConfigFinder(FileOperations.Instance, UnixFileOperations.Instance, DirectoryOperations.Instance, EnvironmentOperations.Instance); - internal EasyLoggingConfigFinder(FileOperations fileOperations, DirectoryOperations directoryOperations, EnvironmentOperations environmentOperations) + internal EasyLoggingConfigFinder(FileOperations fileOperations, UnixFileOperations unixFileOperations, DirectoryOperations directoryOperations, EnvironmentOperations environmentOperations) { _fileOperations = fileOperations; + _unixFileOperations = unixFileOperations; _directoryOperations = directoryOperations; _environmentOperations = environmentOperations; } @@ -110,9 +112,9 @@ private void CheckIfValidPermissions(string filePath) return; // Check if others have permissions to modify the file and fail if so - _fileOperations.SetUnixFileInfo(filePath); - if (_fileOperations.HasFlag(FileAccessPermissions.GroupWrite) || - _fileOperations.HasFlag(FileAccessPermissions.OtherWrite)) + _unixFileOperations.SetUnixFileInfo(filePath); + if (_unixFileOperations.HasFlag(FileAccessPermissions.GroupWrite) || + _unixFileOperations.HasFlag(FileAccessPermissions.OtherWrite)) { var errorMessage = $"Error due to other users having permission to modify the config file: {filePath}"; s_logger.Error(errorMessage); diff --git a/Snowflake.Data/Core/Tools/FileOperations.cs b/Snowflake.Data/Core/Tools/FileOperations.cs index d290087dc..9efe481bd 100644 --- a/Snowflake.Data/Core/Tools/FileOperations.cs +++ b/Snowflake.Data/Core/Tools/FileOperations.cs @@ -2,7 +2,6 @@ * Copyright (c) 2023 Snowflake Computing Inc. All rights reserved. */ -using Mono.Unix; using System.IO; namespace Snowflake.Data.Core.Tools @@ -11,21 +10,9 @@ internal class FileOperations { public static readonly FileOperations Instance = new FileOperations(); - private UnixFileInfo _unixFileInfo; - public virtual bool Exists(string path) { return File.Exists(path); } - - public virtual void SetUnixFileInfo(string path) - { - _unixFileInfo = new UnixFileInfo(path); - } - - public virtual bool HasFlag(FileAccessPermissions permissions) - { - return _unixFileInfo.FileAccessPermissions.HasFlag(permissions); - } } } diff --git a/Snowflake.Data/Core/Tools/UnixFileOperations.cs b/Snowflake.Data/Core/Tools/UnixFileOperations.cs new file mode 100644 index 000000000..1220a460b --- /dev/null +++ b/Snowflake.Data/Core/Tools/UnixFileOperations.cs @@ -0,0 +1,25 @@ +/* + * Copyright (c) 2024 Snowflake Computing Inc. All rights reserved. + */ + +using Mono.Unix; + +namespace Snowflake.Data.Core.Tools +{ + internal class UnixFileOperations + { + public static readonly UnixFileOperations Instance = new UnixFileOperations(); + + private UnixFileInfo _unixFileInfo; + + public virtual void SetUnixFileInfo(string path) + { + _unixFileInfo = new UnixFileInfo(path); + } + + public virtual bool HasFlag(FileAccessPermissions permissions) + { + return _unixFileInfo.FileAccessPermissions.HasFlag(permissions); + } + } +} From ea8d52e2905ec2857f98cecf78a6e81dd08730bd Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Wed, 24 Jan 2024 11:51:06 -0800 Subject: [PATCH 31/45] SNOW-990111: Modify config finder tests --- .../UnitTests/Configuration/EasyLoggingConfigFinderTest.cs | 4 +--- Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs | 3 +-- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs b/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs index 6f6e32fff..7d1915ead 100644 --- a/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs @@ -209,9 +209,7 @@ private static void MockHasFlagReturnsTrue() { t_unixFileOperations .Setup(f => f.HasFlag( - It.Is(p => - p.Equals(FileAccessPermissions.GroupWrite) || - p.Equals(FileAccessPermissions.OtherWrite)))) + It.Is(p => p.Equals(FileAccessPermissions.GroupWrite | FileAccessPermissions.OtherWrite)))) .Returns(true); } diff --git a/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs b/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs index 4c26568fc..aa2072e96 100644 --- a/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs +++ b/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs @@ -113,8 +113,7 @@ private void CheckIfValidPermissions(string filePath) // Check if others have permissions to modify the file and fail if so _unixFileOperations.SetUnixFileInfo(filePath); - if (_unixFileOperations.HasFlag(FileAccessPermissions.GroupWrite) || - _unixFileOperations.HasFlag(FileAccessPermissions.OtherWrite)) + if (_unixFileOperations.HasFlag(FileAccessPermissions.GroupWrite | FileAccessPermissions.OtherWrite)) { var errorMessage = $"Error due to other users having permission to modify the config file: {filePath}"; s_logger.Error(errorMessage); From 3fabd055540a3793066f744f6334873e8a2866f8 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Thu, 25 Jan 2024 14:24:24 -0800 Subject: [PATCH 32/45] SNOW-990111: Remove umask test and create dotnet subfolder with Mono.Unix --- .../EasyLoggingConfigFinderTest.cs | 10 +-- .../Session/EasyLoggingStarterTest.cs | 89 ++++++++----------- .../Configuration/EasyLoggingConfigFinder.cs | 12 +-- .../Core/Session/EasyLoggingStarter.cs | 29 ++++-- .../Core/Tools/UnixFileOperations.cs | 25 ------ Snowflake.Data/Core/Tools/UnixOperations.cs | 48 ++++++++++ 6 files changed, 119 insertions(+), 94 deletions(-) delete mode 100644 Snowflake.Data/Core/Tools/UnixFileOperations.cs create mode 100644 Snowflake.Data/Core/Tools/UnixOperations.cs diff --git a/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs b/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs index 7d1915ead..a188682e9 100644 --- a/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs @@ -27,7 +27,7 @@ public class EasyLoggingConfigFinderTest private static Mock t_fileOperations; [ThreadStatic] - private static Mock t_unixFileOperations; + private static Mock t_unixOperations; [ThreadStatic] private static Mock t_directoryOperations; @@ -42,10 +42,10 @@ public class EasyLoggingConfigFinderTest public void Setup() { t_fileOperations = new Mock(); - t_unixFileOperations = new Mock(); + t_unixOperations = new Mock(); t_directoryOperations = new Mock(); t_environmentOperations = new Mock(); - t_finder = new EasyLoggingConfigFinder(t_fileOperations.Object, t_unixFileOperations.Object, t_directoryOperations.Object, t_environmentOperations.Object); + t_finder = new EasyLoggingConfigFinder(t_fileOperations.Object, t_unixOperations.Object, t_directoryOperations.Object, t_environmentOperations.Object); MockDirectoriesExist(); MockHomeDirectory(); } @@ -207,8 +207,8 @@ private static void MockDirectoriesExist() private static void MockHasFlagReturnsTrue() { - t_unixFileOperations - .Setup(f => f.HasFlag( + t_unixOperations + .Setup(f => f.CheckFileHasPermissions( It.Is(p => p.Equals(FileAccessPermissions.GroupWrite | FileAccessPermissions.OtherWrite)))) .Returns(true); } diff --git a/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs b/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs index 162d90639..9fb6936ac 100644 --- a/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs @@ -3,10 +3,10 @@ */ using System; -using System.Diagnostics; using System.IO; using System.Runtime.InteropServices; using Mono.Unix; +using Mono.Unix.Native; using Moq; using NUnit.Framework; using Snowflake.Data.Configuration; @@ -19,7 +19,6 @@ namespace Snowflake.Data.Tests.UnitTests.Session [TestFixture] public class EasyLoggingStarterTest { - private static readonly int AllPermissions = 777; 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"; @@ -58,6 +57,9 @@ public class EasyLoggingStarterTest [ThreadStatic] private static Mock t_easyLoggerManager; + [ThreadStatic] + private static Mock t_unixOperations; + [ThreadStatic] private static Mock t_directoryOperations; @@ -72,43 +74,15 @@ public void BeforeEach() { t_easyLoggingProvider = new Mock(); t_easyLoggerManager = new Mock(); + t_unixOperations = new Mock(); t_directoryOperations = new Mock(); t_environmentOperations = new Mock(); - 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)) - { - Assert.Ignore("skip test on 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)); - - var expectedDirPermissions = AllPermissions - int.Parse(CallBash("umask")); - - // act - t_easyLoggerStarter.Init(ConfigPath); - var dirInfo = new UnixDirectoryInfo(s_expectedLogPath); - var dirPermissions = EasyLoggerUtil.ConvertFileAccessPermissionsToInt(dirInfo.FileAccessPermissions); - - // assert - Assert.AreEqual(expectedDirPermissions, dirPermissions); - - // cleanup - Directory.Delete(s_expectedLogPath); + t_easyLoggerStarter = new EasyLoggingStarter( + t_easyLoggingProvider.Object, + t_easyLoggerManager.Object, + t_unixOperations.Object, + t_directoryOperations.Object, + t_environmentOperations.Object); } [Test] @@ -150,6 +124,34 @@ public void TestThatThrowsErrorWhenLogPathIsNotSetAndHomeDirectoryThrowsAnExcept Assert.AreEqual(thrown.Message, $"Error while trying to retrieve the home directory: {ex}"); } + [Test] + public void TestThatDoesNotFailWhenDirectoryPermissionIsNot700() + { + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + Assert.Ignore("skip test on Windows"); + } + + // arrange + t_easyLoggingProvider + .Setup(provider => provider.ProvideConfig(ConfigPath)) + .Returns(s_configWithInfoLevel); + t_directoryOperations + .Setup(provider => provider.Exists(ConfigPath)) + .Returns(false); + t_unixOperations + .Setup(provider => provider.GetDirPermissions()) + .Returns(FileAccessPermissions.AllPermissions); + + // act + t_easyLoggerStarter.Init(ConfigPath); + + // assert + t_unixOperations.Verify(u => u.CreateDirectoryWithPermissions(s_expectedLogPath, + FilePermissions.S_IRUSR | FilePermissions.S_IWUSR | FilePermissions.S_IXUSR), Times.Once); + t_unixOperations.Verify(u => u.SetDirInfo(s_expectedLogPath), Times.Once); + } + [Test] public void TestThatConfiguresEasyLoggingOnlyOnceWhenInitializedWithConfigPath() { @@ -222,18 +224,5 @@ public void TestThatReconfiguresEasyLoggingWithConfigPathIfNotGivenForTheFirstTi t_easyLoggerManager.Verify(manager => manager.ReconfigureEasyLogging(EasyLoggingLogLevel.Info, s_expectedLogPath), Times.Once); t_easyLoggerManager.VerifyNoOtherCalls(); } - - private static string CallBash(string command) - { - using (Process process = new Process()) - { - process.StartInfo.FileName = "/bin/bash"; - process.StartInfo.Arguments = $"-c \"{command}\""; - process.StartInfo.UseShellExecute = false; - process.StartInfo.RedirectStandardOutput = true; - process.Start(); - return process.StandardOutput.ReadToEnd().Replace("\n", string.Empty); - } - } } } diff --git a/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs b/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs index aa2072e96..402df0716 100644 --- a/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs +++ b/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs @@ -19,16 +19,16 @@ internal class EasyLoggingConfigFinder internal const string ClientConfigEnvironmentName = "SF_CLIENT_CONFIG_FILE"; private readonly FileOperations _fileOperations; - private readonly UnixFileOperations _unixFileOperations; + private readonly UnixOperations _unixOperations; private readonly DirectoryOperations _directoryOperations; private readonly EnvironmentOperations _environmentOperations; - public static readonly EasyLoggingConfigFinder Instance = new EasyLoggingConfigFinder(FileOperations.Instance, UnixFileOperations.Instance, DirectoryOperations.Instance, EnvironmentOperations.Instance); + public static readonly EasyLoggingConfigFinder Instance = new EasyLoggingConfigFinder(FileOperations.Instance, UnixOperations.Instance, DirectoryOperations.Instance, EnvironmentOperations.Instance); - internal EasyLoggingConfigFinder(FileOperations fileOperations, UnixFileOperations unixFileOperations, DirectoryOperations directoryOperations, EnvironmentOperations environmentOperations) + internal EasyLoggingConfigFinder(FileOperations fileOperations, UnixOperations unixFileOperations, DirectoryOperations directoryOperations, EnvironmentOperations environmentOperations) { _fileOperations = fileOperations; - _unixFileOperations = unixFileOperations; + _unixOperations = unixFileOperations; _directoryOperations = directoryOperations; _environmentOperations = environmentOperations; } @@ -112,8 +112,8 @@ private void CheckIfValidPermissions(string filePath) return; // Check if others have permissions to modify the file and fail if so - _unixFileOperations.SetUnixFileInfo(filePath); - if (_unixFileOperations.HasFlag(FileAccessPermissions.GroupWrite | FileAccessPermissions.OtherWrite)) + _unixOperations.SetFileInfo(filePath); + if (_unixOperations.CheckFileHasPermissions(FileAccessPermissions.GroupWrite | FileAccessPermissions.OtherWrite)) { var errorMessage = $"Error due to other users having permission to modify the config file: {filePath}"; s_logger.Error(errorMessage); diff --git a/Snowflake.Data/Core/Session/EasyLoggingStarter.cs b/Snowflake.Data/Core/Session/EasyLoggingStarter.cs index b6950e511..e325bde90 100644 --- a/Snowflake.Data/Core/Session/EasyLoggingStarter.cs +++ b/Snowflake.Data/Core/Session/EasyLoggingStarter.cs @@ -6,6 +6,7 @@ using System.IO; using System.Runtime.InteropServices; using Mono.Unix; +using Mono.Unix.Native; using Snowflake.Data.Configuration; using Snowflake.Data.Core.Tools; using Snowflake.Data.Log; @@ -20,6 +21,8 @@ internal class EasyLoggingStarter private readonly EasyLoggerManager _easyLoggerManager; + private readonly UnixOperations _unixOperations; + private readonly DirectoryOperations _directoryOperations; private readonly EnvironmentOperations _environmentOperations; @@ -29,16 +32,18 @@ internal class EasyLoggingStarter private EasyLoggingInitTrialParameters _initTrialParameters = null; public static readonly EasyLoggingStarter Instance = new EasyLoggingStarter(EasyLoggingConfigProvider.Instance, - EasyLoggerManager.Instance, DirectoryOperations.Instance, EnvironmentOperations.Instance); + EasyLoggerManager.Instance, UnixOperations.Instance, DirectoryOperations.Instance, EnvironmentOperations.Instance); internal EasyLoggingStarter( EasyLoggingConfigProvider easyLoggingConfigProvider, EasyLoggerManager easyLoggerManager, + UnixOperations unixOperations, DirectoryOperations directoryOperations, EnvironmentOperations environmentOperations) { _easyLoggingConfigProvider = easyLoggingConfigProvider; _easyLoggerManager = easyLoggerManager; + _unixOperations = unixOperations; _directoryOperations = directoryOperations; _environmentOperations = environmentOperations; } @@ -127,9 +132,16 @@ private string GetLogPath(string logPath) var pathWithDotnetSubdirectory = Path.Combine(logPathOrDefault, "dotnet"); if (!_directoryOperations.Exists(pathWithDotnetSubdirectory)) { - _directoryOperations.CreateDirectory(pathWithDotnetSubdirectory); - - CheckDirPermissionsOnlyAllowUser(pathWithDotnetSubdirectory); + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + _directoryOperations.CreateDirectory(pathWithDotnetSubdirectory); + } + else + { + _unixOperations.CreateDirectoryWithPermissions(pathWithDotnetSubdirectory, + FilePermissions.S_IRUSR | FilePermissions.S_IWUSR | FilePermissions.S_IXUSR); + CheckDirPermissionsOnlyAllowUser(pathWithDotnetSubdirectory); + } } return pathWithDotnetSubdirectory; @@ -140,11 +152,12 @@ private void CheckDirPermissionsOnlyAllowUser(string dirPath) if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) return; - var dirInfo = new UnixDirectoryInfo(dirPath); - if (dirInfo.Exists && dirInfo.FileAccessPermissions != FileAccessPermissions.UserReadWriteExecute) + _unixOperations.SetDirInfo(dirPath); + var dirPermissions = _unixOperations.GetDirPermissions(); + if (dirPermissions != FileAccessPermissions.UserReadWriteExecute) { - var dirPermissions = EasyLoggerUtil.ConvertFileAccessPermissionsToInt(dirInfo.FileAccessPermissions); - s_logger.Warn($"Access permission for the logs directory is currently {dirPermissions}"); + s_logger.Warn($"Access permission for the logs directory is currently " + + $"{EasyLoggerUtil.ConvertFileAccessPermissionsToInt(dirPermissions)}"); } } } diff --git a/Snowflake.Data/Core/Tools/UnixFileOperations.cs b/Snowflake.Data/Core/Tools/UnixFileOperations.cs deleted file mode 100644 index 1220a460b..000000000 --- a/Snowflake.Data/Core/Tools/UnixFileOperations.cs +++ /dev/null @@ -1,25 +0,0 @@ -/* - * Copyright (c) 2024 Snowflake Computing Inc. All rights reserved. - */ - -using Mono.Unix; - -namespace Snowflake.Data.Core.Tools -{ - internal class UnixFileOperations - { - public static readonly UnixFileOperations Instance = new UnixFileOperations(); - - private UnixFileInfo _unixFileInfo; - - public virtual void SetUnixFileInfo(string path) - { - _unixFileInfo = new UnixFileInfo(path); - } - - public virtual bool HasFlag(FileAccessPermissions permissions) - { - return _unixFileInfo.FileAccessPermissions.HasFlag(permissions); - } - } -} diff --git a/Snowflake.Data/Core/Tools/UnixOperations.cs b/Snowflake.Data/Core/Tools/UnixOperations.cs new file mode 100644 index 000000000..dd044d180 --- /dev/null +++ b/Snowflake.Data/Core/Tools/UnixOperations.cs @@ -0,0 +1,48 @@ +/* + * Copyright (c) 2024 Snowflake Computing Inc. All rights reserved. + */ + +using Mono.Unix; +using Mono.Unix.Native; +using System.IO; + +namespace Snowflake.Data.Core.Tools +{ + internal class UnixOperations + { + public static readonly UnixOperations Instance = new UnixOperations(); + + private UnixFileInfo _unixFileInfo; + private UnixDirectoryInfo _unixDirInfo; + + public virtual void SetDirInfo(string path) + { + _unixDirInfo = new UnixDirectoryInfo(path); + } + + public virtual void CreateDirectoryWithPermissions(string path, FilePermissions permissions) + { + string subPath = Path.GetDirectoryName(path); + if (!Directory.Exists(subPath)) + { + Directory.CreateDirectory(subPath); + } + Syscall.mkdir(path, permissions); + } + + public virtual FileAccessPermissions GetDirPermissions() + { + return _unixDirInfo.FileAccessPermissions; + } + + public virtual void SetFileInfo(string path) + { + _unixFileInfo = new UnixFileInfo(path); + } + + public virtual bool CheckFileHasPermissions(FileAccessPermissions permissions) + { + return _unixFileInfo.FileAccessPermissions.HasFlag(permissions); + } + } +} From 5635772a3f17870fbae5378b97b13c1f4c86e80e Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Thu, 25 Jan 2024 14:43:00 -0800 Subject: [PATCH 33/45] SNOW-990111: Rename EasyLoggingUtil and edit EasyLoggerManagerTest --- .../UnitTests/Logger/EasyLoggerManagerTest.cs | 6 +++--- ...ggerUtilTest.cs => UnixFilePermissionsConverterTest.cs} | 7 +++---- Snowflake.Data/Core/Session/EasyLoggingStarter.cs | 2 +- ...{EasyLoggerUtil.cs => UnixFilePermissionsConverter .cs} | 2 +- 4 files changed, 8 insertions(+), 9 deletions(-) rename Snowflake.Data.Tests/UnitTests/Logger/{EasyLoggerUtilTest.cs => UnixFilePermissionsConverterTest.cs} (97%) rename Snowflake.Data/Logger/{EasyLoggerUtil.cs => UnixFilePermissionsConverter .cs} (97%) diff --git a/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerManagerTest.cs b/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerManagerTest.cs index 34e2d4f96..4dfba8c83 100644 --- a/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerManagerTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerManagerTest.cs @@ -109,13 +109,13 @@ public void TestThatOnlyUnknownFieldsAreLogged() var configFilePath = Guid.NewGuid().ToString() + ".json"; using (var writer = File.CreateText(configFilePath)) { - writer.Write(ConfigWithUnknownFields.Replace("\\", "\\\\")); + writer.Write(ConfigWithUnknownFields); } EasyLoggerManager.Instance.ReconfigureEasyLogging(EasyLoggingLogLevel.Warn, t_directoryLogPath); + var parser = new EasyLoggingConfigParser(); // act - var parser = new EasyLoggingConfigParser(); - var config = parser.Parse(configFilePath); + parser.Parse(configFilePath); // assert var logLines = File.ReadLines(FindLogFilePath(t_directoryLogPath)); diff --git a/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerUtilTest.cs b/Snowflake.Data.Tests/UnitTests/Logger/UnixFilePermissionsConverterTest.cs similarity index 97% rename from Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerUtilTest.cs rename to Snowflake.Data.Tests/UnitTests/Logger/UnixFilePermissionsConverterTest.cs index debd7e244..12f4ec756 100644 --- a/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerUtilTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Logger/UnixFilePermissionsConverterTest.cs @@ -1,17 +1,16 @@ /* - * Copyright (c) 2023 Snowflake Computing Inc. All rights reserved. + * Copyright (c) 2024 Snowflake Computing Inc. All rights reserved. */ using Mono.Unix; using NUnit.Framework; using Snowflake.Data.Log; -using System; using System.Collections.Generic; namespace Snowflake.Data.Tests.UnitTests.Logger { [TestFixture] - public class EasyLoggerUtilTest + public class UnixFilePermissionsConverterTest { [Test] public void TestConversionForAllPermissionCombinations( @@ -24,7 +23,7 @@ public void TestConversionForAllPermissionCombinations( var expectedPermissions = userTestCase.expectedPermissions + groupTestCase.expectedPermissions + otherTestCase.expectedPermissions; // act - var convertedPermissions = EasyLoggerUtil.ConvertFileAccessPermissionsToInt(permissions); + var convertedPermissions = UnixFilePermissionsConverter.ConvertFileAccessPermissionsToInt(permissions); // assert Assert.AreEqual(expectedPermissions, convertedPermissions); diff --git a/Snowflake.Data/Core/Session/EasyLoggingStarter.cs b/Snowflake.Data/Core/Session/EasyLoggingStarter.cs index e325bde90..9caeeb473 100644 --- a/Snowflake.Data/Core/Session/EasyLoggingStarter.cs +++ b/Snowflake.Data/Core/Session/EasyLoggingStarter.cs @@ -157,7 +157,7 @@ private void CheckDirPermissionsOnlyAllowUser(string dirPath) if (dirPermissions != FileAccessPermissions.UserReadWriteExecute) { s_logger.Warn($"Access permission for the logs directory is currently " + - $"{EasyLoggerUtil.ConvertFileAccessPermissionsToInt(dirPermissions)}"); + $"{UnixFilePermissionsConverter.ConvertFileAccessPermissionsToInt(dirPermissions)}"); } } } diff --git a/Snowflake.Data/Logger/EasyLoggerUtil.cs b/Snowflake.Data/Logger/UnixFilePermissionsConverter .cs similarity index 97% rename from Snowflake.Data/Logger/EasyLoggerUtil.cs rename to Snowflake.Data/Logger/UnixFilePermissionsConverter .cs index 823819c76..d7278164f 100644 --- a/Snowflake.Data/Logger/EasyLoggerUtil.cs +++ b/Snowflake.Data/Logger/UnixFilePermissionsConverter .cs @@ -7,7 +7,7 @@ namespace Snowflake.Data.Log { - internal class EasyLoggerUtil + internal class UnixFilePermissionsConverter { internal static int ConvertFileAccessPermissionsToInt(FileAccessPermissions permissions) { From 31fefeb6062de307a3a5c608fe595d5b79e93539 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Thu, 25 Jan 2024 14:43:52 -0800 Subject: [PATCH 34/45] SNOW-990111: Update EasyLoggingStarterTest --- .../Session/EasyLoggingStarterTest.cs | 30 +++++++++++++++++-- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs b/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs index 9fb6936ac..82b12a14a 100644 --- a/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs @@ -170,7 +170,15 @@ public void TestThatConfiguresEasyLoggingOnlyOnceWhenInitializedWithConfigPath() t_easyLoggerStarter.Init(ConfigPath); // assert - t_directoryOperations.Verify(d => d.CreateDirectory(s_expectedLogPath), Times.Once); + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + t_directoryOperations.Verify(d => d.CreateDirectory(s_expectedLogPath), Times.Once); + } + else + { + t_unixOperations.Verify(u => u.CreateDirectoryWithPermissions(s_expectedLogPath, + FilePermissions.S_IRUSR | FilePermissions.S_IWUSR | FilePermissions.S_IXUSR), Times.Once); + } t_easyLoggerManager.Verify(manager => manager.ReconfigureEasyLogging(EasyLoggingLogLevel.Error, s_expectedLogPath), Times.Once); // act @@ -195,7 +203,15 @@ public void TestThatConfiguresEasyLoggingOnlyOnceForInitializationsWithoutConfig t_easyLoggerStarter.Init(null); // assert - t_directoryOperations.Verify(d => d.CreateDirectory(s_expectedLogPath), Times.Once); + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + t_directoryOperations.Verify(d => d.CreateDirectory(s_expectedLogPath), Times.Once); + } + else + { + t_unixOperations.Verify(u => u.CreateDirectoryWithPermissions(s_expectedLogPath, + FilePermissions.S_IRUSR | FilePermissions.S_IWUSR | FilePermissions.S_IXUSR), Times.Once); + } t_easyLoggerManager.Verify(manager => manager.ReconfigureEasyLogging(EasyLoggingLogLevel.Error, s_expectedLogPath), Times.Once); } @@ -214,7 +230,15 @@ public void TestThatReconfiguresEasyLoggingWithConfigPathIfNotGivenForTheFirstTi t_easyLoggerStarter.Init(null); // assert - t_directoryOperations.Verify(d => d.CreateDirectory(s_expectedLogPath), Times.Once); + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + t_directoryOperations.Verify(d => d.CreateDirectory(s_expectedLogPath), Times.Once); + } + else + { + t_unixOperations.Verify(u => u.CreateDirectoryWithPermissions(s_expectedLogPath, + FilePermissions.S_IRUSR | FilePermissions.S_IWUSR | FilePermissions.S_IXUSR), Times.Once); + } t_easyLoggerManager.Verify(manager => manager.ReconfigureEasyLogging(EasyLoggingLogLevel.Error, s_expectedLogPath), Times.Once); // act From bb70c74bf9ca62884990e3b53b14b695d8a9e8d9 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Thu, 25 Jan 2024 15:52:46 -0800 Subject: [PATCH 35/45] SNOW-990111: Refactor tests --- .../Configuration/EasyLoggingConfigFinderTest.cs | 1 + .../UnitTests/Session/EasyLoggingStarterTest.cs | 8 ++++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs b/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs index a188682e9..bcbef3e97 100644 --- a/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs @@ -141,6 +141,7 @@ public void TestThatConfigFileIsNotUsedIfOthersCanModifyTheConfigFile() // assert Assert.IsNotNull(thrown); Assert.AreEqual(thrown.Message, $"Error due to other users having permission to modify the config file: {s_homeConfigFilePath}"); + t_unixOperations.Verify(u => u.SetFileInfo(s_homeConfigFilePath), Times.Once); } [Test] diff --git a/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs b/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs index 82b12a14a..b42e2f59a 100644 --- a/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs @@ -93,7 +93,7 @@ public void TestThatThrowsErrorWhenLogPathAndHomeDirectoryIsNotSet() .Setup(provider => provider.ProvideConfig(ConfigPath)) .Returns(s_configWithNoLogPath); t_environmentOperations - .Setup(provider => provider.GetFolderPath(Environment.SpecialFolder.UserProfile)) + .Setup(env => env.GetFolderPath(Environment.SpecialFolder.UserProfile)) .Returns(""); // act @@ -113,7 +113,7 @@ public void TestThatThrowsErrorWhenLogPathIsNotSetAndHomeDirectoryThrowsAnExcept .Setup(provider => provider.ProvideConfig(ConfigPath)) .Returns(s_configWithNoLogPath); t_environmentOperations - .Setup(provider => provider.GetFolderPath(Environment.SpecialFolder.UserProfile)) + .Setup(env => env.GetFolderPath(Environment.SpecialFolder.UserProfile)) .Throws(() => ex); // act @@ -137,10 +137,10 @@ public void TestThatDoesNotFailWhenDirectoryPermissionIsNot700() .Setup(provider => provider.ProvideConfig(ConfigPath)) .Returns(s_configWithInfoLevel); t_directoryOperations - .Setup(provider => provider.Exists(ConfigPath)) + .Setup(dir => dir.Exists(ConfigPath)) .Returns(false); t_unixOperations - .Setup(provider => provider.GetDirPermissions()) + .Setup(unix => unix.GetDirPermissions()) .Returns(FileAccessPermissions.AllPermissions); // act From 9f2c87420f70b70e007148a06957219f219ba3b9 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Fri, 26 Jan 2024 09:39:38 -0800 Subject: [PATCH 36/45] SNOW-990111: Refactor hash set to list --- .../Configuration/EasyLoggingConfigParser.cs | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/Snowflake.Data/Configuration/EasyLoggingConfigParser.cs b/Snowflake.Data/Configuration/EasyLoggingConfigParser.cs index 7b59afa21..27fa3a88e 100644 --- a/Snowflake.Data/Configuration/EasyLoggingConfigParser.cs +++ b/Snowflake.Data/Configuration/EasyLoggingConfigParser.cs @@ -70,18 +70,10 @@ private void Validate(ClientConfig config) private void CheckForUnknownFields(string fileContent) { // Parse the specified config file and get the key-value pairs from the "common" section - HashSet knownProperties = new HashSet(); -#if NET471 - foreach (var property in typeof(ClientConfigCommonProps).GetProperties()) - { - var jsonPropertyAttribute = property.GetCustomAttribute(); - knownProperties.Add(jsonPropertyAttribute.PropertyName); - } -#else + List knownProperties = new List(); knownProperties = typeof(ClientConfigCommonProps).GetProperties() .Select(property => property.GetCustomAttribute().PropertyName) - .ToHashSet(); -#endif + .ToList(); JObject.Parse(fileContent).GetValue("common", StringComparison.OrdinalIgnoreCase)? .Cast() From 16335abb7254eb1e156b8541802df2bfba91b516 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Fri, 26 Jan 2024 10:42:50 -0800 Subject: [PATCH 37/45] SNOW-990111: Modify check to allow permissions without user write --- .../UnitTests/Session/EasyLoggingStarterTest.cs | 2 +- Snowflake.Data/Core/Session/EasyLoggingStarter.cs | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs b/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs index b42e2f59a..691e7322b 100644 --- a/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs @@ -125,7 +125,7 @@ public void TestThatThrowsErrorWhenLogPathIsNotSetAndHomeDirectoryThrowsAnExcept } [Test] - public void TestThatDoesNotFailWhenDirectoryPermissionIsNot700() + public void TestThatDoesNotFailWhenDirectoryPermissionIsNot600Or700() { if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { diff --git a/Snowflake.Data/Core/Session/EasyLoggingStarter.cs b/Snowflake.Data/Core/Session/EasyLoggingStarter.cs index 9caeeb473..d8a7a013c 100644 --- a/Snowflake.Data/Core/Session/EasyLoggingStarter.cs +++ b/Snowflake.Data/Core/Session/EasyLoggingStarter.cs @@ -154,7 +154,8 @@ private void CheckDirPermissionsOnlyAllowUser(string dirPath) _unixOperations.SetDirInfo(dirPath); var dirPermissions = _unixOperations.GetDirPermissions(); - if (dirPermissions != FileAccessPermissions.UserReadWriteExecute) + if (dirPermissions != FileAccessPermissions.UserReadWriteExecute && + dirPermissions != (FileAccessPermissions.UserRead | FileAccessPermissions.UserExecute)) { s_logger.Warn($"Access permission for the logs directory is currently " + $"{UnixFilePermissionsConverter.ConvertFileAccessPermissionsToInt(dirPermissions)}"); From 0f266fc7ded4de0b45e585eff1c1125e39f7f550 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Fri, 26 Jan 2024 14:29:57 -0800 Subject: [PATCH 38/45] SNOW-990111: Refactor getting home directory --- .../Session/EasyLoggingStarterTest.cs | 4 +-- .../Configuration/EasyLoggingConfigFinder.cs | 2 +- .../Core/Session/EasyLoggingStarter.cs | 15 ++------- .../Core/Tools/HomeDirectoryProvider.cs | 31 +++++++++++++++++++ 4 files changed, 37 insertions(+), 15 deletions(-) create mode 100644 Snowflake.Data/Core/Tools/HomeDirectoryProvider.cs diff --git a/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs b/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs index 691e7322b..30aef1ede 100644 --- a/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs @@ -101,7 +101,7 @@ public void TestThatThrowsErrorWhenLogPathAndHomeDirectoryIsNotSet() // assert Assert.IsNotNull(thrown); - Assert.AreEqual(thrown.InnerException.Message, "No log path found for easy logging. Home directory is not configured and log path is not provided"); + Assert.AreEqual(thrown.Message, "No log path found for easy logging. Home directory is not configured and log path is not provided"); } [Test] @@ -121,7 +121,7 @@ public void TestThatThrowsErrorWhenLogPathIsNotSetAndHomeDirectoryThrowsAnExcept // assert Assert.IsNotNull(thrown); - Assert.AreEqual(thrown.Message, $"Error while trying to retrieve the home directory: {ex}"); + Assert.AreEqual(thrown.Message, "No log path found for easy logging. Home directory is not configured and log path is not provided"); } [Test] diff --git a/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs b/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs index 402df0716..8bfdb77b6 100644 --- a/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs +++ b/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs @@ -68,7 +68,7 @@ private string GetFilePathFromInputParameter(string filePath, string inputDescri return filePath; } - private string GetHomeDirectory() =>_environmentOperations.GetFolderPath(Environment.SpecialFolder.UserProfile); + private string GetHomeDirectory() => HomeDirectoryProvider.HomeDirectory(_environmentOperations); private string GetFilePathFromDriverLocation() => SearchForConfigInDirectory(() => ".", "driver"); diff --git a/Snowflake.Data/Core/Session/EasyLoggingStarter.cs b/Snowflake.Data/Core/Session/EasyLoggingStarter.cs index d8a7a013c..bd0a6eac0 100644 --- a/Snowflake.Data/Core/Session/EasyLoggingStarter.cs +++ b/Snowflake.Data/Core/Session/EasyLoggingStarter.cs @@ -114,19 +114,10 @@ private string GetLogPath(string logPath) if (string.IsNullOrEmpty(logPath)) { s_logger.Warn("LogPath in client config not found. Using home directory as a default value"); - try + logPathOrDefault = HomeDirectoryProvider.HomeDirectory(_environmentOperations); + if (string.IsNullOrEmpty(logPathOrDefault)) { - logPathOrDefault = _environmentOperations.GetFolderPath(Environment.SpecialFolder.UserProfile); - if (string.IsNullOrEmpty(logPathOrDefault)) - { - throw new Exception("No log path found for easy logging. Home directory is not configured and log path is not provided"); - } - } - catch(Exception e) - { - var errorMessage = $"Error while trying to retrieve the home directory: {e}"; - s_logger.Error(errorMessage); - throw new Exception(errorMessage, e); + throw new Exception("No log path found for easy logging. Home directory is not configured and log path is not provided"); } } var pathWithDotnetSubdirectory = Path.Combine(logPathOrDefault, "dotnet"); diff --git a/Snowflake.Data/Core/Tools/HomeDirectoryProvider.cs b/Snowflake.Data/Core/Tools/HomeDirectoryProvider.cs new file mode 100644 index 000000000..a39a0987c --- /dev/null +++ b/Snowflake.Data/Core/Tools/HomeDirectoryProvider.cs @@ -0,0 +1,31 @@ +/* + * Copyright (c) 2024 Snowflake Computing Inc. All rights reserved. + */ + +using Snowflake.Data.Log; +using System; + +namespace Snowflake.Data.Core.Tools +{ + internal class HomeDirectoryProvider + { + private static readonly SFLogger s_logger = SFLoggerFactory.GetLogger(); + + public static string HomeDirectory(EnvironmentOperations _environmentOperations) { + try + { + var homeDirectory = _environmentOperations.GetFolderPath(Environment.SpecialFolder.UserProfile); + if (string.IsNullOrEmpty(homeDirectory) || homeDirectory.Equals("/")) + { + return null; + } + return homeDirectory; + } + catch (Exception e) + { + s_logger.Error($"Error while trying to retrieve the home directory: {e}"); + return null; + } + } + } +} From a5be03fc657ae5fe6fd42cc5a0e1963e3d96aaea Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Tue, 30 Jan 2024 11:10:05 -0800 Subject: [PATCH 39/45] Refactor UnixOperations and clean up --- .../EasyLoggingConfigFinderTest.cs | 3 +- .../Session/EasyLoggingStarterTest.cs | 3 +- .../Configuration/EasyLoggingConfigParser.cs | 3 +- .../Core/Session/EasyLoggingStarter.cs | 7 +++-- Snowflake.Data/Core/Tools/UnixOperations.cs | 29 ++++--------------- 5 files changed, 14 insertions(+), 31 deletions(-) diff --git a/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs b/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs index bcbef3e97..7d1de4930 100644 --- a/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs @@ -141,7 +141,6 @@ public void TestThatConfigFileIsNotUsedIfOthersCanModifyTheConfigFile() // assert Assert.IsNotNull(thrown); Assert.AreEqual(thrown.Message, $"Error due to other users having permission to modify the config file: {s_homeConfigFilePath}"); - t_unixOperations.Verify(u => u.SetFileInfo(s_homeConfigFilePath), Times.Once); } [Test] @@ -209,7 +208,7 @@ private static void MockDirectoriesExist() private static void MockHasFlagReturnsTrue() { t_unixOperations - .Setup(f => f.CheckFileHasPermissions( + .Setup(f => f.CheckFileHasPermissions(s_homeConfigFilePath, It.Is(p => p.Equals(FileAccessPermissions.GroupWrite | FileAccessPermissions.OtherWrite)))) .Returns(true); } diff --git a/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs b/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs index 30aef1ede..a0301e835 100644 --- a/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs @@ -140,7 +140,7 @@ public void TestThatDoesNotFailWhenDirectoryPermissionIsNot600Or700() .Setup(dir => dir.Exists(ConfigPath)) .Returns(false); t_unixOperations - .Setup(unix => unix.GetDirPermissions()) + .Setup(unix => unix.GetDirPermissions(s_expectedLogPath)) .Returns(FileAccessPermissions.AllPermissions); // act @@ -149,7 +149,6 @@ public void TestThatDoesNotFailWhenDirectoryPermissionIsNot600Or700() // assert t_unixOperations.Verify(u => u.CreateDirectoryWithPermissions(s_expectedLogPath, FilePermissions.S_IRUSR | FilePermissions.S_IWUSR | FilePermissions.S_IXUSR), Times.Once); - t_unixOperations.Verify(u => u.SetDirInfo(s_expectedLogPath), Times.Once); } [Test] diff --git a/Snowflake.Data/Configuration/EasyLoggingConfigParser.cs b/Snowflake.Data/Configuration/EasyLoggingConfigParser.cs index 27fa3a88e..dbc6820b4 100644 --- a/Snowflake.Data/Configuration/EasyLoggingConfigParser.cs +++ b/Snowflake.Data/Configuration/EasyLoggingConfigParser.cs @@ -70,8 +70,7 @@ private void Validate(ClientConfig config) private void CheckForUnknownFields(string fileContent) { // Parse the specified config file and get the key-value pairs from the "common" section - List knownProperties = new List(); - knownProperties = typeof(ClientConfigCommonProps).GetProperties() + List knownProperties = typeof(ClientConfigCommonProps).GetProperties() .Select(property => property.GetCustomAttribute().PropertyName) .ToList(); diff --git a/Snowflake.Data/Core/Session/EasyLoggingStarter.cs b/Snowflake.Data/Core/Session/EasyLoggingStarter.cs index bd0a6eac0..17d6ed26e 100644 --- a/Snowflake.Data/Core/Session/EasyLoggingStarter.cs +++ b/Snowflake.Data/Core/Session/EasyLoggingStarter.cs @@ -129,6 +129,10 @@ private string GetLogPath(string logPath) } else { + if (!Directory.Exists(logPathOrDefault)) + { + Directory.CreateDirectory(logPathOrDefault); + } _unixOperations.CreateDirectoryWithPermissions(pathWithDotnetSubdirectory, FilePermissions.S_IRUSR | FilePermissions.S_IWUSR | FilePermissions.S_IXUSR); CheckDirPermissionsOnlyAllowUser(pathWithDotnetSubdirectory); @@ -143,8 +147,7 @@ private void CheckDirPermissionsOnlyAllowUser(string dirPath) if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) return; - _unixOperations.SetDirInfo(dirPath); - var dirPermissions = _unixOperations.GetDirPermissions(); + var dirPermissions = _unixOperations.GetDirPermissions(dirPath); if (dirPermissions != FileAccessPermissions.UserReadWriteExecute && dirPermissions != (FileAccessPermissions.UserRead | FileAccessPermissions.UserExecute)) { diff --git a/Snowflake.Data/Core/Tools/UnixOperations.cs b/Snowflake.Data/Core/Tools/UnixOperations.cs index dd044d180..d0e1fdf36 100644 --- a/Snowflake.Data/Core/Tools/UnixOperations.cs +++ b/Snowflake.Data/Core/Tools/UnixOperations.cs @@ -4,7 +4,6 @@ using Mono.Unix; using Mono.Unix.Native; -using System.IO; namespace Snowflake.Data.Core.Tools { @@ -12,37 +11,21 @@ internal class UnixOperations { public static readonly UnixOperations Instance = new UnixOperations(); - private UnixFileInfo _unixFileInfo; - private UnixDirectoryInfo _unixDirInfo; - - public virtual void SetDirInfo(string path) - { - _unixDirInfo = new UnixDirectoryInfo(path); - } - public virtual void CreateDirectoryWithPermissions(string path, FilePermissions permissions) { - string subPath = Path.GetDirectoryName(path); - if (!Directory.Exists(subPath)) - { - Directory.CreateDirectory(subPath); - } Syscall.mkdir(path, permissions); } - public virtual FileAccessPermissions GetDirPermissions() - { - return _unixDirInfo.FileAccessPermissions; - } - - public virtual void SetFileInfo(string path) + public virtual FileAccessPermissions GetDirPermissions(string path) { - _unixFileInfo = new UnixFileInfo(path); + var dirInfo = new UnixDirectoryInfo(path); + return dirInfo.FileAccessPermissions; } - public virtual bool CheckFileHasPermissions(FileAccessPermissions permissions) + public virtual bool CheckFileHasPermissions(string path, FileAccessPermissions permissions) { - return _unixFileInfo.FileAccessPermissions.HasFlag(permissions); + var fileInfo = new UnixFileInfo(path); + return fileInfo.FileAccessPermissions.HasFlag(permissions); } } } From 652499895f897a483afbed9693a4b95936d4bb27 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Tue, 30 Jan 2024 11:13:54 -0800 Subject: [PATCH 40/45] Refactor UnixOperations and clean up --- Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs b/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs index 8bfdb77b6..e81243378 100644 --- a/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs +++ b/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs @@ -79,12 +79,9 @@ private string SearchForConfigInDirectory(Func directoryProvider, string var directory = directoryProvider.Invoke(); if (string.IsNullOrEmpty(directory)) { + s_logger.Warn($"The {directoryDescription} directory could not be determined and will be skipped"); return null; } - if (!_directoryOperations.Exists(directory)) - { - s_logger.Warn($"Searching for a config file in the {directoryDescription} directory that does not exist: {directory}"); - } var filePath = Path.Combine(directory, ClientConfigFileName); return OnlyIfFileExists(filePath, directoryDescription); @@ -112,8 +109,7 @@ private void CheckIfValidPermissions(string filePath) return; // Check if others have permissions to modify the file and fail if so - _unixOperations.SetFileInfo(filePath); - if (_unixOperations.CheckFileHasPermissions(FileAccessPermissions.GroupWrite | FileAccessPermissions.OtherWrite)) + if (_unixOperations.CheckFileHasPermissions(filePath, FileAccessPermissions.GroupWrite | FileAccessPermissions.OtherWrite)) { var errorMessage = $"Error due to other users having permission to modify the config file: {filePath}"; s_logger.Error(errorMessage); From 59945da178b1638fe6fcfb3bdaf4510ecd60ac53 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Tue, 30 Jan 2024 13:37:49 -0800 Subject: [PATCH 41/45] Refactor checking directory permissions --- .../UnitTests/Session/EasyLoggingStarterTest.cs | 8 ++++---- Snowflake.Data/Core/Session/EasyLoggingStarter.cs | 5 ++++- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs b/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs index a0301e835..39796b722 100644 --- a/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs @@ -125,7 +125,7 @@ public void TestThatThrowsErrorWhenLogPathIsNotSetAndHomeDirectoryThrowsAnExcept } [Test] - public void TestThatDoesNotFailWhenDirectoryPermissionIsNot600Or700() + public void TestThatDoesNotFailWhenExistingDirectoryContainsUnexpectedPermissions() { if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { @@ -137,8 +137,8 @@ public void TestThatDoesNotFailWhenDirectoryPermissionIsNot600Or700() .Setup(provider => provider.ProvideConfig(ConfigPath)) .Returns(s_configWithInfoLevel); t_directoryOperations - .Setup(dir => dir.Exists(ConfigPath)) - .Returns(false); + .Setup(dir => dir.Exists(s_expectedLogPath)) + .Returns(true); t_unixOperations .Setup(unix => unix.GetDirPermissions(s_expectedLogPath)) .Returns(FileAccessPermissions.AllPermissions); @@ -148,7 +148,7 @@ public void TestThatDoesNotFailWhenDirectoryPermissionIsNot600Or700() // assert t_unixOperations.Verify(u => u.CreateDirectoryWithPermissions(s_expectedLogPath, - FilePermissions.S_IRUSR | FilePermissions.S_IWUSR | FilePermissions.S_IXUSR), Times.Once); + FilePermissions.S_IRUSR | FilePermissions.S_IWUSR | FilePermissions.S_IXUSR), Times.Never); } [Test] diff --git a/Snowflake.Data/Core/Session/EasyLoggingStarter.cs b/Snowflake.Data/Core/Session/EasyLoggingStarter.cs index 17d6ed26e..ab155b8cd 100644 --- a/Snowflake.Data/Core/Session/EasyLoggingStarter.cs +++ b/Snowflake.Data/Core/Session/EasyLoggingStarter.cs @@ -135,9 +135,12 @@ private string GetLogPath(string logPath) } _unixOperations.CreateDirectoryWithPermissions(pathWithDotnetSubdirectory, FilePermissions.S_IRUSR | FilePermissions.S_IWUSR | FilePermissions.S_IXUSR); - CheckDirPermissionsOnlyAllowUser(pathWithDotnetSubdirectory); } } + else + { + CheckDirPermissionsOnlyAllowUser(pathWithDotnetSubdirectory); + } return pathWithDotnetSubdirectory; } From 17e9efe0ff744fe9761cf0ea38800fa6b80972ca Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Wed, 31 Jan 2024 11:17:14 -0800 Subject: [PATCH 42/45] Revert log directory check, remove unused variable, and refactor permission check --- .../UnitTests/Session/EasyLoggingStarterTest.cs | 2 +- Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs | 1 - Snowflake.Data/Core/Session/EasyLoggingStarter.cs | 8 ++------ 3 files changed, 3 insertions(+), 8 deletions(-) diff --git a/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs b/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs index 39796b722..3b96339a0 100644 --- a/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs @@ -125,7 +125,7 @@ public void TestThatThrowsErrorWhenLogPathIsNotSetAndHomeDirectoryThrowsAnExcept } [Test] - public void TestThatDoesNotFailWhenExistingDirectoryContainsUnexpectedPermissions() + public void TestThatDoesNotFailWhenLogDirectoryPermissionIsNot700() { if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { diff --git a/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs b/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs index e81243378..3a541e955 100644 --- a/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs +++ b/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs @@ -20,7 +20,6 @@ internal class EasyLoggingConfigFinder private readonly FileOperations _fileOperations; private readonly UnixOperations _unixOperations; - private readonly DirectoryOperations _directoryOperations; private readonly EnvironmentOperations _environmentOperations; public static readonly EasyLoggingConfigFinder Instance = new EasyLoggingConfigFinder(FileOperations.Instance, UnixOperations.Instance, DirectoryOperations.Instance, EnvironmentOperations.Instance); diff --git a/Snowflake.Data/Core/Session/EasyLoggingStarter.cs b/Snowflake.Data/Core/Session/EasyLoggingStarter.cs index ab155b8cd..19a007c8f 100644 --- a/Snowflake.Data/Core/Session/EasyLoggingStarter.cs +++ b/Snowflake.Data/Core/Session/EasyLoggingStarter.cs @@ -137,10 +137,7 @@ private string GetLogPath(string logPath) FilePermissions.S_IRUSR | FilePermissions.S_IWUSR | FilePermissions.S_IXUSR); } } - else - { - CheckDirPermissionsOnlyAllowUser(pathWithDotnetSubdirectory); - } + CheckDirPermissionsOnlyAllowUser(pathWithDotnetSubdirectory); return pathWithDotnetSubdirectory; } @@ -151,8 +148,7 @@ private void CheckDirPermissionsOnlyAllowUser(string dirPath) return; var dirPermissions = _unixOperations.GetDirPermissions(dirPath); - if (dirPermissions != FileAccessPermissions.UserReadWriteExecute && - dirPermissions != (FileAccessPermissions.UserRead | FileAccessPermissions.UserExecute)) + if (dirPermissions != FileAccessPermissions.UserReadWriteExecute) { s_logger.Warn($"Access permission for the logs directory is currently " + $"{UnixFilePermissionsConverter.ConvertFileAccessPermissionsToInt(dirPermissions)}"); From 065941fba2f76af6c551061d107edb31dbadfd8e Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Wed, 31 Jan 2024 11:25:30 -0800 Subject: [PATCH 43/45] Remove unused variable --- .../EasyLoggingConfigFinderTest.cs | 23 +------------------ .../Configuration/EasyLoggingConfigFinder.cs | 5 ++-- 2 files changed, 3 insertions(+), 25 deletions(-) diff --git a/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs b/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs index 7d1de4930..bea1e14d0 100644 --- a/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs @@ -29,9 +29,6 @@ public class EasyLoggingConfigFinderTest [ThreadStatic] private static Mock t_unixOperations; - [ThreadStatic] - private static Mock t_directoryOperations; - [ThreadStatic] private static Mock t_environmentOperations; @@ -43,10 +40,8 @@ public void Setup() { t_fileOperations = new Mock(); t_unixOperations = new Mock(); - t_directoryOperations = new Mock(); t_environmentOperations = new Mock(); - t_finder = new EasyLoggingConfigFinder(t_fileOperations.Object, t_unixOperations.Object, t_directoryOperations.Object, t_environmentOperations.Object); - MockDirectoriesExist(); + t_finder = new EasyLoggingConfigFinder(t_fileOperations.Object, t_unixOperations.Object, t_environmentOperations.Object); MockHomeDirectory(); } @@ -186,7 +181,6 @@ public void TestThatDoesNotFailWhenHomeDirectoryDoesNotExist() { // arrange MockFileOnHomePath(); - MockHomeDirectoryDoesNotExist(); MockFileOnHomePathDoesNotExist(); // act @@ -197,14 +191,6 @@ public void TestThatDoesNotFailWhenHomeDirectoryDoesNotExist() t_environmentOperations.Verify(e => e.GetFolderPath(Environment.SpecialFolder.UserProfile), Times.Once); } - private static void MockDirectoriesExist() - { - t_directoryOperations - .Setup(d => d.Exists( - It.Is(dir => dir.Equals(DriverDirectory) || dir.Equals(HomeDirectory)))) - .Returns(true); - } - private static void MockHasFlagReturnsTrue() { t_unixOperations @@ -227,13 +213,6 @@ private static void MockHomeDirectoryFails() .Throws(() => new Exception("No home directory")); } - private static void MockHomeDirectoryDoesNotExist() - { - t_directoryOperations - .Setup(d => d.Exists(HomeDirectory)) - .Returns(false); - } - private static void MockFileOnHomePathDoesNotExist() { t_fileOperations diff --git a/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs b/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs index 3a541e955..3fed8850e 100644 --- a/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs +++ b/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs @@ -22,13 +22,12 @@ internal class EasyLoggingConfigFinder private readonly UnixOperations _unixOperations; private readonly EnvironmentOperations _environmentOperations; - public static readonly EasyLoggingConfigFinder Instance = new EasyLoggingConfigFinder(FileOperations.Instance, UnixOperations.Instance, DirectoryOperations.Instance, EnvironmentOperations.Instance); + public static readonly EasyLoggingConfigFinder Instance = new EasyLoggingConfigFinder(FileOperations.Instance, UnixOperations.Instance, EnvironmentOperations.Instance); - internal EasyLoggingConfigFinder(FileOperations fileOperations, UnixOperations unixFileOperations, DirectoryOperations directoryOperations, EnvironmentOperations environmentOperations) + internal EasyLoggingConfigFinder(FileOperations fileOperations, UnixOperations unixFileOperations, EnvironmentOperations environmentOperations) { _fileOperations = fileOperations; _unixOperations = unixFileOperations; - _directoryOperations = directoryOperations; _environmentOperations = environmentOperations; } From 1c0babc9707171f709eccfec789dbdd6991e55e7 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Thu, 1 Feb 2024 08:32:48 -0800 Subject: [PATCH 44/45] SNOW-990111-: Modify warning message --- Snowflake.Data/Core/Session/EasyLoggingStarter.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Snowflake.Data/Core/Session/EasyLoggingStarter.cs b/Snowflake.Data/Core/Session/EasyLoggingStarter.cs index 19a007c8f..bdda53860 100644 --- a/Snowflake.Data/Core/Session/EasyLoggingStarter.cs +++ b/Snowflake.Data/Core/Session/EasyLoggingStarter.cs @@ -151,7 +151,8 @@ private void CheckDirPermissionsOnlyAllowUser(string dirPath) if (dirPermissions != FileAccessPermissions.UserReadWriteExecute) { s_logger.Warn($"Access permission for the logs directory is currently " + - $"{UnixFilePermissionsConverter.ConvertFileAccessPermissionsToInt(dirPermissions)}"); + $"{UnixFilePermissionsConverter.ConvertFileAccessPermissionsToInt(dirPermissions)} " + + $"and is potentially accessible to users other than the owner of the logs directory"); } } } From dd824e6024b31bcdb7b662efbc59a73ec82e1008 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Fri, 2 Feb 2024 09:19:12 -0800 Subject: [PATCH 45/45] SNOW-990111: Move where logging is executed --- .../Core/Session/EasyLoggingStarter.cs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/Snowflake.Data/Core/Session/EasyLoggingStarter.cs b/Snowflake.Data/Core/Session/EasyLoggingStarter.cs index bdda53860..cd82d3904 100644 --- a/Snowflake.Data/Core/Session/EasyLoggingStarter.cs +++ b/Snowflake.Data/Core/Session/EasyLoggingStarter.cs @@ -54,21 +54,20 @@ internal EasyLoggingStarter() public virtual void Init(string configFilePathFromConnectionString) { - if (string.IsNullOrEmpty(configFilePathFromConnectionString)) - { - s_logger.Info($"Attempting to enable easy logging without a config file specified from connection string"); - } - else - { - s_logger.Info($"Attempting to enable easy logging using config file specified from connection string: {configFilePathFromConnectionString}"); - } - lock (_lockForExclusiveInit) { if (!AllowedToInitialize(configFilePathFromConnectionString)) { return; } + if (string.IsNullOrEmpty(configFilePathFromConnectionString)) + { + s_logger.Info($"Attempting to enable easy logging without a config file specified from connection string"); + } + else + { + s_logger.Info($"Attempting to enable easy logging using config file specified from connection string: {configFilePathFromConnectionString}"); + } var config = _easyLoggingConfigProvider.ProvideConfig(configFilePathFromConnectionString); if (config == null) {