From edba965451dd65bfb3abaa92335fc8991927675d Mon Sep 17 00:00:00 2001 From: Paras-Wednesday Date: Tue, 25 Jun 2024 18:45:28 +0530 Subject: [PATCH 1/2] fix: the db creds injected through secret manager issue in the load env --- internal/config/env.go | 66 +++++++++----- internal/config/env_test.go | 175 ++++++++++++++++++++++++++++++++++-- 2 files changed, 208 insertions(+), 33 deletions(-) diff --git a/internal/config/env.go b/internal/config/env.go index 0505d143..cc981014 100644 --- a/internal/config/env.go +++ b/internal/config/env.go @@ -57,6 +57,9 @@ func FileName() string { } func LoadEnv() error { + const ( + localEnvFile = "local" + ) _, filename, _, ok := runtime.Caller(0) if !ok { return fmt.Errorf("Error getting current file path") @@ -71,18 +74,16 @@ func LoadEnv() error { envName := os.Getenv("ENVIRONMENT_NAME") if envName == "" { - envName = "local" + envName = localEnvFile } log.Println("envName: " + envName) envVarInjection := GetBool("ENV_INJECTION") - if !envVarInjection || envName == "local" { + if !envVarInjection || envName == localEnvFile { err = godotenv.Load(fmt.Sprintf("%s.env.%s", prefix, envName)) if err != nil { - fmt.Printf(".env.%s\n", envName) - log.Println(err) - return err + return fmt.Errorf("couldn't load .env.%s file: %w", envName, err) } fmt.Println("loaded", fmt.Sprintf("%s.env.%s", prefix, envName)) return nil @@ -90,26 +91,43 @@ func LoadEnv() error { dbCredsInjected := GetBool("COPILOT_DB_CREDS_VIA_SECRETS_MANAGER") - if dbCredsInjected { - type copilotSecrets struct { - Username string `json:"username"` - Host string `json:"host"` - DBName string `json:"dbname"` - Password string `json:"password"` - Port int `json:"port"` - } - secrets := &copilotSecrets{} + // except for local environment the db creds should be + // injected through the secret manager + if envName != localEnvFile && !dbCredsInjected { + return fmt.Errorf("db creds should be injected through secret manager") + } - err := json.Unmarshal([]byte(os.Getenv("DB_SECRET")), secrets) - if err != nil { - return err - } + // for local environment the db configs will be loaded through + // env itself by `godotenv`, even if db creds are not injected + if !dbCredsInjected { + return nil + } - os.Setenv("PSQL_DBNAME", secrets.DBName) - os.Setenv("PSQL_HOST", secrets.Host) - os.Setenv("PSQL_PASS", secrets.Password) - os.Setenv("PSQL_PORT", strconv.Itoa(secrets.Port)) - os.Setenv("PSQL_USER", secrets.Username) + // if dbCreds is injected then extract the db creds + return extractDBCredsFromSecret() +} + +// extractDBCredsFromSecret helper function to extract db secret +func extractDBCredsFromSecret() error { + type copilotSecrets struct { + Username string `json:"username"` + Host string `json:"host"` + DBName string `json:"dbname"` + Password string `json:"password"` + Port int `json:"port"` + } + secrets := &copilotSecrets{} + + err := json.Unmarshal([]byte(os.Getenv("DB_SECRET")), secrets) + if err != nil { + return fmt.Errorf("couldn't unmarshal db secret: %w", err) } - return fmt.Errorf("COPILOT_DB_CREDS_VIA_SECRETS_MANAGER should have had a value") + + os.Setenv("PSQL_DBNAME", secrets.DBName) + os.Setenv("PSQL_HOST", secrets.Host) + os.Setenv("PSQL_PASS", secrets.Password) + os.Setenv("PSQL_PORT", strconv.Itoa(secrets.Port)) + os.Setenv("PSQL_USER", secrets.Username) + + return nil } diff --git a/internal/config/env_test.go b/internal/config/env_test.go index 601023f1..7244d0d5 100644 --- a/internal/config/env_test.go +++ b/internal/config/env_test.go @@ -5,6 +5,7 @@ import ( . "go-template/internal/config" "os" "testing" + "time" "github.com/stretchr/testify/assert" ) @@ -41,6 +42,7 @@ func TestGetString(t *testing.T) { if tt.success { os.Setenv(tt.args.key, tt.want) } + time.Sleep(time.Microsecond) if got := GetString(tt.args.key); got != tt.want { t.Errorf("GetString() = %v, want %v", got, tt.want) @@ -231,6 +233,7 @@ func loadLocalEnv() envTestCaseArgs { }, } } + func errorOnEnvInjectionAndCopilotFalse() envTestCaseArgs { return envTestCaseArgs{ name: "Error when ENV_INJECTION and COPILOT_DB_CREDS_VIA_SECRETS_MANAGER false", @@ -254,10 +257,67 @@ func errorOnEnvInjectionAndCopilotFalse() envTestCaseArgs { } } -func loadOnDbCredsInjected(username string, host string, dbname string, password string, port string) envTestCaseArgs { +func loadOnCopilotTrueAndLocalEnv() envTestCaseArgs { + return envTestCaseArgs{ + name: "Load local without copilot", + wantErr: false, + args: args{ + setEnv: []keyValueArgs{ + { + key: "ENV_INJECTION", + value: "true", + }, + { + key: "ENVIRONMENT_NAME", + value: "local", + }, + { + key: "COPILOT_DB_CREDS_VIA_SECRETS_MANAGER", + value: "false", + }, + }, + }, + } +} + +func errorOnDbCredsInjectedInDevEnv() envTestCaseArgs { return envTestCaseArgs{ - name: "dbCredsInjected True", + name: "dbCredsInjected True for `develop` environment,with invalid json in db secret", wantErr: true, + args: args{ + setEnv: []keyValueArgs{ + { + key: "ENV_INJECTION", + value: "true", + }, + { + key: "ENVIRONMENT_NAME", + value: "develop", + }, + { + key: "COPILOT_DB_CREDS_VIA_SECRETS_MANAGER", + value: "true", + }, + { + key: "DB_SECRET", + value: "invalid json", + }, + }, + expectedKeyValues: []keyValueArgs{}, + }, + } +} + +func loadOnDbCredsInjectedInDevEnv( + username string, + host string, + dbname string, + password string, + port string, +) envTestCaseArgs { + return envTestCaseArgs{ + name: "dbCredsInjected True for `develop` environment,and should parse the db secret", + wantErr: false, args: args{ setEnv: []keyValueArgs{ { @@ -274,27 +334,103 @@ func loadOnDbCredsInjected(username string, host string, dbname string, password }, { key: "DB_SECRET", - value: fmt.Sprintf(`{"username": "%s", "password": "%s", "port": "%s", "dbname": "%s", "host": "%s"}`, + value: fmt.Sprintf(`{"username": "%s", "password": "%s", "port": %s, "dbname": "%s", "host": "%s"}`, username, password, port, - host, - dbname), + dbname, + host), }, }, expectedKeyValues: []keyValueArgs{ + { + key: "PSQL_USER", + value: username, + }, + { + key: "PSQL_PORT", + value: port, + }, { key: "PSQL_PASS", value: password, }, + { + key: "PSQL_HOST", + value: host, + }, { key: "PSQL_DBNAME", value: dbname, }, + }, + }, + } +} + +func errorOnDbCredsInjectedInLocalEnv() envTestCaseArgs { + return envTestCaseArgs{ + name: "dbCredsInjected True for `local` environment, with invalid json in db secret", + wantErr: true, + args: args{ + setEnv: []keyValueArgs{ + { + key: "ENV_INJECTION", + value: "true", + }, { - key: "PSQL_HOST", - value: host, + key: "ENVIRONMENT_NAME", + value: "develop", + }, + { + key: "COPILOT_DB_CREDS_VIA_SECRETS_MANAGER", + value: "true", + }, + { + key: "DB_SECRET", + value: `invalid json`, + }, + }, + expectedKeyValues: []keyValueArgs{}, + }, + } +} + +func loadDbCredsInjectedInLocalEnv( + username string, + host string, + dbname string, + password string, + port string, +) envTestCaseArgs { + return envTestCaseArgs{ + name: "dbCredsInjected True for local environment,and should parse the db secret", + wantErr: false, + args: args{ + setEnv: []keyValueArgs{ + { + key: "ENV_INJECTION", + value: "true", }, + { + key: "ENVIRONMENT_NAME", + value: "local", + }, + { + key: "COPILOT_DB_CREDS_VIA_SECRETS_MANAGER", + value: "true", + }, + { + key: "DB_SECRET", + value: fmt.Sprintf(`{"username": "%s", "password": "%s", "port": %s, "dbname": "%s", "host": "%s"}`, + username, + password, + port, + dbname, + host), + }, + }, + expectedKeyValues: []keyValueArgs{ { key: "PSQL_USER", value: username, @@ -303,6 +439,18 @@ func loadOnDbCredsInjected(username string, host string, dbname string, password key: "PSQL_PORT", value: port, }, + { + key: "PSQL_PASS", + value: password, + }, + { + key: "PSQL_HOST", + value: host, + }, + { + key: "PSQL_DBNAME", + value: dbname, + }, }, }, } @@ -310,7 +458,7 @@ func loadOnDbCredsInjected(username string, host string, dbname string, password func errorOnWrongEnvName() envTestCaseArgs { return envTestCaseArgs{ - name: "Failed to load env", + name: "Failed to load env for local1", wantErr: true, args: args{ setEnv: []keyValueArgs{ @@ -318,16 +466,25 @@ func errorOnWrongEnvName() envTestCaseArgs { key: "ENVIRONMENT_NAME", value: "local1", }, + { + key: "ENV_INJECTION", + value: "false", + }, }, }, } } + func getTestCases(username string, host string, dbname string, password string, port string) []envTestCaseArgs { return []envTestCaseArgs{ loadLocalEnvIfNoEnvName(), loadLocalEnv(), errorOnEnvInjectionAndCopilotFalse(), - loadOnDbCredsInjected(username, host, dbname, password, port), + loadOnCopilotTrueAndLocalEnv(), + errorOnDbCredsInjectedInDevEnv(), + loadOnDbCredsInjectedInDevEnv(username, host, dbname, password, port), + errorOnDbCredsInjectedInLocalEnv(), + loadDbCredsInjectedInLocalEnv(username, host, dbname, password, port), errorOnWrongEnvName(), } } From c3ab96b65911355bee80b424ce1ca325088281ba Mon Sep 17 00:00:00 2001 From: Paras-Wednesday Date: Wed, 26 Jun 2024 16:55:06 +0530 Subject: [PATCH 2/2] refactor: the logic for extract db creds - Reverse the condition - Remove sleep as it is redundant in the test file --- internal/config/env.go | 18 ++++++++---------- internal/config/env_test.go | 8 ++++---- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/internal/config/env.go b/internal/config/env.go index cc981014..0c8bf3b3 100644 --- a/internal/config/env.go +++ b/internal/config/env.go @@ -3,7 +3,6 @@ package config import ( "encoding/json" "fmt" - "go-template/pkg/utl/convert" "log" "os" "path/filepath" @@ -11,6 +10,8 @@ import ( "strconv" "github.com/joho/godotenv" + + "go-template/pkg/utl/convert" ) func GetString(key string) string { @@ -81,9 +82,8 @@ func LoadEnv() error { envVarInjection := GetBool("ENV_INJECTION") if !envVarInjection || envName == localEnvFile { err = godotenv.Load(fmt.Sprintf("%s.env.%s", prefix, envName)) - if err != nil { - return fmt.Errorf("couldn't load .env.%s file: %w", envName, err) + return fmt.Errorf("failed load env for environment %q file: %w", envName, err) } fmt.Println("loaded", fmt.Sprintf("%s.env.%s", prefix, envName)) return nil @@ -97,14 +97,12 @@ func LoadEnv() error { return fmt.Errorf("db creds should be injected through secret manager") } - // for local environment the db configs will be loaded through - // env itself by `godotenv`, even if db creds are not injected - if !dbCredsInjected { - return nil + // if db creds are injected, extract those + if dbCredsInjected { + return extractDBCredsFromSecret() } - - // if dbCreds is injected then extract the db creds - return extractDBCredsFromSecret() + // otherwise + return nil } // extractDBCredsFromSecret helper function to extract db secret diff --git a/internal/config/env_test.go b/internal/config/env_test.go index 7244d0d5..6de625a2 100644 --- a/internal/config/env_test.go +++ b/internal/config/env_test.go @@ -2,12 +2,12 @@ package config_test import ( "fmt" - . "go-template/internal/config" "os" "testing" - "time" "github.com/stretchr/testify/assert" + + . "go-template/internal/config" ) func TestGetString(t *testing.T) { @@ -42,7 +42,6 @@ func TestGetString(t *testing.T) { if tt.success { os.Setenv(tt.args.key, tt.want) } - time.Sleep(time.Microsecond) if got := GetString(tt.args.key); got != tt.want { t.Errorf("GetString() = %v, want %v", got, tt.want) @@ -499,7 +498,8 @@ func testLoadEnv(t *testing.T, tt struct { name string wantErr bool args args -}) { +}, +) { if err := LoadEnv(); (err != nil) != tt.wantErr { t.Errorf("LoadEnv() error = %v, wantErr %v", err, tt.wantErr) } else {