Skip to content

CONSOLE-5027: Refactor public/ Directory Components to useK8sWatchResource(s) #15954

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
cajieh wants to merge 1 commit into
base: main
Choose a base branch
from
Open

CONSOLE-5027: Refactor public/ Directory Components to useK8sWatchResource(s) #15954

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions frontend/packages/console-app/locales/en/console-app.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,7 @@
"Node status": "Node status",
"This node's {{conditionDescription}}. Performance may be degraded.": "This node's {{conditionDescription}}. Performance may be degraded.",
"<0>To use host binaries, run <1>chroot /host</1></0>": "<0>To use host binaries, run <1>chroot /host</1></0>",
"Failed to load pod": "Failed to load pod",
"The debug pod failed. ": "The debug pod failed. ",
"This node has requested to join the cluster. After approving its certificate signing request the node will begin running workloads.": "This node has requested to join the cluster. After approving its certificate signing request the node will begin running workloads.",
"This node has a pending server certificate signing request. Approve the request to enable all networking functionality on this node.": "This node has a pending server certificate signing request. Approve the request to enable all networking functionality on this node.",
Expand Down
8 changes: 4 additions & 4 deletions frontend/packages/console-app/src/actions/hooks/useBindingActions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,11 @@ export const useBindingActions = (
const navigate = useNavigate();
const [commonActions] = useCommonActions(model, obj, [CommonActionCreator.Delete] as const);

const { subjectIndex, subjects = [] } = obj;
const { subjectIndex, subjects } = obj ?? {};
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 10, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Incomplete null-guard: obj is used before this line.

The obj ?? {} fallback here doesn't protect line 39 where referenceFor(obj) is called. If obj is nullish during loading states (common in the Firehose → hooks migration), that call will throw before reaching this guard.

Consider either:

  1. Adding an early return if obj is nullish (return empty actions array during loading)
  2. Updating the type signature to obj?: RoleBindingKind | ClusterRoleBindingKind and guarding before line 39
🛡️ Proposed fix: early return guard 
 export const useBindingActions = (
-  obj: RoleBindingKind | ClusterRoleBindingKind,
+  obj: RoleBindingKind | ClusterRoleBindingKind | undefined,
   filterActions?: BindingActionCreator[],
 ): Action[] => {
   const { t } = useTranslation();
+
+  // Return empty actions during loading states
+  if (!obj) {
+    return [];
+  }
+
   const [model] = useK8sModel(referenceFor(obj));

Then you can revert line 48 to the simpler form since obj is guaranteed defined:

-  const { subjectIndex, subjects } = obj ?? {};
+  const { subjectIndex, subjects } = obj;
🤖 Prompt for AI Agents 
In `@frontend/packages/console-app/src/actions/hooks/useBindingActions.ts` at line
48, In useBindingActions, referenceFor(obj) is called before the defensive
destructure "const { subjectIndex, subjects } = obj ?? {}", which throws when
obj is nullish; add an early null/undefined guard at the top of
useBindingActions (e.g., if (!obj) return [] or return the empty actions array
used for loading) so no further work runs on a missing obj, then you can safely
destructure subjectIndex/subjects from obj without the "?? {}" fallback; ensure
the guard uses the same return shape the hook expects and reference the
functions/variables referenceFor, subjectIndex, subjects, and useBindingActions
when making the change.

const subject = subjects?.[subjectIndex];
const deleteBindingSubject = useWarningModal({
title: t('public~Delete {{label}} subject?', {
label: model.kind,
label: model?.kind,
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 10, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Inconsistent model?.kind optional chaining.

Good safety improvement here, but model.kind is still used without optional chaining at lines 102, 106, 116, 120, and 128. If model can be undefined (as this change implies), those usages will throw.

Either guard consistently throughout, or add an early return when model is not yet loaded.

🛡️ Proposed fix: consistent optional chaining 
       [BindingActionCreator.DuplicateBinding]: () => ({
         id: 'duplicate-binding',
         label: t('public~Duplicate {{kindLabel}}', {
-          kindLabel: model.kind,
+          kindLabel: model?.kind,
         }),
         cta: {
           href: `${decodeURIComponent(
-            resourceObjPath(obj, model.kind),
+            resourceObjPath(obj, model?.kind),
           )}/copy?subjectIndex=${subjectIndex}`,
         },

Apply similar changes to lines 116, 120, and 128.

🤖 Prompt for AI Agents 
In `@frontend/packages/console-app/src/actions/hooks/useBindingActions.ts` at line
52, The code now uses optional chaining for model?.kind in the label but still
accesses model.kind elsewhere (at the other uses in this file), which will throw
if model is undefined; update all occurrences of direct model.kind access in
useBindingActions.ts to use optional chaining (model?.kind) or, alternatively,
add an early guard like if (!model) return (or throw) at the top of the
hook/function so the rest can safely assume model is defined—apply this
consistently to every place that references model.kind in the file.

}),
children: t('public~Are you sure you want to delete subject {{name}} of type {{kind}}?', {
name: subject?.name,
Expand Down Expand Up @@ -146,9 +146,9 @@ export const useBindingActions = (
: []),
factory.DuplicateBinding(),
factory.EditBindingSubject(),
...(subjects.length === 1 ? [commonActions.Delete] : [factory.DeleteBindingSubject()]),
...(subjects?.length === 1 ? [commonActions.Delete] : [factory.DeleteBindingSubject()]),
];
}, [memoizedFilterActions, subject?.kind, factory, subjects.length, commonActions.Delete]);
}, [memoizedFilterActions, subject?.kind, factory, subjects?.length, commonActions.Delete]);

return actions;
};
65 changes: 40 additions & 25 deletions frontend/packages/console-app/src/components/nodes/NodeTerminal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ import type { ReactNode, FC } from 'react';
import { useState, useEffect } from 'react';
import { Alert } from '@patternfly/react-core';
import { useTranslation, Trans } from 'react-i18next';
import { WatchK8sResource } from '@console/dynamic-plugin-sdk/src/extensions/console-types';
import { useK8sWatchResource } from '@console/dynamic-plugin-sdk/src/utils/k8s/hooks/useK8sWatchResource';
import { PodConnectLoader } from '@console/internal/components/pod';
import { Firehose } from '@console/internal/components/utils/firehose';
import { LoadingBox } from '@console/internal/components/utils/status-box';
import type { FirehoseResource, FirehoseResult } from '@console/internal/components/utils/types';
import { ImageStreamTagModel, NamespaceModel, PodModel } from '@console/internal/models';
import { NodeKind, PodKind, k8sCreate, k8sGet, k8sKillByName } from '@console/internal/module/k8s';
import PaneBody from '@console/shared/src/components/layout/PaneBody';
Expand All @@ -15,7 +15,9 @@ type NodeTerminalErrorProps = {
};

type NodeTerminalInnerProps = {
obj?: FirehoseResult<PodKind>;
pod: PodKind | undefined;
loaded: boolean;
loadError: any;
};

type NodeTerminalProps = {
Expand Down Expand Up @@ -124,7 +126,7 @@ const NodeTerminalError: FC<NodeTerminalErrorProps> = ({ error }) => {
);
};

const NodeTerminalInner: FC<NodeTerminalInnerProps> = ({ obj }) => {
const NodeTerminalInner: FC<NodeTerminalInnerProps> = ({ pod, loaded, loadError }) => {
const { t } = useTranslation();
const message = (
<Trans t={t} ns="console-app">
Expand All @@ -133,32 +135,44 @@ const NodeTerminalInner: FC<NodeTerminalInnerProps> = ({ obj }) => {
</p>
</Trans>
);
switch (obj?.data?.status?.phase) {

if (loadError) {
return <NodeTerminalError error={loadError.message || t('console-app~Failed to load pod')} />;
}

if (!loaded || !pod) {
return <LoadingBox />;
}

switch (pod.status?.phase) {
case 'Failed':
return (
<NodeTerminalError
error={
<>
{t('console-app~The debug pod failed. ')}
{obj?.data?.status?.containerStatuses?.[0]?.state?.terminated?.message ||
obj?.data?.status?.message}
{pod.status?.containerStatuses?.[0]?.state?.terminated?.message ||
pod.status?.message}
</>
}
/>
);
case 'Running':
return <PodConnectLoader obj={obj.data} message={message} attach />;
return <PodConnectLoader obj={pod} message={message} attach />;
default:
return <LoadingBox />;
}
};

const NodeTerminal: FC<NodeTerminalProps> = ({ obj: node }) => {
const [resources, setResources] = useState<FirehoseResource[]>([]);
const [isCreatingPod, setIsCreatingPod] = useState(true);
const [podWatchResource, setPodWatchResource] = useState<WatchK8sResource | null>(null);
const [errorMessage, setErrorMessage] = useState('');
const nodeName = node.metadata.name;
const isWindows = node.status?.nodeInfo?.operatingSystem === 'windows';

const [pod, loaded, loadError] = useK8sWatchResource<PodKind>(podWatchResource);

useEffect(() => {
let namespace;
const name = `${nodeName?.replace(/\./g, '-')}-debug`;
Expand Down Expand Up @@ -196,18 +210,17 @@ const NodeTerminal: FC<NodeTerminalProps> = ({ obj: node }) => {
await new Promise((resolve) => setTimeout(resolve, 1000));
const debugPod = await k8sCreate(PodModel, podToCreate);
if (debugPod) {
setResources([
{
isList: false,
kind: 'Pod',
name,
namespace: namespace.metadata.name,
prop: 'obj',
},
]);
setPodWatchResource({
kind: 'Pod',
name,
namespace: namespace.metadata.name,
isList: false,
});
setIsCreatingPod(false);
}
} catch (e) {
setErrorMessage(e.message);
setIsCreatingPod(false);
if (namespace) {
deleteNamespace(namespace.metadata.name);
}
Expand All @@ -221,13 +234,15 @@ const NodeTerminal: FC<NodeTerminalProps> = ({ obj: node }) => {
};
}, [nodeName, isWindows]);

return errorMessage ? (
<NodeTerminalError error={errorMessage} />
) : (
<Firehose resources={resources}>
<NodeTerminalInner />
</Firehose>
);
if (errorMessage) {
return <NodeTerminalError error={errorMessage} />;
}

if (isCreatingPod) {
return <LoadingBox />;
}

return <NodeTerminalInner pod={pod} loaded={loaded} loadError={loadError} />;
};

export default NodeTerminal;
20 changes: 17 additions & 3 deletions ...helm-plugin/src/components/details-page/resources/__tests__/HelmReleaseResources.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { ComponentProps } from 'react';
import { screen } from '@testing-library/react';
import * as ReactRouter from 'react-router-dom-v5-compat';
import * as k8sWatchHook from '@console/internal/components/utils/k8s-watch-hook';
import { renderWithProviders } from '@console/shared/src/test-utils/unit-test-utils';
import { mockHelmReleases } from '../../../__tests__/helm-release-mock-data';
import HelmReleaseResources from '../HelmReleaseResources';
Expand All @@ -10,15 +11,28 @@ jest.mock('react-router-dom-v5-compat', () => ({
useParams: jest.fn(),
}));

jest.mock('@console/internal/components/utils/k8s-watch-hook', () => ({
useK8sWatchResources: jest.fn(() => ({})),
useK8sWatchResource: jest.fn(() => [null, true, null]),
}));

const mockUseK8sWatchResources = k8sWatchHook.useK8sWatchResources as jest.Mock;

describe('HelmReleaseResources', () => {
const helmReleaseResourcesProps: ComponentProps<typeof HelmReleaseResources> = {
customData: mockHelmReleases[0],
};

it('should render the MultiListPage component', () => {
it('should render the MultiListPage component and display empty state when no resources exist', () => {
jest.spyOn(ReactRouter, 'useParams').mockReturnValue({ ns: 'test-helm' });

// mockHelmReleases[0] has an empty manifest, so no resources to watch
renderWithProviders(<HelmReleaseResources {...helmReleaseResourcesProps} />);
// MultiListPage typically renders a list/table of resources
expect(screen.getByText('No Resources found')).toBeTruthy();

// Verify useK8sWatchResources hook was called (confirms migration from Firehose to hooks)
expect(mockUseK8sWatchResources).toHaveBeenCalled();

// Verify empty state message is displayed (user-visible content)
expect(screen.getByText('No Resources found')).toBeVisible();
});
});
115 changes: 73 additions & 42 deletions frontend/public/components/RBAC/bindings.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ import { TableColumn } from '@console/internal/module/k8s';
import { GetDataViewRows, ResourceFilters } from '@console/app/src/components/data-view/types';
import { tableFilters } from '../factory/table-filters';
import { ButtonBar } from '../utils/button-bar';
import { Firehose } from '../utils/firehose';
import { getQueryArgument } from '../utils/router';
import { kindObj } from '../utils/inject';
import type { ListDropdownProps } from '../utils/list-dropdown';
Expand All @@ -57,7 +56,7 @@ import { ResourceName } from '../utils/resource-icon';
import { StatusBox, LoadingBox } from '../utils/status-box';
import { useAccessReview } from '../utils/rbac';
import { flagPending } from '../../reducers/features';
import { useK8sWatchResources } from '../utils/k8s-watch-hook';
import { useK8sWatchResource, useK8sWatchResources } from '../utils/k8s-watch-hook';

// Split each binding into one row per subject
export const flatten = (resources): BindingKind[] =>
Expand Down Expand Up @@ -185,18 +184,19 @@ const bindingType = (binding: BindingKind) => {
if (!binding) {
return undefined;
}
if (binding.roleRef.name.startsWith('system:')) {
if (binding.roleRef?.name?.startsWith('system:')) {
return 'system';
}
return binding.metadata.namespace ? 'namespace' : 'cluster';
return binding.metadata?.namespace ? 'namespace' : 'cluster';
};

const getDataViewRows: GetDataViewRows<BindingKind> = (data, columns) => {
return data.map(({ obj: binding }) => {
return data.map((row) => {
const binding = row.obj;
const rowCells = {
[tableColumnInfo[0].id]: {
cell: <BindingName binding={binding} />,
props: getNameCellProps(binding.metadata.name),
props: getNameCellProps(binding.metadata?.name),
},
[tableColumnInfo[1].id]: {
cell: <RoleLink binding={binding} />,
Expand All @@ -208,7 +208,7 @@ const getDataViewRows: GetDataViewRows<BindingKind> = (data, columns) => {
cell: binding.subject.name,
},
[tableColumnInfo[4].id]: {
cell: binding.metadata.namespace ? (
cell: binding.metadata?.namespace ? (
<ResourceLink kind="Namespace" name={binding.metadata.namespace} />
) : (
i18next.t('public~All namespaces')
Expand Down Expand Up @@ -362,9 +362,13 @@ export const RoleBindingsPage: FC<RoleBindingsPageProps> = ({

const data = useMemo(() => flatten(resources), [resources]);

const loaded = Object.values(resources)
.filter((r) => !r.loadError)
.every((r) => r.loaded);
const loaded = useMemo(
() =>
Object.values(resources)
.filter((r) => !r.loadError)
.every((r) => r.loaded),
[resources],
);

return (
<>
Expand Down Expand Up @@ -784,52 +788,79 @@ const getSubjectIndex = () => {
};

const BindingLoadingWrapper: FC<BindingLoadingWrapperProps> = (props) => {
const { obj, loaded, loadError, fixedKeys } = props;
const [, setActiveNamespace] = useActiveNamespace();

if (!loaded) {
return <LoadingBox />;
}

if (loadError) {
return <StatusBox data={obj} loaded={loaded} loadError={loadError} />;
}

if (!obj || _.isEmpty(obj)) {
return <StatusBox data={obj} loaded={loaded} loadError={loadError} />;
}

const fixed: { [key: string]: any } = {};
_.each(props.fixedKeys, (k) => (fixed[k] = _.get(props.obj.data, k)));
fixedKeys.forEach((k) => (fixed[k] = obj?.[k]));

return (
<StatusBox {...props.obj}>
<BaseEditRoleBinding
{...props}
setActiveNamespace={setActiveNamespace}
fixed={fixed}
obj={props.obj.data}
/>
</StatusBox>
<BaseEditRoleBinding
{...props}
setActiveNamespace={setActiveNamespace}
fixed={fixed}
obj={obj}
/>
Comment on lines 790 to +815
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 10, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Fix Biome error: forEach callback shouldn’t return a value.

The concise arrow returns the assignment result; Biome flags this. Use a block body.

Proposed fix 
-  fixedKeys.forEach((k) => (fixed[k] = obj?.[k]));
+  fixedKeys.forEach((k) => {
+    fixed[k] = obj?.[k];
+  });
📝 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
const BindingLoadingWrapper: FC<BindingLoadingWrapperProps> = (props) => {
const { obj, loaded, loadError, fixedKeys } = props;
const [, setActiveNamespace] = useActiveNamespace();
if (!loaded) {
return <LoadingBox />;
}
if (loadError) {
return <StatusBox data={obj} loaded={loaded} loadError={loadError} />;
}
if (!obj || _.isEmpty(obj)) {
return <StatusBox data={obj} loaded={loaded} loadError={loadError} />;
}
const fixed: { [key: string]: any } = {};
_.each(props.fixedKeys, (k) => (fixed[k] = _.get(props.obj.data, k)));
fixedKeys.forEach((k) => (fixed[k] = obj?.[k]));
return (
<StatusBox {...props.obj}>
<BaseEditRoleBinding
{...props}
setActiveNamespace={setActiveNamespace}
fixed={fixed}
obj={props.obj.data}
/>
</StatusBox>
<BaseEditRoleBinding
{...props}
setActiveNamespace={setActiveNamespace}
fixed={fixed}
obj={obj}
/>
const BindingLoadingWrapper: FC<BindingLoadingWrapperProps> = (props) => {
const { obj, loaded, loadError, fixedKeys } = props;
const [, setActiveNamespace] = useActiveNamespace();
if (!loaded) {
return <LoadingBox />;
}
if (loadError) {
return <StatusBox data={obj} loaded={loaded} loadError={loadError} />;
}
if (!obj || _.isEmpty(obj)) {
return <StatusBox data={obj} loaded={loaded} loadError={loadError} />;
}
const fixed: { [key: string]: any } = {};
fixedKeys.forEach((k) => {
fixed[k] = obj?.[k];
});
return (
<BaseEditRoleBinding
{...props}
setActiveNamespace={setActiveNamespace}
fixed={fixed}
obj={obj}
/>
🧰 Tools
🪛 Biome (2.3.13)

[error] 807-807: This callback passed to forEach() iterable method should not return a value.

Either remove this return or remove the returned value.

(lint/suspicious/useIterableCallbackReturn)

🤖 Prompt for AI Agents 
In `@frontend/public/components/RBAC/bindings.tsx` around lines 790 - 815,
BindingLoadingWrapper uses a concise arrow in fixedKeys.forEach that returns the
assignment value which Biome flags; change the callback to a block body so it
does not return a value (e.g., replace the concise arrow in the forEach on
fixedKeys with a block callback that assigns fixed[k] = obj?.[k];) to eliminate
the returned value while keeping the same behavior in BindingLoadingWrapper and
the fixed variable population.

);
};

export const EditRoleBinding: FC<EditRoleBindingProps> = ({ kind }) => {
const { t } = useTranslation();
const params = useParams();

const [obj, loaded, loadError] = useK8sWatchResource<RoleBindingKind | ClusterRoleBindingKind>({
kind,
name: params.name,
namespace: params.ns,
isList: false,
});

return (
<Firehose
resources={[{ kind, name: params.name, namespace: params.ns, isList: false, prop: 'obj' }]}
>
<BindingLoadingWrapper
fixedKeys={['kind', 'metadata', 'roleRef']}
subjectIndex={getSubjectIndex()}
titleVerbAndKind={t('public~Edit RoleBinding')}
saveButtonText={t('public~Save')}
/>
</Firehose>
<BindingLoadingWrapper
obj={obj}
loaded={loaded}
loadError={loadError}
fixedKeys={['kind', 'metadata', 'roleRef']}
subjectIndex={getSubjectIndex()}
titleVerbAndKind={t('public~Edit RoleBinding')}
saveButtonText={t('public~Save')}
/>
);
};

export const CopyRoleBinding: FC<EditRoleBindingProps> = ({ kind }) => {
const { t } = useTranslation();
const params = useParams();

const [obj, loaded, loadError] = useK8sWatchResource<RoleBindingKind | ClusterRoleBindingKind>({
kind,
name: params.name,
namespace: params.ns,
isList: false,
});

return (
<Firehose
resources={[{ kind, name: params.name, namespace: params.ns, isList: false, prop: 'obj' }]}
>
<BindingLoadingWrapper
fixedKeys={['kind']}
subjectIndex={getSubjectIndex()}
isCreate={true}
titleVerbAndKind={t('public~Duplicate RoleBinding')}
/>
</Firehose>
<BindingLoadingWrapper
obj={obj}
loaded={loaded}
loadError={loadError}
fixedKeys={['kind']}
subjectIndex={getSubjectIndex()}
isCreate={true}
titleVerbAndKind={t('public~Duplicate RoleBinding')}
/>
);
};

Expand Down Expand Up @@ -881,9 +912,9 @@ type BindingLoadingWrapperProps = {
titleVerbAndKind: string;
saveButtonText?: string;
isCreate?: boolean;
obj?: {
data: RoleBindingKind | ClusterRoleBindingKind;
};
obj?: RoleBindingKind | ClusterRoleBindingKind;
loaded: boolean;
loadError: any;
};

type EditRoleBindingProps = {
Expand Down
Loading