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
92 changes: 73 additions & 19 deletions client_configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"fmt"
"os"
"path"
"runtime"
"strings"
)

Expand All @@ -26,39 +27,47 @@
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
}

Check warning on line 35 in client_configuration.go

View check run for this annotation

Codecov / codecov/patch

client_configuration.go#L34-L35

Added lines #L34 - L35 were not covered by tests
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)

Check warning on line 61 in client_configuration.go

View check run for this annotation

Codecov / codecov/patch

client_configuration.go#L61

Added line #L61 was not covered by tests
}
if exists {
return filePath
logger.Infof("Using client configuration from a default directory: %s", filePath)
sfc-gh-knozderko marked this conversation as resolved.
Show resolved Hide resolved
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 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{"."}

Check warning on line 88 in client_configuration.go

View check run for this annotation

Codecov / codecov/patch

client_configuration.go#L87-L88

Added lines #L87 - L88 were not covered by tests
}
return []string{".", homeDir, os.TempDir()}
return []string{".", homeDir}
}

// ClientConfig config root
Expand All @@ -100,18 +109,47 @@
if err != nil {
return nil, parsingClientConfigError(err)
}
err = validateCfgPerm(filePath)
if err != nil {
return nil, parsingClientConfigError(err)
}

Check warning on line 115 in client_configuration.go

View check run for this annotation

Codecov / codecov/patch

client_configuration.go#L114-L115

Added lines #L114 - L115 were not covered by tests
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])
}

Check warning on line 125 in client_configuration.go

View check run for this annotation

Codecov / codecov/patch

client_configuration.go#L123-L125

Added lines #L123 - L125 were not covered by tests
}
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 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
}

Check warning on line 185 in client_configuration.go

View check run for this annotation

Codecov / codecov/patch

client_configuration.go#L184-L185

Added lines #L184 - L185 were not covered by tests
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
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
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