Skip to content

Commit

Permalink
[FIXED] Some Leafnode issues
Browse files Browse the repository at this point in the history 
- When soliciting a Leafnode connection, if there is an account
  specified and the lookup fails, the server would only print
  as a debug and the connection would never been retried. After
  discussion, it seems that proper behavior would be to produce
  a fatal error.

- When using basic auth (user/password), it was possible for a
  solicited Leafnode connection to not use user/password when
  trying an URL that was discovered through gossip. The server
  now saves the credentials of a configured URL to use with
  the discovered ones.

Updated RouteRTT test in case RTT does not seem to be updated
because getting always the same value.

Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
  • Loading branch information
@kozlovic
kozlovic committed Aug 23, 2019
1 parent 2959b98 commit 0c6e1fa
Show file tree
Hide file tree
Showing 4 changed files with 220 additions and 49 deletions.
41 changes: 30 additions & 11 deletions server/leafnode.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,11 @@ type leaf struct {
type leafNodeCfg struct {
sync.RWMutex
*RemoteLeafOpts
urls []*url.URL
curURL *url.URL
tlsName string
urls []*url.URL
curURL *url.URL
tlsName string
username string
password string
}

// Check to see if this is a solicited leafnode. We do special processing for solicited.
Expand Down Expand Up @@ -117,8 +119,11 @@ func newLeafNodeCfg(remote *RemoteLeafOpts) *leafNodeCfg {
// array when receiving async leafnode INFOs.
cfg.urls = append(cfg.urls, cfg.URLs...)
// If we are TLS make sure we save off a proper servername if possible.
// Do same for user/password since we may need them to connect to
// a bare URL that we get from INFO protocol.
for _, u := range cfg.urls {
cfg.saveTLSHostname(u)
cfg.saveUserPassword(u)
}
return cfg
}
Expand Down Expand Up @@ -225,10 +230,19 @@ func (s *Server) connectToRemoteLeafNode(remote *leafNodeCfg, firstConnect bool)

// Save off the tlsName for when we use TLS and mix hostnames and IPs. IPs usually
// come from the server we connect to.
func (lcfg *leafNodeCfg) saveTLSHostname(u *url.URL) {
isTLS := lcfg.TLSConfig != nil || u.Scheme == "tls"
if isTLS && lcfg.tlsName == "" && net.ParseIP(u.Hostname()) == nil {
lcfg.tlsName = u.Hostname()
func (cfg *leafNodeCfg) saveTLSHostname(u *url.URL) {
isTLS := cfg.TLSConfig != nil || u.Scheme == "tls"
if isTLS && cfg.tlsName == "" && net.ParseIP(u.Hostname()) == nil {
cfg.tlsName = u.Hostname()
}
}

// Save off the username/password for when we connect using a bare URL
// that we get from the INFO protocol.
func (cfg *leafNodeCfg) saveUserPassword(u *url.URL) {
if cfg.username == _EMPTY_ && u.User != nil {
cfg.username = u.User.Username()
cfg.password, _ = u.User.Password()
}
}

Expand Down Expand Up @@ -384,8 +398,10 @@ func (c *client) sendLeafConnect(tlsRequired bool) {
cinfo.Sig = sig
} else if userInfo := c.leaf.remote.curURL.User; userInfo != nil {
cinfo.User = userInfo.Username()
pass, _ := userInfo.Password()
cinfo.Pass = pass
cinfo.Pass, _ = userInfo.Password()
} else if c.leaf.remote.username != _EMPTY_ {
cinfo.User = c.leaf.remote.username
cinfo.Pass = c.leaf.remote.password
}

b, err := json.Marshal(cinfo)
Expand Down Expand Up @@ -499,7 +515,7 @@ func (s *Server) createLeafNode(conn net.Conn, remote *leafNodeCfg) *client {
// FIXME(dlc) - Make this resolve at startup.
acc, err := s.LookupAccount(remote.LocalAccount)
if err != nil {
c.Debugf("No local account %q for leafnode", remote.LocalAccount)
s.Fatalf("No local account %q for leafnode: %v", remote.LocalAccount, err)
c.closeConnection(MissingAccount)
return nil
}
Expand Down Expand Up @@ -725,7 +741,10 @@ func (c *client) updateLeafNodeURLs(info *Info) {
// Do not add if it's the same as what we already have configured.
var dup bool
for _, u := range cfg.URLs {
if urlsAreEqual(url, u) {
// URLs that we receive never have user info, but the
// ones that were configured may have. Simply compare
// host and port to decide if they are equal or not.
if url.Host == u.Host && url.Port() == u.Port() {
dup = true
break
}
Expand Down
132 changes: 132 additions & 0 deletions server/leafnode_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"net/url"
"os"
"strings"
"sync"
"testing"
"time"
)
Expand Down Expand Up @@ -261,3 +262,134 @@ func TestLeafNodeTLSRemoteWithNoCerts(t *testing.T) {
t.Fatalf("Expected %v, got: %v", expected, got)
}
}

type captureFatalLogger struct {
DummyLogger
fatal chan string
}

func (l *captureFatalLogger) Fatalf(format string, v ...interface{}) {
l.fatal <- fmt.Sprintf(format, v...)
}

// This test ensures that the server fails [FTL] when soliciting a LN
// connection when account is not found.
func TestLeafNodeAccountNotFound(t *testing.T) {
ob := DefaultOptions()
ob.LeafNode.Host = "127.0.0.1"
ob.LeafNode.Port = -1
sb := RunServer(ob)
defer sb.Shutdown()

u, _ := url.Parse(fmt.Sprintf("nats://127.0.0.1:%d", ob.LeafNode.Port))

logFileName := createConfFile(t, []byte(""))
defer os.Remove(logFileName)

oa := DefaultOptions()
oa.LeafNode.ReconnectInterval = 15 * time.Millisecond
oa.LeafNode.Remotes = []*RemoteLeafOpts{
{
LocalAccount: "foo",
URLs: []*url.URL{u},
},
}
oa.NoLog = false
oa.Debug = false
oa.Trace = false
sa, err := NewServer(oa)
if err != nil {
t.Fatalf("Error creating server: %v", err)
}
defer sa.Shutdown()
l := &captureFatalLogger{fatal: make(chan string, 1)}
sa.SetLogger(l, false, false)

wg := sync.WaitGroup{}
wg.Add(1)
go func() {
defer wg.Done()
sa.Start()
}()

select {
case e := <-l.fatal:
if !strings.Contains(e, "No local account") {
t.Fatalf("Expected fatal error about no local account, got %s", e)
}
case <-time.After(time.Second):
t.Fatalf("Did not get the fatal error")

}
// Since this is a fake logger, the FTL will not make the
// server stop (otherwise the testsuite would exit, which would not be good)
// So shutdown the server to make the go routine with sa.Start() exit
// and wait for the routine to finish.
sa.Shutdown()
wg.Wait()
}

// This test ensures that we can connect using proper user/password
// to a LN URL that was discovered through the INFO protocol.
func TestLeafNodeBasicAuthFailover(t *testing.T) {
content := `
listen: "127.0.0.1:-1"
cluster {
listen: "127.0.0.1:-1"
%s
}
leafnodes {
listen: "127.0.0.1:-1"
authorization {
user: foo
password: pwd
timeout: 1
}
}
`
conf := createConfFile(t, []byte(fmt.Sprintf(content, "")))
defer os.Remove(conf)

sb1, ob1 := RunServerWithConfig(conf)
defer sb1.Shutdown()

conf = createConfFile(t, []byte(fmt.Sprintf(content, fmt.Sprintf("routes: [nats://127.0.0.1:%d]", ob1.Cluster.Port))))
defer os.Remove(conf)

sb2, _ := RunServerWithConfig(conf)
defer sb2.Shutdown()

checkClusterFormed(t, sb1, sb2)

content = `
port: -1
accounts {
foo {}
}
leafnodes {
listen: "127.0.0.1:-1"
remotes [
{
account: "foo"
url: "nats://foo:pwd@127.0.0.1:%d"
}
]
}
`
conf = createConfFile(t, []byte(fmt.Sprintf(content, ob1.LeafNode.Port)))
defer os.Remove(conf)

sa, _ := RunServerWithConfig(conf)
defer sa.Shutdown()

checkLeafNodeConnected(t, sa)

// Shutdown sb1, sa should reconnect to sb2
sb1.Shutdown()

// Wait a bit to make sure there was a disconnect and attempt to reconnect.
time.Sleep(250 * time.Millisecond)

// Should be able to reconnect
checkLeafNodeConnected(t, sa)
}
94 changes: 58 additions & 36 deletions server/routes_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1083,38 +1083,6 @@ func TestRouteNoCrashOnAddingSubToRoute(t *testing.T) {
}

func TestRouteRTT(t *testing.T) {
checkRTT := func(t *testing.T, s *Server, checkForUpdate bool) {
t.Helper()
var route *client
s.mu.Lock()
for _, r := range s.routes {
route = r
break
}
s.mu.Unlock()

prev := time.Duration(0)
check := func(t *testing.T) {
t.Helper()
checkFor(t, time.Second, 15*time.Millisecond, func() error {
route.mu.Lock()
rtt := route.rtt
route.mu.Unlock()
if rtt == 0 || rtt == prev {
return fmt.Errorf("RTT probably not tracked")
}
prev = rtt
return nil
})
}
check(t)
if checkForUpdate {
// Wait a bit and check that rtt is updated
time.Sleep(30 * time.Millisecond)
check(t)
}
}

ob := DefaultOptions()
ob.PingInterval = 15 * time.Millisecond
sb := RunServer(ob)
Expand All @@ -1127,8 +1095,62 @@ func TestRouteRTT(t *testing.T) {
defer sa.Shutdown()

checkClusterFormed(t, sa, sb)
checkRTT(t, sa, true)
checkRTT(t, sb, true)

checkRTT := func(t *testing.T, s *Server) time.Duration {
t.Helper()
var route *client
s.mu.Lock()
for _, r := range s.routes {
route = r
break
}
s.mu.Unlock()

var rtt time.Duration
checkFor(t, time.Second, 15*time.Millisecond, func() error {
route.mu.Lock()
rtt = route.rtt
route.mu.Unlock()
if rtt == 0 {
return fmt.Errorf("RTT not tracked")
}
return nil
})
return rtt
}

prevA := checkRTT(t, sa)
prevB := checkRTT(t, sb)

checkUpdated := func(t *testing.T, s *Server, prev time.Duration) {
t.Helper()
attempts := 0
timeout := time.Now().Add(time.Second)
for time.Now().Before(timeout) {
if rtt := checkRTT(t, s); rtt != 0 {
return
}
attempts++
if attempts == 5 {
// If could be that we are very unlucky
// and the RTT is constant. So override
// the route's RTT to 0 to see if it gets
// updated.
s.mu.Lock()
for _, r := range s.routes {
r.mu.Lock()
r.rtt = 0
r.mu.Unlock()
break
}
s.mu.Unlock()
}
time.Sleep(15 * time.Millisecond)
}
t.Fatalf("RTT probably not updated")
}
checkUpdated(t, sa, prevA)
checkUpdated(t, sb, prevB)

sa.Shutdown()
sb.Shutdown()
Expand All @@ -1147,6 +1169,6 @@ func TestRouteRTT(t *testing.T) {
defer sa.Shutdown()

checkClusterFormed(t, sa, sb)
checkRTT(t, sa, false)
checkRTT(t, sb, false)
checkRTT(t, sa)
checkRTT(t, sb)
}
2 changes: 0 additions & 2 deletions test/leafnode_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -916,8 +916,6 @@ func TestLeafNodeBasicAuth(t *testing.T) {
// This one should work.
lc = createLeafConn(t, opts.LeafNode.Host, opts.LeafNode.Port)
defer lc.Close()

// This should fail since we want u/p
leafSend, leafExpect := setupConnWithUserPass(t, lc, "derek", "s3cr3t!")
leafSend("PING\r\n")
leafExpect(pongRe)
Expand Down

0 comments on commit 0c6e1fa

Please sign in to comment.