diff --git a/pkg/scyllaclient/client_agent.go b/pkg/scyllaclient/client_agent.go index 7c747abbc4..1a78a77d6e 100644 --- a/pkg/scyllaclient/client_agent.go +++ b/pkg/scyllaclient/client_agent.go @@ -6,7 +6,6 @@ import ( "context" "net" "net/url" - "time" "github.com/pkg/errors" scyllaversion "github.com/scylladb/scylla-manager/v3/pkg/util/version" @@ -47,7 +46,17 @@ func (c *Client) AnyNodeInfo(ctx context.Context) (*NodeInfo, error) { // is added. // `fallback` argument is used in case any of above addresses is zero address. func (ni *NodeInfo) CQLAddr(fallback string) string { - addr, port := ni.cqlAddr(fallback), ni.CQLPort(fallback) + addr, port := ni.cqlAddr(fallback), ni.CQLPort() + return net.JoinHostPort(addr, port) +} + +// CQLSSLAddr returns CQL SSL address from NodeInfo. +// Scylla can have separate rpc_address (CQL), listen_address and respectfully +// broadcast_rpc_address and broadcast_address if some 3rd party routing +// is added. +// `fallback` argument is used in case any of above addresses is zero address. +func (ni *NodeInfo) CQLSSLAddr(fallback string) string { + addr, port := ni.cqlAddr(fallback), ni.CQLSSLPort() return net.JoinHostPort(addr, port) } @@ -71,31 +80,15 @@ func (ni *NodeInfo) cqlAddr(fallback string) string { } // CQLPort returns CQL port from NodeInfo. -// `fallbackAddress` argument is needed for Scylla bug workaround, see CQLAddr for description. -func (ni *NodeInfo) CQLPort(fallbackAddress string) string { - if ni.ClientEncryptionEnabled { - // Scylla API always returns non-empty NativeTransportPortSSL even when - // value is explicitly disabled in configuration file. - // This makes impossible to determine which port is being used for CQL - // frontend. To workaround it, we try to dial SSL port when - // client encryption is enabled. If any error happens, assume this port - // is not used. - // Ref: https://github.com/scylladb/scylla/issues/7206 - - d := &net.Dialer{ - Timeout: time.Second, - } - addr := net.JoinHostPort(ni.cqlAddr(fallbackAddress), ni.NativeTransportPortSsl) - c, err := d.Dial("tcp", addr) - if err != nil { - return ni.NativeTransportPort - } - defer c.Close() - return ni.NativeTransportPortSsl - } +func (ni *NodeInfo) CQLPort() string { return ni.NativeTransportPort } +// CQLSSLPort returns CQL SSL port from NodeInfo. +func (ni *NodeInfo) CQLSSLPort() string { + return ni.NativeTransportPortSsl +} + // AlternatorEnabled returns if Alternator is enabled on host. func (ni *NodeInfo) AlternatorEnabled() bool { return (ni.AlternatorHTTPSPort != "0" && ni.AlternatorHTTPSPort != "") || diff --git a/pkg/scyllaclient/client_agent_test.go b/pkg/scyllaclient/client_agent_test.go index 27c0cfeb3d..8b8639ca17 100644 --- a/pkg/scyllaclient/client_agent_test.go +++ b/pkg/scyllaclient/client_agent_test.go @@ -8,7 +8,6 @@ import ( "github.com/google/go-cmp/cmp" "github.com/scylladb/scylla-manager/v3/pkg/scyllaclient" - "go.uber.org/atomic" ) const fallback = "4.3.2.1" @@ -80,26 +79,6 @@ func TestNodeInfoCQLAddr(t *testing.T) { }, GoldenAddress: net.JoinHostPort(fallback, "1234"), }, - { - Name: "Native Transport Port SSL with client encryption enabled without server listening on", - NodeInfo: &scyllaclient.NodeInfo{ - NativeTransportPort: "4321", - NativeTransportPortSsl: "1234", - ListenAddress: "1.2.3.4", - ClientEncryptionEnabled: true, - }, - GoldenAddress: net.JoinHostPort("1.2.3.4", "4321"), - }, - { - Name: "Native Transport Port SSL with client encryption disabled", - NodeInfo: &scyllaclient.NodeInfo{ - NativeTransportPort: "4321", - NativeTransportPortSsl: "1234", - ListenAddress: "1.2.3.4", - ClientEncryptionEnabled: false, - }, - GoldenAddress: net.JoinHostPort("1.2.3.4", "4321"), - }, } for i := range table { @@ -115,51 +94,85 @@ func TestNodeInfoCQLAddr(t *testing.T) { } } -// Test workaround used in NodeInfo.CQLPort(). -func TestNodeInfoCQLAddrNativeTransportPortSSL(t *testing.T) { - l, err := net.Listen("tcp", "127.0.0.1:0") - if err != nil { - t.Fatal(err) - } - defer l.Close() +func TestNodeInfoCQLSSLAddr(t *testing.T) { + t.Parallel() - address, port, err := net.SplitHostPort(l.Addr().String()) - if err != nil { - t.Fatal(err) + table := []struct { + Name string + NodeInfo *scyllaclient.NodeInfo + GoldenAddress string + }{ + { + Name: "Broadcast RPC address is set", + NodeInfo: &scyllaclient.NodeInfo{ + BroadcastRPCAddress: "1.2.3.4", + RPCAddress: "1.2.3.5", + ListenAddress: "1.2.3.6", + NativeTransportPortSsl: "1234", + }, + GoldenAddress: "1.2.3.4:1234", + }, + { + Name: "RPC address is set", + NodeInfo: &scyllaclient.NodeInfo{ + NativeTransportPortSsl: "1234", + RPCAddress: "1.2.3.5", + ListenAddress: "1.2.3.6", + }, + GoldenAddress: "1.2.3.5:1234", + }, + { + Name: "Listen Address is set", + NodeInfo: &scyllaclient.NodeInfo{ + NativeTransportPortSsl: "1234", + ListenAddress: "1.2.3.6", + }, + GoldenAddress: "1.2.3.6:1234", + }, + { + Name: "Fallback is returned when RPC Address is IPv4 zero", + NodeInfo: &scyllaclient.NodeInfo{ + NativeTransportPortSsl: "1234", + RPCAddress: "0.0.0.0", + }, + GoldenAddress: net.JoinHostPort(fallback, "1234"), + }, + { + Name: "Fallback is returned when RPC Address is IPv6 zero", + NodeInfo: &scyllaclient.NodeInfo{ + NativeTransportPortSsl: "1234", + RPCAddress: "::0", + }, + GoldenAddress: net.JoinHostPort(fallback, "1234"), + }, + { + Name: "Fallback is returned when Listen Address is IPv4 zero", + NodeInfo: &scyllaclient.NodeInfo{ + NativeTransportPortSsl: "1234", + ListenAddress: "0.0.0.0", + }, + GoldenAddress: net.JoinHostPort(fallback, "1234"), + }, + { + Name: "Fallback is returned when Listen Address is IPv6 zero", + NodeInfo: &scyllaclient.NodeInfo{ + NativeTransportPortSsl: "1234", + ListenAddress: "::0", + }, + GoldenAddress: net.JoinHostPort(fallback, "1234"), + }, } - var ( - connections atomic.Int64 - ready = make(chan struct{}) - ) - go func() { - for { - c, err := l.Accept() - if err != nil { - return - } - connections.Inc() - _ = c.Close() - close(ready) - break - } - }() - - ni := &scyllaclient.NodeInfo{ - NativeTransportPort: "4321", - NativeTransportPortSsl: port, - ListenAddress: address, - ClientEncryptionEnabled: true, - } - addr := ni.CQLAddr(fallback) - golden := net.JoinHostPort(ni.ListenAddress, ni.NativeTransportPortSsl) - if addr != golden { - t.Errorf("expected %s address, got %s", golden, addr) - } + for i := range table { + test := table[i] + t.Run(test.Name, func(t *testing.T) { + t.Parallel() - <-ready - if c := connections.Load(); c == 0 { - t.Errorf("expected connection during figuring out CQL port got %d", c) + addr := test.NodeInfo.CQLSSLAddr(fallback) + if addr != test.GoldenAddress { + t.Errorf("expected %s address, got %s", test.GoldenAddress, addr) + } + }) } } diff --git a/pkg/service/cluster/service.go b/pkg/service/cluster/service.go index a41aa3527f..672c43bb62 100644 --- a/pkg/service/cluster/service.go +++ b/pkg/service/cluster/service.go @@ -8,6 +8,7 @@ import ( "crypto/tls" "fmt" "sort" + "strconv" "github.com/gocql/gocql" "github.com/pkg/errors" @@ -549,6 +550,7 @@ func (s *Service) GetSession(ctx context.Context, clusterID uuid.UUID) (session } scyllaCluster := gocql.NewCluster(sessionHosts...) + cqlPort := ni.CQLPort() if ni.CqlPasswordProtected { credentials := secrets.CQLCreds{ @@ -568,20 +570,41 @@ func (s *Service) GetSession(ctx context.Context, clusterID uuid.UUID) (session } } - if ni.ClientEncryptionEnabled { + keyPair, err := s.loadTLSIdentity(clusterID) + if err != nil && !errors.Is(err, service.ErrNotFound) { + return session, err + } + + if ni.ClientEncryptionEnabled && !ni.ClientEncryptionRequireAuth { + cqlPort = ni.CQLSSLPort() scyllaCluster.SslOpts = &gocql.SslOptions{ Config: &tls.Config{ InsecureSkipVerify: true, }, } - if ni.ClientEncryptionRequireAuth { - keyPair, err := s.loadTLSIdentity(clusterID) - if err != nil { - return session, err - } - scyllaCluster.SslOpts.Config.Certificates = []tls.Certificate{keyPair} + } + + if ni.ClientEncryptionEnabled && ni.ClientEncryptionRequireAuth && !errors.Is(err, service.ErrNotFound) { + cqlPort = ni.CQLSSLPort() + scyllaCluster.SslOpts = &gocql.SslOptions{ + Config: &tls.Config{ + InsecureSkipVerify: true, + }, } + scyllaCluster.SslOpts.Config.Certificates = []tls.Certificate{keyPair} + } + + if ni.ClientEncryptionEnabled && ni.ClientEncryptionRequireAuth && errors.Is(err, service.ErrNotFound) { + s.logger.Info(ctx, "Client encryption is enabled, but Cluster wasn't registered with certificate in Scylla Manager, falling back to nonSSL port.", + "cluster_id", clusterID, + ) + } + + p, err := strconv.Atoi(cqlPort) + if err != nil { + return session, errors.Wrap(err, "parse cql port") } + scyllaCluster.Port = p return gocqlx.WrapSession(scyllaCluster.CreateSession()) } diff --git a/pkg/service/healthcheck/service.go b/pkg/service/healthcheck/service.go index 539be9d8d2..89f0a5d943 100644 --- a/pkg/service/healthcheck/service.go +++ b/pkg/service/healthcheck/service.go @@ -354,6 +354,7 @@ func (s *Service) pingCQL(ctx context.Context, clusterID uuid.UUID, host string, tlsConfig := ni.tlsConfig(cqlPing) if tlsConfig != nil { + config.Addr = ni.CQLSSLAddr(host) config.TLSConfig = tlsConfig.Clone() } @@ -418,10 +419,16 @@ func (s *Service) nodeInfo(ctx context.Context, clusterID uuid.UUID, host string } if tlsEnabled { tlsConfig, err := s.tlsConfig(clusterID, clientCertAuth) - if err != nil { + if err != nil && !errors.Is(err, service.ErrNotFound) { return ni, errors.Wrap(err, "fetch TLS config") } - ni.TLSConfig[p] = tlsConfig + if clientCertAuth && errors.Is(err, service.ErrNotFound) { + s.logger.Info(ctx, "Client encryption is enabled, but Cluster wasn't registered with certificate in Scylla Manager, falling back to nonSSL port.", + "cluster_id", clusterID, + ) + } else { + ni.TLSConfig[p] = tlsConfig + } } }