From b8d168cf8321dfca1893484690db6d58de4e463a Mon Sep 17 00:00:00 2001 From: Shashank Date: Sat, 5 Aug 2023 22:23:34 +0530 Subject: [PATCH 1/3] fixed password endline bug Signed-off-by: Shashank --- pkg/configs/db/db.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/configs/db/db.go b/pkg/configs/db/db.go index 4de9ef000bb..92f4fa974ec 100644 --- a/pkg/configs/db/db.go +++ b/pkg/configs/db/db.go @@ -6,6 +6,7 @@ import ( "fmt" "net/url" "os" + "strings" "github.com/cortexproject/cortex/pkg/configs/db/memory" "github.com/cortexproject/cortex/pkg/configs/db/postgres" @@ -77,7 +78,8 @@ func New(cfg Config) (DB, error) { if err != nil { return nil, fmt.Errorf("Could not read database password file: %v", err) } - u.User = url.UserPassword(u.User.Username(), string(passwordBytes)) + password := strings.TrimSpace(string(passwordBytes)) + u.User = url.UserPassword(u.User.Username(), password) } var d DB From e7fa481410c80d1460da50db61ef1220f7c3ea7f Mon Sep 17 00:00:00 2001 From: Shashank Date: Fri, 1 Dec 2023 21:09:44 +0530 Subject: [PATCH 2/3] added unit test Signed-off-by: Shashank --- pkg/configs/db/db.go | 25 +++++++++++++----- pkg/configs/db/dbtest/unit.go | 48 +++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 7 deletions(-) diff --git a/pkg/configs/db/db.go b/pkg/configs/db/db.go index 92f4fa974ec..f7d6565c847 100644 --- a/pkg/configs/db/db.go +++ b/pkg/configs/db/db.go @@ -59,6 +59,21 @@ type DB interface { Close() error } +func SetUserPassword(u *url.URL, passwordFilePath string) (*url.URL, error) { + if u.User == nil { + return nil, fmt.Errorf("--database.password-file requires username in --database.uri") + } + + passwordBytes, err := os.ReadFile(passwordFilePath) + if err != nil { + return nil, fmt.Errorf("Could not read database password file: %v", err) + } + + password := strings.TrimSpace(string(passwordBytes)) + u.User = url.UserPassword(u.User.Username(), password) + return u, nil +} + // New creates a new database. func New(cfg Config) (DB, error) { if cfg.Mock != nil { @@ -71,15 +86,11 @@ func New(cfg Config) (DB, error) { } if len(cfg.PasswordFile) != 0 { - if u.User == nil { - return nil, fmt.Errorf("--database.password-file requires username in --database.uri") - } - passwordBytes, err := os.ReadFile(cfg.PasswordFile) + updatedUrl, err := SetUserPassword(u, cfg.PasswordFile) if err != nil { - return nil, fmt.Errorf("Could not read database password file: %v", err) + return nil, err } - password := strings.TrimSpace(string(passwordBytes)) - u.User = url.UserPassword(u.User.Username(), password) + u = updatedUrl } var d DB diff --git a/pkg/configs/db/dbtest/unit.go b/pkg/configs/db/dbtest/unit.go index f5d7635b086..f5a291e3c0b 100644 --- a/pkg/configs/db/dbtest/unit.go +++ b/pkg/configs/db/dbtest/unit.go @@ -4,6 +4,9 @@ package dbtest import ( + "io/ioutil" + "net/url" + "os" "testing" "github.com/stretchr/testify/require" @@ -26,3 +29,48 @@ func Setup(t *testing.T) db.DB { func Cleanup(t *testing.T, database db.DB) { require.NoError(t, database.Close()) } + +func TestUserPasswordFromPasswordFile(t *testing.T) { + + tempFile, _ := ioutil.TempFile("", "testpassword") + defer tempFile.Close() + defer os.Remove(tempFile.Name()) + + _, writeErr := tempFile.WriteString(" testpassword ") + if writeErr != nil { + t.Fatalf("Error writing to temporary file: %v", writeErr) + } + + testCases := []struct { + testName string + url string + expectedUsername string + expectedPassword string + }{ + { + testName: "set username and password correctly from url and password file", + url: "testscheme://testuser@testhost.com", + expectedUsername: "testuser", + expectedPassword: "testpassword", + }, + } + + for _, tc := range testCases { + t.Run(tc.testName, func(t *testing.T) { + u, _ := url.Parse(tc.url) + + updatedUrl, err := db.SetUserPassword(u, tempFile.Name()) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + + if u.User == nil || updatedUrl.User.Username() != tc.expectedUsername { + t.Errorf("Username not set as expected. Got: %v, Expected: %v", updatedUrl.User.Username(), tc.expectedUsername) + } + + if trimmedPassword, _ := updatedUrl.User.Password(); trimmedPassword != tc.expectedPassword { + t.Errorf("Password not set as expected. Got: %v, Expected: %v", trimmedPassword, tc.expectedPassword) + } + }) + } +} From 82776842c91b94918a9521f9d8094ca40642bb9d Mon Sep 17 00:00:00 2001 From: Shashank Date: Sat, 30 Dec 2023 14:22:42 +0530 Subject: [PATCH 3/3] create seperate test file for changes Signed-off-by: Shashank --- pkg/configs/db/db.go | 4 +-- pkg/configs/db/db_test.go | 52 +++++++++++++++++++++++++++++++++++ pkg/configs/db/dbtest/unit.go | 47 ------------------------------- 3 files changed, 54 insertions(+), 49 deletions(-) create mode 100644 pkg/configs/db/db_test.go diff --git a/pkg/configs/db/db.go b/pkg/configs/db/db.go index f7d6565c847..809d1403c78 100644 --- a/pkg/configs/db/db.go +++ b/pkg/configs/db/db.go @@ -86,11 +86,11 @@ func New(cfg Config) (DB, error) { } if len(cfg.PasswordFile) != 0 { - updatedUrl, err := SetUserPassword(u, cfg.PasswordFile) + updatedURL, err := SetUserPassword(u, cfg.PasswordFile) if err != nil { return nil, err } - u = updatedUrl + u = updatedURL } var d DB diff --git a/pkg/configs/db/db_test.go b/pkg/configs/db/db_test.go new file mode 100644 index 00000000000..82d329428c5 --- /dev/null +++ b/pkg/configs/db/db_test.go @@ -0,0 +1,52 @@ +package db + +import ( + "net/url" + "os" + "testing" +) + +func TestUserPasswordFromPasswordFile(t *testing.T) { + + tempFile, _ := os.CreateTemp("", "testpassword") + defer os.Remove(tempFile.Name()) + defer tempFile.Close() + + _, writeErr := tempFile.WriteString(" testpassword ") + if writeErr != nil { + t.Fatalf("Error writing to temporary file: %v", writeErr) + } + + testCases := []struct { + testName string + url string + expectedUsername string + expectedPassword string + }{ + { + testName: "set username and password correctly from url and password file", + url: "testscheme://testuser@testhost.com", + expectedUsername: "testuser", + expectedPassword: "testpassword", + }, + } + + for _, tc := range testCases { + t.Run(tc.testName, func(t *testing.T) { + u, _ := url.Parse(tc.url) + + updatedURL, err := SetUserPassword(u, tempFile.Name()) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + + if u.User == nil || updatedURL.User.Username() != tc.expectedUsername { + t.Errorf("Username not set as expected. Got: %v, Expected: %v", updatedURL.User.Username(), tc.expectedUsername) + } + + if trimmedPassword, _ := updatedURL.User.Password(); trimmedPassword != tc.expectedPassword { + t.Errorf("Password not set as expected. Got: %v, Expected: %v", trimmedPassword, tc.expectedPassword) + } + }) + } +} \ No newline at end of file diff --git a/pkg/configs/db/dbtest/unit.go b/pkg/configs/db/dbtest/unit.go index f5a291e3c0b..cf8d28adf8f 100644 --- a/pkg/configs/db/dbtest/unit.go +++ b/pkg/configs/db/dbtest/unit.go @@ -4,9 +4,6 @@ package dbtest import ( - "io/ioutil" - "net/url" - "os" "testing" "github.com/stretchr/testify/require" @@ -30,47 +27,3 @@ func Cleanup(t *testing.T, database db.DB) { require.NoError(t, database.Close()) } -func TestUserPasswordFromPasswordFile(t *testing.T) { - - tempFile, _ := ioutil.TempFile("", "testpassword") - defer tempFile.Close() - defer os.Remove(tempFile.Name()) - - _, writeErr := tempFile.WriteString(" testpassword ") - if writeErr != nil { - t.Fatalf("Error writing to temporary file: %v", writeErr) - } - - testCases := []struct { - testName string - url string - expectedUsername string - expectedPassword string - }{ - { - testName: "set username and password correctly from url and password file", - url: "testscheme://testuser@testhost.com", - expectedUsername: "testuser", - expectedPassword: "testpassword", - }, - } - - for _, tc := range testCases { - t.Run(tc.testName, func(t *testing.T) { - u, _ := url.Parse(tc.url) - - updatedUrl, err := db.SetUserPassword(u, tempFile.Name()) - if err != nil { - t.Errorf("Unexpected error: %v", err) - } - - if u.User == nil || updatedUrl.User.Username() != tc.expectedUsername { - t.Errorf("Username not set as expected. Got: %v, Expected: %v", updatedUrl.User.Username(), tc.expectedUsername) - } - - if trimmedPassword, _ := updatedUrl.User.Password(); trimmedPassword != tc.expectedPassword { - t.Errorf("Password not set as expected. Got: %v, Expected: %v", trimmedPassword, tc.expectedPassword) - } - }) - } -}