Skip to content

Commit

Permalink
Merge branch 'master' into fix-test-2024-23
Browse files Browse the repository at this point in the history
  • Loading branch information
@ti-chi-bot
ti-chi-bot[bot] authored Sep 4, 2024
2 parents 02abdfa + 750d475 commit c3ec2ee
Show file tree
Hide file tree
Showing 7 changed files with 205 additions and 155 deletions.
104 changes: 45 additions & 59 deletions client/tlsutil/tlsconfig.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,45 +43,45 @@ import (
"github.com/tikv/pd/client/errs"
)

// TLSInfo stores tls configuration to connect to etcd.
type TLSInfo struct {
CertFile string
KeyFile string
TrustedCAFile string
InsecureSkipVerify bool
// tlsInfo stores tls configuration to connect to etcd.
type tlsInfo struct {
certFile string
keyFile string
trustedCAFile string
insecureSkipVerify bool

// ServerName ensures the cert matches the given host in case of discovery / virtual hosting
ServerName string
// serverName ensures the cert matches the given host in case of discovery / virtual hosting
serverName string

// CipherSuites is a list of supported cipher suites.
// cipherSuites is a list of supported cipher suites.
// If empty, Go auto-populates it by default.
// Note that cipher suites are prioritized in the given order.
CipherSuites []uint16
cipherSuites []uint16

selfCert bool

// parseFunc exists to simplify testing. Typically, parseFunc
// should be left nil. In that case, tls.X509KeyPair will be used.
parseFunc func([]byte, []byte) (tls.Certificate, error)

// AllowedCN is a CN which must be provided by a client.
AllowedCN string
// allowedCNs is a list of CNs which must be provided by a client.
allowedCNs []string
}

// ClientConfig generates a tls.Config object for use by an HTTP client.
func (info TLSInfo) ClientConfig() (*tls.Config, error) {
// clientConfig generates a tls.Config object for use by an HTTP client.
func (info tlsInfo) clientConfig() (*tls.Config, error) {
var cfg *tls.Config
var err error

if !info.Empty() {
if !info.empty() {
cfg, err = info.baseConfig()
if err != nil {
return nil, err
}
} else {
cfg = &tls.Config{ServerName: info.ServerName}
cfg = &tls.Config{ServerName: info.serverName}
}
cfg.InsecureSkipVerify = info.InsecureSkipVerify
cfg.InsecureSkipVerify = info.insecureSkipVerify

CAFiles := info.cafiles()
if len(CAFiles) > 0 {
Expand All @@ -97,36 +97,38 @@ func (info TLSInfo) ClientConfig() (*tls.Config, error) {
return cfg, nil
}

// Empty returns if the TLSInfo is unset.
func (info TLSInfo) Empty() bool {
return info.CertFile == "" && info.KeyFile == ""
// empty returns if the TLSInfo is unset.
func (info tlsInfo) empty() bool {
return info.certFile == "" && info.keyFile == ""
}

func (info TLSInfo) baseConfig() (*tls.Config, error) {
if info.KeyFile == "" || info.CertFile == "" {
return nil, fmt.Errorf("KeyFile and CertFile must both be present[key: %v, cert: %v]", info.KeyFile, info.CertFile)
func (info tlsInfo) baseConfig() (*tls.Config, error) {
if info.keyFile == "" || info.certFile == "" {
return nil, fmt.Errorf("KeyFile and CertFile must both be present[key: %v, cert: %v]", info.keyFile, info.certFile)
}

_, err := NewCert(info.CertFile, info.KeyFile, info.parseFunc)
_, err := NewCert(info.certFile, info.keyFile, info.parseFunc)
if err != nil {
return nil, err
}

cfg := &tls.Config{
MinVersion: tls.VersionTLS12,
ServerName: info.ServerName,
ServerName: info.serverName,
}

if len(info.CipherSuites) > 0 {
cfg.CipherSuites = info.CipherSuites
if len(info.cipherSuites) > 0 {
cfg.CipherSuites = info.cipherSuites
}

if info.AllowedCN != "" {
if len(info.allowedCNs) > 0 {
cfg.VerifyPeerCertificate = func(_ [][]byte, verifiedChains [][]*x509.Certificate) error {
for _, chains := range verifiedChains {
if len(chains) != 0 {
if info.AllowedCN == chains[0].Subject.CommonName {
return nil
for _, allowedCN := range info.allowedCNs {
if allowedCN == chains[0].Subject.CommonName {
return nil
}
}
}
}
Expand All @@ -137,19 +139,19 @@ func (info TLSInfo) baseConfig() (*tls.Config, error) {
// this only reloads certs when there's a client request
// TODO: support server-side refresh (e.g. inotify, SIGHUP), caching
cfg.GetCertificate = func(*tls.ClientHelloInfo) (*tls.Certificate, error) {
return NewCert(info.CertFile, info.KeyFile, info.parseFunc)
return NewCert(info.certFile, info.keyFile, info.parseFunc)
}
cfg.GetClientCertificate = func(*tls.CertificateRequestInfo) (*tls.Certificate, error) {
return NewCert(info.CertFile, info.KeyFile, info.parseFunc)
return NewCert(info.certFile, info.keyFile, info.parseFunc)
}
return cfg, nil
}

// cafiles returns a list of CA file paths.
func (info TLSInfo) cafiles() []string {
func (info tlsInfo) cafiles() []string {
cs := make([]string, 0)
if info.TrustedCAFile != "" {
cs = append(cs, info.TrustedCAFile)
if info.trustedCAFile != "" {
cs = append(cs, info.trustedCAFile)
}
return cs
}
Expand All @@ -162,8 +164,8 @@ type TLSConfig struct {
CertPath string `toml:"cert-path" json:"cert-path"`
// KeyPath is the path of file that contains X509 key in PEM format.
KeyPath string `toml:"key-path" json:"key-path"`
// CertAllowedCN is a CN which must be provided by a client
CertAllowedCN []string `toml:"cert-allowed-cn" json:"cert-allowed-cn"`
// CertAllowedCNs is the list of CN which must be provided by a client
CertAllowedCNs []string `toml:"cert-allowed-cn" json:"cert-allowed-cn"`

SSLCABytes []byte
SSLCertBytes []byte
Expand Down Expand Up @@ -194,33 +196,17 @@ func (s TLSConfig) ToTLSConfig() (*tls.Config, error) {
if len(s.CertPath) == 0 && len(s.KeyPath) == 0 {
return nil, nil
}
allowedCN, err := s.GetOneAllowedCN()
if err != nil {
return nil, err
}

tlsInfo := TLSInfo{
CertFile: s.CertPath,
KeyFile: s.KeyPath,
TrustedCAFile: s.CAPath,
AllowedCN: allowedCN,
tlsInfo := tlsInfo{
certFile: s.CertPath,
keyFile: s.KeyPath,
trustedCAFile: s.CAPath,
allowedCNs: s.CertAllowedCNs,
}

tlsConfig, err := tlsInfo.ClientConfig()
tlsConfig, err := tlsInfo.clientConfig()
if err != nil {
return nil, errs.ErrEtcdTLSConfig.Wrap(err).GenWithStackByCause()
}
return tlsConfig, nil
}

// GetOneAllowedCN only gets the first one CN.
func (s TLSConfig) GetOneAllowedCN() (string, error) {
switch len(s.CertAllowedCN) {
case 1:
return s.CertAllowedCN[0], nil
case 0:
return "", nil
default:
return "", errs.ErrSecurityConfig.FastGenByArgs("only supports one CN")
}
}
15 changes: 10 additions & 5 deletions pkg/schedule/schedulers/evict_leader_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/tikv/pd/pkg/schedule/types"
"github.com/tikv/pd/pkg/storage"
"github.com/tikv/pd/pkg/utils/operatorutil"
"github.com/tikv/pd/pkg/utils/testutil"
)

func TestEvictLeader(t *testing.T) {
Expand Down Expand Up @@ -115,7 +116,7 @@ func TestBatchEvict(t *testing.T) {
tc.AddLeaderStore(2, 0)
tc.AddLeaderStore(3, 0)
// the random might be the same, so we add 1000 regions to make sure the batch is full
for i := 1; i <= 1000; i++ {
for i := 1; i <= 10000; i++ {
tc.AddLeaderRegion(uint64(i), 1, 2, 3)
}
tc.AddLeaderRegion(6, 2, 1, 3)
Expand All @@ -124,9 +125,13 @@ func TestBatchEvict(t *testing.T) {
sl, err := CreateScheduler(types.EvictLeaderScheduler, oc, storage.NewStorageWithMemoryBackend(), ConfigSliceDecoder(types.EvictLeaderScheduler, []string{"1"}), func(string) error { return nil })
re.NoError(err)
re.True(sl.IsScheduleAllowed(tc))
ops, _ := sl.Schedule(tc, false)
re.Len(ops, 3)
testutil.Eventually(re, func() bool {
ops, _ := sl.Schedule(tc, false)
return len(ops) == 3
})
sl.(*evictLeaderScheduler).conf.Batch = 5
ops, _ = sl.Schedule(tc, false)
re.Len(ops, 5)
testutil.Eventually(re, func() bool {
ops, _ := sl.Schedule(tc, false)
return len(ops) == 5
})
}
22 changes: 3 additions & 19 deletions pkg/utils/grpcutil/grpcutil.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ type TLSConfig struct {
CertPath string `toml:"cert-path" json:"cert-path"`
// KeyPath is the path of file that contains X509 key in PEM format.
KeyPath string `toml:"key-path" json:"key-path"`
// CertAllowedCN is a CN which must be provided by a client
CertAllowedCN []string `toml:"cert-allowed-cn" json:"cert-allowed-cn"`
// CertAllowedCNs is the list of CN which must be provided by a client
CertAllowedCNs []string `toml:"cert-allowed-cn" json:"cert-allowed-cn"`

SSLCABytes []byte
SSLCertBytes []byte
Expand All @@ -65,16 +65,12 @@ func (s TLSConfig) ToTLSInfo() (*transport.TLSInfo, error) {
if len(s.CertPath) == 0 && len(s.KeyPath) == 0 {
return nil, nil
}
allowedCN, err := s.GetOneAllowedCN()
if err != nil {
return nil, err
}

return &transport.TLSInfo{
CertFile: s.CertPath,
KeyFile: s.KeyPath,
TrustedCAFile: s.CAPath,
AllowedCN: allowedCN,
AllowedCNs: s.CertAllowedCNs,
}, nil
}

Expand Down Expand Up @@ -114,18 +110,6 @@ func (s TLSConfig) ToTLSConfig() (*tls.Config, error) {
return tlsConfig, nil
}

// GetOneAllowedCN only gets the first one CN.
func (s TLSConfig) GetOneAllowedCN() (string, error) {
switch len(s.CertAllowedCN) {
case 1:
return s.CertAllowedCN[0], nil
case 0:
return "", nil
default:
return "", errs.ErrSecurityConfig.FastGenByArgs("only supports one CN")
}
}

// GetClientConn returns a gRPC client connection.
// creates a client connection to the given target. By default, it's
// a non-blocking dial (the function won't wait for connections to be
Expand Down
51 changes: 35 additions & 16 deletions server/api/region_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,9 +126,13 @@ func (suite *regionTestSuite) TestRegionCheck() {

url = fmt.Sprintf("%s/regions/check/%s", suite.urlPrefix, "down-peer")
r2 := &response.RegionsInfo{}
re.NoError(tu.ReadGetJSON(re, testDialClient, url, r2))
r2.Adjust()
re.Equal(&response.RegionsInfo{Count: 1, Regions: []response.RegionInfo{*response.NewAPIRegionInfo(r)}}, r2)
tu.Eventually(re, func() bool {
if err := tu.ReadGetJSON(re, testDialClient, url, r2); err != nil {
return false
}
r2.Adjust()
return suite.Equal(&response.RegionsInfo{Count: 1, Regions: []response.RegionInfo{*response.NewAPIRegionInfo(r)}}, r2)
})

url = fmt.Sprintf("%s/regions/check/%s", suite.urlPrefix, "pending-peer")
r3 := &response.RegionsInfo{}
Expand All @@ -146,36 +150,51 @@ func (suite *regionTestSuite) TestRegionCheck() {
mustRegionHeartbeat(re, suite.svr, r)
url = fmt.Sprintf("%s/regions/check/%s", suite.urlPrefix, "empty-region")
r5 := &response.RegionsInfo{}
re.NoError(tu.ReadGetJSON(re, testDialClient, url, r5))
r5.Adjust()
re.Equal(&response.RegionsInfo{Count: 1, Regions: []response.RegionInfo{*response.NewAPIRegionInfo(r)}}, r5)
tu.Eventually(re, func() bool {
if err := tu.ReadGetJSON(re, testDialClient, url, r5); err != nil {
return false
}
r5.Adjust()
return suite.Equal(&response.RegionsInfo{Count: 1, Regions: []response.RegionInfo{*response.NewAPIRegionInfo(r)}}, r5)
})

r = r.Clone(core.SetApproximateSize(1))
mustRegionHeartbeat(re, suite.svr, r)
url = fmt.Sprintf("%s/regions/check/%s", suite.urlPrefix, "hist-size")
r6 := make([]*histItem, 1)
re.NoError(tu.ReadGetJSON(re, testDialClient, url, &r6))
histSizes := []*histItem{{Start: 1, End: 1, Count: 1}}
re.Equal(histSizes, r6)
tu.Eventually(re, func() bool {
if err := tu.ReadGetJSON(re, testDialClient, url, &r6); err != nil {
return false
}
histSizes := []*histItem{{Start: 1, End: 1, Count: 1}}
return suite.Equal(histSizes, r6)
})

r = r.Clone(core.SetApproximateKeys(1000))
mustRegionHeartbeat(re, suite.svr, r)
url = fmt.Sprintf("%s/regions/check/%s", suite.urlPrefix, "hist-keys")
r7 := make([]*histItem, 1)
re.NoError(tu.ReadGetJSON(re, testDialClient, url, &r7))
histKeys := []*histItem{{Start: 1000, End: 1999, Count: 1}}
re.Equal(histKeys, r7)
tu.Eventually(re, func() bool {
if err := tu.ReadGetJSON(re, testDialClient, url, &r7); err != nil {
return false
}
histKeys := []*histItem{{Start: 1000, End: 1999, Count: 1}}
return suite.Equal(histKeys, r7)
})

// ref https://github.com/tikv/pd/issues/3558, we should change size to pass `NeedUpdate` for observing.
r = r.Clone(core.SetApproximateKeys(0))
mustPutStore(re, suite.svr, 2, metapb.StoreState_Offline, metapb.NodeState_Removing, []*metapb.StoreLabel{})
mustRegionHeartbeat(re, suite.svr, r)
url = fmt.Sprintf("%s/regions/check/%s", suite.urlPrefix, "offline-peer")
r8 := &response.RegionsInfo{}
re.NoError(tu.ReadGetJSON(re, testDialClient, url, r8))
r4.Adjust()
re.Equal(1, r8.Count)
re.Equal(r.GetID(), r8.Regions[0].ID)
tu.Eventually(re, func() bool {
if err := tu.ReadGetJSON(re, testDialClient, url, r8); err != nil {
return false
}
r4.Adjust()
return suite.Equal(r.GetID(), r8.Regions[0].ID) && r8.Count == 1
})
}

func (suite *regionTestSuite) TestRegions() {
Expand Down
15 changes: 6 additions & 9 deletions server/config/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -726,23 +726,20 @@ func (c *Config) GenEmbedEtcdConfig() (*embed.Config, error) {
cfg.QuotaBackendBytes = int64(c.QuotaBackendBytes)
cfg.MaxRequestBytes = c.MaxRequestBytes

allowedCN, serr := c.Security.GetOneAllowedCN()
if serr != nil {
return nil, serr
}
cfg.ClientTLSInfo.ClientCertAuth = len(c.Security.CAPath) != 0
cfg.ClientTLSInfo.TrustedCAFile = c.Security.CAPath
cfg.ClientTLSInfo.CertFile = c.Security.CertPath
cfg.ClientTLSInfo.KeyFile = c.Security.KeyPath
// Client no need to set the CN. (cfg.ClientTLSInfo.AllowedCN = allowedCN)
// Keep compatibility with https://github.com/tikv/pd/pull/2305
// Only check client cert when there are multiple CNs.
if len(c.Security.CertAllowedCNs) > 1 {
cfg.ClientTLSInfo.AllowedCNs = c.Security.CertAllowedCNs
}
cfg.PeerTLSInfo.ClientCertAuth = len(c.Security.CAPath) != 0
cfg.PeerTLSInfo.TrustedCAFile = c.Security.CAPath
cfg.PeerTLSInfo.CertFile = c.Security.CertPath
cfg.PeerTLSInfo.KeyFile = c.Security.KeyPath
// TODO: After https://github.com/etcd-io/etcd/pull/18015, AllowedCN is Deprecated.
// It will be replaced by AllowedCNs in the future to support multi cn.
// nolint:staticcheck
cfg.PeerTLSInfo.AllowedCN = allowedCN
cfg.PeerTLSInfo.AllowedCNs = c.Security.CertAllowedCNs
cfg.ForceNewCluster = c.ForceNewCluster
cfg.ZapLoggerBuilder = embed.NewZapCoreLoggerBuilder(c.Logger, c.Logger.Core(), c.LogProps.Syncer)
cfg.EnableGRPCGateway = c.EnableGRPCGateway
Expand Down
Loading

0 comments on commit c3ec2ee

Please sign in to comment.