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
@@ -1,25 +1,26 @@
import type { FC } from 'react';
import { useParams } from 'react-router-dom-v5-compat';
import { FirehoseResource, Firehose } from '@console/internal/components/utils';
import { useK8sWatchResource } from '@console/internal/components/utils/k8s-watch-hook';
import { K8sResourceKind } from '@console/internal/module/k8s';
import AddHealthChecksForm from './AddHealthChecksForm';

const HealthChecksPage: FC = () => {
const { ns, kind, name, containerName } = useParams();
const resource: FirehoseResource[] = [
{
kind,
namespace: ns,
isList: false,
name,
prop: 'resource',
},
];

return (
<Firehose resources={resource}>
<AddHealthChecksForm currentContainer={containerName} />
</Firehose>
);
const [resourceData, loaded, loadError] = useK8sWatchResource<K8sResourceKind>({
kind,
namespace: ns,
isList: false,
name,
});
Comment on lines +10 to +15
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Route params may be undefined — consider guard or conditional watch.

The kind, ns, and name values from useParams() are typed as string | undefined. If any of these are missing (e.g., malformed URL or route mismatch), useK8sWatchResource will attempt to watch with undefined values, which could lead to unexpected API calls or silent failures.

Consider either:

  1. Adding a guard clause that returns early (e.g., a loading state or error) when required params are missing, or
  2. Using a conditional resource watch pattern (passing null to skip the watch when params are invalid).
🛡️ Proposed fix using conditional watch
-  const [resourceData, loaded, loadError] = useK8sWatchResource<K8sResourceKind>({
-    kind,
-    namespace: ns,
-    isList: false,
-    name,
-  });
+  const watchResource = kind && ns && name
+    ? { kind, namespace: ns, isList: false, name }
+    : null;
+
+  const [resourceData, loaded, loadError] = useK8sWatchResource<K8sResourceKind>(watchResource);
🤖 Prompt for AI Agents
In
`@frontend/packages/dev-console/src/components/health-checks/HealthChecksPage.tsx`
around lines 10 - 15, The route params (kind, ns, name) from useParams may be
undefined, so change HealthChecksPage.tsx to avoid calling useK8sWatchResource
with undefined values: either add an early guard in the component that returns a
loading/error UI when any of kind/ns/name is missing, or switch to a conditional
watch by only invoking useK8sWatchResource (or passing null as the resource
argument) when all three params are defined; reference the existing
useK8sWatchResource call and the kind/ns/name variables to implement the guard
or conditional call.


const resource = {
data: resourceData,
loaded,
loadError,
};

return <AddHealthChecksForm resource={resource} currentContainer={containerName} />;
};

export default HealthChecksPage;
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,16 @@ import { createOrUpdateDeployImageResources } from './deployImage-submit-utils';
import { deployValidationSchema } from './deployImage-validation-utils';
import DeployImageForm from './DeployImageForm';
import { filterDeployedResources } from './import-submit-utils';
import { DeployImageFormData, FirehoseList, Resources } from './import-types';
import { DeployImageFormData, Resources } from './import-types';
import { useUpdateKnScalingDefaultValues } from './serverless/useUpdateKnScalingDefaultValues';

export interface DeployImageProps {
namespace: string;
projects?: FirehoseList;
projects?: {
data: K8sResourceKind[];
loaded: boolean;
loadError?: any;
};
contextualSource?: string;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import type { FunctionComponent } from 'react';
import { useTranslation } from 'react-i18next';
import { useParams, useLocation } from 'react-router-dom-v5-compat';
import { Firehose } from '@console/internal/components/utils';
import { useK8sWatchResource } from '@console/internal/components/utils/k8s-watch-hook';
import { K8sResourceKind } from '@console/internal/module/k8s';
import { DocumentTitle } from '@console/shared/src/components/document-title/DocumentTitle';
import { PageHeading } from '@console/shared/src/components/heading/PageHeading';
import { QUERY_PROPERTIES } from '../../const';
Expand All @@ -15,19 +16,29 @@ const DeployImagePage: FunctionComponent = () => {
const location = useLocation();
const params = new URLSearchParams(location.search);

const [projectsData, loaded, loadError] = useK8sWatchResource<K8sResourceKind[]>({
kind: 'Project',
isList: true,
});

const projects = {
data: projectsData,
loaded,
loadError,
};

return (
<NamespacedPage disabled variant={NamespacedPageVariants.light}>
<DocumentTitle>{t('devconsole~Deploy Image')}</DocumentTitle>
<PageHeading title={t('devconsole~Deploy Image')} />
<QueryFocusApplication>
{(desiredApplication) => (
<Firehose resources={[{ kind: 'Project', prop: 'projects', isList: true }]}>
<DeployImage
forApplication={desiredApplication}
namespace={namespace}
contextualSource={params.get(QUERY_PROPERTIES.CONTEXT_SOURCE)}
/>
</Firehose>
<DeployImage
forApplication={desiredApplication}
namespace={namespace}
contextualSource={params.get(QUERY_PROPERTIES.CONTEXT_SOURCE)}
projects={projects}
/>
)}
</QueryFocusApplication>
</NamespacedPage>
Expand Down
18 changes: 13 additions & 5 deletions frontend/packages/dev-console/src/components/import/ImportForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { useActivePerspective } from '@console/dynamic-plugin-sdk';
import { GitProvider, ImportStrategy } from '@console/git-service/src';
import { history, AsyncComponent, StatusBox } from '@console/internal/components/utils';
import { RouteModel } from '@console/internal/models';
import { RouteKind } from '@console/internal/module/k8s';
import { RouteKind, K8sResourceKind } from '@console/internal/module/k8s';
import { getActiveApplication } from '@console/internal/reducers/ui';
import { RootState } from '@console/internal/redux';
import { ALL_APPLICATIONS_KEY, usePerspectives, useTelemetry } from '@console/shared';
Expand Down Expand Up @@ -38,7 +38,6 @@ import {
} from './import-submit-utils';
import {
GitImportFormData,
FirehoseList,
ImportData,
Resources,
BaseFormData,
Expand All @@ -55,10 +54,15 @@ export interface ImportFormProps {
namespace: string;
importData: ImportData;
contextualSource?: string;
imageStreams?: FirehoseList;
imageStreams?: {
data: K8sResourceKind | K8sResourceKind[];
loaded: boolean;
loadError?: any;
};
projects?: {
data: K8sResourceKind[];
loaded: boolean;
data: [];
loadError?: any;
};
}

Expand Down Expand Up @@ -153,7 +157,11 @@ const ImportForm: FC<ImportFormProps & StateProps> = ({

const initialVals = useUpdateKnScalingDefaultValues(initialValues);
const builderImages: NormalizedBuilderImages =
imageStreams && imageStreams.loaded && normalizeBuilderImages(imageStreams.data);
imageStreams &&
imageStreams.loaded &&
normalizeBuilderImages(
Array.isArray(imageStreams.data) ? imageStreams.data : [imageStreams.data],
);

const handleSubmit = (values: GitImportFormData, actions) => {
const imageStream = builderImages && builderImages[values.image.selected]?.obj;
Expand Down
89 changes: 49 additions & 40 deletions frontend/packages/dev-console/src/components/import/ImportPage.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import type { FunctionComponent } from 'react';
import { useMemo } from 'react';
import { TFunction } from 'i18next';
import { useTranslation } from 'react-i18next';
import { useParams, useLocation } from 'react-router-dom-v5-compat';
import { Firehose, FirehoseResource } from '@console/internal/components/utils';
import { useK8sWatchResources } from '@console/internal/components/utils/k8s-watch-hook';
import { ImageStreamModel, ProjectModel } from '@console/internal/models';
import { K8sResourceKind } from '@console/internal/module/k8s';
import DevPreviewBadge from '@console/shared/src/components/badges/DevPreviewBadge';
import { DocumentTitle } from '@console/shared/src/components/document-title/DocumentTitle';
import { PageHeading } from '@console/shared/src/components/heading/PageHeading';
Expand Down Expand Up @@ -44,40 +46,47 @@ const ImportPage: FunctionComponent = () => {
const preselectedNamespace = searchParams.get('preselected-ns');
const importType = searchParams.get('importType');

let importData: ImportData;
let resources: FirehoseResource[];
if (imageStreamName && imageStreamNamespace) {
importData = ImportFlows(t).s2i;
resources = [
{
kind: ImageStreamModel.kind,
prop: 'imageStreams',
isList: false,
name: imageStreamName,
namespace: imageStreamNamespace,
},
{
kind: ProjectModel.kind,
prop: 'projects',
isList: true,
},
];
} else {
importData = ImportFlows(t).git;
resources = [
{
kind: ImageStreamModel.kind,
prop: 'imageStreams',
isList: true,
namespace: 'openshift',
},
{
const isS2i = !!(imageStreamName && imageStreamNamespace);
const importData: ImportData = isS2i ? ImportFlows(t).s2i : ImportFlows(t).git;

const watchResources = useMemo(
() => ({
imageStreams: isS2i
? {
kind: ImageStreamModel.kind,
isList: false,
name: imageStreamName,
namespace: imageStreamNamespace,
}
: {
kind: ImageStreamModel.kind,
isList: true,
namespace: 'openshift',
},
projects: {
kind: ProjectModel.kind,
prop: 'projects',
isList: true,
},
];
}
}),
[isS2i, imageStreamName, imageStreamNamespace],
);

const resources = useK8sWatchResources<{
imageStreams: K8sResourceKind | K8sResourceKind[];
projects: K8sResourceKind[];
}>(watchResources);

const imageStreams = {
data: resources.imageStreams.data,
loaded: resources.imageStreams.loaded,
loadError: resources.imageStreams.loadError,
};

const projects = {
data: resources.projects.data,
loaded: resources.projects.loaded,
loadError: resources.projects.loadError,
};

return (
<QueryFocusApplication>
Expand All @@ -88,14 +97,14 @@ const ImportPage: FunctionComponent = () => {
title={importData.title}
badge={importType === ImportTypes.devfile ? <DevPreviewBadge /> : null}
/>
<Firehose resources={resources}>
<ImportForm
forApplication={application}
contextualSource={searchParams.get(QUERY_PROPERTIES.CONTEXT_SOURCE)}
namespace={namespace || preselectedNamespace}
importData={importData}
/>
</Firehose>
<ImportForm
forApplication={application}
contextualSource={searchParams.get(QUERY_PROPERTIES.CONTEXT_SOURCE)}
namespace={namespace || preselectedNamespace}
importData={importData}
imageStreams={imageStreams}
projects={projects}
/>
</NamespacedPage>
)}
</QueryFocusApplication>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { useMemo } from 'react';
import { Formik } from 'formik';
import { useTranslation } from 'react-i18next';
import { useParams } from 'react-router-dom-v5-compat';
import { FirehoseResource, LoadingBox, history } from '@console/internal/components/utils';
import { LoadingBox, history } from '@console/internal/components/utils';
import { useK8sWatchResource } from '@console/internal/components/utils/k8s-watch-hook';
import { ImageStreamModel } from '@console/internal/models';
import { K8sResourceKind } from '@console/internal/module/k8s';
Expand All @@ -30,7 +30,7 @@ const ImportSamplePage: FC = () => {
const { t } = useTranslation();
const { ns: namespace, is: imageStreamName, isNs: imageStreamNamespace } = useParams();

const imageStreamResource: FirehoseResource = useMemo(
const imageStreamResource = useMemo(
() => ({
kind: ImageStreamModel.kind,
prop: 'imageStreams',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,16 +62,12 @@ jest.mock('../../QueryFocusApplication', () => ({
},
}));

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

jest.mock('@console/internal/components/utils', () => ({
...jest.requireActual('@console/internal/components/utils'),
Firehose: (props) => {
const mockProps = {
projects: { data: [], loaded: true },
};
return props.children && typeof props.children === 'function'
? props.children(mockProps)
: 'Firehose Component';
},
usePreventDataLossLock: jest.fn(),
}));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,13 @@ import { PipelineData } from '../pipeline-section/import-types';

export interface DeployImageFormProps {
builderImages?: NormalizedBuilderImages;
projects?: FirehoseList | WatchK8sResultsObject<K8sResourceKind[]>;
projects?:
| {
data: K8sResourceKind[];
loaded: boolean;
loadError?: any;
}
| WatchK8sResultsObject<K8sResourceKind[]>;
}
export type ImageStreamPayload = boolean | K8sResourceKind;

Expand All @@ -38,31 +44,34 @@ export interface ImageStreamContextProps {
export interface SourceToImageFormProps {
builderImages?: NormalizedBuilderImages;
projects?: {
data: [];
data: K8sResourceKind[];
loaded: boolean;
loadError?: any;
};
}

export interface GitImportFormProps {
builderImages?: NormalizedBuilderImages;
imageStreams?: {
data: K8sResourceKind | K8sResourceKind[];
loaded: boolean;
loadError?: any;
};
projects?: {
data: [];
data: K8sResourceKind[];
loaded: boolean;
loadError?: any;
};
}
export interface DevfileImportFormProps {
builderImages?: NormalizedBuilderImages;
projects?: {
data: [];
data: K8sResourceKind[];
loaded: boolean;
loadError?: any;
};
}

export interface FirehoseList {
data?: K8sResourceKind[];
[key: string]: any;
}

export interface DeployImageFormData {
formType?: string;
project: ProjectData;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { useTranslation } from 'react-i18next';
import { Link } from 'react-router-dom-v5-compat';
import { Alert } from '@console/dynamic-plugin-sdk';
import { sortEvents } from '@console/internal/components/events';
import { FirehoseResult, LoadingBox } from '@console/internal/components/utils';
import { LoadingBox } from '@console/internal/components/utils';
import { DeploymentConfigModel } from '@console/internal/models';
import { K8sResourceKind, EventKind, PodKind } from '@console/internal/module/k8s';
import { getFiringAlerts } from '@console/shared';
Expand All @@ -28,7 +28,11 @@ import './MonitoringOverview.scss';
type MonitoringOverviewProps = {
resource: K8sResourceKind;
pods?: PodKind[];
resourceEvents?: FirehoseResult<EventKind[]>;
resourceEvents?: {
data: EventKind[];
loaded: boolean;
loadError?: Error;
};
monitoringAlerts: Alert[];
};

Expand Down
Loading