diff --git a/cmd/keeper/cmd/keeper.go b/cmd/keeper/cmd/keeper.go index cfe08ef76..b79fd1089 100644 --- a/cmd/keeper/cmd/keeper.go +++ b/cmd/keeper/cmd/keeper.go @@ -165,7 +165,7 @@ func readPasswordFromFile(filepath string) (string, error) { if err != nil { return "", fmt.Errorf("unable to read password from file %s: %v", filepath, err) } - return strings.TrimSpace(string(pwBytes)), nil + return string(pwBytes), nil } // walLevel returns the wal_level value to use. @@ -1806,16 +1806,6 @@ func keeper(c *cobra.Command, args []string) { log.Fatalf("only one of --pg-su-password or --pg-su-passwordfile must be provided") } - if cfg.pgSUUsername == cfg.pgReplUsername { - log.Warn("superuser name and replication user name are the same. Different users are suggested.") - if cfg.pgReplAuthMethod != cfg.pgSUAuthMethod { - log.Fatalf("do not support different auth methods when utilizing superuser for replication.") - } - if cfg.pgSUPassword != cfg.pgReplPassword && cfg.pgSUAuthMethod != "trust" && cfg.pgReplAuthMethod != "trust" { - log.Fatalf("provided superuser name and replication user name are the same but provided passwords are different") - } - } - if cfg.pgReplPasswordFile != "" { cfg.pgReplPassword, err = readPasswordFromFile(cfg.pgReplPasswordFile) if err != nil { @@ -1829,6 +1819,35 @@ func keeper(c *cobra.Command, args []string) { } } + // Trim trailing new lines from passwords + tp := strings.TrimRight(cfg.pgSUPassword, "\r\n") + if cfg.pgSUPassword != tp { + log.Warn("superuser password contain trailing new line, removing") + if tp == "" { + log.Fatalf("superuser password is empty after removing trailing new line") + } + cfg.pgSUPassword = tp + } + + tp = strings.TrimRight(cfg.pgReplPassword, "\r\n") + if cfg.pgReplPassword != tp { + log.Warn("replication user password contain trailing new line, removing") + if tp == "" { + log.Fatalf("replication user password is empty after removing trailing new line") + } + cfg.pgReplPassword = tp + } + + if cfg.pgSUUsername == cfg.pgReplUsername { + log.Warn("superuser name and replication user name are the same. Different users are suggested.") + if cfg.pgReplAuthMethod != cfg.pgSUAuthMethod { + log.Fatalf("do not support different auth methods when utilizing superuser for replication.") + } + if cfg.pgSUPassword != cfg.pgReplPassword && cfg.pgSUAuthMethod != "trust" && cfg.pgReplAuthMethod != "trust" { + log.Fatalf("provided superuser name and replication user name are the same but provided passwords are different") + } + } + // Open (and create if needed) the lock file. // There is no need to clean up this file since we don't use the file as an actual lock. We get a lock // on the file. So the lock get released when our process stops (or log.Fatalfs). diff --git a/tests/integration/init_test.go b/tests/integration/init_test.go index cae11ae46..95cab9a69 100644 --- a/tests/integration/init_test.go +++ b/tests/integration/init_test.go @@ -512,3 +512,90 @@ func TestExclusiveLock(t *testing.T) { t.Fatalf("expecting keeper reporting that failed to take an exclusive lock on data dir") } } + +func TestPasswordTrailingNewLine(t *testing.T) { + t.Parallel() + + dir, err := ioutil.TempDir("", "") + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + defer os.RemoveAll(dir) + + tstore, err := NewTestStore(t, dir) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if err := tstore.Start(); err != nil { + t.Fatalf("unexpected err: %v", err) + } + if err := tstore.WaitUp(10 * time.Second); err != nil { + t.Fatalf("error waiting on store up: %v", err) + } + storeEndpoints := fmt.Sprintf("%s:%s", tstore.listenAddress, tstore.port) + defer tstore.Stop() + + clusterName := uuid.NewV4().String() + + u := uuid.NewV4() + id := fmt.Sprintf("%x", u[:4]) + + pgSUPassword := "stolon_superuserpassword\n" + pgReplPassword := "stolon_replpassword\n" + + tk, err := NewTestKeeperWithID(t, dir, id, clusterName, pgSUUsername, pgSUPassword, pgReplUsername, pgReplPassword, tstore.storeBackend, storeEndpoints) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if err := tk.StartExpect(); err != nil { + t.Fatalf("unexpected err: %v", err) + } + + if err := tk.cmd.ExpectTimeout("superuser password contain trailing new line, removing", 30*time.Second); err != nil { + t.Fatalf(`expecting keeper reporting "superuser password contain trailing new line, removing"`) + } + if err := tk.cmd.ExpectTimeout("replication user password contain trailing new line, removing", 30*time.Second); err != nil { + t.Fatalf(`expecting keeper reporting "replication user password contain trailing new line, removing"`) + } + tk.Stop() + + pgSUPassword = "stolon_superuserpassword\n" + pgReplPassword = "\n" + + tk, err = NewTestKeeperWithID(t, dir, id, clusterName, pgSUUsername, pgSUPassword, pgReplUsername, pgReplPassword, tstore.storeBackend, storeEndpoints) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if err := tk.StartExpect(); err != nil { + t.Fatalf("unexpected err: %v", err) + } + + if err := tk.cmd.ExpectTimeout("replication user password contain trailing new line, removing", 30*time.Second); err != nil { + t.Fatalf(`expecting keeper reporting "replication user password contain trailing new line, removing"`) + } + if err := tk.cmd.ExpectTimeout("replication user password is empty after removing trailing new line", 30*time.Second); err != nil { + t.Fatalf(`expecting keeper reporting "replication user password is empty after removing trailing new line"`) + } + + tk.Stop() + + pgSUPassword = "\n" + pgReplPassword = "stolon_replpassword\n" + + tk, err = NewTestKeeperWithID(t, dir, id, clusterName, pgSUUsername, pgSUPassword, pgReplUsername, pgReplPassword, tstore.storeBackend, storeEndpoints) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if err := tk.StartExpect(); err != nil { + t.Fatalf("unexpected err: %v", err) + } + + if err := tk.cmd.ExpectTimeout("superuser password contain trailing new line, removing", 30*time.Second); err != nil { + t.Fatalf(`expecting keeper reporting "superuser password contain trailing new line, removing"`) + } + if err := tk.cmd.ExpectTimeout("superuser password is empty after removing trailing new line", 30*time.Second); err != nil { + t.Fatalf(`expecting keeper reporting "superuser password is empty after removing trailing new line"`) + } + + tk.Stop() +}