Skip to content
Merged
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
20 changes: 5 additions & 15 deletions frontend/packages/console-shared/src/hooks/useLabelsModal.tsx
Original file line number Diff line number Diff line change
@@ -1,27 +1,17 @@
import { useCallback } from 'react';
import type { ModalComponent } from '@console/dynamic-plugin-sdk/src/app/modal-support/ModalProvider';
import { useModal } from '@console/dynamic-plugin-sdk/src/app/modal-support/useModal';
import { useOverlay } from '@console/dynamic-plugin-sdk/src/app/modal-support/useOverlay';
import type { UseLabelsModal } from '@console/dynamic-plugin-sdk/src/extensions/console-types';
import { useK8sModel } from '@console/dynamic-plugin-sdk/src/utils/k8s/hooks/useK8sModel';
import { getGroupVersionKindForResource } from '@console/dynamic-plugin-sdk/src/utils/k8s/k8s-ref';
import { ModalWrapper } from '@console/internal/components/factory/modal';
import type { LabelsModalProps } from '@console/internal/components/modals/labels-modal';
import { LabelsModal } from '@console/internal/components/modals/labels-modal';

const LabelsModalComponent: ModalComponent<LabelsModalProps> = ({ closeModal, kind, resource }) => {
return (
<ModalWrapper blocking onClose={closeModal}>
<LabelsModal cancel={closeModal} close={closeModal} kind={kind} resource={resource} />
</ModalWrapper>
);
};
import { LabelsModalOverlay } from '@console/internal/components/modals/labels-modal';

export const useLabelsModal: UseLabelsModal = (resource) => {
const launcher = useModal();
const launchModal = useOverlay();
const groupVersionKind = getGroupVersionKindForResource(resource);
const [kind] = useK8sModel(groupVersionKind);
return useCallback(
() => resource && kind && launcher<LabelsModalProps>(LabelsModalComponent, { kind, resource }),
[launcher, kind, resource],
() => resource && kind && launchModal<LabelsModalProps>(LabelsModalOverlay, { kind, resource }),
[launchModal, kind, resource],
);
};
262 changes: 147 additions & 115 deletions frontend/public/components/modals/add-secret-to-workload.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,22 @@ import { useState, useEffect, useCallback } from 'react';
import * as _ from 'lodash';
import * as fuzzy from 'fuzzysearch';
import { useNavigate } from 'react-router-dom-v5-compat';
import { FormGroup, Radio } from '@patternfly/react-core';
import {
Button,
Content,
ContentVariants,
Form,
FormGroup,
Modal,
ModalBody,
ModalHeader,
ModalVariant,
Radio,
TextInput,
} from '@patternfly/react-core';

import { K8sKind, k8sList, k8sPatch, K8sResourceKind } from '../../module/k8s';
import { DeploymentModel, DeploymentConfigModel, StatefulSetModel } from '../../models';
import { ModalTitle, ModalBody, ModalSubmitFooter, ModalWrapper } from '../factory/modal';
import { ConsoleSelect } from '@console/internal/components/utils/console-select';
import { ResourceIcon, ResourceName } from '../utils/resource-icon';
import { resourcePathFromModel } from '../utils/resource-link';
Expand All @@ -16,6 +27,8 @@ import { Trans, useTranslation } from 'react-i18next';
import { useOverlay } from '@console/dynamic-plugin-sdk/src/app/modal-support/useOverlay';
import { OverlayComponent } from '@console/dynamic-plugin-sdk/src/app/modal-support/OverlayProvider';
import { ModalCallback } from './types';
import type { ModalComponentProps } from '../factory';
import { ModalFooterWithAlerts } from '@console/shared/src/components/modals/ModalFooterWithAlerts';

const workloadResourceModels = [DeploymentModel, DeploymentConfigModel, StatefulSetModel];
const getContainers = (workload: K8sResourceKind) =>
Expand Down Expand Up @@ -167,132 +180,154 @@ export const AddSecretToWorkloadModal: FC<AddSecretToWorkloadModalProps> = (prop
const selectWorkloadPlaceholder = t('public~Select a workload');

return (
<form
onSubmit={submit}
name="co-add-secret-to-workload"
className="co-add-secret-to-workload modal-content"
>
<ModalTitle>{t('public~Add secret to workload')}</ModalTitle>
<>
<ModalHeader
title={t('public~Add secret to workload')}
data-test-id="modal-title"
labelId="add-secret-to-workload-modal-title"
/>
<ModalBody>
<p className="modal-paragraph">
<Content component={ContentVariants.p}>
<Trans t={t} ns="public">
Add all values from <ResourceIcon kind="Secret" />
{{ secretName }} to a workload as environment variables or a volume.
</Trans>
</p>
<div className="form-group">
<label className="co-required" htmlFor="co-add-secret-to-workload__workload">
{t('public~Add this secret to workload')}
</label>
<ConsoleSelect
items={workloadOptions}
selectedKey={selectedWorkloadUID}
title={selectWorkloadPlaceholder}
onChange={onWorkloadChange}
autocompleteFilter={autocompleteFilter}
autocompletePlaceholder={selectWorkloadPlaceholder}
id="co-add-secret-to-workload__workload"
data-test="add-secret-to-workload-button"
/>
</div>
<fieldset>
<legend className="co-legend co-required">{t('public~Add secret as')}</legend>
<div className="pf-v6-c-form">
<FormGroup
role="radiogroup"
fieldId="co-add-secret-to-workload"
isStack
className="form-group"
>
<Radio
id="co-add-secret-to-workload__envvars"
name="co-add-secret-to-workload__add-as"
label={t('public~Environment variables')}
value="environment"
onChange={onAddAsChange}
isChecked={addAsEnvironment}
data-test="Environment variables-radio-input"
data-checked-state={addAsEnvironment}
/>
{addAsEnvironment && (
<div className="co-m-radio-desc">
<div className="form-group">
<label htmlFor="co-add-secret-to-workload__prefix">{t('public~Prefix')}</label>
<span className="pf-v6-c-form-control">
<input
name="prefix"
id="co-add-secret-to-workload__prefix"
data-test="add-secret-to-workload-prefix"
placeholder="(optional)"
type="text"
onChange={(e) => setPrefix(e.currentTarget.value)}
/>
</span>
</div>
</div>
)}
<Radio
id="co-add-secret-to-workload__volume"
name="co-add-secret-to-workload__add-as"
label={t('public~Volume')}
value="volume"
onChange={onAddAsChange}
isChecked={addAsVolume}
data-test="Volume-radio-input"
data-checked-state={addAsVolume}
/>
{addAsVolume && (
<div className="co-m-radio-desc">
<div className="form-group">
<label htmlFor="co-add-secret-to-workload__mountpath" className="co-required">
{t('public~Mount path')}
</label>
<span className="pf-v6-c-form-control">
<input
name="mountPath"
id="co-add-secret-to-workload__mountpath"
data-test="add-secret-to-workload-mountpath"
type="text"
onChange={(e) => setMountPath(e.currentTarget.value)}
required
/>
</span>
</div>
</div>
)}
</FormGroup>
</div>
</fieldset>
</Content>
<Form id="co-add-secret-to-workload" onSubmit={submit}>
<FormGroup
label={t('public~Add this secret to workload')}
isRequired
fieldId="co-add-secret-to-workload__workload"
>
<ConsoleSelect
items={workloadOptions}
selectedKey={selectedWorkloadUID}
title={selectWorkloadPlaceholder}
onChange={onWorkloadChange}
autocompleteFilter={autocompleteFilter}
autocompletePlaceholder={selectWorkloadPlaceholder}
id="co-add-secret-to-workload__workload"
data-test="add-secret-to-workload-button"
/>
</FormGroup>
<FormGroup
label={t('public~Add secret as')}
isRequired
role="radiogroup"
fieldId="co-add-secret-to-workload"
isStack
>
Comment on lines +196 to +219
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

🧩 Analysis chain

🏁 Script executed:

cat -n frontend/public/components/modals/add-secret-to-workload.tsx | sed -n '190,320p'

Repository: openshift/console

Length of output: 5565


🏁 Script executed:

find frontend/public/components/modals -name "*.tsx" -type f | head -10

Repository: openshift/console

Length of output: 756


🏁 Script executed:

rg -A 8 'role="radiogroup"' frontend/public/components/modals/ -t tsx

Repository: openshift/console

Length of output: 88


🏁 Script executed:

rg -A 8 'role="radiogroup"' frontend/public/components/modals/ --type ts

Repository: openshift/console

Length of output: 2962


🏁 Script executed:

grep -n 'id="co-add-secret-to-workload"' frontend/public/components/modals/configure-cluster-upstream-modal.tsx | head -5

Repository: openshift/console

Length of output: 43


🏁 Script executed:

grep -n '<Form' frontend/public/components/modals/configure-cluster-upstream-modal.tsx | head -5

Repository: openshift/console

Length of output: 293


🏁 Script executed:

sed -n '95,180p' frontend/public/components/modals/configure-cluster-upstream-modal.tsx | cat -n

Repository: openshift/console

Length of output: 3899


🏁 Script executed:

rg 'aria-labelledby' frontend/public/components/modals/add-secret-to-workload.tsx

Repository: openshift/console

Length of output: 119


🏁 Script executed:

rg -B 2 -A 2 'fieldId=' frontend/public/components/modals/configure-ns-pull-secret-modal.tsx | head -20

Repository: openshift/console

Length of output: 315


🏁 Script executed:

rg 'fieldId="co-add-secret-to-workload' frontend/public/components/modals/ --type ts

Repository: openshift/console

Length of output: 697


Use a unique fieldId for the radio group.

The FormGroup at line 217 reuses the form's id (co-add-secret-to-workload) instead of using a unique identifier. This breaks the label-to-field association; the "Add secret as" label will reference the form element rather than the radio group. Change fieldId="co-add-secret-to-workload" to fieldId="co-add-secret-to-workload__add-as" to match the namespace pattern used by adjacent form fields.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/public/components/modals/add-secret-to-workload.tsx` around lines
196 - 219, The FormGroup that renders the radio group (label "Add secret as") is
reusing the form's id via fieldId="co-add-secret-to-workload", breaking
label-to-field association; update that FormGroup's fieldId to a unique
identifier such as "co-add-secret-to-workload__add-as" (same file's
add-secret-to-workload modal, FormGroup for the radio group) so the label
correctly targets the radio controls.

<Radio
id="co-add-secret-to-workload__envvars"
name="co-add-secret-to-workload__add-as"
label={t('public~Environment variables')}
value="environment"
onChange={onAddAsChange}
isChecked={addAsEnvironment}
data-test="Environment variables-radio-input"
data-checked-state={addAsEnvironment}
body={
addAsEnvironment && (
<FormGroup label={t('public~Prefix')} fieldId="co-add-secret-to-workload__prefix">
<TextInput
name="prefix"
id="co-add-secret-to-workload__prefix"
data-test="add-secret-to-workload-prefix"
placeholder="(optional)"
type="text"
value={prefix}
onChange={(_event, value) => setPrefix(value)}
/>
</FormGroup>
)
}
/>
<Radio
id="co-add-secret-to-workload__volume"
name="co-add-secret-to-workload__add-as"
label={t('public~Volume')}
value="volume"
onChange={onAddAsChange}
isChecked={addAsVolume}
data-test="Volume-radio-input"
data-checked-state={addAsVolume}
body={
addAsVolume && (
<FormGroup
label={t('public~Mount path')}
isRequired
fieldId="co-add-secret-to-workload__mountpath"
>
<TextInput
name="mountPath"
id="co-add-secret-to-workload__mountpath"
data-test="add-secret-to-workload-mountpath"
type="text"
value={mountPath}
onChange={(_event, value) => setMountPath(value)}
isRequired
/>
</FormGroup>
)
}
/>
</FormGroup>
</Form>
</ModalBody>
<ModalSubmitFooter
errorMessage={errorMessage}
inProgress={inProgress}
submitText={t('public~Save')}
cancel={props.cancel}
/>
</form>
<ModalFooterWithAlerts errorMessage={errorMessage}>
<Button
type="submit"
variant="primary"
onClick={submit}
isLoading={inProgress}
isDisabled={inProgress}
data-test="confirm-action"
id="confirm-action"
form="co-add-secret-to-workload"
>
Comment on lines +278 to +287
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 | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cd frontend/public/components/modals && wc -l add-secret-to-workload.tsx

Repository: openshift/console

Length of output: 91


🏁 Script executed:

cd frontend/public/components/modals && sed -n '190,200p' add-secret-to-workload.tsx

Repository: openshift/console

Length of output: 552


🏁 Script executed:

cd frontend/public/components/modals && sed -n '275,290p' add-secret-to-workload.tsx

Repository: openshift/console

Length of output: 572


🏁 Script executed:

cd frontend/public/components/modals && sed -n '1,50p' add-secret-to-workload.tsx | head -100

Repository: openshift/console

Length of output: 2215


🏁 Script executed:

cd frontend/public/components/modals && grep -n "const submit" add-secret-to-workload.tsx

Repository: openshift/console

Length of output: 118


🏁 Script executed:

cd frontend/public/components/modals && grep -n "mountPath" add-secret-to-workload.tsx | head -20

Repository: openshift/console

Length of output: 246


🏁 Script executed:

cd frontend/public/components/modals && sed -n '100,180p' add-secret-to-workload.tsx

Repository: openshift/console

Length of output: 2816


🏁 Script executed:

cd frontend/public/components/modals && sed -n '220,275p' add-secret-to-workload.tsx

Repository: openshift/console

Length of output: 2254


🏁 Script executed:

cd frontend/public/components/modals && sed -n '125,145p' alert-routing-modal.tsx

Repository: openshift/console

Length of output: 621


Remove the onClick handler to let the form's validation run.

The button's onClick={submit} creates a secondary submission path that bypasses native form validation. Since the button already targets the form via form="co-add-secret-to-workload" and carries type="submit", the form's onSubmit handler fires automatically on click. The current setup allows submission without validating mountPath even though it's marked isRequired in the FormGroup. This diverges from the cleaner pattern in alert-routing-modal.tsx Lines 129–136, which relies entirely on the button's type and form attribute without a direct onClick handler.

♻️ Suggested fix
         <Button
           type="submit"
           variant="primary"
-          onClick={submit}
           isLoading={inProgress}
           isDisabled={inProgress}
           data-test="confirm-action"
           id="confirm-action"
           form="co-add-secret-to-workload"
📝 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
<Button
type="submit"
variant="primary"
onClick={submit}
isLoading={inProgress}
isDisabled={inProgress}
data-test="confirm-action"
id="confirm-action"
form="co-add-secret-to-workload"
>
<Button
type="submit"
variant="primary"
isLoading={inProgress}
isDisabled={inProgress}
data-test="confirm-action"
id="confirm-action"
form="co-add-secret-to-workload"
>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/public/components/modals/add-secret-to-workload.tsx` around lines
278 - 287, The Button component with id "confirm-action" currently has an
onClick={submit} which creates a secondary submission path that bypasses native
form validation; remove the onClick prop from the Button in
add-secret-to-workload.tsx so the button relies on type="submit" and
form="co-add-secret-to-workload" to trigger the form's onSubmit handler (submit)
and allow the browser/FormGroup (mountPath isRequired) validation to run
normally.

{t('public~Save')}
</Button>
<Button variant="link" onClick={props.cancel} data-test-id="modal-cancel-action">
{t('public~Cancel')}
</Button>
</ModalFooterWithAlerts>
</>
);
};

export const AddSecretToWorkloadModalProvider: OverlayComponent<AddSecretToWorkloadModalProps> = (
export const AddSecretToWorkloadModalOverlay: OverlayComponent<AddSecretToWorkloadModalProps> = (
props,
) => {
return (
<ModalWrapper blocking onClose={props.closeOverlay}>
<AddSecretToWorkloadModal close={props.closeOverlay} cancel={props.closeOverlay} {...props} />
</ModalWrapper>
);
const [isOpen, setIsOpen] = useState(true);
const handleClose = () => {
setIsOpen(false);
props.closeOverlay();
};

return isOpen ? (
<Modal
variant={ModalVariant.small}
isOpen
onClose={handleClose}
aria-labelledby="add-secret-to-workload-modal-title"
>
<AddSecretToWorkloadModal close={handleClose} cancel={handleClose} {...props} />
</Modal>
Comment on lines +307 to +315
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 | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, find and examine the AddSecretToWorkloadModal type definitions
fd -t f "add-secret-to-workload" --exec cat {} \;

Repository: openshift/console

Length of output: 12095


🏁 Script executed:

# Search for AddSecretToWorkloadModalProps definition
rg "AddSecretToWorkloadModalProps" -t ts -t tsx -A 5 -B 2

Repository: openshift/console

Length of output: 88


🏁 Script executed:

# Check the type definition for ModalComponentProps
rg "ModalComponentProps" -t ts -t tsx -A 5 -B 2

Repository: openshift/console

Length of output: 88


🏁 Script executed:

# Examine the referenced alert-routing-modal.tsx at line 161
fd "alert-routing-modal.tsx" --exec cat {} \;

Repository: openshift/console

Length of output: 5910


🏁 Script executed:

# Examine the referenced configure-cluster-upstream-modal.tsx at line 210
fd "configure-cluster-upstream-modal.tsx" --exec cat {} \;

Repository: openshift/console

Length of output: 7555


🏁 Script executed:

# Search for ModalComponentProps definition in factory files
find . -name "factory*" -o -name "modal.ts" -o -name "modal.tsx" | head -20

Repository: openshift/console

Length of output: 795


🏁 Script executed:

# Search for ModalComponentProps type definition
rg "type ModalComponentProps" -A 5

Repository: openshift/console

Length of output: 447


🏁 Script executed:

# Also check for interface ModalComponentProps
rg "interface ModalComponentProps" -A 5

Repository: openshift/console

Length of output: 43


Reorder props to ensure modal cleanup executes properly.

AddSecretToWorkloadModalProps extends ModalComponentProps, which defines optional close and cancel properties. Since {...props} spreads after the explicit prop assignments on the current line, any close or cancel from props will overwrite handleClose, breaking the guarantee that both setIsOpen(false) and closeOverlay() execute together. This differs from the patterns in alert-routing-modal.tsx (Line 161) and configure-cluster-upstream-modal.tsx (Line 210), which correctly spread props first.

Suggested fix
-      <AddSecretToWorkloadModal close={handleClose} cancel={handleClose} {...props} />
+      <AddSecretToWorkloadModal {...props} close={handleClose} cancel={handleClose} />
📝 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
return isOpen ? (
<Modal
variant={ModalVariant.small}
isOpen
onClose={handleClose}
aria-labelledby="add-secret-to-workload-modal-title"
>
<AddSecretToWorkloadModal close={handleClose} cancel={handleClose} {...props} />
</Modal>
return isOpen ? (
<Modal
variant={ModalVariant.small}
isOpen
onClose={handleClose}
aria-labelledby="add-secret-to-workload-modal-title"
>
<AddSecretToWorkloadModal {...props} close={handleClose} cancel={handleClose} />
</Modal>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/public/components/modals/add-secret-to-workload.tsx` around lines
307 - 315, The modal currently spreads {...props} after explicitly passing
close={handleClose} and cancel={handleClose}, allowing incoming
props.close/props.cancel to override handleClose and break cleanup; update the
JSX so that {...props} is spread first on the Modal/AddSecretToWorkloadModal and
then explicitly pass close={handleClose} and cancel={handleClose} (referencing
AddSecretToWorkloadModal, Modal, handleClose) so handleClose (which calls
setIsOpen(false) and closeOverlay()) cannot be overwritten by incoming props.

) : null;
};

export const useAddSecretToWorkloadModalLauncher = (
props: AddSecretToWorkloadModalProps,
): ModalCallback => {
const launcher = useOverlay();
const launchModal = useOverlay();

return useCallback(
() => launcher<AddSecretToWorkloadModalProps>(AddSecretToWorkloadModalProvider, props),
[launcher, props],
);
return useCallback(() => {
// Move focus away from the triggering element to prevent aria-hidden warning
if (document.activeElement instanceof HTMLElement) {
document.activeElement.blur();
}
return launchModal<AddSecretToWorkloadModalProps>(AddSecretToWorkloadModalOverlay, props);
}, [launchModal, props]);
};

type WorkloadItem = {
Expand All @@ -303,10 +338,7 @@ type WorkloadItem = {
export type AddSecretToWorkloadModalProps = {
secretName: string;
namespace: string;
cancel?: () => void;
close?: () => void;
blocking?: boolean;
};
} & ModalComponentProps;

export type AddSecretToWorkloadModalState = {
inProgress: boolean;
Expand Down
Loading