Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions backend/src/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -66,6 +67,7 @@ if (eventsEnabled) {
router.get('/events', events)
}
router.post('/proxy/search', search)
router.post('/placement-debug', placementDebug)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
router.get('/authenticated', authenticated)
router.post('/ansibletower', ansibleTower)
router.get('/username', username)
Expand Down
95 changes: 95 additions & 0 deletions backend/src/routes/placementDebug.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
/* 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 { request } from 'node:https'
import { URL } from 'node:url'
import { getServiceAgent } from '../lib/agent'
import { logger } from '../lib/logger'
import { respondInternalServerError } from '../lib/respond'
import { getAuthenticatedToken } from '../lib/token'

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/`

const MAX_BODY_SIZE = 1024 * 1024 // 1MB

function collectBody(req: Http2ServerRequest): Promise<Buffer> {
return new Promise((resolve, reject) => {
const chunks: Buffer[] = []
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)
})
}
Comment on lines +25 to +41
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Using collectBody here. It's store-and-forward proxy rather than a streaming proxy like search.ts

The pipeline approach (as seen in search.ts) doesn't work with the mock-request test infrastructure. The search.ts tests have // TODO - pipeline is not writing response comments where they can't assert on status codes.

With collectBody, our tests can actually verify status codes (200, 401, 413, 500), which is why all 6 backend tests pass and assert correctly. If we switched to pipeline, we'd lose the ability to test response status codes — the same limitation search.ts has.

Tradeoff: pipeline is more memory-efficient for large payloads (streams directly), while collectBody buffers the full request body (capped at 1MB). For placement debug payloads (small JSON placement specs), the 1MB buffer is fine and gives us better testability.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm OK with this for now, but I think we should explore this some more. The Ansible route uses pipeline and seems to have tests that do validate the response code.


export async function placementDebug(req: Http2ServerRequest, res: Http2ServerResponse): Promise<void> {
const token = await getAuthenticatedToken(req, res)
if (!token) return

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}`,
}
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 url = new URL(process.env.PLACEMENT_DEBUG_URL || defaultPlacementDebugUrl)
headers.host = url.hostname

const options: RequestOptions = {
protocol: url.protocol,
hostname: url.hostname,
port: url.port,
path: url.pathname,
method: 'POST',
headers,
agent: getServiceAgent(),
}

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()
}
62 changes: 62 additions & 0 deletions backend/test/routes/placementDebug.test.ts
Original file line number Diff line number Diff line change
@@ -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)
})
})
52 changes: 29 additions & 23 deletions frontend/packages/react-form-wizard/src/Wizard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -107,29 +108,31 @@ export function Wizard(props: WizardProps & { showHeader?: boolean; showYaml?: b
<ShowValidationProvider>
<ValidationProvider>
<HighlightEditorPathProvider>
<Drawer isExpanded={drawerExpanded} isInline>
<DrawerContent panelContent={<WizardDrawer yamlEditor={props.yamlEditor} />}>
<DrawerContentBody>
<ItemContext.Provider value={data}>
<StringContext.Provider value={wizardStrings || defaultStrings}>
<WizardInternal
id={props.id}
reviewStorageKey={props.reviewStorageKey}
showYaml={props.showYaml}
onSubmit={props.onSubmit}
onCancel={props.onCancel}
hasButtons={props.hasButtons}
submitButtonText={props.submitButtonText}
submittingButtonText={props.submittingButtonText}
isLoading={props.isLoading}
>
{props.children}
</WizardInternal>
</StringContext.Provider>
</ItemContext.Provider>
</DrawerContentBody>
</DrawerContent>
</Drawer>
<FooterContentProvider>
<Drawer isExpanded={drawerExpanded} isInline>
<DrawerContent panelContent={<WizardDrawer yamlEditor={props.yamlEditor} />}>
<DrawerContentBody>
<ItemContext.Provider value={data}>
<StringContext.Provider value={wizardStrings || defaultStrings}>
<WizardInternal
id={props.id}
reviewStorageKey={props.reviewStorageKey}
showYaml={props.showYaml}
onSubmit={props.onSubmit}
onCancel={props.onCancel}
hasButtons={props.hasButtons}
submitButtonText={props.submitButtonText}
submittingButtonText={props.submittingButtonText}
isLoading={props.isLoading}
>
{props.children}
</WizardInternal>
</StringContext.Provider>
</ItemContext.Provider>
</DrawerContentBody>
</DrawerContent>
</Drawer>
</FooterContentProvider>
</HighlightEditorPathProvider>
</ValidationProvider>
</ShowValidationProvider>
Expand Down Expand Up @@ -344,6 +347,8 @@ function MyFooter(props: WizardFooterProps) {
nextButtonText,
} = useStringContext()

const footerContent = useFooterContent()

if (isLastStep) {
return (
<div className="pf-v6-u-box-shadow-sm-top">
Expand Down Expand Up @@ -405,6 +410,7 @@ function MyFooter(props: WizardFooterProps) {
<Alert title={fixValidationErrorsMsg} isInline variant="danger" />
)}
<WizardFooterWrapper>
{footerContent}
<ActionList>
<ActionListGroup>
<ActionListItem>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { createContext, useContext } from 'react'
export enum DisplayMode {
Step,
StepsHidden,
Details,
}

export const DisplayModeContext = createContext<DisplayMode>(DisplayMode.Step)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/* Copyright Contributors to the Open Cluster Management project */
import { createContext, ReactNode, useCallback, useContext, useState } from 'react'

const FooterContentContext = createContext<ReactNode>(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<ReactNode>(undefined)
const setFooterContent = useCallback((content: ReactNode) => {
setFooterContentState(content)
}, [])

return (
<SetFooterContentContext.Provider value={setFooterContent}>
<FooterContentContext.Provider value={footerContent}>{props.children}</FooterContentContext.Provider>
</SetFooterContentContext.Provider>
)
}
1 change: 1 addition & 0 deletions frontend/packages/react-form-wizard/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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)
Expand Down Expand Up @@ -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 <div id={id}>{children}</div>
}
27 changes: 24 additions & 3 deletions frontend/packages/react-form-wizard/src/inputs/WizLabelSelect.tsx
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -28,9 +37,9 @@ export type WizLabelSelectProps = InputCommonProps<string> & {
* 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<string[]>([])
Expand Down Expand Up @@ -100,6 +109,18 @@ export function WizLabelSelect(props: WizLabelSelectProps) {

if (hidden) return null

if (mode === DisplayMode.Details) {
if (!value) return null
return (
<DescriptionListGroup>
<DescriptionListTerm>{label}</DescriptionListTerm>
<DescriptionListDescription id={id}>
<Label variant="outline">{displayLabel}</Label>
</DescriptionListDescription>
</DescriptionListGroup>
)
}

const toggle = (toggleRef: React.Ref<MenuToggleElement>) =>
value ? (
<MenuToggle
Expand Down
Loading
Loading