From e4623b901da26446708ea1673017bd547376a5c6 Mon Sep 17 00:00:00 2001 From: Walid Baruni Date: Thu, 24 Oct 2024 14:06:42 +0200 Subject: [PATCH] improve routing of webui requests (#4645) Have the webui server act as a reverse proxy and route requests to the orchestrator instead of the frontend directly calling the orchestrator. This will simplify webui setup instead of having to define the `WebUI.Backend` to the orchestrator's public ip with bacalhau serve The configuration `WebUI.Backend` is still available and make accepting endpoints more graceful ### Testing done: Mostly manual testing, but verified the following: 1. Routing of basic http requests 2. Routing of websockets, such as log stream 3. Disconnecting and reconnecting the backend 4. Setting `WebUI.Backend` to the demo network. Doesn't make sense today, but in case in the future we want to spin up the WebUI server in a separate process from the orchestrator --- webui/app/providers/ApiProvider.tsx | 15 +-- .../components/ConnectionStatusIndicator.tsx | 8 +- webui/components/jobs/details/JobLogs.tsx | 12 +-- webui/hooks/useConnectionMonitor.tsx | 4 +- webui/lib/api/index.ts | 37 +------ webui/webui.go | 99 ++++++++++++++----- 6 files changed, 91 insertions(+), 84 deletions(-) diff --git a/webui/app/providers/ApiProvider.tsx b/webui/app/providers/ApiProvider.tsx index 56c82367a9..ade3c12910 100644 --- a/webui/app/providers/ApiProvider.tsx +++ b/webui/app/providers/ApiProvider.tsx @@ -1,30 +1,21 @@ 'use client' -import { - ReactNode, - createContext, - useContext, - useState, - useEffect, -} from 'react' -import { useApiInitialization, useApiUrl } from '@/lib/api' +import { ReactNode, createContext, useContext } from 'react' +import { useApiInitialization } from '@/lib/api' interface ApiContextType { isInitialized: boolean - apiUrl: string | null } const ApiContext = createContext({ isInitialized: false, - apiUrl: null, }) export function ApiProvider({ children }: { children: ReactNode }) { const isInitialized = useApiInitialization() - const apiUrl = useApiUrl() return ( - + {children} ) diff --git a/webui/components/ConnectionStatusIndicator.tsx b/webui/components/ConnectionStatusIndicator.tsx index 4aa0a98808..6a937c12df 100644 --- a/webui/components/ConnectionStatusIndicator.tsx +++ b/webui/components/ConnectionStatusIndicator.tsx @@ -12,7 +12,7 @@ import { } from '@/components/ui/tooltip' export function ConnectionStatusIndicator() { - const { isOnline, clientUrl } = useConnectionMonitor() + const { isOnline } = useConnectionMonitor() const { toast } = useToast() const [prevOnlineState, setPrevOnlineState] = useState( undefined @@ -26,7 +26,7 @@ export function ConnectionStatusIndicator() { toast({ variant: 'destructive', title: 'Connection Lost', - description: `You are currently offline. Please check your connection to ${clientUrl}.`, + description: `You are currently offline. Please check your connection and that Bacalhau is still running.`, duration: Infinity, }) } else if (isOnline && prevOnlineState === false) { @@ -41,7 +41,7 @@ export function ConnectionStatusIndicator() { } setPrevOnlineState(isOnline) - }, [isOnline, prevOnlineState, toast, clientUrl]) + }, [isOnline, prevOnlineState, toast]) const getIconColor = () => { if (isOnline === undefined) return 'text-gray-500' @@ -51,7 +51,7 @@ export function ConnectionStatusIndicator() { const tooltipContent = isOnline === undefined ? 'Checking connection...' - : `${isOnline ? 'Connected to' : 'Failed to connect to'} ${clientUrl}` + : `${isOnline ? 'Connected successfully' : 'Failed to connect'}` return ( <> diff --git a/webui/components/jobs/details/JobLogs.tsx b/webui/components/jobs/details/JobLogs.tsx index d86f9b3830..6ccafa1737 100644 --- a/webui/components/jobs/details/JobLogs.tsx +++ b/webui/components/jobs/details/JobLogs.tsx @@ -9,7 +9,6 @@ import { CheckCircle, } from 'lucide-react' import { useApi } from '@/app/providers/ApiProvider' -import { client } from '@/lib/api/generated' interface LogEntry { type: number @@ -43,15 +42,7 @@ const JobLogs = ({ jobId }: { jobId: string | undefined }) => { setError(null) setIsStreamEnded(false) - const baseUrl = client.getConfig().baseUrl - if (!baseUrl) { - console.error('Base URL is not set') - setError( - 'Failed to connect to log stream. Client not configured properly.' - ) - return - } - const wsUrl = `${baseUrl.replace(/^http/, 'ws')}/api/v1/orchestrator/jobs/${jobId}/logs?follow=true` + const wsUrl = `/api/v1/orchestrator/jobs/${jobId}/logs?follow=true` console.log('Attempting to connect to:', wsUrl) const ws = new WebSocket(wsUrl) @@ -62,7 +53,6 @@ const JobLogs = ({ jobId }: { jobId: string | undefined }) => { } ws.onmessage = (event) => { - console.log('Received message:', event.data) try { const message = JSON.parse(event.data) if (message.value && message.value.Line) { diff --git a/webui/hooks/useConnectionMonitor.tsx b/webui/hooks/useConnectionMonitor.tsx index d7702d7487..e968ae7fc0 100644 --- a/webui/hooks/useConnectionMonitor.tsx +++ b/webui/hooks/useConnectionMonitor.tsx @@ -5,7 +5,7 @@ import { useApi } from '@/app/providers/ApiProvider' import { Ops } from '@/lib/api/generated' export const useConnectionMonitor = (checkInterval = 5000) => { - const { isInitialized, apiUrl } = useApi() + const { isInitialized } = useApi() const [isOnline, setIsOnline] = useState(undefined) const [error, setError] = useState(null) @@ -35,5 +35,5 @@ export const useConnectionMonitor = (checkInterval = 5000) => { return () => clearInterval(intervalId) }, [checkConnection, checkInterval]) - return { isOnline, checkConnection, clientUrl: apiUrl, error } + return { isOnline, checkConnection, error } } diff --git a/webui/lib/api/index.ts b/webui/lib/api/index.ts index 03942bf328..7a7baabf1f 100644 --- a/webui/lib/api/index.ts +++ b/webui/lib/api/index.ts @@ -1,47 +1,18 @@ import { client } from './generated' import { useState, useEffect } from 'react' -interface Config { - APIEndpoint: string -} - -const DEFAULT_API_URL = 'http://localhost:1234' - -async function fetchConfig(): Promise { - try { - const response = await fetch('/_config') - if (!response.ok) { - throw new Error(`Failed to fetch config: ${response.statusText}`) - } - return await response.json() - } catch (error) { - console.warn('Config fetch failed, assuming standalone mode:', error) - return null - } -} - -let apiUrl: string | null = null - -export async function initializeApi(): Promise { - const config = await fetchConfig() - apiUrl = config?.APIEndpoint || DEFAULT_API_URL - - client.setConfig({ baseUrl: apiUrl }) - - console.log('API initialized with URL:', apiUrl) - return apiUrl +export function initializeApi(): void { + client.setConfig({ baseUrl: "" }) } export function useApiInitialization() { const [isInitialized, setIsInitialized] = useState(false) useEffect(() => { - initializeApi().then(() => setIsInitialized(true)) + initializeApi() + setIsInitialized(true) }, []) return isInitialized } -export function useApiUrl() { - return apiUrl -} diff --git a/webui/webui.go b/webui/webui.go index ab226e60ae..fa235c65e2 100644 --- a/webui/webui.go +++ b/webui/webui.go @@ -4,15 +4,17 @@ import ( "context" "embed" "encoding/json" + "errors" "fmt" "io" "io/fs" "net" "net/http" + "net/http/httputil" + "net/url" "os" "path" "strings" - "sync" "time" "github.com/rs/zerolog/log" @@ -27,25 +29,30 @@ type Config struct { } type Server struct { - config Config - configLock sync.RWMutex - mux *http.ServeMux + apiURL *url.URL + listenAddress string + mux *http.ServeMux + apiProxy *httputil.ReverseProxy } func NewServer(cfg Config) (*Server, error) { if cfg.Listen == "" { return nil, fmt.Errorf("listen address cannot be empty") } - if cfg.APIEndpoint == "" { - return nil, fmt.Errorf("API endpoint cannot be empty") + // Parse and validate API endpoint + apiURL, err := normalizeAPIEndpoint(cfg.APIEndpoint) + if err != nil { + return nil, fmt.Errorf("invalid API endpoint: %w", err) } s := &Server{ - config: cfg, - mux: http.NewServeMux(), + apiURL: apiURL, + listenAddress: cfg.Listen, + mux: http.NewServeMux(), + apiProxy: newReverseProxy(apiURL), } - s.mux.HandleFunc("/_config", s.handleConfig) + s.mux.HandleFunc("/api/", s.handleAPIProxy) s.mux.HandleFunc("/", s.handleFiles) return s, nil @@ -53,7 +60,7 @@ func NewServer(cfg Config) (*Server, error) { func (s *Server) ListenAndServe(ctx context.Context) error { server := &http.Server{ - Addr: s.config.Listen, + Addr: s.listenAddress, Handler: s.mux, ReadTimeout: time.Minute, WriteTimeout: time.Minute, @@ -62,8 +69,7 @@ func (s *Server) ListenAndServe(ctx context.Context) error { BaseContext: func(l net.Listener) context.Context { return ctx }, } - log.Info().Str("listen", s.config.Listen).Msg("Starting UI server") - + // Graceful shutdown go func() { <-ctx.Done() shutdownCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second) @@ -73,22 +79,21 @@ func (s *Server) ListenAndServe(ctx context.Context) error { } }() - if err := server.ListenAndServe(); err != nil && err != http.ErrServerClosed { + log.Info().Str("listen", server.Addr).Str("backend", sanitizeURL(s.apiURL)).Msg("Starting server") + if err := server.ListenAndServe(); !errors.Is(err, http.ErrServerClosed) { return fmt.Errorf("server error: %w", err) } return nil } -func (s *Server) handleConfig(w http.ResponseWriter, r *http.Request) { - s.configLock.RLock() - defer s.configLock.RUnlock() - - w.Header().Set("Content-Type", "application/json") - if err := json.NewEncoder(w).Encode(s.config); err != nil { - log.Error().Err(err).Msg("Failed to encode config") - http.Error(w, "Internal Server Error", http.StatusInternalServerError) - } +func (s *Server) handleAPIProxy(w http.ResponseWriter, r *http.Request) { + log.Trace(). + Str("path", r.URL.Path). + Str("method", r.Method). + Bool("is_websocket", r.Header.Get("Upgrade") == "websocket"). + Msg("Proxying request") + s.apiProxy.ServeHTTP(w, r) } func (s *Server) handleFiles(w http.ResponseWriter, r *http.Request) { @@ -232,3 +237,53 @@ func (s *Server) serve404(w http.ResponseWriter, r *http.Request) { log.Error().Err(err).Msg("Failed to write 404 page content") } } + +// normalizeAPIEndpoint validates and normalizes the API endpoint URL +func normalizeAPIEndpoint(endpoint string) (*url.URL, error) { + if !strings.HasPrefix(strings.ToLower(endpoint), "http") { + endpoint = "http://" + endpoint + } + + u, err := url.Parse(endpoint) + if err != nil { + return nil, fmt.Errorf("invalid URL: %w", err) + } + + if u.Scheme != "http" && u.Scheme != "https" { + return nil, fmt.Errorf("unsupported scheme %q", u.Scheme) + } + + if u.Host == "" { + return nil, fmt.Errorf("missing host") + } + + u.Path = strings.TrimSuffix(u.Path, "/") + return u, nil +} + +func newReverseProxy(target *url.URL) *httputil.ReverseProxy { + proxy := httputil.NewSingleHostReverseProxy(target) + proxy.ErrorHandler = func(w http.ResponseWriter, r *http.Request, err error) { + response := map[string]string{ + "error": "Backend service unavailable", + "endpoint": sanitizeURL(target), + "method": r.Method, + "path": r.URL.Path, + } + + log.Error().Err(err).Any("response", response).Msg("Proxy error") + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusBadGateway) + json.NewEncoder(w).Encode(response) + } + return proxy +} + +// sanitizeURL removes sensitive information from URL for error messages +func sanitizeURL(u *url.URL) string { + sanitized := *u + sanitized.User = nil + sanitized.RawQuery = "" + sanitized.Fragment = "" + return sanitized.String() +}