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
118 changes: 118 additions & 0 deletions apps/console/src/__tests__/k8s-client/client.subresource.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
import { describe, it, expect, vi, afterEach } from "vitest"
import { K8sClient } from "@cozystack/k8s-client"

function fakeResponse(opts: {
ok?: boolean
status?: number
statusText?: string
body?: string
}): Response {
return {
ok: opts.ok ?? true,
status: opts.status ?? 200,
statusText: opts.statusText ?? "OK",
text: async () => opts.body ?? "",
json: async () => JSON.parse(opts.body ?? "null"),
} as unknown as Response
}

afterEach(() => {
vi.unstubAllGlobals()
})

describe("K8sClient.subresource", () => {
it("PUTs to /{plural}/{name}/{subresource} under the aggregated API group", async () => {
const fetchMock = vi.fn(async () => fakeResponse({ body: "" }))
vi.stubGlobal("fetch", fetchMock)
const client = new K8sClient({ baseUrl: "/base" })

await client.subresource(
"subresources.kubevirt.io",
"v1",
"virtualmachines",
"vm-instance-demo",
"start",
"tenant-root",
)

expect(fetchMock).toHaveBeenCalledWith(
"/base/apis/subresources.kubevirt.io/v1/namespaces/tenant-root/virtualmachines/vm-instance-demo/start",
expect.objectContaining({ method: "PUT" }),
)
})

it("POSTs when method is POST", async () => {
const fetchMock = vi.fn(async () => fakeResponse({ body: "" }))
vi.stubGlobal("fetch", fetchMock)
const client = new K8sClient()

await client.subresource(
"subresources.kubevirt.io",
"v1",
"virtualmachines",
"vm",
"restart",
"ns",
{},
"POST",
)

expect(fetchMock).toHaveBeenCalledWith(
expect.any(String),
expect.objectContaining({ method: "POST" }),
)
})
})

describe("K8sClient.request empty-body handling", () => {
it("returns undefined for an empty 2xx body (KubeVirt action subresources)", async () => {
// virtualmachines/{name}/start|stop|restart answer 2xx with no body;
// JSON.parse("") would throw "Unexpected end of JSON input".
vi.stubGlobal("fetch", vi.fn(async () => fakeResponse({ status: 200, body: "" })))
const client = new K8sClient()

const result = await client.subresource(
"subresources.kubevirt.io",
"v1",
"virtualmachines",
"vm",
"stop",
)

expect(result).toBeUndefined()
})

it("parses the JSON body when a 2xx response is non-empty", async () => {
vi.stubGlobal(
"fetch",
vi.fn(async () => fakeResponse({ status: 200, body: JSON.stringify({ kind: "VirtualMachine" }) })),
)
const client = new K8sClient()

const result = await client.get("kubevirt.io", "v1", "virtualmachines", "vm", "ns")

expect(result).toEqual({ kind: "VirtualMachine" })
})

it("short-circuits to undefined on 204 No Content without reading the body", async () => {
const textSpy = vi.fn(async () => "should-not-be-read")
vi.stubGlobal(
"fetch",
vi.fn(
async () =>
({
ok: true,
status: 204,
statusText: "No Content",
text: textSpy,
}) as unknown as Response,
),
)
const client = new K8sClient()

const result = await client.delete("", "v1", "configmaps", "cm", "ns")

expect(result).toBeUndefined()
expect(textSpy).not.toHaveBeenCalled()
})
})
127 changes: 127 additions & 0 deletions apps/console/src/__tests__/k8s-client/useK8sSubresource.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
import { describe, it, expect, vi } from "vitest"
import { renderHook, act } from "@testing-library/react"
import { QueryClient, QueryClientProvider } from "@tanstack/react-query"
import { K8sClient, K8sProvider, useK8sSubresource } from "@cozystack/k8s-client"
import type { ReactNode } from "react"

const actionRef = {
apiGroup: "subresources.kubevirt.io",
apiVersion: "v1",
plural: "virtualmachines",
name: "vm-instance-demo",
namespace: "tenant-root",
}

// The VirtualMachine (with its status) is served under kubevirt.io, not the
// subresources.kubevirt.io aggregated API the action endpoint lives under.
const statusRef = {
apiGroup: "kubevirt.io",
apiVersion: "v1",
plural: "virtualmachines",
namespace: "tenant-root",
}

function setup(gcTime = 0) {
const client = new K8sClient()
vi.spyOn(client, "subresource").mockResolvedValue(undefined)
const queryClient = new QueryClient({
defaultOptions: { queries: { retry: false, gcTime } },
})
const invalidateSpy = vi.spyOn(queryClient, "invalidateQueries")
function wrapper({ children }: { children: ReactNode }) {
return (
<QueryClientProvider client={queryClient}>
<K8sProvider client={client} queryClient={queryClient}>
{children}
</K8sProvider>
</QueryClientProvider>
)
}
return { client, queryClient, invalidateSpy, wrapper }
}

describe("useK8sSubresource", () => {
it("calls the action subresource with a default empty body", async () => {
const { client, wrapper } = setup()
const { result } = renderHook(() => useK8sSubresource(actionRef), { wrapper })

await act(async () => {
await result.current.mutateAsync({ subresource: "start" })
})

expect(client.subresource).toHaveBeenCalledWith(
"subresources.kubevirt.io",
"v1",
"virtualmachines",
"vm-instance-demo",
"start",
"tenant-root",
{},
undefined,
)
})

it("invalidates the action ref's own resource key when no invalidate target is given", async () => {
const { invalidateSpy, wrapper } = setup()
const { result } = renderHook(() => useK8sSubresource(actionRef), { wrapper })

await act(async () => {
await result.current.mutateAsync({ subresource: "start" })
})

expect(invalidateSpy).toHaveBeenCalledWith({
queryKey: ["k8s", "subresources.kubevirt.io", "v1", "virtualmachines", "tenant-root"],
})
})

it("invalidates the target resource (kubevirt.io) when invalidate is supplied", async () => {
const { invalidateSpy, wrapper } = setup()
const { result } = renderHook(
() => useK8sSubresource(actionRef, { invalidate: statusRef }),
{ wrapper },
)

await act(async () => {
await result.current.mutateAsync({ subresource: "start" })
})

// Regression guard: the key must be built from the kubevirt.io status ref,
// otherwise it never matches the query that holds printableStatus.
expect(invalidateSpy).toHaveBeenCalledWith({
queryKey: ["k8s", "kubevirt.io", "v1", "virtualmachines", "tenant-root"],
})
expect(invalidateSpy).not.toHaveBeenCalledWith({
queryKey: ["k8s", "subresources.kubevirt.io", "v1", "virtualmachines", "tenant-root"],
})
})

it("invalidates a metadata.name field-selected list of the target resource", async () => {
// gcTime Infinity so the manually seeded (observer-less) query isn't
// garbage-collected before we read its post-invalidation state.
const { queryClient, wrapper } = setup(Infinity)
// The watch-based status read keys its list query with a field-selector.
// The resource-prefix invalidation must reach it (React Query prefix match),
// not just an unfiltered list — otherwise the post-action refresh is dead.
const fieldSelectedListKey = [
"k8s",
"kubevirt.io",
"v1",
"virtualmachines",
"tenant-root",
"",
"metadata.name=vm-instance-demo",
]
queryClient.setQueryData(fieldSelectedListKey, { items: [] })
expect(queryClient.getQueryState(fieldSelectedListKey)?.isInvalidated).toBe(false)

const { result } = renderHook(
() => useK8sSubresource(actionRef, { invalidate: statusRef }),
{ wrapper },
)
await act(async () => {
await result.current.mutateAsync({ subresource: "start" })
})

expect(queryClient.getQueryState(fieldSelectedListKey)?.isInvalidated).toBe(true)
})
})
29 changes: 21 additions & 8 deletions apps/console/src/components/SchemaForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,19 @@ function addAdditionalPropertiesWidgets(schema: RJSFSchema, uiSchema: UiSchema =
return result
}

/**
* Minimal structural view of a JSON-schema node used by the walk below.
* RJSFSchema is intersected with an `any` index signature, so reading fields
* straight off it yields `any`; routing through this interface keeps the walk
* typed without an `as any` cast.
*/
interface SchemaNode {
type?: string | string[]
properties?: Record<string, unknown>
additionalProperties?: unknown
items?: unknown
}

/**
* Resolve the uiSchema fragment for one schema node: bind the custom field to
* an additionalProperties map, recurse into nested objects, or recurse into
Expand All @@ -112,30 +125,30 @@ function bindAdditionalProperties(
fieldSchema: RJSFSchema,
uiNode: UiSchema | undefined,
): UiSchema | undefined {
const node = fieldSchema as any
const node: SchemaNode = fieldSchema

const isAdditionalPropertiesMap =
node.type === "object" &&
(!node.properties || Object.keys(node.properties).length === 0) &&
typeof node.additionalProperties === "object" &&
node.additionalProperties !== null &&
node.additionalProperties !== true
node.additionalProperties !== null

if (isAdditionalPropertiesMap) {
return { ...uiNode, "ui:field": "AdditionalPropertiesField" }
}

if (node.properties) {
return addAdditionalPropertiesWidgets(fieldSchema, uiNode as UiSchema)
return addAdditionalPropertiesWidgets(fieldSchema, uiNode)
}

const items = node.items
if (
node.type === "array" &&
node.items &&
typeof node.items === "object" &&
!Array.isArray(node.items)
items &&
typeof items === "object" &&
!Array.isArray(items)
) {
const itemsUi = bindAdditionalProperties(node.items as RJSFSchema, (uiNode as any)?.items)
const itemsUi = bindAdditionalProperties(items as RJSFSchema, uiNode?.items)
Comment on lines 145 to +151

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Don't skip tuple-array item schemas.

!Array.isArray(items) means this walker no longer visits tuple-style arrays (items: [...]), so any additionalProperties map nested in those item schemas will miss the custom field binding and fall back to the broken native map rendering path. Please recurse through both object items and array items here. As per coding guidelines: "For form widget binding, walk both properties and items for arrays, but do not walk oneOf/anyOf/allOf unless a real chart needs it".

Suggested fix
   const items = node.items
-  if (
-    node.type === "array" &&
-    items &&
-    typeof items === "object" &&
-    !Array.isArray(items)
-  ) {
-    const itemsUi = bindAdditionalProperties(items as RJSFSchema, uiNode?.items)
+  if (node.type === "array" && items && typeof items === "object") {
+    if (Array.isArray(items)) {
+      const nextItems = items.map((item, index) =>
+        item && typeof item === "object"
+          ? bindAdditionalProperties(
+              item as RJSFSchema,
+              Array.isArray(uiNode?.items) ? (uiNode.items[index] as UiSchema | undefined) : undefined,
+            )
+          : undefined,
+      )
+      if (nextItems.some((itemUi) => itemUi !== undefined)) {
+        return { ...uiNode, items: nextItems }
+      }
+      return uiNode
+    }
+
+    const itemsUi = bindAdditionalProperties(items as RJSFSchema, uiNode?.items)
     if (itemsUi !== undefined) {
       return { ...uiNode, items: itemsUi }
     }
   }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (
node.type === "array" &&
node.items &&
typeof node.items === "object" &&
!Array.isArray(node.items)
items &&
typeof items === "object" &&
!Array.isArray(items)
) {
const itemsUi = bindAdditionalProperties(node.items as RJSFSchema, (uiNode as any)?.items)
const itemsUi = bindAdditionalProperties(items as RJSFSchema, uiNode?.items)
if (node.type === "array" && items && typeof items === "object") {
if (Array.isArray(items)) {
const nextItems = items.map((item, index) =>
item && typeof item === "object"
? bindAdditionalProperties(
item as RJSFSchema,
Array.isArray(uiNode?.items) ? (uiNode.items[index] as UiSchema | undefined) : undefined,
)
: undefined,
)
if (nextItems.some((itemUi) => itemUi !== undefined)) {
return { ...uiNode, items: nextItems }
}
return uiNode
}
const itemsUi = bindAdditionalProperties(items as RJSFSchema, uiNode?.items)
if (itemsUi !== undefined) {
return { ...uiNode, items: itemsUi }
}
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/console/src/components/SchemaForm.tsx` around lines 145 - 151, The
walker currently skips tuple-style arrays due to the "!Array.isArray(items)"
check so tuple item schemas never get bindAdditionalProperties applied; update
the logic in SchemaForm.tsx where node, items, bindAdditionalProperties, and
uiNode?.items are used to handle both cases: if items is an object (typeof items
=== "object" && !Array.isArray(items") keep the existing
bindAdditionalProperties call for the single items schema, and if
Array.isArray(items) iterate over each tuple schema calling
bindAdditionalProperties(items[i] as RJSFSchema, uiNode?.items?.[i]) so each
tuple element recurses; ensure the uiNode item lookup aligns with tuple indices
and preserve existing behavior for non-array items.

if (itemsUi !== undefined) {
return { ...uiNode, items: itemsUi }
}
Expand Down
48 changes: 48 additions & 0 deletions apps/console/src/lib/app-definitions.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { describe, it, expect } from "vitest"
import { releasePrefix } from "./app-definitions.ts"
import type { ApplicationDefinition } from "@cozystack/types"

function ad(overrides: Partial<ApplicationDefinition> = {}): ApplicationDefinition {
return {
apiVersion: "cozystack.io/v1alpha1",
kind: "ApplicationDefinition",
metadata: { name: "virtual-machine" },
spec: {
application: {
kind: "VMInstance",
plural: "vminstances",
singular: "vm-instance",
openAPISchema: "{}",
},
},
...overrides,
}
}

describe("releasePrefix", () => {
it("returns the explicit release.prefix when set", () => {
expect(
releasePrefix(
ad({
spec: {
application: {
kind: "VMInstance",
plural: "vminstances",
singular: "vm-instance",
openAPISchema: "{}",
},
release: { prefix: "custom-" },
},
}),
),
).toBe("custom-")
})

it("falls back to '<singular>-' when release.prefix is unset", () => {
expect(releasePrefix(ad())).toBe("vm-instance-")
})

it("falls back to '<metadata.name>-' when neither prefix nor spec is present", () => {
expect(releasePrefix(ad({ spec: undefined }))).toBe("virtual-machine-")
})
})
4 changes: 4 additions & 0 deletions apps/console/src/routes/detail/ApplicationDetailPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import { IngressesTab } from "./IngressesTab.tsx"
import { SecretsTab } from "./SecretsTab.tsx"
import { EventsTab } from "./EventsTab.tsx"
import { VncTab } from "./VncTab.tsx"
import { VMPowerControls } from "./VMPowerControls.tsx"

export function ApplicationDetailPage() {
const { plural, name } = useParams<{ plural: string; name: string }>()
Expand Down Expand Up @@ -155,6 +156,9 @@ export function ApplicationDetailPage() {
) : null}
</div>
<div className="flex items-center gap-2">
{kind === "VMInstance" && (
<VMPowerControls ad={ad} instance={instance} />
)}
<Link to={`/console/${plural}/${name}/edit`}>
<Button variant="outline" size="sm">
<Edit className="size-3.5" /> Edit
Expand Down
Loading
Loading