Skip to content

Commit

Permalink
SNOW-999076: Easy Logging fixes (#1030)
Browse files Browse the repository at this point in the history
  • Loading branch information
@sfc-gh-pbulawa
sfc-gh-pbulawa authored Jan 26, 2024
1 parent 7b3319d commit d63ec27
Show file tree
Hide file tree
Showing 7 changed files with 330 additions and 65 deletions.
92 changes: 73 additions & 19 deletions client_configuration.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"fmt"
"os"
"path"
"runtime"
"strings"
)

Expand All @@ -26,39 +27,47 @@ const (
clientConfEnvName = "SF_CLIENT_CONFIG_FILE"
)

func getClientConfig(filePathFromConnectionString string) (*ClientConfig, error) {
configPredefinedFilePaths := clientConfigPredefinedDirs()
filePath := findClientConfigFilePath(filePathFromConnectionString, configPredefinedFilePaths)
func getClientConfig(filePathFromConnectionString string) (*ClientConfig, string, error) {
configPredefinedDirPaths := clientConfigPredefinedDirs()
filePath, err := findClientConfigFilePath(filePathFromConnectionString, configPredefinedDirPaths)
if err != nil {
return nil, "", err
}
if filePath == "" { // we did not find a config file
return nil, nil
return nil, "", nil
}
return parseClientConfiguration(filePath)
config, err := parseClientConfiguration(filePath)
return config, filePath, err
}

func findClientConfigFilePath(filePathFromConnectionString string, configPredefinedDirs []string) string {
func findClientConfigFilePath(filePathFromConnectionString string, configPredefinedDirs []string) (string, error) {
if filePathFromConnectionString != "" {
return filePathFromConnectionString
logger.Infof("Using client configuration path from a connection string: %s", filePathFromConnectionString)
return filePathFromConnectionString, nil
}
envConfigFilePath := os.Getenv(clientConfEnvName)
if envConfigFilePath != "" {
return envConfigFilePath
logger.Infof("Using client configuration path from an environment variable: %s", envConfigFilePath)
return envConfigFilePath, nil
}
return searchForConfigFile(configPredefinedDirs)
}

func searchForConfigFile(directories []string) string {
func searchForConfigFile(directories []string) (string, error) {
for _, dir := range directories {
filePath := path.Join(dir, defaultConfigName)
exists, err := existsFile(filePath)
if err != nil {
logger.Errorf("Error while searching for the client config in %s directory: %s", dir, err)
continue
return "", fmt.Errorf("error while searching for client config in directory: %s, err: %s", dir, err)
}
if exists {
return filePath
logger.Infof("Using client configuration from a default directory: %s", filePath)
return filePath, nil
}
logger.Debugf("No client config found in directory: %s", dir)
}
return ""
logger.Info("No client config file found in default directories")
return "", nil
}

func existsFile(filePath string) (bool, error) {
Expand All @@ -75,10 +84,10 @@ func existsFile(filePath string) (bool, error) {
func clientConfigPredefinedDirs() []string {
homeDir, err := os.UserHomeDir()
if err != nil {
logger.Warnf("Home dir could not be determined: %w", err)
return []string{".", os.TempDir()}
logger.Warnf("Unable to access Home directory for client configuration search, err: %v", err)
return []string{"."}
}
return []string{".", homeDir, os.TempDir()}
return []string{".", homeDir}
}

// ClientConfig config root
Expand All @@ -100,18 +109,47 @@ func parseClientConfiguration(filePath string) (*ClientConfig, error) {
if err != nil {
return nil, parsingClientConfigError(err)
}
err = validateCfgPerm(filePath)
if err != nil {
return nil, parsingClientConfigError(err)
}
var clientConfig ClientConfig
err = json.Unmarshal(fileContents, &clientConfig)
if err != nil {
return nil, parsingClientConfigError(err)
}
unknownValues := getUnknownValues(fileContents)
if len(unknownValues) > 0 {
for val := range unknownValues {
logger.Warnf("Unknown configuration entry: %s with value: %s", val, unknownValues[val])
}
}
err = validateClientConfiguration(&clientConfig)
if err != nil {
return nil, parsingClientConfigError(err)
}
return &clientConfig, nil
}

func getUnknownValues(fileContents []byte) map[string]interface{} {
var values map[string]interface{}
err := json.Unmarshal(fileContents, &values)
if err != nil {
return nil
}
if values["common"] == nil {
return nil
}
commonValues := values["common"].(map[string]interface{})
lowercaseCommonValues := make(map[string]interface{}, len(commonValues))
for k, v := range commonValues {
lowercaseCommonValues[strings.ToLower(k)] = v
}
delete(lowercaseCommonValues, "log_level")
delete(lowercaseCommonValues, "log_path")
return lowercaseCommonValues
}

func parsingClientConfigError(err error) error {
return fmt.Errorf("parsing client config failed: %w", err)
}
Expand All @@ -129,14 +167,30 @@ func validateClientConfiguration(clientConfig *ClientConfig) error {
func validateLogLevel(clientConfig ClientConfig) error {
var logLevel = clientConfig.Common.LogLevel
if logLevel != "" {
_, error := toLogLevel(logLevel)
if error != nil {
return error
_, err := toLogLevel(logLevel)
if err != nil {
return err
}
}
return nil
}

func validateCfgPerm(filePath string) error {
if runtime.GOOS == "windows" {
return nil
}
stat, err := os.Stat(filePath)
if err != nil {
return err
}
perm := stat.Mode()
// Check if group (5th LSB) or others (2nd LSB) have a write permission to the file
if perm&(1<<4) != 0 || perm&(1<<1) != 0 {
return fmt.Errorf("configuration file: %s can be modified by group or others", filePath)
}
return nil
}

func toLogLevel(logLevelString string) (string, error) {
var logLevel = strings.ToUpper(logLevelString)
switch logLevel {
Expand Down
96 changes: 85 additions & 11 deletions client_configuration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@ func TestFindConfigFileFromConnectionParameters(t *testing.T) {
createFile(t, defaultConfigName, "random content", dirs.predefinedDir1)
createFile(t, defaultConfigName, "random content", dirs.predefinedDir2)

clientConfigFilePath := findClientConfigFilePath(connParameterConfigPath, predefinedTestDirs(dirs))
clientConfigFilePath, err := findClientConfigFilePath(connParameterConfigPath, predefinedTestDirs(dirs))

assertEqualE(t, err, nil)
assertEqualE(t, clientConfigFilePath, connParameterConfigPath, "config file path")
}

Expand All @@ -30,8 +31,9 @@ func TestFindConfigFileFromEnvVariable(t *testing.T) {
createFile(t, defaultConfigName, "random content", dirs.predefinedDir1)
createFile(t, defaultConfigName, "random content", dirs.predefinedDir2)

clientConfigFilePath := findClientConfigFilePath("", predefinedTestDirs(dirs))
clientConfigFilePath, err := findClientConfigFilePath("", predefinedTestDirs(dirs))

assertEqualE(t, err, nil)
assertEqualE(t, clientConfigFilePath, envConfigPath, "config file path")
}

Expand All @@ -40,8 +42,9 @@ func TestFindConfigFileFromFirstPredefinedDir(t *testing.T) {
configPath := createFile(t, defaultConfigName, "random content", dirs.predefinedDir1)
createFile(t, defaultConfigName, "random content", dirs.predefinedDir2)

clientConfigFilePath := findClientConfigFilePath("", predefinedTestDirs(dirs))
clientConfigFilePath, err := findClientConfigFilePath("", predefinedTestDirs(dirs))

assertEqualE(t, err, nil)
assertEqualE(t, clientConfigFilePath, configPath, "config file path")
}

Expand All @@ -50,8 +53,9 @@ func TestFindConfigFileFromSubsequentDirectoryIfNotFoundInPreviousOne(t *testing
createFile(t, "wrong_file_name.json", "random content", dirs.predefinedDir1)
configPath := createFile(t, defaultConfigName, "random content", dirs.predefinedDir2)

clientConfigFilePath := findClientConfigFilePath("", predefinedTestDirs(dirs))
clientConfigFilePath, err := findClientConfigFilePath("", predefinedTestDirs(dirs))

assertEqualE(t, err, nil)
assertEqualE(t, clientConfigFilePath, configPath, "config file path")
}

Expand All @@ -60,8 +64,9 @@ func TestNotFindConfigFileWhenNotDefined(t *testing.T) {
createFile(t, "wrong_file_name.json", "random content", dirs.predefinedDir1)
createFile(t, "wrong_file_name.json", "random content", dirs.predefinedDir2)

clientConfigFilePath := findClientConfigFilePath("", predefinedTestDirs(dirs))
clientConfigFilePath, err := findClientConfigFilePath("", predefinedTestDirs(dirs))

assertEqualE(t, err, nil)
assertEqualE(t, clientConfigFilePath, "", "config file path")
}

Expand All @@ -71,10 +76,9 @@ func TestCreatePredefinedDirs(t *testing.T) {

locations := clientConfigPredefinedDirs()

assertEqualF(t, len(locations), 3, "size")
assertEqualF(t, len(locations), 2, "size")
assertEqualE(t, locations[0], ".", "driver directory")
assertEqualE(t, locations[1], homeDir, "home directory")
assertEqualE(t, locations[2], os.TempDir(), "temp directory")
}

func TestGetClientConfig(t *testing.T) {
Expand All @@ -84,7 +88,7 @@ func TestGetClientConfig(t *testing.T) {
createFile(t, fileName, configContents, dir)
filePath := path.Join(dir, fileName)

clientConfigFilePath, err := getClientConfig(filePath)
clientConfigFilePath, _, err := getClientConfig(filePath)

assertNilF(t, err)
assertNotNilF(t, clientConfigFilePath)
Expand All @@ -93,7 +97,7 @@ func TestGetClientConfig(t *testing.T) {
}

func TestNoResultForGetClientConfigWhenNoFileFound(t *testing.T) {
clientConfigFilePath, err := getClientConfig("")
clientConfigFilePath, _, err := getClientConfig("")

assertNilF(t, err)
assertNilF(t, clientConfigFilePath)
Expand Down Expand Up @@ -223,6 +227,76 @@ func TestParseConfigurationFails(t *testing.T) {
}
}

func TestUnknownValues(t *testing.T) {
testCases := []struct {
testName string
inputString string
expectedOutput map[string]string
}{
{
testName: "EmptyCommon",
inputString: `{
"common": {}
}`,
expectedOutput: map[string]string{},
},
{
testName: "CommonMissing",
inputString: `{
}`,
expectedOutput: map[string]string{},
},
{
testName: "UnknownProperty",
inputString: `{
"common": {
"unknown_key": "unknown_value"
}
}`,
expectedOutput: map[string]string{
"unknown_key": "unknown_value",
},
},
{
testName: "KnownAndUnknownProperty",
inputString: `{
"common": {
"lOg_level": "level",
"log_PATH": "path",
"unknown_key": "unknown_value"
}
}`,
expectedOutput: map[string]string{
"unknown_key": "unknown_value",
},
},
{
testName: "KnownProperties",
inputString: `{
"common": {
"log_level": "level",
"log_path": "path"
}
}`,
expectedOutput: map[string]string{},
},

{
testName: "EmptyInput",
inputString: "",
expectedOutput: map[string]string{},
},
}

for _, tc := range testCases {
t.Run(tc.testName, func(t *testing.T) {
inputBytes := []byte(tc.inputString)
result := getUnknownValues(inputBytes)
assertEqualE(t, fmt.Sprint(result), fmt.Sprint(tc.expectedOutput))
})
}
}

func createFile(t *testing.T, fileName string, fileContents string, directory string) string {
fullFileName := path.Join(directory, fileName)
err := os.WriteFile(fullFileName, []byte(fileContents), 0644)
Expand All @@ -237,10 +311,10 @@ func createTestDirectories(t *testing.T) struct {
} {
dir := t.TempDir()
predefinedDir1 := path.Join(dir, "dir1")
err := os.Mkdir(predefinedDir1, 0755)
err := os.Mkdir(predefinedDir1, 0700)
assertNilF(t, err, "predefined dir1 error")
predefinedDir2 := path.Join(dir, "dir2")
err = os.Mkdir(predefinedDir2, 0755)
err = os.Mkdir(predefinedDir2, 0700)
assertNilF(t, err, "predefined dir2 error")
return struct {
dir string
Expand Down
9 changes: 4 additions & 5 deletions connection.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -757,11 +757,10 @@ func buildSnowflakeConn(ctx context.Context, config Config) (*snowflakeConn, err
queryContextCache: (&queryContextCache{}).init(),
currentTimeProvider: defaultTimeProvider,
}
// Easy logging is temporarily disabled
//err := initEasyLogging(config.ClientConfigFile)
//if err != nil {
// return nil, err
//}
err := initEasyLogging(config.ClientConfigFile)
if err != nil {
return nil, err
}
var st http.RoundTripper = SnowflakeTransport
if sc.cfg.Transporter == nil {
if sc.cfg.InsecureMode {
Expand Down
Loading

0 comments on commit d63ec27

Please sign in to comment.