Praefect should be able to connect directly to the PostgreSQL

In order to omit misunderstanding in configuration the old settings
remain without changes and to specify a direct connection a new
settings were added: host_no_proxy and port_no_proxy.
Connections should use old host and port settings as before without
changes. If there is a connection that requires a direct link to
database it should use new settings: host_no_proxy and port_no_proxy.

Closes: https://gitlab.com/gitlab-org/gitaly/-/issues/3119

5 files changed, 39 insertions, 59 deletions

diff --git a/internal/praefect/config/config.go b/internal/praefect/config/config.go
index cf63074ef..a06acf914 100644
--- a/internal/praefect/config/config.go
+++ b/internal/praefect/config/config.go
@@ -258,23 +258,21 @@ type DB struct {
 	SSLCert     string `toml:"sslcert"`
 	SSLKey      string `toml:"sslkey"`
 	SSLRootCert string `toml:"sslrootcert"`
-	ProxyHost   string `toml:"proxy_host"`
-	ProxyPort   int    `toml:"proxy_port"`
+	HostNoProxy string `toml:"host_no_proxy"`
+	PortNoProxy int    `toml:"port_no_proxy"`
 }
 
 // ToPQString returns a connection string that can be passed to github.com/lib/pq.
-func (db DB) ToPQString(useProxy bool) string {
-	hostVal := db.Host
-	portVal := db.Port
-
-	if useProxy {
-		if db.ProxyHost != "" {
-			hostVal = db.ProxyHost
-		}
-
-		if db.ProxyPort != 0 {
-			portVal = db.ProxyPort
-		}
+func (db DB) ToPQString(direct bool) string {
+	var hostVal string
+	var portVal int
+
+	if direct {
+		hostVal = db.HostNoProxy
+		portVal = db.PortNoProxy
+	} else {
+		hostVal = db.Host
+		portVal = db.Port
 	}
 
 	var fields []string
diff --git a/internal/praefect/config/config_test.go b/internal/praefect/config/config_test.go
index 053577f7a..be87c9f89 100644
--- a/internal/praefect/config/config_test.go
+++ b/internal/praefect/config/config_test.go
@@ -271,8 +271,8 @@ func TestConfigParsing(t *testing.T) {
 					SSLCert:     "/path/to/cert",
 					SSLKey:      "/path/to/key",
 					SSLRootCert: "/path/to/root-cert",
-					ProxyHost:   "2.3.4.5",
-					ProxyPort:   6432,
+					HostNoProxy: "2.3.4.5",
+					PortNoProxy: 6432,
 				},
 				MemoryQueueEnabled:  true,
 				GracefulStopTimeout: config.Duration(30 * time.Second),
@@ -360,14 +360,14 @@ func TestStorageNames(t *testing.T) {
 
 func TestToPQString(t *testing.T) {
 	testCases := []struct {
-		desc     string
-		in       DB
-		useProxy bool
-		out      string
+		desc   string
+		in     DB
+		direct bool
+		out    string
 	}{
 		{desc: "empty", in: DB{}, out: "binary_parameters=yes"},
 		{
-			desc: "basic example",
+			desc: "proxy connection",
 			in: DB{
 				Host:        "1.2.3.4",
 				Port:        2345,
@@ -379,32 +379,14 @@ func TestToPQString(t *testing.T) {
 				SSLKey:      "/path/to/key",
 				SSLRootCert: "/path/to/root-cert",
 			},
-			useProxy: false,
-			out:      `port=2345 host=1.2.3.4 user=praefect-user password=secret dbname=praefect_production sslmode=require sslcert=/path/to/cert sslkey=/path/to/key sslrootcert=/path/to/root-cert binary_parameters=yes`,
+			direct: false,
+			out:    `port=2345 host=1.2.3.4 user=praefect-user password=secret dbname=praefect_production sslmode=require sslcert=/path/to/cert sslkey=/path/to/key sslrootcert=/path/to/root-cert binary_parameters=yes`,
 		},
 		{
-			desc: "with proxy and set proxy values",
+			desc: "direct connection",
 			in: DB{
-				Host:        "1.2.3.4",
-				Port:        2345,
-				User:        "praefect-user",
-				Password:    "secret",
-				DBName:      "praefect_production",
-				SSLMode:     "require",
-				SSLCert:     "/path/to/cert",
-				SSLKey:      "/path/to/key",
-				SSLRootCert: "/path/to/root-cert",
-				ProxyHost:   "2.2.3.4",
-				ProxyPort:   1234,
-			},
-			useProxy: true,
-			out:      `port=1234 host=2.2.3.4 user=praefect-user password=secret dbname=praefect_production sslmode=require sslcert=/path/to/cert sslkey=/path/to/key sslrootcert=/path/to/root-cert binary_parameters=yes`,
-		},
-		{
-			desc: "with proxy and no proxy values set",
-			in: DB{
-				Host:        "1.2.3.4",
-				Port:        2345,
+				HostNoProxy: "1.2.3.4",
+				PortNoProxy: 2345,
 				User:        "praefect-user",
 				Password:    "secret",
 				DBName:      "praefect_production",
@@ -413,8 +395,8 @@ func TestToPQString(t *testing.T) {
 				SSLKey:      "/path/to/key",
 				SSLRootCert: "/path/to/root-cert",
 			},
-			useProxy: true,
-			out:      `port=2345 host=1.2.3.4 user=praefect-user password=secret dbname=praefect_production sslmode=require sslcert=/path/to/cert sslkey=/path/to/key sslrootcert=/path/to/root-cert binary_parameters=yes`,
+			direct: true,
+			out:    `port=2345 host=1.2.3.4 user=praefect-user password=secret dbname=praefect_production sslmode=require sslcert=/path/to/cert sslkey=/path/to/key sslrootcert=/path/to/root-cert binary_parameters=yes`,
 		},
 		{
 			desc: "with spaces and quotes",
@@ -427,7 +409,7 @@ func TestToPQString(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.desc, func(t *testing.T) {
-			require.Equal(t, tc.out, tc.in.ToPQString(tc.useProxy))
+			require.Equal(t, tc.out, tc.in.ToPQString(tc.direct))
 		})
 	}
 }
diff --git a/internal/praefect/config/testdata/config.toml b/internal/praefect/config/testdata/config.toml
index e5a6d712b..4fe025293 100644
--- a/internal/praefect/config/testdata/config.toml
+++ b/internal/praefect/config/testdata/config.toml
@@ -53,8 +53,8 @@ sslmode = "require"
 sslcert = "/path/to/cert"
 sslkey = "/path/to/key"
 sslrootcert = "/path/to/root-cert"
-proxy_host = "2.3.4.5"
-proxy_port = 6432
+host_no_proxy = "2.3.4.5"
+port_no_proxy = 6432
 
 [failover]
 error_threshold_window = "20s"
diff --git a/internal/praefect/datastore/glsql/postgres.go b/internal/praefect/datastore/glsql/postgres.go
index 83b56f9d2..4b474c179 100644
--- a/internal/praefect/datastore/glsql/postgres.go
+++ b/internal/praefect/datastore/glsql/postgres.go
@@ -15,7 +15,7 @@ import (
 
 // OpenDB returns connection pool to the database.
 func OpenDB(conf config.DB) (*sql.DB, error) {
-	db, err := sql.Open("postgres", conf.ToPQString(true))
+	db, err := sql.Open("postgres", conf.ToPQString(false))
 	if err != nil {
 		return nil, err
 	}
diff --git a/internal/praefect/datastore/glsql/testing.go b/internal/praefect/datastore/glsql/testing.go
index 4842ad4dd..44a1e02f9 100644
--- a/internal/praefect/datastore/glsql/testing.go
+++ b/internal/praefect/datastore/glsql/testing.go
@@ -117,18 +117,18 @@ func GetDBConfig(t testing.TB, database string) config.DB {
 
 	// connect to 'postgres' database first to re-create testing database from scratch
 	conf := config.DB{
-		Host:      host,
-		ProxyHost: host,
-		Port:      portNumber,
-		ProxyPort: portNumber,
-		DBName:    database,
-		SSLMode:   "disable",
-		User:      os.Getenv("PGUSER"),
+		Host:        host,
+		HostNoProxy: host,
+		Port:        portNumber,
+		PortNoProxy: portNumber,
+		DBName:      database,
+		SSLMode:     "disable",
+		User:        os.Getenv("PGUSER"),
 	}
 
 	bouncerHost, bouncerHostFound := os.LookupEnv("PGHOST_PGBOUNCER")
 	if bouncerHostFound {
-		conf.ProxyHost = bouncerHost
+		conf.Host = bouncerHost
 	}
 
 	bouncerPort, bouncerPortFound := os.LookupEnv("PGPORT_PGBOUNCER")
@@ -136,7 +136,7 @@ func GetDBConfig(t testing.TB, database string) config.DB {
 		bouncerPortNumber, pErr := strconv.Atoi(bouncerPort)
 		require.NoError(t, pErr, "PGPORT_PGBOUNCER must be a port number of the PgBouncer")
 
-		conf.ProxyPort = bouncerPortNumber
+		conf.Port = bouncerPortNumber
 	}
 
 	return conf