Skip to content

Commit 653cf30

Browse files
zlaynezlayne
authored
VirtualMachine actions rbac (stolostron#3905)
Signed-off-by: zlayne <zlayne@redhat.com>
1 parent 7635abc commit 653cf30

File tree

5 files changed

+184
-69
lines changed

5 files changed

+184
-69
lines changed

‎backend/src/routes/events.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,11 @@ import { Stream } from 'stream'
1010
import { promisify } from 'util'
1111
import { jsonPost } from '../lib/json-request'
1212
import { logger } from '../lib/logger'
13+
import { ITransformedResource } from '../lib/pagination'
1314
import { ServerSideEvent, ServerSideEvents } from '../lib/server-side-events'
1415
import { getServiceAccountToken } from '../lib/serviceAccountToken'
1516
import { getAuthenticatedToken } from '../lib/token'
1617
import { IResource } from '../resources/resource'
17-
import { ITransformedResource } from '../lib/pagination'
1818

1919
const { map, split } = eventStream
2020
const pipeline = promisify(Stream.pipeline)
@@ -631,7 +631,7 @@ function canGetResource(resource: IResource, token: string): Promise<boolean> {
631631
return canAccess(resource, 'get', token)
632632
}
633633

634-
function canAccess(
634+
export function canAccess(
635635
resource: { kind: string; apiVersion: string; metadata?: { name?: string; namespace?: string } },
636636
verb: 'get' | 'list' | 'create',
637637
token: string

‎backend/src/routes/virtualMachineProxy.ts

+75-58
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,13 @@
22
import { constants, Http2ServerRequest, Http2ServerResponse, OutgoingHttpHeaders } from 'http2'
33
import { jsonPut, jsonRequest } from '../lib/json-request'
44
import { logger } from '../lib/logger'
5-
import { respondInternalServerError } from '../lib/respond'
5+
import { respond, respondInternalServerError } from '../lib/respond'
66
import { getServiceAccountToken } from '../lib/serviceAccountToken'
77
import { getAuthenticatedToken } from '../lib/token'
88
import { ResourceList } from '../resources/resource-list'
99
import { Route } from '../resources/route'
1010
import { Secret } from '../resources/secret'
11+
import { canAccess } from './events'
1112

1213
const { HTTP_STATUS_INTERNAL_SERVER_ERROR } = constants
1314
const proxyHeaders = [
@@ -37,66 +38,82 @@ export async function virtualMachineProxy(req: Http2ServerRequest, res: Http2Ser
3738
req.on('end', async () => {
3839
const body = JSON.parse(chucks.join()) as ActionBody
3940

40-
// console-mce ClusterRole does not allow for GET on secrets. Have to list in a namespace
41-
const secretPath = process.env.CLUSTER_API_URL + `/api/v1/namespaces/${body.managedCluster}/secrets`
42-
const managedClusterToken: string = await jsonRequest(secretPath, serviceAccountToken)
43-
.then((response: ResourceList<Secret>) => {
44-
const secret = response.items.find((secret) => secret.metadata.name === 'vm-actor')
45-
const proxyToken = secret.data?.token ?? ''
46-
return Buffer.from(proxyToken, 'base64').toString('ascii')
47-
})
48-
.catch((err: Error): undefined => {
49-
logger.error({ msg: `Error getting secret in namespace ${body.managedCluster}`, error: err.message })
50-
return undefined
51-
})
52-
53-
// Get cluster proxy host
54-
const proxyServer = await jsonRequest(
55-
process.env.CLUSTER_API_URL +
56-
'/apis/route.openshift.io/v1/namespaces/multicluster-engine/routes/cluster-proxy-addon-user',
41+
// If user is not able to create an MCA in the managed cluster namespace -> they aren't authorized to trigger actions.
42+
const hasAuth = await canAccess(
43+
{
44+
kind: 'ManagedClusterAction',
45+
apiVersion: 'action.open-cluster-management.io/v1beta1',
46+
metadata: { namespace: body.managedCluster },
47+
},
48+
'create',
5749
token
58-
)
59-
.then((response: Route) => {
60-
const scheme = response?.spec?.tls?.termination ? 'https' : 'http'
61-
return response?.spec?.host ? `${scheme}://${response.spec.host}` : ''
62-
})
63-
.catch((err: Error): undefined => {
64-
logger.error({ msg: 'Error getting cluster proxy Route', error: err.message })
65-
return undefined
66-
})
50+
).then((allowed) => allowed)
6751

68-
// req.url is one of: /virtualmachines/<action> OR /virtualmachineinstances/<action>
69-
// the VM name is need between the kind and action for the correct api url.
70-
const splitURL = req.url.split('/')
71-
const joinedURL = `${splitURL[1]}/${body.vmName}/${splitURL[2]}`
72-
const path = `${proxyServer}/${body.managedCluster}/apis/subresources.kubevirt.io/v1/namespaces/${body.vmNamespace}/${joinedURL}`
73-
const headers: OutgoingHttpHeaders = { authorization: `Bearer ${managedClusterToken}` }
74-
for (const header of proxyHeaders) {
75-
if (req.headers[header]) headers[header] = req.headers[header]
76-
}
52+
if (hasAuth) {
53+
// console-mce ClusterRole does not allow for GET on secrets. Have to list in a namespace
54+
const secretPath = process.env.CLUSTER_API_URL + `/api/v1/namespaces/${body.managedCluster}/secrets`
55+
const managedClusterToken: string = await jsonRequest(secretPath, serviceAccountToken)
56+
.then((response: ResourceList<Secret>) => {
57+
const secret = response.items.find((secret) => secret.metadata.name === 'vm-actor')
58+
const proxyToken = secret.data?.token ?? ''
59+
return Buffer.from(proxyToken, 'base64').toString('ascii')
60+
})
61+
.catch((err: Error): undefined => {
62+
logger.error({ msg: `Error getting secret in namespace ${body.managedCluster}`, error: err.message })
63+
return undefined
64+
})
65+
66+
// Get cluster proxy host
67+
const proxyServer = await jsonRequest(
68+
process.env.CLUSTER_API_URL +
69+
'/apis/route.openshift.io/v1/namespaces/multicluster-engine/routes/cluster-proxy-addon-user',
70+
token
71+
)
72+
.then((response: Route) => {
73+
const scheme = response?.spec?.tls?.termination ? 'https' : 'http'
74+
return response?.spec?.host ? `${scheme}://${response.spec.host}` : ''
75+
})
76+
.catch((err: Error): undefined => {
77+
logger.error({ msg: 'Error getting cluster proxy Route', error: err.message })
78+
return undefined
79+
})
7780

78-
if (!path) return respondInternalServerError(req, res)
79-
await jsonPut(path, {}, managedClusterToken)
80-
.then((results) => {
81-
if (results?.statusCode >= 200 && results?.statusCode < 300) {
82-
res.setHeader('Content-Type', 'application/json')
83-
res.end(JSON.stringify(results))
84-
} else {
85-
logger.error({
86-
msg: 'Error in VirtualMachine action response',
87-
error: results.body.message,
88-
})
89-
res.setHeader('Content-Type', 'application/json')
90-
res.writeHead(results.statusCode ?? HTTP_STATUS_INTERNAL_SERVER_ERROR)
91-
delete results.body?.code // code is added via writeHead
92-
res.end(JSON.stringify(results.body))
93-
}
94-
})
95-
.catch((err: Error) => {
96-
logger.error({ msg: 'Error on VirtualMachine action request', error: err.message })
97-
respondInternalServerError(req, res)
98-
return undefined
99-
})
81+
// req.url is one of: /virtualmachines/<action> OR /virtualmachineinstances/<action>
82+
// the VM name is needed between the kind and action for the correct api url.
83+
const splitURL = req.url.split('/')
84+
const joinedURL = `${splitURL[1]}/${body.vmName}/${splitURL[2]}`
85+
const path = `${proxyServer}/${body.managedCluster}/apis/subresources.kubevirt.io/v1/namespaces/${body.vmNamespace}/${joinedURL}`
86+
const headers: OutgoingHttpHeaders = { authorization: `Bearer ${managedClusterToken}` }
87+
for (const header of proxyHeaders) {
88+
if (req.headers[header]) headers[header] = req.headers[header]
89+
}
90+
91+
if (!path) return respondInternalServerError(req, res)
92+
await jsonPut(path, {}, managedClusterToken)
93+
.then((results) => {
94+
if (results?.statusCode >= 200 && results?.statusCode < 300) {
95+
res.setHeader('Content-Type', 'application/json')
96+
res.end(JSON.stringify(results))
97+
} else {
98+
logger.error({
99+
msg: 'Error in VirtualMachine action response',
100+
error: results.body.message,
101+
})
102+
res.setHeader('Content-Type', 'application/json')
103+
res.writeHead(results.statusCode ?? HTTP_STATUS_INTERNAL_SERVER_ERROR)
104+
delete results.body?.code // code is added via writeHead
105+
res.end(JSON.stringify(results.body))
106+
}
107+
})
108+
.catch((err: Error) => {
109+
logger.error({ msg: 'Error on VirtualMachine action request', error: err.message })
110+
respondInternalServerError(req, res)
111+
return undefined
112+
})
113+
} else {
114+
logger.error({ msg: `Unauthorized request ${req.url.split('/')[2]} on VirtualMachine ${body.vmName}` })
115+
return respond(res, `Unauthorized request ${req.url.split('/')[2]} on VirtualMachine ${body.vmName}`, 401)
116+
}
100117
})
101118
} catch (err) {
102119
logger.error(err)

‎backend/test/routes/virtualMachineProxy.test.ts

+30
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,16 @@ import { request } from '../mock-request'
66
describe('Virtual Machine actions', function () {
77
it('should successfully call start action', async function () {
88
nock(process.env.CLUSTER_API_URL).get('/apis').reply(200)
9+
nock(process.env.CLUSTER_API_URL)
10+
.post(
11+
'/apis/authorization.k8s.io/v1/selfsubjectaccessreviews',
12+
'{"apiVersion":"authorization.k8s.io/v1","kind":"SelfSubjectAccessReview","metadata":{},"spec":{"resourceAttributes":{"group":"action.open-cluster-management.io","namespace":"testCluster","resource":"managedclusteractions","verb":"create"}}}'
13+
)
14+
.reply(200, {
15+
status: {
16+
allowed: true,
17+
},
18+
})
919
nock(process.env.CLUSTER_API_URL)
1020
.get('/api/v1/namespaces/testCluster/secrets')
1121
.reply(200, {
@@ -69,6 +79,16 @@ describe('Virtual Machine actions', function () {
6979

7080
it('should error on start action request', async function () {
7181
nock(process.env.CLUSTER_API_URL).get('/apis').reply(200)
82+
nock(process.env.CLUSTER_API_URL)
83+
.post(
84+
'/apis/authorization.k8s.io/v1/selfsubjectaccessreviews',
85+
'{"apiVersion":"authorization.k8s.io/v1","kind":"SelfSubjectAccessReview","metadata":{},"spec":{"resourceAttributes":{"group":"action.open-cluster-management.io","namespace":"testCluster","resource":"managedclusteractions","verb":"create"}}}'
86+
)
87+
.reply(200, {
88+
status: {
89+
allowed: true,
90+
},
91+
})
7292
nock(process.env.CLUSTER_API_URL)
7393
.get('/api/v1/namespaces/testCluster/secrets')
7494
.reply(200, {
@@ -139,6 +159,16 @@ describe('Virtual Machine actions', function () {
139159

140160
it('should fail with invalid route and secret', async function () {
141161
nock(process.env.CLUSTER_API_URL).get('/apis').reply(200)
162+
nock(process.env.CLUSTER_API_URL)
163+
.post(
164+
'/apis/authorization.k8s.io/v1/selfsubjectaccessreviews',
165+
'{"apiVersion":"authorization.k8s.io/v1","kind":"SelfSubjectAccessReview","metadata":{},"spec":{"resourceAttributes":{"group":"action.open-cluster-management.io","namespace":"testCluster","resource":"managedclusteractions","verb":"create"}}}'
166+
)
167+
.reply(200, {
168+
status: {
169+
allowed: true,
170+
},
171+
})
142172
nock(process.env.CLUSTER_API_URL).get('/api/v1/namespaces/testCluster/secrets').reply(400, {
143173
statusCode: 400,
144174
apiVersion: 'v1',

‎frontend/src/routes/Search/SearchResults/utils.test.tsx

+55-2
Original file line numberDiff line numberDiff line change
@@ -230,8 +230,61 @@ test('Correctly return VirtualMachine with actions disabled', () => {
230230
)
231231
expect(res).toMatchSnapshot()
232232
})
233-
test('should handle vm action buttons', () => {
234-
const item = { managedHub: 'cluster1' }
233+
test('should handle managed vm action buttons', () => {
234+
const item = {
235+
_uid: 'cluster1/42634581-0cc1-4aa9-bec6-69f59049e2d3',
236+
apigroup: 'kubevirt.io',
237+
apiversion: 'v1',
238+
cluster: 'cluster1',
239+
created: '2024-09-09T20:00:42Z',
240+
kind: 'VirtualMachine',
241+
kind_plural: 'virtualmachines',
242+
name: 'centos7-gray-owl-35',
243+
namespace: 'openshift-cnv',
244+
ready: 'False',
245+
status: 'Paused',
246+
}
247+
const vmActionsEnabled = true
248+
const actions = getRowActions(
249+
'VirtualMachine',
250+
'kind:VirtualMachine',
251+
false,
252+
() => {},
253+
() => {},
254+
allClusters,
255+
navigate,
256+
toastContextMock,
257+
vmActionsEnabled,
258+
t
259+
)
260+
const startVMAction = actions.find((action) => action.id === 'startVM')
261+
const stopVMAction = actions.find((action) => action.id === 'stopVM')
262+
const restartVMAction = actions.find((action) => action.id === 'restartVM')
263+
const pauseVMAction = actions.find((action) => action.id === 'pauseVM')
264+
const unpauseVMAction = actions.find((action) => action.id === 'unpauseVM')
265+
266+
startVMAction?.click(item)
267+
stopVMAction?.click(item)
268+
restartVMAction?.click(item)
269+
pauseVMAction?.click(item)
270+
unpauseVMAction?.click(item)
271+
})
272+
273+
test('should handle hub vm action buttons', () => {
274+
const item = {
275+
_hubClusterResource: 'true',
276+
_uid: 'local-cluster/42634581-0cc1-4aa9-bec6-69f59049e2d3',
277+
apigroup: 'kubevirt.io',
278+
apiversion: 'v1',
279+
cluster: 'local-cluster',
280+
created: '2024-09-09T20:00:42Z',
281+
kind: 'VirtualMachine',
282+
kind_plural: 'virtualmachines',
283+
name: 'centos7-gray-owl-35',
284+
namespace: 'openshift-cnv',
285+
ready: 'False',
286+
status: 'Paused',
287+
}
235288
const vmActionsEnabled = true
236289
const actions = getRowActions(
237290
'VirtualMachine',

‎frontend/src/routes/Search/SearchResults/utils.tsx

+22-7
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,8 @@ export function handleVMActions(
5555
.catch((err) => {
5656
console.error(`VirtualMachine: ${item.name} ${action} error. ${err}`)
5757

58-
let errMessage = err?.message ?? t('An unexpected error occurred.')
59-
if (errMessage.includes(':')) errMessage = errMessage.split(':')[1]
58+
let errMessage: string = err?.message ?? t('An unexpected error occurred.')
59+
if (errMessage.includes(':')) errMessage = errMessage.split(':').slice(1).join(':')
6060
if (errMessage === 'Unauthorized') errMessage = t('Unauthorized to execute this action.')
6161
toast.addAlert({
6262
title: t('Error triggering action {{action}} on VirtualMachine {{name}}', {
@@ -267,9 +267,12 @@ export function getRowActions(
267267
id: 'startVM',
268268
title: t('Start {{resourceKind}}', { resourceKind }),
269269
click: (item: any) => {
270+
const path = item?._hubClusterResource
271+
? `/apis/subresources.kubevirt.io/v1/namespaces/${item.namespace}/virtualmachines/${item.name}/start`
272+
: `/virtualmachines/start`
270273
handleVMActions(
271274
'start',
272-
'/virtualmachines/start',
275+
path,
273276
item,
274277
() => searchClient.refetchQueries({ include: ['searchResultItems'] }),
275278
toast,
@@ -281,9 +284,12 @@ export function getRowActions(
281284
id: 'stopVM',
282285
title: t('Stop {{resourceKind}}', { resourceKind }),
283286
click: (item: any) => {
287+
const path = item?._hubClusterResource
288+
? `/apis/subresources.kubevirt.io/v1/namespaces/${item.namespace}/virtualmachines/${item.name}/stop`
289+
: `/virtualmachines/stop`
284290
handleVMActions(
285291
'stop',
286-
'/virtualmachines/stop',
292+
path,
287293
item,
288294
() => searchClient.refetchQueries({ include: ['searchResultItems'] }),
289295
toast,
@@ -295,9 +301,12 @@ export function getRowActions(
295301
id: 'restartVM',
296302
title: t('Restart {{resourceKind}}', { resourceKind }),
297303
click: (item: any) => {
304+
const path = item?._hubClusterResource
305+
? `/apis/subresources.kubevirt.io/v1/namespaces/${item.namespace}/virtualmachines/${item.name}/restart`
306+
: `/virtualmachines/restart`
298307
handleVMActions(
299308
'restart',
300-
'/virtualmachines/restart',
309+
path,
301310
item,
302311
() => searchClient.refetchQueries({ include: ['searchResultItems'] }),
303312
toast,
@@ -309,9 +318,12 @@ export function getRowActions(
309318
id: 'pauseVM',
310319
title: t('Pause {{resourceKind}}', { resourceKind }),
311320
click: (item: any) => {
321+
const path = item?._hubClusterResource
322+
? `/apis/subresources.kubevirt.io/v1/namespaces/${item.namespace}/virtualmachineinstances/${item.name}/pause`
323+
: `/virtualmachineinstances/pause`
312324
handleVMActions(
313325
'pause',
314-
'/virtualmachineinstances/pause',
326+
path,
315327
item,
316328
() => searchClient.refetchQueries({ include: ['searchResultItems'] }),
317329
toast,
@@ -323,9 +335,12 @@ export function getRowActions(
323335
id: 'unpauseVM',
324336
title: t('Unpause {{resourceKind}}', { resourceKind }),
325337
click: (item: any) => {
338+
const path = item?._hubClusterResource
339+
? `/apis/subresources.kubevirt.io/v1/namespaces/${item.namespace}/virtualmachineinstances/${item.name}/unpause`
340+
: `/virtualmachineinstances/unpause`
326341
handleVMActions(
327342
'unpause',
328-
'/virtualmachineinstances/unpause',
343+
path,
329344
item,
330345
() => searchClient.refetchQueries({ include: ['searchResultItems'] }),
331346
toast,

0 commit comments

Comments
 (0)