fix: race condition UI remotecfg (#2160)

* Refactor ui remtoecfg components to avoid race condition

* Fix accidental cast to pointer that should have been struct

* Update changelog

Author: dehaansa

CHANGELOG.md

@@ -41,6 +41,8 @@ Main (unreleased)
 
 - Fixed a race condition that could lead to a deadlock when using `import` statements, which could lead to a memory leak on `/metrics` endpoint of an Alloy instance. (@thampiotr) 
 
+- Fix a race condition where the ui service was dependent on starting after the remotecfg service, which is not guaranteed. (@dehaansa & @erikbaranowski)
+
 ### Other changes
 
 - Change the stability of the `livedebugging` feature from "experimental" to "generally available". (@wildum)

internal/service/ui/ui.go

@@ -78,10 +78,7 @@ func (s *Service) Data() any {
 func (s *Service) ServiceHandler(host service.Host) (base string, handler http.Handler) {
 	r := mux.NewRouter()
 
-	remotecfgSvc, _ := host.GetService(remotecfg_service.ServiceName)
-	remotecfgHost := remotecfgSvc.Data().(remotecfg_service.Data).Host
-
-	fa := api.NewAlloyAPI(host, remotecfgHost, s.opts.CallbackManager)
+	fa := api.NewAlloyAPI(host, s.opts.CallbackManager)
 	fa.RegisterRoutes(path.Join(s.opts.UIPrefix, "/api/v0/web"), r)
 	ui.RegisterRoutes(s.opts.UIPrefix, r)
 

internal/web/api/api.go

@@ -18,19 +18,19 @@ import (
 	"github.com/grafana/alloy/internal/service"
 	"github.com/grafana/alloy/internal/service/cluster"
 	"github.com/grafana/alloy/internal/service/livedebugging"
+	"github.com/grafana/alloy/internal/service/remotecfg"
 	"github.com/prometheus/prometheus/util/httputil"
 )
 
 // AlloyAPI is a wrapper around the component API.
 type AlloyAPI struct {
 	alloy           service.Host
-	remotecfg       service.Host
 	CallbackManager livedebugging.CallbackManager
 }
 
 // NewAlloyAPI instantiates a new Alloy API.
-func NewAlloyAPI(alloy, remotecfg service.Host, CallbackManager livedebugging.CallbackManager) *AlloyAPI {
-	return &AlloyAPI{alloy: alloy, remotecfg: remotecfg, CallbackManager: CallbackManager}
+func NewAlloyAPI(alloy service.Host, CallbackManager livedebugging.CallbackManager) *AlloyAPI {
+	return &AlloyAPI{alloy: alloy, CallbackManager: CallbackManager}
 }
 
 // RegisterRoutes registers all the API's routes.
@@ -40,67 +40,99 @@ func (a *AlloyAPI) RegisterRoutes(urlPrefix string, r *mux.Router) {
 	// component IDs.
 
 	r.Handle(path.Join(urlPrefix, "/modules/{moduleID:.+}/components"), httputil.CompressionHandler{Handler: listComponentsHandler(a.alloy)})
-	r.Handle(path.Join(urlPrefix, "/remotecfg/modules/{moduleID:.+}/components"), httputil.CompressionHandler{Handler: listComponentsHandler(a.remotecfg)})
+	r.Handle(path.Join(urlPrefix, "/remotecfg/modules/{moduleID:.+}/components"), httputil.CompressionHandler{Handler: listComponentsHandlerRemoteCfg(a.alloy)})
 
 	r.Handle(path.Join(urlPrefix, "/components"), httputil.CompressionHandler{Handler: listComponentsHandler(a.alloy)})
-	r.Handle(path.Join(urlPrefix, "/remotecfg/components"), httputil.CompressionHandler{Handler: listComponentsHandler(a.remotecfg)})
+	r.Handle(path.Join(urlPrefix, "/remotecfg/components"), httputil.CompressionHandler{Handler: listComponentsHandlerRemoteCfg(a.alloy)})
 
 	r.Handle(path.Join(urlPrefix, "/components/{id:.+}"), httputil.CompressionHandler{Handler: getComponentHandler(a.alloy)})
-	r.Handle(path.Join(urlPrefix, "/remotecfg/components/{id:.+}"), httputil.CompressionHandler{Handler: getComponentHandler(a.remotecfg)})
+	r.Handle(path.Join(urlPrefix, "/remotecfg/components/{id:.+}"), httputil.CompressionHandler{Handler: getComponentHandlerRemoteCfg(a.alloy)})
 
 	r.Handle(path.Join(urlPrefix, "/peers"), httputil.CompressionHandler{Handler: getClusteringPeersHandler(a.alloy)})
 	r.Handle(path.Join(urlPrefix, "/debug/{id:.+}"), liveDebugging(a.alloy, a.CallbackManager))
 }
 
 func listComponentsHandler(host service.Host) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
-		// moduleID is set from the /modules/{moduleID:.+}/components route above
-		// but not from the /components route.
-		var moduleID string
-		if vars := mux.Vars(r); vars != nil {
-			moduleID = vars["moduleID"]
-		}
+		listComponentsHandlerInternal(host, w, r)
+	}
+}
 
-		components, err := host.ListComponents(moduleID, component.InfoOptions{
-			GetHealth: true,
-		})
-		if err != nil {
-			http.Error(w, err.Error(), http.StatusInternalServerError)
+func listComponentsHandlerRemoteCfg(host service.Host) http.HandlerFunc {
+	return func(w http.ResponseWriter, r *http.Request) {
+		svc, found := host.GetService(remotecfg.ServiceName)
+		if !found {
+			http.Error(w, "remote config service not available", http.StatusInternalServerError)
 			return
 		}
 
-		bb, err := json.Marshal(components)
-		if err != nil {
-			http.Error(w, err.Error(), http.StatusInternalServerError)
-			return
-		}
-		_, _ = w.Write(bb)
+		listComponentsHandlerInternal(svc.Data().(remotecfg.Data).Host, w, r)
 	}
 }
 
+func listComponentsHandlerInternal(host service.Host, w http.ResponseWriter, r *http.Request) {
+	// moduleID is set from the /modules/{moduleID:.+}/components route above
+	// but not from the /components route.
+	var moduleID string
+	if vars := mux.Vars(r); vars != nil {
+		moduleID = vars["moduleID"]
+	}
+
+	components, err := host.ListComponents(moduleID, component.InfoOptions{
+		GetHealth: true,
+	})
+	if err != nil {
+		http.Error(w, err.Error(), http.StatusInternalServerError)
+		return
+	}
+
+	bb, err := json.Marshal(components)
+	if err != nil {
+		http.Error(w, err.Error(), http.StatusInternalServerError)
+		return
+	}
+	_, _ = w.Write(bb)
+}
+
 func getComponentHandler(host service.Host) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
-		vars := mux.Vars(r)
-		requestedComponent := component.ParseID(vars["id"])
+		getComponentHandlerInternal(host, w, r)
+	}
+}
 
-		component, err := host.GetComponent(requestedComponent, component.InfoOptions{
-			GetHealth:    true,
-			GetArguments: true,
-			GetExports:   true,
-			GetDebugInfo: true,
-		})
-		if err != nil {
-			http.NotFound(w, r)
+func getComponentHandlerRemoteCfg(host service.Host) http.HandlerFunc {
+	return func(w http.ResponseWriter, r *http.Request) {
+		svc, found := host.GetService(remotecfg.ServiceName)
+		if !found {
+			http.Error(w, "remote config service not available", http.StatusInternalServerError)
 			return
 		}
 
-		bb, err := json.Marshal(component)
-		if err != nil {
-			http.Error(w, err.Error(), http.StatusInternalServerError)
-			return
-		}
-		_, _ = w.Write(bb)
+		getComponentHandlerInternal(svc.Data().(remotecfg.Data).Host, w, r)
+	}
+}
+
+func getComponentHandlerInternal(host service.Host, w http.ResponseWriter, r *http.Request) {
+	vars := mux.Vars(r)
+	requestedComponent := component.ParseID(vars["id"])
+
+	component, err := host.GetComponent(requestedComponent, component.InfoOptions{
+		GetHealth:    true,
+		GetArguments: true,
+		GetExports:   true,
+		GetDebugInfo: true,
+	})
+	if err != nil {
+		http.NotFound(w, r)
+		return
+	}
+
+	bb, err := json.Marshal(component)
+	if err != nil {
+		http.Error(w, err.Error(), http.StatusInternalServerError)
+		return
 	}
+	_, _ = w.Write(bb)
 }
 
 func getClusteringPeersHandler(host service.Host) http.HandlerFunc {