Skip to content

Commit

Permalink
frontend: Fix TypeError in cluster config comparison
Browse files Browse the repository at this point in the history
- 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]>
  • Loading branch information
knrt10 committed Sep 11, 2024
1 parent a40c319 commit 5fc4a52
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 46 deletions.
65 changes: 54 additions & 11 deletions frontend/src/components/App/Layout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { setConfig } from '../../redux/configSlice';
import { ConfigState } from '../../redux/configSlice';
import { useTypedSelector } from '../../redux/reducers/reducers';
import store from '../../redux/stores/store';
import { fetchStatelessClusterKubeConfigs, processClusterComparison } from '../../stateless/';
import { fetchStatelessClusterKubeConfigs, isEqualClusterConfigs } from '../../stateless/';
import ActionsNotifier from '../common/ActionsNotifier';
import AlertNotification from '../common/AlertNotification';
import Sidebar, { NavigationTabs } from '../Sidebar';
Expand Down Expand Up @@ -56,6 +56,44 @@ function ClusterNotFoundPopup() {
const Div = styled('div')``;
const Main = styled('main')``;

/**
* Merges the new cluster with the current cluster.
* Stateless clusters are merged with the current cluster.
* 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(
newClusters: Record<string, any>,
currentClusters: Record<string, any>,
statelessClusters: Record<string, any> | null
): Record<string, any> {
const mergedClusters = { ...newClusters };

// 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 mergedClusters;
}

export default function Layout({}: LayoutProps) {
const arePluginsLoaded = useTypedSelector(state => state.plugins.loaded);
const dispatch = useDispatch();
Expand Down Expand Up @@ -109,17 +147,22 @@ export default function Layout({}: LayoutProps) {
if (clusters === null) {
dispatch(setConfig(configToStore));
} else {
const isConfigDifferent = processClusterComparison(clusters, clustersToConfig, false);

if (
isConfigDifferent ||
Object.keys(clustersToConfig).length !== Object.keys(clusters).length
) {
if (statelessClusters !== null) {
processClusterComparison(clusters, statelessClusters, true);
}
// Check if the config is different
const configDifferent = isEqualClusterConfigs(clusters, clustersToConfig);

dispatch(setConfig(configToStore));
if (configDifferent) {
// Merge the new config with the current config
const mergedClusters = mergeClusterConfigs(
configToStore.clusters,
clusters,
statelessClusters
);
dispatch(
setConfig({
...configToStore,
clusters: mergedClusters,
})
);
}
}

Expand Down
55 changes: 20 additions & 35 deletions frontend/src/stateless/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -314,47 +314,31 @@ function generateSecureToken(length = 16): string {
* Compares the cluster config from the backend and the redux store
* @param clusters
* @param clustersToConfig
* @param isStateless
* @returns true if the present stored config is different from the fetched one.
*/
export function processClusterComparison(
clusters: ConfigState['clusters'],
clustersToConfig: ConfigState['clusters'],
isStateless: boolean
) {
if (!clusters) {
// If no clusters, then add all clusters
return false;
export function isEqualClusterConfigs(
currentConfig: ConfigState['clusters'],
newConfig: ConfigState['clusters']
): boolean {
if (!currentConfig || !newConfig) {
return true; // Config is different if either is null/undefined
}

if (!clustersToConfig) {
// If no clustersToConfig, then delete all clusters
return true;
}

let isConfigDifferent = false;

Object.keys(clustersToConfig).forEach(key => {
if (!!clusters[key]) {
let clusterToCompare = clusters[key];
const currentKeys = Object.keys(currentConfig);
const newKeys = Object.keys(newConfig);

if (clusterToCompare.useToken !== undefined) {
clusterToCompare = _.cloneDeep(clusters[key]);
delete clusterToCompare.useToken;
}
if (currentKeys.length !== newKeys.length) {
return true; // Different number of clusters
}

if (isStateless) {
if (_.isEqual(clustersToConfig[key], clusterToCompare)) {
delete clusters[key];
}
} else {
isConfigDifferent =
isConfigDifferent || !_.isEqual(clustersToConfig[key], clusterToCompare);
}
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 isConfigDifferent;
}

/**
Expand Down Expand Up @@ -389,7 +373,6 @@ export async function fetchStatelessClusterKubeConfigs(dispatch: any) {
});

const configToStore = {
...config,
statelessClusters: clustersToConfig,
};
if (statelessClusters === null) {
Expand Down Expand Up @@ -590,10 +573,12 @@ const exportFunctions = {
getStatelessClusterKubeConfigs,
findKubeconfigByClusterName,
getUserIdFromLocalStorage,
processClusterComparison,
isEqualClusterConfigs,
fetchStatelessClusterKubeConfigs,
deleteClusterKubeconfig,
updateStatelessClusterKubeconfig,
// @deprecated - use isEqualClusterConfigs instead
processClusterComparison: isEqualClusterConfigs,
};

export default exportFunctions;

0 comments on commit 5fc4a52

Please sign in to comment.