From 3d76524d0b40bc030219c2ee309f8fa388bb043a Mon Sep 17 00:00:00 2001 From: okJiang <819421878@qq.com> Date: Mon, 3 Jun 2024 16:00:40 +0800 Subject: [PATCH 1/5] fix https client Signed-off-by: okJiang <819421878@qq.com> --- tools/pd-ctl/pdctl/command/cluster_command.go | 8 +- tools/pd-ctl/pdctl/command/health_command.go | 8 +- tools/pd-ctl/tests/health/health_test.go | 84 +++++++++++++++++++ 3 files changed, 92 insertions(+), 8 deletions(-) diff --git a/tools/pd-ctl/pdctl/command/cluster_command.go b/tools/pd-ctl/pdctl/command/cluster_command.go index 0b3cda8a867..cd961257107 100644 --- a/tools/pd-ctl/pdctl/command/cluster_command.go +++ b/tools/pd-ctl/pdctl/command/cluster_command.go @@ -19,10 +19,10 @@ import "github.com/spf13/cobra" // NewClusterCommand return a cluster subcommand of rootCmd func NewClusterCommand() *cobra.Command { cmd := &cobra.Command{ - Use: "cluster", - Short: "show the cluster information", - PersistentPreRunE: requirePDClient, - Run: showClusterCommandFunc, + Use: "cluster", + Short: "show the cluster information", + PreRunE: requirePDClient, + Run: showClusterCommandFunc, } cmd.AddCommand(NewClusterStatusCommand()) return cmd diff --git a/tools/pd-ctl/pdctl/command/health_command.go b/tools/pd-ctl/pdctl/command/health_command.go index a10ee118397..54aaa525184 100644 --- a/tools/pd-ctl/pdctl/command/health_command.go +++ b/tools/pd-ctl/pdctl/command/health_command.go @@ -21,10 +21,10 @@ import ( // NewHealthCommand return a health subcommand of rootCmd func NewHealthCommand() *cobra.Command { m := &cobra.Command{ - Use: "health", - Short: "show all node's health information of the PD cluster", - PersistentPreRunE: requirePDClient, - Run: showHealthCommandFunc, + Use: "health", + Short: "show all node's health information of the PD cluster", + PreRunE: requirePDClient, + Run: showHealthCommandFunc, } return m } diff --git a/tools/pd-ctl/tests/health/health_test.go b/tools/pd-ctl/tests/health/health_test.go index 9150a56c91b..f1d3c7cfbf1 100644 --- a/tools/pd-ctl/tests/health/health_test.go +++ b/tools/pd-ctl/tests/health/health_test.go @@ -17,14 +17,21 @@ package health_test import ( "context" "encoding/json" + "os" + "os/exec" + "path/filepath" + "strings" "testing" "github.com/stretchr/testify/require" + "github.com/tikv/pd/pkg/utils/grpcutil" "github.com/tikv/pd/server/api" "github.com/tikv/pd/server/cluster" + "github.com/tikv/pd/server/config" pdTests "github.com/tikv/pd/tests" ctl "github.com/tikv/pd/tools/pd-ctl/pdctl" "github.com/tikv/pd/tools/pd-ctl/tests" + "go.etcd.io/etcd/pkg/transport" ) func TestHealth(t *testing.T) { @@ -68,3 +75,80 @@ func TestHealth(t *testing.T) { re.NoError(json.Unmarshal(output, &h)) re.Equal(healths, h) } + +func TestHealthTLS(t *testing.T) { + re := require.New(t) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + certPath := "../cert" + certScript := "../cert_opt.sh" + // generate certs + if err := os.Mkdir(certPath, 0755); err != nil { + t.Fatal(err) + } + if err := exec.Command(certScript, "generate", certPath).Run(); err != nil { + t.Fatal(err) + } + defer func() { + if err := exec.Command(certScript, "cleanup", certPath).Run(); err != nil { + t.Fatal(err) + } + if err := os.RemoveAll(certPath); err != nil { + t.Fatal(err) + } + }() + + tlsInfo := transport.TLSInfo{ + KeyFile: filepath.Join(certPath, "pd-server-key.pem"), + CertFile: filepath.Join(certPath, "pd-server.pem"), + TrustedCAFile: filepath.Join(certPath, "ca.pem"), + } + tc, err := pdTests.NewTestCluster(ctx, 1, func(conf *config.Config, _ string) { + conf.Security.TLSConfig = grpcutil.TLSConfig{ + KeyPath: tlsInfo.KeyFile, + CertPath: tlsInfo.CertFile, + CAPath: tlsInfo.TrustedCAFile, + } + conf.AdvertiseClientUrls = strings.ReplaceAll(conf.AdvertiseClientUrls, "http", "https") + conf.ClientUrls = strings.ReplaceAll(conf.ClientUrls, "http", "https") + conf.AdvertisePeerUrls = strings.ReplaceAll(conf.AdvertisePeerUrls, "http", "https") + conf.PeerUrls = strings.ReplaceAll(conf.PeerUrls, "http", "https") + conf.InitialCluster = strings.ReplaceAll(conf.InitialCluster, "http", "https") + }) + re.NoError(err) + defer tc.Destroy() + err = tc.RunInitialServers() + re.NoError(err) + tc.WaitLeader() + cmd := ctl.GetRootCmd() + + client := tc.GetEtcdClient() + members, err := cluster.GetMembers(client) + re.NoError(err) + healthMembers := cluster.CheckHealth(tc.GetHTTPClient(), members) + healths := []api.Health{} + for _, member := range members { + h := api.Health{ + Name: member.Name, + MemberID: member.MemberId, + ClientUrls: member.ClientUrls, + Health: false, + } + if _, ok := healthMembers[member.GetMemberId()]; ok { + h.Health = true + } + healths = append(healths, h) + } + + pdAddr := tc.GetConfig().GetClientURL() + pdAddr = strings.ReplaceAll(pdAddr, "http", "https") + args := []string{"-u", pdAddr, "health", + "--cacert=../cert/ca.pem", + "--cert=../cert/client.pem", + "--key=../cert/client-key.pem"} + output, err := tests.ExecuteCommand(cmd, args...) + re.NoError(err) + h := make([]api.Health, len(healths)) + re.NoError(json.Unmarshal(output, &h)) + re.Equal(healths, h) +} From 1d078989d7de5828a03e80baece0a21f211f9a02 Mon Sep 17 00:00:00 2001 From: okJiang <819421878@qq.com> Date: Mon, 3 Jun 2024 18:22:05 +0800 Subject: [PATCH 2/5] fix comment & add one ut Signed-off-by: okJiang <819421878@qq.com> --- tools/pd-ctl/pdctl/command/cluster_command.go | 8 +- tools/pd-ctl/pdctl/command/global.go | 80 +++++++++---------- tools/pd-ctl/pdctl/command/global_test.go | 58 ++++++++++++++ tools/pd-ctl/pdctl/command/health_command.go | 8 +- 4 files changed, 105 insertions(+), 49 deletions(-) create mode 100644 tools/pd-ctl/pdctl/command/global_test.go diff --git a/tools/pd-ctl/pdctl/command/cluster_command.go b/tools/pd-ctl/pdctl/command/cluster_command.go index cd961257107..0b3cda8a867 100644 --- a/tools/pd-ctl/pdctl/command/cluster_command.go +++ b/tools/pd-ctl/pdctl/command/cluster_command.go @@ -19,10 +19,10 @@ import "github.com/spf13/cobra" // NewClusterCommand return a cluster subcommand of rootCmd func NewClusterCommand() *cobra.Command { cmd := &cobra.Command{ - Use: "cluster", - Short: "show the cluster information", - PreRunE: requirePDClient, - Run: showClusterCommandFunc, + Use: "cluster", + Short: "show the cluster information", + PersistentPreRunE: requirePDClient, + Run: showClusterCommandFunc, } cmd.AddCommand(NewClusterStatusCommand()) return cmd diff --git a/tools/pd-ctl/pdctl/command/global.go b/tools/pd-ctl/pdctl/command/global.go index f7c04c3ca5c..545e1f05aac 100644 --- a/tools/pd-ctl/pdctl/command/global.go +++ b/tools/pd-ctl/pdctl/command/global.go @@ -55,23 +55,20 @@ var PDCli pd.Client func requirePDClient(cmd *cobra.Command, _ []string) error { var ( - caPath string - err error + tlsConfig *tls.Config + err error ) - caPath, err = cmd.Flags().GetString("cacert") - if err == nil && len(caPath) != 0 { - var certPath, keyPath string - certPath, err = cmd.Flags().GetString("cert") - if err != nil { - return err - } - keyPath, err = cmd.Flags().GetString("key") - if err != nil { + tlsConfig, err = parseTLSConfig(cmd) + if err != nil { + return err + } + if tlsConfig != nil { + if err = initHTTPSClient(tlsConfig); err != nil { return err } - return initNewPDClientWithTLS(cmd, caPath, certPath, keyPath) } - return initNewPDClient(cmd) + + return initNewPDClient(cmd, pd.WithTLSConfig(tlsConfig)) } // shouldInitPDClient checks whether we should create a new PD client according to the cluster information. @@ -111,46 +108,47 @@ func initNewPDClient(cmd *cobra.Command, opts ...pd.ClientOption) error { return nil } -func initNewPDClientWithTLS(cmd *cobra.Command, caPath, certPath, keyPath string) error { - tlsConfig, err := initTLSConfig(caPath, certPath, keyPath) - if err != nil { - return err - } - initNewPDClient(cmd, pd.WithTLSConfig(tlsConfig)) - return nil -} - // TODO: replace dialClient with the PD HTTP client completely. var dialClient = &http.Client{ Transport: apiutil.NewCallerIDRoundTripper(http.DefaultTransport, PDControlCallerID), } -// RequireHTTPSClient creates a HTTPS client if the related flags are set -func RequireHTTPSClient(cmd *cobra.Command, _ []string) error { +func parseTLSConfig(cmd *cobra.Command) (*tls.Config, error) { caPath, err := cmd.Flags().GetString("cacert") - if err == nil && len(caPath) != 0 { - certPath, err := cmd.Flags().GetString("cert") - if err != nil { - return err - } - keyPath, err := cmd.Flags().GetString("key") - if err != nil { - return err - } - err = initHTTPSClient(caPath, certPath, keyPath) - if err != nil { - cmd.Println(err) - return err - } + if err != nil || len(caPath) == 0 { + return nil, err } - return nil + certPath, err := cmd.Flags().GetString("cert") + if err != nil { + return nil, err + } + keyPath, err := cmd.Flags().GetString("key") + if err != nil { + return nil, err + } + tlsConfig, err := initTLSConfig(caPath, certPath, keyPath) + if err != nil { + return nil, err + } + + return tlsConfig, nil } -func initHTTPSClient(caPath, certPath, keyPath string) error { - tlsConfig, err := initTLSConfig(caPath, certPath, keyPath) +// RequireHTTPSClient creates a HTTPS client if the related flags are set +func RequireHTTPSClient(cmd *cobra.Command, _ []string) error { + tlsConfig, err := parseTLSConfig(cmd) + if err != nil || tlsConfig == nil { + return err + } + err = initHTTPSClient(tlsConfig) if err != nil { + cmd.Println(err) return err } + return nil +} + +func initHTTPSClient(tlsConfig *tls.Config) error { dialClient = &http.Client{ Transport: apiutil.NewCallerIDRoundTripper( &http.Transport{TLSClientConfig: tlsConfig}, PDControlCallerID), diff --git a/tools/pd-ctl/pdctl/command/global_test.go b/tools/pd-ctl/pdctl/command/global_test.go new file mode 100644 index 00000000000..86eb4366d04 --- /dev/null +++ b/tools/pd-ctl/pdctl/command/global_test.go @@ -0,0 +1,58 @@ +// Copyright 2024 TiKV Project Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package command + +import ( + "os" + "os/exec" + "testing" + + "github.com/spf13/cobra" + "github.com/stretchr/testify/require" +) + +func TestParseTLSConfig(t *testing.T) { + re := require.New(t) + + rootCmd := &cobra.Command{ + Use: "pd-ctl", + Short: "Placement Driver control", + SilenceErrors: true, + } + certPath := "../../tests/cert" + rootCmd.Flags().String("cacert", certPath+"/ca.pem", "path of file that contains list of trusted SSL CAs") + rootCmd.Flags().String("cert", certPath+"/client.pem", "path of file that contains X509 certificate in PEM format") + rootCmd.Flags().String("key", certPath+"/client-key.pem", "path of file that contains X509 key in PEM format") + + // generate certs + if err := os.Mkdir(certPath, 0755); err != nil { + t.Fatal(err) + } + certScript := "../../tests/cert_opt.sh" + if err := exec.Command(certScript, "generate", certPath).Run(); err != nil { + t.Fatal(err) + } + defer func() { + if err := exec.Command(certScript, "cleanup", certPath).Run(); err != nil { + t.Fatal(err) + } + if err := os.RemoveAll(certPath); err != nil { + t.Fatal(err) + } + }() + + tlsConfig, err := parseTLSConfig(rootCmd) + re.NoError(err) + re.NotNil(tlsConfig) +} diff --git a/tools/pd-ctl/pdctl/command/health_command.go b/tools/pd-ctl/pdctl/command/health_command.go index 54aaa525184..a10ee118397 100644 --- a/tools/pd-ctl/pdctl/command/health_command.go +++ b/tools/pd-ctl/pdctl/command/health_command.go @@ -21,10 +21,10 @@ import ( // NewHealthCommand return a health subcommand of rootCmd func NewHealthCommand() *cobra.Command { m := &cobra.Command{ - Use: "health", - Short: "show all node's health information of the PD cluster", - PreRunE: requirePDClient, - Run: showHealthCommandFunc, + Use: "health", + Short: "show all node's health information of the PD cluster", + PersistentPreRunE: requirePDClient, + Run: showHealthCommandFunc, } return m } From 44688fc5c17554d8e9c568bccacab362816d6f1c Mon Sep 17 00:00:00 2001 From: okJiang <819421878@qq.com> Date: Tue, 4 Jun 2024 11:15:11 +0800 Subject: [PATCH 3/5] EnableTraverseRunHooks Signed-off-by: okJiang <819421878@qq.com> --- tools/pd-ctl/pdctl/command/global.go | 5 ----- tools/pd-ctl/pdctl/ctl.go | 1 + 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/tools/pd-ctl/pdctl/command/global.go b/tools/pd-ctl/pdctl/command/global.go index 545e1f05aac..2aa563d2d51 100644 --- a/tools/pd-ctl/pdctl/command/global.go +++ b/tools/pd-ctl/pdctl/command/global.go @@ -62,11 +62,6 @@ func requirePDClient(cmd *cobra.Command, _ []string) error { if err != nil { return err } - if tlsConfig != nil { - if err = initHTTPSClient(tlsConfig); err != nil { - return err - } - } return initNewPDClient(cmd, pd.WithTLSConfig(tlsConfig)) } diff --git a/tools/pd-ctl/pdctl/ctl.go b/tools/pd-ctl/pdctl/ctl.go index f8eaff5e76e..fbacd65dc53 100644 --- a/tools/pd-ctl/pdctl/ctl.go +++ b/tools/pd-ctl/pdctl/ctl.go @@ -30,6 +30,7 @@ import ( func init() { cobra.EnablePrefixMatching = true + cobra.EnableTraverseRunHooks = true } // GetRootCmd is exposed for integration tests. But it can be embedded into another suite, too. From 9cb829add06912b66efaffa4647f22370afd238f Mon Sep 17 00:00:00 2001 From: okJiang <819421878@qq.com> Date: Tue, 4 Jun 2024 11:42:05 +0800 Subject: [PATCH 4/5] fix comment Signed-off-by: okJiang <819421878@qq.com> --- tools/pd-ctl/pdctl/command/global.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/tools/pd-ctl/pdctl/command/global.go b/tools/pd-ctl/pdctl/command/global.go index 2aa563d2d51..b29e2b63278 100644 --- a/tools/pd-ctl/pdctl/command/global.go +++ b/tools/pd-ctl/pdctl/command/global.go @@ -135,15 +135,6 @@ func RequireHTTPSClient(cmd *cobra.Command, _ []string) error { if err != nil || tlsConfig == nil { return err } - err = initHTTPSClient(tlsConfig) - if err != nil { - cmd.Println(err) - return err - } - return nil -} - -func initHTTPSClient(tlsConfig *tls.Config) error { dialClient = &http.Client{ Transport: apiutil.NewCallerIDRoundTripper( &http.Transport{TLSClientConfig: tlsConfig}, PDControlCallerID), From ff1b88a0393eb8db7792d438aeffa2ab21a8192f Mon Sep 17 00:00:00 2001 From: okJiang <819421878@qq.com> Date: Tue, 4 Jun 2024 14:20:55 +0800 Subject: [PATCH 5/5] empty Signed-off-by: okJiang <819421878@qq.com>