move the newClientCfg into clientv3 package so as to be reused by both etcdctl and v3discovery

client/v3/config.go

@@ -19,6 +19,7 @@ import (
 	"crypto/tls"
 	"time"
 
+	"go.etcd.io/etcd/client/pkg/v3/transport"
 	"go.uber.org/zap"
 	"google.golang.org/grpc"
 )
@@ -118,3 +119,65 @@ type AuthConfig struct {
 	Username string `json:"username"`
 	Password string `json:"password"`
 }
+
+// NewClientConfig creates a Config based on the provided ConfigSpec.
+func NewClientConfig(confSpec *ConfigSpec, lg *zap.Logger) (*Config, error) {
+	tlsCfg, err := newTLSConfig(confSpec.Secure, lg)
+	if err != nil {
+		return nil, err
+	}
+
+	cfg := &Config{
+		Endpoints:            confSpec.Endpoints,
+		DialTimeout:          confSpec.DialTimeout,
+		DialKeepAliveTime:    confSpec.KeepAliveTime,
+		DialKeepAliveTimeout: confSpec.KeepAliveTimeout,
+		TLS:                  tlsCfg,
+	}
+
+	if confSpec.Auth != nil {
+		cfg.Username = confSpec.Auth.Username
+		cfg.Password = confSpec.Auth.Password
+	}
+
+	return cfg, nil
+}
+
+func newTLSConfig(scfg *SecureConfig, lg *zap.Logger) (*tls.Config, error) {
+	var (
+		tlsCfg *tls.Config
+		err    error
+	)
+
+	if scfg == nil {
+		return nil, nil
+	}
+
+	if scfg.Cert != "" || scfg.Key != "" || scfg.Cacert != "" || scfg.ServerName != "" {
+		cfgtls := &transport.TLSInfo{
+			CertFile:      scfg.Cert,
+			KeyFile:       scfg.Key,
+			TrustedCAFile: scfg.Cacert,
+			ServerName:    scfg.ServerName,
+			Logger:        lg,
+		}
+		if tlsCfg, err = cfgtls.ClientConfig(); err != nil {
+			return nil, err
+		}
+	}
+
+	// If key/cert is not given but user wants secure connection, we
+	// should still setup an empty tls configuration for gRPC to setup
+	// secure connection.
+	if tlsCfg == nil && !scfg.InsecureTransport {
+		tlsCfg = &tls.Config{}
+	}
+
+	// If the user wants to skip TLS verification then we should set
+	// the InsecureSkipVerify flag in tls configuration.
+	if tlsCfg != nil && scfg.InsecureSkipVerify {
+		tlsCfg.InsecureSkipVerify = true
+	}
+
+	return tlsCfg, nil
+}

client/v3/go.mod

@@ -6,6 +6,7 @@ require (
 	github.com/dustin/go-humanize v1.0.0
 	github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0
 	github.com/prometheus/client_golang v1.11.0
+	github.com/stretchr/testify v1.7.0
 	go.etcd.io/etcd/api/v3 v3.6.0-alpha.0
 	go.etcd.io/etcd/client/pkg/v3 v3.6.0-alpha.0
 	go.uber.org/zap v1.17.0
@@ -26,7 +27,6 @@ require (
 	github.com/prometheus/client_model v0.2.0 // indirect
 	github.com/prometheus/common v0.26.0 // indirect
 	github.com/prometheus/procfs v0.6.0 // indirect
-	github.com/stretchr/testify v1.7.0 // indirect
 	go.uber.org/atomic v1.7.0 // indirect
 	go.uber.org/multierr v1.6.0 // indirect
 	golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4 // indirect

etcdctl/ctlv3/command/ep_command.go

@@ -22,7 +22,7 @@ import (
 
 	"go.etcd.io/etcd/api/v3/etcdserverpb"
 	"go.etcd.io/etcd/api/v3/v3rpc/rpctypes"
-	v3 "go.etcd.io/etcd/client/v3"
+	"go.etcd.io/etcd/client/v3"
 	"go.etcd.io/etcd/pkg/v3/cobrautl"
 	"go.etcd.io/etcd/pkg/v3/flags"
 
@@ -100,9 +100,16 @@ func epHealthCommandFunc(cmd *cobra.Command, args []string) {
 	ka := keepAliveTimeFromCmd(cmd)
 	kat := keepAliveTimeoutFromCmd(cmd)
 	auth := authCfgFromCmd(cmd)
-	cfgs := []*v3.Config{}
+	cfgs := []*clientv3.Config{}
 	for _, ep := range endpointsFromCluster(cmd) {
-		cfg, err := newClientCfg([]string{ep}, dt, ka, kat, sec, auth)
+		cfg, err := clientv3.NewClientConfig(&clientv3.ConfigSpec{
+			Endpoints:        []string{ep},
+			DialTimeout:      dt,
+			KeepAliveTime:    ka,
+			KeepAliveTimeout: kat,
+			Secure:           sec,
+			Auth:             auth,
+		}, lg)
 		if err != nil {
 			cobrautl.ExitWithError(cobrautl.ExitBadArgs, err)
 		}
@@ -113,11 +120,11 @@ func epHealthCommandFunc(cmd *cobra.Command, args []string) {
 	hch := make(chan epHealth, len(cfgs))
 	for _, cfg := range cfgs {
 		wg.Add(1)
-		go func(cfg *v3.Config) {
+		go func(cfg *clientv3.Config) {
 			defer wg.Done()
 			ep := cfg.Endpoints[0]
 			cfg.Logger = lg.Named("client")
-			cli, err := v3.New(*cfg)
+			cli, err := clientv3.New(*cfg)
 			if err != nil {
 				hch <- epHealth{Ep: ep, Health: false, Error: err.Error()}
 				return
@@ -178,8 +185,8 @@ func epHealthCommandFunc(cmd *cobra.Command, args []string) {
 }
 
 type epStatus struct {
-	Ep   string             `json:"Endpoint"`
+	Ep   string                   `json:"Endpoint"`
-	Resp *v3.StatusResponse `json:"Status"`
+	Resp *clientv3.StatusResponse `json:"Status"`
 }
 
 func epStatusCommandFunc(cmd *cobra.Command, args []string) {
@@ -207,8 +214,8 @@ func epStatusCommandFunc(cmd *cobra.Command, args []string) {
 }
 
 type epHashKV struct {
-	Ep   string             `json:"Endpoint"`
+	Ep   string                   `json:"Endpoint"`
-	Resp *v3.HashKVResponse `json:"HashKV"`
+	Resp *clientv3.HashKVResponse `json:"HashKV"`
 }
 
 func epHashKVCommandFunc(cmd *cobra.Command, args []string) {
@@ -253,12 +260,18 @@ func endpointsFromCluster(cmd *cobra.Command) []string {
 		cobrautl.ExitWithError(cobrautl.ExitError, err)
 	}
 	// exclude auth for not asking needless password (MemberList() doesn't need authentication)
-
+	lg, _ := zap.NewProduction()
-	cfg, err := newClientCfg(eps, dt, ka, kat, sec, nil)
+	cfg, err := clientv3.NewClientConfig(&clientv3.ConfigSpec{
+		Endpoints:        eps,
+		DialTimeout:      dt,
+		KeepAliveTime:    ka,
+		KeepAliveTimeout: kat,
+		Secure:           sec,
+	}, lg)
 	if err != nil {
 		cobrautl.ExitWithError(cobrautl.ExitError, err)
 	}
-	c, err := v3.New(*cfg)
+	c, err := clientv3.New(*cfg)
 	if err != nil {
 		cobrautl.ExitWithError(cobrautl.ExitError, err)
 	}

etcdctl/ctlv3/command/global.go

@@ -15,7 +15,6 @@
 package command
 
 import (
-	"crypto/tls"
 	"errors"
 	"fmt"
 	"io"
@@ -138,7 +137,8 @@ func clientConfigFromCmd(cmd *cobra.Command) *clientv3.ConfigSpec {
 
 func mustClientCfgFromCmd(cmd *cobra.Command) *clientv3.Config {
 	cc := clientConfigFromCmd(cmd)
-	cfg, err := newClientCfg(cc.Endpoints, cc.DialTimeout, cc.KeepAliveTime, cc.KeepAliveTimeout, cc.Secure, cc.Auth)
+	lg, _ := zap.NewProduction()
+	cfg, err := clientv3.NewClientConfig(cc, lg)
 	if err != nil {
 		cobrautl.ExitWithError(cobrautl.ExitBadArgs, err)
 	}
@@ -151,7 +151,8 @@ func mustClientFromCmd(cmd *cobra.Command) *clientv3.Client {
 }
 
 func mustClient(cc *clientv3.ConfigSpec) *clientv3.Client {
-	cfg, err := newClientCfg(cc.Endpoints, cc.DialTimeout, cc.KeepAliveTime, cc.KeepAliveTimeout, cc.Secure, cc.Auth)
+	lg, _ := zap.NewProduction()
+	cfg, err := clientv3.NewClientConfig(cc, lg)
 	if err != nil {
 		cobrautl.ExitWithError(cobrautl.ExitBadArgs, err)
 	}
@@ -164,67 +165,6 @@ func mustClient(cc *clientv3.ConfigSpec) *clientv3.Client {
 	return client
 }
 
-func newClientCfg(endpoints []string, dialTimeout, keepAliveTime, keepAliveTimeout time.Duration, scfg *clientv3.SecureConfig, acfg *clientv3.AuthConfig) (*clientv3.Config, error) {
-	// set tls if any one tls option set
-	var cfgtls *transport.TLSInfo
-	tlsinfo := transport.TLSInfo{}
-	tlsinfo.Logger, _ = zap.NewProduction()
-	if scfg.Cert != "" {
-		tlsinfo.CertFile = scfg.Cert
-		cfgtls = &tlsinfo
-	}
-
-	if scfg.Key != "" {
-		tlsinfo.KeyFile = scfg.Key
-		cfgtls = &tlsinfo
-	}
-
-	if scfg.Cacert != "" {
-		tlsinfo.TrustedCAFile = scfg.Cacert
-		cfgtls = &tlsinfo
-	}
-
-	if scfg.ServerName != "" {
-		tlsinfo.ServerName = scfg.ServerName
-		cfgtls = &tlsinfo
-	}
-
-	cfg := &clientv3.Config{
-		Endpoints:            endpoints,
-		DialTimeout:          dialTimeout,
-		DialKeepAliveTime:    keepAliveTime,
-		DialKeepAliveTimeout: keepAliveTimeout,
-	}
-
-	if cfgtls != nil {
-		clientTLS, err := cfgtls.ClientConfig()
-		if err != nil {
-			return nil, err
-		}
-		cfg.TLS = clientTLS
-	}
-
-	// if key/cert is not given but user wants secure connection, we
-	// should still setup an empty tls configuration for gRPC to setup
-	// secure connection.
-	if cfg.TLS == nil && !scfg.InsecureTransport {
-		cfg.TLS = &tls.Config{}
-	}
-
-	// If the user wants to skip TLS verification then we should set
-	// the InsecureSkipVerify flag in tls configuration.
-	if scfg.InsecureSkipVerify && cfg.TLS != nil {
-		cfg.TLS.InsecureSkipVerify = true
-	}
-
-	if acfg != nil {
-		cfg.Username = acfg.Username
-		cfg.Password = acfg.Password
-	}
-
-	return cfg, nil
-}
-
 func argOrStdin(args []string, stdin io.Reader, i int) (string, error) {
 	if i < len(args) {
 		return args[i], nil

etcdctl/ctlv3/command/global_test.go

@@ -1,131 +0,0 @@
-// Copyright 2022 The etcd 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 (
-	"crypto/tls"
-
-	"go.uber.org/zap"
-	"testing"
-	"time"
-
-	"github.com/stretchr/testify/assert"
-	"go.etcd.io/etcd/client/pkg/v3/transport"
-	clientv3 "go.etcd.io/etcd/client/v3"
-)
-
-func TestNewClientConfig(t *testing.T) {
-	cases := []struct {
-		name         string
-		spec         clientv3.ConfigSpec
-		expectedConf clientv3.Config
-	}{
-		{
-			name: "default secure transport",
-			spec: clientv3.ConfigSpec{
-				Endpoints:        []string{"http://192.168.0.10:2379"},
-				DialTimeout:      2 * time.Second,
-				KeepAliveTime:    3 * time.Second,
-				KeepAliveTimeout: 5 * time.Second,
-				Secure: &clientv3.SecureConfig{
-					InsecureTransport: false,
-				},
-			},
-			expectedConf: clientv3.Config{
-				Endpoints:            []string{"http://192.168.0.10:2379"},
-				DialTimeout:          2 * time.Second,
-				DialKeepAliveTime:    3 * time.Second,
-				DialKeepAliveTimeout: 5 * time.Second,
-				TLS:                  &tls.Config{},
-			},
-		},
-		{
-			name: "default secure transport and auth enabled",
-			spec: clientv3.ConfigSpec{
-				Endpoints:        []string{"http://192.168.0.12:2379"},
-				DialTimeout:      1 * time.Second,
-				KeepAliveTime:    4 * time.Second,
-				KeepAliveTimeout: 6 * time.Second,
-				Secure: &clientv3.SecureConfig{
-					InsecureTransport: false,
-				},
-				Auth: &clientv3.AuthConfig{
-					Username: "test",
-					Password: "changeme",
-				},
-			},
-			expectedConf: clientv3.Config{
-				Endpoints:            []string{"http://192.168.0.12:2379"},
-				DialTimeout:          1 * time.Second,
-				DialKeepAliveTime:    4 * time.Second,
-				DialKeepAliveTimeout: 6 * time.Second,
-				TLS:                  &tls.Config{},
-				Username:             "test",
-				Password:             "changeme",
-			},
-		},
-		{
-			name: "default secure transport and skip TLS verification",
-			spec: clientv3.ConfigSpec{
-				Endpoints:        []string{"http://192.168.0.13:2379"},
-				DialTimeout:      1 * time.Second,
-				KeepAliveTime:    3 * time.Second,
-				KeepAliveTimeout: 5 * time.Second,
-				Secure: &clientv3.SecureConfig{
-					InsecureTransport:  false,
-					InsecureSkipVerify: true,
-				},
-			},
-			expectedConf: clientv3.Config{
-				Endpoints:            []string{"http://192.168.0.13:2379"},
-				DialTimeout:          1 * time.Second,
-				DialKeepAliveTime:    3 * time.Second,
-				DialKeepAliveTimeout: 5 * time.Second,
-				TLS: &tls.Config{
-					InsecureSkipVerify: true,
-				},
-			},
-		},
-	}
-
-	for _, tc := range cases {
-		t.Run(tc.name, func(t *testing.T) {
-			cfg, err := newClientCfg(tc.spec.Endpoints, tc.spec.DialTimeout, tc.spec.KeepAliveTime, tc.spec.KeepAliveTimeout, tc.spec.Secure, tc.spec.Auth)
-			if err != nil {
-				t.Fatalf("Unexpected error: %v", err)
-			}
-
-			assert.Equal(t, tc.expectedConf, *cfg)
-		})
-	}
-}
-
-func TestNewClientConfigWithSecureCfg(t *testing.T) {
-	tls, err := transport.SelfCert(zap.NewNop(), t.TempDir(), []string{"localhost"}, 1)
-	if err != nil {
-		t.Fatalf("Unexpected error: %v", err)
-	}
-
-	scfg := &clientv3.SecureConfig{
-		Cert:   tls.CertFile,
-		Key:    tls.KeyFile,
-		Cacert: tls.TrustedCAFile,
-	}
-
-	cfg, err := newClientCfg([]string{"http://192.168.0.13:2379"}, 2*time.Second, 3*time.Second, 5*time.Second, scfg, nil)
-	if cfg == nil || err != nil {
-		t.Fatalf("Unexpected result client config: %v", err)
-	}
-}

server/etcdserver/api/v3discovery/discovery.go

@@ -18,7 +18,6 @@ package v3discovery
 
 import (
 	"context"
-	"crypto/tls"
 	"errors"
 
 	"math"
@@ -28,7 +27,6 @@ import (
 	"strings"
 	"time"
 
-	"go.etcd.io/etcd/client/pkg/v3/transport"
 	"go.etcd.io/etcd/client/pkg/v3/types"
 	"go.etcd.io/etcd/client/v3"
 
@@ -173,7 +171,7 @@ func newDiscovery(lg *zap.Logger, dcfg *DiscoveryConfig, id types.ID) (*discover
 	}
 
 	lg = lg.With(zap.String("discovery-token", dcfg.Token), zap.String("discovery-endpoints", strings.Join(dcfg.Endpoints, ",")))
-	cfg, err := newClientCfg(dcfg, lg)
+	cfg, err := clientv3.NewClientConfig(&dcfg.ConfigSpec, lg)
 	if err != nil {
 		return nil, err
 	}
@@ -192,53 +190,6 @@ func newDiscovery(lg *zap.Logger, dcfg *DiscoveryConfig, id types.ID) (*discover
 	}, nil
 }
 
-// The following function follows the same logic as etcdctl, refer to
-// https://github.com/etcd-io/etcd/blob/f9a8c49c695b098d66a07948666664ea10d01a82/etcdctl/ctlv3/command/global.go#L191-L250
-func newClientCfg(dcfg *DiscoveryConfig, lg *zap.Logger) (*clientv3.Config, error) {
-	var cfgtls *transport.TLSInfo
-
-	if dcfg.Secure.Cert != "" || dcfg.Secure.Key != "" || dcfg.Secure.Cacert != "" {
-		cfgtls = &transport.TLSInfo{
-			CertFile:      dcfg.Secure.Cert,
-			KeyFile:       dcfg.Secure.Key,
-			TrustedCAFile: dcfg.Secure.Cacert,
-			Logger:        lg,
-		}
-	}
-
-	cfg := &clientv3.Config{
-		Endpoints:            dcfg.Endpoints,
-		DialTimeout:          dcfg.DialTimeout,
-		DialKeepAliveTime:    dcfg.KeepAliveTime,
-		DialKeepAliveTimeout: dcfg.KeepAliveTimeout,
-		Username:             dcfg.Auth.Username,
-		Password:             dcfg.Auth.Password,
-	}
-
-	if cfgtls != nil {
-		if clientTLS, err := cfgtls.ClientConfig(); err == nil {
-			cfg.TLS = clientTLS
-		} else {
-			return nil, err
-		}
-	}
-
-	// If key/cert is not given but user wants secure connection, we
-	// should still setup an empty tls configuration for gRPC to setup
-	// secure connection.
-	if cfg.TLS == nil && !dcfg.Secure.InsecureTransport {
-		cfg.TLS = &tls.Config{}
-	}
-
-	// If the user wants to skip TLS verification then we should set
-	// the InsecureSkipVerify flag in tls configuration.
-	if cfg.TLS != nil && dcfg.Secure.InsecureSkipVerify {
-		cfg.TLS.InsecureSkipVerify = true
-	}
-
-	return cfg, nil
-}
-
 func (d *discovery) getCluster() (string, error) {
 	cls, clusterSize, rev, err := d.checkCluster()
 	if err != nil {