diff --git a/builtin/logical/database/path_config_connection.go b/builtin/logical/database/path_config_connection.go index 283d8f054aaf..2fe02cfefea7 100644 --- a/builtin/logical/database/path_config_connection.go +++ b/builtin/logical/database/path_config_connection.go @@ -9,7 +9,6 @@ import ( "fmt" "net/url" "sort" - "strings" "github.com/fatih/structs" "github.com/hashicorp/go-uuid" @@ -173,6 +172,7 @@ func (b *databaseBackend) reloadPlugin() framework.OperationFunc { return nil, err } reloaded := []string{} + reloadFailed := []string{} for _, connName := range connNames { entry, err := req.Storage.Get(ctx, fmt.Sprintf("config/%s", connName)) if err != nil { @@ -188,15 +188,14 @@ func (b *databaseBackend) reloadPlugin() framework.OperationFunc { } if config.PluginName == pluginName { if err := b.reloadConnection(ctx, req.Storage, connName); err != nil { - var successfullyReloaded string - if len(reloaded) > 0 { - successfullyReloaded = fmt.Sprintf("successfully reloaded %d connection(s): %s; ", - len(reloaded), - strings.Join(reloaded, ", ")) - } - return nil, fmt.Errorf("%sfailed to reload connection %q: %w", successfullyReloaded, connName, err) + b.Logger().Error("failed to reload connection", "name", connName, "error", err) + b.dbEvent(ctx, "reload-connection-fail", req.Path, "", false, "name", connName) + reloadFailed = append(reloadFailed, connName) + } else { + b.Logger().Debug("reloaded connection", "name", connName) + b.dbEvent(ctx, "reload-connection", req.Path, "", true, "name", connName) + reloaded = append(reloaded, connName) } - reloaded = append(reloaded, connName) } } @@ -207,10 +206,12 @@ func (b *databaseBackend) reloadPlugin() framework.OperationFunc { }, } - if len(reloaded) == 0 { - resp.AddWarning(fmt.Sprintf("no connections were found with plugin_name %q", pluginName)) + if len(reloaded) > 0 { + b.dbEvent(ctx, "reload", req.Path, "", true, "plugin_name", pluginName) + } else if len(reloaded) == 0 && len(reloadFailed) == 0 { + b.Logger().Debug("no connections were found", "plugin_name", pluginName) } - b.dbEvent(ctx, "reload", req.Path, "", true, "plugin_name", pluginName) + return resp, nil } } diff --git a/changelog/29519.txt b/changelog/29519.txt new file mode 100644 index 000000000000..974b17668ae7 --- /dev/null +++ b/changelog/29519.txt @@ -0,0 +1,3 @@ +```release-note:bug +secrets/database: Fix a bug where a global database plugin reload exits if any of the database connections are not available +``` diff --git a/vault/external_tests/plugin/external_plugin_test.go b/vault/external_tests/plugin/external_plugin_test.go index ef508b44e136..63ef0792a4b1 100644 --- a/vault/external_tests/plugin/external_plugin_test.go +++ b/vault/external_tests/plugin/external_plugin_test.go @@ -817,56 +817,81 @@ func TestExternalPlugin_DatabaseReload(t *testing.T) { t.Fatal(err) } - dbName := fmt.Sprintf("%s-%d", plugin.Name, 0) - roleName := "test-role-" + dbName - - cleanupContainer, connURL := postgreshelper.PrepareTestContainerWithVaultUser(t, context.Background()) - t.Cleanup(cleanupContainer) + roleName := "test-role" + + // create 4 databases to create connections for + cleanupContainer0, connURL0 := postgreshelper.PrepareTestContainerWithVaultUser(t, context.Background()) + t.Cleanup(cleanupContainer0) + cleanupContainer1, connURL1 := postgreshelper.PrepareTestContainerWithVaultUser(t, context.Background()) + t.Cleanup(cleanupContainer1) + cleanupContainer2, connURL2 := postgreshelper.PrepareTestContainerWithVaultUser(t, context.Background()) + t.Cleanup(cleanupContainer2) + + var roles []string + // write the config and roles for the first 3 DBs + for i, url := range []string{connURL0, connURL1, connURL2} { + dbName := fmt.Sprintf("%s-%d", plugin.Name, i) + _, err := client.Logical().Write("database/config/"+dbName, map[string]interface{}{ + "connection_url": url, + "plugin_name": plugin.Name, + "allowed_roles": []string{"*"}, + "username": "vaultadmin", + "password": "vaultpass", + }) + require.NoError(t, err) - _, err := client.Logical().Write("database/config/"+dbName, map[string]interface{}{ - "connection_url": connURL, - "plugin_name": plugin.Name, - "allowed_roles": []string{roleName}, - "username": "vaultadmin", - "password": "vaultpass", - }) - if err != nil { - t.Fatal(err) + r := fmt.Sprintf("%s-%d", roleName, i) + roles = append(roles, r) + _, err = client.Logical().Write("database/roles/"+r, map[string]interface{}{ + "db_name": dbName, + "creation_statements": testRole, + "max_ttl": "10m", + }) + require.NoError(t, err) } - _, err = client.Logical().Write("database/roles/"+roleName, map[string]interface{}{ - "db_name": dbName, - "creation_statements": testRole, - "max_ttl": "10m", + // the 4th db connection has a bad connURL on purpose, it should not fail + // the plugin reload + _, err := client.Logical().Write("database/config/"+plugin.Name+"-3", map[string]interface{}{ + "connection_url": "foobar", + "verify_connection": false, // this db connection should not fail the plugin reload + "plugin_name": plugin.Name, + "allowed_roles": []string{"*"}, + "username": "vaultadmin", + "password": "vaultpass", }) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) - resp, err := client.Logical().Read("database/creds/" + roleName) - if err != nil { - t.Fatal(err) - } - if resp == nil { - t.Fatal("read creds response is nil") + for _, role := range roles { + resp, err := client.Logical().Read("database/creds/" + role) + require.NoError(t, err) + if resp == nil { + t.Fatal("read creds response is nil") + } } // Reload plugin - if _, err := client.Sys().ReloadPlugin(&api.ReloadPluginInput{ + _, err = client.Sys().ReloadPlugin(&api.ReloadPluginInput{ Plugin: plugin.Name, - }); err != nil { - t.Fatal(err) - } + }) + require.NoError(t, err) // Generate credentials after reload - resp, err = client.Logical().Read("database/creds/" + roleName) - if err != nil { - t.Fatal(err) - } - if resp == nil { - t.Fatal("read creds response is nil") + for _, role := range roles { + resp, err := client.Logical().Read("database/creds/" + role) + require.NoError(t, err) + if resp == nil { + t.Fatal("read creds response is nil") + } } + // remove one postgres database so that the plugin reload will fail + cleanupContainer1() + _, err = client.Sys().ReloadPlugin(&api.ReloadPluginInput{ + Plugin: plugin.Name, + }) + require.NoError(t, err) + if err := client.Sys().Unmount(plugin.Name); err != nil { t.Fatal(err) }