config: validate dashboard-address (#2517)

pkg/dashboard/adapter/manager.go

@@ -15,7 +15,6 @@ package adapter
 
 import (
 	"context"
-	"net/url"
 	"sort"
 	"sync"
 	"time"
@@ -137,11 +136,6 @@ func (m *Manager) checkAddress() {
 		m.stopService()
 		return
 	default:
-		if _, err := url.Parse(dashboardAddress); err != nil {
-			log.Error("illegal dashboard address", zap.String("address", dashboardAddress))
-			return
-		}
-
 		if m.isLeader && m.needResetAddress(dashboardAddress) {
 			m.setNewAddress()
 			return

pkg/dashboard/adapter/redirector.go

@@ -69,7 +69,7 @@ func (h *Redirector) SetAddress(addr string) {
 
 	h.status.Start()
 	h.address = addr
-	target, _ := url.Parse(addr) // error has been handled in checkAddress
+	target, _ := url.Parse(addr) // error has been handled
 	h.proxy = httputil.NewSingleHostReverseProxy(target)
 
 	defaultDirector := h.proxy.Director

server/cluster/cluster.go

@@ -1695,3 +1695,19 @@ func GetMembers(etcdClient *clientv3.Client) ([]*pdpb.Member, error) {
 
 	return members, nil
 }
+
+// IsClientURL returns whether addr is a ClientUrl of any member.
+func IsClientURL(addr string, etcdClient *clientv3.Client) bool {
+	members, err := GetMembers(etcdClient)
+	if err != nil {
+		return false
+	}
+	for _, member := range members {
+		for _, u := range member.GetClientUrls() {
+			if u == addr {
+				return true
+			}
+		}
+	}
+	return false
+}

server/config/config.go

@@ -948,10 +948,10 @@ func (c *PDServerConfig) adjust(meta *configMetaData) error {
 	if !meta.IsDefined("dashboard-address") {
 		c.DashboardAddress = defaultDashboardAddress
 	}
-	return nil
+	return c.Validate()
 }
 
-// Clone retruns a cloned PD server config.
+// Clone returns a cloned PD server config.
 func (c *PDServerConfig) Clone() *PDServerConfig {
 	runtimeServices := make(typeutil.StringSlice, len(c.RuntimeServices))
 	copy(runtimeServices, c.RuntimeServices)
@@ -965,6 +965,20 @@ func (c *PDServerConfig) Clone() *PDServerConfig {
 	}
 }
 
+// Validate is used to validate if some pd-server configurations are right.
+func (c *PDServerConfig) Validate() error {
+	switch c.DashboardAddress {
+	case "auto":
+	case "none":
+	default:
+		if err := ValidateURLWithScheme(c.DashboardAddress); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
 // StoreLabel is the config item of LabelPropertyConfig.
 type StoreLabel struct {
 	Key   string `toml:"key" json:"key"`

server/config/config_test.go

@@ -237,6 +237,71 @@ func newTestScheduleOption() (*PersistOptions, error) {
 	return opt, nil
 }
 
+func (s *testConfigSuite) TestPDServerConfig(c *C) {
+	tests := []struct {
+		cfgData          string
+		hasErr           bool
+		dashboardAddress string
+	}{
+		{
+			`
+[pd-server]
+dashboard-address = "http://127.0.0.1:2379"
+`,
+			false,
+			"http://127.0.0.1:2379",
+		},
+		{
+			`
+[pd-server]
+dashboard-address = "auto"
+`,
+			false,
+			"auto",
+		},
+		{
+			`
+[pd-server]
+dashboard-address = "none"
+`,
+			false,
+			"none",
+		},
+		{
+			"",
+			false,
+			"auto",
+		},
+		{
+			`
+[pd-server]
+dashboard-address = "127.0.0.1:2379"
+`,
+			true,
+			"",
+		},
+		{
+			`
+[pd-server]
+dashboard-address = "foo"
+`,
+			true,
+			"",
+		},
+	}
+
+	for _, t := range tests {
+		cfg := NewConfig()
+		meta, err := toml.Decode(t.cfgData, &cfg)
+		c.Assert(err, IsNil)
+		err = cfg.Adjust(&meta)
+		c.Assert(err != nil, Equals, t.hasErr)
+		if !t.hasErr {
+			c.Assert(cfg.PDServerCfg.DashboardAddress, Equals, t.dashboardAddress)
+		}
+	}
+}
+
 func (s *testConfigSuite) TestDashboardConfig(c *C) {
 	cfgData := `
 [dashboard]

server/config/util.go

@@ -14,6 +14,7 @@
 package config
 
 import (
+	"net/url"
 	"regexp"
 
 	"github.com/pingcap/kvproto/pkg/metapb"
@@ -49,3 +50,15 @@ func ValidateLabels(labels []*metapb.StoreLabel) error {
 	}
 	return nil
 }
+
+// ValidateURLWithScheme checks the format of the URL.
+func ValidateURLWithScheme(rawurl string) error {
+	u, err := url.ParseRequestURI(rawurl)
+	if err != nil {
+		return err
+	}
+	if u.Scheme == "" || u.Host == "" {
+		return errors.Errorf("%s has no scheme", rawurl)
+	}
+	return nil
+}

server/config/util_test.go

@@ -22,7 +22,7 @@ var _ = Suite(&testUtilSuite{})
 
 type testUtilSuite struct{}
 
-func (s *testUtilSuite) TestVerifyLabels(c *C) {
+func (s *testUtilSuite) TestValidateLabels(c *C) {
 	tests := []struct {
 		label  string
 		hasErr bool
@@ -54,3 +54,25 @@ func (s *testUtilSuite) TestVerifyLabels(c *C) {
 		c.Assert(ValidateLabels([]*metapb.StoreLabel{{Key: t.label}}) != nil, Equals, t.hasErr)
 	}
 }
+
+func (s *testUtilSuite) TestValidateURLWithScheme(c *C) {
+	tests := []struct {
+		addr   string
+		hasErr bool
+	}{
+		{"", true},
+		{"foo", true},
+		{"/foo", true},
+		{"http", true},
+		{"http://", true},
+		{"http://foo", false},
+		{"https://foo", false},
+		{"http://127.0.0.1", false},
+		{"http://127.0.0.1/", false},
+		{"https://foo.com/bar", false},
+		{"https://foo.com/bar/", false},
+	}
+	for _, t := range tests {
+		c.Assert(ValidateURLWithScheme(t.addr) != nil, Equals, t.hasErr)
+	}
+}

server/server.go

@@ -595,6 +595,14 @@ func (s *Server) GetAddr() string {
 	return s.cfg.AdvertiseClientUrls
 }
 
+// GetClientScheme returns the client URL scheme
+func (s *Server) GetClientScheme() string {
+	if len(s.cfg.Security.CertPath) == 0 && len(s.cfg.Security.KeyPath) == 0 {
+		return "http"
+	}
+	return "https"
+}
+
 // GetMemberInfo returns the server member information.
 func (s *Server) GetMemberInfo() *pdpb.Member {
 	return proto.Clone(s.member.Member()).(*pdpb.Member)
@@ -786,6 +794,21 @@ func (s *Server) GetPDServerConfig() *config.PDServerConfig {
 
 // SetPDServerConfig sets the server config.
 func (s *Server) SetPDServerConfig(cfg config.PDServerConfig) error {
+	switch cfg.DashboardAddress {
+	case "auto":
+	case "none":
+	default:
+		if !strings.HasPrefix(cfg.DashboardAddress, "http") {
+			cfg.DashboardAddress = fmt.Sprintf("%s://%s", s.GetClientScheme(), cfg.DashboardAddress)
+		}
+		if !cluster.IsClientURL(cfg.DashboardAddress, s.client) {
+			return errors.Errorf("%s is not the client url of any member", cfg.DashboardAddress)
+		}
+	}
+	if err := cfg.Validate(); err != nil {
+		return err
+	}
+
 	old := s.persistOptions.GetPDServerConfig()
 	s.persistOptions.SetPDServerConfig(&cfg)
 	if err := s.persistOptions.Persist(s.storage); err != nil {