From 2915bd13fca960e7b52519cfa9c2b13b12d3338c Mon Sep 17 00:00:00 2001
From: Benjamin Wang <benjamin.ahrtr@gmail.com>
Date: Mon, 14 Oct 2024 17:14:20 +0100
Subject: [PATCH] update epHealthCommandFunc to reuse clientConfigFromCmd

Signed-off-by: Benjamin Wang <benjamin.ahrtr@gmail.com>
---
 client/v3/config.go                 |  24 +++++
 client/v3/config_test.go            | 132 ++++++++++++++++++++++++++++
 etcdctl/ctlv3/command/ep_command.go |  18 ++--
 3 files changed, 161 insertions(+), 13 deletions(-)

diff --git a/client/v3/config.go b/client/v3/config.go
index 89b40ce12ec..f0221774ee3 100644
--- a/client/v3/config.go
+++ b/client/v3/config.go
@@ -130,6 +130,30 @@ type AuthConfig struct {
 	Password string `json:"password"`
 }
 
+func (cs *ConfigSpec) Clone() *ConfigSpec {
+	if cs == nil {
+		return nil
+	}
+
+	clone := *cs
+
+	if len(cs.Endpoints) > 0 {
+		clone.Endpoints = make([]string, len(cs.Endpoints))
+		copy(clone.Endpoints, cs.Endpoints)
+	}
+
+	if cs.Secure != nil {
+		clone.Secure = &SecureConfig{}
+		*clone.Secure = *cs.Secure
+	}
+	if cs.Auth != nil {
+		clone.Auth = &AuthConfig{}
+		*clone.Auth = *cs.Auth
+	}
+
+	return &clone
+}
+
 func (cfg AuthConfig) Empty() bool {
 	return cfg.Username == "" && cfg.Password == ""
 }
diff --git a/client/v3/config_test.go b/client/v3/config_test.go
index 20c175d9e08..7e52fef8c44 100644
--- a/client/v3/config_test.go
+++ b/client/v3/config_test.go
@@ -16,6 +16,8 @@ package clientv3
 
 import (
 	"crypto/tls"
+	"encoding/json"
+	"reflect"
 	"testing"
 	"time"
 
@@ -168,3 +170,133 @@ func TestNewClientConfigWithSecureCfg(t *testing.T) {
 		t.Fatalf("Unexpected result client config: %v", err)
 	}
 }
+
+func TestConfigSpecClone(t *testing.T) {
+	cfgSpec := &ConfigSpec{
+		Endpoints:        []string{"ep1", "ep2", "ep3"},
+		RequestTimeout:   10 * time.Second,
+		DialTimeout:      2 * time.Second,
+		KeepAliveTime:    5 * time.Second,
+		KeepAliveTimeout: 2 * time.Second,
+
+		Secure: &SecureConfig{
+			Cert:               "path/2/cert",
+			Key:                "path/2/key",
+			Cacert:             "path/2/cacert",
+			InsecureTransport:  true,
+			InsecureSkipVerify: false,
+		},
+
+		Auth: &AuthConfig{
+			Username: "foo",
+			Password: "changeme",
+		},
+	}
+
+	testCases := []struct {
+		name          string
+		cs            *ConfigSpec
+		newEp         []string
+		newSecure     *SecureConfig
+		newAuth       *AuthConfig
+		expectedEqual bool
+	}{
+		{
+			name:          "normal case",
+			cs:            cfgSpec,
+			expectedEqual: true,
+		},
+		{
+			name:          "point to a new slice of endpoint, but with the same data",
+			cs:            cfgSpec,
+			newEp:         []string{"ep1", "ep2", "ep3"},
+			expectedEqual: true,
+		},
+		{
+			name:          "updated endpoint",
+			cs:            cfgSpec,
+			newEp:         []string{"ep1", "newep2", "ep3"},
+			expectedEqual: false,
+		},
+		{
+			name: "point to a new secureConfig, but with the same data",
+			cs:   cfgSpec,
+			newSecure: &SecureConfig{
+				Cert:               "path/2/cert",
+				Key:                "path/2/key",
+				Cacert:             "path/2/cacert",
+				InsecureTransport:  true,
+				InsecureSkipVerify: false,
+			},
+			expectedEqual: true,
+		},
+		{
+			name: "update key in secureConfig",
+			cs:   cfgSpec,
+			newSecure: &SecureConfig{
+				Cert:               "path/2/cert",
+				Key:                "newPath/2/key",
+				Cacert:             "path/2/cacert",
+				InsecureTransport:  true,
+				InsecureSkipVerify: false,
+			},
+			expectedEqual: false,
+		},
+		{
+			name: "update bool values in secureConfig",
+			cs:   cfgSpec,
+			newSecure: &SecureConfig{
+				Cert:               "path/2/cert",
+				Key:                "path/2/key",
+				Cacert:             "path/2/cacert",
+				InsecureTransport:  false,
+				InsecureSkipVerify: true,
+			},
+			expectedEqual: false,
+		},
+		{
+			name: "point to a new authConfig, but with the same data",
+			cs:   cfgSpec,
+			newAuth: &AuthConfig{
+				Username: "foo",
+				Password: "changeme",
+			},
+			expectedEqual: true,
+		},
+		{
+			name: "update authConfig",
+			cs:   cfgSpec,
+			newAuth: &AuthConfig{
+				Username: "newUser",
+				Password: "newPassword",
+			},
+			expectedEqual: false,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			dataBeforeTest, err := json.Marshal(tc.cs)
+			require.NoError(t, err)
+
+			clonedCfgSpec := tc.cs.Clone()
+			if len(tc.newEp) > 0 {
+				clonedCfgSpec.Endpoints = tc.newEp
+			}
+			if tc.newSecure != nil {
+				clonedCfgSpec.Secure = tc.newSecure
+			}
+			if tc.newAuth != nil {
+				clonedCfgSpec.Auth = tc.newAuth
+			}
+
+			actualEqual := reflect.DeepEqual(tc.cs, clonedCfgSpec)
+			require.Equal(t, tc.expectedEqual, actualEqual)
+
+			// double-check the original ConfigSpec isn't updated
+			dataAfterTest, err := json.Marshal(tc.cs)
+			require.NoError(t, err)
+			require.True(t, reflect.DeepEqual(dataBeforeTest, dataAfterTest))
+		})
+	}
+}
diff --git a/etcdctl/ctlv3/command/ep_command.go b/etcdctl/ctlv3/command/ep_command.go
index c0df89d26a8..501253c67d6 100644
--- a/etcdctl/ctlv3/command/ep_command.go
+++ b/etcdctl/ctlv3/command/ep_command.go
@@ -97,21 +97,13 @@ func epHealthCommandFunc(cmd *cobra.Command, args []string) {
 	flags.SetPflagsFromEnv(lg, "ETCDCTL", cmd.InheritedFlags())
 	initDisplayFromCmd(cmd)
 
-	sec := secureCfgFromCmd(cmd)
-	dt := dialTimeoutFromCmd(cmd)
-	ka := keepAliveTimeFromCmd(cmd)
-	kat := keepAliveTimeoutFromCmd(cmd)
-	auth := authCfgFromCmd(cmd)
+	cfgSpec := clientConfigFromCmd(cmd)
+
 	var cfgs []*clientv3.Config
 	for _, ep := range endpointsFromCluster(cmd) {
-		cfg, err := clientv3.NewClientConfig(&clientv3.ConfigSpec{
-			Endpoints:        []string{ep},
-			DialTimeout:      dt,
-			KeepAliveTime:    ka,
-			KeepAliveTimeout: kat,
-			Secure:           sec,
-			Auth:             auth,
-		}, lg)
+		cloneCfgSpec := cfgSpec.Clone()
+		cloneCfgSpec.Endpoints = []string{ep}
+		cfg, err := clientv3.NewClientConfig(cloneCfgSpec, lg)
 		if err != nil {
 			cobrautl.ExitWithError(cobrautl.ExitBadArgs, err)
 		}