From fa34e1b5d6aea69dff04339b197f76d996517fd4 Mon Sep 17 00:00:00 2001 From: Randy Bruno Piverger <21374229+Randy424@users.noreply.github.com> Date: Fri, 17 Apr 2026 17:55:23 -0700 Subject: [PATCH 01/16] Add wizard framework enhancements for placement workflow Add FooterContentProvider context, WizLabelSelect component, nonEditable and alertVariant support in WizCustomInputWrapper and ReviewStep, and useSetFooterContent hook export. Signed-off-by: Randy Bruno Piverger <21374229+Randy424@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 --- .../packages/react-form-wizard/src/Wizard.tsx | 52 +++++++++++-------- .../src/contexts/FooterContentProvider.tsx | 24 +++++++++ .../packages/react-form-wizard/src/index.ts | 1 + .../src/inputs/WizCustomWrapper.tsx | 22 +++++++- .../src/inputs/WizLabelSelect.tsx | 37 +++++++++++-- .../src/review/ReviewStep.tsx | 21 +++++++- .../src/review/ReviewStepContexts.tsx | 8 +++ 7 files changed, 135 insertions(+), 30 deletions(-) create mode 100644 frontend/packages/react-form-wizard/src/contexts/FooterContentProvider.tsx diff --git a/frontend/packages/react-form-wizard/src/Wizard.tsx b/frontend/packages/react-form-wizard/src/Wizard.tsx index 98e9d6d8f34..8a859d78da2 100644 --- a/frontend/packages/react-form-wizard/src/Wizard.tsx +++ b/frontend/packages/react-form-wizard/src/Wizard.tsx @@ -48,6 +48,7 @@ import { useStepShowValidation, } from './contexts/StepShowValidationProvider' import { StepValidationProvider, useStepHasValidationError } from './contexts/StepValidationProvider' +import { FooterContentProvider, useFooterContent } from './contexts/FooterContentProvider' import { defaultStrings, StringContext, useStringContext, WizardStrings } from './contexts/StringContext' import { EditorValidationStatus, @@ -107,29 +108,31 @@ export function Wizard(props: WizardProps & { showHeader?: boolean; showYaml?: b - - }> - - - - - {props.children} - - - - - - + + + }> + + + + + {props.children} + + + + + + + @@ -344,6 +347,8 @@ function MyFooter(props: WizardFooterProps) { nextButtonText, } = useStringContext() + const footerContent = useFooterContent() + if (isLastStep) { return (
@@ -405,6 +410,7 @@ function MyFooter(props: WizardFooterProps) { )} + {footerContent} diff --git a/frontend/packages/react-form-wizard/src/contexts/FooterContentProvider.tsx b/frontend/packages/react-form-wizard/src/contexts/FooterContentProvider.tsx new file mode 100644 index 00000000000..91f0ce02dae --- /dev/null +++ b/frontend/packages/react-form-wizard/src/contexts/FooterContentProvider.tsx @@ -0,0 +1,24 @@ +/* Copyright Contributors to the Open Cluster Management project */ +import { createContext, ReactNode, useCallback, useContext, useState } from 'react' + +const FooterContentContext = createContext(undefined) +FooterContentContext.displayName = 'FooterContentContext' + +const SetFooterContentContext = createContext<(content: ReactNode) => void>(() => null) +SetFooterContentContext.displayName = 'SetFooterContentContext' + +export const useFooterContent = () => useContext(FooterContentContext) +export const useSetFooterContent = () => useContext(SetFooterContentContext) + +export function FooterContentProvider(props: { children: ReactNode }) { + const [footerContent, setFooterContentState] = useState(undefined) + const setFooterContent = useCallback((content: ReactNode) => { + setFooterContentState(content) + }, []) + + return ( + + {props.children} + + ) +} diff --git a/frontend/packages/react-form-wizard/src/index.ts b/frontend/packages/react-form-wizard/src/index.ts index 8c07a4533af..f5a73b08e34 100644 --- a/frontend/packages/react-form-wizard/src/index.ts +++ b/frontend/packages/react-form-wizard/src/index.ts @@ -7,6 +7,7 @@ export * from './contexts/HasInputsProvider' export * from './contexts/HasValueProvider' export * from './contexts/ItemContext' export * from './contexts/DisplayModeContext' +export * from './contexts/FooterContentProvider' export * from './contexts/ValidationProvider' export * from './contexts/StringContext' export * from './review/ReviewStepContexts' diff --git a/frontend/packages/react-form-wizard/src/inputs/WizCustomWrapper.tsx b/frontend/packages/react-form-wizard/src/inputs/WizCustomWrapper.tsx index 6f892e14516..bf795769368 100644 --- a/frontend/packages/react-form-wizard/src/inputs/WizCustomWrapper.tsx +++ b/frontend/packages/react-form-wizard/src/inputs/WizCustomWrapper.tsx @@ -26,6 +26,10 @@ export type WizCustomWrapperInputProps = WizCustomWrapperBase & { id?: string label?: string value: ReactNode + /** When true, the review row omits the edit pen — used for computed / read-only values. */ + nonEditable?: boolean + /** When set, the review row renders as a PatternFly Alert instead of a description-list entry. */ + alertVariant?: 'info' | 'warning' | 'danger' | 'success' inputValueToPathValue?: (inputValue: unknown, pathValue: unknown) => unknown } @@ -42,6 +46,8 @@ export function WizCustomWrapper(props: WizCustomWrapperProps) { const isGroup = props.type === InputReviewMeta.GROUP const { path, id: idProp, label, children } = props const value = isGroup ? undefined : props.value + const nonEditable = isGroup ? undefined : props.nonEditable + const alertVariant = isGroup ? undefined : props.alertVariant const inputValueToPathValue = isGroup ? undefined : props.inputValueToPathValue const hidden = useInputHidden(props) @@ -82,11 +88,25 @@ export function WizCustomWrapper(props: WizCustomWrapperProps) { label, error: undefined, type: InputReviewMeta.INPUT, + nonEditable, + alertVariant, }) } bumpReviewDomTree?.() return () => stepInputsRegistry.unregister(id) - }, [stepInputsRegistry, currentStepId, hidden, id, registrationPath, value, label, bumpReviewDomTree, isGroup]) + }, [ + stepInputsRegistry, + currentStepId, + hidden, + id, + registrationPath, + value, + label, + nonEditable, + alertVariant, + bumpReviewDomTree, + isGroup, + ]) return
{children}
} diff --git a/frontend/packages/react-form-wizard/src/inputs/WizLabelSelect.tsx b/frontend/packages/react-form-wizard/src/inputs/WizLabelSelect.tsx index 3eb21086e2f..243c286b0db 100644 --- a/frontend/packages/react-form-wizard/src/inputs/WizLabelSelect.tsx +++ b/frontend/packages/react-form-wizard/src/inputs/WizLabelSelect.tsx @@ -1,6 +1,15 @@ /* Copyright Contributors to the Open Cluster Management project */ -import { Label, MenuToggle, MenuToggleElement, Select as PfSelect } from '@patternfly/react-core' +import { + DescriptionListDescription, + DescriptionListGroup, + DescriptionListTerm, + Label, + MenuToggle, + MenuToggleElement, + Select as PfSelect, +} from '@patternfly/react-core' import { ReactNode, useCallback, useEffect, useMemo, useState } from 'react' +import { DisplayMode } from '../contexts/DisplayModeContext' import { useStringContext } from '../contexts/StringContext' import { getSelectPlaceholder, InputCommonProps, useInput } from './Input' import { InputSelect, SelectListOptions } from './InputSelect' @@ -28,9 +37,9 @@ export type WizLabelSelectProps = InputCommonProps & { * When a value is selected, shows a simple toggle with the Label pill. */ export function WizLabelSelect(props: WizLabelSelectProps) { - const { value, setValue, validated, hidden, id, disabled, required } = useInput(props) + const { displayMode: mode, value, setValue, validated, hidden, id, disabled, required } = useInput(props) const { noResults } = useStringContext() - const { readonly, isCreatable, footer } = props + const { label, readonly, isCreatable, footer } = props const placeholder = getSelectPlaceholder(props) const [open, setOpen] = useState(false) const [filteredOptions, setFilteredOptions] = useState([]) @@ -100,6 +109,18 @@ export function WizLabelSelect(props: WizLabelSelectProps) { if (hidden) return null + if (mode === DisplayMode.Details) { + if (!value) return null + return ( + + {label} + + + + + ) + } + const toggle = (toggleRef: React.Ref) => value ? ( - + ) : ( - {onReviewEdit != null ? ( + {onReviewEdit != null && !inputNode.nonEditable ? ( + ) + precedingDlGroup = false + i++ + continue + } const run: WizardInputDomNode[] = [] - while (i < nodes.length && isReviewInputNode(nodes[i]!)) { + while (i < nodes.length && isReviewInputNode(nodes[i]!) && !(nodes[i] as WizardInputDomNode).alertVariant) { run.push(nodes[i] as WizardInputDomNode) i++ } diff --git a/frontend/packages/react-form-wizard/src/review/ReviewStepContexts.tsx b/frontend/packages/react-form-wizard/src/review/ReviewStepContexts.tsx index ead50d48d35..a399a430a3d 100644 --- a/frontend/packages/react-form-wizard/src/review/ReviewStepContexts.tsx +++ b/frontend/packages/react-form-wizard/src/review/ReviewStepContexts.tsx @@ -29,6 +29,10 @@ export type InputReviewStepMeta = type: InputReviewMeta.INPUT /** Nearest enclosing wizard step `id` (set when building the review DOM tree). */ stepId?: string + /** When true, the review row omits the edit pen — used for computed / read-only values. */ + nonEditable?: boolean + /** When set, the review row renders as a PatternFly Alert instead of a description-list entry. */ + alertVariant?: 'info' | 'warning' | 'danger' | 'success' } | { id: string @@ -75,6 +79,10 @@ export type WizardDomTreeNode = | (Omit & { type: InputReviewMeta.INPUT stepId: string + /** When true, the review row omits the edit pen — used for computed / read-only values. */ + nonEditable?: boolean + /** When set, the review row renders as a PatternFly Alert instead of a description-list entry. */ + alertVariant?: 'info' | 'warning' | 'danger' | 'success' children?: WizardDomTreeNode[] }) | (Omit & { type: InputReviewMeta.ARRAY_INPUT; children?: WizardDomTreeNode[] }) From d16fbb1edd1866515a4f3017bf60f1200abd381a Mon Sep 17 00:00:00 2001 From: Randy Bruno Piverger <21374229+Randy424@users.noreply.github.com> Date: Fri, 17 Apr 2026 17:55:49 -0700 Subject: [PATCH 02/16] Add PlacementDebugServer backend proxy and frontend API client Add backend route proxying POST requests to the cluster-internal PlacementDebugServer API for live cluster matching. Add frontend request client, usePlacementDebug hook with debouncing and feature flag gating, and MatchedClustersModal component. Signed-off-by: Randy Bruno Piverger <21374229+Randy424@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 --- backend/src/app.ts | 2 + backend/src/routes/placementDebug.ts | 79 ++++++++++++ frontend/src/resources/managed-cluster.ts | 8 ++ frontend/src/resources/placement-debug.ts | 31 +++++ .../Placement/MatchedClustersModal.tsx | 118 ++++++++++++++++++ .../wizards/Placement/usePlacementDebug.ts | 104 +++++++++++++++ 6 files changed, 342 insertions(+) create mode 100644 backend/src/routes/placementDebug.ts create mode 100644 frontend/src/resources/placement-debug.ts create mode 100644 frontend/src/wizards/Placement/MatchedClustersModal.tsx create mode 100644 frontend/src/wizards/Placement/usePlacementDebug.ts diff --git a/backend/src/app.ts b/backend/src/app.ts index 975c0a35b69..99795f9fb16 100644 --- a/backend/src/app.ts +++ b/backend/src/app.ts @@ -25,6 +25,7 @@ import { operatorCheck } from './routes/operatorCheck' import { proxy } from './routes/proxy' import { readiness } from './routes/readiness' import { search } from './routes/search' +import { placementDebug } from './routes/placementDebug' import { serveHandler } from './routes/serve' import { upgradeRiskPredictions } from './routes/upgrade-risks-prediction' import { username } from './routes/username' @@ -66,6 +67,7 @@ if (eventsEnabled) { router.get('/events', events) } router.post('/proxy/search', search) +router.post('/placement-debug', placementDebug) router.get('/authenticated', authenticated) router.post('/ansibletower', ansibleTower) router.get('/username', username) diff --git a/backend/src/routes/placementDebug.ts b/backend/src/routes/placementDebug.ts new file mode 100644 index 00000000000..cafd0318bb1 --- /dev/null +++ b/backend/src/routes/placementDebug.ts @@ -0,0 +1,79 @@ +/* Copyright Contributors to the Open Cluster Management project */ +import type { Http2ServerRequest, Http2ServerResponse, OutgoingHttpHeaders } from 'node:http2' +import { constants } from 'node:http2' +import type { RequestOptions } from 'node:https' +import { Agent, request } from 'node:https' +import { URL } from 'node:url' +import { getServiceAgent } from '../lib/agent' +import { logger } from '../lib/logger' +import { respondInternalServerError, unauthorized } from '../lib/respond' +import { getToken } from '../lib/token' + +const proxyHeaders = [constants.HTTP2_HEADER_ACCEPT, constants.HTTP2_HEADER_CONTENT_TYPE] + +const defaultServiceHost = 'cluster-manager-placement.open-cluster-management-hub.svc.cluster.local' +const defaultPlacementDebugUrl = `https://${defaultServiceHost}:9443/debug/placements/` + +let devAgent: Agent | undefined +function getDevAgent(): Agent { + if (!devAgent) { + devAgent = new Agent({ rejectUnauthorized: false, keepAlive: true }) + } + return devAgent +} + +function collectBody(req: Http2ServerRequest): Promise { + return new Promise((resolve, reject) => { + const chunks: Buffer[] = [] + req.on('data', (chunk: Buffer) => chunks.push(chunk)) + req.on('end', () => resolve(Buffer.concat(chunks))) + req.on('error', reject) + }) +} + +export async function placementDebug(req: Http2ServerRequest, res: Http2ServerResponse): Promise { + const body = await collectBody(req) + + const token = getToken(req) + if (!token) return unauthorized(req, res) + + const headers: OutgoingHttpHeaders = { + authorization: `Bearer ${token}`, + 'content-type': 'application/json', + 'content-length': body.length, + } + for (const header of proxyHeaders) { + if (req.headers[header]) headers[header] = req.headers[header] + } + + const overrideUrl = process.env.PLACEMENT_DEBUG_URL + const url = new URL(overrideUrl || defaultPlacementDebugUrl) + headers.host = defaultServiceHost + + const options: RequestOptions = { + protocol: url.protocol, + hostname: url.hostname, + port: url.port, + path: url.pathname, + method: 'POST', + headers, + agent: overrideUrl ? getDevAgent() : getServiceAgent(), + servername: defaultServiceHost, + } + + const upstream = request(options, (response) => { + if (!response) return respondInternalServerError(req, res) + res.writeHead(response.statusCode ?? 500, { + 'content-type': 'application/json', + }) + response.pipe(res as unknown as NodeJS.WritableStream) + }) + + upstream.on('error', (err) => { + logger.error({ msg: 'placement debug upstream error', error: err.message }) + if (!res.headersSent) respondInternalServerError(req, res) + }) + + upstream.write(body) + upstream.end() +} diff --git a/frontend/src/resources/managed-cluster.ts b/frontend/src/resources/managed-cluster.ts index 2a7bb89f2b8..9084ff7d74f 100644 --- a/frontend/src/resources/managed-cluster.ts +++ b/frontend/src/resources/managed-cluster.ts @@ -23,6 +23,7 @@ export interface ManagedCluster extends IResource { hubAcceptsClient: boolean leaseDurationSeconds?: number managedClusterClientConfigs?: any[] + taints?: ManagedClusterTaint[] } status?: { allocatable: { @@ -41,6 +42,13 @@ export interface ManagedCluster extends IResource { } } +export interface ManagedClusterTaint { + key: string + value?: string + effect: 'NoSelect' | 'PreferNoSelect' | 'NoSelectIfNew' + timeAdded?: string +} + export const createManagedCluster = (data: { clusterName: string | undefined clusterLabels: Record diff --git a/frontend/src/resources/placement-debug.ts b/frontend/src/resources/placement-debug.ts new file mode 100644 index 00000000000..19df0b28d71 --- /dev/null +++ b/frontend/src/resources/placement-debug.ts @@ -0,0 +1,31 @@ +/* Copyright Contributors to the Open Cluster Management project */ +import { IPlacement, PlacementApiVersion, PlacementKind } from '../wizards/common/resources/IPlacement' +import { IRequestResult, postRequest } from './utils/resource-request' +import { getBackendUrl } from './utils/resource-request' + +export interface ClusterScore { + clusterName: string + score: number +} + +export interface PlacementDebugResult { + placement?: IPlacement + filteredPipelineResults?: Array<{ + name: string + filteredClusters: string[] + }> + prioritizeResults?: unknown + aggregatedScores?: ClusterScore[] + error?: string +} + +export function postPlacementDebug(placement: IPlacement): IRequestResult { + const url = getBackendUrl() + '/placement-debug' + const body = { + apiVersion: PlacementApiVersion, + kind: PlacementKind, + metadata: placement.metadata, + spec: placement.spec, + } + return postRequest(url, body) +} diff --git a/frontend/src/wizards/Placement/MatchedClustersModal.tsx b/frontend/src/wizards/Placement/MatchedClustersModal.tsx new file mode 100644 index 00000000000..869e671cd88 --- /dev/null +++ b/frontend/src/wizards/Placement/MatchedClustersModal.tsx @@ -0,0 +1,118 @@ +/* Copyright Contributors to the Open Cluster Management project */ +import { Button, Modal, ModalBody, ModalHeader, ModalVariant, SearchInput, Tooltip } from '@patternfly/react-core' +import { OutlinedQuestionCircleIcon } from '@patternfly/react-icons' +import { useState, useMemo } from 'react' +import { useTranslation } from '../../lib/acm-i18next' + +export interface MatchedClustersModalProps { + isOpen: boolean + onClose: () => void + matchedClusters: string[] + notMatchedClusters: string[] + totalClusters: number +} + +export function MatchedClustersModal(props: MatchedClustersModalProps) { + const { t } = useTranslation() + const [searchTerm, setSearchTerm] = useState('') + + const filteredMatched = useMemo(() => { + if (!searchTerm) return props.matchedClusters + const lower = searchTerm.toLowerCase() + return props.matchedClusters.filter((name) => name.toLowerCase().includes(lower)) + }, [props.matchedClusters, searchTerm]) + + const filteredNotMatched = useMemo(() => { + if (!searchTerm) return props.notMatchedClusters + const lower = searchTerm.toLowerCase() + return props.notMatchedClusters.filter((name) => name.toLowerCase().includes(lower)) + }, [props.notMatchedClusters, searchTerm]) + + const hasLimit = props.notMatchedClusters.length > 0 + const title = hasLimit + ? t('{{matched}} of {{total}} clusters matched', { + matched: props.matchedClusters.length, + total: props.totalClusters, + }) + : t('{{count}} clusters matched', { count: props.matchedClusters.length }) + + const rowStyle = { + padding: '0.75rem 1rem', + backgroundColor: 'var(--pf-t--global--background--color--secondary--default)', + borderBottom: '1px solid var(--pf-t--global--border--color--default)', + } + + return ( + + + +
+

+ {t('Showing clusters that match your defined labels, tolerations, and limits.')}{' '} + +

+ + setSearchTerm(value)} + onClear={() => setSearchTerm('')} + /> + +
+ {hasLimit && filteredMatched.length > 0 && ( +
+

+ {t('Matched')}{' '} + + + +

+ {filteredMatched.map((name) => ( +
+ {name} +
+ ))} +
+ )} + + {hasLimit && filteredNotMatched.length > 0 && ( +
+

+ {t('Not matched')}{' '} + + + +

+ {filteredNotMatched.map((name) => ( +
+ {name} +
+ ))} +
+ )} + + {!hasLimit && + filteredMatched.map((name) => ( +
+ {name} +
+ ))} + + {filteredMatched.length === 0 && filteredNotMatched.length === 0 && ( +
+ {searchTerm ? t('No clusters found matching "{{search}}"', { search: searchTerm }) : t('No clusters')} +
+ )} +
+
+ + + ) +} diff --git a/frontend/src/wizards/Placement/usePlacementDebug.ts b/frontend/src/wizards/Placement/usePlacementDebug.ts new file mode 100644 index 00000000000..a345b194dc9 --- /dev/null +++ b/frontend/src/wizards/Placement/usePlacementDebug.ts @@ -0,0 +1,104 @@ +/* Copyright Contributors to the Open Cluster Management project */ +import { useEffect, useRef, useState } from 'react' +import debounce from 'debounce' +import { IPlacement } from '../common/resources/IPlacement' +import { postPlacementDebug, PlacementDebugResult } from '../../resources/placement-debug' +import { isRequestAbortedError } from '../../resources/utils/resource-request' + +export interface PlacementDebugState { + matched: string[] + notMatched: string[] + totalClusters: number + matchedCount: number | undefined + loading: boolean + error: Error | undefined +} + +const EMPTY_STATE: PlacementDebugState = { + matched: [], + notMatched: [], + totalClusters: 0, + matchedCount: undefined, + loading: false, + error: undefined, +} + +function mapDebugResult(result: PlacementDebugResult): PlacementDebugState { + if (result.error) { + return { ...EMPTY_STATE, error: new Error(result.error) } + } + + const allMatched = (result.aggregatedScores ?? []).map((s) => s.clusterName) + const limit = result.placement?.spec?.numberOfClusters + const matched = limit !== undefined && limit >= 0 ? allMatched.slice(0, limit) : allMatched + const matchedSet = new Set(matched) + + const notMatched: string[] = [] + for (const clusterName of allMatched) { + if (!matchedSet.has(clusterName)) { + notMatched.push(clusterName) + } + } + for (const pipeline of result.filteredPipelineResults ?? []) { + for (const clusterName of pipeline.filteredClusters) { + if (!matchedSet.has(clusterName) && !notMatched.includes(clusterName)) { + notMatched.push(clusterName) + } + } + } + + return { + matched, + notMatched, + totalClusters: matched.length + notMatched.length, + matchedCount: matched.length, + loading: false, + error: undefined, + } +} + +export function usePlacementDebug(placement: IPlacement | undefined, enabled = true): PlacementDebugState { + const [state, setState] = useState(EMPTY_STATE) + const abortRef = useRef<(() => void) | undefined>(undefined) + + // Serialize the full placement to detect in-place mutations from `set-value`. + // No namespace guard — the server validates; the hook re-fetches when namespace appears. + const specKey = placement ? JSON.stringify({ metadata: placement.metadata, spec: placement.spec }) : undefined + + const debouncedFetchRef = useRef( + debounce((p: IPlacement) => { + abortRef.current?.() + setState((prev) => ({ ...prev, loading: true, error: undefined })) + + const { promise, abort } = postPlacementDebug(p) + abortRef.current = abort + + promise + .then((result) => { + setState(mapDebugResult(result)) + }) + .catch((err: unknown) => { + if (isRequestAbortedError(err)) return + setState({ ...EMPTY_STATE, error: err instanceof Error ? err : new Error(String(err)) }) + }) + }, 500) + ) + + useEffect(() => { + const debouncedFetch = debouncedFetchRef.current + if (!enabled || !specKey || !placement) { + setState(EMPTY_STATE) + return + } + + setState((prev) => ({ ...prev, loading: true, matchedCount: undefined })) + debouncedFetch(placement) + + return () => { + debouncedFetch.clear() + abortRef.current?.() + } + }, [specKey, enabled]) // eslint-disable-line react-hooks/exhaustive-deps + + return state +} From c7614b9a73a94258e6836c3081d64616a833c2e3 Mon Sep 17 00:00:00 2001 From: Randy Bruno Piverger <21374229+Randy424@users.noreply.github.com> Date: Fri, 17 Apr 2026 17:56:11 -0700 Subject: [PATCH 03/16] Integrate PlacementDebugServer into placement wizard UI Wire usePlacementDebug hook into Placement and PlacementSection components behind the enhancedPlacement feature flag. Add matched cluster count in wizard footer, review step alert, expandable label expression and toleration sections, and WizLabelSelect integration in MatchExpression. Signed-off-by: Randy Bruno Piverger <21374229+Randy424@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 --- frontend/public/locales/en/translation.json | 17 +++ .../src/wizards/Placement/MatchExpression.tsx | 6 + frontend/src/wizards/Placement/Placement.tsx | 89 +++++++++++- .../wizards/Placement/PlacementSection.tsx | 135 +++++++++++++++++- 4 files changed, 242 insertions(+), 5 deletions(-) diff --git a/frontend/public/locales/en/translation.json b/frontend/public/locales/en/translation.json index cbaf6a54e2d..86c557913f2 100644 --- a/frontend/public/locales/en/translation.json +++ b/frontend/public/locales/en/translation.json @@ -12,6 +12,8 @@ "{{action}} {{itemKind}}?": "{{action}} {{itemKind}}?", "{{count}} cluster cannot be edited ": "{{count}} cluster cannot be edited ", "{{count}} cluster cannot be edited _plural": "{{count}} clusters cannot be edited ", + "{{count}} clusters matched": "{{count}} cluster matched", + "{{count}} clusters matched_plural": "{{count}} clusters matched", "{{count}} clusters with unknown status": "{{count}} cluster with unknown status", "{{count}} clusters with unknown status_plural": "{{count}} clusters with unknown status", "{{count}} clusters with unsatisfied dependencies": "{{count}} cluster with unsatisfied dependencies", @@ -26,9 +28,14 @@ "{{count}} labels_plural": "{{count}} labels", "{{count}} more": "{{count}} more", "{{count}} more_plural": "{{count}} more", + "{{count}} of {{total}} clusters matched by placement": "{{count}} of {{total}} clusters matched by placement", + "{{count}} of {{total}} clusters matched by placement_plural": "{{count}} of {{total}} clusters matched by placement", "{{count}} selected": "{{count}} selected", "{{count}} selected_plural": "{{count}} selected", "{{kind}} details": "{{kind}} details", + "{{matched}} of {{total}} clusters": "{{matched}} of {{total}} clusters", + "{{matched}} of {{total}} clusters matched": "{{matched}} of {{total}} clusters matched", + "{{matched}} of {{total}} clusters matched by placement": "{{matched}} of {{total}} clusters matched by placement", "{{name}} group has been successfully added.": "{{name}} group has been successfully added.", "{{name}} group has been successfully created.": "{{name}} group has been successfully created.", "{{name}} has been updated.": "{{name}} has been updated.", @@ -800,6 +807,7 @@ "clusterPools": "Cluster pools", "Clusters": "Clusters", "clusters affected": "clusters affected", + "Clusters currently targeted for deployment.": "Clusters currently targeted for deployment.", "clusters need to be reviewed before updating": "clusters need to be reviewed before updating", "Clusters with pending policies": "Clusters with pending policies", "Clusters with policy violations": "Clusters with policy violations", @@ -1889,6 +1897,7 @@ "Kubernetes version": "Kubernetes version", "Label": "Label", "Label expressions": "Label expressions", + "Label expressions and tolerations": "Label expressions and tolerations", "Label selectors": "Label selectors", "Labels": "Labels", "Labels to match clusters by": "Labels to match clusters by", @@ -2034,6 +2043,9 @@ "Map infrastructure volume snapshot classes to guest cluster volume snapshot classes. These mappings cannot be changed after cluster creation.": "Map infrastructure volume snapshot classes to guest cluster volume snapshot classes. These mappings cannot be changed after cluster creation.", "Match clusters using label selectors. Multiple expressions are combined using AND logic (all inputs must be true).": "Match clusters using label selectors. Multiple expressions are combined using AND logic (all inputs must be true).", "Match labels": "Match labels", + "Matched": "Matched", + "Matched by Placement": "Matched by Placement", + "Matched by Placement:": "Matched by Placement:", "Matched Clusters": "Matched Clusters", "Matched on": "Matched on", "Maximum replicas must be greater than or equal to minimum replicas.": "Maximum replicas must be greater than or equal to minimum replicas.", @@ -2132,7 +2144,9 @@ "No clusters are reporting status for this policy.": "No clusters are reporting status for this policy.", "No clusters available": "No clusters available", "No clusters found": "No clusters found", + "No clusters found matching \"{{search}}\"": "No clusters found matching \"{{search}}\"", "No clusters in this cluster set have the Submariner add-on installed. To get started, install Submariner add-ons to install the add-on on any available clusters in this cluster set.": "No clusters in this cluster set have the Submariner add-on installed. To get started, install Submariner add-ons to install the add-on on any available clusters in this cluster set.", + "No clusters match the current placement criteria. To identify available clusters, check your label expressions, tolerations, or limits.": "No clusters match the current placement criteria. To identify available clusters, check your label expressions, tolerations, or limits.", "No clusters selection to create projects for": "No clusters selection to create projects for", "No common projects found": "No common projects found", "No conditions": "No conditions", @@ -2230,6 +2244,7 @@ "Not found": "Not found", "Not implemented": "Not implemented", "Not mapped": "Not mapped", + "Not matched": "Not matched", "Not ready": "Not ready", "Not selected": "Not selected", "Not selected by placement": "Not selected by placement", @@ -2780,6 +2795,7 @@ "Show workloads running on your fleet": "Show workloads running on your fleet", "show.more": "{{count}} more", "show.more_plural": "{{count}} more", + "Showing clusters that match your defined labels, tolerations, and limits.": "Showing clusters that match your defined labels, tolerations, and limits.", "Simple Table": "Simple Table", "Single cluster": "Single cluster", "Single node OpenShift": "Single node OpenShift", @@ -3258,6 +3274,7 @@ "There is no history for the policy template on this cluster.": "There is no history for the policy template on this cluster.", "There is not enough information in the subscription to retrieve deployed objects data.": "There is not enough information in the subscription to retrieve deployed objects data.", "there.were.errors": "There were errors processing the requests", + "These clusters match your label expressions and tolerations, but are not currently assigned due to placement limit or prioritization.": "These clusters match your label expressions and tolerations, but are not currently assigned due to placement limit or prioritization.", "These examples show different ways to scope role assignments.": "These examples show different ways to scope role assignments.", "This action removes the automation template from the following list of clusters. Only clusters that have an automation template are listed.": "This action removes the automation template from the following list of clusters. Only clusters that have an automation template are listed.", "This application has no matched subscription. Make sure the subscription match selector spec.selector.matchExpressions exists and matches a Subscription resource created in the {{0}} namespace.": "This application has no matched subscription. Make sure the subscription match selector spec.selector.matchExpressions exists and matches a Subscription resource created in the {{0}} namespace.", diff --git a/frontend/src/wizards/Placement/MatchExpression.tsx b/frontend/src/wizards/Placement/MatchExpression.tsx index 99f7b3811dd..e8b5f0b0298 100644 --- a/frontend/src/wizards/Placement/MatchExpression.tsx +++ b/frontend/src/wizards/Placement/MatchExpression.tsx @@ -1,5 +1,6 @@ /* Copyright Contributors to the Open Cluster Management project */ import { Flex, Label } from '@patternfly/react-core' +import { Fragment } from 'react' import set from 'set-value' import { ItemContext, @@ -8,6 +9,8 @@ import { WizMultiSelect, WizStringsInput, WizTextInput, + DisplayMode, + useDisplayMode, } from '@patternfly-labs/react-form-wizard' import { IExpression } from '../common/resources/IMatchExpression' import { useTranslation } from '../../lib/acm-i18next' @@ -121,7 +124,10 @@ export function MatchExpressionSummary(props: { expression: IExpression }) { break } + const displayMode = useDisplayMode() + if (!expression?.key) { + if (displayMode === DisplayMode.Details) return return
{t('Expand to enter expression')}
} diff --git a/frontend/src/wizards/Placement/Placement.tsx b/frontend/src/wizards/Placement/Placement.tsx index eaf6398243e..0fae70f7f11 100644 --- a/frontend/src/wizards/Placement/Placement.tsx +++ b/frontend/src/wizards/Placement/Placement.tsx @@ -1,21 +1,25 @@ /* Copyright Contributors to the Open Cluster Management project */ import { + DisplayMode, EditMode, useData, + useDisplayMode, useEditMode, useItem, + useSetFooterContent, WizArrayInput, WizCheckbox, + WizCustomWrapper, WizKeyValue, + WizLabelSelect, WizMultiSelect, WizNumberInput, WizTextInput, - WizLabelSelect, } from '@patternfly-labs/react-form-wizard' -import { Button, Divider, ExpandableSection, Label } from '@patternfly/react-core' +import { Alert, Button, ButtonVariant, Divider, ExpandableSection, Label, Tooltip } from '@patternfly/react-core' import { ExternalLinkAltIcon } from '@patternfly/react-icons' import get from 'get-value' -import { Fragment, ReactNode, useMemo, useState } from 'react' +import { Fragment, ReactNode, useCallback, useEffect, useMemo, useState } from 'react' import set from 'set-value' import { useTranslation } from '../../lib/acm-i18next' import { useValidation } from '../../hooks/useValidation' @@ -24,6 +28,8 @@ import { IPlacement, PlacementKind, PlacementType, Predicate, Toleration } from import { IResource } from '../common/resources/IResource' import { useLabelValuesMap } from '../common/useLabelValuesMap' import { MatchExpression, MatchExpressionCollapsed, MatchExpressionSummary } from './MatchExpression' +import { MatchedClustersModal } from './MatchedClustersModal' +import { PlacementDebugState, usePlacementDebug } from './usePlacementDebug' function TolerationCollapsed() { const toleration = useItem() as Toleration @@ -40,6 +46,7 @@ export function Placements(props: { clusters: IResource[] createClusterSetCallback?: () => void alertTitle?: string + showPlacementPreview?: boolean }) { const editMode = useEditMode() const resources = useItem() as IResource[] @@ -82,6 +89,7 @@ export function Placements(props: { clusters={props.clusters} createClusterSetCallback={props.createClusterSetCallback} alertTitle={props.alertTitle} + showPlacementPreview={props.showPlacementPreview} /> ) @@ -94,17 +102,82 @@ export function Placement(props: { createClusterSetCallback?: () => void alertTitle?: string alertContent?: ReactNode + showPlacementPreview?: boolean + placementDebugState?: PlacementDebugState }) { const placement = useItem() as IPlacement const editMode = useEditMode() + const displayMode = useDisplayMode() const { update } = useData() + const [isMatchedClustersModalOpen, setIsMatchedClustersModalOpen] = useState(false) const [isTolerationsExpanded, setIsTolerationsExpanded] = useState(true) + const featureEnabled = props.showPlacementPreview === true + const ownsDebugUI = featureEnabled && !props.placementDebugState + const ownDebugState = usePlacementDebug(placement, ownsDebugUI) + const { matched, notMatched, totalClusters, matchedCount, error } = props.placementDebugState ?? ownDebugState const { t } = useTranslation() const { validateKubernetesResourceName } = useValidation() + const matchedLabel = + matchedCount === undefined + ? '-' + : t('{{matched}} of {{total}} clusters', { matched: matchedCount, total: totalClusters }) + + const setFooterContent = useSetFooterContent() + const openMatchedModal = useCallback(() => setIsMatchedClustersModalOpen(true), []) + + useEffect(() => { + if (!ownsDebugUI) return + if (displayMode === DisplayMode.Step) { + setFooterContent( +
+ {t('Matched by Placement')}:{' '} + {error ? ( + + + + ) : ( + + )} +
+ ) + } else { + setFooterContent(undefined) + } + return () => setFooterContent(undefined) + }, [ownsDebugUI, displayMode, matchedLabel, error, setFooterContent, openMatchedModal, t]) + return ( + {featureEnabled && ( + 0 + ? t('{{matched}} of {{total}} clusters matched by placement', { + matched: matchedCount, + total: totalClusters, + }) + : t( + 'No clusters match the current placement criteria. To identify available clusters, check your label expressions, tolerations, or limits.' + ) + } + nonEditable + alertVariant={ + error ? 'warning' : matchedCount === undefined ? undefined : matchedCount > 0 ? 'info' : 'warning' + } + > + + + )} {!props.hideName && ( + + {ownsDebugUI && ( + setIsMatchedClustersModalOpen(false)} + matchedClusters={matched} + notMatchedClusters={notMatched} + totalClusters={totalClusters} + /> + )} ) } diff --git a/frontend/src/wizards/Placement/PlacementSection.tsx b/frontend/src/wizards/Placement/PlacementSection.tsx index 9567e4bae37..b2017777b69 100644 --- a/frontend/src/wizards/Placement/PlacementSection.tsx +++ b/frontend/src/wizards/Placement/PlacementSection.tsx @@ -1,6 +1,6 @@ /* Copyright Contributors to the Open Cluster Management project */ -import { Button, ToggleGroup, ToggleGroupItem } from '@patternfly/react-core' -import { Fragment, useEffect, useMemo, useState } from 'react' +import { Alert, Button, ButtonVariant, Label, LabelGroup, ToggleGroup, ToggleGroupItem } from '@patternfly/react-core' +import { Fragment, useCallback, useEffect, useMemo, useState } from 'react' import { WizDetailsHidden, EditMode, @@ -13,6 +13,9 @@ import { useItem, useValidate, Sync, + useSetFooterContent, + DisplayMode, + useDisplayMode, } from '@patternfly-labs/react-form-wizard' import { IResource } from '../common/resources/IResource' @@ -30,6 +33,9 @@ import { Placement, Placements } from './Placement' import { PlacementBindings } from './PlacementBinding' import { useTranslation } from '../../lib/acm-i18next' import { NavigationPath } from '../../NavigationPath' +import { usePlacementDebug } from './usePlacementDebug' +import { MatchedClustersModal } from './MatchedClustersModal' +import { useRecoilValue, useSharedAtoms } from '../../shared-recoil' export function PlacementSection(props: { bindingSubjectKind: string @@ -47,6 +53,10 @@ export function PlacementSection(props: { const { update } = useData() const resources = useItem() as IResource[] const editMode = useEditMode() + const displayMode = useDisplayMode() + const { settingsState } = useSharedAtoms() + const settings = useRecoilValue(settingsState) + const [isMatchedClustersModalOpen, setIsMatchedClustersModalOpen] = useState(false) const [placementCount, setPlacementCount] = useState(0) const [placementBindingCount, setPlacementBindingCount] = useState(0) @@ -141,6 +151,52 @@ export function PlacementSection(props: { setHasInputs() }, [setHasInputs]) + // Calculate matched clusters for the current placement + const currentPlacement = useMemo(() => { + return resources?.find((resource) => resource.kind === PlacementKind) as IPlacement | undefined + }, [resources]) + + const { matched, notMatched, matchedCount, totalClusters } = usePlacementDebug(currentPlacement) + + const setFooterContent = useSetFooterContent() + const openMatchedModal = useCallback(() => setIsMatchedClustersModalOpen(true), []) + + useEffect(() => { + if ( + settings.enhancedPlacement === 'enabled' && + placementCount === 1 && + currentPlacement && + displayMode !== DisplayMode.Details + ) { + const matchedLabel = + matchedCount === undefined + ? '-' + : t('{{matched}} of {{total}} clusters', { matched: matchedCount, total: totalClusters }) + + setFooterContent( +
+ {t('Matched by Placement:')}{' '} + +
+ ) + } else { + setFooterContent(undefined) + } + return () => setFooterContent(undefined) + }, [ + settings.enhancedPlacement, + placementCount, + currentPlacement, + displayMode, + matchedCount, + totalClusters, + setFooterContent, + openMatchedModal, + t, + ]) + if (isAdvanced) { return ( @@ -211,6 +267,7 @@ export function PlacementSection(props: { {t('Add cluster set')} } + useFeatureFlag={settings.enhancedPlacement === 'enabled'} /> @@ -226,6 +283,80 @@ export function PlacementSection(props: { /> )} + + {/* Review step content */} + {settings.enhancedPlacement === 'enabled' && + displayMode === DisplayMode.Details && + placementCount === 1 && + currentPlacement && ( +
+ {/* Placement info alert */} + {matchedCount !== undefined && totalClusters > 0 && ( + + )} + + {/* Label expressions and tolerations */} + {(currentPlacement.spec?.predicates?.[0]?.requiredClusterSelector?.labelSelector?.matchExpressions + ?.length || + currentPlacement.spec?.tolerations?.length) && ( +
+

{t('Label expressions and tolerations')}

+
+ {/* Label expressions */} + {currentPlacement.spec?.predicates?.[0]?.requiredClusterSelector?.labelSelector?.matchExpressions + ?.length && ( +
+ {t('Label expressions')}: + + {currentPlacement.spec.predicates[0].requiredClusterSelector.labelSelector.matchExpressions.map( + (expr, idx) => ( + + + {expr.values && expr.values.length > 0 && } + + ) + )} + +
+ )} + + {/* Tolerations */} + {currentPlacement.spec?.tolerations?.length && ( +
+ {t('Tolerations')}: + + {currentPlacement.spec.tolerations.map((toleration, idx) => ( + + + + {toleration.value && } + {toleration.effect && } + {toleration.tolerationSeconds && } + + ))} + +
+ )} +
+
+ )} +
+ )} + + setIsMatchedClustersModalOpen(false)} + matchedClusters={matched} + notMatchedClusters={notMatched} + totalClusters={totalClusters} + /> ) } From 33b846f270e4445842f25dd055df2d6997b9a38b Mon Sep 17 00:00:00 2001 From: Randy Bruno Piverger <21374229+Randy424@users.noreply.github.com> Date: Fri, 17 Apr 2026 19:01:09 -0700 Subject: [PATCH 04/16] Address CodeRabbit review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add 1MB body size limit and HTTP 413 response in placementDebug - Move auth check before body buffering to prevent unauthenticated memory consumption - Initialize and repopulate filteredOptions in WizLabelSelect on menu open - Honor nonEditable flag in collapsed review badges - Remove dead "Learn more" button from MatchedClustersModal - Normalize useFeatureFlag to strict boolean, gate footer content - Fix toleration helper text (NoSchedule → NoSelect), remove empty string from effect options - Gate usePlacementDebug in PlacementSection behind feature flag - Translate 'Exists' fallback label Signed-off-by: Randy Bruno Piverger <21374229+Randy424@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 --- backend/src/routes/placementDebug.ts | 26 ++++++++++++++++--- .../src/review/ReviewStep.tsx | 5 +++- .../Placement/MatchedClustersModal.tsx | 9 ++----- .../wizards/Placement/PlacementSection.tsx | 7 +++-- 4 files changed, 34 insertions(+), 13 deletions(-) diff --git a/backend/src/routes/placementDebug.ts b/backend/src/routes/placementDebug.ts index cafd0318bb1..6aa9d33246a 100644 --- a/backend/src/routes/placementDebug.ts +++ b/backend/src/routes/placementDebug.ts @@ -22,21 +22,41 @@ function getDevAgent(): Agent { return devAgent } +const MAX_BODY_SIZE = 1024 * 1024 // 1MB + function collectBody(req: Http2ServerRequest): Promise { return new Promise((resolve, reject) => { const chunks: Buffer[] = [] - req.on('data', (chunk: Buffer) => chunks.push(chunk)) + let size = 0 + req.on('data', (chunk: Buffer) => { + size += chunk.length + if (size > MAX_BODY_SIZE) { + req.destroy() + reject(new Error('Request body too large')) + return + } + chunks.push(chunk) + }) req.on('end', () => resolve(Buffer.concat(chunks))) req.on('error', reject) }) } export async function placementDebug(req: Http2ServerRequest, res: Http2ServerResponse): Promise { - const body = await collectBody(req) - const token = getToken(req) if (!token) return unauthorized(req, res) + let body: Buffer + try { + body = await collectBody(req) + } catch (err) { + if (!res.headersSent) { + res.writeHead(413, { 'content-type': 'application/json' }) + res.end(JSON.stringify({ error: 'Request body too large' })) + } + return + } + const headers: OutgoingHttpHeaders = { authorization: `Bearer ${token}`, 'content-type': 'application/json', diff --git a/frontend/packages/react-form-wizard/src/review/ReviewStep.tsx b/frontend/packages/react-form-wizard/src/review/ReviewStep.tsx index 54833cff269..92de28bf421 100644 --- a/frontend/packages/react-form-wizard/src/review/ReviewStep.tsx +++ b/frontend/packages/react-form-wizard/src/review/ReviewStep.tsx @@ -345,7 +345,10 @@ function ReviewCollapsedValueBadge(props: { showYaml?: boolean }) { const { content, error, inputNode, onReviewEdit, showYaml } = props - const editable = onReviewEdit != null && inputNode != null + const editable = + onReviewEdit != null && + inputNode != null && + !('nonEditable' in inputNode && inputNode.nonEditable) const yamlVisible = showYaml !== false const activateEdit = () => { if (inputNode != null && onReviewEdit != null) { diff --git a/frontend/src/wizards/Placement/MatchedClustersModal.tsx b/frontend/src/wizards/Placement/MatchedClustersModal.tsx index 869e671cd88..0649cac6097 100644 --- a/frontend/src/wizards/Placement/MatchedClustersModal.tsx +++ b/frontend/src/wizards/Placement/MatchedClustersModal.tsx @@ -1,5 +1,5 @@ /* Copyright Contributors to the Open Cluster Management project */ -import { Button, Modal, ModalBody, ModalHeader, ModalVariant, SearchInput, Tooltip } from '@patternfly/react-core' +import { Modal, ModalBody, ModalHeader, ModalVariant, SearchInput, Tooltip } from '@patternfly/react-core' import { OutlinedQuestionCircleIcon } from '@patternfly/react-icons' import { useState, useMemo } from 'react' import { useTranslation } from '../../lib/acm-i18next' @@ -47,12 +47,7 @@ export function MatchedClustersModal(props: MatchedClustersModalProps) {
-

- {t('Showing clusters that match your defined labels, tolerations, and limits.')}{' '} - -

+

{t('Showing clusters that match your defined labels, tolerations, and limits.')}

resource.kind === PlacementKind) as IPlacement | undefined }, [resources]) - const { matched, notMatched, matchedCount, totalClusters } = usePlacementDebug(currentPlacement) + const { matched, notMatched, matchedCount, totalClusters } = usePlacementDebug( + currentPlacement, + settings.enhancedPlacement === 'enabled' + ) const setFooterContent = useSetFooterContent() const openMatchedModal = useCallback(() => setIsMatchedClustersModalOpen(true), []) @@ -335,7 +338,7 @@ export function PlacementSection(props: { {currentPlacement.spec.tolerations.map((toleration, idx) => ( - + {toleration.value && } {toleration.effect && } {toleration.tolerationSeconds && } From d8ee2364a042d1c937e478fe4054cef6887b8012 Mon Sep 17 00:00:00 2001 From: Randy Bruno Piverger <21374229+Randy424@users.noreply.github.com> Date: Fri, 17 Apr 2026 19:32:25 -0700 Subject: [PATCH 05/16] Wire feature flag and improve test coverage - Pass enhancedPlacement flag to all Placement consumers - Add RecoilRoot to wizard tests for settingsState dependency - Gate WizLabelSelect onClose when disabled/readonly - Fix tolerationSeconds=0 truthy check - Show zero-match warning in review alert - Use getAuthenticatedToken in backend proxy route - Add MatchedClustersModal, usePlacementDebug, and backend tests Signed-off-by: Randy Bruno Piverger <21374229+Randy424@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 --- backend/src/routes/placementDebug.ts | 16 +- backend/test/routes/placementDebug.test.ts | 62 +++++ .../src/inputs/WizLabelSelect.tsx | 12 +- .../src/review/ReviewStep.tsx | 5 +- frontend/public/locales/en/translation.json | 1 + .../src/resources/placement-debug.test.ts | 35 +++ frontend/src/wizards/Argo/ArgoWizard.tsx | 5 +- .../Governance/Policy/policyWizard.test.tsx | 93 +++---- .../PolicySet/PolicySetWizard.test.tsx | 29 ++- .../Placement/MatchedClustersModal.test.tsx | 101 ++++++++ .../wizards/Placement/PlacementSection.tsx | 40 +-- .../Placement/usePlacementDebug.test.ts | 227 ++++++++++++++++++ .../wizards/Placement/usePlacementDebug.ts | 2 +- 13 files changed, 543 insertions(+), 85 deletions(-) create mode 100644 backend/test/routes/placementDebug.test.ts create mode 100644 frontend/src/resources/placement-debug.test.ts create mode 100644 frontend/src/wizards/Placement/MatchedClustersModal.test.tsx create mode 100644 frontend/src/wizards/Placement/usePlacementDebug.test.ts diff --git a/backend/src/routes/placementDebug.ts b/backend/src/routes/placementDebug.ts index 6aa9d33246a..03c64643fc3 100644 --- a/backend/src/routes/placementDebug.ts +++ b/backend/src/routes/placementDebug.ts @@ -6,10 +6,16 @@ import { Agent, request } from 'node:https' import { URL } from 'node:url' import { getServiceAgent } from '../lib/agent' import { logger } from '../lib/logger' -import { respondInternalServerError, unauthorized } from '../lib/respond' -import { getToken } from '../lib/token' +import { respondInternalServerError } from '../lib/respond' +import { getAuthenticatedToken } from '../lib/token' -const proxyHeaders = [constants.HTTP2_HEADER_ACCEPT, constants.HTTP2_HEADER_CONTENT_TYPE] +const proxyHeaders = [ + constants.HTTP2_HEADER_ACCEPT, + constants.HTTP2_HEADER_ACCEPT_ENCODING, + constants.HTTP2_HEADER_CONTENT_ENCODING, + constants.HTTP2_HEADER_CONTENT_LENGTH, + constants.HTTP2_HEADER_CONTENT_TYPE, +] const defaultServiceHost = 'cluster-manager-placement.open-cluster-management-hub.svc.cluster.local' const defaultPlacementDebugUrl = `https://${defaultServiceHost}:9443/debug/placements/` @@ -43,8 +49,8 @@ function collectBody(req: Http2ServerRequest): Promise { } export async function placementDebug(req: Http2ServerRequest, res: Http2ServerResponse): Promise { - const token = getToken(req) - if (!token) return unauthorized(req, res) + const token = await getAuthenticatedToken(req, res) + if (!token) return let body: Buffer try { diff --git a/backend/test/routes/placementDebug.test.ts b/backend/test/routes/placementDebug.test.ts new file mode 100644 index 00000000000..0d466fb376a --- /dev/null +++ b/backend/test/routes/placementDebug.test.ts @@ -0,0 +1,62 @@ +/* Copyright Contributors to the Open Cluster Management project */ +import { request } from '../mock-request' +import nock from 'nock' + +const upstreamHost = 'https://cluster-manager-placement.open-cluster-management-hub.svc.cluster.local:9443' + +function nockAuth(status = 200) { + nock(process.env.CLUSTER_API_URL).get('/apis').reply(status, { status }) +} + +describe(`placementDebug Route`, function () { + it(`proxies placement debug request to upstream service`, async function () { + nockAuth() + nock(upstreamHost).post('/debug/placements/').reply(200, { aggregatedScores: [] }) + const res = await request('POST', '/placement-debug', { placement: 'test' }) + expect(res.statusCode).toEqual(200) + }) + + it(`handles upstream errors`, async function () { + nockAuth() + nock(upstreamHost).post('/debug/placements/').reply(500, { error: 'internal server error' }) + const res = await request('POST', '/placement-debug', { placement: 'test' }) + expect(res.statusCode).toEqual(500) + }) + + it(`rejects unauthenticated requests`, async function () { + nockAuth(401) + const res = await request('POST', '/placement-debug', { placement: 'test' }) + expect(res.statusCode).toEqual(401) + }) + + it(`uses dev agent when PLACEMENT_DEBUG_URL is set`, async function () { + const original = process.env.PLACEMENT_DEBUG_URL + process.env.PLACEMENT_DEBUG_URL = 'https://localhost:9443/debug/placements/' + try { + nockAuth() + nock('https://localhost:9443').post('/debug/placements/').reply(200, { aggregatedScores: [] }) + const res = await request('POST', '/placement-debug', { placement: 'test' }) + expect(res.statusCode).toEqual(200) + } finally { + if (original === undefined) { + delete process.env.PLACEMENT_DEBUG_URL + } else { + process.env.PLACEMENT_DEBUG_URL = original + } + } + }) + + it(`handles upstream connection errors`, async function () { + nockAuth() + nock(upstreamHost).post('/debug/placements/').replyWithError('ECONNREFUSED') + const res = await request('POST', '/placement-debug', { placement: 'test' }) + expect(res.statusCode).toEqual(500) + }) + + it(`rejects oversized request body`, async function () { + nockAuth() + const largeBody = { data: 'x'.repeat(1024 * 1024 + 1) } + const res = await request('POST', '/placement-debug', largeBody) + expect(res.statusCode).toEqual(413) + }) +}) diff --git a/frontend/packages/react-form-wizard/src/inputs/WizLabelSelect.tsx b/frontend/packages/react-form-wizard/src/inputs/WizLabelSelect.tsx index 243c286b0db..14552b0c330 100644 --- a/frontend/packages/react-form-wizard/src/inputs/WizLabelSelect.tsx +++ b/frontend/packages/react-form-wizard/src/inputs/WizLabelSelect.tsx @@ -135,10 +135,14 @@ export function WizLabelSelect(props: WizLabelSelectProps) { > diff --git a/frontend/packages/react-form-wizard/src/review/ReviewStep.tsx b/frontend/packages/react-form-wizard/src/review/ReviewStep.tsx index 92de28bf421..83259a9ee9b 100644 --- a/frontend/packages/react-form-wizard/src/review/ReviewStep.tsx +++ b/frontend/packages/react-form-wizard/src/review/ReviewStep.tsx @@ -345,10 +345,7 @@ function ReviewCollapsedValueBadge(props: { showYaml?: boolean }) { const { content, error, inputNode, onReviewEdit, showYaml } = props - const editable = - onReviewEdit != null && - inputNode != null && - !('nonEditable' in inputNode && inputNode.nonEditable) + const editable = onReviewEdit != null && inputNode != null && !('nonEditable' in inputNode && inputNode.nonEditable) const yamlVisible = showYaml !== false const activateEdit = () => { if (inputNode != null && onReviewEdit != null) { diff --git a/frontend/public/locales/en/translation.json b/frontend/public/locales/en/translation.json index 86c557913f2..6aa34ac4f4e 100644 --- a/frontend/public/locales/en/translation.json +++ b/frontend/public/locales/en/translation.json @@ -3471,6 +3471,7 @@ "Unable to communicate with the server because the network connection was reset.": "Unable to communicate with the server because the network connection was reset.", "Unable to communicate with the server because the service is unavailable.": "Unable to communicate with the server because the service is unavailable.", "Unable to communicate with the server due to a gateway timeout.": "Unable to communicate with the server due to a gateway timeout.", + "Unable to determine cluster matches.": "Unable to determine cluster matches.", "Unable to update the resource because of a resource conflict.": "Unable to update the resource because of a resource conflict.", "Unauthorized": "Unauthorized", "unavailable": "unavailable", diff --git a/frontend/src/resources/placement-debug.test.ts b/frontend/src/resources/placement-debug.test.ts new file mode 100644 index 00000000000..4a01fc20147 --- /dev/null +++ b/frontend/src/resources/placement-debug.test.ts @@ -0,0 +1,35 @@ +/* Copyright Contributors to the Open Cluster Management project */ +import { postPlacementDebug } from './placement-debug' +import { IPlacement } from '../wizards/common/resources/IPlacement' + +jest.mock('./utils/resource-request', () => ({ + postRequest: jest.fn((_url: string, body: unknown) => ({ + promise: Promise.resolve(body), + abort: jest.fn(), + })), + getBackendUrl: jest.fn(() => 'https://localhost'), +})) + +const { postRequest } = jest.requireMock('./utils/resource-request') as { postRequest: jest.Mock } + +describe('postPlacementDebug', () => { + it('sends placement metadata and spec to backend', () => { + const placement: IPlacement = { + apiVersion: 'cluster.open-cluster-management.io/v1beta1', + kind: 'Placement', + metadata: { name: 'test', namespace: 'default' }, + spec: { clusterSets: ['my-set'] }, + } + + const result = postPlacementDebug(placement) + + expect(postRequest).toHaveBeenCalledWith('https://localhost/placement-debug', { + apiVersion: 'cluster.open-cluster-management.io/v1beta1', + kind: 'Placement', + metadata: { name: 'test', namespace: 'default' }, + spec: { clusterSets: ['my-set'] }, + }) + expect(result).toHaveProperty('promise') + expect(result).toHaveProperty('abort') + }) +}) diff --git a/frontend/src/wizards/Argo/ArgoWizard.tsx b/frontend/src/wizards/Argo/ArgoWizard.tsx index 615a4c44920..629bdc53de3 100644 --- a/frontend/src/wizards/Argo/ArgoWizard.tsx +++ b/frontend/src/wizards/Argo/ArgoWizard.tsx @@ -30,7 +30,7 @@ import { useValidation } from '../../hooks/useValidation' import { useWizardStrings } from '../../lib/wizardStrings' import { NavigationPath } from '../../NavigationPath' import { ApplicationSetKind, GitOpsCluster, Secret } from '../../resources' -import { useSharedSelectors } from '../../shared-recoil' +import { useRecoilValue, useSharedAtoms, useSharedSelectors } from '../../shared-recoil' import { IClusterSetBinding } from '../common/resources/IClusterSetBinding' import { IPlacement, PlacementApiVersion, PlacementKind, PlacementType } from '../common/resources/IPlacement' import { IResource } from '../common/resources/IResource' @@ -871,6 +871,8 @@ function ArgoWizardPlacementSection(props: { isPullModel?: boolean }) { const { t } = useTranslation() + const { settingsState } = useSharedAtoms() + const settings = useRecoilValue(settingsState) const resources = useItem() as IResource[] const editMode = useEditMode() const hasPlacement = resources.find((r) => r.kind === PlacementKind) !== undefined @@ -991,6 +993,7 @@ function ArgoWizardPlacementSection(props: { {t('Add cluster set')} } + useFeatureFlag={settings.enhancedPlacement === 'enabled'} /> ) : ( diff --git a/frontend/src/wizards/Governance/Policy/policyWizard.test.tsx b/frontend/src/wizards/Governance/Policy/policyWizard.test.tsx index 9c8e710bd52..64657d2b452 100644 --- a/frontend/src/wizards/Governance/Policy/policyWizard.test.tsx +++ b/frontend/src/wizards/Governance/Policy/policyWizard.test.tsx @@ -12,6 +12,7 @@ import { import { IResource } from '@patternfly-labs/react-form-wizard' import { ReactNode } from 'react' import { BrowserRouter as Router } from 'react-router-dom-v5-compat' +import { RecoilRoot } from 'recoil' import { waitForText } from '../../../lib/test-util' import { Policy } from '../../../resources' import { WizardSyncEditor } from '../../../routes/Governance/policies/CreatePolicy' @@ -31,20 +32,22 @@ describe('ExistingTemplateName', () => { function TestPolicyWizard(props?: { yamlEditor?: () => ReactNode }) { return ( - - new Promise(() => {})} - onCancel={() => {}} - yamlEditor={props?.yamlEditor} - /> - + + + new Promise(() => {})} + onCancel={() => {}} + yamlEditor={props?.yamlEditor} + /> + + ) } @@ -70,20 +73,22 @@ function TestPolicyWizardGK() { ] return ( - - new Promise(() => {})} - onCancel={() => {}} - resources={[mockPolicyGK as IResource]} - /> - + + + new Promise(() => {})} + onCancel={() => {}} + resources={[mockPolicyGK as IResource]} + /> + + ) } @@ -101,21 +106,23 @@ function TestPolicyWizardOperatorPolicy() { ] return ( - - new Promise(() => {})} - onCancel={() => {}} - resources={[mockPolicyOperatorPlc as IResource]} - yamlEditor={() => } - /> - + + + new Promise(() => {})} + onCancel={() => {}} + resources={[mockPolicyOperatorPlc as IResource]} + yamlEditor={() => } + /> + + ) } diff --git a/frontend/src/wizards/Governance/PolicySet/PolicySetWizard.test.tsx b/frontend/src/wizards/Governance/PolicySet/PolicySetWizard.test.tsx index bb996bf30bb..9f6beb5efb1 100644 --- a/frontend/src/wizards/Governance/PolicySet/PolicySetWizard.test.tsx +++ b/frontend/src/wizards/Governance/PolicySet/PolicySetWizard.test.tsx @@ -11,22 +11,25 @@ import { import { PolicySetWizard } from './PolicySetWizard' import { IResource } from '@patternfly-labs/react-form-wizard' import { BrowserRouter as Router } from 'react-router-dom-v5-compat' +import { RecoilRoot } from 'recoil' function TestPolicySetWizard() { return ( - - new Promise(() => {})} - onCancel={() => {}} - /> - + + + new Promise(() => {})} + onCancel={() => {}} + /> + + ) } diff --git a/frontend/src/wizards/Placement/MatchedClustersModal.test.tsx b/frontend/src/wizards/Placement/MatchedClustersModal.test.tsx new file mode 100644 index 00000000000..d2823616f38 --- /dev/null +++ b/frontend/src/wizards/Placement/MatchedClustersModal.test.tsx @@ -0,0 +1,101 @@ +/* Copyright Contributors to the Open Cluster Management project */ +import { render, screen, fireEvent } from '@testing-library/react' +import { MatchedClustersModal } from './MatchedClustersModal' + +jest.mock('../../lib/acm-i18next', () => ({ + useTranslation: () => ({ + t: (key: string, vars?: Record) => { + if (vars) { + return Object.entries(vars).reduce((s, [k, v]) => s.replace(`{{${k}}}`, String(v)), key) + } + return key + }, + }), +})) + +const defaultProps = { + isOpen: true, + onClose: jest.fn(), + matchedClusters: ['cluster1', 'cluster2'], + notMatchedClusters: ['cluster3'], + totalClusters: 3, +} + +describe('MatchedClustersModal', () => { + beforeEach(() => { + jest.clearAllMocks() + }) + + it('renders matched and not-matched sections when both are present', () => { + render() + expect(screen.getByText('Matched')).toBeInTheDocument() + expect(screen.getByText('Not matched')).toBeInTheDocument() + expect(screen.getByText('cluster1')).toBeInTheDocument() + expect(screen.getByText('cluster2')).toBeInTheDocument() + expect(screen.getByText('cluster3')).toBeInTheDocument() + }) + + it('renders flat list without section headers when no notMatchedClusters', () => { + render() + expect(screen.getByText('cluster1')).toBeInTheDocument() + expect(screen.getByText('cluster2')).toBeInTheDocument() + expect(screen.queryByText('Matched')).not.toBeInTheDocument() + expect(screen.queryByText('Not matched')).not.toBeInTheDocument() + }) + + it('filters clusters by search term', () => { + render() + const searchInput = screen.getByPlaceholderText('Find by name') + fireEvent.change(searchInput, { target: { value: 'cluster1' } }) + expect(screen.getByText('cluster1')).toBeInTheDocument() + expect(screen.queryByText('cluster2')).not.toBeInTheDocument() + expect(screen.queryByText('cluster3')).not.toBeInTheDocument() + }) + + it('shows empty state when search matches nothing', () => { + render() + const searchInput = screen.getByPlaceholderText('Find by name') + fireEvent.change(searchInput, { target: { value: 'nonexistent' } }) + expect(screen.getByText('No clusters found matching "nonexistent"')).toBeInTheDocument() + }) + + it('shows "No clusters" when empty and no search term', () => { + render() + expect(screen.getByText('No clusters')).toBeInTheDocument() + }) + + it('does not render when closed', () => { + render() + expect(screen.queryByText('cluster1')).not.toBeInTheDocument() + }) + + it('calls onClose when modal is dismissed', () => { + render() + const closeButton = screen.getByLabelText('Close') + fireEvent.click(closeButton) + expect(defaultProps.onClose).toHaveBeenCalledTimes(1) + }) + + it('filters notMatched clusters independently', () => { + render() + const searchInput = screen.getByPlaceholderText('Find by name') + fireEvent.change(searchInput, { target: { value: 'cluster3' } }) + expect(screen.queryByText('cluster1')).not.toBeInTheDocument() + expect(screen.getByText('cluster3')).toBeInTheDocument() + expect(screen.queryByText('Matched')).not.toBeInTheDocument() + expect(screen.getByText('Not matched')).toBeInTheDocument() + }) + + it('clears search when clear button is clicked', () => { + render() + const searchInput = screen.getByPlaceholderText('Find by name') + fireEvent.change(searchInput, { target: { value: 'cluster1' } }) + expect(screen.queryByText('cluster2')).not.toBeInTheDocument() + + const clearButton = screen.getByLabelText('Reset') + fireEvent.click(clearButton) + expect(screen.getByText('cluster1')).toBeInTheDocument() + expect(screen.getByText('cluster2')).toBeInTheDocument() + expect(screen.getByText('cluster3')).toBeInTheDocument() + }) +}) diff --git a/frontend/src/wizards/Placement/PlacementSection.tsx b/frontend/src/wizards/Placement/PlacementSection.tsx index bc06e8ea888..b3245a23b36 100644 --- a/frontend/src/wizards/Placement/PlacementSection.tsx +++ b/frontend/src/wizards/Placement/PlacementSection.tsx @@ -156,10 +156,8 @@ export function PlacementSection(props: { return resources?.find((resource) => resource.kind === PlacementKind) as IPlacement | undefined }, [resources]) - const { matched, notMatched, matchedCount, totalClusters } = usePlacementDebug( - currentPlacement, - settings.enhancedPlacement === 'enabled' - ) + const debugState = usePlacementDebug(currentPlacement, settings.enhancedPlacement === 'enabled') + const { matched, notMatched, matchedCount, totalClusters, error } = debugState const setFooterContent = useSetFooterContent() const openMatchedModal = useCallback(() => setIsMatchedClustersModalOpen(true), []) @@ -209,6 +207,7 @@ export function PlacementSection(props: { clusterSetBindings={props.existingClusterSetBindings} bindingKind={props.bindingSubjectKind} clusters={props.clusters} + useFeatureFlag={settings.enhancedPlacement === 'enabled'} /> ) : null} } useFeatureFlag={settings.enhancedPlacement === 'enabled'} + placementDebugState={debugState} /> @@ -294,15 +294,25 @@ export function PlacementSection(props: { currentPlacement && (
{/* Placement info alert */} - {matchedCount !== undefined && totalClusters > 0 && ( - + {error ? ( + + ) : ( + matchedCount !== undefined && ( + 0 ? 'info' : 'warning'} + isInline + title={ + matchedCount > 0 + ? t('{{count}} of {{total}} clusters matched by placement', { + count: matchedCount, + total: totalClusters, + }) + : t( + 'No clusters match the current placement criteria. To identify available clusters, check your label expressions, tolerations, or limits.' + ) + } + /> + ) )} {/* Label expressions and tolerations */} @@ -341,7 +351,9 @@ export function PlacementSection(props: { {toleration.value && } {toleration.effect && } - {toleration.tolerationSeconds && } + {toleration.tolerationSeconds != null && ( + + )} ))} diff --git a/frontend/src/wizards/Placement/usePlacementDebug.test.ts b/frontend/src/wizards/Placement/usePlacementDebug.test.ts new file mode 100644 index 00000000000..38a911dd35f --- /dev/null +++ b/frontend/src/wizards/Placement/usePlacementDebug.test.ts @@ -0,0 +1,227 @@ +/* Copyright Contributors to the Open Cluster Management project */ +import { renderHook, act } from '@testing-library/react-hooks' +import { usePlacementDebug } from './usePlacementDebug' +import { IPlacement } from '../common/resources/IPlacement' +import { postPlacementDebug } from '../../resources/placement-debug' + +jest.mock('../../resources/placement-debug', () => ({ + postPlacementDebug: jest.fn(), +})) + +jest.mock('../../resources/utils/resource-request', () => ({ + isRequestAbortedError: jest.fn((err) => err?.name === 'AbortError'), +})) + +const mockPostPlacementDebug = postPlacementDebug as jest.Mock + +const mockPlacement: IPlacement = { + apiVersion: 'cluster.open-cluster-management.io/v1beta1', + kind: 'Placement', + metadata: { name: 'test', namespace: 'default' }, + spec: {}, +} + +const mockResult = { + aggregatedScores: [ + { clusterName: 'cluster1', score: 100 }, + { clusterName: 'cluster2', score: 80 }, + ], + filteredPipelineResults: [{ name: 'filter1', filteredClusters: ['cluster3'] }], +} + +describe('usePlacementDebug', () => { + beforeEach(() => { + jest.useFakeTimers() + mockPostPlacementDebug.mockReset() + }) + + afterEach(() => { + jest.useRealTimers() + }) + + it('returns empty state when disabled', () => { + const { result } = renderHook(() => usePlacementDebug(mockPlacement, false)) + + expect(result.current.matchedCount).toBeUndefined() + expect(result.current.loading).toBe(false) + expect(result.current.matched).toEqual([]) + expect(result.current.notMatched).toEqual([]) + expect(result.current.error).toBeUndefined() + }) + + it('returns empty state when placement is undefined', () => { + const { result } = renderHook(() => usePlacementDebug(undefined)) + + expect(result.current.matchedCount).toBeUndefined() + expect(result.current.loading).toBe(false) + expect(result.current.matched).toEqual([]) + expect(result.current.notMatched).toEqual([]) + expect(result.current.error).toBeUndefined() + }) + + it('fetches and maps successful result', async () => { + mockPostPlacementDebug.mockReturnValue({ + promise: Promise.resolve(mockResult), + abort: jest.fn(), + }) + + const { result } = renderHook(() => usePlacementDebug(mockPlacement, true)) + + // After render but before debounce fires, should be loading + expect(result.current.loading).toBe(true) + + // Advance past the 500ms debounce + act(() => { + jest.advanceTimersByTime(500) + }) + + // Wait for the promise to resolve + await act(async () => { + await Promise.resolve() + }) + + expect(result.current.matched).toEqual(['cluster1', 'cluster2']) + expect(result.current.notMatched).toEqual(['cluster3']) + expect(result.current.matchedCount).toBe(2) + expect(result.current.totalClusters).toBe(3) + expect(result.current.loading).toBe(false) + expect(result.current.error).toBeUndefined() + }) + + it('maps server-returned error from result', async () => { + mockPostPlacementDebug.mockReturnValue({ + promise: Promise.resolve({ error: 'placement namespace not found' }), + abort: jest.fn(), + }) + + const { result } = renderHook(() => usePlacementDebug(mockPlacement, true)) + + act(() => { + jest.advanceTimersByTime(500) + }) + + await act(async () => { + await Promise.resolve() + }) + + expect(result.current.error).toEqual(new Error('placement namespace not found')) + expect(result.current.matched).toEqual([]) + expect(result.current.matchedCount).toBeUndefined() + }) + + it('limits matched clusters by numberOfClusters and puts rest in notMatched', async () => { + mockPostPlacementDebug.mockReturnValue({ + promise: Promise.resolve({ + placement: { spec: { numberOfClusters: 1 } }, + aggregatedScores: [ + { clusterName: 'cluster1', score: 100 }, + { clusterName: 'cluster2', score: 80 }, + { clusterName: 'cluster3', score: 60 }, + ], + filteredPipelineResults: [], + }), + abort: jest.fn(), + }) + + const { result } = renderHook(() => usePlacementDebug(mockPlacement, true)) + + act(() => { + jest.advanceTimersByTime(500) + }) + + await act(async () => { + await Promise.resolve() + }) + + expect(result.current.matched).toEqual(['cluster1']) + expect(result.current.notMatched).toEqual(['cluster2', 'cluster3']) + expect(result.current.matchedCount).toBe(1) + expect(result.current.totalClusters).toBe(3) + }) + + it('handles error state', async () => { + const testError = new Error('Network failure') + mockPostPlacementDebug.mockReturnValue({ + promise: Promise.reject(testError), + abort: jest.fn(), + }) + + const { result } = renderHook(() => usePlacementDebug(mockPlacement, true)) + + act(() => { + jest.advanceTimersByTime(500) + }) + + await act(async () => { + await Promise.resolve() + }) + + expect(result.current.error).toEqual(testError) + expect(result.current.matched).toEqual([]) + expect(result.current.notMatched).toEqual([]) + expect(result.current.matchedCount).toBeUndefined() + expect(result.current.loading).toBe(false) + }) + + it('clears stale state on re-fetch', async () => { + // First fetch resolves successfully + mockPostPlacementDebug.mockReturnValue({ + promise: Promise.resolve(mockResult), + abort: jest.fn(), + }) + + const { result, rerender } = renderHook(({ placement }) => usePlacementDebug(placement, true), { + initialProps: { placement: mockPlacement }, + }) + + act(() => { + jest.advanceTimersByTime(500) + }) + + await act(async () => { + await Promise.resolve() + }) + + expect(result.current.matched).toEqual(['cluster1', 'cluster2']) + + // Prepare a new pending promise for the second fetch + let resolveSecond: (value: typeof mockResult) => void + const secondPromise = new Promise((resolve) => { + resolveSecond = resolve + }) + mockPostPlacementDebug.mockReturnValue({ + promise: secondPromise, + abort: jest.fn(), + }) + + // Trigger re-fetch by changing the placement spec + const updatedPlacement: IPlacement = { + ...mockPlacement, + spec: { clusterSets: ['new-set'] }, + } + + rerender({ placement: updatedPlacement }) + + // After rerender with new spec, state should be cleared and loading + expect(result.current.loading).toBe(true) + expect(result.current.matched).toEqual([]) + expect(result.current.notMatched).toEqual([]) + + // Resolve the second fetch + act(() => { + jest.advanceTimersByTime(500) + }) + + await act(async () => { + resolveSecond!(mockResult) + await Promise.resolve() + }) + + await act(async () => { + await Promise.resolve() + }) + + expect(result.current.matched).toEqual(['cluster1', 'cluster2']) + expect(result.current.loading).toBe(false) + }) +}) diff --git a/frontend/src/wizards/Placement/usePlacementDebug.ts b/frontend/src/wizards/Placement/usePlacementDebug.ts index a345b194dc9..23abf2abc31 100644 --- a/frontend/src/wizards/Placement/usePlacementDebug.ts +++ b/frontend/src/wizards/Placement/usePlacementDebug.ts @@ -91,7 +91,7 @@ export function usePlacementDebug(placement: IPlacement | undefined, enabled = t return } - setState((prev) => ({ ...prev, loading: true, matchedCount: undefined })) + setState({ ...EMPTY_STATE, loading: true }) debouncedFetch(placement) return () => { From 8b2ffdab6cd968c57e042350e062c5582f7ddd08 Mon Sep 17 00:00:00 2001 From: Randy Bruno Piverger <21374229+Randy424@users.noreply.github.com> Date: Mon, 20 Apr 2026 18:10:27 -0700 Subject: [PATCH 06/16] Cache placement debug results across step navigation - Cache hook results to avoid refetch on wizard step changes - Remove X button from WizLabelSelect label pills - Remove unused ClusterScore export Signed-off-by: Randy Bruno Piverger <21374229+Randy424@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 --- .../src/inputs/WizLabelSelect.tsx | 14 +------ frontend/src/resources/placement-debug.ts | 2 +- .../Placement/usePlacementDebug.test.ts | 3 +- .../wizards/Placement/usePlacementDebug.ts | 40 ++++++++++++++----- 4 files changed, 35 insertions(+), 24 deletions(-) diff --git a/frontend/packages/react-form-wizard/src/inputs/WizLabelSelect.tsx b/frontend/packages/react-form-wizard/src/inputs/WizLabelSelect.tsx index 14552b0c330..405d03439f3 100644 --- a/frontend/packages/react-form-wizard/src/inputs/WizLabelSelect.tsx +++ b/frontend/packages/react-form-wizard/src/inputs/WizLabelSelect.tsx @@ -133,19 +133,7 @@ export function WizLabelSelect(props: WizLabelSelectProps) { isDisabled={disabled || readonly} status={validated === 'error' ? 'danger' : undefined} > - + ) : ( { beforeEach(() => { jest.useFakeTimers() mockPostPlacementDebug.mockReset() + clearPlacementDebugCache() }) afterEach(() => { diff --git a/frontend/src/wizards/Placement/usePlacementDebug.ts b/frontend/src/wizards/Placement/usePlacementDebug.ts index 23abf2abc31..d38859b72f3 100644 --- a/frontend/src/wizards/Placement/usePlacementDebug.ts +++ b/frontend/src/wizards/Placement/usePlacementDebug.ts @@ -23,6 +23,14 @@ const EMPTY_STATE: PlacementDebugState = { error: undefined, } +let cachedSpecKey: string | undefined +let cachedState: PlacementDebugState | undefined + +export function clearPlacementDebugCache() { + cachedSpecKey = undefined + cachedState = undefined +} + function mapDebugResult(result: PlacementDebugResult): PlacementDebugState { if (result.error) { return { ...EMPTY_STATE, error: new Error(result.error) } @@ -58,15 +66,18 @@ function mapDebugResult(result: PlacementDebugResult): PlacementDebugState { } export function usePlacementDebug(placement: IPlacement | undefined, enabled = true): PlacementDebugState { - const [state, setState] = useState(EMPTY_STATE) - const abortRef = useRef<(() => void) | undefined>(undefined) - - // Serialize the full placement to detect in-place mutations from `set-value`. - // No namespace guard — the server validates; the hook re-fetches when namespace appears. const specKey = placement ? JSON.stringify({ metadata: placement.metadata, spec: placement.spec }) : undefined + const [state, setState] = useState(() => { + if (enabled && specKey && specKey === cachedSpecKey && cachedState) { + return cachedState + } + return EMPTY_STATE + }) + const abortRef = useRef<(() => void) | undefined>(undefined) + const debouncedFetchRef = useRef( - debounce((p: IPlacement) => { + debounce((p: IPlacement, fetchKey: string) => { abortRef.current?.() setState((prev) => ({ ...prev, loading: true, error: undefined })) @@ -75,11 +86,17 @@ export function usePlacementDebug(placement: IPlacement | undefined, enabled = t promise .then((result) => { - setState(mapDebugResult(result)) + const mapped = mapDebugResult(result) + cachedSpecKey = fetchKey + cachedState = mapped + setState(mapped) }) .catch((err: unknown) => { if (isRequestAbortedError(err)) return - setState({ ...EMPTY_STATE, error: err instanceof Error ? err : new Error(String(err)) }) + const errorState = { ...EMPTY_STATE, error: err instanceof Error ? err : new Error(String(err)) } + cachedSpecKey = fetchKey + cachedState = errorState + setState(errorState) }) }, 500) ) @@ -91,8 +108,13 @@ export function usePlacementDebug(placement: IPlacement | undefined, enabled = t return } + if (specKey === cachedSpecKey && cachedState) { + setState(cachedState) + return + } + setState({ ...EMPTY_STATE, loading: true }) - debouncedFetch(placement) + debouncedFetch(placement, specKey) return () => { debouncedFetch.clear() From 5e0c14218d0602fd35a703aaa7c9ddf20db94055 Mon Sep 17 00:00:00 2001 From: Randy Bruno Piverger <21374229+Randy424@users.noreply.github.com> Date: Tue, 21 Apr 2026 13:32:50 -0700 Subject: [PATCH 07/16] Remove general wizard updates from placement preview PR Migrate non-placement-preview changes to separate branch: WizLabelSelect, MatchExpression styling, toleration expandable, label expandable, and copy updates. Address PR feedback by deduplicating translation key and surfacing API error messages. Fix proxy header forwarding order. Signed-off-by: Randy Bruno Piverger <21374229+Randy424@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 --- backend/src/routes/placementDebug.ts | 4 +- .../packages/react-form-wizard/src/index.ts | 1 - .../Governance/policies/CreatePolicy.test.tsx | 2 +- .../policy-sets/CreatePolicySet.test.tsx | 2 +- frontend/src/wizards/Argo/ArgoWizard.test.tsx | 12 ++- .../src/wizards/Placement/MatchExpression.css | 9 -- .../src/wizards/Placement/MatchExpression.tsx | 86 +++++++++---------- .../wizards/Placement/PlacementSection.tsx | 6 +- 8 files changed, 57 insertions(+), 65 deletions(-) delete mode 100644 frontend/src/wizards/Placement/MatchExpression.css diff --git a/backend/src/routes/placementDebug.ts b/backend/src/routes/placementDebug.ts index 03c64643fc3..bb7047aa3ec 100644 --- a/backend/src/routes/placementDebug.ts +++ b/backend/src/routes/placementDebug.ts @@ -65,12 +65,12 @@ export async function placementDebug(req: Http2ServerRequest, res: Http2ServerRe const headers: OutgoingHttpHeaders = { authorization: `Bearer ${token}`, - 'content-type': 'application/json', - 'content-length': body.length, } for (const header of proxyHeaders) { if (req.headers[header]) headers[header] = req.headers[header] } + headers['content-type'] = 'application/json' + headers['content-length'] = body.length const overrideUrl = process.env.PLACEMENT_DEBUG_URL const url = new URL(overrideUrl || defaultPlacementDebugUrl) diff --git a/frontend/packages/react-form-wizard/src/index.ts b/frontend/packages/react-form-wizard/src/index.ts index f5a73b08e34..9a0e958100e 100644 --- a/frontend/packages/react-form-wizard/src/index.ts +++ b/frontend/packages/react-form-wizard/src/index.ts @@ -18,7 +18,6 @@ export * from './inputs/WizHidden' export * from './inputs/WizItemSelector' export * from './inputs/WizItemText' export * from './inputs/WizKeyValue' -export * from './inputs/WizLabelSelect' export * from './inputs/WizMultiSelect' export * from './inputs/WizNumberInput' export * from './inputs/WizRadio' diff --git a/frontend/src/routes/Governance/policies/CreatePolicy.test.tsx b/frontend/src/routes/Governance/policies/CreatePolicy.test.tsx index 23050d0c49b..df9473fdef3 100755 --- a/frontend/src/routes/Governance/policies/CreatePolicy.test.tsx +++ b/frontend/src/routes/Governance/policies/CreatePolicy.test.tsx @@ -92,7 +92,7 @@ describe('Create Policy Page', () => { // new placement screen.getByRole('button', { name: 'New placement' }).click() - screen.getAllByRole('button', { name: /action/i })[0].click() + screen.getByRole('button', { name: /action/i }).click() screen.getByPlaceholderText(/select the label/i).click() screen.getByRole('option', { name: /cloud/i }).click() screen.getByPlaceholderText(/select the values/i).click() diff --git a/frontend/src/routes/Governance/policy-sets/CreatePolicySet.test.tsx b/frontend/src/routes/Governance/policy-sets/CreatePolicySet.test.tsx index 55f367ee8b4..8f8279b3a84 100644 --- a/frontend/src/routes/Governance/policy-sets/CreatePolicySet.test.tsx +++ b/frontend/src/routes/Governance/policy-sets/CreatePolicySet.test.tsx @@ -54,7 +54,7 @@ describe('Create Policy Page', () => { await waitForText('How do you want to select clusters?') screen.getByRole('button', { name: 'New placement' }).click() - screen.getAllByRole('button', { name: /action/i })[0].click() + screen.getByRole('button', { name: /action/i }).click() screen.getByPlaceholderText(/select the label/i).click() screen.getByRole('option', { name: /cloud/i }).click() screen.getByPlaceholderText(/select the values/i).click() diff --git a/frontend/src/wizards/Argo/ArgoWizard.test.tsx b/frontend/src/wizards/Argo/ArgoWizard.test.tsx index e175d4db818..a87671cfcb3 100644 --- a/frontend/src/wizards/Argo/ArgoWizard.test.tsx +++ b/frontend/src/wizards/Argo/ArgoWizard.test.tsx @@ -218,11 +218,13 @@ describe('ArgoWizard tests', () => { // placement page //===================================================================== await clickByText('New placement') - await clickByRole('button', { name: 'Action' }, 0) + await clickByRole('button', { name: 'Action' }) await clickByRole('combobox', { name: 'Select the label' }) await clickByRole('option', { name: /cloud/i }) - await clickByText('equals any of') + await clickByRole('combobox', { + name: /select the operator/i, + }) await clickByRole('option', { name: /does not equal any of/i }) await clickByRole('combobox', { @@ -329,11 +331,13 @@ describe('ArgoWizard tests', () => { // placement page //===================================================================== await clickByText('New placement') - await clickByRole('button', { name: 'Action' }, 0) + await clickByRole('button', { name: 'Action' }) await clickByRole('combobox', { name: 'Select the label' }) await clickByRole('option', { name: /cloud/i }) - await clickByText('equals any of') + await clickByRole('combobox', { + name: /select the operator/i, + }) await clickByRole('option', { name: /does not equal any of/i }) await clickByRole('combobox', { diff --git a/frontend/src/wizards/Placement/MatchExpression.css b/frontend/src/wizards/Placement/MatchExpression.css deleted file mode 100644 index a291f63c0a8..00000000000 --- a/frontend/src/wizards/Placement/MatchExpression.css +++ /dev/null @@ -1,9 +0,0 @@ -.match-expression-field .pf-v6-c-menu-toggle:not(.pf-m-typeahead) { - height: calc( - var(--pf-t--global--font--size--body--default) * var(--pf-t--global--font--line-height--body) + 2 * - var(--pf-t--global--spacer--control--vertical--default) - ); - padding-block: 0; - display: flex; - align-items: center; -} diff --git a/frontend/src/wizards/Placement/MatchExpression.tsx b/frontend/src/wizards/Placement/MatchExpression.tsx index e8b5f0b0298..5224435807d 100644 --- a/frontend/src/wizards/Placement/MatchExpression.tsx +++ b/frontend/src/wizards/Placement/MatchExpression.tsx @@ -1,12 +1,13 @@ /* Copyright Contributors to the Open Cluster Management project */ -import { Flex, Label } from '@patternfly/react-core' +import { Flex } from '@patternfly/react-core' import { Fragment } from 'react' import set from 'set-value' import { ItemContext, useItem, - WizLabelSelect, + WizSelect, WizMultiSelect, + WizSingleSelect, WizStringsInput, WizTextInput, DisplayMode, @@ -14,54 +15,49 @@ import { } from '@patternfly-labs/react-form-wizard' import { IExpression } from '../common/resources/IMatchExpression' import { useTranslation } from '../../lib/acm-i18next' -import './MatchExpression.css' export function MatchExpression(props: { labelValuesMap?: Record }) { const labelValuesMap = props.labelValuesMap const { t } = useTranslation() return ( -
- {labelValuesMap ? ( - set(item as object, 'values', [])} - /> - ) : ( - set(item as object, 'values', [])} - /> - )} -
-
- { - switch (value) { - case 'Exists': - case 'DoesNotExist': - set(item as object, 'values', undefined) - break - } - }} + onValueChange={(_value, item) => set(item as object, 'values', [])} + /> + ) : ( + set(item as object, 'values', [])} /> -
+ )} + { + switch (value) { + case 'Exists': + case 'DoesNotExist': + set(item, 'values', undefined) + break + } + }} + /> {labelValuesMap ? ( {(item: IExpression) => { @@ -132,8 +128,8 @@ export function MatchExpressionSummary(props: { expression: IExpression }) { } return ( - +
+ {expression?.key} {operator} {expression?.values?.map((value) => value).join(', ')} +
) } diff --git a/frontend/src/wizards/Placement/PlacementSection.tsx b/frontend/src/wizards/Placement/PlacementSection.tsx index b3245a23b36..1e881349de9 100644 --- a/frontend/src/wizards/Placement/PlacementSection.tsx +++ b/frontend/src/wizards/Placement/PlacementSection.tsx @@ -176,7 +176,7 @@ export function PlacementSection(props: { setFooterContent(
- {t('Matched by Placement:')}{' '} + {t('Matched by Placement')}:{' '} @@ -295,7 +295,9 @@ export function PlacementSection(props: {
{/* Placement info alert */} {error ? ( - + + {error.message &&

{error.message}

} + ) : ( matchedCount !== undefined && ( Date: Tue, 21 Apr 2026 14:33:27 -0700 Subject: [PATCH 08/16] Fix rebase artifacts: align naming and remove duplicate translations Resolve issues introduced during rebase onto main: rename WizCustomInputWrapper to WizCustomWrapper to match main's rename, restore MatchExpression.tsx to main's version (DisplayMode.Details belongs to general-updates branch), replace DisplayMode.Details references with type-safe equivalents, and remove duplicate i18n keys. Signed-off-by: Randy Bruno Piverger <21374229+Randy424@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 --- frontend/src/wizards/Placement/MatchExpression.tsx | 6 ------ frontend/src/wizards/Placement/PlacementSection.tsx | 4 ++-- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/frontend/src/wizards/Placement/MatchExpression.tsx b/frontend/src/wizards/Placement/MatchExpression.tsx index 5224435807d..78d4b134d97 100644 --- a/frontend/src/wizards/Placement/MatchExpression.tsx +++ b/frontend/src/wizards/Placement/MatchExpression.tsx @@ -1,6 +1,5 @@ /* Copyright Contributors to the Open Cluster Management project */ import { Flex } from '@patternfly/react-core' -import { Fragment } from 'react' import set from 'set-value' import { ItemContext, @@ -10,8 +9,6 @@ import { WizSingleSelect, WizStringsInput, WizTextInput, - DisplayMode, - useDisplayMode, } from '@patternfly-labs/react-form-wizard' import { IExpression } from '../common/resources/IMatchExpression' import { useTranslation } from '../../lib/acm-i18next' @@ -120,10 +117,7 @@ export function MatchExpressionSummary(props: { expression: IExpression }) { break } - const displayMode = useDisplayMode() - if (!expression?.key) { - if (displayMode === DisplayMode.Details) return return
{t('Expand to enter expression')}
} diff --git a/frontend/src/wizards/Placement/PlacementSection.tsx b/frontend/src/wizards/Placement/PlacementSection.tsx index 1e881349de9..ef11620ad1b 100644 --- a/frontend/src/wizards/Placement/PlacementSection.tsx +++ b/frontend/src/wizards/Placement/PlacementSection.tsx @@ -167,7 +167,7 @@ export function PlacementSection(props: { settings.enhancedPlacement === 'enabled' && placementCount === 1 && currentPlacement && - displayMode !== DisplayMode.Details + displayMode === DisplayMode.Step ) { const matchedLabel = matchedCount === undefined @@ -289,7 +289,7 @@ export function PlacementSection(props: { {/* Review step content */} {settings.enhancedPlacement === 'enabled' && - displayMode === DisplayMode.Details && + displayMode !== DisplayMode.Step && placementCount === 1 && currentPlacement && (
From b63ef5f3eada4ee88548f4dd3bced556b89f61d2 Mon Sep 17 00:00:00 2001 From: Randy Bruno Piverger <21374229+Randy424@users.noreply.github.com> Date: Tue, 21 Apr 2026 18:51:40 -0700 Subject: [PATCH 09/16] Surface placement-debug API errors with inline alert and tooltip Preserve HTTP status code and reason from ResourceError in the error message so users see actionable details (e.g. "500 Internal Server Error: upstream service unavailable"). Render errors as plain inline danger alerts with a tooltip for the full message, replacing the previous text-in-value approach. Signed-off-by: Randy Bruno Piverger <21374229+Randy424@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 --- .../wizards/Placement/PlacementSection.tsx | 52 +++++++++++++------ .../Placement/usePlacementDebug.test.ts | 38 ++++++++++++-- .../wizards/Placement/usePlacementDebug.ts | 12 ++++- 3 files changed, 81 insertions(+), 21 deletions(-) diff --git a/frontend/src/wizards/Placement/PlacementSection.tsx b/frontend/src/wizards/Placement/PlacementSection.tsx index ef11620ad1b..eeabe75f187 100644 --- a/frontend/src/wizards/Placement/PlacementSection.tsx +++ b/frontend/src/wizards/Placement/PlacementSection.tsx @@ -1,5 +1,14 @@ /* Copyright Contributors to the Open Cluster Management project */ -import { Alert, Button, ButtonVariant, Label, LabelGroup, ToggleGroup, ToggleGroupItem } from '@patternfly/react-core' +import { + Alert, + Button, + ButtonVariant, + Label, + LabelGroup, + ToggleGroup, + ToggleGroupItem, + Tooltip, +} from '@patternfly/react-core' import { Fragment, useCallback, useEffect, useMemo, useState } from 'react' import { WizDetailsHidden, @@ -169,19 +178,29 @@ export function PlacementSection(props: { currentPlacement && displayMode === DisplayMode.Step ) { - const matchedLabel = - matchedCount === undefined - ? '-' - : t('{{matched}} of {{total}} clusters', { matched: matchedCount, total: totalClusters }) + if (error) { + setFooterContent( +
+ + + +
+ ) + } else { + const matchedLabel = + matchedCount === undefined + ? '-' + : t('{{matched}} of {{total}} clusters', { matched: matchedCount, total: totalClusters }) - setFooterContent( -
- {t('Matched by Placement')}:{' '} - -
- ) + setFooterContent( +
+ {t('Matched by Placement')}:{' '} + +
+ ) + } } else { setFooterContent(undefined) } @@ -193,6 +212,7 @@ export function PlacementSection(props: { displayMode, matchedCount, totalClusters, + error, setFooterContent, openMatchedModal, t, @@ -295,9 +315,9 @@ export function PlacementSection(props: {
{/* Placement info alert */} {error ? ( - - {error.message &&

{error.message}

} - + + + ) : ( matchedCount !== undefined && ( ({ postPlacementDebug: jest.fn(), })) -jest.mock('../../resources/utils/resource-request', () => ({ - isRequestAbortedError: jest.fn((err) => err?.name === 'AbortError'), -})) +jest.mock('../../resources/utils/resource-request', () => { + const actual = jest.requireActual('../../resources/utils/resource-request') + return { + isRequestAbortedError: jest.fn((err) => err?.name === 'AbortError'), + ResourceError: actual.ResourceError, + ResourceErrorCode: actual.ResourceErrorCode, + } +}) const mockPostPlacementDebug = postPlacementDebug as jest.Mock @@ -164,6 +170,32 @@ describe('usePlacementDebug', () => { expect(result.current.loading).toBe(false) }) + it('formats ResourceError with status code and reason', async () => { + const resourceError = new ResourceError( + ResourceErrorCode.InternalServerError, + 'Internal Server Error', + 'upstream service unavailable' + ) + mockPostPlacementDebug.mockReturnValue({ + promise: Promise.reject(resourceError), + abort: jest.fn(), + }) + + const { result } = renderHook(() => usePlacementDebug(mockPlacement, true)) + + act(() => { + jest.advanceTimersByTime(500) + }) + + await act(async () => { + await Promise.resolve() + }) + + expect(result.current.error?.message).toBe('500 Internal Server Error: upstream service unavailable') + expect(result.current.matched).toEqual([]) + expect(result.current.matchedCount).toBeUndefined() + }) + it('clears stale state on re-fetch', async () => { // First fetch resolves successfully mockPostPlacementDebug.mockReturnValue({ diff --git a/frontend/src/wizards/Placement/usePlacementDebug.ts b/frontend/src/wizards/Placement/usePlacementDebug.ts index d38859b72f3..ca71e0bf292 100644 --- a/frontend/src/wizards/Placement/usePlacementDebug.ts +++ b/frontend/src/wizards/Placement/usePlacementDebug.ts @@ -3,7 +3,7 @@ import { useEffect, useRef, useState } from 'react' import debounce from 'debounce' import { IPlacement } from '../common/resources/IPlacement' import { postPlacementDebug, PlacementDebugResult } from '../../resources/placement-debug' -import { isRequestAbortedError } from '../../resources/utils/resource-request' +import { isRequestAbortedError, ResourceError } from '../../resources/utils/resource-request' export interface PlacementDebugState { matched: string[] @@ -93,7 +93,15 @@ export function usePlacementDebug(placement: IPlacement | undefined, enabled = t }) .catch((err: unknown) => { if (isRequestAbortedError(err)) return - const errorState = { ...EMPTY_STATE, error: err instanceof Error ? err : new Error(String(err)) } + let error: Error + if (err instanceof ResourceError) { + const parts = [`${err.code} ${err.message}`] + if (err.reason) parts.push(err.reason) + error = new Error(parts.join(': ')) + } else { + error = err instanceof Error ? err : new Error(String(err)) + } + const errorState = { ...EMPTY_STATE, error } cachedSpecKey = fetchKey cachedState = errorState setState(errorState) From ce366e5ff5b9d2818fc599e3d3c1bbdb25e2755f Mon Sep 17 00:00:00 2001 From: Randy Bruno Piverger <21374229+Randy424@users.noreply.github.com> Date: Tue, 21 Apr 2026 18:57:23 -0700 Subject: [PATCH 10/16] Keep Matched by Placement label visible during error state Move error alert inline next to the label instead of replacing the entire section, so "Matched by Placement:" remains visible with the danger alert and tooltip appearing where the count would be. Signed-off-by: Randy Bruno Piverger <21374229+Randy424@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 --- .../wizards/Placement/PlacementSection.tsx | 30 ++++++++----------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/frontend/src/wizards/Placement/PlacementSection.tsx b/frontend/src/wizards/Placement/PlacementSection.tsx index eeabe75f187..ab8a3067be6 100644 --- a/frontend/src/wizards/Placement/PlacementSection.tsx +++ b/frontend/src/wizards/Placement/PlacementSection.tsx @@ -178,29 +178,25 @@ export function PlacementSection(props: { currentPlacement && displayMode === DisplayMode.Step ) { - if (error) { - setFooterContent( -
+ const matchedLabel = + matchedCount === undefined + ? '-' + : t('{{matched}} of {{total}} clusters', { matched: matchedCount, total: totalClusters }) + + setFooterContent( +
+ {t('Matched by Placement')}:{' '} + {error ? ( -
- ) - } else { - const matchedLabel = - matchedCount === undefined - ? '-' - : t('{{matched}} of {{total}} clusters', { matched: matchedCount, total: totalClusters }) - - setFooterContent( -
- {t('Matched by Placement')}:{' '} + ) : ( -
- ) - } + )} +
+ ) } else { setFooterContent(undefined) } From d36d9221585221652baecd926376d4bcc783e845 Mon Sep 17 00:00:00 2001 From: Randy Bruno Piverger <21374229+Randy424@users.noreply.github.com> Date: Tue, 21 Apr 2026 19:27:33 -0700 Subject: [PATCH 11/16] Use warning alert for errors, adjust footer spacing, reset modal search Switch error alerts from danger to warning variant, adjust footer padding to 0 top / 1rem bottom, and reset search term when the matched clusters modal reopens. Signed-off-by: Randy Bruno Piverger <21374229+Randy424@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 --- frontend/src/wizards/Placement/MatchedClustersModal.tsx | 6 +++++- frontend/src/wizards/Placement/PlacementSection.tsx | 6 +++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/frontend/src/wizards/Placement/MatchedClustersModal.tsx b/frontend/src/wizards/Placement/MatchedClustersModal.tsx index 0649cac6097..002f802eec1 100644 --- a/frontend/src/wizards/Placement/MatchedClustersModal.tsx +++ b/frontend/src/wizards/Placement/MatchedClustersModal.tsx @@ -1,7 +1,7 @@ /* Copyright Contributors to the Open Cluster Management project */ import { Modal, ModalBody, ModalHeader, ModalVariant, SearchInput, Tooltip } from '@patternfly/react-core' import { OutlinedQuestionCircleIcon } from '@patternfly/react-icons' -import { useState, useMemo } from 'react' +import { useEffect, useMemo, useState } from 'react' import { useTranslation } from '../../lib/acm-i18next' export interface MatchedClustersModalProps { @@ -16,6 +16,10 @@ export function MatchedClustersModal(props: MatchedClustersModalProps) { const { t } = useTranslation() const [searchTerm, setSearchTerm] = useState('') + useEffect(() => { + if (props.isOpen) setSearchTerm('') + }, [props.isOpen]) + const filteredMatched = useMemo(() => { if (!searchTerm) return props.matchedClusters const lower = searchTerm.toLowerCase() diff --git a/frontend/src/wizards/Placement/PlacementSection.tsx b/frontend/src/wizards/Placement/PlacementSection.tsx index ab8a3067be6..2622a7f8ecd 100644 --- a/frontend/src/wizards/Placement/PlacementSection.tsx +++ b/frontend/src/wizards/Placement/PlacementSection.tsx @@ -184,11 +184,11 @@ export function PlacementSection(props: { : t('{{matched}} of {{total}} clusters', { matched: matchedCount, total: totalClusters }) setFooterContent( -
+
{t('Matched by Placement')}:{' '} {error ? ( - + ) : ( } - useFeatureFlag={settings.enhancedPlacement === 'enabled'} + showPlacementPreview={settings.enhancedPlacement === 'enabled'} /> ) : ( diff --git a/frontend/src/wizards/Placement/MatchExpression.css b/frontend/src/wizards/Placement/MatchExpression.css new file mode 100644 index 00000000000..a291f63c0a8 --- /dev/null +++ b/frontend/src/wizards/Placement/MatchExpression.css @@ -0,0 +1,9 @@ +.match-expression-field .pf-v6-c-menu-toggle:not(.pf-m-typeahead) { + height: calc( + var(--pf-t--global--font--size--body--default) * var(--pf-t--global--font--line-height--body) + 2 * + var(--pf-t--global--spacer--control--vertical--default) + ); + padding-block: 0; + display: flex; + align-items: center; +} diff --git a/frontend/src/wizards/Placement/MatchExpression.tsx b/frontend/src/wizards/Placement/MatchExpression.tsx index 78d4b134d97..99f7b3811dd 100644 --- a/frontend/src/wizards/Placement/MatchExpression.tsx +++ b/frontend/src/wizards/Placement/MatchExpression.tsx @@ -1,60 +1,64 @@ /* Copyright Contributors to the Open Cluster Management project */ -import { Flex } from '@patternfly/react-core' +import { Flex, Label } from '@patternfly/react-core' import set from 'set-value' import { ItemContext, useItem, - WizSelect, + WizLabelSelect, WizMultiSelect, - WizSingleSelect, WizStringsInput, WizTextInput, } from '@patternfly-labs/react-form-wizard' import { IExpression } from '../common/resources/IMatchExpression' import { useTranslation } from '../../lib/acm-i18next' +import './MatchExpression.css' export function MatchExpression(props: { labelValuesMap?: Record }) { const labelValuesMap = props.labelValuesMap const { t } = useTranslation() return ( - {labelValuesMap ? ( - set(item as object, 'values', [])} - /> - ) : ( - + {labelValuesMap ? ( + set(item as object, 'values', [])} + /> + ) : ( + set(item as object, 'values', [])} + /> + )} +
+
+ set(item as object, 'values', [])} + onValueChange={(value, item) => { + switch (value) { + case 'Exists': + case 'DoesNotExist': + set(item as object, 'values', undefined) + break + } + }} /> - )} - { - switch (value) { - case 'Exists': - case 'DoesNotExist': - set(item, 'values', undefined) - break - } - }} - /> +
{labelValuesMap ? ( {(item: IExpression) => { @@ -122,8 +126,8 @@ export function MatchExpressionSummary(props: { expression: IExpression }) { } return ( -
- {expression?.key} {operator} {expression?.values?.map((value) => value).join(', ')} -
+ ) } diff --git a/frontend/src/wizards/Placement/Placement.test.tsx b/frontend/src/wizards/Placement/Placement.test.tsx index 99e3106f7a1..31c19ee4776 100644 --- a/frontend/src/wizards/Placement/Placement.test.tsx +++ b/frontend/src/wizards/Placement/Placement.test.tsx @@ -72,6 +72,7 @@ jest.mock('@patternfly-labs/react-form-wizard', () => ({
), WizKeyValue: ({ label }: any) =>
, + WizLabelSelect: ({ label }: any) =>
, WizMultiSelect: ({ label }: any) =>
, WizNumberInput: ({ label }: any) =>
, WizTextInput: ({ label, id }: any) =>
, @@ -180,14 +181,14 @@ describe('Placement', () => { expect(screen.queryByText('No cluster sets available')).not.toBeInTheDocument() }) - it('does not render feature flag UI when useFeatureFlag is false', () => { + it('does not render feature flag UI when showPlacementPreview is false', () => { render() expect(screen.queryByTestId('custom-wrapper-Matched by Placement')).not.toBeInTheDocument() }) - it('renders WizCustomWrapper when useFeatureFlag is true', () => { - render() + it('renders WizCustomWrapper when showPlacementPreview is true', () => { + render() expect(screen.getByTestId('custom-wrapper-Matched by Placement')).toBeInTheDocument() expect(screen.getByTestId('custom-wrapper-value')).toHaveTextContent('-') @@ -203,7 +204,9 @@ describe('Placement', () => { error: undefined, } - render() + render( + + ) expect(screen.getByTestId('custom-wrapper-value')).toHaveTextContent('2 of 3 clusters matched by placement') }) @@ -218,7 +221,9 @@ describe('Placement', () => { error: undefined, } - render() + render( + + ) expect(screen.getByTestId('custom-wrapper-value')).toHaveTextContent( 'No clusters match the current placement criteria' @@ -235,7 +240,9 @@ describe('Placement', () => { error: new Error('500 Internal Server Error'), } - render() + render( + + ) expect(screen.getByText('Unable to determine cluster matches.')).toBeInTheDocument() }) @@ -249,7 +256,7 @@ describe('Placement', () => { }) it('sets footer content when owning debug UI', () => { - render() + render() expect(mockSetFooterContent).toHaveBeenCalled() }) @@ -283,7 +290,7 @@ describe('PlacementPredicate', () => { render() expect(screen.getByTestId('key-value-Label selectors')).toBeInTheDocument() - expect(screen.getByTestId('array-input-Label expressions')).toBeInTheDocument() + expect(screen.getByText('Label expressions')).toBeInTheDocument() expect(screen.getByTestId('array-input-Cluster claim expressions')).toBeInTheDocument() }) }) diff --git a/frontend/src/wizards/Placement/Placement.tsx b/frontend/src/wizards/Placement/Placement.tsx index 0fae70f7f11..770deeed2a5 100644 --- a/frontend/src/wizards/Placement/Placement.tsx +++ b/frontend/src/wizards/Placement/Placement.tsx @@ -106,6 +106,7 @@ export function Placement(props: { placementDebugState?: PlacementDebugState }) { const placement = useItem() as IPlacement + const isClusterSet = placement.spec?.clusterSets?.length const editMode = useEditMode() const displayMode = useDisplayMode() const { update } = useData() @@ -193,6 +194,12 @@ export function Placement(props: { /> )} + {!isClusterSet && !props.namespaceClusterSetNames.length && props.alertTitle ? ( + + {props.alertContent} + + ) : null} +