config: validate dashboard-address (#2517) (#2522)

Signed-off-by: Zheng Xiangsheng <hundundm@gmail.com>

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 {