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

add user and password to uri if the uri does not contain user password #560

Merged
merged 12 commits into from
Sep 13, 2023
15 changes: 13 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,25 @@ Connecting user should have sufficient rights to query needed stats:
More info about roles in MongoDB [documentation](https://docs.mongodb.com/manual/reference/built-in-roles/#mongodb-authrole-clusterMonitor).

#### Example
```
```sh
mongodb_exporter_linux_amd64/mongodb_exporter --mongodb.uri=mongodb://127.0.0.1:17001
```

#### MongoDB Authentication
You can supply the mongodb user/password direct in the `--mongodb.uri=` like `--mongodb.uri=mongodb://user:[email protected]:17001`, you can also supply the mongodb user/password with `--mongodb.user=`, `--mongodb.password=`
but the user and password info will be leaked via `ps` or `top` command, for security issue, you can use `MONGODB_USER` and `MONGODB_PASSWORD` env variable to set user/password for given uri
```sh
MONGODB_USER=XXX MONGODB_PASSWORD=YYY mongodb_exporter_linux_amd64/mongodb_exporter --mongodb.uri=mongodb://127.0.0.1:17001 --mongodb.collstats-colls=db1.c1,db2.c2
# or
export MONGODB_USER=XXX
export MONGODB_PASSWORD=YYY
mongodb_exporter_linux_amd64/mongodb_exporter --mongodb.uri=mongodb://127.0.0.1:17001 --mongodb.collstats-colls=db1.c1,db2.c2
```

#### Enabling collstats metrics gathering
`--mongodb.collstats-colls` receives a list of databases and collections to monitor using collstats.
Usage example: `--mongodb.collstats-colls=database1.collection1,database2.collection2`
```
```sh
mongodb_exporter_linux_amd64/mongodb_exporter --mongodb.uri=mongodb://127.0.0.1:17001 --mongodb.collstats-colls=db1.c1,db2.c2
```
#### Enabling compatibility mode.
Expand Down
23 changes: 19 additions & 4 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ var (

// GlobalFlags has command line flags to configure the exporter.
type GlobalFlags struct {
User string `name:"mongodb.user" help:"monitor user, need clusterMonitor role in admin db and read role in local db" env:"MONGODB_USER" placeholder:"monitorUser"`
Password string `name:"mongodb.password" help:"monitor user password" env:"MONGODB_PASSWORD" placeholder:"monitorPassword"`
CollStatsNamespaces string `name:"mongodb.collstats-colls" help:"List of comma separared databases.collections to get $collStats" placeholder:"db1,db2.col2"`
IndexStatsCollections string `name:"mongodb.indexstats-colls" help:"List of comma separared databases.collections to get $indexStats" placeholder:"db1.col1,db2.col2"`
URI string `name:"mongodb.uri" help:"MongoDB connection URI" env:"MONGODB_URI" placeholder:"mongodb://user:[email protected]:27017/admin?ssl=true"`
Expand Down Expand Up @@ -89,6 +91,22 @@ func main() {
e.Run()
}

func buildURI(uri string, user string, password string) string {
// IF user@pass not contained in uri AND custom user and pass supplied in arguments
// DO concat a new uri with user and pass arguments value
if !strings.Contains(uri, "@") && user != "" && password != "" {
// trim mongodb:// prefix to handle user and pass logic
uri = strings.TrimPrefix(uri, "mongodb://")

// log.Debugf("add user and pass to the uri")
uri = fmt.Sprintf("%s:%s@%s", user, password, uri)

// add back mongodb://
uri = "mongodb://" + uri
Copy link
Member

Choose a reason for hiding this comment

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

let's remove it, anyway we do it a few lines later

}
return uri
}

func buildExporter(opts GlobalFlags) *exporter.Exporter {
log := logrus.New()

Expand All @@ -103,10 +121,7 @@ func buildExporter(opts GlobalFlags) *exporter.Exporter {

log.Debugf("Compatible mode: %v", opts.CompatibleMode)

if !strings.HasPrefix(opts.URI, "mongodb") {
log.Debugf("Prepending mongodb:// to the URI")
opts.URI = "mongodb://" + opts.URI
}
opts.URI = buildURI(opts.URI, opts.User, opts.Password)

log.Debugf("Connection URI: %s", opts.URI)

Expand Down
76 changes: 76 additions & 0 deletions main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package main

import (
"github.com/stretchr/testify/assert"
Copy link
Member

Choose a reason for hiding this comment

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

this import should be moved below as a separate block

Copy link
Contributor Author

@naughtyGitCat naughtyGitCat Sep 2, 2023

Choose a reason for hiding this comment

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

I guess you mean code orders like this

import (
	"context"
	"fmt"
	"strings"
	"testing"
	"time"

	"github.com/prometheus/client_golang/prometheus/testutil"
	"github.com/sirupsen/logrus"
	"github.com/stretchr/testify/assert"
	"go.mongodb.org/mongo-driver/bson"

	"github.com/percona/mongodb_exporter/internal/tu"
)

changed, years ago I did the same order like this, but when the Goland and gofmt auto changed dependency order, I thought the auto reordered was more go-style.

"testing"
)

Expand All @@ -37,3 +38,78 @@ func TestBuildExporter(t *testing.T) {

buildExporter(opts)
}

func TestBuildURI(t *testing.T) {

tests := []struct {
situation string
origin string
newUser string
newPassword string
expect string
}{
{
situation: "uri with prefix and auth, and auth supplied in opt.User/Password",
origin: "mongodb://usr:[email protected]",
newUser: "xxx",
newPassword: "yyy",
expect: "mongodb://usr:[email protected]",
},
{
situation: "uri with prefix and auth, no auth supplied in opt.User/Password",
origin: "mongodb://usr:[email protected]",
newUser: "",
newPassword: "",
expect: "mongodb://usr:[email protected]",
},
{
situation: "uri with no prefix and auth, and auth supplied in opt.User/Password",
origin: "usr:[email protected]",
newUser: "xxx",
newPassword: "yyy",
expect: "usr:[email protected]",
},
{
situation: "uri with no prefix and auth, no auth supplied in opt.User/Password",
origin: "usr:[email protected]",
newUser: "",
newPassword: "",
expect: "usr:[email protected]",
},
{
situation: "uri with prefix and no auth, and auth supplied in opt.User/Password",
origin: "mongodb://127.0.0.1",
newUser: "xxx",
newPassword: "yyy",
expect: "mongodb://xxx:[email protected]",
},
{
situation: "uri with prefix and no auth, no auth supplied in opt.User/Password",
origin: "mongodb://127.0.0.1",
newUser: "",
newPassword: "",
expect: "mongodb://127.0.0.1",
},
{
situation: "uri with no prefix and no auth, and auth supplied in opt.User/Password",
origin: "127.0.0.1",
newUser: "xxx",
newPassword: "yyy",
expect: "mongodb://xxx:[email protected]",
},
{
situation: "uri with no prefix and no auth, no auth supplied in opt.User/Password",
origin: "127.0.0.1",
newUser: "",
newPassword: "",
expect: "127.0.0.1",
Copy link
Member

Choose a reason for hiding this comment

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

the expectation here is mongodb://127.0.0.1, that's how it used to work

Suggested change
expect: "127.0.0.1",
expect: "mongodb://127.0.0.1",

Copy link
Contributor Author

Choose a reason for hiding this comment

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

my mistake, have correct it both in main.go and main_test.go

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it does need a mongod:// prefix in all situation, according to https://www.mongodb.com/docs/manual/reference/connection-string/

},
}
for _, tc := range tests {
newUri := buildURI(tc.origin, tc.newUser, tc.newPassword)
// t.Logf("Origin: %s", tc.origin)
// t.Logf("Expect: %s", tc.expect)
// t.Logf("Result: %s", newUri)
assert.Equal(t, newUri, tc.expect)
}
}
Loading