[DBPW 5/5] Use AutoMTLS with DB plugins#10008
Conversation
pcman312
requested review from
austingebauer,
briankassouf,
calvn and
tvoran
| cmd.Env = append(cmd.Env, fmt.Sprintf("%s=%s", PluginMetadataModeEnv, "false")) | ||
|
|
||
| // Get a CA TLS Certificate | ||
| certBytes, key, err := generateCert() |
There was a problem hiding this comment.
I think we want to avoid creating these certs and then response wrapping them if we are using AutoMTLS, right?
There was a problem hiding this comment.
It doesn't seem to make a difference in terms of using the plugin (i.e. it doesn't seem to break any of my tests). I can certainly make this conditional but I defer that decision that knows go-plugin better than I do.
For what it's worth, this is a copy-paste of the previous runCommon function, so nothing is changed here other than AutoMTLS and how it gets its values.
There was a problem hiding this comment.
The client should not set TLSConfig, nor should the server set a TLSProvider, because AutoMTLS implies that a new certificate and tls configuration will be generated at startup.
There was a problem hiding this comment.
Done. I also included some tests around the ClientConfig creation.
| } | ||
| } | ||
|
|
||
| func (r *PluginRunner) RunConfig(ctx context.Context, opts ...RunOpt) (*plugin.Client, error) { |
There was a problem hiding this comment.
Nit: This might have been derived from the runCommon terminology and to keep things as close as it was before, but it's really creating a plugin client so something like NewPluginClient would be more appropriate.
| return client, nil | ||
| } | ||
|
|
||
| type RunOpt func(*runConfig) |
There was a problem hiding this comment.
Nit: Can we rename this to RunnerOpt or Options (or ClientOpt, see below)? "Run" makes it sound like this is some sort of execution func, especially since this is a typed function.
pcman312
changed the title
3 participants
Overview
This PR is part of a larger feature adding support for password policies into the combined database engine. This feature is being split into multiple PRs to make for smaller reviews & earlier feedback.
Uses go-plugin's
AutoMTLSfeature for DB v5 plugins. This adds a new functionRunConfigto thepluginutilpackage alongsideRunandRunMetadataMode. This function follows the functional options pattern which will allow us to specify future changes to theRunConfigfunction (such as allowing the user to configure other fields) without actually changing the function signature.Related PRs
Original password policies PR
1/X - Database interface & gRPC
2/X - Middleware
3/X - Plugin management
4/X - DB engine