fix: support HTTP and HTTPS mixture for DM-master config items (#1217)

dm/master/server_test.go

@@ -19,7 +19,6 @@ import (
 	"database/sql"
 	"fmt"
 	"io/ioutil"
-	"net/http"
 	"os"
 	"sort"
 	"strings"
@@ -34,6 +33,7 @@ import (
 	"github.com/pingcap/parser"
 	"github.com/pingcap/parser/ast"
 	"github.com/pingcap/parser/model"
+	toolutils "github.com/pingcap/tidb-tools/pkg/utils"
 	tiddl "github.com/pingcap/tidb/ddl"
 	"github.com/pingcap/tidb/sessionctx"
 	tidbmock "github.com/pingcap/tidb/util/mock"
@@ -967,8 +967,184 @@ func (t *testMaster) TestServer(c *check.C) {
 	}), check.IsTrue)
 }
 
+func (t *testMaster) TestMasterTLS(c *check.C) {
+	masterAddr := tempurl.Alloc()[len("http://"):]
+	peerAddr := tempurl.Alloc()[len("http://"):]
+
+	// all with `https://` prefix
+	cfg := NewConfig()
+	c.Assert(cfg.Parse([]string{
+		"--name=master-tls",
+		fmt.Sprintf("--data-dir=%s", c.MkDir()),
+		fmt.Sprintf("--master-addr=https://%s", masterAddr),
+		fmt.Sprintf("--advertise-addr=https://%s", masterAddr),
+		fmt.Sprintf("--peer-urls=https://%s", peerAddr),
+		fmt.Sprintf("--advertise-peer-urls=https://%s", peerAddr),
+		fmt.Sprintf("--initial-cluster=master-tls=https://%s", peerAddr),
+		"--ssl-ca=./tls_for_test/ca.pem",
+		"--ssl-cert=./tls_for_test/dm.pem",
+		"--ssl-key=./tls_for_test/dm.key",
+	}), check.IsNil)
+	t.testTLSPrefix(c, cfg)
+	c.Assert(cfg.MasterAddr, check.Equals, masterAddr)
+	c.Assert(cfg.AdvertiseAddr, check.Equals, masterAddr)
+	c.Assert(cfg.PeerUrls, check.Equals, "https://"+peerAddr)
+	c.Assert(cfg.AdvertisePeerUrls, check.Equals, "https://"+peerAddr)
+	c.Assert(cfg.InitialCluster, check.Equals, "master-tls=https://"+peerAddr)
+
+	// no `https://` prefix for `--master-addr`
+	cfg = NewConfig()
+	c.Assert(cfg.Parse([]string{
+		"--name=master-tls",
+		fmt.Sprintf("--data-dir=%s", c.MkDir()),
+		fmt.Sprintf("--master-addr=%s", masterAddr),
+		fmt.Sprintf("--advertise-addr=https://%s", masterAddr),
+		fmt.Sprintf("--peer-urls=https://%s", peerAddr),
+		fmt.Sprintf("--advertise-peer-urls=https://%s", peerAddr),
+		fmt.Sprintf("--initial-cluster=master-tls=https://%s", peerAddr),
+		"--ssl-ca=./tls_for_test/ca.pem",
+		"--ssl-cert=./tls_for_test/dm.pem",
+		"--ssl-key=./tls_for_test/dm.key",
+	}), check.IsNil)
+	t.testTLSPrefix(c, cfg)
+
+	// no `https://` prefix for `--master-addr` and `--advertise-addr`
+	cfg = NewConfig()
+	c.Assert(cfg.Parse([]string{
+		"--name=master-tls",
+		fmt.Sprintf("--data-dir=%s", c.MkDir()),
+		fmt.Sprintf("--master-addr=%s", masterAddr),
+		fmt.Sprintf("--advertise-addr=%s", masterAddr),
+		fmt.Sprintf("--peer-urls=https://%s", peerAddr),
+		fmt.Sprintf("--advertise-peer-urls=https://%s", peerAddr),
+		fmt.Sprintf("--initial-cluster=master-tls=https://%s", peerAddr),
+		"--ssl-ca=./tls_for_test/ca.pem",
+		"--ssl-cert=./tls_for_test/dm.pem",
+		"--ssl-key=./tls_for_test/dm.key",
+	}), check.IsNil)
+	t.testTLSPrefix(c, cfg)
+
+	// no `https://` prefix for `--master-addr`, `--advertise-addr` and `--peer-urls`
+	cfg = NewConfig()
+	c.Assert(cfg.Parse([]string{
+		"--name=master-tls",
+		fmt.Sprintf("--data-dir=%s", c.MkDir()),
+		fmt.Sprintf("--master-addr=%s", masterAddr),
+		fmt.Sprintf("--advertise-addr=%s", masterAddr),
+		fmt.Sprintf("--peer-urls=%s", peerAddr),
+		fmt.Sprintf("--advertise-peer-urls=https://%s", peerAddr),
+		fmt.Sprintf("--initial-cluster=master-tls=https://%s", peerAddr),
+		"--ssl-ca=./tls_for_test/ca.pem",
+		"--ssl-cert=./tls_for_test/dm.pem",
+		"--ssl-key=./tls_for_test/dm.key",
+	}), check.IsNil)
+	t.testTLSPrefix(c, cfg)
+
+	// no `https://` prefix for `--master-addr`, `--advertise-addr`, `--peer-urls` and `--advertise-peer-urls`
+	cfg = NewConfig()
+	c.Assert(cfg.Parse([]string{
+		"--name=master-tls",
+		fmt.Sprintf("--data-dir=%s", c.MkDir()),
+		fmt.Sprintf("--master-addr=%s", masterAddr),
+		fmt.Sprintf("--advertise-addr=%s", masterAddr),
+		fmt.Sprintf("--peer-urls=%s", peerAddr),
+		fmt.Sprintf("--advertise-peer-urls=%s", peerAddr),
+		fmt.Sprintf("--initial-cluster=master-tls=https://%s", peerAddr),
+		"--ssl-ca=./tls_for_test/ca.pem",
+		"--ssl-cert=./tls_for_test/dm.pem",
+		"--ssl-key=./tls_for_test/dm.key",
+	}), check.IsNil)
+	t.testTLSPrefix(c, cfg)
+
+	// all without `https://`/`http://` prefix
+	cfg = NewConfig()
+	c.Assert(cfg.Parse([]string{
+		"--name=master-tls",
+		fmt.Sprintf("--data-dir=%s", c.MkDir()),
+		fmt.Sprintf("--master-addr=%s", masterAddr),
+		fmt.Sprintf("--advertise-addr=%s", masterAddr),
+		fmt.Sprintf("--peer-urls=%s", peerAddr),
+		fmt.Sprintf("--advertise-peer-urls=%s", peerAddr),
+		fmt.Sprintf("--initial-cluster=master-tls=%s", peerAddr),
+		"--ssl-ca=./tls_for_test/ca.pem",
+		"--ssl-cert=./tls_for_test/dm.pem",
+		"--ssl-key=./tls_for_test/dm.key",
+	}), check.IsNil)
+	t.testTLSPrefix(c, cfg)
+	c.Assert(cfg.MasterAddr, check.Equals, masterAddr)
+	c.Assert(cfg.AdvertiseAddr, check.Equals, masterAddr)
+	c.Assert(cfg.PeerUrls, check.Equals, "https://"+peerAddr)
+	c.Assert(cfg.AdvertisePeerUrls, check.Equals, "https://"+peerAddr)
+	c.Assert(cfg.InitialCluster, check.Equals, "master-tls=https://"+peerAddr)
+
+	// all with `http://` prefix, but with TLS enabled.
+	cfg = NewConfig()
+	c.Assert(cfg.Parse([]string{
+		"--name=master-tls",
+		fmt.Sprintf("--data-dir=%s", c.MkDir()),
+		fmt.Sprintf("--master-addr=http://%s", masterAddr),
+		fmt.Sprintf("--advertise-addr=http://%s", masterAddr),
+		fmt.Sprintf("--peer-urls=http://%s", peerAddr),
+		fmt.Sprintf("--advertise-peer-urls=http://%s", peerAddr),
+		fmt.Sprintf("--initial-cluster=master-tls=http://%s", peerAddr),
+		"--ssl-ca=./tls_for_test/ca.pem",
+		"--ssl-cert=./tls_for_test/dm.pem",
+		"--ssl-key=./tls_for_test/dm.key",
+	}), check.IsNil)
+	c.Assert(cfg.MasterAddr, check.Equals, masterAddr)
+	c.Assert(cfg.AdvertiseAddr, check.Equals, masterAddr)
+	c.Assert(cfg.PeerUrls, check.Equals, "https://"+peerAddr)
+	c.Assert(cfg.AdvertisePeerUrls, check.Equals, "https://"+peerAddr)
+	c.Assert(cfg.InitialCluster, check.Equals, "master-tls=https://"+peerAddr)
+
+	// different prefix for `--peer-urls` and `--initial-cluster`
+	cfg = NewConfig()
+	c.Assert(cfg.Parse([]string{
+		"--name=master-tls",
+		fmt.Sprintf("--data-dir=%s", c.MkDir()),
+		fmt.Sprintf("--master-addr=https://%s", masterAddr),
+		fmt.Sprintf("--advertise-addr=https://%s", masterAddr),
+		fmt.Sprintf("--peer-urls=https://%s", peerAddr),
+		fmt.Sprintf("--advertise-peer-urls=https://%s", peerAddr),
+		fmt.Sprintf("--initial-cluster=master-tls=http://%s", peerAddr),
+		"--ssl-ca=./tls_for_test/ca.pem",
+		"--ssl-cert=./tls_for_test/dm.pem",
+		"--ssl-key=./tls_for_test/dm.key",
+	}), check.IsNil)
+	c.Assert(cfg.MasterAddr, check.Equals, masterAddr)
+	c.Assert(cfg.AdvertiseAddr, check.Equals, masterAddr)
+	c.Assert(cfg.PeerUrls, check.Equals, "https://"+peerAddr)
+	c.Assert(cfg.AdvertisePeerUrls, check.Equals, "https://"+peerAddr)
+	c.Assert(cfg.InitialCluster, check.Equals, "master-tls=https://"+peerAddr)
+	t.testTLSPrefix(c, cfg)
+}
+
+func (t *testMaster) testTLSPrefix(c *check.C, cfg *Config) {
+	s := NewServer(cfg)
+
+	ctx, cancel := context.WithCancel(context.Background())
+	err1 := s.Start(ctx)
+	c.Assert(err1, check.IsNil)
+
+	t.testHTTPInterface(c, fmt.Sprintf("https://%s/status", cfg.AdvertiseAddr), []byte(utils.GetRawInfo()))
+	t.testHTTPInterface(c, fmt.Sprintf("https://%s/debug/pprof/", cfg.AdvertiseAddr), []byte("Types of profiles available"))
+
+	// close
+	cancel()
+	s.Close()
+
+	c.Assert(utils.WaitSomething(30, 100*time.Millisecond, func() bool {
+		return s.closed.Get()
+	}), check.IsTrue)
+}
+
 func (t *testMaster) testHTTPInterface(c *check.C, url string, contain []byte) {
-	resp, err := http.Get(url)
+	// we use HTTPS in some test cases.
+	tls, err := toolutils.NewTLS("./tls_for_test/ca.pem", "./tls_for_test/dm.pem", "./tls_for_test/dm.key", url, []string{})
+	c.Assert(err, check.IsNil)
+	cli := toolutils.ClientWithTLS(tls.TLSConfig())
+
+	resp, err := cli.Get(url)
 	c.Assert(err, check.IsNil)
 	defer resp.Body.Close()
 	c.Assert(resp.StatusCode, check.Equals, 200)

dm/master/tls_for_test/ca.pem

@@ -0,0 +1,8 @@
+-----BEGIN CERTIFICATE-----
+MIIBGDCBwAIJAOjYXLFw5V1HMAoGCCqGSM49BAMCMBQxEjAQBgNVBAMMCWxvY2Fs
+aG9zdDAgFw0yMDAzMTcxMjAwMzNaGA8yMjkzMTIzMTEyMDAzM1owFDESMBAGA1UE
+AwwJbG9jYWxob3N0MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEglCIJD8uVBfD
+kuM+UQP+VA7Srbz17WPLA0Sqc+sQ2p6fT6HYKCW60EXiZ/yEC0925iyVbXEEbX4J
+xCc2Heow5TAKBggqhkjOPQQDAgNHADBEAiAILL3Zt/3NFeDW9c9UAcJ9lc92E0ZL
+GNDuH6i19Fex3wIgT0ZMAKAFSirGGtcLu0emceuk+zVKjJzmYbsLdpj/JuQ=
+-----END CERTIFICATE-----

dm/master/tls_for_test/dm.key

@@ -0,0 +1,8 @@
+-----BEGIN EC PARAMETERS-----
+BggqhkjOPQMBBw==
+-----END EC PARAMETERS-----
+-----BEGIN EC PRIVATE KEY-----
+MHcCAQEEICF/GDtVxhTPTP501nOu4jgwGSDY01xN+61xd9MfChw+oAoGCCqGSM49
+AwEHoUQDQgAEgQOv5bQO7xK16vZWhwJqlz2vl19+AXW2Ql7KQyGiBJVSvLbyDLOr
+kIeFlHN04iqQ39SKSOSfeGSfRt6doU6IcA==
+-----END EC PRIVATE KEY-----

dm/master/tls_for_test/dm.pem

@@ -0,0 +1,10 @@
+-----BEGIN CERTIFICATE-----
+MIIBZDCCAQqgAwIBAgIJAIT/lgXUc1JqMAoGCCqGSM49BAMCMBQxEjAQBgNVBAMM
+CWxvY2FsaG9zdDAgFw0yMDAzMTcxMjAwMzNaGA8yMjkzMTIzMTEyMDAzM1owDTEL
+MAkGA1UEAwwCZG0wWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAASBA6/ltA7vErXq
+9laHAmqXPa+XX34BdbZCXspDIaIElVK8tvIMs6uQh4WUc3TiKpDf1IpI5J94ZJ9G
+3p2hTohwo0owSDAaBgNVHREEEzARgglsb2NhbGhvc3SHBH8AAAEwCwYDVR0PBAQD
+AgWgMB0GA1UdJQQWMBQGCCsGAQUFBwMCBggrBgEFBQcDATAKBggqhkjOPQQDAgNI
+ADBFAiEAx6ljJ+tNa55ypWLGNqmXlB4UdMmKmE4RSKJ8mmEelfECIG2ZmCE59rv5
+wImM6KnK+vM2QnEiISH3PeYyyRzQzycu
+-----END CERTIFICATE-----

pkg/utils/util.go

@@ -203,17 +203,14 @@ func wrapScheme(s string, https bool) string {
 	if s == "" {
 		return s
 	}
-	if strings.HasPrefix(s, "http://") || strings.HasPrefix(s, "https://") {
-		return s
-	}
+	s = UnwrapScheme(s)
 	if https {
 		return "https://" + s
 	}
 	return "http://" + s
 }
 
-// WrapSchemes adds http or https scheme to input if missing. input could be a comma-separated list
-// if input has wrong scheme, don't correct it (maybe user deliberately?)
+// WrapSchemes adds http or https scheme to input if missing. input could be a comma-separated list.
 func WrapSchemes(s string, https bool) string {
 	items := strings.Split(s, ",")
 	output := make([]string, 0, len(items))

pkg/utils/util_test.go

@@ -246,10 +246,9 @@ func (t *testUtilsSuite) TestWrapSchemes(c *C) {
 			"https://abc.com:123",
 		},
 		{
-			// if input has wrong scheme, don't correct it (maybe user deliberately?)
 			"abc.com:123,http://abc.com:123,0.0.0.0:123,https://0.0.0.0:123",
-			"http://abc.com:123,http://abc.com:123,http://0.0.0.0:123,https://0.0.0.0:123",
-			"https://abc.com:123,http://abc.com:123,https://0.0.0.0:123,https://0.0.0.0:123",
+			"http://abc.com:123,http://abc.com:123,http://0.0.0.0:123,http://0.0.0.0:123",
+			"https://abc.com:123,https://abc.com:123,https://0.0.0.0:123,https://0.0.0.0:123",
 		},
 		{
 			"",
@@ -264,17 +263,16 @@ func (t *testUtilsSuite) TestWrapSchemes(c *C) {
 }
 
 func (t *testUtilsSuite) TestWrapSchemesForInitialCluster(c *C) {
-	// no change
 	c.Assert(WrapSchemesForInitialCluster("master1=http://127.0.0.1:8291,master2=http://127.0.0.1:8292,master3=http://127.0.0.1:8293", false), Equals,
 		"master1=http://127.0.0.1:8291,master2=http://127.0.0.1:8292,master3=http://127.0.0.1:8293")
 	c.Assert(WrapSchemesForInitialCluster("master1=http://127.0.0.1:8291,master2=http://127.0.0.1:8292,master3=http://127.0.0.1:8293", true), Equals,
-		"master1=http://127.0.0.1:8291,master2=http://127.0.0.1:8292,master3=http://127.0.0.1:8293")
+		"master1=https://127.0.0.1:8291,master2=https://127.0.0.1:8292,master3=https://127.0.0.1:8293")
 
-	// add `http` or `https` for some URLs
-	c.Assert(WrapSchemesForInitialCluster("master1=http://127.0.0.1:8291,master2=127.0.0.1:8292,master3=http://127.0.0.1:8293", false), Equals,
+	// correct `http` or `https` for some URLs
+	c.Assert(WrapSchemesForInitialCluster("master1=http://127.0.0.1:8291,master2=127.0.0.1:8292,master3=https://127.0.0.1:8293", false), Equals,
 		"master1=http://127.0.0.1:8291,master2=http://127.0.0.1:8292,master3=http://127.0.0.1:8293")
-	c.Assert(WrapSchemesForInitialCluster("master1=http://127.0.0.1:8291,master2=127.0.0.1:8292,master3=http://127.0.0.1:8293", true), Equals,
-		"master1=http://127.0.0.1:8291,master2=https://127.0.0.1:8292,master3=http://127.0.0.1:8293")
+	c.Assert(WrapSchemesForInitialCluster("master1=http://127.0.0.1:8291,master2=127.0.0.1:8292,master3=https://127.0.0.1:8293", true), Equals,
+		"master1=https://127.0.0.1:8291,master2=https://127.0.0.1:8292,master3=https://127.0.0.1:8293")
 
 	// add `http` or `https` for all URLs
 	c.Assert(WrapSchemesForInitialCluster("master1=127.0.0.1:8291,master2=127.0.0.1:8292,master3=127.0.0.1:8293", false), Equals,

tests/tls/conf/dm-master1.toml

@@ -3,7 +3,7 @@ name = "master1"
 master-addr = ":8261"
 advertise-addr = "127.0.0.1:8261"
 peer-urls = "127.0.0.1:8291"
-initial-cluster = "master1=https://127.0.0.1:8291,master2=https://127.0.0.1:8292,master3=https://127.0.0.1:8293"
+initial-cluster = "master1=127.0.0.1:8291,master2=http://127.0.0.1:8292,master3=https://127.0.0.1:8293"
 
 ssl-ca = "dir-placeholer/ca.pem"
 ssl-cert = "dir-placeholer/dm.pem"

tests/tls/conf/dm-master2.toml

@@ -2,8 +2,8 @@
 name = "master2"
 master-addr = ":8361"
 advertise-addr = "127.0.0.1:8361"
-peer-urls = "127.0.0.1:8292"
-initial-cluster = "master1=https://127.0.0.1:8291,master2=https://127.0.0.1:8292,master3=https://127.0.0.1:8293"
+peer-urls = "http://127.0.0.1:8292"
+initial-cluster = "master1=https://127.0.0.1:8291,master2=http://127.0.0.1:8292,master3=127.0.0.1:8293"
 
 ssl-ca = "dir-placeholer/ca.pem"
 ssl-cert = "dir-placeholer/dm.pem"

tests/tls/conf/dm-master3.toml

@@ -2,8 +2,8 @@
 name = "master3"
 master-addr = ":8461"
 advertise-addr = "127.0.0.1:8461"
-peer-urls = "127.0.0.1:8293"
-initial-cluster = "master1=https://127.0.0.1:8291,master2=https://127.0.0.1:8292,master3=https://127.0.0.1:8293"
+peer-urls = "https://127.0.0.1:8293"
+initial-cluster = "master1=http://127.0.0.1:8291,master2=https://127.0.0.1:8292,master3=127.0.0.1:8293"
 
 ssl-ca = "dir-placeholer/ca.pem"
 ssl-cert = "dir-placeholer/dm.pem"