From 52641ae66f895be3a2f5d990fe4386c401c6e9f0 Mon Sep 17 00:00:00 2001 From: Colton McCurdy Date: Tue, 26 Nov 2019 06:57:11 -0500 Subject: [PATCH 1/9] physical/posgresql: add ability to use CONNECTION_URL environment variable instead of requiring it to be configured in the Vault config file. Signed-off-by: Colton McCurdy --- physical/postgresql/postgresql.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/physical/postgresql/postgresql.go b/physical/postgresql/postgresql.go index dbd3a0d2ef77..a625584465b3 100644 --- a/physical/postgresql/postgresql.go +++ b/physical/postgresql/postgresql.go @@ -4,6 +4,7 @@ import ( "context" "database/sql" "fmt" + "os" "strconv" "strings" "sync" @@ -88,9 +89,12 @@ type PostgreSQLLock struct { // API client, server address, credentials, and database. func NewPostgreSQLBackend(conf map[string]string, logger log.Logger) (physical.Backend, error) { // Get the PostgreSQL credentials to perform read/write operations. - connURL, ok := conf["connection_url"] - if !ok || connURL == "" { - return nil, fmt.Errorf("missing connection_url") + connURL := os.Getenv("CONNECTION_URL") + if connURL == "" { + connURL = conf["connection_url"] + if connURL == "" { + return nil, fmt.Errorf("missing connection_url") + } } unquoted_table, ok := conf["table"] From d648e88938ce4aa7794485ba0b41fc48ff831357 Mon Sep 17 00:00:00 2001 From: Colton McCurdy Date: Tue, 26 Nov 2019 08:11:30 -0500 Subject: [PATCH 2/9] storage/postgresql: update configuration documentation for postgresql storage backend to include connection_url configuration via the PG_CONNECTION_URL environment variable Signed-off-by: Colton McCurdy --- website/source/docs/configuration/storage/postgresql.html.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/website/source/docs/configuration/storage/postgresql.html.md b/website/source/docs/configuration/storage/postgresql.html.md index 8ee819520efa..3942a28b3e86 100644 --- a/website/source/docs/configuration/storage/postgresql.html.md +++ b/website/source/docs/configuration/storage/postgresql.html.md @@ -93,7 +93,8 @@ LANGUAGE plpgsql; ## `postgresql` Parameters - `connection_url` `(string: )` – Specifies the connection string to - use to authenticate and connect to PostgreSQL. A full list of supported + use to authenticate and connect to PostgreSQL. The connection URL can also be + set using the `PG_CONNECTION_URL` environment variable. A full list of supported parameters can be found in [the pq library documentation][pglib]. For example connection string URLs, see the examples section below. From 7a24748d141ddffe03fffbf72310ed0ddbc3796f Mon Sep 17 00:00:00 2001 From: Colton McCurdy Date: Tue, 26 Nov 2019 08:12:18 -0500 Subject: [PATCH 3/9] physical/postgresql: add a configuration file and tests for getting the connection_url from the config file or environment Signed-off-by: Colton McCurdy --- physical/postgresql/configure.go | 21 +++++++++ physical/postgresql/configure_test.go | 65 +++++++++++++++++++++++++++ 2 files changed, 86 insertions(+) create mode 100644 physical/postgresql/configure.go create mode 100644 physical/postgresql/configure_test.go diff --git a/physical/postgresql/configure.go b/physical/postgresql/configure.go new file mode 100644 index 000000000000..64967939adb6 --- /dev/null +++ b/physical/postgresql/configure.go @@ -0,0 +1,21 @@ +package postgresql + +import ( + "os" +) + +// connectionURL first check the environment variables for a connection URL. If +// no connection URL exists in the environment variable, the Vault config file is +// checked. If neither the environment variables or the config file set the connection +// URL for the Postgres backend, because it is a required field, an error is returned. +func connectionURL(conf map[string]string) string { + connURL := os.Getenv("PG_CONNECTION_URL") + if connURL == "" { + connURL = conf["connection_url"] + if connURL == "" { + return "" + } + } + + return connURL +} diff --git a/physical/postgresql/configure_test.go b/physical/postgresql/configure_test.go new file mode 100644 index 000000000000..85aae2745112 --- /dev/null +++ b/physical/postgresql/configure_test.go @@ -0,0 +1,65 @@ +package postgresql + +import ( + "os" + "testing" +) + +func TestConnectionURL(t *testing.T) { + type input struct { + envar string + conf map[string]string + } + + var cases = []struct { + name string + want string + input input + }{ + { + name: "environment_variable_not_set_use_config_value", + want: "abc", + input: input{ + envar: "", + conf: map[string]string{"connection_url": "abc"}, + }, + }, + { + name: "no_value_connection_url_set_key_exists", + want: "", + input: input{ + envar: "", + conf: map[string]string{"connection_url": ""}, + }, + }, + { + name: "no_value_connection_url_set_key_doesnt_exist", + want: "", + input: input{ + envar: "", + conf: map[string]string{}, + }, + }, + { + name: "environment_variable_set", + want: "abc", + input: input{ + envar: "abc", + conf: map[string]string{"connection_url": "def"}, + }, + }, + } + + for _, tt := range cases { + t.Run(tt.name, func(t *testing.T) { + os.Setenv("PG_CONNECTION_URL", tt.input.envar) + defer os.Setenv("PG_CONNECTION_URL", "") + + got := connectionURL(tt.input.conf) + + if got != tt.want { + t.Errorf("connectionURL(%s): want '%s', got '%s'", tt.input, tt.want, got) + } + }) + } +} From ac54bca6b5e7bfcbefac0001c78a1dd2ce03b2be Mon Sep 17 00:00:00 2001 From: Colton McCurdy Date: Tue, 26 Nov 2019 08:13:24 -0500 Subject: [PATCH 4/9] physical/postgresql: update postgresql backend to pull the required connection_url from the PG_CONNECTION_URL environment variable if it exists, otherwise, fallback to using the config file Signed-off-by: Colton McCurdy --- physical/postgresql/postgresql.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/physical/postgresql/postgresql.go b/physical/postgresql/postgresql.go index a625584465b3..d0a5fffb4640 100644 --- a/physical/postgresql/postgresql.go +++ b/physical/postgresql/postgresql.go @@ -4,7 +4,6 @@ import ( "context" "database/sql" "fmt" - "os" "strconv" "strings" "sync" @@ -89,12 +88,9 @@ type PostgreSQLLock struct { // API client, server address, credentials, and database. func NewPostgreSQLBackend(conf map[string]string, logger log.Logger) (physical.Backend, error) { // Get the PostgreSQL credentials to perform read/write operations. - connURL := os.Getenv("CONNECTION_URL") + connURL := connectionURL(conf) if connURL == "" { - connURL = conf["connection_url"] - if connURL == "" { - return nil, fmt.Errorf("missing connection_url") - } + return nil, fmt.Errorf("missing connection_url") } unquoted_table, ok := conf["table"] From 65156d594443ee603c564c24db8e99469ad1bb8a Mon Sep 17 00:00:00 2001 From: Colton McCurdy Date: Wed, 27 Nov 2019 06:44:00 -0500 Subject: [PATCH 5/9] physical/postgresql: remove configure*.go files and prefer the postgresql*.go files Signed-off-by: Colton McCurdy --- physical/postgresql/configure.go | 21 --------- physical/postgresql/configure_test.go | 65 --------------------------- 2 files changed, 86 deletions(-) delete mode 100644 physical/postgresql/configure.go delete mode 100644 physical/postgresql/configure_test.go diff --git a/physical/postgresql/configure.go b/physical/postgresql/configure.go deleted file mode 100644 index 64967939adb6..000000000000 --- a/physical/postgresql/configure.go +++ /dev/null @@ -1,21 +0,0 @@ -package postgresql - -import ( - "os" -) - -// connectionURL first check the environment variables for a connection URL. If -// no connection URL exists in the environment variable, the Vault config file is -// checked. If neither the environment variables or the config file set the connection -// URL for the Postgres backend, because it is a required field, an error is returned. -func connectionURL(conf map[string]string) string { - connURL := os.Getenv("PG_CONNECTION_URL") - if connURL == "" { - connURL = conf["connection_url"] - if connURL == "" { - return "" - } - } - - return connURL -} diff --git a/physical/postgresql/configure_test.go b/physical/postgresql/configure_test.go deleted file mode 100644 index 85aae2745112..000000000000 --- a/physical/postgresql/configure_test.go +++ /dev/null @@ -1,65 +0,0 @@ -package postgresql - -import ( - "os" - "testing" -) - -func TestConnectionURL(t *testing.T) { - type input struct { - envar string - conf map[string]string - } - - var cases = []struct { - name string - want string - input input - }{ - { - name: "environment_variable_not_set_use_config_value", - want: "abc", - input: input{ - envar: "", - conf: map[string]string{"connection_url": "abc"}, - }, - }, - { - name: "no_value_connection_url_set_key_exists", - want: "", - input: input{ - envar: "", - conf: map[string]string{"connection_url": ""}, - }, - }, - { - name: "no_value_connection_url_set_key_doesnt_exist", - want: "", - input: input{ - envar: "", - conf: map[string]string{}, - }, - }, - { - name: "environment_variable_set", - want: "abc", - input: input{ - envar: "abc", - conf: map[string]string{"connection_url": "def"}, - }, - }, - } - - for _, tt := range cases { - t.Run(tt.name, func(t *testing.T) { - os.Setenv("PG_CONNECTION_URL", tt.input.envar) - defer os.Setenv("PG_CONNECTION_URL", "") - - got := connectionURL(tt.input.conf) - - if got != tt.want { - t.Errorf("connectionURL(%s): want '%s', got '%s'", tt.input, tt.want, got) - } - }) - } -} From e4726b108ef285078c702cd4eeb8d6f3c44af636 Mon Sep 17 00:00:00 2001 From: Colton McCurdy Date: Wed, 27 Nov 2019 06:44:54 -0500 Subject: [PATCH 6/9] physical/postgresql: move and simplify connectionURL function Signed-off-by: Colton McCurdy --- physical/postgresql/postgresql.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/physical/postgresql/postgresql.go b/physical/postgresql/postgresql.go index d0a5fffb4640..f7a741b2d70b 100644 --- a/physical/postgresql/postgresql.go +++ b/physical/postgresql/postgresql.go @@ -4,6 +4,7 @@ import ( "context" "database/sql" "fmt" + "os" "strconv" "strings" "sync" @@ -197,6 +198,19 @@ func NewPostgreSQLBackend(conf map[string]string, logger log.Logger) (physical.B return m, nil } +// connectionURL first check the environment variables for a connection URL. If +// no connection URL exists in the environment variable, the Vault config file is +// checked. If neither the environment variables or the config file set the connection +// URL for the Postgres backend, because it is a required field, an error is returned. +func connectionURL(conf map[string]string) string { + connURL := conf["connection_url"] + if envURL := os.Getenv("PG_CONNECTION_URL"); envURL != "" { + connURL = envURL + } + + return connURL +} + // splitKey is a helper to split a full path key into individual // parts: parentPath, path, key func (m *PostgreSQLBackend) splitKey(fullPath string) (string, string, string) { From 1cfbef4aca565255165a566fbdcbb141fd6a52bb Mon Sep 17 00:00:00 2001 From: Colton McCurdy Date: Wed, 27 Nov 2019 06:45:45 -0500 Subject: [PATCH 7/9] physical/postgresql: update connectionURL test to use an unordered map instead of slice to avoid test flakiness Signed-off-by: Colton McCurdy --- physical/postgresql/postgresql_test.go | 61 ++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/physical/postgresql/postgresql_test.go b/physical/postgresql/postgresql_test.go index 97d42d084276..6d7d7c35f31c 100644 --- a/physical/postgresql/postgresql_test.go +++ b/physical/postgresql/postgresql_test.go @@ -114,6 +114,67 @@ func TestPostgreSQLBackendMaxIdleConnectionsParameter(t *testing.T) { } } +func TestConnectionURL(t *testing.T) { + type input struct { + envar string + conf map[string]string + } + + var cases = map[string]struct { + want string + input input + }{ + "environment_variable_not_set_use_config_value": { + want: "abc", + input: input{ + envar: "", + conf: map[string]string{"connection_url": "abc"}, + }, + }, + + "no_value_connection_url_set_key_exists": { + want: "", + input: input{ + envar: "", + conf: map[string]string{"connection_url": ""}, + }, + }, + + "no_value_connection_url_set_key_doesnt_exist": { + want: "", + input: input{ + envar: "", + conf: map[string]string{}, + }, + }, + + "environment_variable_set": { + want: "abc", + input: input{ + envar: "abc", + conf: map[string]string{"connection_url": "def"}, + }, + }, + } + + for name, tt := range cases { + t.Run(name, func(t *testing.T) { + // This is necessary to avoid always testing the branch where the env is set. + // As long the the env is set --- even if the value is "" --- `ok` returns true. + if tt.input.envar != "" { + os.Setenv("PG_CONNECTION_URL", tt.input.envar) + defer os.Setenv("PG_CONNECTION_URL", "") + } + + got := connectionURL(tt.input.conf) + + if got != tt.want { + t.Errorf("connectionURL(%s): want '%s', got '%s'", tt.input, tt.want, got) + } + }) + } +} + // Similar to testHABackend, but using internal implementation details to // trigger the lock failure scenario by setting the lock renew period for one // of the locks to a higher value than the lock TTL. From 93dd7945b1ed671f099299046cc674ad07fbc424 Mon Sep 17 00:00:00 2001 From: Colton McCurdy Date: Tue, 3 Dec 2019 06:18:40 -0500 Subject: [PATCH 8/9] physical/postgresql: update config env to be prefixed with VAULT_ - VAULT_PG_CONNECTION_URL Signed-off-by: Colton McCurdy --- physical/postgresql/postgresql.go | 2 +- physical/postgresql/postgresql_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/physical/postgresql/postgresql.go b/physical/postgresql/postgresql.go index f7a741b2d70b..596b6c519bea 100644 --- a/physical/postgresql/postgresql.go +++ b/physical/postgresql/postgresql.go @@ -204,7 +204,7 @@ func NewPostgreSQLBackend(conf map[string]string, logger log.Logger) (physical.B // URL for the Postgres backend, because it is a required field, an error is returned. func connectionURL(conf map[string]string) string { connURL := conf["connection_url"] - if envURL := os.Getenv("PG_CONNECTION_URL"); envURL != "" { + if envURL := os.Getenv("VAULT_PG_CONNECTION_URL"); envURL != "" { connURL = envURL } diff --git a/physical/postgresql/postgresql_test.go b/physical/postgresql/postgresql_test.go index de0e0e4bcc41..759e1a0673b1 100644 --- a/physical/postgresql/postgresql_test.go +++ b/physical/postgresql/postgresql_test.go @@ -162,8 +162,8 @@ func TestConnectionURL(t *testing.T) { // This is necessary to avoid always testing the branch where the env is set. // As long the the env is set --- even if the value is "" --- `ok` returns true. if tt.input.envar != "" { - os.Setenv("PG_CONNECTION_URL", tt.input.envar) - defer os.Setenv("PG_CONNECTION_URL", "") + os.Setenv("VAULT_PG_CONNECTION_URL", tt.input.envar) + defer os.Unsetenv("VAULT_PG_CONNECTION_URL") } got := connectionURL(tt.input.conf) From 386c79fceb912da6b1bdd0acabfb379b754c271a Mon Sep 17 00:00:00 2001 From: Colton McCurdy Date: Tue, 3 Dec 2019 06:19:43 -0500 Subject: [PATCH 9/9] docs/web: update postgresql backend docs to use updated, VAULT_ prefixed config env Signed-off-by: Colton McCurdy --- website/source/docs/configuration/storage/postgresql.html.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/source/docs/configuration/storage/postgresql.html.md b/website/source/docs/configuration/storage/postgresql.html.md index 3942a28b3e86..59c192cbfbf8 100644 --- a/website/source/docs/configuration/storage/postgresql.html.md +++ b/website/source/docs/configuration/storage/postgresql.html.md @@ -94,7 +94,7 @@ LANGUAGE plpgsql; - `connection_url` `(string: )` – Specifies the connection string to use to authenticate and connect to PostgreSQL. The connection URL can also be - set using the `PG_CONNECTION_URL` environment variable. A full list of supported + set using the `VAULT_PG_CONNECTION_URL` environment variable. A full list of supported parameters can be found in [the pq library documentation][pglib]. For example connection string URLs, see the examples section below.