Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SNOW-999076: Easy Logging fixes #1030

Merged
merged 14 commits into from
Jan 26, 2024
70 changes: 60 additions & 10 deletions client_configuration.go
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,21 +27,24 @@ const (
clientConfEnvName = "SF_CLIENT_CONFIG_FILE"
)

func getClientConfig(filePathFromConnectionString string) (*ClientConfig, error) {
func getClientConfig(filePathFromConnectionString string) (*ClientConfig, string, error) {
configPredefinedFilePaths := clientConfigPredefinedDirs()
sfc-gh-knozderko marked this conversation as resolved.
Show resolved Hide resolved
filePath := findClientConfigFilePath(filePathFromConnectionString, configPredefinedFilePaths)
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 {
if filePathFromConnectionString != "" {
logger.Infof("Using Easy Logging configuration path from a connection string: %s", filePathFromConnectionString)
sfc-gh-pfus marked this conversation as resolved.
Show resolved Hide resolved
return filePathFromConnectionString
}
envConfigFilePath := os.Getenv(clientConfEnvName)
if envConfigFilePath != "" {
logger.Infof("Using Easy Logging configuration path from an environment variable: %s", envConfigFilePath)
return envConfigFilePath
}
return searchForConfigFile(configPredefinedDirs)
Expand All @@ -51,13 +55,15 @@ func searchForConfigFile(directories []string) string {
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)
logger.Debugf("No client config found in directory: %s, err: %s", dir, err)
sfc-gh-knozderko marked this conversation as resolved.
Show resolved Hide resolved
continue
sfc-gh-knozderko marked this conversation as resolved.
Show resolved Hide resolved
}
if exists {
logger.Infof("Using Easy Logging configuration from a default directory: %s", filePath)
return filePath
}
}
logger.Info("No Easy Logging config file found in default directories")
return ""
}

Expand All @@ -75,10 +81,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 Easy Logging configuration search, err: %v", err)
sfc-gh-pbulawa marked this conversation as resolved.
Show resolved Hide resolved
return nil
sfc-gh-knozderko marked this conversation as resolved.
Show resolved Hide resolved
}
return []string{".", homeDir, os.TempDir()}
return []string{homeDir}
}

// ClientConfig config root
Expand All @@ -100,18 +106,47 @@ func parseClientConfiguration(filePath string) (*ClientConfig, error) {
if err != nil {
return nil, parsingClientConfigError(err)
}
_, err = isCfgPermValid(filePath)
sfc-gh-pfus marked this conversation as resolved.
Show resolved Hide resolved
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

sfc-gh-knozderko marked this conversation as resolved.
Show resolved Hide resolved
}
if values["common"] == nil {
return nil
}
commonValues := values["common"].(map[string]interface{})
delete(commonValues, "log_level")
sfc-gh-knozderko marked this conversation as resolved.
Show resolved Hide resolved
delete(commonValues, "log_path")
if len(commonValues) == 0 {
return nil
}
return commonValues
}

func parsingClientConfigError(err error) error {
return fmt.Errorf("parsing client config failed: %w", err)
}
Expand All @@ -129,14 +164,29 @@ 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 isCfgPermValid(filePath string) (bool, error) {
if runtime.GOOS != "windows" {
sfc-gh-knozderko marked this conversation as resolved.
Show resolved Hide resolved
stat, err := os.Stat(filePath)
if err != nil {
return false, 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 {
sfc-gh-knozderko marked this conversation as resolved.
Show resolved Hide resolved
return false, fmt.Errorf("configuration file: %s can be modified by group or others", filePath)
}
}
return true, nil
}

func toLogLevel(logLevelString string) (string, error) {
var logLevel = strings.ToUpper(logLevelString)
switch logLevel {
Expand Down
137 changes: 129 additions & 8 deletions client_configuration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@

import (
"fmt"
"golang.org/x/sys/unix"
"os"
"path"
"runtime"
"strings"
"testing"
)
Expand Down Expand Up @@ -71,10 +73,8 @@

locations := clientConfigPredefinedDirs()

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

func TestGetClientConfig(t *testing.T) {
Expand All @@ -84,7 +84,7 @@
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 +93,7 @@
}

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

assertNilF(t, err)
assertNilF(t, clientConfigFilePath)
Expand Down Expand Up @@ -223,6 +223,127 @@
}
}

func TestUnknownValues(t *testing.T) {
testCases := []struct {
testName string
inputString string
expectedOutput map[string]string
}{
{
testName: "EmptyCommon",
inputString: `{
"common": {}
}`,
expectedOutput: nil,
},
{
testName: "CommonMissing",
inputString: `{
}`,
expectedOutput: nil,
},
{
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: nil,
},

{
testName: "EmptyInput",
inputString: "",
expectedOutput: nil,
},
}

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 TestConfigPermissions(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("We do not check permissions on Windows")
}
testCases := []struct {
filePerm int
isValid bool
}{
{filePerm: 0700, isValid: true},
{filePerm: 0600, isValid: true},
{filePerm: 0500, isValid: true},
{filePerm: 0400, isValid: true},
{filePerm: 0300, isValid: true},
{filePerm: 0200, isValid: true},
{filePerm: 0100, isValid: true},
{filePerm: 0707, isValid: false},
{filePerm: 0706, isValid: false},
{filePerm: 0705, isValid: true},
{filePerm: 0704, isValid: true},
{filePerm: 0703, isValid: false},
{filePerm: 0702, isValid: false},
{filePerm: 0701, isValid: true},
{filePerm: 0770, isValid: false},
{filePerm: 0760, isValid: false},
{filePerm: 0750, isValid: true},
{filePerm: 0740, isValid: true},
{filePerm: 0730, isValid: false},
{filePerm: 0720, isValid: false},
{filePerm: 0710, isValid: true},
}

oldMask := unix.Umask(0000)

Check failure on line 327 in client_configuration_test.go

View workflow job for this annotation

GitHub Actions / AWS Go 1.21 on Windows

undefined: unix.Umask

Check failure on line 327 in client_configuration_test.go

View workflow job for this annotation

GitHub Actions / GCP Go 1.20 on Windows

undefined: unix.Umask

Check failure on line 327 in client_configuration_test.go

View workflow job for this annotation

GitHub Actions / AWS Go 1.19 on Windows

undefined: unix.Umask

Check failure on line 327 in client_configuration_test.go

View workflow job for this annotation

GitHub Actions / AZURE Go 1.21 on Windows

undefined: unix.Umask

Check failure on line 327 in client_configuration_test.go

View workflow job for this annotation

GitHub Actions / GCP Go 1.19 on Windows

undefined: unix.Umask

Check failure on line 327 in client_configuration_test.go

View workflow job for this annotation

GitHub Actions / GCP Go 1.21 on Windows

undefined: unix.Umask

Check failure on line 327 in client_configuration_test.go

View workflow job for this annotation

GitHub Actions / AZURE Go 1.20 on Windows

undefined: unix.Umask

Check failure on line 327 in client_configuration_test.go

View workflow job for this annotation

GitHub Actions / AWS Go 1.20 on Windows

undefined: unix.Umask

Check failure on line 327 in client_configuration_test.go

View workflow job for this annotation

GitHub Actions / AZURE Go 1.19 on Windows

undefined: unix.Umask
defer unix.Umask(oldMask)

Check failure on line 328 in client_configuration_test.go

View workflow job for this annotation

GitHub Actions / AWS Go 1.21 on Windows

undefined: unix.Umask

Check failure on line 328 in client_configuration_test.go

View workflow job for this annotation

GitHub Actions / GCP Go 1.20 on Windows

undefined: unix.Umask

Check failure on line 328 in client_configuration_test.go

View workflow job for this annotation

GitHub Actions / AWS Go 1.19 on Windows

undefined: unix.Umask

Check failure on line 328 in client_configuration_test.go

View workflow job for this annotation

GitHub Actions / AZURE Go 1.21 on Windows

undefined: unix.Umask

Check failure on line 328 in client_configuration_test.go

View workflow job for this annotation

GitHub Actions / GCP Go 1.19 on Windows

undefined: unix.Umask

Check failure on line 328 in client_configuration_test.go

View workflow job for this annotation

GitHub Actions / GCP Go 1.21 on Windows

undefined: unix.Umask

Check failure on line 328 in client_configuration_test.go

View workflow job for this annotation

GitHub Actions / AZURE Go 1.20 on Windows

undefined: unix.Umask

Check failure on line 328 in client_configuration_test.go

View workflow job for this annotation

GitHub Actions / AWS Go 1.20 on Windows

undefined: unix.Umask

Check failure on line 328 in client_configuration_test.go

View workflow job for this annotation

GitHub Actions / AZURE Go 1.19 on Windows

undefined: unix.Umask

for _, tc := range testCases {
t.Run(fmt.Sprintf("0%o", tc.filePerm), func(t *testing.T) {
tempFile := path.Join(t.TempDir(), fmt.Sprintf("filePerm_%o", tc.filePerm))
err := os.WriteFile(tempFile, nil, os.FileMode(tc.filePerm))
if err != nil {
sfc-gh-knozderko marked this conversation as resolved.
Show resolved Hide resolved
t.Error(err)
}
defer os.Remove(tempFile)
result, err := isCfgPermValid(tempFile)
if err != nil && tc.isValid {
t.Error(err)
}
assertEqualE(t, result, tc.isValid)
})
}
}

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 +358,10 @@
} {
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
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
Loading