From f978da4f4f33421c22ce0e3198370d709e9cdfa2 Mon Sep 17 00:00:00 2001 From: ahrtr Date: Fri, 18 Mar 2022 14:14:21 +0800 Subject: [PATCH] move the newClientCfg into clientv3 package so as to be reused by both etcdctl and v3discovery --- client/v3/config.go | 63 +++++++++ client/v3/go.mod | 2 +- etcdctl/ctlv3/command/ep_command.go | 37 +++-- etcdctl/ctlv3/command/global.go | 68 +-------- etcdctl/ctlv3/command/global_test.go | 131 ------------------ .../etcdserver/api/v3discovery/discovery.go | 51 +------ 6 files changed, 94 insertions(+), 258 deletions(-) delete mode 100644 etcdctl/ctlv3/command/global_test.go diff --git a/client/v3/config.go b/client/v3/config.go index 297261608..47462905c 100644 --- a/client/v3/config.go +++ b/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 +} diff --git a/client/v3/go.mod b/client/v3/go.mod index 18ada5112..f8b11edbd 100644 --- a/client/v3/go.mod +++ b/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 diff --git a/etcdctl/ctlv3/command/ep_command.go b/etcdctl/ctlv3/command/ep_command.go index 9e4aad04d..2a6db17a4 100644 --- a/etcdctl/ctlv3/command/ep_command.go +++ b/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"` - Resp *v3.StatusResponse `json:"Status"` + Ep string `json:"Endpoint"` + 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"` - Resp *v3.HashKVResponse `json:"HashKV"` + Ep string `json:"Endpoint"` + 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) - - cfg, err := newClientCfg(eps, dt, ka, kat, sec, nil) + lg, _ := zap.NewProduction() + 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) } diff --git a/etcdctl/ctlv3/command/global.go b/etcdctl/ctlv3/command/global.go index 57adae28f..e151c1dd3 100644 --- a/etcdctl/ctlv3/command/global.go +++ b/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 diff --git a/etcdctl/ctlv3/command/global_test.go b/etcdctl/ctlv3/command/global_test.go deleted file mode 100644 index 0dc56abdd..000000000 --- a/etcdctl/ctlv3/command/global_test.go +++ /dev/null @@ -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) - } -} diff --git a/server/etcdserver/api/v3discovery/discovery.go b/server/etcdserver/api/v3discovery/discovery.go index 1c274cdba..f5be6b8f7 100644 --- a/server/etcdserver/api/v3discovery/discovery.go +++ b/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 {