Skip to content

CONSOLE-5030 Refactor remaining firehose impacted components #16039

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
+289 −268
Open

CONSOLE-5030 Refactor remaining firehose impacted components #16039

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
61 changes: 39 additions & 22 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
@@ -1,11 +1,10 @@
import type { ReactNode, FC } from 'react';
import { useState, useEffect } from 'react';
import { useState, useEffect, useMemo } from 'react';
import { Alert } from '@patternfly/react-core';
import { useTranslation, Trans } from 'react-i18next';
import { PodConnectLoader } from '@console/internal/components/pod';
import { Firehose } from '@console/internal/components/utils/firehose';
import { useK8sWatchResource } from '@console/internal/components/utils/k8s-watch-hook';
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 type { NodeKind, PodKind } from '@console/internal/module/k8s';
import { k8sCreate, k8sGet, k8sKillByName } from '@console/internal/module/k8s';
Expand All @@ -16,7 +15,9 @@ type NodeTerminalErrorProps = {
};

type NodeTerminalInnerProps = {
obj?: FirehoseResult<PodKind>;
pod?: PodKind;
loaded: boolean;
loadError?: Error;
Copy link
Contributor Author

@cajieh cajieh Feb 24, 2026

Choose a reason for hiding this comment

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

I'd prefer using the Error type here to avoid verbose type guard for unknown when accessing .message in the loadError.

 loadError && typeof loadError === 'object' && 'message' in loadError
    ? String(loadError.message)
    : undefined

Or casting it: error={(loadError as Error).message}.

};

type NodeTerminalProps = {
Expand Down Expand Up @@ -125,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 @@ -134,32 +135,57 @@ const NodeTerminalInner: FC<NodeTerminalInnerProps> = ({ obj }) => {
</p>
</Trans>
);
switch (obj?.data?.status?.phase) {

if (loadError) {
return <NodeTerminalError error={loadError.message} />;
}

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 [podName, setPodName] = useState<string>('');
const [podNamespace, setPodNamespace] = useState<string>('');
const [errorMessage, setErrorMessage] = useState('');
const nodeName = node.metadata.name;
const isWindows = node.status?.nodeInfo?.operatingSystem === 'windows';

const watchResource = useMemo(
() =>
podName && podNamespace
? {
isList: false,
kind: 'Pod',
name: podName,
namespace: podNamespace,
}
: null,
[podName, podNamespace],
);

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

useEffect(() => {
let namespace;
const name = `${nodeName?.replace(/\./g, '-')}-debug`;
Expand Down Expand Up @@ -197,15 +223,8 @@ 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',
},
]);
setPodName(name);
setPodNamespace(namespace.metadata.name);
}
} catch (e) {
setErrorMessage(e.message);
Expand All @@ -225,9 +244,7 @@ const NodeTerminal: FC<NodeTerminalProps> = ({ obj: node }) => {
return errorMessage ? (
<NodeTerminalError error={errorMessage} />
) : (
<Firehose resources={resources}>
<NodeTerminalInner />
</Firehose>
<NodeTerminalInner pod={pod} loaded={loaded} loadError={loadError} />
);
};

Expand Down
41 changes: 20 additions & 21 deletions frontend/packages/container-security/src/components/image-manifest-vuln.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,15 @@ import {
ListPage,
} from '@console/internal/components/factory';
import { ContainerLink } from '@console/internal/components/pod';
import type { FirehoseResult } from '@console/internal/components/utils';
import {
ResourceLink,
navFactory,
SectionHeading,
ResourceSummary,
DetailsItem,
Firehose,
Loading,
} from '@console/internal/components/utils';
import { useK8sWatchResource } from '@console/internal/components/utils/k8s-watch-hook';
import type { PodKind, ContainerStatus } from '@console/internal/module/k8s';
import { referenceForModel } from '@console/internal/module/k8s';
import { EmptyStateResourceBadge, GreenCheckCircleIcon } from '@console/shared/';
Expand Down Expand Up @@ -356,7 +355,7 @@ export const ContainerVulnerabilities: FC<ContainerVulnerabilitiesProps> = (prop
{props.pod.spec.containers.find((c) => c.name === status.name).image}
</Td>
<Td>
{props.loaded ? (
{props.imageManifestVuln.loaded ? (
withVuln(
vulnFor(status),
(vuln) => (
Expand Down Expand Up @@ -400,30 +399,30 @@ export const ContainerVulnerabilities: FC<ContainerVulnerabilitiesProps> = (prop

export const ImageManifestVulnPodTab: FC<ImageManifestVulnPodTabProps> = (props) => {
const params = useParams();
const [imageManifestVuln, loaded, loadError] = useK8sWatchResource<ImageManifestVuln[]>({
isList: true,
kind: referenceForModel(ImageManifestVulnModel),
namespace: params.ns,
selector: {
matchLabels: { [podKey(props.obj)]: 'true' },
},
});

return (
<Firehose
resources={[
{
isList: true,
kind: referenceForModel(ImageManifestVulnModel),
namespace: params.ns,
selector: {
matchLabels: { [podKey(props.obj)]: 'true' },
},
prop: 'imageManifestVuln',
},
]}
>
{/* FIXME(alecmerdler): Hack because `Firehose` injects props without TypeScript knowing about it */}
<ContainerVulnerabilities pod={props.obj} {...(props as any)} />
</Firehose>
<ContainerVulnerabilities
pod={props.obj}
imageManifestVuln={{ data: imageManifestVuln, loaded, loadError }}
/>
);
};

export type ContainerVulnerabilitiesProps = {
loaded: boolean;
pod: PodKind;
imageManifestVuln: FirehoseResult<ImageManifestVuln[]>;
imageManifestVuln: {
data: ImageManifestVuln[];
loaded: boolean;
loadError?: unknown;
};
};

export type ImageManifestVulnPageProps = {
Expand Down
43 changes: 16 additions & 27 deletions ...tend/packages/operator-lifecycle-manager/src/components/__tests__/catalog-source.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { cloneElement } from 'react';
import { screen } from '@testing-library/react';
import * as _ from 'lodash';
import * as Router from 'react-router-dom-v5-compat';
import { DetailsPage } from '@console/internal/components/factory';
import { Firehose } from '@console/internal/components/utils';
import { useK8sWatchResource } from '@console/internal/components/utils/k8s-watch-hook';
import {
useK8sWatchResource,
useK8sWatchResources,
} from '@console/internal/components/utils/k8s-watch-hook';
import { referenceForModel } from '@console/internal/module/k8s';
import { renderWithProviders } from '@console/shared/src/test-utils/unit-test-utils';
import { testCatalogSource, testPackageManifest, dummyPackageManifest } from '../../../mocks';
Expand All @@ -18,6 +19,7 @@ import {

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

jest.mock('react-router-dom-v5-compat', () => ({
Expand All @@ -35,10 +37,6 @@ jest.mock('@console/internal/components/factory', () => ({

jest.mock('@console/internal/components/utils', () => ({
...jest.requireActual('@console/internal/components/utils'),
Firehose: jest.fn(({ children }) => {
const props = { packageManifest: { loaded: false } };
return typeof children === 'function' ? children(props) : children;
}),
LoadingBox: jest.fn(() => 'Loading...'),
ResourceSummary: jest.fn(() => null),
SectionHeading: jest.fn(() => null),
Expand Down Expand Up @@ -86,8 +84,8 @@ jest.mock('@patternfly/react-core', () => ({
}));

const mockDetailsPage = (DetailsPage as unknown) as jest.Mock;
const mockFirehose = (Firehose as unknown) as jest.Mock;
const mockUseK8sWatchResource = useK8sWatchResource as jest.Mock;
const mockUseK8sWatchResources = useK8sWatchResources as jest.Mock;

describe('CatalogSourceDetails', () => {
let obj;
Expand Down Expand Up @@ -169,20 +167,17 @@ describe('CreateSubscriptionYAML', () => {
hash: '',
key: 'default',
});
mockFirehose.mockClear();
mockUseK8sWatchResources.mockClear();
});

afterEach(() => {
jest.restoreAllMocks();
});

it('displays package name in the subscription YAML when loaded', () => {
mockFirehose.mockImplementationOnce((firehoseProps) => {
const childElement = firehoseProps.children;
return cloneElement(childElement, {
packageManifest: { loaded: true, data: testPackageManifest },
operatorGroup: { loaded: true, data: [] },
});
mockUseK8sWatchResources.mockReturnValue({
packageManifest: { loaded: true, data: testPackageManifest, loadError: null },
operatorGroup: { loaded: true, data: [], loadError: null },
});

renderWithProviders(<CreateSubscriptionYAML />);
Expand All @@ -191,12 +186,9 @@ describe('CreateSubscriptionYAML', () => {
});

it('displays loading indicator when package manifest is not yet loaded', () => {
mockFirehose.mockImplementationOnce((firehoseProps) => {
const childElement = firehoseProps.children;
return cloneElement(childElement, {
packageManifest: { loaded: false },
operatorGroup: { loaded: false },
});
mockUseK8sWatchResources.mockReturnValue({
packageManifest: { loaded: false, data: undefined, loadError: null },
operatorGroup: { loaded: false, data: undefined, loadError: null },
});

renderWithProviders(<CreateSubscriptionYAML />);
Expand All @@ -205,12 +197,9 @@ describe('CreateSubscriptionYAML', () => {
});

it('displays subscription YAML with default channel information', () => {
mockFirehose.mockImplementationOnce((firehoseProps) => {
const childElement = firehoseProps.children;
return cloneElement(childElement, {
packageManifest: { loaded: true, data: testPackageManifest },
operatorGroup: { loaded: true, data: [] },
});
mockUseK8sWatchResources.mockReturnValue({
packageManifest: { loaded: true, data: testPackageManifest, loadError: null },
operatorGroup: { loaded: true, data: [], loadError: null },
});

renderWithProviders(<CreateSubscriptionYAML />);
Expand Down
54 changes: 28 additions & 26 deletions frontend/packages/operator-lifecycle-manager/src/components/catalog-source.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,7 @@ import type {
RowFunctionArgs,
} from '@console/internal/components/factory';
import { DetailsPage, Table, TableData, MultiListPage } from '@console/internal/components/factory';
import type { FirehoseResult } from '@console/internal/components/utils';
import {
Firehose,
LoadingBox,
ConsoleEmptyState,
navFactory,
Expand All @@ -27,6 +25,7 @@ import {
ResourceSummary,
DetailsItem,
} from '@console/internal/components/utils';
import { useK8sWatchResources } from '@console/internal/components/utils/k8s-watch-hook';
import i18n from '@console/internal/i18n';
import { ConfigMapModel } from '@console/internal/models';
import type { K8sKind, K8sModel } from '@console/internal/module/k8s';
Expand Down Expand Up @@ -213,12 +212,30 @@ export const CatalogSourceDetailsPage: FC = (props) => {

export const CreateSubscriptionYAML: FC = (props) => {
type CreateProps = {
packageManifest: { loaded: boolean; data?: PackageManifestKind };
operatorGroup: { loaded: boolean; data?: OperatorGroupKind[] };
packageManifest: { loaded: boolean; data?: PackageManifestKind; loadError?: unknown };
operatorGroup: { loaded: boolean; data?: OperatorGroupKind[]; loadError?: unknown };
};
const { t } = useTranslation();
const params = useParams();
const location = useLocation();

const resources = useK8sWatchResources<{
packageManifest: PackageManifestKind;
operatorGroup: OperatorGroupKind[];
}>({
packageManifest: {
kind: referenceForModel(PackageManifestModel),
isList: false,
name: new URLSearchParams(location.search).get('pkg'),
namespace: new URLSearchParams(location.search).get('catalogNamespace'),
Comment on lines +229 to +230
Copy link
Member

@logonoff logonoff Feb 23, 2026

Choose a reason for hiding this comment

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

feels like we should use useSearchParams

Copy link
Contributor Author

@cajieh cajieh Feb 23, 2026

Choose a reason for hiding this comment

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

Not new, and appears to be out of scope for this work. We can open a ticket if this cleanup is required.

},
operatorGroup: {
kind: referenceForModel(OperatorGroupModel),
isList: true,
namespace: params.ns,
},
});

const Create = requireOperatorGroup(
withFallback<CreateProps>(
(createProps) => {
Expand Down Expand Up @@ -256,26 +273,11 @@ export const CreateSubscriptionYAML: FC = (props) => {
);

return (
<Firehose
resources={[
{
kind: referenceForModel(PackageManifestModel),
isList: false,
name: new URLSearchParams(location.search).get('pkg'),
namespace: new URLSearchParams(location.search).get('catalogNamespace'),
prop: 'packageManifest',
},
{
kind: referenceForModel(OperatorGroupModel),
isList: true,
namespace: params.ns,
prop: 'operatorGroup',
},
]}
>
{/* FIXME(alecmerdler): Hack because `Firehose` injects props without TypeScript knowing about it */}
<Create {...(props as any)} />
</Firehose>
<Create
{...(props as any)}
packageManifest={resources.packageManifest}
operatorGroup={resources.operatorGroup}
/>
);
};

Expand Down Expand Up @@ -546,8 +548,8 @@ type DisabledPopoverProps = {
};

type FlattenArgType = {
catalogSources?: FirehoseResult<CatalogSourceKind[]>;
packageManifests?: FirehoseResult<PackageManifestKind[]>;
catalogSources?: { data?: CatalogSourceKind[]; loaded?: boolean; loadError?: unknown };
packageManifests?: { data?: PackageManifestKind[]; loaded?: boolean; loadError?: unknown };
operatorHub: OperatorHubKind;
};

Expand Down
Loading