refactor(installer): update validation to work with comma-sep beacon …

…node addresses
ethpandaops · Feb 14, 2025 · 9648665 · 9648665
1 parent b6fcbe2
commit 9648665
Show file tree

Hide file tree

Showing 2 changed files with 125 additions and 35 deletions.
diff --git a/internal/validate/beacon.go b/internal/validate/beacon.go
@@ -7,30 +7,57 @@ import (
 	"time"
 )
 
-// ValidateBeaconNodeAddress checks if a beacon node is accessible and healthy.
-func ValidateBeaconNodeAddress(address string) error {
-	if !strings.HasPrefix(address, "http://") && !strings.HasPrefix(address, "https://") {
-		return fmt.Errorf("beacon node address must start with http:// or https://")
-	}
+// ValidateBeaconNodeAddress checks if any beacon node in the comma-separated list is accessible and healthy.
+func ValidateBeaconNodeAddress(addresses string) error {
+	var (
+		nodes   = strings.Split(addresses, ",")
+		lastErr error
+	)
+
+	for _, address := range nodes {
+		address = strings.TrimSpace(address)
+		if !strings.HasPrefix(address, "http://") && !strings.HasPrefix(address, "https://") {
+			lastErr = fmt.Errorf("beacon node address must start with http:// or https://")
 
-	// Skip health check if using Docker network hostname (non-localhost).
-	host := strings.TrimPrefix(strings.TrimPrefix(address, "http://"), "https://")
-	host = strings.Split(host, ":")[0]
+			continue
+		}
+	}
 
-	if !strings.HasPrefix(host, "127.0.0.1") && !strings.HasPrefix(host, "localhost") {
-		return nil
+	if lastErr != nil {
+		return lastErr
 	}
 
-	client := &http.Client{Timeout: 5 * time.Second}
+	for _, address := range nodes {
+		address = strings.TrimSpace(address)
+
+		// Skip health check if using Docker network hostname (non-localhost).
+		host := strings.TrimPrefix(strings.TrimPrefix(address, "http://"), "https://")
+		host = strings.Split(host, ":")[0]
+
+		if !strings.HasPrefix(host, "127.0.0.1") && !strings.HasPrefix(host, "localhost") {
+			return nil
+		}
+
+		client := &http.Client{Timeout: 5 * time.Second}
+
+		resp, err := client.Get(fmt.Sprintf("%s/eth/v1/node/health", address))
+		if err != nil {
+			lastErr = fmt.Errorf("unable to connect to beacon node %s: %w", address, err)
+
+			continue
+		}
+
+		defer resp.Body.Close()
+
+		if resp.StatusCode != http.StatusOK {
+			lastErr = fmt.Errorf("beacon node %s returned status %d", address, resp.StatusCode)
 
-	resp, err := client.Get(fmt.Sprintf("%s/eth/v1/node/health", address))
-	if err != nil {
-		return fmt.Errorf("we're unable to connect to your beacon node: %w", err)
+			continue
+		}
 	}
-	defer resp.Body.Close()
 
-	if resp.StatusCode != http.StatusOK {
-		return fmt.Errorf("beacon node returned status %d", resp.StatusCode)
+	if lastErr != nil {
+		return lastErr
 	}
 
 	return nil

diff --git a/internal/validate/beacon_test.go b/internal/validate/beacon_test.go
@@ -3,36 +3,68 @@ package validate
 import (
 	"net/http"
 	"net/http/httptest"
+	"strings"
 	"testing"
 )
 
 func TestValidateBeaconNodeAddress(t *testing.T) {
 	tests := []struct {
 		name    string
-		server  *httptest.Server
+		servers []*httptest.Server
 		address string
 		wantErr bool
 	}{
 		{
-			name: "valid beacon node",
-			server: httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-				if r.URL.Path != "/eth/v1/node/health" {
-					t.Errorf("expected path /eth/v1/node/health, got %s", r.URL.Path)
-				}
-
-				w.WriteHeader(http.StatusOK)
-			})),
+			name: "single valid beacon node",
+			servers: []*httptest.Server{
+				httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+					if r.URL.Path != "/eth/v1/node/health" {
+						t.Errorf("expected path /eth/v1/node/health, got %s", r.URL.Path)
+					}
+					w.WriteHeader(http.StatusOK)
+				})),
+			},
+			wantErr: false,
+		},
+		{
+			name: "multiple valid nodes",
+			servers: []*httptest.Server{
+				httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+					w.WriteHeader(http.StatusOK)
+				})),
+				httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+					w.WriteHeader(http.StatusOK)
+				})),
+			},
 			wantErr: false,
 		},
 		{
-			name: "unhealthy beacon node",
-			server: httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-				w.WriteHeader(http.StatusServiceUnavailable)
-			})),
+			name:    "multiple nodes with spaces",
+			address: "http://localhost:5053,  http://localhost:5054  ",
+			servers: []*httptest.Server{
+				httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+					w.WriteHeader(http.StatusOK)
+				})),
+				httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+					w.WriteHeader(http.StatusOK)
+				})),
+			},
+			wantErr: false,
+		},
+		{
+			name: "all nodes unhealthy but valid URLs",
+			servers: []*httptest.Server{
+				httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+					w.WriteHeader(http.StatusServiceUnavailable)
+				})),
+				httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+					w.WriteHeader(http.StatusServiceUnavailable)
+				})),
+			},
 			wantErr: true,
 		},
 		{
-			name:    "invalid url scheme",
+			name:    "single invalid url scheme",
 			address: "abc://localhost:5052",
 			wantErr: true,
 		},
@@ -41,24 +73,55 @@ func TestValidateBeaconNodeAddress(t *testing.T) {
 			address: "localhost:5052",
 			wantErr: true,
 		},
+		{
+			name:    "mixed valid and invalid schemes",
+			address: "abc://localhost:5052,http://localhost:5053",
+			wantErr: true,
+		},
 		{
 			name:    "unreachable address",
 			address: "http://localhost:1",
 			wantErr: true,
 		},
+		{
+			name:    "multiple unreachable addresses",
+			address: "http://localhost:1,http://localhost:2",
+			wantErr: true,
+		},
+		{
+			name:    "empty address",
+			address: "",
+			wantErr: true,
+		},
+		{
+			name:    "whitespace only",
+			address: "  ",
+			wantErr: true,
+		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			defer func() {
-				if tt.server != nil {
-					tt.server.Close()
+				for _, server := range tt.servers {
+					if server != nil {
+						server.Close()
+					}
 				}
 			}()
 
 			address := tt.address
-			if tt.server != nil {
-				address = tt.server.URL
+			if len(tt.servers) > 0 {
+				addresses := make([]string, len(tt.servers))
+				for i, server := range tt.servers {
+					addresses[i] = server.URL
+				}
+				// Add spaces for the "multiple nodes with spaces" test
+				if tt.name == "multiple nodes with spaces" {
+					address = strings.Join(addresses, ", ")
+				} else {
+					address = strings.Join(addresses, ",")
+				}
 			}
 
 			err := ValidateBeaconNodeAddress(address)