From cd545f0160e9dea5a9637eb574fe7ff2b380e0ac Mon Sep 17 00:00:00 2001 From: Leonard Lyubich Date: Wed, 4 May 2022 15:34:26 +0300 Subject: [PATCH] [#1351] cli: Fix connection scheme loss during endpoint parsing In previous implementation NeoFS CLI app used `network.Address.HostAddr` as a server URI, which caused scheme loss since host address doesn't contain it. Rename `HostAddr` to `URIAddr` and make it to return URI address with `grpcs` scheme if TLS is enabled. Make `TLSEnabled` unexported since it was used to provide default `tls.Config` only (it is used by default in SDK). Signed-off-by: Leonard Lyubich --- CHANGELOG.md | 3 +++ cmd/neofs-cli/internal/client/sdk.go | 7 +----- cmd/neofs-cli/modules/control.go | 26 ++--------------------- cmd/neofs-cli/modules/root.go | 14 ++++++++---- cmd/neofs-node/config/node/config_test.go | 7 ++---- pkg/network/address.go | 16 +++++++++++--- pkg/network/address_test.go | 5 +++-- pkg/network/cache/multi.go | 6 +----- pkg/network/group.go | 2 +- pkg/network/tls.go | 4 ++-- pkg/network/tls_test.go | 2 +- 11 files changed, 39 insertions(+), 53 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 068baaca4f..bdb6c9e17e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,9 @@ Changelog for NeoFS Node ## [Unreleased] +### Fixed +- Loss of the connection scheme during address parsing in NeoFS CLI (#1351) + ## [0.28.0] - 2022-04-29 - Heuksando (흑산도, 黑山島) ### Added diff --git a/cmd/neofs-cli/internal/client/sdk.go b/cmd/neofs-cli/internal/client/sdk.go index ab254a7d44..9f74578208 100644 --- a/cmd/neofs-cli/internal/client/sdk.go +++ b/cmd/neofs-cli/internal/client/sdk.go @@ -2,7 +2,6 @@ package internal import ( "crypto/ecdsa" - "crypto/tls" "fmt" "github.com/nspcc-dev/neofs-node/pkg/network" @@ -19,11 +18,7 @@ func GetSDKClient(key *ecdsa.PrivateKey, addr network.Address) (*client.Client, prmInit.SetDefaultPrivateKey(*key) prmInit.ResolveNeoFSFailures() - prmDial.SetServerURI(addr.HostAddr()) - - if addr.TLSEnabled() { - prmDial.SetTLSConfig(&tls.Config{}) - } + prmDial.SetServerURI(addr.URIAddr()) c.Init(prmInit) diff --git a/cmd/neofs-cli/modules/control.go b/cmd/neofs-cli/modules/control.go index bf02668844..3ef24e694e 100644 --- a/cmd/neofs-cli/modules/control.go +++ b/cmd/neofs-cli/modules/control.go @@ -2,7 +2,6 @@ package cmd import ( "crypto/ecdsa" - "crypto/tls" "fmt" "github.com/mr-tron/base58" @@ -457,30 +456,9 @@ func listShards(cmd *cobra.Command, _ []string) { prettyPrintShards(cmd, resp.GetBody().GetShards()) } -// getControlSDKClient is the same getSDKClient but with -// another RPC endpoint flag. +// getControlSDKClient calls getSDKClientFlag with "endpoint" flag. func getControlSDKClient(key *ecdsa.PrivateKey) (*client.Client, error) { - var ( - c client.Client - prmInit client.PrmInit - prmDial client.PrmDial - ) - - netAddr, err := getEndpointAddress(controlRPC) - if err != nil { - return nil, err - } - - prmInit.SetDefaultPrivateKey(*key) - prmDial.SetServerURI(netAddr.HostAddr()) - - if netAddr.TLSEnabled() { - prmDial.SetTLSConfig(&tls.Config{}) - } - - c.Init(prmInit) - - return &c, c.Dial(prmDial) + return getSDKClientFlag(key, controlRPC) } func prettyPrintShards(cmd *cobra.Command, ii []*control.ShardInfo) { diff --git a/cmd/neofs-cli/modules/root.go b/cmd/neofs-cli/modules/root.go index 661bbd7206..3d27e38ece 100644 --- a/cmd/neofs-cli/modules/root.go +++ b/cmd/neofs-cli/modules/root.go @@ -188,7 +188,7 @@ func getEndpointAddress(endpointFlag string) (addr network.Address, err error) { err = addr.FromString(endpoint) if err != nil { - err = errInvalidEndpoint + err = fmt.Errorf("%v: %w", errInvalidEndpoint, err) } return @@ -227,13 +227,19 @@ func prepareBearerPrm(cmd *cobra.Command, prm bearerPrm) { prm.SetBearerToken(btok) } -// getSDKClient returns default neofs-api-go sdk client. Consider using -// opts... to provide TTL or other global configuration flags. +// getSDKClient calls getSDKGClientFlag with "rpc-endpoint" flag. func getSDKClient(key *ecdsa.PrivateKey) (*client.Client, error) { - netAddr, err := getEndpointAddress(rpc) + return getSDKClientFlag(key, rpc) +} + +// getSDKClientFlag returns NeoFS API client connection to the network address +// set by the given flag. +func getSDKClientFlag(key *ecdsa.PrivateKey, endpointFlag string) (*client.Client, error) { + netAddr, err := getEndpointAddress(endpointFlag) if err != nil { return nil, err } + return internalclient.GetSDKClient(key, netAddr) } diff --git a/cmd/neofs-node/config/node/config_test.go b/cmd/neofs-node/config/node/config_test.go index e83592aa04..f0bf39eca9 100644 --- a/cmd/neofs-node/config/node/config_test.go +++ b/cmd/neofs-node/config/node/config_test.go @@ -89,12 +89,10 @@ func TestNodeSection(t *testing.T) { expectedAddr := []struct { str string host string - tls bool }{ { str: "/dns4/localhost/tcp/8083/tls", - host: "localhost:8083", - tls: true, + host: "grpcs://localhost:8083", }, { str: "/dns4/s01.neofs.devenv/tcp/8080", @@ -118,8 +116,7 @@ func TestNodeSection(t *testing.T) { addrs.IterateAddresses(func(addr network.Address) bool { require.Equal(t, expectedAddr[ind].str, addr.String()) - require.Equal(t, expectedAddr[ind].host, addr.HostAddr()) - require.Equal(t, expectedAddr[ind].tls, addr.TLSEnabled()) + require.Equal(t, expectedAddr[ind].host, addr.URIAddr()) ind++ diff --git a/pkg/network/address.go b/pkg/network/address.go index 5e094be43c..3419d87562 100644 --- a/pkg/network/address.go +++ b/pkg/network/address.go @@ -3,6 +3,7 @@ package network import ( "fmt" "net" + "net/url" "strings" "github.com/multiformats/go-multiaddr" @@ -33,10 +34,12 @@ func (a Address) equal(addr Address) bool { return a.ma.Equal(addr.ma) } -// HostAddr returns host address in string format. +// URIAddr returns Address as a URI. // // Panics if host address cannot be fetched from Address. -func (a Address) HostAddr() string { +// +// See also FromString. +func (a Address) URIAddr() string { _, host, err := manet.DialArgs(a.ma) if err != nil { // the only correct way to construct Address is AddressFromString @@ -44,7 +47,14 @@ func (a Address) HostAddr() string { panic(fmt.Errorf("could not get host addr: %w", err)) } - return host + if !a.isTLSEnabled() { + return host + } + + return (&url.URL{ + Scheme: "grpcs", + Host: host, + }).String() } // FromString restores Address from a string representation. diff --git a/pkg/network/address_test.go b/pkg/network/address_test.go index 78d4008f98..c4e7bea2e3 100644 --- a/pkg/network/address_test.go +++ b/pkg/network/address_test.go @@ -50,12 +50,13 @@ func TestAddress_HostAddrString(t *testing.T) { }{ {buildMultiaddr("/dns4/neofs.bigcorp.com/tcp/8080", t), "neofs.bigcorp.com:8080"}, {buildMultiaddr("/ip4/172.16.14.1/tcp/8080", t), "172.16.14.1:8080"}, + {buildMultiaddr("/ip4/192.168.0.1/tcp/8888/tls", t), "grpcs://192.168.0.1:8888"}, } for _, testcase := range testcases { addr := Address{testcase.ma} - got := addr.HostAddr() + got := addr.URIAddr() require.Equal(t, testcase.exp, got) } @@ -68,7 +69,7 @@ func TestAddress_HostAddrString(t *testing.T) { for _, testcase := range testcases { addr := Address{testcase} - require.Panics(t, func() { addr.HostAddr() }) + require.Panics(t, func() { addr.URIAddr() }) } }) } diff --git a/pkg/network/cache/multi.go b/pkg/network/cache/multi.go index da9aa39274..10c579810a 100644 --- a/pkg/network/cache/multi.go +++ b/pkg/network/cache/multi.go @@ -2,7 +2,6 @@ package cache import ( "context" - "crypto/tls" "errors" "sync" @@ -38,10 +37,7 @@ func (x *multiClient) createForAddress(addr network.Address) clientcore.Client { prmDial client.PrmDial ) - prmDial.SetServerURI(addr.HostAddr()) - if addr.TLSEnabled() { - prmDial.SetTLSConfig(&tls.Config{}) - } + prmDial.SetServerURI(addr.URIAddr()) if x.opts.Key != nil { prmInit.SetDefaultPrivateKey(*x.opts.Key) diff --git a/pkg/network/group.go b/pkg/network/group.go index ebe6bf4c88..113fd0c3b3 100644 --- a/pkg/network/group.go +++ b/pkg/network/group.go @@ -57,7 +57,7 @@ func (x AddressGroup) Len() int { // Less returns true if i-th address in AddressGroup supports TLS // and j-th one doesn't. func (x AddressGroup) Less(i, j int) bool { - return x[i].TLSEnabled() && !x[j].TLSEnabled() + return x[i].isTLSEnabled() && !x[j].isTLSEnabled() } // Swap swaps i-th and j-th addresses in AddressGroup. diff --git a/pkg/network/tls.go b/pkg/network/tls.go index 0081d43e6e..de2c93694c 100644 --- a/pkg/network/tls.go +++ b/pkg/network/tls.go @@ -11,8 +11,8 @@ const ( // tls var is used for (un)wrapping other multiaddrs around TLS multiaddr. var tls, _ = multiaddr.NewMultiaddr("/" + tlsProtocolName) -// TLSEnabled searches for wrapped TLS protocol in multiaddr. -func (a Address) TLSEnabled() bool { +// isTLSEnabled searches for wrapped TLS protocol in multiaddr. +func (a Address) isTLSEnabled() bool { for _, protoc := range a.ma.Protocols() { if protoc.Code == multiaddr.P_TLS { return true diff --git a/pkg/network/tls_test.go b/pkg/network/tls_test.go index c7715b449d..25775eaf16 100644 --- a/pkg/network/tls_test.go +++ b/pkg/network/tls_test.go @@ -24,6 +24,6 @@ func TestAddress_TLSEnabled(t *testing.T) { err := addr.FromString(test.input) require.NoError(t, err) - require.Equal(t, test.wantTLS, addr.TLSEnabled(), test.input) + require.Equal(t, test.wantTLS, addr.isTLSEnabled(), test.input) } }