Fixed db password file endline issue (#5920)

* trimmed space in db password and added db test Signed-off-by: Yaxhveer <[email protected]> * updated changes Signed-off-by: Yaxhveer <[email protected]> * updated changelog.md Signed-off-by: Yaxhveer <[email protected]> * updated changes Signed-off-by: Yaxhveer <[email protected]> * updated test Signed-off-by: Yaxhveer <[email protected]> * corrected lint Signed-off-by: Yaxhveer <[email protected]> * updated the db test Signed-off-by: Yaxhveer <[email protected]> * corrected the db test Signed-off-by: Yaxhveer <[email protected]> * corrected test Signed-off-by: Yaxhveer <[email protected]> --------- Signed-off-by: Yaxhveer <[email protected]>
cortexproject · May 6, 2024 · b12e0b2 · b12e0b2
1 parent a1309a7
commit b12e0b2
Show file tree

Hide file tree

Showing 3 changed files with 84 additions and 7 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,7 @@
 * [ENHANCEMENT] Query Frontend/Querier: Returns `warnings` on prometheus query responses. #5916
 * [ENHANCEMENT] Ingester: Allowing to configure `-blocks-storage.tsdb.head-compaction-interval` flag up to 30 min and add a jitter on the first head compaction. #5919
 * [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
 

diff --git 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 := setPassword(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 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(passwordFile)
+	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
@@ -0,0 +1,63 @@
+package db
+
+import (
+	"net/url"
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestSetPassword(t *testing.T) {
+
+	testCases := []struct {
+		testName    string
+		url         string
+		passwordStr string
+		isError     bool
+		expected    string
+	}{
+		{
+			testName:    "Test1",
+			url:         "scheme://[email protected]",
+			passwordStr: "\n\tpassword\n\n\t",
+			isError:     false,
+			expected:    "scheme://user:[email protected]",
+		},
+		{
+			testName:    "Test2",
+			url:         "scheme://host.com",
+			passwordStr: "\n\tpassword\n\n\t",
+			isError:     true,
+			expected:    "--database.password-file requires username in --database.uri",
+		},
+	}
+
+	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.Error())
+			} else {
+				assert.NoError(t, err)
+				uExp, _ := url.Parse(tc.expected)
+				assert.Equal(t, uNew, uExp)
+			}
+		})
+	}
+}