Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ const getUsedPortNamesForNode = (nns: V1beta1NodeNetworkState) => {
const interfaces = getInterfaces(nns);
return interfaces.reduce((acc, iface) => {
if (bridgeTypes.includes(iface?.type)) {
const ports = iface?.bridge?.port?.map((port) => port?.name);
const ports = iface?.bridge?.port?.map((port) => port?.name) || [];
acc = [...acc, ...ports];
}

Expand Down
2 changes: 1 addition & 1 deletion src/utils/resources/enactments/utils.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { V1beta1NodeNetworkConfigurationEnactment } from '@kubevirt-ui/kubevirt-api/nmstate';

export const getEnactmentStatus = (enactment: V1beta1NodeNetworkConfigurationEnactment): string =>
enactment?.status?.conditions?.find((condition) => condition.status === 'True').type;
enactment?.status?.conditions?.find((condition) => condition.status === 'True')?.type;
Comment on lines 3 to +4
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Return a concrete default status instead of allowing undefined

Line 4 can now evaluate to undefined, but getEnactmentStatus is typed as string and callers use the result as a status key/value. This can silently create undefined entries and break filtering/state grouping behavior downstream.

Suggested fix
 export const getEnactmentStatus = (enactment: V1beta1NodeNetworkConfigurationEnactment): string =>
-  enactment?.status?.conditions?.find((condition) => condition.status === 'True')?.type;
+  enactment?.status?.conditions?.find((condition) => condition.status === 'True')?.type ?? 'Pending';
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export const getEnactmentStatus = (enactment: V1beta1NodeNetworkConfigurationEnactment): string =>
enactment?.status?.conditions?.find((condition) => condition.status === 'True').type;
enactment?.status?.conditions?.find((condition) => condition.status === 'True')?.type;
export const getEnactmentStatus = (enactment: V1beta1NodeNetworkConfigurationEnactment): string =>
enactment?.status?.conditions?.find((condition) => condition.status === 'True')?.type ?? 'Pending';
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/utils/resources/enactments/utils.ts` around lines 3 - 4,
getEnactmentStatus currently can return undefined even though it's typed as
string; update the function (getEnactmentStatus with parameter
V1beta1NodeNetworkConfigurationEnactment) to return a concrete default status
when no condition with status === 'True' is found (e.g., return 'Unknown' or
'Pending') so callers always receive a string and downstream grouping/filtering
won't break.


export const categorizeEnactments = (enactments: V1beta1NodeNetworkConfigurationEnactment[]) => {
return (enactments || []).reduce(
Expand Down
2 changes: 1 addition & 1 deletion src/utils/resources/policies/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export const isPolicyAppliedInNode = (
export const filterPolicyAppliedNodes = (
nodes: IoK8sApiCoreV1Node[],
policy: V1NodeNetworkConfigurationPolicy,
) => nodes.filter((node) => isPolicyAppliedInNode(policy, node));
) => (nodes || []).filter((node) => isPolicyAppliedInNode(policy, node));

export const getPolicyInterfaces = (
policy: V1NodeNetworkConfigurationPolicy,
Expand Down
10 changes: 6 additions & 4 deletions src/views/nodenetworkconfiguration/Topology.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React, { FC, useEffect, useMemo, useState } from 'react';
import { useHistory } from 'react-router';
import { useNavigate } from 'react-router-dom-v5-compat';
import { NodeModelGroupVersionKind } from 'src/console-models/NodeModel';
import './components/TopologySidebar/TopologySidebar.scss';
import { IoK8sApiCoreV1Node } from '@kubevirt-ui/kubevirt-api/kubernetes/models';
Expand Down Expand Up @@ -51,7 +51,7 @@ const Topology: FC = () => {
const { t } = useNMStateTranslation();
const [visualization, setVisualization] = useState<Visualization>(null);
const [selectedNodeFilters, setSelectedNodeFilters] = useState<string[]>([]);
const history = useHistory();
const navigate = useNavigate();

const queryParams = useQueryParams();

Expand Down Expand Up @@ -83,7 +83,9 @@ const Topology: FC = () => {
[states],
);

useSignalEffect(() => (creatingPolicySignal.value = null));
useSignalEffect(() => {
creatingPolicySignal.value = null;
});

useEffect(() => {
if (!statesLoaded || statesError || isEmpty(states)) return;
Expand All @@ -107,7 +109,7 @@ const Topology: FC = () => {

newVisualization.addEventListener(SELECTION_EVENT, (id) => {
const newParams = new URLSearchParams({ [SELECTED_ID_QUERY_PARAM]: id });
history.push({ search: newParams.toString() });
navigate({ search: newParams.toString() });
});

newVisualization.addEventListener(NODE_POSITIONING_EVENT, () =>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React, { FC, useEffect, useRef, useState } from 'react';
import { useHistory } from 'react-router';
import { useNavigate } from 'react-router-dom-v5-compat';
import NodeNetworkConfigurationPolicyModel from 'src/console-models/NodeNetworkConfigurationPolicyModel';
import { useImmer } from 'use-immer';

Expand Down Expand Up @@ -39,7 +39,7 @@ const CreatePolicyDrawer: FC<CreatePolicyDrawerProps> = ({
const [createAnotherPolicy] = createAnotherPolicyState;
const completed = useRef(false);
const currentStepId = useRef<string | number>('policy-wizard-basicinfo');
const history = useHistory();
const navigate = useNavigate();

creatingPolicySignal.value = policy;

Expand Down Expand Up @@ -75,7 +75,7 @@ const CreatePolicyDrawer: FC<CreatePolicyDrawerProps> = ({

if (createAnotherPolicy) resetPolicyWizard();

history.push(
navigate(
createAnotherPolicy
? `node-network-configuration?createPolicy=true&physicalNetworkName=${networkName}`
: getResourceUrl({ model: NodeNetworkConfigurationPolicyModel, resource: createdPolicy }),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React, { FC, ReactNode, useState } from 'react';
import { useNMStateTranslation } from 'src/utils/hooks/useNMStateTranslation';
import { useHistory } from 'react-router';
import { useNavigate } from 'react-router-dom-v5-compat';
import {
Drawer,
DrawerContent,
Expand All @@ -25,15 +25,15 @@ type Props = {
const TopologyDrawer: FC<Props> = ({ states, children }) => {
const { t } = useNMStateTranslation();
const location = useLocation();
const history = useHistory();
const navigate = useNavigate();
const queryParams = new URLSearchParams(location.search);

const [successMessage, setSuccessMessage] = useState<string>('');

const closeDrawer = () => {
const updatedParams = new URLSearchParams(location.search);
updatedParams.delete(CREATE_POLICY_QUERY_PARAM);
history.push({ search: updatedParams.toString() });
navigate({ search: updatedParams.toString() });
};

const isDrawerOpen = queryParams.get(CREATE_POLICY_QUERY_PARAM) === 'true';
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import React, { Dispatch, FC, SetStateAction } from 'react';
import { useHistory } from 'react-router';
import { useNavigate } from 'react-router-dom-v5-compat';
import NodeNetworkConfigurationPolicyModel from 'src/console-models/NodeNetworkConfigurationPolicyModel';
import { NodeNetworkConfigurationPolicyModelRef, NodeNetworkStateModelRef } from '@models';
Expand Down Expand Up @@ -32,7 +31,6 @@ const TopologyToolbar: FC<TopologyToolbarProps> = ({
}) => {
const { t } = useNMStateTranslation();
const navigate = useNavigate();
const history = useHistory();
const createItems = {
form: t('From Form'),
yaml: t('With YAML'),
Expand All @@ -46,8 +44,8 @@ const TopologyToolbar: FC<TopologyToolbarProps> = ({
const newParams = new URLSearchParams({ [CREATE_POLICY_QUERY_PARAM]: 'true' });

return type === 'form'
? history.push({ search: newParams.toString() })
: history.push(`${baseURL}~new`);
? navigate({ search: newParams.toString() })
: navigate(`${baseURL}~new`);
};

return (
Expand Down
6 changes: 3 additions & 3 deletions src/views/nodenetworkconfiguration/utils/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ export const transformDataToTopologyModel = (

const childNodes = createNodes(
nnsName,
[...nodeState.status.currentState.interfaces, ...creatingPolicyToAdd],
[...(nodeState.status?.currentState?.interfaces || []), ...creatingPolicyToAdd],
nnceConfigredInterfaces,
);

Expand Down Expand Up @@ -221,8 +221,8 @@ const getCorrelatedEnactment = (
availableEnhancments: V1beta1NodeNetworkConfigurationEnactment[],
nnsName: string,
): V1beta1NodeNetworkConfigurationEnactment => {
return availableEnhancments.find((nnce) =>
nnce.metadata.ownerReferences.some((ref) => ref.name === nnsName),
return availableEnhancments?.find((nnce) =>
nnce.metadata?.ownerReferences?.some((ref) => ref.name === nnsName),
Comment on lines +224 to +225
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Function signature and return expression =="
rg -n -C2 'const getCorrelatedEnactment|availableEnhancments\?\.\s*find|V1beta1NodeNetworkConfigurationEnactment' src/views/nodenetworkconfiguration/utils/utils.ts

echo
echo "== TypeScript strictness flags in tsconfig files =="
fd 'tsconfig.*json' -t f | while read -r f; do
  echo "-- $f --"
  rg -n '"strict"\s*:|"strictNullChecks"\s*:' "$f" || true
done

Repository: openshift/nmstate-console-plugin

Length of output: 1204


🏁 Script executed:

# Search for all usages of getCorrelatedEnactment
rg -n 'getCorrelatedEnactment' src/views/nodenetworkconfiguration/utils/utils.ts

# Also check if it's exported and used elsewhere
rg -r 'getCorrelatedEnactment' src/ --type ts --type tsx

Repository: openshift/nmstate-console-plugin

Length of output: 239


🏁 Script executed:

# Get context around the function call at line 179 to see how the result is used
rg -n -A 10 -B 5 'const enhancment = getCorrelatedEnactment' src/views/nodenetworkconfiguration/utils/utils.ts

Repository: openshift/nmstate-console-plugin

Length of output: 702


🏁 Script executed:

# Get the full implementation of getNNCEConfiguredInterfaces
rg -n -A 20 'const getNNCEConfiguredInterfaces' src/views/nodenetworkconfiguration/utils/utils.ts

Repository: openshift/nmstate-console-plugin

Length of output: 793


Type contract no longer matches actual return value.

getCorrelatedEnactment can return undefined when .find() yields no match, but the function signature declares a non-optional return type. The downstream caller getNNCEConfiguredInterfaces defends against this with an isEmpty check, but this masks a type contract mismatch that weakens maintainability. Align both the parameter and return types with the null-safe behavior:

Proposed fix
const getCorrelatedEnactment = (
-  availableEnhancments: V1beta1NodeNetworkConfigurationEnactment[],
+  availableEnhancments: V1beta1NodeNetworkConfigurationEnactment[] | undefined,
   nnsName: string,
-): V1beta1NodeNetworkConfigurationEnactment => {
+): V1beta1NodeNetworkConfigurationEnactment | undefined => {
   return availableEnhancments?.find((nnce) =>
     nnce.metadata?.ownerReferences?.some((ref) => ref.name === nnsName),
   );
 };
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/views/nodenetworkconfiguration/utils/utils.ts` around lines 224 - 225,
The function getCorrelatedEnactment currently declares a non-optional return but
uses .find() and can return undefined; change its signature to return the NNCE
type as optional (e.g., NNCE | undefined) and update any parameter types that
assume a non-null value accordingly, and then update callers such as
getNNCEConfiguredInterfaces to accept the optional return (adjust its parameter
type to accept undefined) so the type contract matches the runtime null-safe
behavior.

);
};

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React, { FC } from 'react';
import { useHistory } from 'react-router-dom';
import { useNavigate } from 'react-router-dom-v5-compat';

import { useNMStateTranslation } from '@utils/hooks/useNMStateTranslation';
import {
Expand All @@ -16,7 +16,7 @@ import { NODE_NETWORK_CONFIGURATION_WIZARD_PATH } from '../utils/constants';

const PhysicalNetworksEmptyState: FC = () => {
const { t } = useNMStateTranslation();
const history = useHistory();
const navigate = useNavigate();

return (
<EmptyState
Expand All @@ -32,7 +32,7 @@ const PhysicalNetworksEmptyState: FC = () => {
</EmptyStateBody>
<EmptyStateFooter>
<EmptyStateActions>
<Button onClick={() => history.push(NODE_NETWORK_CONFIGURATION_WIZARD_PATH)}>
<Button onClick={() => navigate(NODE_NETWORK_CONFIGURATION_WIZARD_PATH)}>
{t('Create network')}
</Button>
</EmptyStateActions>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React, { FC } from 'react';
import { useHistory } from 'react-router-dom';
import { useNavigate } from 'react-router-dom-v5-compat';

import PaneHeading from '@utils/components/PaneHeading/PaneHeading';
import { useNMStateTranslation } from '@utils/hooks/useNMStateTranslation';
Expand All @@ -13,7 +13,7 @@ type PhysicalNetworksPageHeaderProps = {

const PhysicalNetworksPageHeader: FC<PhysicalNetworksPageHeaderProps> = ({ showCreateButton }) => {
const { t } = useNMStateTranslation();
const history = useHistory();
const navigate = useNavigate();

return (
<PaneHeading>
Expand All @@ -22,7 +22,7 @@ const PhysicalNetworksPageHeader: FC<PhysicalNetworksPageHeaderProps> = ({ showC
</Title>
{showCreateButton && (
<Button
onClick={() => history.push(NODE_NETWORK_CONFIGURATION_WIZARD_PATH)}
onClick={() => navigate(NODE_NETWORK_CONFIGURATION_WIZARD_PATH)}
variant="primary"
>
{t('Create network')}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React, { Dispatch, FC, SetStateAction, useState } from 'react';
import { useHistory } from 'react-router-dom';
import { useNavigate } from 'react-router-dom-v5-compat';

import { Button } from '@patternfly/react-core';
import { ActionsColumn, ExpandableRowContent, Tbody, Td, Tr } from '@patternfly/react-table';
Expand Down Expand Up @@ -31,7 +31,7 @@ const PhysicalNetworkRow: FC<PhysicalNetworksRowProps> = ({
}) => {
const { t } = useNMStateTranslation();
const [isNodesModalOpen, setIsNodesModalOpen] = useState(false);
const history = useHistory();
const navigate = useNavigate();
const isNetworkExpanded = (physicalNetwork: PhysicalNetwork) =>
expandedNetworks.includes(physicalNetwork.name);

Expand Down Expand Up @@ -59,7 +59,7 @@ const PhysicalNetworkRow: FC<PhysicalNetworksRowProps> = ({
</Button>
</Td>
<Td id="actionColumn" isActionCell key="actionColumn">
<ActionsColumn items={getRowActions(t, history, network.name)} />
<ActionsColumn items={getRowActions(t, navigate, network.name)} />
</Td>
</Tr>
{!isEmpty(network.nncps) ? (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@ import { PhysicalNetworksRowActions } from './types';

export const getRowActions = (
t: TFunction,
history,
navigate: (path: string) => void,
networkName: string,
): PhysicalNetworksRowActions => [
{
onClick: () =>
history.push(
navigate(
`${NODE_NETWORK_CONFIGURATION_WIZARD_PATH}&${PHYSICAL_NETWORK_NAME_PARAM_KEY}=${encodeURIComponent(
networkName,
)}`,
Expand All @@ -29,7 +29,7 @@ export const getRowActions = (
},
{
onClick: () =>
history.push(
navigate(
`k8s/cluster/virtualmachine-networks/~new?${PHYSICAL_NETWORK_NAME_PARAM_KEY}=${encodeURIComponent(
networkName,
)}`,
Expand Down
6 changes: 3 additions & 3 deletions src/views/policies/actions/PolicyActions.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React, { FC, useState } from 'react';
import { useHistory } from 'react-router';
import { useNavigate } from 'react-router-dom-v5-compat';
import NodeNetworkConfigurationPolicyModel from 'src/console-models/NodeNetworkConfigurationPolicyModel';
import { asAccessReview, getResourceUrl } from 'src/utils/helpers';
import { useNMStateTranslation } from 'src/utils/hooks/useNMStateTranslation';
Expand All @@ -22,7 +22,7 @@ type PolicyActionsProps = {
};

const PolicyActions: FC<PolicyActionsProps> = ({ policy, isKebabToggle }) => {
const history = useHistory();
const navigate = useNavigate();
const formSupported = isPolicySupported(policy);
const { t } = useNMStateTranslation();
const [isEditModalOpen, setIsEditModalOpen] = useState(false);
Expand All @@ -46,7 +46,7 @@ const PolicyActions: FC<PolicyActionsProps> = ({ policy, isKebabToggle }) => {
resource: policy,
});

history.push(`${policyDetailPage}/yaml`);
navigate(`${policyDetailPage}/yaml`);
};

const onDeleteModalToggle = () => {
Expand Down
6 changes: 3 additions & 3 deletions src/views/policies/components/DeleteModal.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React, { FC, MouseEventHandler, useState } from 'react';
import { Trans } from 'react-i18next';
import { useHistory } from 'react-router';
import { useNavigate } from 'react-router-dom-v5-compat';
import NodeNetworkConfigurationPolicyModel from 'src/console-models/NodeNetworkConfigurationPolicyModel';
import { useNMStateTranslation } from 'src/utils/hooks/useNMStateTranslation';

Expand Down Expand Up @@ -32,7 +32,7 @@ type DeleteModalProps = {

const DeleteModal: FC<DeleteModalProps> = ({ closeModal, isOpen, policy }) => {
const { t } = useNMStateTranslation();
const history = useHistory();
const navigate = useNavigate();
const [error, setError] = useState(undefined);
const [loading, setLoading] = useState(false);
const [inputValue, setInputValue] = useState('');
Expand All @@ -47,7 +47,7 @@ const DeleteModal: FC<DeleteModalProps> = ({ closeModal, isOpen, policy }) => {
ns: policy?.metadata?.namespace,
name: policy?.metadata?.name,
})
.then(() => history.push(getResourceUrl({ model: NodeNetworkConfigurationPolicyModel })))
.then(() => navigate(getResourceUrl({ model: NodeNetworkConfigurationPolicyModel })))
.catch(setError)
.finally(() => {
setError(undefined);
Expand Down
8 changes: 4 additions & 4 deletions src/views/policies/list/components/CreatePolicyButtons.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React, { FC } from 'react';
import { useHistory } from 'react-router-dom';
import { useNavigate } from 'react-router-dom-v5-compat';
import NodeNetworkConfigurationPolicyModel, {
NodeNetworkConfigurationPolicyModelRef,
} from 'src/console-models/NodeNetworkConfigurationPolicyModel';
Expand All @@ -12,7 +12,7 @@ import { logNMStateEvent } from '@utils/telemetry/telemetry';

const CreatePolicyButtons: FC = ({ children }) => {
const { t } = useNMStateTranslation();
const history = useHistory();
const navigate = useNavigate();

const createItems = {
form: t('From Form'),
Expand All @@ -29,8 +29,8 @@ const CreatePolicyButtons: FC = ({ children }) => {
}

return type === 'form'
? history.push('/node-network-configuration?createPolicy=true')
: history.push(`${baseURL}~new`);
? navigate('/node-network-configuration?createPolicy=true')
: navigate(`${baseURL}~new`);
};

return (
Expand Down
10 changes: 5 additions & 5 deletions src/views/policies/new/NewPolicy.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React, { FC, MouseEventHandler, useState } from 'react';
import { useHistory } from 'react-router';
import { useNavigate } from 'react-router-dom-v5-compat';
import NodeNetworkConfigurationPolicyModel from 'src/console-models/NodeNetworkConfigurationPolicyModel';
import { getResourceUrl } from 'src/utils/helpers';
import { useNMStateTranslation } from 'src/utils/hooks/useNMStateTranslation';
Expand Down Expand Up @@ -58,7 +58,7 @@ const NewPolicy: FC = () => {
const [policy, setPolicy] = useImmer(initialPolicy);
const [loading, setLoading] = useState(false);
const [error, setError] = useState<Error>(undefined);
const history = useHistory();
const navigate = useNavigate();

const onFormSubmit: MouseEventHandler<HTMLButtonElement> = (event) => {
event.preventDefault();
Expand All @@ -72,7 +72,7 @@ const NewPolicy: FC = () => {
data: policy,
})
.then(() =>
history.push(
navigate(
getResourceUrl({ model: NodeNetworkConfigurationPolicyModel, resource: policy }),
),
)
Expand All @@ -82,7 +82,7 @@ const NewPolicy: FC = () => {

const goToYAMLEditor = () => {
const baseUrl = getResourceUrl({ model: NodeNetworkConfigurationPolicyModel });
history.push(`${baseUrl}~new`);
navigate(`${baseUrl}~new`);
};

return (
Expand Down Expand Up @@ -124,7 +124,7 @@ const NewPolicy: FC = () => {
{t('Create')}
</Button>

<Button onClick={history.goBack} variant={ButtonVariant.secondary}>
<Button onClick={() => navigate(-1)} variant={ButtonVariant.secondary}>
{t('Cancel')}
</Button>
</ActionGroup>
Expand Down
Loading