pkg/api/api0: Refactor server verification process

* Add an additional pending state for unverified servers.
* This allows verification to be done without knowing ahead of time
  if a server is going to be created or updated.
* Also moves more logic into the serverlist.

2 files changed, 113 insertions, 59 deletions

diff --git a/pkg/api/api0/server.go b/pkg/api/api0/server.go
index dc0391d..29cbcda 100644
--- a/pkg/api/api0/server.go
+++ b/pkg/api/api0/server.go
@@ -206,51 +206,6 @@ func (h *Handler) handleServerAddServer(w http.ResponseWriter, r *http.Request)
 			Msgf("failed to parse modinfo")
 	}
 
-	verifyDeadline := time.Now().Add(time.Second * 10)
-	if err := func() error {
-		ctx, cancel := context.WithDeadline(r.Context(), verifyDeadline)
-		defer cancel()
-
-		req, err := http.NewRequestWithContext(ctx, http.MethodGet, fmt.Sprintf("http://%s/verify", s.AuthAddr()), nil)
-		if err != nil {
-			return err
-		}
-		req.Header.Set("User-Agent", "Atlas")
-
-		resp, err := http.DefaultClient.Do(req)
-		if err != nil {
-			return err
-		}
-		defer resp.Body.Close()
-
-		buf, err := io.ReadAll(io.LimitReader(resp.Body, 100))
-		if err != nil {
-			return err
-		}
-		if string(bytes.TrimSpace(buf)) != "I am a northstar server!" {
-			return fmt.Errorf("unexpected response")
-		}
-		return nil
-	}(); err != nil {
-		if errors.Is(err, context.DeadlineExceeded) {
-			err = fmt.Errorf("request timed out")
-		}
-		respJSON(w, r, http.StatusBadGateway, map[string]any{
-			"success": false,
-			"error":   ErrorCode_BAD_GAMESERVER_RESPONSE,
-			"msg":     ErrorCode_BAD_GAMESERVER_RESPONSE.Messagef("failed to connect to auth port: %v", err),
-		})
-		return
-	}
-	if err := a2s.Probe(s.Addr, time.Until(verifyDeadline)); err != nil {
-		respJSON(w, r, http.StatusBadGateway, map[string]any{
-			"success": false,
-			"error":   ErrorCode_BAD_GAMESERVER_RESPONSE,
-			"msg":     ErrorCode_BAD_GAMESERVER_RESPONSE.Messagef("failed to connect to game port: %v", err),
-		})
-		return
-	}
-
 	var l ServerListLimit
 	if n := h.MaxServers; n > 0 {
 		l.MaxServers = n
@@ -292,6 +247,65 @@ func (h *Handler) handleServerAddServer(w http.ResponseWriter, r *http.Request)
 		return
 	}
 
+	if !nsrv.VerificationDeadline.IsZero() {
+		if err := func() error {
+			ctx, cancel := context.WithDeadline(r.Context(), nsrv.VerificationDeadline)
+			defer cancel()
+
+			req, err := http.NewRequestWithContext(ctx, http.MethodGet, fmt.Sprintf("http://%s/verify", s.AuthAddr()), nil)
+			if err != nil {
+				return err
+			}
+			req.Header.Set("User-Agent", "Atlas")
+
+			resp, err := http.DefaultClient.Do(req)
+			if err != nil {
+				return err
+			}
+			defer resp.Body.Close()
+
+			buf, err := io.ReadAll(io.LimitReader(resp.Body, 100))
+			if err != nil {
+				return err
+			}
+			if string(bytes.TrimSpace(buf)) != "I am a northstar server!" {
+				return fmt.Errorf("unexpected response")
+			}
+			return nil
+		}(); err != nil {
+			if errors.Is(err, context.DeadlineExceeded) {
+				err = fmt.Errorf("request timed out")
+			}
+			var code ErrorCode
+			if err.Error() == "unexpected response" {
+				code = ErrorCode_BAD_GAMESERVER_RESPONSE
+			} else {
+				code = ErrorCode_NO_GAMESERVER_RESPONSE
+			}
+			respJSON(w, r, http.StatusBadGateway, map[string]any{
+				"success": false,
+				"error":   code,
+				"msg":     code.Messagef("failed to connect to auth port: %v", err),
+			})
+			return
+		}
+		if err := a2s.Probe(s.Addr, time.Until(nsrv.VerificationDeadline)); err != nil {
+			respJSON(w, r, http.StatusBadGateway, map[string]any{
+				"success": false,
+				"error":   ErrorCode_BAD_GAMESERVER_RESPONSE,
+				"msg":     ErrorCode_BAD_GAMESERVER_RESPONSE.Messagef("failed to connect to game port: %v", err),
+			})
+			return
+		}
+		if !h.ServerList.VerifyServer(nsrv.ID) {
+			respJSON(w, r, http.StatusBadGateway, map[string]any{
+				"success": false,
+				"error":   ErrorCode_NO_GAMESERVER_RESPONSE,
+				"msg":     ErrorCode_NO_GAMESERVER_RESPONSE.Messagef("verification timed out"),
+			})
+		}
+	}
+
 	respJSON(w, r, http.StatusOK, map[string]any{
 		"success":         true,
 		"id":              nsrv.ID,
diff --git a/pkg/api/api0/serverlist.go b/pkg/api/api0/serverlist.go
index ca57280..3248465 100644
--- a/pkg/api/api0/serverlist.go
+++ b/pkg/api/api0/serverlist.go
@@ -16,11 +16,12 @@ import (
 
 // ServerList stores information about registered game servers. It does not do
 // any validation of its own except for ensuring ID and addr/port are unique,
-// and filtering dead servers.
+// and filtering dead/unverified/ghost servers.
 type ServerList struct {
 	// config (must not be changed after the ServerList is used)
-	deadTime  time.Duration
-	ghostTime time.Duration
+	verifyTime time.Duration
+	deadTime   time.Duration
+	ghostTime  time.Duration
 
 	// servers
 	mu       sync.RWMutex               // must be held while modifying the order and maps below
@@ -56,11 +57,13 @@ type Server struct {
 	AuthPort    uint16
 	Password    string // blank for none
 
-	LastHeartbeat time.Time
-	PlayerCount   int
-	MaxPlayers    int
-	Map           string
-	Playlist      string
+	VerificationDeadline time.Time // zero once verified
+	LastHeartbeat        time.Time
+
+	PlayerCount int
+	MaxPlayers  int
+	Map         string
+	Playlist    string
 
 	ServerAuthToken string // used for authenticating the masterserver to the gameserver authserver
 
@@ -111,6 +114,9 @@ type ServerListLimit struct {
 
 // NewServerList initializes a new server list.
 //
+// verifyTime is the amount of time a server has to complete verification after
+// it is created.
+//
 // deadTime is the time since the last heartbeat after which a server is
 // considered dead. A dead server will not be listed on the server list. It must
 // be >= verifyTime if nonzero.
@@ -122,9 +128,15 @@ type ServerListLimit struct {
 //
 // If both are nonzero, they must be positive, and deadTime must be less than
 // ghostTime. Otherwise, NewServerList will panic.
-func NewServerList(deadTime, ghostTime time.Duration) *ServerList {
+func NewServerList(deadTime, ghostTime, verifyTime time.Duration) *ServerList {
+	if verifyTime < 0 {
+		panic("api0: serverlist: verifyTime must be >= 0")
+	}
 	if deadTime < 0 {
-		panic("api0: serverlist: deadTime must be >= 0")
+		panic("api0: serverlist: deadTime must be >= verifyTime")
+	}
+	if deadTime != 0 && verifyTime != 0 && deadTime <= verifyTime {
+		panic("api0: server")
 	}
 	if ghostTime < 0 {
 		panic("api0: serverlist: ghostTime must be >= 0")
@@ -133,8 +145,9 @@ func NewServerList(deadTime, ghostTime time.Duration) *ServerList {
 		panic("api0: serverlist: deadTime must be <= ghostTime")
 	}
 	return &ServerList{
-		deadTime:  deadTime,
-		ghostTime: ghostTime,
+		verifyTime: verifyTime,
+		deadTime:   deadTime,
+		ghostTime:  ghostTime,
 	}
 }
 
@@ -416,6 +429,8 @@ var (
 // non-nil) (reviving it if necessary), and if that fails, then attempts to
 // create/replace a server by the gameserver ip/port instead (if c is non-nil)
 // while following the limits in l. It returns a copy of the resulting Server.
+// If the resulting server's VerificationDeadline is nonzero, the server must be
+// verified.
 //
 // If the returned error is non-nil, it will either be an unavoidable internal
 // error (e.g, failure to get random data for the server id) or one of the
@@ -427,8 +442,8 @@ var (
 //   - ErrServerListLimitExceeded - if adding the server would exceed server limits (if c and l)
 //
 // When creating a server using the values from c: c.Order, c.ID,
-// c.ServerAuthToken, and c.LastHeartbeat will be generated by this function
-// (any existing value is ignored).
+// c.ServerAuthToken, c.VerificationDeadline, and c.LastHeartbeat will be
+// generated by this function (any existing value is ignored).
 func (s *ServerList) ServerHybridUpdatePut(u *ServerUpdate, c *Server, l ServerListLimit) (*Server, error) {
 	t := s.now()
 
@@ -588,6 +603,11 @@ func (s *ServerList) ServerHybridUpdatePut(u *ServerUpdate, c *Server, l ServerL
 		// set the heartbeat time to the current time
 		nsrv.LastHeartbeat = t
 
+		// set the verification deadline
+		if s.verifyTime != 0 {
+			nsrv.VerificationDeadline = t.Add(s.verifyTime)
+		}
+
 		// remove the existing server so we can add the new one
 		if toReplace != nil {
 			s.freeServer(toReplace)
@@ -612,6 +632,20 @@ func (s *ServerList) ServerHybridUpdatePut(u *ServerUpdate, c *Server, l ServerL
 	return nil, ErrServerListUpdateServerDead
 }
 
+// VerifyServer marks the server with the provided id as verified. If it does
+// not exist, false is returned.
+func (s *ServerList) VerifyServer(id string) bool {
+	// take a write lock on the server list
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
+	if srv, exists := s.servers2[id]; exists {
+		srv.VerificationDeadline = time.Time{}
+		return true
+	}
+	return false
+}
+
 // ReapServers deletes dead servers from memory.
 func (s *ServerList) ReapServers() {
 	t := s.now()
@@ -668,6 +702,12 @@ func (s *ServerList) serverState(x *Server, t time.Time) serverListState {
 	if x == nil {
 		return serverListStateGone
 	}
+	if !x.VerificationDeadline.IsZero() {
+		if t.After(x.VerificationDeadline) {
+			return serverListStateGone
+		}
+		return serverListStatePending
+	}
 
 	d := t.Sub(x.LastHeartbeat)
 	if s.deadTime == 0 || d < s.deadTime {