Fix parse monitor URL

monitoring/influxdb.go

@@ -2,7 +2,7 @@ package monitoring
 
 import (
 	"fmt"
-	"net/url"
+	"regexp"
 	"strings"
 	"time"
 
@@ -16,6 +16,49 @@ import (
 // InfluxDB
 type InfluxMonitorConfig string
 
+type influxMonitorCfg struct {
+	Addr     string
+	Username string
+	Password string
+	Database string
+}
+
+var configRegexp = regexp.MustCompile(`^(?P<scheme>https?):\/\/(?:(?P<username>.*?)(?::(?P<password>.*?)|)@)?(?P<host>.+?)\/(?P<database>.+?)$`)
+
+func parseConfig(config string) (*influxMonitorCfg, error) {
+	match := configRegexp.FindStringSubmatch(config)
+	if match == nil {
+		return nil, errors.New("config format error")
+	}
+
+	var scheme string
+	var username string
+	var password string
+	var host string
+	var database string
+	for i, name := range configRegexp.SubexpNames() {
+		switch name {
+		case "scheme":
+			scheme = match[i]
+		case "username":
+			username = match[i]
+		case "password":
+			password = match[i]
+		case "host":
+			host = match[i]
+		case "database":
+			database = match[i]
+		}
+	}
+
+	return &influxMonitorCfg{
+		Addr:     scheme + "://" + host,
+		Username: username,
+		Password: password,
+		Database: database,
+	}, nil
+}
+
 // NewInfluxdbMonitor creates new monitoring influxdb
 // client. config URL syntax is `https://<username>:<password>@<influxDB host>/<database>`
 //
@@ -26,26 +69,15 @@ type InfluxMonitorConfig string
 func NewInfluxdbMonitor(config InfluxMonitorConfig, logger log.Logger) (Monitor, error) {
 	monitorURL := string(config)
 
-	u, err := url.Parse(monitorURL)
+	cfg, err := parseConfig(monitorURL)
 	if err != nil {
 		return nil, errors.Wrapf(err, "couldn't parse influxdb url %v", monitorURL)
-	} else if !u.IsAbs() {
-		return nil, errors.Errorf("influxdb monitoring url %v not absolute url", monitorURL)
-	}
-
-	username := ""
-	password := ""
-
-	if u.User != nil {
-		username = u.User.Username()
-		// Skips identify of "whether password is set" as password not a must
-		password, _ = u.User.Password()
 	}
 
 	httpConfig := influxdb.HTTPConfig{
-		Addr:     fmt.Sprintf("%s://%s", u.Scheme, u.Host),
-		Username: username,
-		Password: password,
+		Addr:     cfg.Addr,
+		Username: cfg.Username,
+		Password: cfg.Password,
 	}
 
 	client, err := influxdb.NewHTTPClient(httpConfig)
@@ -54,23 +86,20 @@ func NewInfluxdbMonitor(config InfluxMonitorConfig, logger log.Logger) (Monitor,
 		return nil, errors.Wrapf(err, "couldn't initialize influxdb http client with http config %+v", httpConfig)
 	}
 
-	database := strings.TrimLeft(u.Path, "/")
-
-	if strings.TrimSpace(database) == "" {
+	if strings.TrimSpace(cfg.Database) == "" {
 		return nil, errors.Errorf("influxdb monitoring url %v not database", monitorURL)
 	}
 
 	monitor := influxdbMonitor{
-		database: database,
+		database: cfg.Database,
 		client:   client,
 		logger:   logger,
 	}
 
 	logger = logger.With(
-		"scheme", u.Scheme,
-		"username", username,
+		"addr", cfg.Addr,
+		"username", cfg.Username,
 		"database", monitor.database,
-		"host", u.Host,
 	)
 
 	// check connectivity to InfluxDB every 5 minutes
@@ -93,7 +122,7 @@ func NewInfluxdbMonitor(config InfluxMonitorConfig, logger log.Logger) (Monitor,
 	}()
 
 	logger.Info().Log(
-		"msg", fmt.Sprintf("influxdb instrumentation writing to %s://%s@%s/%s", u.Scheme, username, u.Host, monitor.database),
+		"msg", fmt.Sprintf("influxdb instrumentation writing to %s/%s on user %v", cfg.Addr, monitor.database, cfg.Username),
 	)
 
 	return &monitor, nil

monitoring/influxdb_test.go

@@ -5,21 +5,21 @@ import (
 	"testing"
 
 	"github.com/theplant/appkit/log"
+	"github.com/theplant/testingutils/errorassert"
 )
 
 func TestInvalidInfluxdbConfig(t *testing.T) {
 	logger := log.NewNopLogger()
-	cases := map[string]string{
-		"not absolute url":            "",
-		"Unsupported protocol scheme": "localhost:8086/local",
-		"not database":                "http://root:password@localhost:8086",
+	configs := []string{
+		"",
+		"localhost:8086/local",
+		"http://root:password@localhost:8086",
 	}
 
-	for reason, config := range cases {
-		_, err := NewInfluxdbMonitor(InfluxMonitorConfig(config), logger)
-
-		if err == nil || !strings.Contains(err.Error(), reason) {
-			t.Fatalf("no error creating influxdb monitor with config url %s", config)
+	for _, conf := range configs {
+		_, err := NewInfluxdbMonitor(InfluxMonitorConfig(conf), logger)
+		if err == nil || !strings.Contains(err.Error(), "config format error") {
+			t.Fatalf("no error creating influxdb monitor with config url %s", conf)
 		}
 	}
 }
@@ -42,3 +42,93 @@ func TestValidInfluxdbConfig(t *testing.T) {
 		}
 	}
 }
+
+func TestParseConfig(t *testing.T) {
+	tests := []struct {
+		name string
+
+		config string
+
+		expectedCfg      *influxMonitorCfg
+		expectedErrorStr string
+	}{
+		{
+			name:   "http scheme",
+			config: "http://localhost:8086/local",
+			expectedCfg: &influxMonitorCfg{
+				Addr:     "http://localhost:8086",
+				Username: "",
+				Password: "",
+				Database: "local",
+			},
+		},
+
+		{
+			name:   "https scheme",
+			config: "https://localhost:8086/local",
+			expectedCfg: &influxMonitorCfg{
+				Addr:     "https://localhost:8086",
+				Username: "",
+				Password: "",
+				Database: "local",
+			},
+		},
+
+		{
+			name:   "has username and no password",
+			config: "https://root@localhost:8086/local",
+			expectedCfg: &influxMonitorCfg{
+				Addr:     "https://localhost:8086",
+				Username: "root",
+				Password: "",
+				Database: "local",
+			},
+		},
+
+		{
+			name:   "no username and has password",
+			config: "https://:password@localhost:8086/local",
+			expectedCfg: &influxMonitorCfg{
+				Addr:     "https://localhost:8086",
+				Username: "",
+				Password: "password",
+				Database: "local",
+			},
+		},
+
+		{
+			name:   "has username and password",
+			config: "https://root:password@localhost:8086/local",
+			expectedCfg: &influxMonitorCfg{
+				Addr:     "https://localhost:8086",
+				Username: "root",
+				Password: "password",
+				Database: "local",
+			},
+		},
+
+		{
+			name:             "no database",
+			config:           "https://localhost:8086/",
+			expectedErrorStr: "config format error",
+		},
+
+		{
+			name:             "no scheme",
+			config:           "localhost:8086/local",
+			expectedErrorStr: "config format error",
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			cfg, err := parseConfig(test.config)
+			if err != nil {
+				errorassert.Equal(t, test.expectedErrorStr, err.Error())
+			} else {
+				errorassert.Equal(t, test.expectedErrorStr, "")
+			}
+
+			errorassert.Equal(t, test.expectedCfg, cfg)
+		})
+	}
+}