Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion exporter/diagnostic_data_collector.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ func (d *diagnosticDataCollector) collect(ch chan<- prometheus.Metric) {
debugResult(logger, m)

// MongoDB 8.0 splits the diagnostic data into multiple blocks, so we need to merge them
if d.buildInfo.VersionArray[0] >= 8 { //nolint:gomnd
if _, ok := m["common"]; ok { //nolint:gomnd,mng
b := bson.M{}
for _, mv := range m {
block, ok := mv.(bson.M)
Expand Down
28 changes: 23 additions & 5 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -268,18 +268,36 @@
return URIs
}

// buildURIManually builds the URI manually by checking if the user and password are supplied
func buildURIManually(uri string, user string, password string) string {
uriArray := strings.SplitN(uri, "://", 2)

Check failure on line 273 in main.go

View workflow job for this annotation

GitHub Actions / Lint Check

Magic number: 2, in <argument> detected (mnd)
prefix := uriArray[0] + "://"
uri = uriArray[1]

// 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 != "" {
Copy link
Member

Choose a reason for hiding this comment

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

I'm wondering if we shouldn't support a case where the user is non-empty and the password is empty.

Copy link
Member Author

Choose a reason for hiding this comment

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

good point for improvement, but in scope of this fix I think we can leave as it used to be. I just reverted old logic for cases where url.Parse doesn't work. Additionally for that users can set user in initial DSN. let's create github issue with good-first-task label for those who would like to contribute

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

// add back prefix after adding the user and pass
uri = prefix + uri

return uri
}

func buildURI(uri string, user string, password string, log *logrus.Logger) string {
defaultPrefix := "mongodb://" // default prefix
matchRegexp := regexp.MustCompile(`^mongodb(\+srv)?://`)

// Split the uri defaultPrefix if there is any
if !matchRegexp.MatchString(uri) {
if !strings.HasPrefix(uri, defaultPrefix) && !strings.HasPrefix(uri, "mongodb+srv://") {
uri = defaultPrefix + uri
}
parsedURI, err := url.Parse(uri)
if err != nil {
log.Fatalf("Failed to parse URI %s: %v", uri, err)
return uri
// PMM generates URI with escaped path to socket file, so url.Parse fails
// in this case we build URI manually
return buildURIManually(uri, user, password)
}

if parsedURI.User == nil && user != "" && password != "" {
Expand Down
42 changes: 42 additions & 0 deletions main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,48 @@ func TestBuildURI(t *testing.T) {
newPassword: "yyy?!#$%^&*()_+",
expect: "mongodb://xxx%3F%21%23$%25%5E&%2A%28%29_+:yyy%3F%21%23$%25%5E&%2A%28%[email protected]",
},
{
situation: "path to socket",
origin: "mongodb:///tmp/mongodb-27017.sock",
newUser: "",
newPassword: "",
expect: "mongodb:///tmp/mongodb-27017.sock",
},
{
situation: "path to socket with params",
origin: "mongodb://username:s3cur3%20p%40$$w0r4.@%2Fvar%2Frun%2Fmongodb%2Fmongodb.sock/database?connectTimeoutMS=1000&directConnection=true&serverSelectionTimeoutMS=1000",
newUser: "",
newPassword: "",
expect: "mongodb://username:s3cur3%20p%40$$w0r4.@%2Fvar%2Frun%2Fmongodb%2Fmongodb.sock/database?connectTimeoutMS=1000&directConnection=true&serverSelectionTimeoutMS=1000",
},
{
situation: "path to socket with auth",
origin: "mongodb://xxx:yyy@/tmp/mongodb-27017.sock",
newUser: "",
newPassword: "",
expect: "mongodb://xxx:yyy@/tmp/mongodb-27017.sock",
},
{
situation: "path to socket with auth and user params",
origin: "mongodb:///tmp/mongodb-27017.sock",
newUser: "xxx",
newPassword: "yyy",
expect: "mongodb://xxx:yyy@/tmp/mongodb-27017.sock",
},
{
situation: "path to socket without prefix",
origin: "/tmp/mongodb-27017.sock",
newUser: "",
newPassword: "",
expect: "mongodb:///tmp/mongodb-27017.sock",
},
{
situation: "path to socket without prefix with auth",
origin: "/tmp/mongodb-27017.sock",
newUser: "xxx",
newPassword: "yyy",
expect: "mongodb://xxx:yyy@/tmp/mongodb-27017.sock",
},
}
for _, tc := range tests {
t.Run(tc.situation, func(t *testing.T) {
Expand Down
Loading