Skip to content

Commit 11232dc

Browse files
authored
Fix error handling of VirtualMachine actions (stolostron#3972)
* Fix error handling of VirtualMachine actions Signed-off-by: zlayne <zlayne@redhat.com> * update test Signed-off-by: zlayne <zlayne@redhat.com> --------- Signed-off-by: zlayne <zlayne@redhat.com>
1 parent 53e8c5b commit 11232dc

File tree

3 files changed

+104
-10
lines changed

3 files changed

+104
-10
lines changed

backend/src/lib/json-request.ts

+25-8
Original file line numberDiff line numberDiff line change
@@ -76,15 +76,32 @@ export function jsonPut(url: string, body: unknown, token?: string): Promise<Put
7676
compress: true,
7777
})
7878
.then(async (response) => {
79-
try {
80-
const resBody = (await response.json()) as unknown
81-
return {
82-
statusCode: response.status,
83-
body: resBody,
79+
let responseData = undefined
80+
if (response.headers.get('content-type')?.includes('text/plain')) {
81+
try {
82+
responseData = await response.text()
83+
return {
84+
statusCode: response.status,
85+
body: responseData,
86+
}
87+
} catch (err) {
88+
return {
89+
statusCode: response.status,
90+
body: 'Error getting resource text response.',
91+
}
8492
}
85-
} catch (err) {
86-
return {
87-
statusCode: response.status,
93+
} else {
94+
try {
95+
responseData = (await response.json()) as unknown
96+
return {
97+
statusCode: response.status,
98+
body: responseData,
99+
}
100+
} catch (err) {
101+
return {
102+
statusCode: response.status,
103+
body: 'Error getting resource json response.',
104+
}
88105
}
89106
}
90107
})

backend/src/routes/virtualMachineProxy.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -97,12 +97,12 @@ export async function virtualMachineProxy(req: Http2ServerRequest, res: Http2Ser
9797
} else {
9898
logger.error({
9999
msg: 'Error in VirtualMachine action response',
100-
error: results.body.message,
100+
error: results.body?.message ?? '',
101101
})
102102
res.setHeader('Content-Type', 'application/json')
103103
res.writeHead(results.statusCode ?? HTTP_STATUS_INTERNAL_SERVER_ERROR)
104104
delete results.body?.code // code is added via writeHead
105-
res.end(JSON.stringify(results.body))
105+
res.end(JSON.stringify(results.body ?? ''))
106106
}
107107
})
108108
.catch((err: Error) => {

backend/test/routes/virtualMachineProxy.test.ts

+77
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,14 @@
11
/* Copyright Contributors to the Open Cluster Management project */
2+
import { constants } from 'http2'
23
import nock from 'nock'
34
import { parsePipedJsonBody } from '../../src/lib/body-parser'
45
import { request } from '../mock-request'
56

67
describe('Virtual Machine actions', function () {
8+
afterEach(() => {
9+
nock.cleanAll()
10+
})
11+
712
it('should successfully call start action', async function () {
813
nock(process.env.CLUSTER_API_URL).get('/apis').reply(200)
914
nock(process.env.CLUSTER_API_URL)
@@ -191,4 +196,76 @@ describe('Virtual Machine actions', function () {
191196
})
192197
expect(res.statusCode).toEqual(500)
193198
})
199+
200+
it('should fail with 502 - content type text/plain error', async function () {
201+
nock(process.env.CLUSTER_API_URL).get('/apis').reply(200)
202+
nock(process.env.CLUSTER_API_URL)
203+
.post(
204+
'/apis/authorization.k8s.io/v1/selfsubjectaccessreviews',
205+
'{"apiVersion":"authorization.k8s.io/v1","kind":"SelfSubjectAccessReview","metadata":{},"spec":{"resourceAttributes":{"group":"action.open-cluster-management.io","namespace":"testCluster","resource":"managedclusteractions","verb":"create"}}}'
206+
)
207+
.reply(200, {
208+
status: {
209+
allowed: true,
210+
},
211+
})
212+
nock(process.env.CLUSTER_API_URL)
213+
.get('/api/v1/namespaces/testCluster/secrets')
214+
.reply(200, {
215+
statusCode: 200,
216+
apiVersion: 'v1',
217+
kind: 'SecretList',
218+
items: [
219+
{
220+
kind: 'Secret',
221+
apiVersion: 'v1',
222+
metadata: {
223+
name: 'vm-actor',
224+
namespace: 'testCluster',
225+
},
226+
data: {
227+
// test-vm-token
228+
token: 'dGVzdC12bS10b2tlbg==', // notsecret
229+
},
230+
type: 'Opaque',
231+
},
232+
],
233+
})
234+
nock(process.env.CLUSTER_API_URL)
235+
.get('/apis/route.openshift.io/v1/namespaces/multicluster-engine/routes/cluster-proxy-addon-user')
236+
.reply(200, {
237+
kind: 'Route',
238+
apiVersion: 'route.openshift.io/v1',
239+
metadata: {
240+
name: 'cluster-proxy-addon-user',
241+
namespace: 'multicluster-engine',
242+
},
243+
spec: {
244+
host: 'testCluster.red-chesterfield.com',
245+
to: {
246+
kind: 'Service',
247+
name: 'cluster-proxy-addon-user',
248+
weight: 100,
249+
},
250+
port: {
251+
targetPort: 'user-port',
252+
},
253+
tls: {
254+
termination: 'reencrypt',
255+
insecureEdgeTerminationPolicy: 'Redirect',
256+
},
257+
wildcardPolicy: 'None',
258+
},
259+
})
260+
nock('https://testcluster.red-chesterfield.com')
261+
.put('/testCluster/apis/subresources.kubevirt.io/v1/namespaces/vmNamespace/virtualmachines/vmName/start')
262+
.reply(502, 'plain text error', { [constants.HTTP2_HEADER_CONTENT_TYPE]: ['text/plain'] })
263+
const res = await request('PUT', '/virtualmachines/start', {
264+
managedCluster: 'testCluster',
265+
vmName: 'vmName',
266+
vmNamespace: 'vmNamespace',
267+
})
268+
expect(res.statusCode).toEqual(502)
269+
expect(await parsePipedJsonBody(res)).toEqual('plain text error')
270+
})
194271
})

0 commit comments

Comments
 (0)