Skip to content

Commit bbeae29

Browse files
myasnikovdaniilmyasnikovdaniil
committed
feat(console): show image-upload state on the VMDisk detail page
A VMDisk created with `source: upload` had no upload affordance at all: the detail page offered only Edit and Delete, so the disk sat in UploadReady with nothing in the UI explaining what it was waiting for. The browser cannot perform the upload itself. CDI's upload proxy reads its token only from the Authorization header, and the two routes a same-origin SPA has both fail: - through the API server's services/proxy subresource, the API server strips Authorization, so the proxy answers 400 (missing token) even for a valid one; - directly at cdi-uploadproxy.<root-host>, Cozystack publishes the service with TLS passthrough, so CDI's internal self-signed certificate reaches the browser — untrusted CA and no matching SAN. CORS is not the obstacle; CDI serves Access-Control-Allow-Origin: *. So the panel reports what it can read and hands over the command that works. It renders only for an upload-source disk, derives the stage from the DataVolume phase, and offers a copyable `virtctl image-upload` line only while the upload server actually exists (UploadReady) or after a failure. The proxy URL comes from CDIConfig.status.uploadProxyURL, which CDI exposes to system:authenticated; when the platform published none, the placeholder is called out rather than passed off as an address. The DataVolume is read with a named get, not a field-selected list: the vm-disk chart grants tenants get/list/watch restricted by resourceNames, under which only get is actually permitted. A SelfSubjectAccessReview drives a warning when the user cannot create uploadtokenrequests, so tenant admins are told the command will fail rather than being sent to a Forbidden (#3759). Signed-off-by: Myasnikov Daniil <[email protected]>
1 parent 334e2d8 commit bbeae29

5 files changed

Lines changed: 636 additions & 1 deletion

File tree

‎packages/system/dashboard/images/console/apps/console/src/lib/vm-disk-upload.test.ts‎

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
import { describe, it, expect } from "vitest"
2+
import {
3+
isUploadSource,
4+
uploadState,
5+
virtctlUploadCommand,
6+
UPLOAD_PROXY_URL_PLACEHOLDER,
7+
type DataVolume,
8+
} from "./vm-disk-upload.ts"
9+
10+
function dv(
11+
source: Record<string, unknown> | undefined,
12+
status?: DataVolume["status"],
13+
): DataVolume {
14+
return {
15+
apiVersion: "cdi.kubevirt.io/v1beta1",
16+
kind: "DataVolume",
17+
metadata: { name: "vm-disk-demo", namespace: "tenant-root" },
18+
...(source === undefined ? {} : { spec: { source } }),
19+
...(status ? { status } : {}),
20+
}
21+
}
22+
23+
describe("isUploadSource", () => {
24+
it("recognises an upload source", () => {
25+
expect(isUploadSource(dv({ upload: {} }))).toBe(true)
26+
})
27+
28+
it("rejects the other vm-disk sources", () => {
29+
expect(isUploadSource(dv({ http: { url: "https://example.org/i.qcow2" } }))).toBe(false)
30+
expect(isUploadSource(dv({ pvc: { name: "vm-default-images-ubuntu" } }))).toBe(false)
31+
expect(isUploadSource(dv({ blank: {} }))).toBe(false)
32+
})
33+
34+
it("rejects a DataVolume with no source and an unreadable one", () => {
35+
expect(isUploadSource(dv(undefined))).toBe(false)
36+
expect(isUploadSource(undefined)).toBe(false)
37+
})
38+
39+
it("does not mistake an inherited property for an upload source", () => {
40+
// Object.create puts `upload` on the prototype, where `in` would find it.
41+
const inherited = Object.create({ upload: {} }) as Record<string, unknown>
42+
expect(isUploadSource(dv(inherited))).toBe(false)
43+
})
44+
})
45+
46+
describe("uploadState", () => {
47+
it("is awaiting-upload only at UploadReady — the one phase CDI accepts data in", () => {
48+
expect(uploadState(dv({ upload: {} }, { phase: "UploadReady" })).stage).toBe(
49+
"awaiting-upload",
50+
)
51+
})
52+
53+
it.each([
54+
"",
55+
"Pending",
56+
"PVCBound",
57+
"WaitForFirstConsumer",
58+
"PendingPopulation",
59+
"UploadScheduled",
60+
])("treats phase %s as preparing", (phase) => {
61+
expect(uploadState(dv({ upload: {} }, { phase })).stage).toBe("preparing")
62+
})
63+
64+
it("reports Succeeded and Failed distinctly", () => {
65+
expect(uploadState(dv({ upload: {} }, { phase: "Succeeded" })).stage).toBe("succeeded")
66+
expect(uploadState(dv({ upload: {} }, { phase: "Failed" })).stage).toBe("failed")
67+
})
68+
69+
it("surfaces the failure message from the Running condition", () => {
70+
const state = uploadState(
71+
dv({ upload: {} }, {
72+
phase: "Failed",
73+
conditions: [
74+
{ type: "Bound", message: "bound" },
75+
{ type: "Running", message: "upload server crashed" },
76+
],
77+
}),
78+
)
79+
expect(state.message).toBe("upload server crashed")
80+
})
81+
82+
it("falls back to the Bound condition when Running carries no message", () => {
83+
const state = uploadState(
84+
dv({ upload: {} }, {
85+
phase: "Failed",
86+
conditions: [{ type: "Bound", message: "no capacity" }],
87+
}),
88+
)
89+
expect(state.message).toBe("no capacity")
90+
})
91+
92+
it("carries progress through at UploadReady", () => {
93+
expect(
94+
uploadState(dv({ upload: {} }, { phase: "UploadReady", progress: "42.0%" })).progress,
95+
).toBe("42.0%")
96+
})
97+
98+
it("is unknown for a missing DataVolume and for an unrecognised phase", () => {
99+
expect(uploadState(undefined).stage).toBe("unknown")
100+
expect(uploadState(dv({ upload: {} }, { phase: "ImportInProgress" })).stage).toBe(
101+
"unknown",
102+
)
103+
})
104+
105+
it("does not report a phase it was never given", () => {
106+
expect(uploadState(dv({ upload: {} }, {})).stage).toBe("preparing")
107+
expect(uploadState(dv({ upload: {} }, {})).phase).toBe("")
108+
})
109+
})
110+
111+
describe("virtctlUploadCommand", () => {
112+
it("reuses the chart-created DataVolume and skips certificate verification", () => {
113+
const cmd = virtctlUploadCommand({
114+
name: "vm-disk-demo",
115+
namespace: "tenant-root",
116+
uploadProxyURL: "https://cdi-uploadproxy.example.org",
117+
})
118+
expect(cmd).toContain("virtctl image-upload dv vm-disk-demo")
119+
// The Helm release owns the DataVolume, so creating another one fails.
120+
expect(cmd).toContain("--no-create")
121+
// cdi-uploadproxy is published with TLS passthrough: CDI's own cert is on the wire.
122+
expect(cmd).toContain("--insecure")
123+
expect(cmd).toContain("--namespace=tenant-root")
124+
expect(cmd).toContain("--uploadproxy-url=https://cdi-uploadproxy.example.org")
125+
})
126+
127+
it("falls back to a visible placeholder when the cluster published no proxy URL", () => {
128+
for (const uploadProxyURL of [undefined, "", " "]) {
129+
const cmd = virtctlUploadCommand({
130+
name: "vm-disk-demo",
131+
namespace: "tenant-root",
132+
uploadProxyURL,
133+
})
134+
expect(cmd).toContain(`--uploadproxy-url=${UPLOAD_PROXY_URL_PLACEHOLDER}`)
135+
}
136+
})
137+
})

‎packages/system/dashboard/images/console/apps/console/src/lib/vm-disk-upload.ts‎

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
import type { K8sResource } from "@cozystack/k8s-client"
2+
3+
export interface DataVolumeSpec {
4+
source?: Record<string, unknown>
5+
}
6+
7+
export interface DataVolumeStatus {
8+
phase?: string
9+
progress?: string
10+
conditions?: {
11+
type?: string
12+
status?: string
13+
reason?: string
14+
message?: string
15+
}[]
16+
}
17+
18+
export type DataVolume = K8sResource<DataVolumeSpec, DataVolumeStatus>
19+
20+
export interface CDIConfigStatus {
21+
uploadProxyURL?: string
22+
}
23+
24+
export type CDIConfig = K8sResource<unknown, CDIConfigStatus>
25+
26+
/**
27+
* Where an upload-source disk sits in the CDI lifecycle.
28+
*
29+
* `awaiting-upload` is the only stage in which the CDI upload server exists and
30+
* accepts data — a token minted at any other stage has nothing to talk to.
31+
*/
32+
export type UploadStage =
33+
| "preparing"
34+
| "awaiting-upload"
35+
| "succeeded"
36+
| "failed"
37+
| "unknown"
38+
39+
export interface UploadState {
40+
stage: UploadStage
41+
phase: string
42+
progress?: string
43+
message?: string
44+
}
45+
46+
// CDI DataVolumePhase values that precede the upload server being reachable.
47+
const PREPARING_PHASES = new Set([
48+
"",
49+
"Pending",
50+
"PVCBound",
51+
"WaitForFirstConsumer",
52+
"PendingPopulation",
53+
"UploadScheduled",
54+
])
55+
56+
export function isUploadSource(dv: DataVolume | undefined): boolean {
57+
const source = dv?.spec?.source
58+
return !!source && Object.prototype.hasOwnProperty.call(source, "upload")
59+
}
60+
61+
function failureMessage(dv: DataVolume): string | undefined {
62+
const running = dv.status?.conditions?.find((c) => c.type === "Running")
63+
const bound = dv.status?.conditions?.find((c) => c.type === "Bound")
64+
return running?.message ?? bound?.message
65+
}
66+
67+
export function uploadState(dv: DataVolume | undefined): UploadState {
68+
if (!dv) return { stage: "unknown", phase: "" }
69+
const phase = dv.status?.phase ?? ""
70+
if (phase === "Succeeded") return { stage: "succeeded", phase }
71+
if (phase === "Failed") {
72+
return { stage: "failed", phase, message: failureMessage(dv) }
73+
}
74+
if (phase === "UploadReady") {
75+
return { stage: "awaiting-upload", phase, progress: dv.status?.progress }
76+
}
77+
if (PREPARING_PHASES.has(phase)) return { stage: "preparing", phase }
78+
return { stage: "unknown", phase }
79+
}
80+
81+
export interface UploadCommandOptions {
82+
name: string
83+
namespace: string
84+
/** CDIConfig.status.uploadProxyURL — absent when the platform published none. */
85+
uploadProxyURL?: string
86+
}
87+
88+
export const UPLOAD_PROXY_URL_PLACEHOLDER = "https://cdi-uploadproxy.<your-cozystack-domain>"
89+
90+
/**
91+
* The disk's DataVolume is created by the vm-disk Helm release, so the upload
92+
* has to reuse it (`--no-create`); `--insecure` is required because Cozystack
93+
* publishes cdi-uploadproxy through TLS passthrough, leaving CDI's internal
94+
* self-signed certificate on the wire.
95+
*/
96+
export function virtctlUploadCommand(opts: UploadCommandOptions): string {
97+
const proxy = opts.uploadProxyURL?.trim() || UPLOAD_PROXY_URL_PLACEHOLDER
98+
return [
99+
"virtctl image-upload dv",
100+
opts.name,
101+
"--no-create",
102+
`--namespace=${opts.namespace}`,
103+
"--image-path=./disk.qcow2",
104+
`--uploadproxy-url=${proxy}`,
105+
"--insecure",
106+
].join(" ")
107+
}

‎packages/system/dashboard/images/console/apps/console/src/routes/detail/ApplicationDetailPage.tsx‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import { SecretsTab } from "./SecretsTab.tsx"
3434
import { EventsTab } from "./EventsTab.tsx"
3535
import { VncTab } from "./VncTab.tsx"
3636
import { VMPowerControls } from "./VMPowerControls.tsx"
37+
import { DiskUploadPanel } from "./DiskUploadPanel.tsx"
3738

3839
export function ApplicationDetailPage() {
3940
const { plural, name } = useParams<{ plural: string; name: string }>()
@@ -184,7 +185,17 @@ export function ApplicationDetailPage() {
184185

185186
<div className="flex-1 overflow-auto">
186187
<Routes>
187-
<Route index element={<OverviewTab ad={ad} instance={instance} />} />
188+
<Route
189+
index
190+
element={
191+
<>
192+
{kind === "VMDisk" && (
193+
<DiskUploadPanel ad={ad} instance={instance} />
194+
)}
195+
<OverviewTab ad={ad} instance={instance} />
196+
</>
197+
}
198+
/>
188199
<Route
189200
path="workloads"
190201
element={<WorkloadsTab ad={ad} instance={instance} />}

0 commit comments

Comments
 (0)