secret/database: ensure plugins are closed if they cannot be initialized #3768
Conversation
|
Shouldn't we call Database.Close() on Initialize errors for builtin database plugins as well? |
| func (dc *DatabasePluginClient) Initialize(ctx context.Context, config map[string]interface{}, verifyConnection bool) error { | ||
| err := dc.Database.Initialize(ctx, config, verifyConnection) | ||
| if err != nil { | ||
| dc.Close() |
There was a problem hiding this comment.
The database object in here might be nil if verify_connection was set to false (thus never calling Connection() to instantiate the object). dc.Close() -> dc.Database.Close() would be called on a nil db if the Initialize error comes from something else (e.g. https://github.com/hashicorp/vault/blob/master/plugins/helper/database/connutil/sql.go#L58).
There was a problem hiding this comment.
Isn't that something that should be handled by the Close() implementation in the database though?
There was a problem hiding this comment.
In fact isn't it handled already? https://github.com/hashicorp/vault/blob/master/plugins/helper/database/connutil/sql.go#L132
There was a problem hiding this comment.
Yeah the plugin implementation handles that. https://github.com/hashicorp/vault/blob/master/plugins/helper/database/connutil/sql.go#L127
There was a problem hiding this comment.
Ah, missed that. It's all good here!
No description provided.