From dd43c6e38c210abb51d363a76bcbe97d4200294f Mon Sep 17 00:00:00 2001 From: David De Leon <56207066+davidadeleon@users.noreply.github.com> Date: Wed, 14 Aug 2024 07:26:48 -0400 Subject: [PATCH 1/6] skip connection verification on config read --- builtin/logical/database/backend.go | 14 +++++++++++++- .../logical/database/path_config_connection.go | 17 ++++++++++++----- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/builtin/logical/database/backend.go b/builtin/logical/database/backend.go index 970583ef850a..5e6bfada625e 100644 --- a/builtin/logical/database/backend.go +++ b/builtin/logical/database/backend.go @@ -291,6 +291,18 @@ func (b *databaseBackend) GetConnection(ctx context.Context, s logical.Storage, return b.GetConnectionWithConfig(ctx, name, config) } +func (b *databaseBackend) GetConnectionSkipVerify(ctx context.Context, s logical.Storage, name string) (*dbPluginInstance, error) { + config, err := b.DatabaseConfig(ctx, s, name) + if err != nil { + return nil, err + } + + // Force the skip verifying the connection + config.VerifyConnection = false + + return b.GetConnectionWithConfig(ctx, name, config) +} + func (b *databaseBackend) GetConnectionWithConfig(ctx context.Context, name string, config *DatabaseConfig) (*dbPluginInstance, error) { // fast path, reuse the existing connection dbi := b.connections.Get(name) @@ -331,7 +343,7 @@ func (b *databaseBackend) GetConnectionWithConfig(ctx context.Context, name stri initReq := v5.InitializeRequest{ Config: config.ConnectionDetails, - VerifyConnection: true, + VerifyConnection: config.VerifyConnection, } _, err = dbw.Initialize(ctx, initReq) if err != nil { diff --git a/builtin/logical/database/path_config_connection.go b/builtin/logical/database/path_config_connection.go index 2f8f40a1bf04..7ee608c2cd62 100644 --- a/builtin/logical/database/path_config_connection.go +++ b/builtin/logical/database/path_config_connection.go @@ -40,7 +40,8 @@ type DatabaseConfig struct { RootCredentialsRotateStatements []string `json:"root_credentials_rotate_statements" structs:"root_credentials_rotate_statements" mapstructure:"root_credentials_rotate_statements"` - PasswordPolicy string `json:"password_policy" structs:"password_policy" mapstructure:"password_policy"` + PasswordPolicy string `json:"password_policy" structs:"password_policy" mapstructure:"password_policy"` + VerifyConnection bool `json:"verify_connection" structs:"verify_connection" mapstructure:"verify_connection"` } func (c *DatabaseConfig) SupportsCredentialType(credentialType v5.CredentialType) bool { @@ -378,7 +379,7 @@ func (b *databaseBackend) connectionReadHandler() framework.OperationFunc { delete(config.ConnectionDetails, "service_account_json") resp := &logical.Response{} - if dbi, err := b.GetConnection(ctx, req.Storage, name); err == nil { + if dbi, err := b.GetConnectionSkipVerify(ctx, req.Storage, name); err == nil { config.RunningPluginVersion = dbi.runningPluginVersion if config.PluginVersion != "" && config.PluginVersion != config.RunningPluginVersion { warning := fmt.Sprintf("Plugin version is configured as %q, but running %q", config.PluginVersion, config.RunningPluginVersion) @@ -392,6 +393,7 @@ func (b *databaseBackend) connectionReadHandler() framework.OperationFunc { } resp.Data = structs.New(config).Map() + delete(resp.Data, "verify_connection") return resp, nil } } @@ -422,8 +424,6 @@ func (b *databaseBackend) connectionDeleteHandler() framework.OperationFunc { // both builtin and plugin database types. func (b *databaseBackend) connectionWriteHandler() framework.OperationFunc { return func(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { - verifyConnection := data.Get("verify_connection").(bool) - name := data.Get("name").(string) if name == "" { return logical.ErrorResponse(respErrEmptyName), nil @@ -442,6 +442,13 @@ func (b *databaseBackend) connectionWriteHandler() framework.OperationFunc { } } + // If this value was provided as part of the request we want to set it to this value + if verifyConnectionRaw, ok := data.GetOk("verify_connection"); ok { + config.VerifyConnection = verifyConnectionRaw.(bool) + } else if req.Operation == logical.CreateOperation { + config.VerifyConnection = data.Get("verify_connection").(bool) + } + if pluginNameRaw, ok := data.GetOk("plugin_name"); ok { config.PluginName = pluginNameRaw.(string) } else if req.Operation == logical.CreateOperation { @@ -509,7 +516,7 @@ func (b *databaseBackend) connectionWriteHandler() framework.OperationFunc { initReq := v5.InitializeRequest{ Config: config.ConnectionDetails, - VerifyConnection: verifyConnection, + VerifyConnection: config.VerifyConnection, } initResp, err := dbw.Initialize(ctx, initReq) if err != nil { From 08ecabd2dd437ce773ed0f3c4df409e5293b53f6 Mon Sep 17 00:00:00 2001 From: David De Leon <56207066+davidadeleon@users.noreply.github.com> Date: Wed, 21 Aug 2024 11:42:03 -0400 Subject: [PATCH 2/6] ensure appropriate default on config update call that results in a creation --- builtin/logical/database/path_config_connection.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/builtin/logical/database/path_config_connection.go b/builtin/logical/database/path_config_connection.go index 7ee608c2cd62..4532c71a79b6 100644 --- a/builtin/logical/database/path_config_connection.go +++ b/builtin/logical/database/path_config_connection.go @@ -430,7 +430,9 @@ func (b *databaseBackend) connectionWriteHandler() framework.OperationFunc { } // Baseline - config := &DatabaseConfig{} + config := &DatabaseConfig{ + VerifyConnection: true, + } entry, err := req.Storage.Get(ctx, fmt.Sprintf("config/%s", name)) if err != nil { From 585a9d7e5af416d14ff0c90bf047466937cea8db Mon Sep 17 00:00:00 2001 From: David De Leon <56207066+davidadeleon@users.noreply.github.com> Date: Wed, 21 Aug 2024 12:31:29 -0400 Subject: [PATCH 3/6] changelog --- changelog/28139.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog/28139.txt diff --git a/changelog/28139.txt b/changelog/28139.txt new file mode 100644 index 000000000000..f538ddf769dd --- /dev/null +++ b/changelog/28139.txt @@ -0,0 +1,3 @@ +```release-note:bug +secrets/database: Skip connection verification on reading existing DB connection configuration +``` \ No newline at end of file From e11b3194c2d096a06820681228e042fe9b7b746e Mon Sep 17 00:00:00 2001 From: David De Leon <56207066+davidadeleon@users.noreply.github.com> Date: Wed, 21 Aug 2024 13:19:36 -0400 Subject: [PATCH 4/6] leave verify_connection in config read response --- builtin/logical/database/path_config_connection.go | 1 - 1 file changed, 1 deletion(-) diff --git a/builtin/logical/database/path_config_connection.go b/builtin/logical/database/path_config_connection.go index 4532c71a79b6..0f373f371d74 100644 --- a/builtin/logical/database/path_config_connection.go +++ b/builtin/logical/database/path_config_connection.go @@ -393,7 +393,6 @@ func (b *databaseBackend) connectionReadHandler() framework.OperationFunc { } resp.Data = structs.New(config).Map() - delete(resp.Data, "verify_connection") return resp, nil } } From 9362981db5fb647aa249f6bfd29ef55f7310f211 Mon Sep 17 00:00:00 2001 From: David De Leon <56207066+davidadeleon@users.noreply.github.com> Date: Wed, 21 Aug 2024 14:46:50 -0400 Subject: [PATCH 5/6] update test to handle output of verify_connection parameter --- builtin/logical/database/backend_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/builtin/logical/database/backend_test.go b/builtin/logical/database/backend_test.go index f5ba7246fcbe..8dea00aba6da 100644 --- a/builtin/logical/database/backend_test.go +++ b/builtin/logical/database/backend_test.go @@ -764,6 +764,7 @@ func TestBackend_connectionCrud(t *testing.T) { "root_credentials_rotate_statements": []any{}, "password_policy": "", "plugin_version": "", + "verify_connection": false, } resp, err = client.Read("database/config/plugin-test") if err != nil { From d2b93a48f33f334a2edc7918621f1c4fd2177808 Mon Sep 17 00:00:00 2001 From: David De Leon <56207066+davidadeleon@users.noreply.github.com> Date: Wed, 21 Aug 2024 15:48:30 -0400 Subject: [PATCH 6/6] fix remaining tests --- builtin/logical/database/backend_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/builtin/logical/database/backend_test.go b/builtin/logical/database/backend_test.go index 8dea00aba6da..1573d2146ac9 100644 --- a/builtin/logical/database/backend_test.go +++ b/builtin/logical/database/backend_test.go @@ -210,6 +210,7 @@ func TestBackend_config_connection(t *testing.T) { "root_credentials_rotate_statements": []string{}, "password_policy": "", "plugin_version": "", + "verify_connection": false, } configReq.Operation = logical.ReadOperation resp, err = b.HandleRequest(namespace.RootContext(nil), configReq) @@ -264,6 +265,7 @@ func TestBackend_config_connection(t *testing.T) { "root_credentials_rotate_statements": []string{}, "password_policy": "", "plugin_version": "", + "verify_connection": false, } configReq.Operation = logical.ReadOperation resp, err = b.HandleRequest(namespace.RootContext(nil), configReq) @@ -307,6 +309,7 @@ func TestBackend_config_connection(t *testing.T) { "root_credentials_rotate_statements": []string{}, "password_policy": "", "plugin_version": "", + "verify_connection": false, } configReq.Operation = logical.ReadOperation resp, err = b.HandleRequest(namespace.RootContext(nil), configReq)