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

PMM-13543 Support socket path. #961

Merged
merged 3 commits into from
Nov 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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 {
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 @@ func parseURIList(uriList []string, logger *logrus.Logger, splitCluster bool) []
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) //nolint: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