Merge pull request #2465 from headlamp-k8s/prevent-double-refresh

backend: Prevent sending refresh signal before the app is ready

backend/cmd/headlamp.go

@@ -304,6 +304,12 @@ func addPluginRoutes(config *HeadlampConfig, r *mux.Router) {
 		}
 		if err := json.NewEncoder(w).Encode(pluginsList); err != nil {
 			logger.Log(logger.LevelError, nil, err, "encoding plugins base paths list")
+		} else {
+			// Notify that the client has requested the plugins list. So we can start sending
+			// refresh requests.
+			if err := config.cache.Set(context.Background(), plugins.PluginCanSendRefreshKey, true); err != nil {
+				logger.Log(logger.LevelError, nil, err, "setting plugin-can-send-refresh key")
+			}
 		}
 	}).Methods("GET")
 

backend/pkg/plugins/plugins.go

@@ -3,6 +3,7 @@ package plugins
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io/fs"
 	"net/http"
@@ -19,9 +20,10 @@ import (
 )
 
 const (
-	PluginRefreshKey       = "PLUGIN_REFRESH"
-	PluginListKey          = "PLUGIN_LIST"
-	subFolderWatchInterval = 5 * time.Second
+	PluginRefreshKey        = "PLUGIN_REFRESH"
+	PluginListKey           = "PLUGIN_LIST"
+	PluginCanSendRefreshKey = "PLUGIN_CAN_SEND_REFRESH"
+	subFolderWatchInterval  = 5 * time.Second
 )
 
 // Watch watches the given path for changes and sends the events to the notify channel.
@@ -203,7 +205,7 @@ func pluginBasePathListForDir(pluginDir string, baseURL string) ([]string, error
 		_, err = os.Stat(packageJSONPath)
 		if err != nil {
 			logger.Log(logger.LevelInfo, map[string]string{"packageJSONPath": packageJSONPath},
-				err, `Not including plugin path, package.json not found. 
+				err, `Not including plugin path, package.json not found.
 				Please run 'headlamp-plugin extract' again with headlamp-plugin >= 0.6.0`)
 		}
 
@@ -214,14 +216,35 @@ func pluginBasePathListForDir(pluginDir string, baseURL string) ([]string, error
 	return pluginListURLs, nil
 }
 
+func canSendRefresh(c cache.Cache[interface{}]) bool {
+	value, err := c.Get(context.Background(), PluginCanSendRefreshKey)
+	if err != nil {
+		if errors.Is(err, cache.ErrNotFound) {
+			return false
+		}
+
+		logger.Log(logger.LevelError, map[string]string{"key": PluginCanSendRefreshKey},
+			err, "getting plugin-can-send-refresh key")
+	}
+
+	canSendRefresh, ok := value.(bool)
+	if !ok {
+		logger.Log(logger.LevelInfo, nil, nil, "converting plugin-can-send-refresh key to bool")
+	}
+
+	return canSendRefresh
+}
+
 // HandlePluginEvents handles the plugin events by updating the plugin list
 // and plugin refresh key in the cache.
 func HandlePluginEvents(staticPluginDir, pluginDir string,
 	notify <-chan string, cache cache.Cache[interface{}],
 ) {
 	for range notify {
-		// set the plugin refresh key to true
-		err := cache.Set(context.Background(), PluginRefreshKey, true)
+		// Set the refresh signal only if we cannot send it. We prevent it here
+		// because we only want to send refresh signals that *happen after* we are
+		// allowed to send them.
+		err := cache.Set(context.Background(), PluginRefreshKey, canSendRefresh(cache))
 		if err != nil {
 			logger.Log(logger.LevelError, nil, err, "setting plugin refresh key")
 		}
@@ -267,6 +290,11 @@ func PopulatePluginsCache(staticPluginDir, pluginDir string, cache cache.Cache[i
 // and sends a signal to the frontend to reload the plugins by setting
 // the X-Reload header to reload.
 func HandlePluginReload(cache cache.Cache[interface{}], w http.ResponseWriter) {
+	// Avoid processing if we cannot send refresh signals.
+	if !canSendRefresh(cache) {
+		return
+	}
+
 	value, err := cache.Get(context.Background(), PluginRefreshKey)
 	if err != nil {
 		logger.Log(logger.LevelError, map[string]string{"key": PluginRefreshKey},

backend/pkg/plugins/plugins_test.go

@@ -343,8 +343,38 @@ func TestHandlePluginEvents(t *testing.T) { //nolint:funlen
 	require.NoError(t, err)
 	require.NotNil(t, pluginRefresh)
 
+	// Refresh should be set to false as we cannot send the refresh request
 	pluginRefreshBool, ok := pluginRefresh.(bool)
 	require.True(t, ok)
+	require.False(t, pluginRefreshBool)
+
+	// Allow the plugins module to send the refresh request
+	err = ch.Set(context.Background(), plugins.PluginCanSendRefreshKey, true)
+	require.NoError(t, err)
+
+	// Reset the plugin list again to test the plugin handling
+	err = ch.Delete(context.Background(), plugins.PluginListKey)
+	require.NoError(t, err)
+
+	go plugins.HandlePluginEvents("", testDirPath, events, ch)
+
+	// send event
+	events <- "test"
+
+	// wait for the plugin list and refresh keys to be set
+	for {
+		_, err = ch.Get(context.Background(), plugins.PluginListKey)
+		if err == nil {
+			break
+		}
+	}
+
+	pluginRefresh, err = ch.Get(context.Background(), plugins.PluginRefreshKey)
+	require.NoError(t, err)
+
+	// Refresh should be set to true now that we can send the refresh request
+	pluginRefreshBool, ok = pluginRefresh.(bool)
+	require.True(t, ok)
 	require.True(t, pluginRefreshBool)
 
 	// check if the plugin list key is set
@@ -373,7 +403,23 @@ func TestHandlePluginReload(t *testing.T) {
 	// call HandlePluginReload
 	plugins.HandlePluginReload(ch, w)
 
+	// verify that we cannot send the refresh request yet as the
+	// canSendRefresh key is not set.
+	assert.Equal(t, "", w.Header().Get("X-RELOAD"))
+
+	err = ch.Set(context.Background(), plugins.PluginCanSendRefreshKey, false)
+	require.NoError(t, err)
+
+	// verify that we cannot send the refresh request yet as the
+	// canSendRefresh key is false.
+	plugins.HandlePluginReload(ch, w)
+	assert.Equal(t, "", w.Header().Get("X-RELOAD"))
+
+	err = ch.Set(context.Background(), plugins.PluginCanSendRefreshKey, true)
+	require.NoError(t, err)
+
 	// check if the header X-RELOAD is set to true
+	plugins.HandlePluginReload(ch, w)
 	assert.Equal(t, "reload", w.Header().Get("X-RELOAD"))
 
 	// create new recorder