From 85a49d5d9979b2bd457fba926c14764533f897be Mon Sep 17 00:00:00 2001 From: pingcap-github-bot Date: Mon, 15 Jun 2020 13:00:01 +0800 Subject: [PATCH] config: validate dashboard-address (#2517) (#2522) Signed-off-by: Zheng Xiangsheng --- pkg/dashboard/adapter/manager.go | 6 --- pkg/dashboard/adapter/redirector.go | 2 +- server/cluster/cluster.go | 16 +++++++ server/config/config.go | 18 +++++++- server/config/config_test.go | 65 +++++++++++++++++++++++++++++ server/config/util.go | 13 ++++++ server/config/util_test.go | 24 ++++++++++- server/server.go | 23 ++++++++++ 8 files changed, 157 insertions(+), 10 deletions(-) diff --git a/pkg/dashboard/adapter/manager.go b/pkg/dashboard/adapter/manager.go index 4ee2f281fd6..f469a1ae6f5 100644 --- a/pkg/dashboard/adapter/manager.go +++ b/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 diff --git a/pkg/dashboard/adapter/redirector.go b/pkg/dashboard/adapter/redirector.go index 20d0350e21d..c24ddef04bd 100644 --- a/pkg/dashboard/adapter/redirector.go +++ b/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 diff --git a/server/cluster/cluster.go b/server/cluster/cluster.go index 8df117597ed..b614087defc 100644 --- a/server/cluster/cluster.go +++ b/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 +} diff --git a/server/config/config.go b/server/config/config.go index 6bc8f8a3b73..433dcfa25d9 100644 --- a/server/config/config.go +++ b/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"` diff --git a/server/config/config_test.go b/server/config/config_test.go index c908672f5b6..e437bdcca0b 100644 --- a/server/config/config_test.go +++ b/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] diff --git a/server/config/util.go b/server/config/util.go index bdcb4747427..498a475d807 100644 --- a/server/config/util.go +++ b/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 +} diff --git a/server/config/util_test.go b/server/config/util_test.go index 2bc0f1aa344..399de3aa748 100644 --- a/server/config/util_test.go +++ b/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) + } +} diff --git a/server/server.go b/server/server.go index aa369bb4bd3..488253f7a33 100644 --- a/server/server.go +++ b/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 {