Skip to content
Closed
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,12 +1,11 @@
import * as React from 'react';
import { Formik, useFormikContext } from 'formik';
import { Formik } from 'formik';
import { TFunction } from 'i18next';
import * as Yup from 'yup';
import {
ClusterWizardStep,
TechnologyPreview,
sshPublicKeyValidationSchema,
pullSecretValidationSchema,
getFormikErrorFields,
httpProxyValidationSchema,
noProxyValidationSchema,
Expand All @@ -16,15 +15,14 @@ import {
AdditionalNTPSourcesField,
ProxyFieldsType,
} from '../../../../common';
import { Split, SplitItem, Grid, GridItem, Form, Content, Checkbox } from '@patternfly/react-core';
import { Split, SplitItem, Grid, GridItem, Form, Content } from '@patternfly/react-core';
import { useClusterWizardContext } from '../ClusterWizardContext';
import ClusterWizardFooter from '../ClusterWizardFooter';
import ClusterWizardNavigation from '../ClusterWizardNavigation';
import { WithErrorBoundary } from '../../../../common/components/ErrorHandling/WithErrorBoundary';
import UploadSSH from '../../../../common/components/clusterConfiguration/UploadSSH';
import PullSecretField from '../../../../common/components/ui/formik/PullSecretField';
import { ProxyInputFields } from '../../../../common/components/clusterConfiguration/ProxyFields';
import { isInOcm, handleApiError, getApiErrorMessage } from '../../../../common/api';
import { handleApiError, getApiErrorMessage } from '../../../../common/api';
import { useAlerts } from '../../../../common/components/AlertsContextProvider';
import { AlertVariant } from '@patternfly/react-core';
import InfraEnvsService from '../../../services/InfraEnvsService';
Expand All @@ -49,7 +47,6 @@ const DISCONNECTED_IMAGE_TYPE = 'disconnected-iso' as const;

type OptionalConfigurationsFormValues = ProxyFieldsType & {
sshPublicKey?: string;
pullSecret: string;
enableNtpSources: boolean;
additionalNtpSources?: string;
hostsNetworkConfigurationType: HostsNetworkConfigurationType;
Expand Down Expand Up @@ -105,7 +102,7 @@ const buildInfraEnvParams = (values: OptionalConfigurationsFormValues) => {
const hasProxy = Object.keys(proxy).length > 0;

return {
pullSecret: values.pullSecret,
// pullSecret,
...(values.sshPublicKey && { sshAuthorizedKey: values.sshPublicKey }),
...(hasProxy && { proxy }),
...(values.additionalNtpSources && {
Expand All @@ -124,7 +121,6 @@ const getValidationSchema = (t: TFunction) =>
Yup.lazy((values: OptionalConfigurationsFormValues) =>
Yup.object().shape({
sshPublicKey: sshPublicKeyValidationSchema(t),
pullSecret: pullSecretValidationSchema(t).required('Required field'),
enableProxy: Yup.boolean().required(),
httpProxy: httpProxyValidationSchema({
values,
Expand Down Expand Up @@ -154,26 +150,11 @@ const getValidationSchema = (t: TFunction) =>
}),
);

const PullSecretSync = () => {
const defaultPullSecret = usePullSecret();
const {
setFieldValue,
values: { pullSecret },
} = useFormikContext<OptionalConfigurationsFormValues>();

React.useEffect(() => {
if (defaultPullSecret !== undefined && pullSecret === '') {
setFieldValue('pullSecret', defaultPullSecret);
}
}, [defaultPullSecret, pullSecret, setFieldValue]);

return null;
};

const OptionalConfigurationsStep = () => {
const { clusterId } = useParams<{ clusterId: string }>();
const [cluster, setCluster] = React.useState<Cluster | null>(null);
const { t } = useTranslation();
const defaultPullSecret = usePullSecret();

const {
moveNext,
Expand Down Expand Up @@ -208,9 +189,17 @@ const OptionalConfigurationsStep = () => {
void fetchCluster();
}, [clusterId, addAlert, t]);

const initialValues: OptionalConfigurationsFormValues = disconnectedInfraEnv
? infraEnvToFormValues(disconnectedInfraEnv, disconnectedFormPullSecret)
: DEFAULT_INITIAL_VALUES;
const initialValues: OptionalConfigurationsFormValues = {
sshPublicKey: '',
enableProxy: false,
httpProxy: '',
httpsProxy: '',
noProxy: '',
enableNtpSources: false,
additionalNtpSources: '',
hostsNetworkConfigurationType: HostsNetworkConfigurationType.DHCP,
rendezvousIp: '',
};

return (
<Formik
Expand Down Expand Up @@ -258,6 +247,7 @@ const OptionalConfigurationsStep = () => {
openshiftVersion: cluster.openshiftVersion,
cpuArchitecture: DEFAULT_CPU_ARCHITECTURE,
imageType: DISCONNECTED_IMAGE_TYPE,
pullSecret: defaultPullSecret ?? '',
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Risk of creating InfraEnv with an empty pull secret.

usePullSecret() returns undefined on initial render and '' on fetch failure. Since isNextDisabled on line 239 doesn't gate on defaultPullSecret being available, a user can submit the form before the pull secret resolves (or after it fails), sending an empty string to InfraEnvCreateParams.pullSecret.

Disable "Next" until a valid pull secret is available:

Proposed fix
-                isNextDisabled={!isValid || !cluster}
+                isNextDisabled={!isValid || !cluster || !defaultPullSecret}

This also covers the error/empty-data cases from usePullSecret (which sets ''), since an empty string is falsy.

Also applies to: 239-239

🤖 Prompt for AI Agents
In
`@libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/OptionalConfigurationsStep.tsx`
at line 201, The form currently allows submitting an InfraEnv with an empty pull
secret because usePullSecret() can be undefined or '' and the current
isNextDisabled check doesn't verify defaultPullSecret; update the component to
require a truthy pull secret before enabling Next and to avoid sending an empty
string as InfraEnvCreateParams.pullSecret: change the payload assignment for
pullSecret to use defaultPullSecret only when truthy (do not default to ''), and
update the isNextDisabled logic (the check used to enable the Next button) to
include a `!defaultPullSecret` check (or `!Boolean(defaultPullSecret)`) so Next
remains disabled until usePullSecret() resolves to a non-empty value.

...commonParams,
};
const createdInfraEnv = await InfraEnvsService.create(createParams);
Expand Down Expand Up @@ -306,7 +296,6 @@ const OptionalConfigurationsStep = () => {
}
>
<WithErrorBoundary title="Failed to load Optional Configurations step">
<PullSecretSync />
<Grid hasGutter>
<GridItem>
<Split>
Expand All @@ -332,17 +321,6 @@ const OptionalConfigurationsStep = () => {
/>

<UploadSSH />
<Checkbox
label={t('ai:Edit pull secret')}
isChecked={
disconnectedFormEditPullSecret ?? !!disconnectedInfraEnv?.pullSecretSet
}
onChange={(_, checked) => setDisconnectedFormEditPullSecret(checked)}
id="edit-pull-secret-checkbox"
/>
{(disconnectedFormEditPullSecret ?? !!disconnectedInfraEnv?.pullSecretSet) && (
<PullSecretField isOcm={isInOcm} />
)}

{/* Proxy Settings */}
<CheckboxField
Expand Down
Loading