From c2e289700b7d8640b9ebfd752f0a31b7d5a7b80d Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang Date: Mon, 20 Nov 2017 14:50:33 -0500 Subject: [PATCH 1/5] Initial work on write concern support, set for the lifetime of the session --- plugins/database/mongodb/connection_producer.go | 16 ++++++++++++++++ plugins/database/mongodb/util.go | 10 ++++++++++ 2 files changed, 26 insertions(+) diff --git a/plugins/database/mongodb/connection_producer.go b/plugins/database/mongodb/connection_producer.go index f802dc35e5aa..43328240767b 100644 --- a/plugins/database/mongodb/connection_producer.go +++ b/plugins/database/mongodb/connection_producer.go @@ -2,6 +2,7 @@ package mongodb import ( "crypto/tls" + "encoding/json" "errors" "fmt" "net" @@ -21,6 +22,7 @@ import ( // interface for databases to make connections. type mongoDBConnectionProducer struct { ConnectionURL string `json:"connection_url" structs:"connection_url" mapstructure:"connection_url"` + WriteConcern string `json:"write_concern" structs:"write_concern" mapstructure:"write_concern"` Initialized bool Type string @@ -78,6 +80,20 @@ func (c *mongoDBConnectionProducer) Connection() (interface{}, error) { if err != nil { return nil, err } + + if c.WriteConcern != "" { + var concern writeConcern + json.Unmarshal([]byte(c.WriteConcern), &concern) + + c.session.SetSafe(&mgo.Safe{ + W: concern.W, + WMode: concern.WMode, + WTimeout: concern.WTimeout, + FSync: concern.FSync, + J: concern.J, + }) + } + c.session.SetSyncTimeout(1 * time.Minute) c.session.SetSocketTimeout(1 * time.Minute) diff --git a/plugins/database/mongodb/util.go b/plugins/database/mongodb/util.go index 9004a3c710c4..2424324f09b1 100644 --- a/plugins/database/mongodb/util.go +++ b/plugins/database/mongodb/util.go @@ -17,6 +17,16 @@ type mongoDBStatement struct { Roles mongodbRoles `json:"roles"` } +// writeConcern is the mgo.Safe struct with JSON tags +// More info: https://godoc.org/gopkg.in/mgo.v2#Safe +type writeConcern struct { + W int `json:"w"` // Min # of servers to ack before success + WMode string `json:"w_mode"` // Write mode for MongoDB 2.0+ (e.g. "majority") + WTimeout int `json:"wtimeout"` // Milliseconds to wait for W before timing out + FSync bool `json:"fsync"` // Sync via the journal if present, or via data files sync otherwise + J bool `json:"j"` // Sync via the journal if present +} + // Convert array of role documents like: // // [ { "role": "readWrite" }, { "role": "readWrite", "db": "test" } ] From 8807a2782081dacc96f88c555359414714df2b40 Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang Date: Mon, 4 Dec 2017 10:07:18 -0500 Subject: [PATCH 2/5] Add base64 encoded value support, include docs and tests --- .../database/mongodb/connection_producer.go | 12 +++++- plugins/database/mongodb/mongodb_test.go | 40 +++++++++++++++++++ .../api/secret/databases/mongodb.html.md | 21 +++++++--- 3 files changed, 67 insertions(+), 6 deletions(-) diff --git a/plugins/database/mongodb/connection_producer.go b/plugins/database/mongodb/connection_producer.go index 43328240767b..7ed1f22814b8 100644 --- a/plugins/database/mongodb/connection_producer.go +++ b/plugins/database/mongodb/connection_producer.go @@ -2,6 +2,7 @@ package mongodb import ( "crypto/tls" + "encoding/base64" "encoding/json" "errors" "fmt" @@ -82,8 +83,17 @@ func (c *mongoDBConnectionProducer) Connection() (interface{}, error) { } if c.WriteConcern != "" { + input := c.WriteConcern + + // Try to base64 decode the input. If successful, consider the decoded + // value as input. + inputBytes, err := base64.StdEncoding.DecodeString(input) + if err == nil { + input = string(inputBytes) + } + var concern writeConcern - json.Unmarshal([]byte(c.WriteConcern), &concern) + json.Unmarshal([]byte(input), &concern) c.session.SetSafe(&mgo.Safe{ W: concern.W, diff --git a/plugins/database/mongodb/mongodb_test.go b/plugins/database/mongodb/mongodb_test.go index 95f6e90888c3..75675283305d 100644 --- a/plugins/database/mongodb/mongodb_test.go +++ b/plugins/database/mongodb/mongodb_test.go @@ -16,6 +16,8 @@ import ( const testMongoDBRole = `{ "db": "admin", "roles": [ { "role": "readWrite" } ] }` +const testMongoDBWriteConcern = `{ "w": "majority", "wtimeout": 5000 }` + func prepareMongoDBTestContainer(t *testing.T) (cleanup func(), retURL string) { if os.Getenv("MONGODB_URL") != "" { return func() {}, os.Getenv("MONGODB_URL") @@ -129,6 +131,44 @@ func TestMongoDB_CreateUser(t *testing.T) { } } +func TestMongoDB_CreateUser_writeConcern(t *testing.T) { + cleanup, connURL := prepareMongoDBTestContainer(t) + defer cleanup() + + connectionDetails := map[string]interface{}{ + "connection_url": connURL, + "write_concern": testMongoDBWriteConcern, + } + + dbRaw, err := New() + if err != nil { + t.Fatalf("err: %s", err) + } + db := dbRaw.(*MongoDB) + err = db.Initialize(connectionDetails, true) + if err != nil { + t.Fatalf("err: %s", err) + } + + statements := dbplugin.Statements{ + CreationStatements: testMongoDBRole, + } + + usernameConfig := dbplugin.UsernameConfig{ + DisplayName: "test", + RoleName: "test", + } + + username, password, err := db.CreateUser(statements, usernameConfig, time.Now().Add(time.Minute)) + if err != nil { + t.Fatalf("err: %s", err) + } + + if err := testCredsExist(t, connURL, username, password); err != nil { + t.Fatalf("Could not connect with new credentials: %s", err) + } +} + func TestMongoDB_RevokeUser(t *testing.T) { cleanup, connURL := prepareMongoDBTestContainer(t) defer cleanup() diff --git a/website/source/api/secret/databases/mongodb.html.md b/website/source/api/secret/databases/mongodb.html.md index 48a8ae2c401d..afe2b8a1c8cd 100644 --- a/website/source/api/secret/databases/mongodb.html.md +++ b/website/source/api/secret/databases/mongodb.html.md @@ -20,10 +20,17 @@ has a number of parameters to further configure a connection. | Method | Path | Produces | | :------- | :--------------------------- | :--------------------- | -| `POST` | `/database/config/:name` | `204 (empty body)` | +| `POST` | `/database/config/:name` | `204 (empty body)` | ### Parameters -- `connection_url` `(string: )` – Specifies the MongoDB standard connection string (URI). + +- `connection_url` `(string: )` – Specifies the MongoDB standard + connection string (URI). +- `write_concern` `(string: "")` - Specifies the MongoDB [write + concern][mongodb-write-concern]. This is set for the entirety of the session, + maintained for the lifecycle of the plugin process. Must be a serialized JSON + object, or a base64-encoded serialized JSON object. The JSON payload values + map to the values in the [Safe][mgo-safe] struct from the mgo driver. ### Sample Payload @@ -31,7 +38,8 @@ has a number of parameters to further configure a connection. { "plugin_name": "mongodb-database-plugin", "allowed_roles": "readonly", - "connection_url": "mongodb://admin:Password!@mongodb.acme.com:27017/admin?ssl=true" + "connection_url": "mongodb://admin:Password!@mongodb.acme.com:27017/admin?ssl=true", + "write_concern": "{ \"w\": \"majority\", \"wtimeout\": 5000 }" } ``` @@ -68,7 +76,7 @@ list the plugin does not support that statement type. [MongoDB's documentation](https://docs.mongodb.com/manual/reference/method/db.createUser/). - `revocation_statements` `(string: "")` – Specifies the database statements to - be executed to revoke a user. Must be a serialized JSON object, or a base64-encoded + be executed to revoke a user. Must be a serialized JSON object, or a base64-encoded serialized JSON object. The object can optionally contain a "db" string. If no "db" value is provided, it defaults to the "admin" database. @@ -84,4 +92,7 @@ list the plugin does not support that statement type. } ] } -``` \ No newline at end of file +``` + +[mongodb-write-concern]: https://docs.mongodb.com/manual/reference/write-concern/ +[mgo-safe]: https://godoc.org/gopkg.in/mgo.v2#Safe \ No newline at end of file From 24f37a2402b336ead759fab6daa75e1d27c0f060 Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang Date: Mon, 4 Dec 2017 13:38:53 -0500 Subject: [PATCH 3/5] Handle error from json.Unmarshal, fix test and docs --- plugins/database/mongodb/connection_producer.go | 5 ++++- plugins/database/mongodb/mongodb_test.go | 2 +- website/source/api/secret/databases/mongodb.html.md | 2 +- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/plugins/database/mongodb/connection_producer.go b/plugins/database/mongodb/connection_producer.go index 7ed1f22814b8..d79f093cbdf2 100644 --- a/plugins/database/mongodb/connection_producer.go +++ b/plugins/database/mongodb/connection_producer.go @@ -93,7 +93,10 @@ func (c *mongoDBConnectionProducer) Connection() (interface{}, error) { } var concern writeConcern - json.Unmarshal([]byte(input), &concern) + err = json.Unmarshal([]byte(input), &concern) + if err != nil { + return nil, err + } c.session.SetSafe(&mgo.Safe{ W: concern.W, diff --git a/plugins/database/mongodb/mongodb_test.go b/plugins/database/mongodb/mongodb_test.go index 75675283305d..4c1eacb66cb5 100644 --- a/plugins/database/mongodb/mongodb_test.go +++ b/plugins/database/mongodb/mongodb_test.go @@ -16,7 +16,7 @@ import ( const testMongoDBRole = `{ "db": "admin", "roles": [ { "role": "readWrite" } ] }` -const testMongoDBWriteConcern = `{ "w": "majority", "wtimeout": 5000 }` +const testMongoDBWriteConcern = `{ "wmode": "majority", "wtimeout": 5000 }` func prepareMongoDBTestContainer(t *testing.T) (cleanup func(), retURL string) { if os.Getenv("MONGODB_URL") != "" { diff --git a/website/source/api/secret/databases/mongodb.html.md b/website/source/api/secret/databases/mongodb.html.md index afe2b8a1c8cd..0d4857b6828b 100644 --- a/website/source/api/secret/databases/mongodb.html.md +++ b/website/source/api/secret/databases/mongodb.html.md @@ -39,7 +39,7 @@ has a number of parameters to further configure a connection. "plugin_name": "mongodb-database-plugin", "allowed_roles": "readonly", "connection_url": "mongodb://admin:Password!@mongodb.acme.com:27017/admin?ssl=true", - "write_concern": "{ \"w\": \"majority\", \"wtimeout\": 5000 }" + "write_concern": "{ \"wmode\": \"majority\", \"wtimeout\": 5000 }" } ``` From 6934d5cf5c11f28163f06f4aef052f84add34345 Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang Date: Tue, 5 Dec 2017 14:11:50 -0500 Subject: [PATCH 4/5] Remove writeConcern struct, move JSON unmarshal to Initialize --- .../database/mongodb/connection_producer.go | 49 ++++++++++--------- plugins/database/mongodb/util.go | 10 ---- 2 files changed, 26 insertions(+), 33 deletions(-) diff --git a/plugins/database/mongodb/connection_producer.go b/plugins/database/mongodb/connection_producer.go index d79f093cbdf2..162283d7885f 100644 --- a/plugins/database/mongodb/connection_producer.go +++ b/plugins/database/mongodb/connection_producer.go @@ -28,6 +28,7 @@ type mongoDBConnectionProducer struct { Initialized bool Type string session *mgo.Session + safe *mgo.Safe sync.Mutex } @@ -45,6 +46,29 @@ func (c *mongoDBConnectionProducer) Initialize(conf map[string]interface{}, veri return fmt.Errorf("connection_url cannot be empty") } + if c.WriteConcern != "" { + input := c.WriteConcern + + // Try to base64 decode the input. If successful, consider the decoded + // value as input. + inputBytes, err := base64.StdEncoding.DecodeString(input) + if err == nil { + input = string(inputBytes) + } + + concern := &mgo.Safe{} + err = json.Unmarshal([]byte(input), concern) + if err != nil { + return fmt.Errorf("error mashalling write concern: %s", err) + } + + // Only set c.safe if anything got marshaled into concern. We don't want to + // pass an empty, non-nil mgo.Safe object into mgo.SetSafe in Connection(). + if (mgo.Safe{} != *concern) { + c.safe = concern + } + } + // Set initialized to true at this point since all fields are set, // and the connection can be established at a later time. c.Initialized = true @@ -82,29 +106,8 @@ func (c *mongoDBConnectionProducer) Connection() (interface{}, error) { return nil, err } - if c.WriteConcern != "" { - input := c.WriteConcern - - // Try to base64 decode the input. If successful, consider the decoded - // value as input. - inputBytes, err := base64.StdEncoding.DecodeString(input) - if err == nil { - input = string(inputBytes) - } - - var concern writeConcern - err = json.Unmarshal([]byte(input), &concern) - if err != nil { - return nil, err - } - - c.session.SetSafe(&mgo.Safe{ - W: concern.W, - WMode: concern.WMode, - WTimeout: concern.WTimeout, - FSync: concern.FSync, - J: concern.J, - }) + if c.safe != nil { + c.session.SetSafe(c.safe) } c.session.SetSyncTimeout(1 * time.Minute) diff --git a/plugins/database/mongodb/util.go b/plugins/database/mongodb/util.go index 2424324f09b1..9004a3c710c4 100644 --- a/plugins/database/mongodb/util.go +++ b/plugins/database/mongodb/util.go @@ -17,16 +17,6 @@ type mongoDBStatement struct { Roles mongodbRoles `json:"roles"` } -// writeConcern is the mgo.Safe struct with JSON tags -// More info: https://godoc.org/gopkg.in/mgo.v2#Safe -type writeConcern struct { - W int `json:"w"` // Min # of servers to ack before success - WMode string `json:"w_mode"` // Write mode for MongoDB 2.0+ (e.g. "majority") - WTimeout int `json:"wtimeout"` // Milliseconds to wait for W before timing out - FSync bool `json:"fsync"` // Sync via the journal if present, or via data files sync otherwise - J bool `json:"j"` // Sync via the journal if present -} - // Convert array of role documents like: // // [ { "role": "readWrite" }, { "role": "readWrite", "db": "test" } ] From 2415f8119d49bcde85e9b84c40b380f3464e1250 Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang Date: Tue, 5 Dec 2017 14:57:36 -0500 Subject: [PATCH 5/5] Return error on empty mapping of write_concern into mgo.Safe struct --- plugins/database/mongodb/connection_producer.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/plugins/database/mongodb/connection_producer.go b/plugins/database/mongodb/connection_producer.go index 162283d7885f..a9ff64a943fa 100644 --- a/plugins/database/mongodb/connection_producer.go +++ b/plugins/database/mongodb/connection_producer.go @@ -59,14 +59,15 @@ func (c *mongoDBConnectionProducer) Initialize(conf map[string]interface{}, veri concern := &mgo.Safe{} err = json.Unmarshal([]byte(input), concern) if err != nil { - return fmt.Errorf("error mashalling write concern: %s", err) + return fmt.Errorf("error mashalling write_concern: %s", err) } - // Only set c.safe if anything got marshaled into concern. We don't want to - // pass an empty, non-nil mgo.Safe object into mgo.SetSafe in Connection(). - if (mgo.Safe{} != *concern) { - c.safe = concern + // Guard against empty, non-nil mgo.Safe object; we don't want to pass that + // into mgo.SetSafe in Connection(). + if (mgo.Safe{} == *concern) { + return fmt.Errorf("provided write_concern values did not map to any mgo.Safe fields") } + c.safe = concern } // Set initialized to true at this point since all fields are set,