Rework peer connection status based on the update channel existence (#1213)

With this change, we don't need to update all peers on startup. We will
check the existence of an update channel when returning a list or single peer on API.
Then after restarting of server consumers of API will see peer not
connected status till the creation of an updated channel which indicates
peer successful connection.

management/server/http/peers_handler.go

@@ -31,6 +31,24 @@ func NewPeersHandler(accountManager server.AccountManager, authCfg AuthCfg) *Pee
 	}
 }
 
+func (h *PeersHandler) checkPeerStatus(peer *server.Peer) (*server.Peer, error) {
+	peerToReturn := peer.Copy()
+	if peer.Status.Connected {
+		statuses, err := h.accountManager.GetAllConnectedPeers()
+		if err != nil {
+			return peerToReturn, err
+		}
+
+		// Although we have online status in store we do not yet have an updated channel so have to show it as disconnected
+		// This may happen after server restart when not all peers are yet connected
+		if _, connected := statuses[peerToReturn.ID]; !connected {
+			peerToReturn.Status.Connected = false
+		}
+	}
+
+	return peerToReturn, nil
+}
+
 func (h *PeersHandler) getPeer(account *server.Account, peerID, userID string, w http.ResponseWriter) {
 	peer, err := h.accountManager.GetPeer(account.Id, peerID, userID)
 	if err != nil {
@@ -38,7 +56,13 @@ func (h *PeersHandler) getPeer(account *server.Account, peerID, userID string, w
 		return
 	}
 
-	util.WriteJSONObject(w, toPeerResponse(peer, account, h.accountManager.GetDNSDomain()))
+	peerToReturn, err := h.checkPeerStatus(peer)
+	if err != nil {
+		util.WriteError(err, w)
+		return
+	}
+
+	util.WriteJSONObject(w, toPeerResponse(peerToReturn, account, h.accountManager.GetDNSDomain()))
 }
 
 func (h *PeersHandler) updatePeer(account *server.Account, user *server.User, peerID string, w http.ResponseWriter, r *http.Request) {
@@ -120,7 +144,12 @@ func (h *PeersHandler) GetAllPeers(w http.ResponseWriter, r *http.Request) {
 
 		respBody := []*api.Peer{}
 		for _, peer := range peers {
-			respBody = append(respBody, toPeerResponse(peer, account, dnsDomain))
+			peerToReturn, err := h.checkPeerStatus(peer)
+			if err != nil {
+				util.WriteError(err, w)
+				return
+			}
+			respBody = append(respBody, toPeerResponse(peerToReturn, account, dnsDomain))
 		}
 		util.WriteJSONObject(w, respBody)
 		return

management/server/http/peers_handler_test.go

@@ -3,6 +3,7 @@ package http
 import (
 	"bytes"
 	"encoding/json"
+	"fmt"
 	"io"
 	"net"
 	"net/http"
@@ -23,19 +24,33 @@ import (
 )
 
 const testPeerID = "test_peer"
+const noUpdateChannelTestPeerID = "no-update-channel"
 
 func initTestMetaData(peers ...*server.Peer) *PeersHandler {
 	return &PeersHandler{
 		accountManager: &mock_server.MockAccountManager{
 			UpdatePeerFunc: func(accountID, userID string, update *server.Peer) (*server.Peer, error) {
-				p := peers[0].Copy()
+				var p *server.Peer
+				for _, peer := range peers {
+					if update.ID == peer.ID {
+						p = peer.Copy()
+						break
+					}
+				}
 				p.SSHEnabled = update.SSHEnabled
 				p.LoginExpirationEnabled = update.LoginExpirationEnabled
 				p.Name = update.Name
 				return p, nil
 			},
 			GetPeerFunc: func(accountID, peerID, userID string) (*server.Peer, error) {
-				return peers[0], nil
+				var p *server.Peer
+				for _, peer := range peers {
+					if peerID == peer.ID {
+						p = peer.Copy()
+						break
+					}
+				}
+				return p, nil
 			},
 			GetPeersFunc: func(accountID, userID string) ([]*server.Peer, error) {
 				return peers, nil
@@ -57,6 +72,16 @@ func initTestMetaData(peers ...*server.Peer) *PeersHandler {
 					},
 				}, user, nil
 			},
+			GetAllConnectedPeersFunc: func() (map[string]struct{}, error) {
+				statuses := make(map[string]struct{})
+				for _, peer := range peers {
+					if peer.ID == noUpdateChannelTestPeerID {
+						break
+					}
+					statuses[peer.ID] = struct{}{}
+				}
+				return statuses, nil
+			},
 		},
 		claimsExtractor: jwtclaims.NewClaimsExtractor(
 			jwtclaims.WithFromRequestContext(func(r *http.Request) jwtclaims.AuthorizationClaims {
@@ -79,7 +104,7 @@ func TestGetPeers(t *testing.T) {
 		Key:                    "key",
 		SetupKey:               "setupkey",
 		IP:                     net.ParseIP("100.64.0.1"),
-		Status:                 &server.PeerStatus{},
+		Status:                 &server.PeerStatus{Connected: true},
 		Name:                   "PeerName",
 		LoginExpirationEnabled: false,
 		Meta: server.PeerSystemMeta{
@@ -93,11 +118,17 @@ func TestGetPeers(t *testing.T) {
 		},
 	}
 
+	peer1 := peer.Copy()
+	peer1.ID = noUpdateChannelTestPeerID
+
 	expectedUpdatedPeer := peer.Copy()
 	expectedUpdatedPeer.LoginExpirationEnabled = true
 	expectedUpdatedPeer.SSHEnabled = true
 	expectedUpdatedPeer.Name = "New Name"
 
+	expectedPeer1 := peer1.Copy()
+	expectedPeer1.Status.Connected = false
+
 	tt := []struct {
 		name           string
 		expectedStatus int
@@ -116,13 +147,21 @@ func TestGetPeers(t *testing.T) {
 			expectedPeer:   peer,
 		},
 		{
-			name:           "GetPeer",
+			name:           "GetPeer with update channel",
 			requestType:    http.MethodGet,
 			requestPath:    "/api/peers/" + testPeerID,
 			expectedStatus: http.StatusOK,
 			expectedArray:  false,
 			expectedPeer:   peer,
 		},
+		{
+			name:           "GetPeer with no update channel",
+			requestType:    http.MethodGet,
+			requestPath:    "/api/peers/" + peer1.ID,
+			expectedStatus: http.StatusOK,
+			expectedArray:  false,
+			expectedPeer:   expectedPeer1,
+		},
 		{
 			name:           "PutPeer",
 			requestType:    http.MethodPut,
@@ -136,7 +175,7 @@ func TestGetPeers(t *testing.T) {
 
 	rr := httptest.NewRecorder()
 
-	p := initTestMetaData(peer)
+	p := initTestMetaData(peer, peer1)
 
 	for _, tc := range tt {
 		t.Run(tc.name, func(t *testing.T) {
@@ -171,6 +210,10 @@ func TestGetPeers(t *testing.T) {
 					t.Fatalf("Sent content is not in correct json format; %v", err)
 				}
 
+				// hardcode this check for now as we only have two peers in this suite
+				assert.Equal(t, len(respBody), 2)
+				assert.Equal(t, respBody[1].Connected, false)
+
 				got = respBody[0]
 			} else {
 				got = &api.Peer{}
@@ -180,12 +223,15 @@ func TestGetPeers(t *testing.T) {
 				}
 			}
 
+			fmt.Println(got)
+
 			assert.Equal(t, got.Name, tc.expectedPeer.Name)
 			assert.Equal(t, got.Version, tc.expectedPeer.Meta.WtVersion)
 			assert.Equal(t, got.Ip, tc.expectedPeer.IP.String())
 			assert.Equal(t, got.Os, "OS core")
 			assert.Equal(t, got.LoginExpirationEnabled, tc.expectedPeer.LoginExpirationEnabled)
 			assert.Equal(t, got.SshEnabled, tc.expectedPeer.SSHEnabled)
+			assert.Equal(t, got.Connected, tc.expectedPeer.Status.Connected)
 		})
 	}
 }