From ac5228dfd9f81ac79f352c385c1f13b2d987e8ff Mon Sep 17 00:00:00 2001 From: Alex Aizman Date: Fri, 10 Nov 2023 16:19:00 -0500 Subject: [PATCH] http(s) clients: unify naming, construction; reduce code * part two Signed-off-by: Alex Aizman --- ais/backend/ais.go | 8 +------ ais/backend/http.go | 25 ++++++-------------- bench/tools/aisloader/client.go | 8 +++---- bench/tools/aisloader/run.go | 8 +++---- cmd/authn/mgr.go | 6 ++--- cmd/cli/cli/auth_hdlr.go | 17 +++++++------ cmd/cli/cli/init.go | 42 ++++++++++++++++----------------- cmd/cli/cli/show_hdlr.go | 13 ++++++---- cmd/cli/cli/utils.go | 9 ++++--- cmd/cli/go.mod | 2 +- cmd/cli/go.sum | 4 ++-- cmn/client.go | 10 ++++---- ext/dload/dispatcher.go | 5 ++-- reb/globrun.go | 18 +++++++------- 14 files changed, 80 insertions(+), 95 deletions(-) diff --git a/ais/backend/ais.go b/ais/backend/ais.go index 90c3a5e3e12..8d1720c7b15 100644 --- a/ais/backend/ais.go +++ b/ais/backend/ais.go @@ -233,14 +233,8 @@ func (m *AISBackendProvider) GetInfo(clusterConf cmn.BackendConfAIS) (res cluste return } -// TODO: cmn.TLSArgs empty and hardcoded - m.b. defined by cmn.ClientConf or env func remaisClients(clientConf *cmn.ClientConf) (client, clientTLS *http.Client) { - cargs := cmn.TransportArgs{Timeout: clientConf.Timeout.D()} - client = cmn.NewClient(cargs) - - sargs := cmn.TLSArgs{SkipVerify: true} - clientTLS = cmn.NewClientTLS(cargs, sargs) - return + return cmn.NewDefaultClients(clientConf.Timeout.D()) } // A list of remote AIS URLs can contains both HTTP and HTTPS links at the diff --git a/ais/backend/http.go b/ais/backend/http.go index eef58b731d2..1d55c95340f 100644 --- a/ais/backend/http.go +++ b/ais/backend/http.go @@ -21,9 +21,9 @@ import ( type ( httpProvider struct { - t cluster.TargetPut - clientH *http.Client - clientTLS *http.Client + t cluster.TargetPut + cliH *http.Client + cliTLS *http.Client } ) @@ -31,27 +31,16 @@ type ( var _ cluster.BackendProvider = (*httpProvider)(nil) func NewHTTP(t cluster.TargetPut, config *cmn.Config) cluster.BackendProvider { - var ( - hp = &httpProvider{t: t} - cargs = cmn.TransportArgs{ - Timeout: config.Client.TimeoutLong.D(), - WriteBufferSize: config.Net.HTTP.WriteBufferSize, - ReadBufferSize: config.Net.HTTP.ReadBufferSize, - } - sargs = cmn.TLSArgs{ - SkipVerify: true, // TODO: may need more tls config to access remote URLs - } - ) - hp.clientH = cmn.NewClient(cargs) - hp.clientTLS = cmn.NewClientTLS(cargs, sargs) + hp := &httpProvider{t: t} + hp.cliH, hp.cliTLS = cmn.NewDefaultClients(config.Client.TimeoutLong.D()) return hp } func (hp *httpProvider) client(u string) *http.Client { if cos.IsHTTPS(u) { - return hp.clientTLS + return hp.cliTLS } - return hp.clientH + return hp.cliH } func (*httpProvider) Provider() string { return apc.HTTP } diff --git a/bench/tools/aisloader/client.go b/bench/tools/aisloader/client.go index a7b3f0c5265..81182c49fe0 100644 --- a/bench/tools/aisloader/client.go +++ b/bench/tools/aisloader/client.go @@ -29,14 +29,14 @@ const longListTime = 10 * time.Second // list-objects progress var ( // see related command-line: `transportArgs.Timeout` and UseHTTPS - transportArgs = cmn.TransportArgs{ + cargs = cmn.TransportArgs{ UseHTTPProxyEnv: true, } // NOTE: client X509 certificate and other `cmn.TLSArgs` variables can be provided via (os.Getenv) environment. // See also: // - docs/aisloader.md, section "Environment variables" // - AIS_ENDPOINT and aisEndpoint - tlsArgs = cmn.TLSArgs{ + sargs = cmn.TLSArgs{ SkipVerify: true, } ) @@ -251,11 +251,11 @@ func putWithTrace(proxyURL string, bck cmn.Bck, objName string, cksum *cos.Cksum func newTraceCtx(proxyURL string) *traceCtx { var ( tctx = &traceCtx{} - transport = cmn.NewTransport(transportArgs) + transport = cmn.NewTransport(cargs) err error ) if cos.IsHTTPS(proxyURL) { - transport.TLSClientConfig, err = cmn.NewTLS(tlsArgs) + transport.TLSClientConfig, err = cmn.NewTLS(sargs) cos.AssertNoErr(err) } tctx.tr = &traceableTransport{ diff --git a/bench/tools/aisloader/run.go b/bench/tools/aisloader/run.go index df0d0a4f283..bf16b5850c3 100644 --- a/bench/tools/aisloader/run.go +++ b/bench/tools/aisloader/run.go @@ -500,7 +500,7 @@ func addCmdLine(f *flag.FlagSet, p *params) { f.BoolVar(&flagUsage, "usage", false, "Show command-line options, usage, and examples") f.BoolVar(&flagVersion, "version", false, "Show aisloader version") f.BoolVar(&flagQuiet, "quiet", false, "When starting to run, do not print command line arguments, default settings, and usage examples") - f.DurationVar(&transportArgs.Timeout, "timeout", 10*time.Minute, "Client HTTP timeout - used in LIST/GET/PUT/DELETE") + f.DurationVar(&cargs.Timeout, "timeout", 10*time.Minute, "Client HTTP timeout - used in LIST/GET/PUT/DELETE") f.IntVar(&p.statsShowInterval, "statsinterval", 10, "Interval in seconds to print performance counters; 0 - disabled") f.StringVar(&p.bck.Name, "bucket", "", "Bucket name or bucket URI. If empty, a bucket with random name will be created") f.StringVar(&p.bck.Provider, "provider", apc.AIS, @@ -889,10 +889,10 @@ func _init(p *params) (err error) { p.bp = api.BaseParams{URL: p.proxyURL} if useHTTPS { // environment to override client config - cmn.EnvToTLS(&tlsArgs) - p.bp.Client = cmn.NewClientTLS(transportArgs, tlsArgs) + cmn.EnvToTLS(&sargs) + p.bp.Client = cmn.NewClientTLS(cargs, sargs) } else { - p.bp.Client = cmn.NewClient(transportArgs) + p.bp.Client = cmn.NewClient(cargs) } // NOTE: auth token is assigned below when we execute the very first API call diff --git a/cmd/authn/mgr.go b/cmd/authn/mgr.go index 185c4c1206e..2b5b130f121 100644 --- a/cmd/authn/mgr.go +++ b/cmd/authn/mgr.go @@ -45,12 +45,10 @@ var ( // If user DB exists, loads the data from the file and decrypts passwords func newMgr(driver kvdb.Driver) (m *mgr, err error) { - timeout := time.Duration(Conf.Timeout.Default) m = &mgr{ - clientH: cmn.NewClient(cmn.TransportArgs{Timeout: timeout}), - clientTLS: cmn.NewClientTLS(cmn.TransportArgs{Timeout: timeout}, cmn.TLSArgs{SkipVerify: true}), - db: driver, + db: driver, } + m.clientH, m.clientTLS = cmn.NewDefaultClients(time.Duration(Conf.Timeout.Default)) err = initializeDB(driver) return } diff --git a/cmd/cli/cli/auth_hdlr.go b/cmd/cli/cli/auth_hdlr.go index e185ef3064a..0a0eb364c6e 100644 --- a/cmd/cli/cli/auth_hdlr.go +++ b/cmd/cli/cli/auth_hdlr.go @@ -218,7 +218,7 @@ var ( // and augments API errors if needed. func wrapAuthN(f cli.ActionFunc) cli.ActionFunc { return func(c *cli.Context) error { - if authnHTTPClient == nil { + if authParams.Client == nil { return errors.New(env.AuthN.URL + " is not set") } err := f(c) @@ -411,15 +411,18 @@ func addAuthClusterHandler(c *cli.Context) (err error) { cluSpec.URLs = append(cluSpec.URLs, clusterURL) } else { bp := api.BaseParams{ - Client: defaultHTTPClient, - URL: cluSpec.URLs[0], - Token: loggedUserToken, - UA: ua, + URL: cluSpec.URLs[0], + Token: loggedUserToken, + UA: ua, + } + if cos.IsHTTPS(bp.URL) { + bp.Client = clientTLS + } else { + bp.Client = clientH } smap, err = api.GetClusterMap(bp) if err != nil { - err = fmt.Errorf("failed to add cluster %q(%q, %s): %v", - cluSpec.ID, cluSpec.Alias, cluSpec.URLs[0], err) + err = fmt.Errorf("failed to add cluster %q(%q, %s): %v", cluSpec.ID, cluSpec.Alias, cluSpec.URLs[0], err) } } if err != nil { diff --git a/cmd/cli/cli/init.go b/cmd/cli/cli/init.go index 9894e6ee711..8cc91a4d8ca 100644 --- a/cmd/cli/cli/init.go +++ b/cmd/cli/cli/init.go @@ -36,11 +36,9 @@ func Init() (err error) { clusterURL = _clusterURL(cfg) var ( - useHTTPS = cos.IsHTTPS(clusterURL) - cargs = cmn.TransportArgs{ + cargs = cmn.TransportArgs{ DialTimeout: cfg.Timeout.TCPTimeout, Timeout: cfg.Timeout.HTTPTimeout, - UseHTTPS: useHTTPS, } sargs = cmn.TLSArgs{ ClientCA: cfg.Cluster.ClientCA, @@ -49,31 +47,33 @@ func Init() (err error) { SkipVerify: cfg.Cluster.SkipVerifyCrt, } ) - if useHTTPS { - // environment to override client config - cmn.EnvToTLS(&sargs) + + clientH = cmn.NewClient(cargs) + cmn.EnvToTLS(&sargs) + clientTLS = cmn.NewClientTLS(cargs, sargs) + + apiBP = api.BaseParams{ + URL: clusterURL, + Token: loggedUserToken, + UA: ua, } - if useHTTPS { - defaultHTTPClient = cmn.NewClientTLS(cargs, sargs) + if cos.IsHTTPS(clusterURL) { + apiBP.Client = clientTLS } else { - defaultHTTPClient = cmn.NewClient(cargs) + apiBP.Client = clientH } if authnURL := cliAuthnURL(cfg); authnURL != "" { - debug.Assert(useHTTPS == cos.IsHTTPS(authnURL)) - authnHTTPClient = defaultHTTPClient authParams = api.BaseParams{ - Client: authnHTTPClient, - URL: authnURL, - Token: loggedUserToken, - UA: ua, + URL: authnURL, + Token: loggedUserToken, + UA: ua, + } + if cos.IsHTTPS(authnURL) { + authParams.Client = clientTLS + } else { + authParams.Client = clientH } - } - apiBP = api.BaseParams{ - Client: defaultHTTPClient, - URL: clusterURL, - Token: loggedUserToken, - UA: ua, } return } diff --git a/cmd/cli/cli/show_hdlr.go b/cmd/cli/cli/show_hdlr.go index 03363aad861..ab9b8200881 100644 --- a/cmd/cli/cli/show_hdlr.go +++ b/cmd/cli/cli/show_hdlr.go @@ -850,10 +850,15 @@ For details and usage examples, see: docs/cli/config.md` for _, ra := range all.A { uptime := teb.UnknownStatusVal bp := api.BaseParams{ - Client: defaultHTTPClient, - URL: ra.URL, - Token: loggedUserToken, - UA: ua, + URL: ra.URL, + Token: loggedUserToken, + UA: ua, + } + if cos.IsHTTPS(bp.URL) { + // NOTE: alternatively, cmn.NewClientTLS(..., TLSArgs{SkipVerify: true}) + bp.Client = clientTLS + } else { + bp.Client = clientH } if clutime, _, err := api.HealthUptime(bp); err == nil { ns, _ := strconv.ParseInt(clutime, 10, 64) diff --git a/cmd/cli/cli/utils.go b/cmd/cli/cli/utils.go index 0941c3a6c37..01f239dbb2f 100644 --- a/cmd/cli/cli/utils.go +++ b/cmd/cli/cli/utils.go @@ -52,11 +52,10 @@ const ( ) var ( - clusterURL string - defaultHTTPClient *http.Client - authnHTTPClient *http.Client - apiBP api.BaseParams - authParams api.BaseParams + clusterURL string + clientH, clientTLS *http.Client + apiBP api.BaseParams + authParams api.BaseParams ) type ( diff --git a/cmd/cli/go.mod b/cmd/cli/go.mod index e101f54f43f..27f8e361f8e 100644 --- a/cmd/cli/go.mod +++ b/cmd/cli/go.mod @@ -4,7 +4,7 @@ go 1.21 // direct require ( - github.com/NVIDIA/aistore v1.3.22-0.20231107185203-518fb18b7844 + github.com/NVIDIA/aistore v1.3.22-0.20231110211900-280f9e6ed697 github.com/fatih/color v1.15.0 github.com/json-iterator/go v1.1.12 github.com/onsi/ginkgo v1.16.5 diff --git a/cmd/cli/go.sum b/cmd/cli/go.sum index 49d263e5c60..e4550cdf172 100644 --- a/cmd/cli/go.sum +++ b/cmd/cli/go.sum @@ -1,7 +1,7 @@ code.cloudfoundry.org/bytefmt v0.0.0-20190710193110-1eb035ffe2b6/go.mod h1:wN/zk7mhREp/oviagqUXY3EwuHhWyOvAdsn5Y4CzOrc= github.com/BurntSushi/toml v1.3.2/go.mod h1:CxXYINrC8qIiEnFrOxCa7Jy5BFHlXnUU2pbicEuybxQ= -github.com/NVIDIA/aistore v1.3.22-0.20231107185203-518fb18b7844 h1:VFZz9PVWfVt9YFYuE1ADixHY9manzpDQrBRD6mBCZfs= -github.com/NVIDIA/aistore v1.3.22-0.20231107185203-518fb18b7844/go.mod h1:+iSnZg0ovMaLgaT9fLAs2WmYBP7IfeTW1WYkbKrwP4g= +github.com/NVIDIA/aistore v1.3.22-0.20231110211900-280f9e6ed697 h1:aTqLO8ZjOHwvoGbGyJy0g+5IG6WwevBpDp+uv2irWz4= +github.com/NVIDIA/aistore v1.3.22-0.20231110211900-280f9e6ed697/go.mod h1:+iSnZg0ovMaLgaT9fLAs2WmYBP7IfeTW1WYkbKrwP4g= github.com/OneOfOne/xxhash v1.2.8 h1:31czK/TI9sNkxIKfaUfGlU47BAxQ0ztGgd9vPyqimf8= github.com/OneOfOne/xxhash v1.2.8/go.mod h1:eZbhyaAYD41SGSSsnmcpxVoRiQ/MPUTjUdIIOT9Um7Q= github.com/VividCortex/ewma v1.1.1/go.mod h1:2Tkkvm3sRDVXaiyucHiACn4cqf7DpdyLvmxzcbUokwA= diff --git a/cmn/client.go b/cmn/client.go index 96f576da2a9..4160bdaa90f 100644 --- a/cmn/client.go +++ b/cmn/client.go @@ -115,13 +115,13 @@ func NewTLS(sargs TLSArgs) (tlsConf *tls.Config, _ error) { return tlsConf, nil } -// TODO -- FIXME -func NewClients(timeout time.Duration) (clientH, clientTLS *http.Client) { +func NewDefaultClients(timeout time.Duration) (clientH, clientTLS *http.Client) { clientH = NewClient(TransportArgs{Timeout: timeout}) + clientTLS = NewClientTLS(TransportArgs{Timeout: timeout}, TLSArgs{SkipVerify: true}) return } -// http client +// NOTE: `NewTransport` (below) fills-in certain defaults func NewClient(cargs TransportArgs) *http.Client { return &http.Client{Transport: NewTransport(cargs), Timeout: cargs.Timeout} } @@ -130,11 +130,11 @@ func NewIntraClientTLS(cargs TransportArgs, config *Config) *http.Client { return NewClientTLS(cargs, config.Net.HTTP.ToTLS()) } -// https client +// https client (ditto) func NewClientTLS(cargs TransportArgs, sargs TLSArgs) *http.Client { transport := NewTransport(cargs) - // initialize TLS config and panic on err + // initialize TLS config tlsConfig, err := NewTLS(sargs) if err != nil { cos.ExitLog(err) diff --git a/ext/dload/dispatcher.go b/ext/dload/dispatcher.go index 88f0a9e1244..4cb6535513b 100644 --- a/ext/dload/dispatcher.go +++ b/ext/dload/dispatcher.go @@ -61,9 +61,8 @@ type ( var g global func Init(t cluster.Target, stats stats.Tracker, db kvdb.Driver, clientConf *cmn.ClientConf) { - cargs := cmn.TransportArgs{Timeout: clientConf.TimeoutLong.D()} - g.clientH = cmn.NewClient(cargs) - g.clientTLS = cmn.NewClientTLS(cargs, cmn.TLSArgs{SkipVerify: true}) + g.clientH, g.clientTLS = cmn.NewDefaultClients(clientConf.TimeoutLong.D()) + if db == nil { // unit tests only debug.Assert(t == nil) return diff --git a/reb/globrun.go b/reb/globrun.go index 2a235e63408..2a76737fa69 100644 --- a/reb/globrun.go +++ b/reb/globrun.go @@ -118,19 +118,17 @@ type ( func New(t cluster.Target, config *cmn.Config) *Reb { var ( - client *http.Client - cargs = cmn.TransportArgs{Timeout: config.Client.Timeout.D()} + reb = &Reb{ + t: t, + filterGFN: prob.NewDefaultFilter(), + stages: newNodeStages(), + } + cargs = cmn.TransportArgs{Timeout: config.Client.Timeout.D()} ) if config.Net.HTTP.UseHTTPS { - client = cmn.NewIntraClientTLS(cargs, config) + reb.ecClient = cmn.NewIntraClientTLS(cargs, config) } else { - client = cmn.NewClient(cargs) - } - reb := &Reb{ - t: t, - filterGFN: prob.NewDefaultFilter(), - stages: newNodeStages(), - ecClient: client, + reb.ecClient = cmn.NewClient(cargs) } dmExtra := bundle.Extra{ RecvAck: reb.recvAck,