frontend: Fix TypeError in cluster config comparison

- Resolved TypeError: "Cannot delete property 'teststateless' of #<Object>" occurring in processClusterComparison function - Implemented isClusterConfigDifferent function to compare configs without modifying objects, preventing property deletion errors - Added mergeClusterConfigs function to properly handle stateless clusters and useToken property - Updated Layout component to use new comparison and merging logic - Prevented unnecessary reloads due to useToken changes while maintaining correct state in Redux store This fix addresses the error occurring in Layout.tsx and index.ts when processing cluster configurations, ensuring proper handling of stateless clusters without attempting to delete properties from immutable objects. Co-authored-by: Kautilya Tripathi <[email protected]> Signed-off-by: Joaquim Rocha <[email protected]> Signed-off-by: Kautilya Tripathi <[email protected]>
headlamp-k8s · Sep 7, 2024 · 8b10bb1 · 8b10bb1
1 parent d723ddd
commit 8b10bb1
Show file tree

Hide file tree

Showing 2 changed files with 66 additions and 30 deletions.
diff --git a/frontend/src/components/App/Layout.tsx b/frontend/src/components/App/Layout.tsx
@@ -56,6 +56,46 @@ function ClusterNotFoundPopup() {
 const Div = styled('div')``;
 const Main = styled('main')``;
 
+/**
+ * Merges the new cluster config with the current cluster config.
+ * It also preserves the useToken property.
+ * @param newConfig - The new cluster config.
+ * @param currentClusters - The current cluster config.
+ * @param statelessClusters - The stateless cluster config.
+ * @returns The merged cluster config.
+ */
+function mergeClusterConfigs(
+  newConfig: Omit<ConfigState, 'clusters'> & { clusters: Record<string, any> },
+  currentClusters: Record<string, any>,
+  statelessClusters: Record<string, any> | null
+): ConfigState {
+  const mergedClusters = { ...newConfig.clusters };
+
+  // Merge stateless clusters
+  if (statelessClusters) {
+    Object.entries(statelessClusters).forEach(([key, cluster]) => {
+      if (!mergedClusters[key]) {
+        mergedClusters[key] = cluster;
+      }
+    });
+  }
+
+  // Preserve useToken property
+  Object.entries(mergedClusters).forEach(([key, cluster]) => {
+    if (currentClusters[key]?.useToken !== undefined) {
+      mergedClusters[key] = {
+        ...cluster,
+        useToken: currentClusters[key].useToken,
+      };
+    }
+  });
+
+  return {
+    ...newConfig,
+    clusters: mergedClusters,
+  } as ConfigState;
+}
+
 export default function Layout({}: LayoutProps) {
   const arePluginsLoaded = useTypedSelector(state => state.plugins.loaded);
   const dispatch = useDispatch();
@@ -92,6 +132,7 @@ export default function Layout({}: LayoutProps) {
    */
   const fetchConfig = () => {
     const clusters = store.getState().config.clusters;
+    const statelessClusters = store.getState().config.statelessClusters;
 
     request('/config', {}, false, false)
       .then(config => {
@@ -103,9 +144,19 @@ export default function Layout({}: LayoutProps) {
           clustersToConfig[cluster.name] = cluster;
         });
 
-        if (isClusterConfigDifferent(clusters, clustersToConfig)) {
-          const configToStore = { ...config, clusters: clustersToConfig };
+        const configToStore = { ...config, clusters: clustersToConfig };
+
+        if (clusters === null) {
           dispatch(setConfig(configToStore));
+        } else {
+          // Check if the config is different
+          const configDifferent = isClusterConfigDifferent(clusters, clustersToConfig);
+
+          if (configDifferent) {
+            // Merge the new config with the current config
+            const mergedConfig = mergeClusterConfigs(configToStore, clusters, statelessClusters);
+            dispatch(setConfig(mergedConfig));
+          }
         }
 
         /**

diff --git a/frontend/src/stateless/index.ts b/frontend/src/stateless/index.ts
@@ -319,40 +319,26 @@ function generateSecureToken(length = 16): string {
 export function isClusterConfigDifferent(
   currentConfig: ConfigState['clusters'],
   newConfig: ConfigState['clusters']
-) {
+): boolean {
   if (!currentConfig || !newConfig) {
-    // If one of the configs is null but the other is not, we want to update the config
-    return !currentConfig !== !newConfig;
+    return true; // Config is different if either is null/undefined
   }
 
-  if (Object.keys(newConfig).length !== Object.keys(currentConfig).length) {
-    return true;
-  }
-
-  Object.keys(newConfig).forEach(key => {
-    if (!currentConfig[key]) {
-      return true;
-    }
+  const currentKeys = Object.keys(currentConfig);
+  const newKeys = Object.keys(newConfig);
 
-    let clusterToCompare = currentConfig[key];
-
-    if (clusterToCompare.useToken !== undefined) {
-      clusterToCompare = _.cloneDeep(currentConfig[key]);
-      delete clusterToCompare.useToken;
-    }
-
-    let newCluster = newConfig[key];
-    if (newCluster.useToken !== undefined) {
-      newCluster = _.cloneDeep(newConfig[key]);
-      delete newCluster.useToken;
-    }
+  if (currentKeys.length !== newKeys.length) {
+    return true; // Different number of clusters
+  }
 
-    if (!_.isEqual(newCluster, clusterToCompare)) {
-      return true;
+  return currentKeys.some(key => {
+    if (!newConfig[key]) {
+      return true; // Cluster in current config doesn't exist in new config
     }
+    const currentCluster = _.omit(currentConfig[key], ['useToken']);
+    const newCluster = _.omit(newConfig[key], ['useToken']);
+    return !_.isEqual(currentCluster, newCluster);
   });
-
-  return false;
 }
 
 /**
@@ -387,7 +373,6 @@ export async function fetchStatelessClusterKubeConfigs(dispatch: any) {
       });
 
       const configToStore = {
-        ...config,
         statelessClusters: clustersToConfig,
       };
       if (statelessClusters === null) {