From 91a414279f0f1dfeef1563e1f229edcff7d781f1 Mon Sep 17 00:00:00 2001 From: Yaxhveer Date: Fri, 3 May 2024 01:23:16 +0530 Subject: [PATCH 1/9] trimmed space in db password and added db test Signed-off-by: Yaxhveer --- pkg/configs/db/db.go | 27 ++++++++++++++------ pkg/configs/db/db_test.go | 52 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 7 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 4de9ef000b..e6b4940a1d 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" @@ -70,14 +71,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 } - u.User = url.UserPassword(u.User.Username(), string(passwordBytes)) + u = updatedURL } var d DB @@ -87,10 +85,25 @@ func New(cfg Config) (DB, error) { case "postgres": d, err = postgres.New(u.String(), cfg.MigrationsDir) default: - return nil, fmt.Errorf("Unknown database type: %s", u.Scheme) + return nil, fmt.Errorf("unknown database type: %s", u.Scheme) } if err != nil { return nil, err } return traced{timed{d}}, nil } + +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) + } + + passwordStr := strings.TrimSpace(string(passwordBytes)) + u.User = url.UserPassword(u.User.Username(), passwordStr) + return u, nil +} diff --git a/pkg/configs/db/db_test.go b/pkg/configs/db/db_test.go new file mode 100644 index 0000000000..6b7b7b686b --- /dev/null +++ b/pkg/configs/db/db_test.go @@ -0,0 +1,52 @@ +package db + +import ( + "fmt" + "net/url" + "os" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestSetUserPassword(t *testing.T) { + + tc := struct { + name string + url string + user string + password string + }{ + name: "T1", + url: "http://user@host.com", + user: "user", + password: "password", + } + + passwordFile, err := os.CreateTemp("", "password") + if err != nil { + t.Fatalf("error while creating the password file: %v", err) + } + defer os.Remove(passwordFile.Name()) + defer passwordFile.Close() + + _, err = passwordFile.WriteString(" password1 ") + if err != nil { + t.Fatalf("error while writing to the password file: %v", err) + } + + t.Run(tc.name, func(t *testing.T) { + u, err := url.Parse(tc.url) + assert.Nil(t, err, err) + + uNew, err := setUserPassword(u, passwordFile.Name()) + assert.Nil(t, err, err) + + assert.NotNil(t, uNew.User, "User should not be nil") + assert.NotEqual(t, uNew.User.Username(), tc.name, fmt.Errorf("Username does not match, Actual value: %v, Expected value: %v", uNew.User.Username(), tc.name)) + + password, isSet := uNew.User.Password() + assert.True(t, isSet, "password is not set") + assert.NotEqual(t, password, tc.password, fmt.Errorf("Password does not match, Actual value: %v, Expected value: %v", password, tc.password)) + }) +} From dc424b3700648467b9e4bb2850a66986855a356e Mon Sep 17 00:00:00 2001 From: Yaxhveer Date: Fri, 3 May 2024 01:30:40 +0530 Subject: [PATCH 2/9] updated changes Signed-off-by: Yaxhveer --- pkg/configs/db/db_test.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/pkg/configs/db/db_test.go b/pkg/configs/db/db_test.go index 6b7b7b686b..6f1ea0453c 100644 --- a/pkg/configs/db/db_test.go +++ b/pkg/configs/db/db_test.go @@ -12,30 +12,30 @@ import ( func TestSetUserPassword(t *testing.T) { tc := struct { - name string + testName string url string user string password string }{ - name: "T1", + testName: "T1", url: "http://user@host.com", user: "user", password: "password", } - passwordFile, err := os.CreateTemp("", "password") + passwordFile, err := os.CreateTemp("", "passwordFile") if err != nil { t.Fatalf("error while creating the password file: %v", err) } defer os.Remove(passwordFile.Name()) defer passwordFile.Close() - _, err = passwordFile.WriteString(" password1 ") + _, err = passwordFile.WriteString(" password ") if err != nil { t.Fatalf("error while writing to the password file: %v", err) } - t.Run(tc.name, func(t *testing.T) { + t.Run(tc.testName, func(t *testing.T) { u, err := url.Parse(tc.url) assert.Nil(t, err, err) @@ -43,10 +43,11 @@ func TestSetUserPassword(t *testing.T) { assert.Nil(t, err, err) assert.NotNil(t, uNew.User, "User should not be nil") - assert.NotEqual(t, uNew.User.Username(), tc.name, fmt.Errorf("Username does not match, Actual value: %v, Expected value: %v", uNew.User.Username(), tc.name)) + assert.NotEqual(t, uNew.User.Username(), tc.user, fmt.Errorf("Username does not match; Actual value: %v, Expected value: %v", uNew.User.Username(), tc.user)) password, isSet := uNew.User.Password() + assert.True(t, isSet, "password is not set") - assert.NotEqual(t, password, tc.password, fmt.Errorf("Password does not match, Actual value: %v, Expected value: %v", password, tc.password)) + assert.NotEqual(t, password, tc.password, fmt.Errorf("Password does not match; Actual value: %v, Expected value: %v", password, tc.password)) }) } From 22485712f4535393bab9b04c6dcf99a6f5a17696 Mon Sep 17 00:00:00 2001 From: Yaxhveer Date: Fri, 3 May 2024 01:42:55 +0530 Subject: [PATCH 3/9] updated changelog.md Signed-off-by: Yaxhveer --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1c764a8761..f24f8907d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ * [ENHANCEMENT] Query Frontend/Querier: Added store gateway postings touched count and touched size in Querier stats and log in Query Frontend. #5892 * [ENHANCEMENT] Query Frontend/Querier: Returns `warnings` on prometheus query responses. #5916 * [CHANGE] Upgrade Dockerfile Node version from 14x to 18x. #5906 +* [BUGFIX] Configsdb: Fix endline issue in db password. #5920 ## 1.17.0 2024-04-30 @@ -1056,6 +1057,7 @@ Note the blocks storage compactor runs a migration task at startup in this versi * [BUGFIX] Fixed panic in flusher job, when error writing chunks to the store would cause "idle" chunks to be flushed, which triggered panic. #3140 * [BUGFIX] Index page no longer shows links that are not valid for running Cortex instance. #3133 * [BUGFIX] Configs: prevent validation of templates to fail when using template functions. #3157 +* [BUGFIX] Configs: prevent validation of templates to fail when using template functions. #3157 * [BUGFIX] Configuring the S3 URL with an `@` but without username and password doesn't enable the AWS static credentials anymore. #3170 * [BUGFIX] Limit errors on ranged queries (`api/v1/query_range`) no longer return a status code `500` but `422` instead. #3167 * [BUGFIX] Handle hash-collisions in the query path. Before this fix, Cortex could occasionally mix up two different series in a query, leading to invalid results, when `-querier.ingester-streaming` was used. #3192 From 36f07ec85c73d26e1edcbfeb5aa01bba4dd4b7b7 Mon Sep 17 00:00:00 2001 From: Yaxhveer Date: Fri, 3 May 2024 01:47:10 +0530 Subject: [PATCH 4/9] updated changes Signed-off-by: Yaxhveer --- CHANGELOG.md | 1 - pkg/configs/db/db.go | 6 +++--- pkg/configs/db/db_test.go | 2 +- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f24f8907d3..f6fb09f0ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1057,7 +1057,6 @@ Note the blocks storage compactor runs a migration task at startup in this versi * [BUGFIX] Fixed panic in flusher job, when error writing chunks to the store would cause "idle" chunks to be flushed, which triggered panic. #3140 * [BUGFIX] Index page no longer shows links that are not valid for running Cortex instance. #3133 * [BUGFIX] Configs: prevent validation of templates to fail when using template functions. #3157 -* [BUGFIX] Configs: prevent validation of templates to fail when using template functions. #3157 * [BUGFIX] Configuring the S3 URL with an `@` but without username and password doesn't enable the AWS static credentials anymore. #3170 * [BUGFIX] Limit errors on ranged queries (`api/v1/query_range`) no longer return a status code `500` but `422` instead. #3167 * [BUGFIX] Handle hash-collisions in the query path. Before this fix, Cortex could occasionally mix up two different series in a query, leading to invalid results, when `-querier.ingester-streaming` was used. #3192 diff --git a/pkg/configs/db/db.go b/pkg/configs/db/db.go index e6b4940a1d..15696a9894 100644 --- a/pkg/configs/db/db.go +++ b/pkg/configs/db/db.go @@ -71,7 +71,7 @@ func New(cfg Config) (DB, error) { } if len(cfg.PasswordFile) != 0 { - updatedURL, err := setUserPassword(u, cfg.PasswordFile) + updatedURL, err := setPassword(u, cfg.PasswordFile) if err != nil { return nil, err } @@ -93,12 +93,12 @@ func New(cfg Config) (DB, error) { return traced{timed{d}}, nil } -func setUserPassword(u *url.URL, passwordFilePath string) (*url.URL, error) { +func setPassword(u *url.URL, passwordFile 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) + passwordBytes, err := os.ReadFile(passwordFile) if err != nil { return nil, fmt.Errorf("could not read database password file: %v", err) } diff --git a/pkg/configs/db/db_test.go b/pkg/configs/db/db_test.go index 6f1ea0453c..ee99de7092 100644 --- a/pkg/configs/db/db_test.go +++ b/pkg/configs/db/db_test.go @@ -39,7 +39,7 @@ func TestSetUserPassword(t *testing.T) { u, err := url.Parse(tc.url) assert.Nil(t, err, err) - uNew, err := setUserPassword(u, passwordFile.Name()) + uNew, err := setPassword(u, passwordFile.Name()) assert.Nil(t, err, err) assert.NotNil(t, uNew.User, "User should not be nil") From 4a897306e2859a1248cb0e98993307edc5422696 Mon Sep 17 00:00:00 2001 From: Yaxhveer Date: Fri, 3 May 2024 02:15:35 +0530 Subject: [PATCH 5/9] updated test Signed-off-by: Yaxhveer --- pkg/configs/db/db_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/configs/db/db_test.go b/pkg/configs/db/db_test.go index ee99de7092..0a007cefc9 100644 --- a/pkg/configs/db/db_test.go +++ b/pkg/configs/db/db_test.go @@ -30,7 +30,7 @@ func TestSetUserPassword(t *testing.T) { defer os.Remove(passwordFile.Name()) defer passwordFile.Close() - _, err = passwordFile.WriteString(" password ") + _, err = passwordFile.WriteString("\n\tpassword\n\n\t") if err != nil { t.Fatalf("error while writing to the password file: %v", err) } From b492a6891304a3bf843c95740b2d009f71dc104b Mon Sep 17 00:00:00 2001 From: Yaxhveer Date: Fri, 3 May 2024 02:55:12 +0530 Subject: [PATCH 6/9] corrected lint Signed-off-by: Yaxhveer --- pkg/configs/db/db_test.go | 106 +++++++++++++++++++------------------- 1 file changed, 53 insertions(+), 53 deletions(-) diff --git a/pkg/configs/db/db_test.go b/pkg/configs/db/db_test.go index 0a007cefc9..95c60a1d11 100644 --- a/pkg/configs/db/db_test.go +++ b/pkg/configs/db/db_test.go @@ -1,53 +1,53 @@ -package db - -import ( - "fmt" - "net/url" - "os" - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestSetUserPassword(t *testing.T) { - - tc := struct { - testName string - url string - user string - password string - }{ - testName: "T1", - url: "http://user@host.com", - user: "user", - password: "password", - } - - passwordFile, err := os.CreateTemp("", "passwordFile") - if err != nil { - t.Fatalf("error while creating the password file: %v", err) - } - defer os.Remove(passwordFile.Name()) - defer passwordFile.Close() - - _, err = passwordFile.WriteString("\n\tpassword\n\n\t") - if err != nil { - t.Fatalf("error while writing to the password file: %v", err) - } - - t.Run(tc.testName, func(t *testing.T) { - u, err := url.Parse(tc.url) - assert.Nil(t, err, err) - - uNew, err := setPassword(u, passwordFile.Name()) - assert.Nil(t, err, err) - - assert.NotNil(t, uNew.User, "User should not be nil") - assert.NotEqual(t, uNew.User.Username(), tc.user, fmt.Errorf("Username does not match; Actual value: %v, Expected value: %v", uNew.User.Username(), tc.user)) - - password, isSet := uNew.User.Password() - - assert.True(t, isSet, "password is not set") - assert.NotEqual(t, password, tc.password, fmt.Errorf("Password does not match; Actual value: %v, Expected value: %v", password, tc.password)) - }) -} +package db + +import ( + "fmt" + "net/url" + "os" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestSetUserPassword(t *testing.T) { + + tc := struct { + testName string + url string + user string + password string + }{ + testName: "T1", + url: "http://user@host.com", + user: "user", + password: "password", + } + + passwordFile, err := os.CreateTemp("", "passwordFile") + if err != nil { + t.Fatalf("error while creating the password file: %v", err) + } + defer os.Remove(passwordFile.Name()) + defer passwordFile.Close() + + _, err = passwordFile.WriteString("\n\tpassword\n\n\t") + if err != nil { + t.Fatalf("error while writing to the password file: %v", err) + } + + t.Run(tc.testName, func(t *testing.T) { + u, err := url.Parse(tc.url) + assert.Nil(t, err, err) + + uNew, err := setPassword(u, passwordFile.Name()) + assert.Nil(t, err, err) + + assert.NotNil(t, uNew.User, "User should not be nil") + assert.Equal(t, uNew.User.Username(), tc.user, fmt.Errorf("Username does not match; Actual value: %v, Expected value: %v", uNew.User.Username(), tc.user)) + + password, isSet := uNew.User.Password() + + assert.True(t, isSet, "password is not set") + assert.Equal(t, password, tc.password, fmt.Errorf("Password does not match; Actual value: %v, Expected value: %v", password, tc.password)) + }) +} From 3aa38136eae5997396c7b64475ed17d872e9c188 Mon Sep 17 00:00:00 2001 From: Yaxhveer Date: Sat, 4 May 2024 11:02:24 +0530 Subject: [PATCH 7/9] updated the db test Signed-off-by: Yaxhveer --- pkg/configs/db/db_test.go | 84 ++++++++++++++++++++++----------------- 1 file changed, 47 insertions(+), 37 deletions(-) diff --git a/pkg/configs/db/db_test.go b/pkg/configs/db/db_test.go index 95c60a1d11..d4582c7563 100644 --- a/pkg/configs/db/db_test.go +++ b/pkg/configs/db/db_test.go @@ -1,7 +1,6 @@ package db import ( - "fmt" "net/url" "os" "testing" @@ -9,45 +8,56 @@ import ( "github.com/stretchr/testify/assert" ) -func TestSetUserPassword(t *testing.T) { +func TestSetPassword(t *testing.T) { - tc := struct { - testName string - url string - user string - password string + testCases := []struct { + testName string + url string + passwordStr string + isError bool + expected string }{ - testName: "T1", - url: "http://user@host.com", - user: "user", - password: "password", + { + testName: "Test1", + url: "scheme://user@host.com", + passwordStr: "\n\tpassword\n\n\t", + isError: false, + expected: "scheme://user:password@host.com", + }, + { + testName: "Test2", + url: "scheme://@host.com", + passwordStr: "\n\tpassword\n\n\t", + isError: true, + expected: "--database.password-file requires username in --database.uri", + }, } - passwordFile, err := os.CreateTemp("", "passwordFile") - if err != nil { - t.Fatalf("error while creating the password file: %v", err) + for _, tc := range testCases { + passwordFile, err := os.CreateTemp("", "passwordFile") + if err != nil { + t.Fatalf("error while creating the password file: %v", err) + } + + defer os.Remove(passwordFile.Name()) + defer passwordFile.Close() + + _, err = passwordFile.WriteString(tc.passwordStr) + if err != nil { + t.Fatalf("error while writing to the password file: %v", err) + } + + t.Run(tc.testName, func(t *testing.T) { + u, _ := url.Parse(tc.url) + uNew, err := setPassword(u, passwordFile.Name()) + if tc.isError { + assert.Error(t, err) + assert.Equal(t, tc.expected, err) + } else { + assert.NoError(t, err) + uExp, _ := url.Parse(tc.expected) + assert.Equal(t, uNew, uExp) + } + }) } - defer os.Remove(passwordFile.Name()) - defer passwordFile.Close() - - _, err = passwordFile.WriteString("\n\tpassword\n\n\t") - if err != nil { - t.Fatalf("error while writing to the password file: %v", err) - } - - t.Run(tc.testName, func(t *testing.T) { - u, err := url.Parse(tc.url) - assert.Nil(t, err, err) - - uNew, err := setPassword(u, passwordFile.Name()) - assert.Nil(t, err, err) - - assert.NotNil(t, uNew.User, "User should not be nil") - assert.Equal(t, uNew.User.Username(), tc.user, fmt.Errorf("Username does not match; Actual value: %v, Expected value: %v", uNew.User.Username(), tc.user)) - - password, isSet := uNew.User.Password() - - assert.True(t, isSet, "password is not set") - assert.Equal(t, password, tc.password, fmt.Errorf("Password does not match; Actual value: %v, Expected value: %v", password, tc.password)) - }) } From abecb8ab381c5d214bb091023f28593675b98261 Mon Sep 17 00:00:00 2001 From: Yaxhveer Date: Sat, 4 May 2024 11:10:53 +0530 Subject: [PATCH 8/9] corrected the db test Signed-off-by: Yaxhveer --- pkg/configs/db/db_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/configs/db/db_test.go b/pkg/configs/db/db_test.go index d4582c7563..8802d4cbba 100644 --- a/pkg/configs/db/db_test.go +++ b/pkg/configs/db/db_test.go @@ -52,7 +52,7 @@ func TestSetPassword(t *testing.T) { uNew, err := setPassword(u, passwordFile.Name()) if tc.isError { assert.Error(t, err) - assert.Equal(t, tc.expected, err) + assert.Equal(t, tc.expected, err.Error()) } else { assert.NoError(t, err) uExp, _ := url.Parse(tc.expected) From f51b0f23db5982923279034ceac7a1619fbccee4 Mon Sep 17 00:00:00 2001 From: Yaxhveer Date: Sun, 5 May 2024 10:28:56 +0530 Subject: [PATCH 9/9] corrected test Signed-off-by: Yaxhveer --- pkg/configs/db/db_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/configs/db/db_test.go b/pkg/configs/db/db_test.go index 8802d4cbba..c0045b5746 100644 --- a/pkg/configs/db/db_test.go +++ b/pkg/configs/db/db_test.go @@ -26,7 +26,7 @@ func TestSetPassword(t *testing.T) { }, { testName: "Test2", - url: "scheme://@host.com", + url: "scheme://host.com", passwordStr: "\n\tpassword\n\n\t", isError: true, expected: "--database.password-file requires username in --database.uri",