From 8b0f654aaff24d481e47b3f613746d69b921ae96 Mon Sep 17 00:00:00 2001 From: andy Date: Sat, 26 Oct 2019 15:05:30 +0200 Subject: [PATCH 1/6] Fix not master error in mongo plugin --- plugins/database/mongodb/mongodb.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/database/mongodb/mongodb.go b/plugins/database/mongodb/mongodb.go index 85c5cdc06723..d71520600110 100644 --- a/plugins/database/mongodb/mongodb.go +++ b/plugins/database/mongodb/mongodb.go @@ -138,7 +138,7 @@ func (m *MongoDB) CreateUser(ctx context.Context, statements dbplugin.Statements err = session.DB(mongoCS.DB).Run(createUserCmd, nil) switch { case err == nil: - case err == io.EOF, strings.Contains(err.Error(), "EOF"): + case err == io.EOF, strings.Contains(err.Error(), "EOF"), strings.Contains(err.Error(), "not master"): // Call getConnection to reset and retry query if we get an EOF error on first attempt. session, err := m.getConnection(ctx) if err != nil { @@ -188,7 +188,7 @@ func (m *MongoDB) SetCredentials(ctx context.Context, statements dbplugin.Statem switch { case err == nil: - case err == io.EOF, strings.Contains(err.Error(), "EOF"): + case err == io.EOF, strings.Contains(err.Error(), "EOF"), strings.Contains(err.Error(), "not master"): // Call getConnection to reset and retry query if we get an EOF error on first attempt. session, err := m.getConnection(ctx) if err != nil { @@ -251,7 +251,7 @@ func (m *MongoDB) RevokeUser(ctx context.Context, statements dbplugin.Statements err = session.DB(db).RemoveUser(username) switch { case err == nil, err == mgo.ErrNotFound: - case err == io.EOF, strings.Contains(err.Error(), "EOF"): + case err == io.EOF, strings.Contains(err.Error(), "EOF"), strings.Contains(err.Error(), "not master"): if err := m.Close(); err != nil { return errwrap.Wrapf("error closing EOF'd mongo connection: {{err}}", err) } From 10d84a17f3ddf4a00cd84a2cf82174a07d57af76 Mon Sep 17 00:00:00 2001 From: andy Date: Sat, 26 Oct 2019 17:26:04 +0200 Subject: [PATCH 2/6] Close connection and reconnect after primary step down --- plugins/database/mongodb/mongodb.go | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/plugins/database/mongodb/mongodb.go b/plugins/database/mongodb/mongodb.go index d71520600110..a23e2b50c7dd 100644 --- a/plugins/database/mongodb/mongodb.go +++ b/plugins/database/mongodb/mongodb.go @@ -138,7 +138,7 @@ func (m *MongoDB) CreateUser(ctx context.Context, statements dbplugin.Statements err = session.DB(mongoCS.DB).Run(createUserCmd, nil) switch { case err == nil: - case err == io.EOF, strings.Contains(err.Error(), "EOF"), strings.Contains(err.Error(), "not master"): + case err == io.EOF, strings.Contains(err.Error(), "EOF"): // Call getConnection to reset and retry query if we get an EOF error on first attempt. session, err := m.getConnection(ctx) if err != nil { @@ -148,6 +148,18 @@ func (m *MongoDB) CreateUser(ctx context.Context, statements dbplugin.Statements if err != nil { return "", "", err } + case strings.Contains(err.Error(), "not master"): + if err := m.Close(); err != nil { + return "", "", errwrap.Wrapf("error closing non-master mongo connection: {{err}}", err) + } + session, err := m.getConnection(ctx) + if err != nil { + return "", "", err + } + err = session.DB(mongoCS.DB).Run(createUserCmd, nil) + if err != nil { + return "", "", err + } default: return "", "", err } @@ -188,7 +200,7 @@ func (m *MongoDB) SetCredentials(ctx context.Context, statements dbplugin.Statem switch { case err == nil: - case err == io.EOF, strings.Contains(err.Error(), "EOF"), strings.Contains(err.Error(), "not master"): + case err == io.EOF, strings.Contains(err.Error(), "EOF"): // Call getConnection to reset and retry query if we get an EOF error on first attempt. session, err := m.getConnection(ctx) if err != nil { @@ -198,6 +210,18 @@ func (m *MongoDB) SetCredentials(ctx context.Context, statements dbplugin.Statem if err != nil { return "", "", err } + case strings.Contains(err.Error(), "not master"): + if err := m.Close(); err != nil { + return "", "", errwrap.Wrapf("error closing non-master mongo connection: {{err}}", err) + } + session, err := m.getConnection(ctx) + if err != nil { + return "", "", err + } + err = session.DB(dialInfo.Database).UpsertUser(&mongoUser) + if err != nil { + return "", "", err + } default: return "", "", err } From 95f52fd99612259a48bfebc07aa2762055db30bd Mon Sep 17 00:00:00 2001 From: andy Date: Sat, 26 Oct 2019 18:51:00 +0200 Subject: [PATCH 3/6] Close connection without trying to acquire lock again --- plugins/database/mongodb/mongodb.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/plugins/database/mongodb/mongodb.go b/plugins/database/mongodb/mongodb.go index a23e2b50c7dd..9903281c039e 100644 --- a/plugins/database/mongodb/mongodb.go +++ b/plugins/database/mongodb/mongodb.go @@ -149,9 +149,11 @@ func (m *MongoDB) CreateUser(ctx context.Context, statements dbplugin.Statements return "", "", err } case strings.Contains(err.Error(), "not master"): - if err := m.Close(); err != nil { - return "", "", errwrap.Wrapf("error closing non-master mongo connection: {{err}}", err) + // Close connection and reconnect if connected node is not primary. + if m.mongoDBConnectionProducer.session != nil { + m.mongoDBConnectionProducer.session.Close() } + m.mongoDBConnectionProducer.session = nil; session, err := m.getConnection(ctx) if err != nil { return "", "", err @@ -211,9 +213,12 @@ func (m *MongoDB) SetCredentials(ctx context.Context, statements dbplugin.Statem return "", "", err } case strings.Contains(err.Error(), "not master"): - if err := m.Close(); err != nil { - return "", "", errwrap.Wrapf("error closing non-master mongo connection: {{err}}", err) + // Close connection and reconnect if connected node is not primary. + if m.mongoDBConnectionProducer.session != nil { + m.mongoDBConnectionProducer.session.Close() } + m.mongoDBConnectionProducer.session = nil; + session, err := m.getConnection(ctx) if err != nil { return "", "", err From a94e3335e44d366e2679c2e83f94d833adf31abc Mon Sep 17 00:00:00 2001 From: andy Date: Sat, 26 Oct 2019 19:04:42 +0200 Subject: [PATCH 4/6] Fix a deadlock in RevokeUser --- plugins/database/mongodb/connection_producer.go | 3 --- plugins/database/mongodb/mongodb.go | 11 +++++------ 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/plugins/database/mongodb/connection_producer.go b/plugins/database/mongodb/connection_producer.go index 1b59496cf319..1b18d0174d27 100644 --- a/plugins/database/mongodb/connection_producer.go +++ b/plugins/database/mongodb/connection_producer.go @@ -138,10 +138,7 @@ func (c *mongoDBConnectionProducer) Connection(_ context.Context) (interface{}, return c.session, nil } -// Close terminates the database connection. func (c *mongoDBConnectionProducer) Close() error { - c.Lock() - defer c.Unlock() if c.session != nil { c.session.Close() diff --git a/plugins/database/mongodb/mongodb.go b/plugins/database/mongodb/mongodb.go index 9903281c039e..afd1cd915133 100644 --- a/plugins/database/mongodb/mongodb.go +++ b/plugins/database/mongodb/mongodb.go @@ -150,10 +150,10 @@ func (m *MongoDB) CreateUser(ctx context.Context, statements dbplugin.Statements } case strings.Contains(err.Error(), "not master"): // Close connection and reconnect if connected node is not primary. - if m.mongoDBConnectionProducer.session != nil { - m.mongoDBConnectionProducer.session.Close() + if err := m.Close(); err != nil { + return "", "", errwrap.Wrapf("error closing EOF'd mongo connection: {{err}}", err) } - m.mongoDBConnectionProducer.session = nil; + session, err := m.getConnection(ctx) if err != nil { return "", "", err @@ -214,10 +214,9 @@ func (m *MongoDB) SetCredentials(ctx context.Context, statements dbplugin.Statem } case strings.Contains(err.Error(), "not master"): // Close connection and reconnect if connected node is not primary. - if m.mongoDBConnectionProducer.session != nil { - m.mongoDBConnectionProducer.session.Close() + if err := m.Close(); err != nil { + return "", "", errwrap.Wrapf("error closing EOF'd mongo connection: {{err}}", err) } - m.mongoDBConnectionProducer.session = nil; session, err := m.getConnection(ctx) if err != nil { From 22c154d61937df007e6689b490665604eef856cf Mon Sep 17 00:00:00 2001 From: andy Date: Sat, 26 Oct 2019 19:12:26 +0200 Subject: [PATCH 5/6] Rewrite error message when failing to close non-primary --- plugins/database/mongodb/mongodb.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/database/mongodb/mongodb.go b/plugins/database/mongodb/mongodb.go index afd1cd915133..14ed34ac11e8 100644 --- a/plugins/database/mongodb/mongodb.go +++ b/plugins/database/mongodb/mongodb.go @@ -151,7 +151,7 @@ func (m *MongoDB) CreateUser(ctx context.Context, statements dbplugin.Statements case strings.Contains(err.Error(), "not master"): // Close connection and reconnect if connected node is not primary. if err := m.Close(); err != nil { - return "", "", errwrap.Wrapf("error closing EOF'd mongo connection: {{err}}", err) + return "", "", errwrap.Wrapf("error closing non-primary mongo connection: {{err}}", err) } session, err := m.getConnection(ctx) @@ -215,7 +215,7 @@ func (m *MongoDB) SetCredentials(ctx context.Context, statements dbplugin.Statem case strings.Contains(err.Error(), "not master"): // Close connection and reconnect if connected node is not primary. if err := m.Close(); err != nil { - return "", "", errwrap.Wrapf("error closing EOF'd mongo connection: {{err}}", err) + return "", "", errwrap.Wrapf("error closing non-primary mongo connection: {{err}}", err) } session, err := m.getConnection(ctx) From 493257960973ba65ccf52d647431dededf6e3f58 Mon Sep 17 00:00:00 2001 From: andy Date: Thu, 31 Oct 2019 10:26:17 +0100 Subject: [PATCH 6/6] Reinstate the Lock() in Close() --- plugins/database/mongodb/connection_producer.go | 2 ++ plugins/database/mongodb/mongodb.go | 14 +++++++------- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/plugins/database/mongodb/connection_producer.go b/plugins/database/mongodb/connection_producer.go index 1b18d0174d27..ab0ddf216caa 100644 --- a/plugins/database/mongodb/connection_producer.go +++ b/plugins/database/mongodb/connection_producer.go @@ -139,6 +139,8 @@ func (c *mongoDBConnectionProducer) Connection(_ context.Context) (interface{}, } func (c *mongoDBConnectionProducer) Close() error { + c.Lock() + defer c.Unlock() if c.session != nil { c.session.Close() diff --git a/plugins/database/mongodb/mongodb.go b/plugins/database/mongodb/mongodb.go index 14ed34ac11e8..75ce16b792e5 100644 --- a/plugins/database/mongodb/mongodb.go +++ b/plugins/database/mongodb/mongodb.go @@ -11,7 +11,6 @@ import ( "fmt" - "github.com/hashicorp/errwrap" "github.com/hashicorp/vault/api" "github.com/hashicorp/vault/sdk/database/dbplugin" "github.com/hashicorp/vault/sdk/database/helper/credsutil" @@ -150,8 +149,8 @@ func (m *MongoDB) CreateUser(ctx context.Context, statements dbplugin.Statements } case strings.Contains(err.Error(), "not master"): // Close connection and reconnect if connected node is not primary. - if err := m.Close(); err != nil { - return "", "", errwrap.Wrapf("error closing non-primary mongo connection: {{err}}", err) + if m.mongoDBConnectionProducer.session != nil { + m.mongoDBConnectionProducer.session.Close(); } session, err := m.getConnection(ctx) @@ -214,9 +213,10 @@ func (m *MongoDB) SetCredentials(ctx context.Context, statements dbplugin.Statem } case strings.Contains(err.Error(), "not master"): // Close connection and reconnect if connected node is not primary. - if err := m.Close(); err != nil { - return "", "", errwrap.Wrapf("error closing non-primary mongo connection: {{err}}", err) + if m.mongoDBConnectionProducer.session != nil { + m.mongoDBConnectionProducer.session.Close(); } + m.mongoDBConnectionProducer.session = nil session, err := m.getConnection(ctx) if err != nil { @@ -280,8 +280,8 @@ func (m *MongoDB) RevokeUser(ctx context.Context, statements dbplugin.Statements switch { case err == nil, err == mgo.ErrNotFound: case err == io.EOF, strings.Contains(err.Error(), "EOF"), strings.Contains(err.Error(), "not master"): - if err := m.Close(); err != nil { - return errwrap.Wrapf("error closing EOF'd mongo connection: {{err}}", err) + if m.mongoDBConnectionProducer.session != nil { + m.mongoDBConnectionProducer.session.Close(); } session, err := m.getConnection(ctx) if err != nil {