Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DBPW 3/5] Add DBv5 plugin serving & management functions #9745

Merged
merged 1 commit into from
Sep 14, 2020

Conversation

pcman312
Copy link
Contributor

@pcman312 pcman312 commented Aug 17, 2020

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.

Adds plugin serving/handling code for Database v5. This is nearly identical to the Database v4 code, but references the updated interface.

Database v4 version:

Prerequisites

Related PRs

Original password policies PR
1/X - Database interface & gRPC
2/X - Middleware
4/X - Database engine

@pcman312 pcman312 changed the title Add DBv5 plugin serving & management functions [DBPW 3/X] Add DBv5 plugin serving & management functions Aug 17, 2020
Base automatically changed from dbpw-middleware to master September 1, 2020 16:46
This mirrors what DBv4 is doing, but with the updated interface
var _ plugin.GRPCPlugin = &GRPCDatabasePlugin{}

func (d GRPCDatabasePlugin) GRPCServer(_ *plugin.GRPCBroker, s *grpc.Server) error {
proto.RegisterDatabaseServer(s, gRPCServer{impl: d.Impl})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need to wrap d.Impl with DatabaseErrorSanitizerMiddleware?

Copy link
Contributor Author

@pcman312 pcman312 Sep 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately we can't at this point in the code. The DatabaseErrorSantizerMiddleware doesn't know what secret values to redact at this point. It only knows those values within the database plugin itself. Here's an example from the existing MongoDB plugin:

dbType := dbplugin.NewDatabaseErrorSanitizerMiddleware(db, db.secretValues)

func (c *mongoDBConnectionProducer) secretValues() map[string]interface{} {

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that in this case it's more of a passthrough/no-op in terms of sanitizing secret values, but sanitize is also performing an specific error check:

if errwrap.ContainsType(err, new(url.Error)) {
return errors.New("unable to parse connection url")
}
.

@pcman312 pcman312 merged commit 75b2f42 into master Sep 14, 2020
@pcman312 pcman312 deleted the dbpw-03-plugin branch September 14, 2020 22:04
@pcman312 pcman312 changed the title [DBPW 3/X] Add DBv5 plugin serving & management functions [DBPW 3/5] Add DBv5 plugin serving & management functions Oct 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants