From d9a5aed101f006866f144ad6bc5382d26095e38f Mon Sep 17 00:00:00 2001 From: DarkIsDude Date: Fri, 27 Mar 2026 14:32:33 +0100 Subject: [PATCH 01/12] =?UTF-8?q?=F0=9F=90=9B=20use=20SUR=20date=20instead?= =?UTF-8?q?=20of=20the=20infostore=20one=20to=20have=20an=20update=20LastM?= =?UTF-8?q?odified=20date?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue: CLDSRV-878 --- lib/routes/veeam/get.js | 10 ++++++---- lib/routes/veeam/head.js | 19 +++++++++++++------ lib/routes/veeam/utils.js | 6 ++++-- tests/unit/api/apiUtils/objectLockHelpers.js | 2 +- tests/unit/routes/veeam-routes.js | 12 ++++++++++++ 5 files changed, 36 insertions(+), 13 deletions(-) diff --git a/lib/routes/veeam/get.js b/lib/routes/veeam/get.js index d802a29dfb..c05618816b 100644 --- a/lib/routes/veeam/get.js +++ b/lib/routes/veeam/get.js @@ -34,7 +34,7 @@ function getVeeamFile(request, response, bucketMd, log) { // Extract the last modified date, but do not include it when computing // the file's ETag (md5) - const modified = fileToBuild.value.LastModified; + const modified = bucketMetrics?.date || fileToBuild.value.LastModified; delete fileToBuild.value.LastModified; // The SOSAPI metrics are dynamically computed using the SUR backend. @@ -53,15 +53,14 @@ function getVeeamFile(request, response, bucketMd, log) { return respondWithData(request, response, log, data, buildHeadXML(builder.buildObject(fileToBuild.value)), modified); }; + if (!isSystemXML(request.objectKey)) { const bucketKey = `${bucketMd._name}_${new Date(bucketMd._creationDate).getTime()}`; return UtilizationService.getUtilizationMetrics('bucket', bucketKey, null, {}, (err, bucketMetrics) => { if (err) { - // Handle errors from UtilizationService/scubaclient - // axios errors have status in err.response.status const statusCode = err.response?.status || err.statusCode || err.code; + // Only handle 404 gracefully (no metrics available yet, e.g. post-install) - // For 404, continue with static capacity data (Used=0 from bucket metadata) if (statusCode === 404) { log.warn('UtilizationService returned 404 when fetching capacity metrics', { method: 'getVeeamFile', @@ -70,14 +69,17 @@ function getVeeamFile(request, response, bucketMd, log) { }); return finalizeRequest(); } + log.error('error fetching capacity metrics from UtilizationService', { method: 'getVeeamFile', bucket: request.bucketName, error: err.message || err.code, statusCode, }); + return responseXMLBody(errors.InternalError, null, response, log); } + return finalizeRequest(bucketMetrics); }); } diff --git a/lib/routes/veeam/head.js b/lib/routes/veeam/head.js index b94624b364..52de2110ed 100644 --- a/lib/routes/veeam/head.js +++ b/lib/routes/veeam/head.js @@ -17,26 +17,33 @@ function headVeeamFile(request, response, bucketMd, log) { if (!bucketMd) { return responseXMLBody(errors.NoSuchBucket, null, response, log); } + return metadata.getBucket(request.bucketName, log, (err, data) => { if (err) { return responseXMLBody(errors.InternalError, null, response, log); } + const fileToBuild = getFileToBuild(request, data._capabilities?.VeeamSOSApi); if (fileToBuild.error) { return responseXMLBody(fileToBuild.error, null, response, log); } - let modified = new Date().toISOString(); - // Extract the last modified date, but do not include it when computing - // the file's ETag (md5) - modified = fileToBuild.value.LastModified; + + // Extract the last modified date, but do not include it when computing the file's ETag (md5) + const modified = fileToBuild.value.LastModified; delete fileToBuild.value.LastModified; // Recompute file content to generate appropriate content-md5 header const builder = new xml2js.Builder({ headless: true, }); const dataBuffer = Buffer.from(buildHeadXML(builder.buildObject(fileToBuild))); - return responseContentHeaders(null, {}, getResponseHeader(request, data, - dataBuffer, modified, log), response, log); + + return responseContentHeaders( + null, + {}, + getResponseHeader(request, data, dataBuffer, modified, log), + response, + log, + ); }); } diff --git a/lib/routes/veeam/utils.js b/lib/routes/veeam/utils.js index 5ab082c6d5..188b534d61 100644 --- a/lib/routes/veeam/utils.js +++ b/lib/routes/veeam/utils.js @@ -172,13 +172,15 @@ function isSystemXML(objectKey) { */ function getFileToBuild(request, data, inlineLastModified = false) { const _isSystemXML = isSystemXML(request.objectKey); - const fileToBuild = _isSystemXML ? data?.SystemInfo - : data?.CapacityInfo; + const fileToBuild = _isSystemXML ? data?.SystemInfo : data?.CapacityInfo; + if (!fileToBuild) { return { error: errors.NoSuchKey }; } + const modified = fileToBuild.LastModified || (new Date()).toISOString(); const fieldName = _isSystemXML ? 'SystemInfo' : 'CapacityInfo'; + if (inlineLastModified) { fileToBuild.LastModified = modified; return { diff --git a/tests/unit/api/apiUtils/objectLockHelpers.js b/tests/unit/api/apiUtils/objectLockHelpers.js index c0c445cef6..285efc9c8f 100644 --- a/tests/unit/api/apiUtils/objectLockHelpers.js +++ b/tests/unit/api/apiUtils/objectLockHelpers.js @@ -172,7 +172,7 @@ describe('objectLockHelpers: calculateRetainUntilDate', () => { }; const date = moment(); const expectedRetainUntilDate - = date.add(mockConfigWithYears.years * 365, 'days'); + = date.add(mockConfigWithYears.years * 365 * 86400000, 'ms'); const retainUntilDate = calculateRetainUntilDate(mockConfigWithYears); assert.strictEqual(retainUntilDate.slice(0, 16), expectedRetainUntilDate.toISOString().slice(0, 16)); diff --git a/tests/unit/routes/veeam-routes.js b/tests/unit/routes/veeam-routes.js index d5f9a7a01b..6d4b8d779b 100644 --- a/tests/unit/routes/veeam-routes.js +++ b/tests/unit/routes/veeam-routes.js @@ -164,8 +164,10 @@ describe('Veeam routes - comprehensive unit tests', () => { }); it('should successfully use metrics when UtilizationService returns data', done => { + const metricsDate = '2026-03-26T19:00:08.996Z'; const bucketMetrics = { bytesTotal: 123456789, + date: metricsDate, }; utilizationStub.callsArgWith(4, null, bucketMetrics); @@ -179,6 +181,16 @@ describe('Veeam routes - comprehensive unit tests', () => { assert(response.writeHead.calledWith(200), 'should return 200 with metrics'); assert(utilizationStub.calledOnce, 'should call UtilizationService once'); assert(response.end.called, 'response should be ended'); + + const lastModifiedCall = response.setHeader.getCalls() + .find(call => call.args[0] === 'Last-Modified'); + + assert(lastModifiedCall, 'Last-Modified header should be set'); + assert.strictEqual( + lastModifiedCall.args[1], + new Date(metricsDate).toUTCString(), + 'Last-Modified should use the date from bucketMetrics', + ); done(); }); }); From 4a70cf77a5ce62079f4c9983b510615986c101f1 Mon Sep 17 00:00:00 2001 From: DarkIsDude Date: Tue, 31 Mar 2026 15:11:19 +0200 Subject: [PATCH 02/12] =?UTF-8?q?=F0=9F=90=9B=20call=20UtilizationService?= =?UTF-8?q?=20on=20HEAD=20and=20LIST=20veeam=20routes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Call UtilizationService.getUtilizationMetrics for capacity.xml on the HEAD and LIST routes, mirroring the existing pattern from GET. This ensures Last-Modified is derived from SUR metrics (not stale metadata) on all three routes. Fall back to new Date() when metrics are unavailable (404) or absent. Also use new Date() as the fallback in GET instead of the stored LastModified value. Issue: CLDSRV-878 --- lib/routes/veeam/get.js | 2 +- lib/routes/veeam/head.js | 82 ++++++++++++++++------ lib/routes/veeam/list.js | 102 +++++++++++++++++++-------- tests/unit/routes/veeam-routes.js | 111 ++++++++++++++++++++++++++++-- 4 files changed, 243 insertions(+), 54 deletions(-) diff --git a/lib/routes/veeam/get.js b/lib/routes/veeam/get.js index c05618816b..e8ee34a7e5 100644 --- a/lib/routes/veeam/get.js +++ b/lib/routes/veeam/get.js @@ -34,7 +34,7 @@ function getVeeamFile(request, response, bucketMd, log) { // Extract the last modified date, but do not include it when computing // the file's ETag (md5) - const modified = bucketMetrics?.date || fileToBuild.value.LastModified; + const modified = bucketMetrics?.date || new Date(); delete fileToBuild.value.LastModified; // The SOSAPI metrics are dynamically computed using the SUR backend. diff --git a/lib/routes/veeam/head.js b/lib/routes/veeam/head.js index 52de2110ed..ae1bba200b 100644 --- a/lib/routes/veeam/head.js +++ b/lib/routes/veeam/head.js @@ -1,8 +1,9 @@ const xml2js = require('xml2js'); const { errors } = require('arsenal'); const metadata = require('../../metadata/wrapper'); -const { getResponseHeader, buildHeadXML, getFileToBuild } = require('./utils'); +const { getResponseHeader, buildHeadXML, getFileToBuild, isSystemXML } = require('./utils'); const { responseXMLBody, responseContentHeaders } = require('arsenal/build/lib/s3routes/routesUtils'); +const UtilizationService = require('../../../lib/utilization/instance'); /** * Returns system.xml or capacity.xml files metadata for a given bucket. @@ -23,27 +24,68 @@ function headVeeamFile(request, response, bucketMd, log) { return responseXMLBody(errors.InternalError, null, response, log); } - const fileToBuild = getFileToBuild(request, data._capabilities?.VeeamSOSApi); - if (fileToBuild.error) { - return responseXMLBody(fileToBuild.error, null, response, log); + const finalizeRequest = bucketMetrics => { + const fileToBuild = getFileToBuild(request, data._capabilities?.VeeamSOSApi); + if (fileToBuild.error) { + return responseXMLBody(fileToBuild.error, null, response, log); + } + + // Extract the last modified date, but do not include it when computing the file's ETag (md5) + const modified = bucketMetrics?.date || new Date(); + delete fileToBuild.value.LastModified; + const builder = new xml2js.Builder({ + headless: true, + }); + const dataBuffer = Buffer.from(buildHeadXML(builder.buildObject(fileToBuild))); + + return responseContentHeaders( + null, + {}, + getResponseHeader(request, data, dataBuffer, modified, log), + response, + log, + ); + }; + + if (!isSystemXML(request.objectKey)) { + const bucketKey = `${bucketMd._name}_${new Date(bucketMd._creationDate).getTime()}`; + + return UtilizationService.getUtilizationMetrics( + 'bucket', + bucketKey, + null, + {}, + (utilizationErr, bucketMetrics) => { + if (utilizationErr) { + const statusCode = utilizationErr.response?.status || + utilizationErr.statusCode || + utilizationErr.code; + + if (statusCode === 404) { + log.warn('UtilizationService returned 404 when fetching capacity metrics', { + method: 'headVeeamFile', + bucket: request.bucketName, + error: utilizationErr.message || utilizationErr.code, + }); + return finalizeRequest(); + } + + log.error('error fetching capacity metrics from UtilizationService', { + method: 'headVeeamFile', + bucket: request.bucketName, + error: utilizationErr.message || utilizationErr.code, + statusCode, + }); + + return responseXMLBody(errors.InternalError, null, response, log); + } + + return finalizeRequest(bucketMetrics); + }, + ); } - // Extract the last modified date, but do not include it when computing the file's ETag (md5) - const modified = fileToBuild.value.LastModified; - delete fileToBuild.value.LastModified; - // Recompute file content to generate appropriate content-md5 header - const builder = new xml2js.Builder({ - headless: true, - }); - const dataBuffer = Buffer.from(buildHeadXML(builder.buildObject(fileToBuild))); - - return responseContentHeaders( - null, - {}, - getResponseHeader(request, data, dataBuffer, modified, log), - response, - log, - ); + return finalizeRequest(); }); } diff --git a/lib/routes/veeam/list.js b/lib/routes/veeam/list.js index 5fd8585bfb..905dbaa3cd 100644 --- a/lib/routes/veeam/list.js +++ b/lib/routes/veeam/list.js @@ -6,6 +6,7 @@ const metadata = require('../../metadata/wrapper'); const { responseXMLBody } = require('arsenal/build/lib/s3routes/routesUtils'); const { respondWithData, getResponseHeader, buildHeadXML, validPath } = require('./utils'); const { processVersions, processMasterVersions } = require('../../api/bucketGet'); +const UtilizationService = require('../../../lib/utilization/instance'); /** @@ -96,37 +97,82 @@ function listVeeamFiles(request, response, bucketMd, log) { if (err) { return responseXMLBody(errors.InternalError, null, response, log); } - const filesToBuild = []; - const fieldsToGenerate = []; - if (data._capabilities?.VeeamSOSApi?.SystemInfo) { - fieldsToGenerate.push({ - ...data._capabilities?.VeeamSOSApi?.SystemInfo, - name: `${validPath}system.xml`, + + const buildAndRespond = bucketMetrics => { + const filesToBuild = []; + const fieldsToGenerate = []; + if (data._capabilities?.VeeamSOSApi?.SystemInfo) { + fieldsToGenerate.push({ + ...data._capabilities?.VeeamSOSApi?.SystemInfo, + name: `${validPath}system.xml`, + }); + } + if (data._capabilities?.VeeamSOSApi?.CapacityInfo) { + fieldsToGenerate.push({ + ...data._capabilities?.VeeamSOSApi?.CapacityInfo, + name: `${validPath}capacity.xml`, + }); + } + fieldsToGenerate.forEach(file => { + const isCapacity = file.name.endsWith('capacity.xml'); + const lastModified = isCapacity + ? (bucketMetrics?.date || new Date()) + : file.LastModified; + // eslint-disable-next-line no-param-reassign + delete file.LastModified; + const builder = new xml2js.Builder({ + headless: true, + }); + const dataBuffer = Buffer.from(buildHeadXML(builder.buildObject(file))); + filesToBuild.push({ + ...getResponseHeader(request, data, + dataBuffer, lastModified, log), + name: file.name, + }); }); - } + // When `versions` is present, listing should return a versioned list + return respondWithData(request, response, log, data, + buildXMLResponse(request, filesToBuild, 'versions' in request.query)); + }; + if (data._capabilities?.VeeamSOSApi?.CapacityInfo) { - fieldsToGenerate.push({ - ...data._capabilities?.VeeamSOSApi?.CapacityInfo, - name: `${validPath}capacity.xml`, - }); + const bucketKey = `${bucketMd._name}_${new Date(bucketMd._creationDate).getTime()}`; + + return UtilizationService.getUtilizationMetrics( + 'bucket', + bucketKey, + null, + {}, + (utilizationErr, bucketMetrics) => { + if (utilizationErr) { + const statusCode = utilizationErr.response?.status || + utilizationErr.statusCode || + utilizationErr.code; + + if (statusCode === 404) { + log.warn('UtilizationService returned 404 when fetching capacity metrics', { + method: 'listVeeamFiles', + bucket: request.bucketName, + error: utilizationErr.message || utilizationErr.code, + }); + return buildAndRespond(); + } + + log.error('error fetching capacity metrics from UtilizationService', { + method: 'listVeeamFiles', + bucket: request.bucketName, + error: utilizationErr.message || utilizationErr.code, + statusCode, + }); + + return responseXMLBody(errors.InternalError, null, response, log); + } + + return buildAndRespond(bucketMetrics); + }, + ); } - fieldsToGenerate.forEach(file => { - const lastModified = file.LastModified; - // eslint-disable-next-line no-param-reassign - delete file.LastModified; - const builder = new xml2js.Builder({ - headless: true, - }); - const dataBuffer = Buffer.from(buildHeadXML(builder.buildObject(file))); - filesToBuild.push({ - ...getResponseHeader(request, data, - dataBuffer, lastModified, log), - name: file.name, - }); - }); - // When `versions` is present, listing should return a versioned list - return respondWithData(request, response, log, data, - buildXMLResponse(request, filesToBuild, 'versions' in request.query)); + return buildAndRespond(); }); } diff --git a/tests/unit/routes/veeam-routes.js b/tests/unit/routes/veeam-routes.js index 6d4b8d779b..93517f130c 100644 --- a/tests/unit/routes/veeam-routes.js +++ b/tests/unit/routes/veeam-routes.js @@ -242,7 +242,7 @@ describe('Veeam routes - comprehensive unit tests', () => { assert(response.end.called, 'response should be ended'); const warnCall = logWarnSpy.getCall(0); assert(warnCall.args[0].includes('404'), 'warning should mention 404'); - + done(); }); }); @@ -280,6 +280,7 @@ describe('Veeam routes - comprehensive unit tests', () => { }); describe('Veeam routes - HEAD request UtilizationService error handling', () => { + let utilizationStub; let metadataStub; let log; let logWarnSpy; @@ -312,6 +313,7 @@ describe('Veeam routes - HEAD request UtilizationService error handling', () => log.end = sinon.stub().returns(logEndStub); log.debug = sinon.stub(); + utilizationStub = sinon.stub(UtilizationService, 'getUtilizationMetrics'); metadataStub = sinon.stub(metadata, 'getBucket'); metadataStub.callsArgWith(2, null, bucketMd, undefined); }); @@ -350,7 +352,7 @@ describe('Veeam routes - HEAD request UtilizationService error handling', () => return response; }; - it('should handle HEAD request for system.xml', done => { + it('should handle HEAD request for system.xml without calling UtilizationService', done => { const bucketMdWithSystem = { ...bucketMd, _capabilities: { @@ -371,31 +373,84 @@ describe('Veeam routes - HEAD request UtilizationService error handling', () => headVeeamFile(request, response, bucketMdWithSystem, log); giveAsyncCallbackTimeToExecute(() => { + assert(!utilizationStub.called, 'should not call UtilizationService for system.xml'); assert(response.setHeader.called, 'should set headers'); assert(response.end.called, 'response should be ended'); done(); }); }); - it('should handle HEAD request for capacity.xml', done => { + it('should call UtilizationService for capacity.xml and use metrics date', done => { + const metricsDate = '2026-03-26T19:00:08.996Z'; + utilizationStub.callsArgWith(4, null, { bytesTotal: 123456789, date: metricsDate }); + + const request = createRequest('.system-d26a9498-cb7c-4a87-a44a-8ae204f5ba6c/capacity.xml'); + const response = createResponse(); + + headVeeamFile(request, response, bucketMd, log); + + giveAsyncCallbackTimeToExecute(() => { + assert(utilizationStub.calledOnce, 'should call UtilizationService once'); + assert(response.setHeader.called, 'should set headers'); + assert(response.end.called, 'response should be ended'); + + const lastModifiedCall = response.setHeader.getCalls() + .find(call => call.args[0] === 'Last-Modified'); + assert(lastModifiedCall, 'Last-Modified header should be set'); + assert.strictEqual( + lastModifiedCall.args[1], + new Date(metricsDate).toUTCString(), + 'Last-Modified should use the date from bucketMetrics', + ); + done(); + }); + }); + + it('should handle 404 from UtilizationService on HEAD and return 200', done => { + const error404 = new Error('Not Found'); + error404.response = { status: 404 }; + utilizationStub.callsArgWith(4, error404); + const request = createRequest('.system-d26a9498-cb7c-4a87-a44a-8ae204f5ba6c/capacity.xml'); const response = createResponse(); headVeeamFile(request, response, bucketMd, log); giveAsyncCallbackTimeToExecute(() => { + assert(logWarnSpy.calledOnce, 'log.warn should have been called once'); + const warnCall = logWarnSpy.getCall(0); + assert(warnCall.args[0].includes('UtilizationService returned 404'), + 'warning message should mention 404'); + assert.strictEqual(warnCall.args[1].method, 'headVeeamFile'); assert(response.setHeader.called, 'should set headers'); assert(response.end.called, 'response should be ended'); done(); }); }); + it('should handle non-404 error from UtilizationService on HEAD and return 500', done => { + const error500 = new Error('Internal Server Error'); + error500.response = { status: 500 }; + utilizationStub.callsArgWith(4, error500); + + const request = createRequest('.system-d26a9498-cb7c-4a87-a44a-8ae204f5ba6c/capacity.xml'); + const response = createResponse(); + + headVeeamFile(request, response, bucketMd, log); + + giveAsyncCallbackTimeToExecute(() => { + assert(response.end.called, 'response should be ended'); + done(); + }); + }); + it('should return 404 when no VeeamSOSApi capabilities', done => { const bucketMdWithoutVeeam = { ...bucketMd, _capabilities: {}, }; metadataStub.callsArgWith(2, null, bucketMdWithoutVeeam); + utilizationStub.callsArgWith(4, null, {}); const request = createRequest(); const response = createResponse(); @@ -411,6 +466,7 @@ describe('Veeam routes - HEAD request UtilizationService error handling', () => }); describe('Veeam routes - LIST request handling', () => { + let utilizationStub; let metadataStub; let log; let logWarnSpy; @@ -449,6 +505,7 @@ describe('Veeam routes - LIST request handling', () => { log.debug = sinon.stub(); log.trace = sinon.stub(); + utilizationStub = sinon.stub(UtilizationService, 'getUtilizationMetrics'); metadataStub = sinon.stub(metadata, 'getBucket'); metadataStub.callsArgWith(2, null, bucketMd, undefined); }); @@ -493,19 +550,63 @@ describe('Veeam routes - LIST request handling', () => { }; it('should list both system.xml and capacity.xml when both are present', done => { + const metricsDate = '2026-03-26T19:00:08.996Z'; + utilizationStub.callsArgWith(4, null, { bytesTotal: 123456789, date: metricsDate }); + const request = createRequest(); const response = createResponse(); listVeeamFiles(request, response, bucketMd, log); giveAsyncCallbackTimeToExecute(() => { + assert(utilizationStub.calledOnce, 'should call UtilizationService once'); assert(response.writeHead.calledWith(200), 'should return 200'); assert(response.end.called, 'response should be ended'); done(); }); }); + it('should handle 404 from UtilizationService on LIST and return 200', done => { + const error404 = new Error('Not Found'); + error404.response = { status: 404 }; + utilizationStub.callsArgWith(4, error404); + + const request = createRequest(); + const response = createResponse(); + + listVeeamFiles(request, response, bucketMd, log); + + giveAsyncCallbackTimeToExecute(() => { + assert(logWarnSpy.calledOnce, 'log.warn should have been called once'); + const warnCall = logWarnSpy.getCall(0); + assert(warnCall.args[0].includes('UtilizationService returned 404'), + 'warning message should mention 404'); + assert.strictEqual(warnCall.args[1].method, 'listVeeamFiles'); + assert(response.writeHead.calledWith(200), 'should return 200 despite 404'); + assert(response.end.called, 'response should be ended'); + done(); + }); + }); + + it('should handle non-404 error from UtilizationService on LIST and return 500', done => { + const error500 = new Error('Internal Server Error'); + error500.response = { status: 500 }; + utilizationStub.callsArgWith(4, error500); + + const request = createRequest(); + const response = createResponse(); + + listVeeamFiles(request, response, bucketMd, log); + + giveAsyncCallbackTimeToExecute(() => { + assert(response.end.called, 'response should be ended'); + done(); + }); + }); + it('should handle versions query parameter', done => { + utilizationStub.callsArgWith(4, null, { bytesTotal: 0, date: new Date().toISOString() }); + const request = createRequest({ versions: '' }); const response = createResponse(); @@ -544,7 +645,7 @@ describe('Veeam routes - LIST request handling', () => { }); }); - it('should list only available files when some capabilities are missing', done => { + it('should list only available files when only SystemInfo is present, without calling UtilizationService', done => { const bucketMdOnlySystem = { ...bucketMd, _capabilities: { @@ -565,10 +666,10 @@ describe('Veeam routes - LIST request handling', () => { listVeeamFiles(request, response, bucketMdOnlySystem, log); giveAsyncCallbackTimeToExecute(() => { + assert(!utilizationStub.called, 'should not call UtilizationService without CapacityInfo'); assert(response.writeHead.calledWith(200), 'should return 200'); assert(response.end.called, 'response should be ended'); done(); }); }); }); - From 225cf9c501d5970e40a0d5fc185b659dce274b9d Mon Sep 17 00:00:00 2001 From: DarkIsDude Date: Tue, 31 Mar 2026 15:32:42 +0200 Subject: [PATCH 03/12] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20extract=20fetchCapac?= =?UTF-8?q?ityMetrics=20helper=20to=20veeam=20utils?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move the shared UtilizationService call pattern (bucket key derivation, 404 warn/fallback, error logging) into a single fetchCapacityMetrics helper in utils.js, used by GET, HEAD, and LIST routes. Issue: CLDSRV-878 --- lib/routes/veeam/get.js | 26 ++--------------------- lib/routes/veeam/head.js | 44 +++++++-------------------------------- lib/routes/veeam/list.js | 44 +++++++-------------------------------- lib/routes/veeam/utils.js | 39 ++++++++++++++++++++++++++++++++++ 4 files changed, 55 insertions(+), 98 deletions(-) diff --git a/lib/routes/veeam/get.js b/lib/routes/veeam/get.js index e8ee34a7e5..fda7bbb59d 100644 --- a/lib/routes/veeam/get.js +++ b/lib/routes/veeam/get.js @@ -1,9 +1,8 @@ const xml2js = require('xml2js'); const { errors } = require('arsenal'); const metadata = require('../../metadata/wrapper'); -const { respondWithData, buildHeadXML, getFileToBuild, isSystemXML } = require('./utils'); +const { respondWithData, buildHeadXML, getFileToBuild, isSystemXML, fetchCapacityMetrics } = require('./utils'); const { responseXMLBody } = require('arsenal/build/lib/s3routes/routesUtils'); -const UtilizationService = require('../../../lib/utilization/instance'); /** * Returns system.xml or capacity.xml files for a given bucket. @@ -55,31 +54,10 @@ function getVeeamFile(request, response, bucketMd, log) { }; if (!isSystemXML(request.objectKey)) { - const bucketKey = `${bucketMd._name}_${new Date(bucketMd._creationDate).getTime()}`; - return UtilizationService.getUtilizationMetrics('bucket', bucketKey, null, {}, (err, bucketMetrics) => { + return fetchCapacityMetrics(bucketMd, request, log, 'getVeeamFile', (err, bucketMetrics) => { if (err) { - const statusCode = err.response?.status || err.statusCode || err.code; - - // Only handle 404 gracefully (no metrics available yet, e.g. post-install) - if (statusCode === 404) { - log.warn('UtilizationService returned 404 when fetching capacity metrics', { - method: 'getVeeamFile', - bucket: request.bucketName, - error: err.message || err.code, - }); - return finalizeRequest(); - } - - log.error('error fetching capacity metrics from UtilizationService', { - method: 'getVeeamFile', - bucket: request.bucketName, - error: err.message || err.code, - statusCode, - }); - return responseXMLBody(errors.InternalError, null, response, log); } - return finalizeRequest(bucketMetrics); }); } diff --git a/lib/routes/veeam/head.js b/lib/routes/veeam/head.js index ae1bba200b..7698629857 100644 --- a/lib/routes/veeam/head.js +++ b/lib/routes/veeam/head.js @@ -1,9 +1,8 @@ const xml2js = require('xml2js'); const { errors } = require('arsenal'); const metadata = require('../../metadata/wrapper'); -const { getResponseHeader, buildHeadXML, getFileToBuild, isSystemXML } = require('./utils'); +const { getResponseHeader, buildHeadXML, getFileToBuild, isSystemXML, fetchCapacityMetrics } = require('./utils'); const { responseXMLBody, responseContentHeaders } = require('arsenal/build/lib/s3routes/routesUtils'); -const UtilizationService = require('../../../lib/utilization/instance'); /** * Returns system.xml or capacity.xml files metadata for a given bucket. @@ -48,41 +47,12 @@ function headVeeamFile(request, response, bucketMd, log) { }; if (!isSystemXML(request.objectKey)) { - const bucketKey = `${bucketMd._name}_${new Date(bucketMd._creationDate).getTime()}`; - - return UtilizationService.getUtilizationMetrics( - 'bucket', - bucketKey, - null, - {}, - (utilizationErr, bucketMetrics) => { - if (utilizationErr) { - const statusCode = utilizationErr.response?.status || - utilizationErr.statusCode || - utilizationErr.code; - - if (statusCode === 404) { - log.warn('UtilizationService returned 404 when fetching capacity metrics', { - method: 'headVeeamFile', - bucket: request.bucketName, - error: utilizationErr.message || utilizationErr.code, - }); - return finalizeRequest(); - } - - log.error('error fetching capacity metrics from UtilizationService', { - method: 'headVeeamFile', - bucket: request.bucketName, - error: utilizationErr.message || utilizationErr.code, - statusCode, - }); - - return responseXMLBody(errors.InternalError, null, response, log); - } - - return finalizeRequest(bucketMetrics); - }, - ); + return fetchCapacityMetrics(bucketMd, request, log, 'headVeeamFile', (err, bucketMetrics) => { + if (err) { + return responseXMLBody(errors.InternalError, null, response, log); + } + return finalizeRequest(bucketMetrics); + }); } return finalizeRequest(); diff --git a/lib/routes/veeam/list.js b/lib/routes/veeam/list.js index 905dbaa3cd..138d906314 100644 --- a/lib/routes/veeam/list.js +++ b/lib/routes/veeam/list.js @@ -4,9 +4,8 @@ const { errors, errorInstances } = require('arsenal'); const querystring = require('querystring'); const metadata = require('../../metadata/wrapper'); const { responseXMLBody } = require('arsenal/build/lib/s3routes/routesUtils'); -const { respondWithData, getResponseHeader, buildHeadXML, validPath } = require('./utils'); +const { respondWithData, getResponseHeader, buildHeadXML, validPath, fetchCapacityMetrics } = require('./utils'); const { processVersions, processMasterVersions } = require('../../api/bucketGet'); -const UtilizationService = require('../../../lib/utilization/instance'); /** @@ -136,41 +135,12 @@ function listVeeamFiles(request, response, bucketMd, log) { }; if (data._capabilities?.VeeamSOSApi?.CapacityInfo) { - const bucketKey = `${bucketMd._name}_${new Date(bucketMd._creationDate).getTime()}`; - - return UtilizationService.getUtilizationMetrics( - 'bucket', - bucketKey, - null, - {}, - (utilizationErr, bucketMetrics) => { - if (utilizationErr) { - const statusCode = utilizationErr.response?.status || - utilizationErr.statusCode || - utilizationErr.code; - - if (statusCode === 404) { - log.warn('UtilizationService returned 404 when fetching capacity metrics', { - method: 'listVeeamFiles', - bucket: request.bucketName, - error: utilizationErr.message || utilizationErr.code, - }); - return buildAndRespond(); - } - - log.error('error fetching capacity metrics from UtilizationService', { - method: 'listVeeamFiles', - bucket: request.bucketName, - error: utilizationErr.message || utilizationErr.code, - statusCode, - }); - - return responseXMLBody(errors.InternalError, null, response, log); - } - - return buildAndRespond(bucketMetrics); - }, - ); + return fetchCapacityMetrics(bucketMd, request, log, 'listVeeamFiles', (err, bucketMetrics) => { + if (err) { + return responseXMLBody(errors.InternalError, null, response, log); + } + return buildAndRespond(bucketMetrics); + }); } return buildAndRespond(); }); diff --git a/lib/routes/veeam/utils.js b/lib/routes/veeam/utils.js index 188b534d61..b3814d4a7d 100644 --- a/lib/routes/veeam/utils.js +++ b/lib/routes/veeam/utils.js @@ -4,6 +4,7 @@ const collectResponseHeaders = require('../../utilities/collectResponseHeaders') const collectCorsHeaders = require('../../utilities/collectCorsHeaders'); const crypto = require('crypto'); const { prepareStream } = require('arsenal/build/lib/s3middleware/prepareStream'); +const UtilizationService = require('../../utilization/instance'); /** * Decodes an URI and return the result. @@ -201,6 +202,43 @@ function getFileToBuild(request, data, inlineLastModified = false) { } } +/** + * Fetches capacity metrics from UtilizationService for a bucket. + * Handles 404 gracefully (no metrics available yet, e.g. post-install). + * + * @param {object} bucketMd - bucket metadata + * @param {object} request - incoming request + * @param {object} log - logger object + * @param {string} method - calling method name for log context + * @param {function} callback - (err, bucketMetrics) where bucketMetrics + * is undefined when metrics are not available (404) + * @returns {undefined} + */ +function fetchCapacityMetrics(bucketMd, request, log, method, callback) { + const bucketKey = `${bucketMd._name}_${new Date(bucketMd._creationDate).getTime()}`; + return UtilizationService.getUtilizationMetrics('bucket', bucketKey, null, {}, (err, bucketMetrics) => { + if (err) { + const statusCode = err.response?.status || err.statusCode || err.code; + if (statusCode === 404) { + log.warn('UtilizationService returned 404 when fetching capacity metrics', { + method, + bucket: request.bucketName, + error: err.message || err.code, + }); + return callback(null); + } + log.error('error fetching capacity metrics from UtilizationService', { + method, + bucket: request.bucketName, + error: err.message || err.code, + statusCode, + }); + return callback(err); + } + return callback(null, bucketMetrics); + }); +} + module.exports = { _decodeURI, receiveData, @@ -210,4 +248,5 @@ module.exports = { validPath, isSystemXML, getFileToBuild, + fetchCapacityMetrics, }; From b779f1f8ae9c65593c390b39f88e3bc73d8f61ba Mon Sep 17 00:00:00 2001 From: DarkIsDude Date: Tue, 31 Mar 2026 16:24:47 +0200 Subject: [PATCH 04/12] =?UTF-8?q?=E2=9C=85=20add=20unit=20tests=20for=20fe?= =?UTF-8?q?tchCapacityMetrics=20helper?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue: CLDSRV-878 --- tests/unit/routes/veeam-utils.js | 120 +++++++++++++++++++++++++++++++ 1 file changed, 120 insertions(+) create mode 100644 tests/unit/routes/veeam-utils.js diff --git a/tests/unit/routes/veeam-utils.js b/tests/unit/routes/veeam-utils.js new file mode 100644 index 0000000000..bdb5173524 --- /dev/null +++ b/tests/unit/routes/veeam-utils.js @@ -0,0 +1,120 @@ +const assert = require('assert'); +const sinon = require('sinon'); +const UtilizationService = require('../../../lib/utilization/instance'); +const { fetchCapacityMetrics } = require('../../../lib/routes/veeam/utils'); +const { DummyRequestLogger } = require('../helpers'); + +describe('fetchCapacityMetrics', () => { + let utilizationStub; + let log; + let logWarnSpy; + let logErrorSpy; + + const bucketMd = { + _name: 'test-bucket', + _creationDate: '2024-01-01T00:00:00.000Z', + }; + + const request = { + bucketName: 'test-bucket', + }; + + beforeEach(() => { + log = new DummyRequestLogger(); + logWarnSpy = sinon.spy(); + logErrorSpy = sinon.spy(); + log.warn = logWarnSpy; + log.error = logErrorSpy; + + utilizationStub = sinon.stub(UtilizationService, 'getUtilizationMetrics'); + }); + + afterEach(() => { + sinon.restore(); + }); + + it('should call UtilizationService with the correct bucket key', done => { + utilizationStub.callsArgWith(4, null, {}); + + fetchCapacityMetrics(bucketMd, request, log, 'testMethod', () => { + const expectedKey = `test-bucket_${new Date('2024-01-01T00:00:00.000Z').getTime()}`; + assert.strictEqual(utilizationStub.getCall(0).args[0], 'bucket'); + assert.strictEqual(utilizationStub.getCall(0).args[1], expectedKey); + done(); + }); + }); + + it('should call back with metrics on success', done => { + const bucketMetrics = { bytesTotal: 42, date: '2026-03-26T19:00:08.996Z' }; + utilizationStub.callsArgWith(4, null, bucketMetrics); + + fetchCapacityMetrics(bucketMd, request, log, 'testMethod', (err, metrics) => { + assert.ifError(err); + assert.strictEqual(metrics, bucketMetrics); + assert(!logWarnSpy.called); + assert(!logErrorSpy.called); + done(); + }); + }); + + it('should call back with no error and no metrics on 404', done => { + const error404 = new Error('Not Found'); + error404.response = { status: 404 }; + utilizationStub.callsArgWith(4, error404); + + fetchCapacityMetrics(bucketMd, request, log, 'testMethod', (err, metrics) => { + assert.ifError(err); + assert.strictEqual(metrics, undefined); + assert(logWarnSpy.calledOnce); + assert(logWarnSpy.getCall(0).args[0].includes('404')); + assert.strictEqual(logWarnSpy.getCall(0).args[1].method, 'testMethod'); + assert.strictEqual(logWarnSpy.getCall(0).args[1].bucket, 'test-bucket'); + assert(!logErrorSpy.called); + done(); + }); + }); + + it('should also handle 404 via statusCode property', done => { + const error404 = new Error('Not Found'); + error404.statusCode = 404; + utilizationStub.callsArgWith(4, error404); + + fetchCapacityMetrics(bucketMd, request, log, 'testMethod', (err, metrics) => { + assert.ifError(err); + assert.strictEqual(metrics, undefined); + assert(logWarnSpy.calledOnce); + done(); + }); + }); + + it('should call back with error on non-404 failures', done => { + const error500 = new Error('Internal Server Error'); + error500.response = { status: 500 }; + utilizationStub.callsArgWith(4, error500); + + fetchCapacityMetrics(bucketMd, request, log, 'testMethod', (err, metrics) => { + assert.strictEqual(err, error500); + assert.strictEqual(metrics, undefined); + assert(logErrorSpy.calledOnce); + assert.strictEqual(logErrorSpy.getCall(0).args[1].method, 'testMethod'); + assert.strictEqual(logErrorSpy.getCall(0).args[1].bucket, 'test-bucket'); + assert.strictEqual(logErrorSpy.getCall(0).args[1].statusCode, 500); + assert(!logWarnSpy.called); + done(); + }); + }); + + it('should call back with error on connection errors', done => { + const connError = new Error('Connection refused'); + connError.code = 'ECONNREFUSED'; + utilizationStub.callsArgWith(4, connError); + + fetchCapacityMetrics(bucketMd, request, log, 'testMethod', (err, metrics) => { + assert.strictEqual(err, connError); + assert.strictEqual(metrics, undefined); + assert(logErrorSpy.calledOnce); + assert.strictEqual(logErrorSpy.getCall(0).args[1].statusCode, 'ECONNREFUSED'); + done(); + }); + }); +}); From 63a33eb8b54b15cbe60509c51d9cef08e0ae721b Mon Sep 17 00:00:00 2001 From: DarkIsDude Date: Wed, 1 Apr 2026 11:41:29 +0200 Subject: [PATCH 05/12] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20factorise=20logic=20?= =?UTF-8?q?in=20get=20and=20head=20lib/route/veeam?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue: CLDSRV-878 --- lib/routes/veeam/get.js | 47 ++------- lib/routes/veeam/head.js | 48 ++------- lib/routes/veeam/list.js | 8 +- lib/routes/veeam/utils.js | 69 +++++++++++- tests/unit/routes/veeam-utils.js | 174 ++++++++++++++++++++++++++++++- 5 files changed, 256 insertions(+), 90 deletions(-) diff --git a/lib/routes/veeam/get.js b/lib/routes/veeam/get.js index fda7bbb59d..a85af07961 100644 --- a/lib/routes/veeam/get.js +++ b/lib/routes/veeam/get.js @@ -1,7 +1,5 @@ -const xml2js = require('xml2js'); const { errors } = require('arsenal'); -const metadata = require('../../metadata/wrapper'); -const { respondWithData, buildHeadXML, getFileToBuild, isSystemXML, fetchCapacityMetrics } = require('./utils'); +const { respondWithData, buildHeadXML, buildVeeamFileData } = require('./utils'); const { responseXMLBody } = require('arsenal/build/lib/s3routes/routesUtils'); /** @@ -17,51 +15,18 @@ function getVeeamFile(request, response, bucketMd, log) { if (!bucketMd) { return responseXMLBody(errors.NoSuchBucket, null, response, log); } + if ('tagging' in request.query) { return respondWithData(request, response, log, bucketMd, buildHeadXML('')); } - return metadata.getBucket(request.bucketName, log, (err, data) => { + + return buildVeeamFileData(request, bucketMd, log, 'getVeeamFile', (err, result) => { if (err) { - return responseXMLBody(errors.InternalError, null, response, log); + return responseXMLBody(err, null, response, log); } - const finalizeRequest = bucketMetrics => { - const fileToBuild = getFileToBuild(request, data._capabilities?.VeeamSOSApi); - if (fileToBuild.error) { - return responseXMLBody(fileToBuild.error, null, response, log); - } - - // Extract the last modified date, but do not include it when computing - // the file's ETag (md5) - const modified = bucketMetrics?.date || new Date(); - delete fileToBuild.value.LastModified; - // The SOSAPI metrics are dynamically computed using the SUR backend. - if (bucketMetrics && !fileToBuild.value.CapacityInfo?.Used) { - fileToBuild.value.CapacityInfo.Used = Number(bucketMetrics.bytesTotal); - fileToBuild.value.CapacityInfo.Available = - Number(fileToBuild.value.CapacityInfo.Capacity) - Number(bucketMetrics.bytesTotal); - // TODO CLDSRV-633 when SUR backend supports realtime metrics: it will - // report the real last cseq/date processed by SUR, instead of the current date, - // ensuring no issue in a SOSAPI context. We should use this information. - } - - const builder = new xml2js.Builder({ - headless: true, - }); - return respondWithData(request, response, log, data, - buildHeadXML(builder.buildObject(fileToBuild.value)), modified); - }; - - if (!isSystemXML(request.objectKey)) { - return fetchCapacityMetrics(bucketMd, request, log, 'getVeeamFile', (err, bucketMetrics) => { - if (err) { - return responseXMLBody(errors.InternalError, null, response, log); - } - return finalizeRequest(bucketMetrics); - }); - } - return finalizeRequest(); + return respondWithData(request, response, log, result.bucketData, result.xmlContent, result.modified); }); } diff --git a/lib/routes/veeam/head.js b/lib/routes/veeam/head.js index 7698629857..30b2b51d08 100644 --- a/lib/routes/veeam/head.js +++ b/lib/routes/veeam/head.js @@ -1,7 +1,5 @@ -const xml2js = require('xml2js'); const { errors } = require('arsenal'); -const metadata = require('../../metadata/wrapper'); -const { getResponseHeader, buildHeadXML, getFileToBuild, isSystemXML, fetchCapacityMetrics } = require('./utils'); +const { getResponseHeader, buildVeeamFileData } = require('./utils'); const { responseXMLBody, responseContentHeaders } = require('arsenal/build/lib/s3routes/routesUtils'); /** @@ -18,44 +16,18 @@ function headVeeamFile(request, response, bucketMd, log) { return responseXMLBody(errors.NoSuchBucket, null, response, log); } - return metadata.getBucket(request.bucketName, log, (err, data) => { + return buildVeeamFileData(request, bucketMd, log, 'headVeeamFile', (err, result) => { if (err) { - return responseXMLBody(errors.InternalError, null, response, log); + return responseXMLBody(err, null, response, log); } - const finalizeRequest = bucketMetrics => { - const fileToBuild = getFileToBuild(request, data._capabilities?.VeeamSOSApi); - if (fileToBuild.error) { - return responseXMLBody(fileToBuild.error, null, response, log); - } - - // Extract the last modified date, but do not include it when computing the file's ETag (md5) - const modified = bucketMetrics?.date || new Date(); - delete fileToBuild.value.LastModified; - const builder = new xml2js.Builder({ - headless: true, - }); - const dataBuffer = Buffer.from(buildHeadXML(builder.buildObject(fileToBuild))); - - return responseContentHeaders( - null, - {}, - getResponseHeader(request, data, dataBuffer, modified, log), - response, - log, - ); - }; - - if (!isSystemXML(request.objectKey)) { - return fetchCapacityMetrics(bucketMd, request, log, 'headVeeamFile', (err, bucketMetrics) => { - if (err) { - return responseXMLBody(errors.InternalError, null, response, log); - } - return finalizeRequest(bucketMetrics); - }); - } - - return finalizeRequest(); + return responseContentHeaders( + null, + {}, + getResponseHeader(request, result.bucketData, result.dataBuffer, result.modified, log), + response, + log, + ); }); } diff --git a/lib/routes/veeam/list.js b/lib/routes/veeam/list.js index 138d906314..bf45599e84 100644 --- a/lib/routes/veeam/list.js +++ b/lib/routes/veeam/list.js @@ -87,11 +87,13 @@ function listVeeamFiles(request, response, bucketMd, log) { if (!bucketMd) { return responseXMLBody(errors.NoSuchBucket, null, response, log); } + // Only accept list-type query parameter if (!('list-type' in request.query) && !('versions' in request.query)) { return responseXMLBody(errorInstances.InvalidRequest .customizeDescription('The Veeam folder does not support this action.'), null, response, log); } + return metadata.getBucket(request.bucketName, log, (err, data) => { if (err) { return responseXMLBody(errors.InternalError, null, response, log); @@ -115,7 +117,7 @@ function listVeeamFiles(request, response, bucketMd, log) { fieldsToGenerate.forEach(file => { const isCapacity = file.name.endsWith('capacity.xml'); const lastModified = isCapacity - ? (bucketMetrics?.date || new Date()) + ? bucketMetrics.date : file.LastModified; // eslint-disable-next-line no-param-reassign delete file.LastModified; @@ -129,6 +131,7 @@ function listVeeamFiles(request, response, bucketMd, log) { name: file.name, }); }); + // When `versions` is present, listing should return a versioned list return respondWithData(request, response, log, data, buildXMLResponse(request, filesToBuild, 'versions' in request.query)); @@ -142,7 +145,8 @@ function listVeeamFiles(request, response, bucketMd, log) { return buildAndRespond(bucketMetrics); }); } - return buildAndRespond(); + + return buildAndRespond({ date: new Date() }); }); } diff --git a/lib/routes/veeam/utils.js b/lib/routes/veeam/utils.js index b3814d4a7d..6d72cf3616 100644 --- a/lib/routes/veeam/utils.js +++ b/lib/routes/veeam/utils.js @@ -1,3 +1,4 @@ +const xml2js = require('xml2js'); const { errors, errorInstances, jsutil } = require('arsenal'); const { Readable } = require('stream'); const collectResponseHeaders = require('../../utilities/collectResponseHeaders'); @@ -5,6 +6,7 @@ const collectCorsHeaders = require('../../utilities/collectCorsHeaders'); const crypto = require('crypto'); const { prepareStream } = require('arsenal/build/lib/s3middleware/prepareStream'); const UtilizationService = require('../../utilization/instance'); +const metadata = require('../../metadata/wrapper'); /** * Decodes an URI and return the result. @@ -195,7 +197,6 @@ function getFileToBuild(request, data, inlineLastModified = false) { return { value: { [fieldName]: fileToBuild, - LastModified: modified, }, fieldName, }; @@ -204,14 +205,16 @@ function getFileToBuild(request, data, inlineLastModified = false) { /** * Fetches capacity metrics from UtilizationService for a bucket. - * Handles 404 gracefully (no metrics available yet, e.g. post-install). + * Handles 404 gracefully (no metrics available yet, e.g. post-install), + * returning a default bucketMetrics with the current date so callers always + * receive a usable object. * * @param {object} bucketMd - bucket metadata * @param {object} request - incoming request * @param {object} log - logger object * @param {string} method - calling method name for log context - * @param {function} callback - (err, bucketMetrics) where bucketMetrics - * is undefined when metrics are not available (404) + * @param {function} callback - (err, bucketMetrics) where bucketMetrics always + * has at least a `date` field; on a real 404 the date defaults to new Date() * @returns {undefined} */ function fetchCapacityMetrics(bucketMd, request, log, method, callback) { @@ -225,7 +228,7 @@ function fetchCapacityMetrics(bucketMd, request, log, method, callback) { bucket: request.bucketName, error: err.message || err.code, }); - return callback(null); + return callback(null, { date: new Date() }); } log.error('error fetching capacity metrics from UtilizationService', { method, @@ -239,6 +242,61 @@ function fetchCapacityMetrics(bucketMd, request, log, method, callback) { }); } +/** + * Builds Veeam file data (XML content + response metadata) for a given request. + * + * @param {object} request - incoming request + * @param {object} bucketMd - bucket metadata from the router + * @param {object} log - logger object + * @param {string} name - calling method name (for log context) + * @param {function} callback - (err, result) where result is { xmlContent, dataBuffer, modified, bucketData } + * @returns {undefined} + */ +function buildVeeamFileData(request, bucketMd, log, name, callback) { + return metadata.getBucket(request.bucketName, log, (err, data) => { + if (err) { + return callback(errors.InternalError); + } + + const fileToBuild = getFileToBuild(request, data._capabilities?.VeeamSOSApi); + + if (fileToBuild.error) { + return callback(fileToBuild.error); + } + + const finalize = bucketMetrics => { + const modified = bucketMetrics.date; + if ( + bucketMetrics.bytesTotal !== undefined + && fileToBuild.value.CapacityInfo + && !fileToBuild.value.CapacityInfo.Used + ) { + fileToBuild.value.CapacityInfo.Used = Number(bucketMetrics.bytesTotal); + fileToBuild.value.CapacityInfo.Available = + Number(fileToBuild.value.CapacityInfo.Capacity) - Number(bucketMetrics.bytesTotal); + // TODO CLDSRV-633 when SUR backend supports realtime metrics: it will + // report the real last cseq/date processed by SUR, instead of the current date, + // ensuring no issue in a SOSAPI context. We should use this information. + } + + const builder = new xml2js.Builder({ headless: true }); + const xmlContent = buildHeadXML(builder.buildObject(fileToBuild.value)); + const dataBuffer = Buffer.from(xmlContent); + return callback(null, { xmlContent, dataBuffer, modified, bucketData: data }); + }; + + if (!isSystemXML(request.objectKey)) { + return fetchCapacityMetrics(bucketMd, request, log, name, (fetchErr, bucketMetrics) => { + if (fetchErr) { + return callback(errors.InternalError); + } + return finalize(bucketMetrics); + }); + } + return finalize({ date: new Date() }); + }); +} + module.exports = { _decodeURI, receiveData, @@ -249,4 +307,5 @@ module.exports = { isSystemXML, getFileToBuild, fetchCapacityMetrics, + buildVeeamFileData, }; diff --git a/tests/unit/routes/veeam-utils.js b/tests/unit/routes/veeam-utils.js index bdb5173524..6466343e73 100644 --- a/tests/unit/routes/veeam-utils.js +++ b/tests/unit/routes/veeam-utils.js @@ -1,7 +1,8 @@ const assert = require('assert'); const sinon = require('sinon'); const UtilizationService = require('../../../lib/utilization/instance'); -const { fetchCapacityMetrics } = require('../../../lib/routes/veeam/utils'); +const metadata = require('../../../lib/metadata/wrapper'); +const { fetchCapacityMetrics, buildVeeamFileData } = require('../../../lib/routes/veeam/utils'); const { DummyRequestLogger } = require('../helpers'); describe('fetchCapacityMetrics', () => { @@ -57,14 +58,14 @@ describe('fetchCapacityMetrics', () => { }); }); - it('should call back with no error and no metrics on 404', done => { + it('should call back with no error and a default date on 404', done => { const error404 = new Error('Not Found'); error404.response = { status: 404 }; utilizationStub.callsArgWith(4, error404); fetchCapacityMetrics(bucketMd, request, log, 'testMethod', (err, metrics) => { assert.ifError(err); - assert.strictEqual(metrics, undefined); + assert(metrics && metrics.date instanceof Date, 'metrics should have a Date for date'); assert(logWarnSpy.calledOnce); assert(logWarnSpy.getCall(0).args[0].includes('404')); assert.strictEqual(logWarnSpy.getCall(0).args[1].method, 'testMethod'); @@ -81,7 +82,7 @@ describe('fetchCapacityMetrics', () => { fetchCapacityMetrics(bucketMd, request, log, 'testMethod', (err, metrics) => { assert.ifError(err); - assert.strictEqual(metrics, undefined); + assert(metrics && metrics.date instanceof Date, 'metrics should have a Date for date'); assert(logWarnSpy.calledOnce); done(); }); @@ -118,3 +119,168 @@ describe('fetchCapacityMetrics', () => { }); }); }); + +describe('buildVeeamFileData', () => { + let utilizationStub; + let metadataStub; + let log; + + const validPath = '.system-d26a9498-cb7c-4a87-a44a-8ae204f5ba6c/'; + + const capacityObjectKey = `${validPath}capacity.xml`; + const systemObjectKey = `${validPath}system.xml`; + + const bucketMd = { + _name: 'test-bucket', + _creationDate: '2024-01-01T00:00:00.000Z', + _capabilities: { + VeeamSOSApi: { + CapacityInfo: { + Capacity: 1099511627776, + Available: 549755813888, + Used: 0, + LastModified: '2024-01-01T00:00:00.000Z', + }, + }, + }, + }; + + const bucketMdWithSystem = { + ...bucketMd, + _capabilities: { + VeeamSOSApi: { + SystemInfo: { + ProtocolVersion: '1.0', + ModelName: 'ARTESCA', + LastModified: '2024-01-01T00:00:00.000Z', + }, + }, + }, + }; + + const createRequest = objectKey => ({ + bucketName: 'test-bucket', + objectKey, + headers: { host: 'test-bucket.s3.amazonaws.com' }, + }); + + beforeEach(() => { + log = new DummyRequestLogger(); + log.warn = sinon.stub(); + log.error = sinon.stub(); + + metadataStub = sinon.stub(metadata, 'getBucket'); + utilizationStub = sinon.stub(UtilizationService, 'getUtilizationMetrics'); + }); + + afterEach(() => { + sinon.restore(); + }); + + it('should return InternalError when metadata.getBucket fails', done => { + metadataStub.callsArgWith(2, new Error('DB error')); + + buildVeeamFileData(createRequest(capacityObjectKey), bucketMd, log, 'test', (err) => { + assert(err); + assert.strictEqual(err.code, 500); + done(); + }); + }); + + it('should return NoSuchKey when capabilities do not include the requested file', done => { + metadataStub.callsArgWith(2, null, { _capabilities: {} }); + + buildVeeamFileData(createRequest(capacityObjectKey), bucketMd, log, 'test', (err) => { + assert(err); + assert.strictEqual(err.code, 404); + done(); + }); + }); + + it('should return InternalError when fetchCapacityMetrics fails', done => { + metadataStub.callsArgWith(2, null, bucketMd); + const error500 = new Error('Internal Server Error'); + error500.response = { status: 500 }; + utilizationStub.callsArgWith(4, error500); + + buildVeeamFileData(createRequest(capacityObjectKey), bucketMd, log, 'test', (err) => { + assert(err); + assert.strictEqual(err.code, 500); + done(); + }); + }); + + it('should build capacity.xml with SUR metrics date and applied Used/Available', done => { + const metricsDate = new Date('2026-03-26T19:00:08.996Z'); + metadataStub.callsArgWith(2, null, bucketMd); + utilizationStub.callsArgWith(4, null, { date: metricsDate, bytesTotal: 100 }); + + buildVeeamFileData(createRequest(capacityObjectKey), bucketMd, log, 'test', (err, result) => { + assert.ifError(err); + assert.strictEqual(result.modified, metricsDate); + assert(result.xmlContent.includes('CapacityInfo')); + assert(result.xmlContent.includes('100')); + assert(result.xmlContent.includes('')); + assert(Buffer.isBuffer(result.dataBuffer)); + assert.deepStrictEqual(result.dataBuffer, Buffer.from(result.xmlContent)); + assert.strictEqual(result.bucketData, bucketMd); + done(); + }); + }); + + it('should use current date for capacity.xml when UtilizationService returns 404', done => { + const before = Date.now(); + metadataStub.callsArgWith(2, null, bucketMd); + const error404 = new Error('Not Found'); + error404.response = { status: 404 }; + utilizationStub.callsArgWith(4, error404); + + buildVeeamFileData(createRequest(capacityObjectKey), bucketMd, log, 'test', (err, result) => { + assert.ifError(err); + assert(result.modified instanceof Date); + assert(result.modified.getTime() >= before); + assert(result.xmlContent.includes('CapacityInfo')); + done(); + }); + }); + + it('should build system.xml without calling UtilizationService', done => { + metadataStub.callsArgWith(2, null, bucketMdWithSystem); + const before = Date.now(); + + buildVeeamFileData(createRequest(systemObjectKey), bucketMdWithSystem, log, 'test', (err, result) => { + assert.ifError(err); + assert(!utilizationStub.called, 'should not call UtilizationService for system.xml'); + assert(result.modified instanceof Date); + assert(result.modified.getTime() >= before); + assert(result.xmlContent.includes('SystemInfo')); + assert(result.xmlContent.includes('ARTESCA')); + assert(Buffer.isBuffer(result.dataBuffer)); + done(); + }); + }); + + it('should not overwrite Used when it is already set', done => { + const bucketMdWithUsed = { + ...bucketMd, + _capabilities: { + VeeamSOSApi: { + CapacityInfo: { + Capacity: 1000, + Available: 600, + Used: 400, + LastModified: '2024-01-01T00:00:00.000Z', + }, + }, + }, + }; + metadataStub.callsArgWith(2, null, bucketMdWithUsed); + utilizationStub.callsArgWith(4, null, { date: new Date(), bytesTotal: 999 }); + + buildVeeamFileData(createRequest(capacityObjectKey), bucketMdWithUsed, log, 'test', (err, result) => { + assert.ifError(err); + assert(result.xmlContent.includes('400'), 'should keep existing Used value'); + done(); + }); + }); +}); From 19a1ee8430cb16e40521a933b6fcbf233e630d35 Mon Sep 17 00:00:00 2001 From: DarkIsDude Date: Wed, 1 Apr 2026 12:17:47 +0200 Subject: [PATCH 06/12] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20migrate=20to=20async?= =?UTF-8?q?/await=20style?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue: CLDSRV-878 --- lib/routes/veeam/get.js | 13 ++- lib/routes/veeam/head.js | 13 ++- lib/routes/veeam/list.js | 106 +++++++++++----------- lib/routes/veeam/put.js | 3 +- lib/routes/veeam/utils.js | 186 ++++++++++++++++++++------------------ 5 files changed, 167 insertions(+), 154 deletions(-) diff --git a/lib/routes/veeam/get.js b/lib/routes/veeam/get.js index a85af07961..f8b94f5a91 100644 --- a/lib/routes/veeam/get.js +++ b/lib/routes/veeam/get.js @@ -11,7 +11,7 @@ const { responseXMLBody } = require('arsenal/build/lib/s3routes/routesUtils'); * @param {object} log - logger object * @returns {undefined} - */ -function getVeeamFile(request, response, bucketMd, log) { +async function getVeeamFile(request, response, bucketMd, log) { if (!bucketMd) { return responseXMLBody(errors.NoSuchBucket, null, response, log); } @@ -21,13 +21,12 @@ function getVeeamFile(request, response, bucketMd, log) { buildHeadXML('')); } - return buildVeeamFileData(request, bucketMd, log, 'getVeeamFile', (err, result) => { - if (err) { - return responseXMLBody(err, null, response, log); - } - + try { + const result = await buildVeeamFileData(request, bucketMd, log, 'getVeeamFile'); return respondWithData(request, response, log, result.bucketData, result.xmlContent, result.modified); - }); + } catch (err) { + return responseXMLBody(err, null, response, log); + } } module.exports = getVeeamFile; diff --git a/lib/routes/veeam/head.js b/lib/routes/veeam/head.js index 30b2b51d08..5c888f1030 100644 --- a/lib/routes/veeam/head.js +++ b/lib/routes/veeam/head.js @@ -11,16 +11,13 @@ const { responseXMLBody, responseContentHeaders } = require('arsenal/build/lib/s * @param {object} log - logger object * @returns {undefined} - */ -function headVeeamFile(request, response, bucketMd, log) { +async function headVeeamFile(request, response, bucketMd, log) { if (!bucketMd) { return responseXMLBody(errors.NoSuchBucket, null, response, log); } - return buildVeeamFileData(request, bucketMd, log, 'headVeeamFile', (err, result) => { - if (err) { - return responseXMLBody(err, null, response, log); - } - + try { + const result = await buildVeeamFileData(request, bucketMd, log, 'headVeeamFile'); return responseContentHeaders( null, {}, @@ -28,7 +25,9 @@ function headVeeamFile(request, response, bucketMd, log) { response, log, ); - }); + } catch (err) { + return responseXMLBody(err, null, response, log); + } } module.exports = headVeeamFile; diff --git a/lib/routes/veeam/list.js b/lib/routes/veeam/list.js index bf45599e84..ff3ee2fc31 100644 --- a/lib/routes/veeam/list.js +++ b/lib/routes/veeam/list.js @@ -83,7 +83,7 @@ function buildXMLResponse(request, arrayOfFiles, versioned = false) { * @param {object} log - logger object * @returns {undefined} - */ -function listVeeamFiles(request, response, bucketMd, log) { +async function listVeeamFiles(request, response, bucketMd, log) { if (!bucketMd) { return responseXMLBody(errors.NoSuchBucket, null, response, log); } @@ -94,60 +94,64 @@ function listVeeamFiles(request, response, bucketMd, log) { .customizeDescription('The Veeam folder does not support this action.'), null, response, log); } - return metadata.getBucket(request.bucketName, log, (err, data) => { - if (err) { - return responseXMLBody(errors.InternalError, null, response, log); - } - - const buildAndRespond = bucketMetrics => { - const filesToBuild = []; - const fieldsToGenerate = []; - if (data._capabilities?.VeeamSOSApi?.SystemInfo) { - fieldsToGenerate.push({ - ...data._capabilities?.VeeamSOSApi?.SystemInfo, - name: `${validPath}system.xml`, - }); - } - if (data._capabilities?.VeeamSOSApi?.CapacityInfo) { - fieldsToGenerate.push({ - ...data._capabilities?.VeeamSOSApi?.CapacityInfo, - name: `${validPath}capacity.xml`, - }); - } - fieldsToGenerate.forEach(file => { - const isCapacity = file.name.endsWith('capacity.xml'); - const lastModified = isCapacity - ? bucketMetrics.date - : file.LastModified; - // eslint-disable-next-line no-param-reassign - delete file.LastModified; - const builder = new xml2js.Builder({ - headless: true, - }); - const dataBuffer = Buffer.from(buildHeadXML(builder.buildObject(file))); - filesToBuild.push({ - ...getResponseHeader(request, data, - dataBuffer, lastModified, log), - name: file.name, - }); - }); - - // When `versions` is present, listing should return a versioned list - return respondWithData(request, response, log, data, - buildXMLResponse(request, filesToBuild, 'versions' in request.query)); - }; + let data; + try { + data = await new Promise((resolve, reject) => + metadata.getBucket(request.bucketName, log, (err, result) => { + if (err) { return reject(err); } + return resolve(result); + }) + ); + } catch { + return responseXMLBody(errors.InternalError, null, response, log); + } - if (data._capabilities?.VeeamSOSApi?.CapacityInfo) { - return fetchCapacityMetrics(bucketMd, request, log, 'listVeeamFiles', (err, bucketMetrics) => { - if (err) { - return responseXMLBody(errors.InternalError, null, response, log); - } - return buildAndRespond(bucketMetrics); - }); + let bucketMetrics; + if (data._capabilities?.VeeamSOSApi?.CapacityInfo) { + try { + bucketMetrics = await fetchCapacityMetrics(bucketMd, request, log, 'listVeeamFiles'); + } catch { + return responseXMLBody(errors.InternalError, null, response, log); } + } else { + bucketMetrics = { date: new Date() }; + } - return buildAndRespond({ date: new Date() }); + const filesToBuild = []; + const fieldsToGenerate = []; + if (data._capabilities?.VeeamSOSApi?.SystemInfo) { + fieldsToGenerate.push({ + ...data._capabilities?.VeeamSOSApi?.SystemInfo, + name: `${validPath}system.xml`, + }); + } + if (data._capabilities?.VeeamSOSApi?.CapacityInfo) { + fieldsToGenerate.push({ + ...data._capabilities?.VeeamSOSApi?.CapacityInfo, + name: `${validPath}capacity.xml`, + }); + } + fieldsToGenerate.forEach(file => { + const isCapacity = file.name.endsWith('capacity.xml'); + const lastModified = isCapacity + ? bucketMetrics.date + : file.LastModified; + // eslint-disable-next-line no-param-reassign + delete file.LastModified; + const builder = new xml2js.Builder({ + headless: true, + }); + const dataBuffer = Buffer.from(buildHeadXML(builder.buildObject(file))); + filesToBuild.push({ + ...getResponseHeader(request, data, + dataBuffer, lastModified, log), + name: file.name, + }); }); + + // When `versions` is present, listing should return a versioned list + return respondWithData(request, response, log, data, + buildXMLResponse(request, filesToBuild, 'versions' in request.query)); } module.exports = listVeeamFiles; diff --git a/lib/routes/veeam/put.js b/lib/routes/veeam/put.js index 521d0d7d59..0e15d86051 100644 --- a/lib/routes/veeam/put.js +++ b/lib/routes/veeam/put.js @@ -1,4 +1,5 @@ const async = require('async'); +const { callbackify } = require('util'); const { parseString } = require('xml2js'); const { receiveData, isSystemXML, getFileToBuild } = require('./utils'); const { s3routes, errors } = require('arsenal'); @@ -28,7 +29,7 @@ function putVeeamFile(request, response, bucketMd, log) { next => { // Extract the data from the request, keep it in memory writeContinue(request, response); - return receiveData(request, log, next); + return callbackify(receiveData)(request, log, next); }, (value, next) => parseString(value, { explicitArray: false }, (err, parsed) => { // Convert the received XML to a JS object diff --git a/lib/routes/veeam/utils.js b/lib/routes/veeam/utils.js index 6d72cf3616..40fad06856 100644 --- a/lib/routes/veeam/utils.js +++ b/lib/routes/veeam/utils.js @@ -23,43 +23,45 @@ function _decodeURI(uri) { * * @param {object} request - incoming request * @param {object} log - logger object - * @param {function} callback - - * @returns {undefined} + * @returns {Promise} */ -function receiveData(request, log, callback) { +async function receiveData(request, log) { // Get keycontent const { parsedContentLength } = request; const ContentLengthThreshold = 1024 * 1024; // 1MB // Prevent memory overloads by limiting the size of the // received data. if (parsedContentLength > ContentLengthThreshold) { - return callback(errorInstances.InvalidInput - .customizeDescription(`maximum allowed content-length is ${ContentLengthThreshold} bytes`)); + throw errorInstances.InvalidInput + .customizeDescription(`maximum allowed content-length is ${ContentLengthThreshold} bytes`); } - const value = Buffer.alloc(parsedContentLength); - const cbOnce = jsutil.once(callback); - const dataStream = prepareStream(request, request.streamingV4Params, log, cbOnce); - let cursor = 0; - let exceeded = false; - dataStream.on('data', data => { - if (cursor + data.length > parsedContentLength) { - exceeded = true; - } - if (!exceeded) { - data.copy(value, cursor); - } - cursor += data.length; - }); - dataStream.on('end', () => { - if (exceeded) { - log.error('data stream exceed announced size', - { parsedContentLength, overflow: cursor }); - return callback(errors.InternalError); - } else { - return callback(null, value.toString()); - } + return new Promise((resolve, reject) => { + const value = Buffer.alloc(parsedContentLength); + const settle = jsutil.once((err, result) => { + if (err) { return reject(err); } + return resolve(result); + }); + const dataStream = prepareStream(request, request.streamingV4Params, log, settle); + let cursor = 0; + let exceeded = false; + dataStream.on('data', data => { + if (cursor + data.length > parsedContentLength) { + exceeded = true; + } + if (!exceeded) { + data.copy(value, cursor); + } + cursor += data.length; + }); + dataStream.on('end', () => { + if (exceeded) { + log.error('data stream exceed announced size', + { parsedContentLength, overflow: cursor }); + return settle(errors.InternalError); + } + return settle(null, value.toString()); + }); }); - return undefined; } /** @@ -213,33 +215,36 @@ function getFileToBuild(request, data, inlineLastModified = false) { * @param {object} request - incoming request * @param {object} log - logger object * @param {string} method - calling method name for log context - * @param {function} callback - (err, bucketMetrics) where bucketMetrics always - * has at least a `date` field; on a real 404 the date defaults to new Date() - * @returns {undefined} + * @returns {Promise} bucketMetrics always has at least a `date` field; + * on a real 404 the date defaults to new Date() */ -function fetchCapacityMetrics(bucketMd, request, log, method, callback) { +async function fetchCapacityMetrics(bucketMd, request, log, method) { const bucketKey = `${bucketMd._name}_${new Date(bucketMd._creationDate).getTime()}`; - return UtilizationService.getUtilizationMetrics('bucket', bucketKey, null, {}, (err, bucketMetrics) => { - if (err) { - const statusCode = err.response?.status || err.statusCode || err.code; - if (statusCode === 404) { - log.warn('UtilizationService returned 404 when fetching capacity metrics', { - method, - bucket: request.bucketName, - error: err.message || err.code, - }); - return callback(null, { date: new Date() }); - } - log.error('error fetching capacity metrics from UtilizationService', { + try { + return await new Promise((resolve, reject) => + UtilizationService.getUtilizationMetrics('bucket', bucketKey, null, {}, (err, bucketMetrics) => { + if (err) { return reject(err); } + return resolve(bucketMetrics); + }) + ); + } catch (err) { + const statusCode = err.response?.status || err.statusCode || err.code; + if (statusCode === 404) { + log.warn('UtilizationService returned 404 when fetching capacity metrics', { method, bucket: request.bucketName, error: err.message || err.code, - statusCode, }); - return callback(err); + return { date: new Date() }; } - return callback(null, bucketMetrics); - }); + log.error('error fetching capacity metrics from UtilizationService', { + method, + bucket: request.bucketName, + error: err.message || err.code, + statusCode, + }); + throw err; + } } /** @@ -249,56 +254,61 @@ function fetchCapacityMetrics(bucketMd, request, log, method, callback) { * @param {object} bucketMd - bucket metadata from the router * @param {object} log - logger object * @param {string} name - calling method name (for log context) - * @param {function} callback - (err, result) where result is { xmlContent, dataBuffer, modified, bucketData } - * @returns {undefined} + * @returns {Promise} result with { xmlContent, dataBuffer, modified, bucketData } */ -function buildVeeamFileData(request, bucketMd, log, name, callback) { - return metadata.getBucket(request.bucketName, log, (err, data) => { - if (err) { - return callback(errors.InternalError); - } +async function buildVeeamFileData(request, bucketMd, log, name) { + let data; + try { + data = await new Promise((resolve, reject) => + metadata.getBucket(request.bucketName, log, (err, result) => { + if (err) { return reject(err); } + return resolve(result); + }) + ); + } catch { + throw errors.InternalError; + } - const fileToBuild = getFileToBuild(request, data._capabilities?.VeeamSOSApi); + const fileToBuild = getFileToBuild(request, data._capabilities?.VeeamSOSApi); - if (fileToBuild.error) { - return callback(fileToBuild.error); - } + if (fileToBuild.error) { + throw fileToBuild.error; + } - const finalize = bucketMetrics => { - const modified = bucketMetrics.date; - if ( - bucketMetrics.bytesTotal !== undefined - && fileToBuild.value.CapacityInfo - && !fileToBuild.value.CapacityInfo.Used - ) { - fileToBuild.value.CapacityInfo.Used = Number(bucketMetrics.bytesTotal); - fileToBuild.value.CapacityInfo.Available = - Number(fileToBuild.value.CapacityInfo.Capacity) - Number(bucketMetrics.bytesTotal); - // TODO CLDSRV-633 when SUR backend supports realtime metrics: it will - // report the real last cseq/date processed by SUR, instead of the current date, - // ensuring no issue in a SOSAPI context. We should use this information. - } + let bucketMetrics; + if (!isSystemXML(request.objectKey)) { + try { + bucketMetrics = await fetchCapacityMetrics(bucketMd, request, log, name); + } catch { + throw errors.InternalError; + } + } else { + bucketMetrics = { date: new Date() }; + } - const builder = new xml2js.Builder({ headless: true }); - const xmlContent = buildHeadXML(builder.buildObject(fileToBuild.value)); - const dataBuffer = Buffer.from(xmlContent); - return callback(null, { xmlContent, dataBuffer, modified, bucketData: data }); - }; + const modified = bucketMetrics.date; + if ( + bucketMetrics.bytesTotal !== undefined + && fileToBuild.value.CapacityInfo + && !fileToBuild.value.CapacityInfo.Used + ) { + fileToBuild.value.CapacityInfo.Used = Number(bucketMetrics.bytesTotal); + fileToBuild.value.CapacityInfo.Available = + Number(fileToBuild.value.CapacityInfo.Capacity) - Number(bucketMetrics.bytesTotal); + // TODO CLDSRV-633 when SUR backend supports realtime metrics: it will + // report the real last cseq/date processed by SUR, instead of the current date, + // ensuring no issue in a SOSAPI context. We should use this information. + } - if (!isSystemXML(request.objectKey)) { - return fetchCapacityMetrics(bucketMd, request, log, name, (fetchErr, bucketMetrics) => { - if (fetchErr) { - return callback(errors.InternalError); - } - return finalize(bucketMetrics); - }); - } - return finalize({ date: new Date() }); - }); + const builder = new xml2js.Builder({ headless: true }); + const xmlContent = buildHeadXML(builder.buildObject(fileToBuild.value)); + const dataBuffer = Buffer.from(xmlContent); + return { xmlContent, dataBuffer, modified, bucketData: data }; } module.exports = { _decodeURI, + // callbackify keeps backward compatibility with put.js which still uses the callback form receiveData, respondWithData, getResponseHeader, From 9524559e0daa4c107ea6f96747713753ecbeecb8 Mon Sep 17 00:00:00 2001 From: DarkIsDude Date: Wed, 1 Apr 2026 12:49:36 +0200 Subject: [PATCH 07/12] =?UTF-8?q?=E2=9C=85=20migrate=20tests=20to=20async/?= =?UTF-8?q?await?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue: CLDSRV-878 --- lib/routes/veeam/get.js | 4 +- lib/routes/veeam/list.js | 2 +- lib/routes/veeam/utils.js | 39 ++-- tests/unit/routes/veeam-routes.js | 335 ++++++++++++------------------ tests/unit/routes/veeam-utils.js | 196 ++++++++--------- 5 files changed, 246 insertions(+), 330 deletions(-) diff --git a/lib/routes/veeam/get.js b/lib/routes/veeam/get.js index f8b94f5a91..09c394b465 100644 --- a/lib/routes/veeam/get.js +++ b/lib/routes/veeam/get.js @@ -17,13 +17,13 @@ async function getVeeamFile(request, response, bucketMd, log) { } if ('tagging' in request.query) { - return respondWithData(request, response, log, bucketMd, + return await respondWithData(request, response, log, bucketMd, buildHeadXML('')); } try { const result = await buildVeeamFileData(request, bucketMd, log, 'getVeeamFile'); - return respondWithData(request, response, log, result.bucketData, result.xmlContent, result.modified); + return await respondWithData(request, response, log, result.bucketData, result.xmlContent, result.modified); } catch (err) { return responseXMLBody(err, null, response, log); } diff --git a/lib/routes/veeam/list.js b/lib/routes/veeam/list.js index ff3ee2fc31..2ff7e62b9d 100644 --- a/lib/routes/veeam/list.js +++ b/lib/routes/veeam/list.js @@ -150,7 +150,7 @@ async function listVeeamFiles(request, response, bucketMd, log) { }); // When `versions` is present, listing should return a versioned list - return respondWithData(request, response, log, data, + return await respondWithData(request, response, log, data, buildXMLResponse(request, filesToBuild, 'versions' in request.query)); } diff --git a/lib/routes/veeam/utils.js b/lib/routes/veeam/utils.js index 40fad06856..008417c9b2 100644 --- a/lib/routes/veeam/utils.js +++ b/lib/routes/veeam/utils.js @@ -1,6 +1,7 @@ const xml2js = require('xml2js'); const { errors, errorInstances, jsutil } = require('arsenal'); -const { Readable } = require('stream'); +const { Readable, pipeline: streamPipeline } = require('stream'); +const { promisify } = require('util'); const collectResponseHeaders = require('../../utilities/collectResponseHeaders'); const collectCorsHeaders = require('../../utilities/collectCorsHeaders'); const crypto = require('crypto'); @@ -8,6 +9,8 @@ const { prepareStream } = require('arsenal/build/lib/s3middleware/prepareStream' const UtilizationService = require('../../utilization/instance'); const metadata = require('../../metadata/wrapper'); +const pipeline = promisify(streamPipeline); + /** * Decodes an URI and return the result. * Do the same decoding than in S3 server @@ -109,23 +112,12 @@ function getResponseHeader(request, bucket, dataBuffer, lastModified, log) { * @param {BucketInfo} bucket - bucket info * @param {string} data - data to send * @param {date} [lastModified] - last modified date of the value - * @returns {undefined} - + * @returns {Promise} - */ -function respondWithData(request, response, log, bucket, data, lastModified) { +async function respondWithData(request, response, log, bucket, data, lastModified) { const dataBuffer = Buffer.from(data); const responseMetaHeaders = getResponseHeader(request, bucket, dataBuffer, lastModified, log); - response.on('finish', () => { - let contentLength = 0; - if (responseMetaHeaders && responseMetaHeaders['Content-Length']) { - contentLength = responseMetaHeaders['Content-Length']; - } - log.end().addDefaultFields({ contentLength }); - log.end().info('responded with streamed content', { - httpCode: response.statusCode, - }); - }); - if (responseMetaHeaders && typeof responseMetaHeaders === 'object') { Object.keys(responseMetaHeaders).forEach(key => { if (responseMetaHeaders[key] !== undefined) { @@ -143,13 +135,17 @@ function respondWithData(request, response, log, bucket, data, lastModified) { } response.writeHead(200); - const stream = Readable.from(dataBuffer); - stream.pipe(response); - stream.on('unpipe', () => { - response.end(); - }); - stream.on('error', () => { - response.end(); + // Use a single-element array so the Buffer is sent as one chunk rather + // than being iterated byte-by-byte by Readable.from. + await pipeline(Readable.from([dataBuffer]), response); + + let contentLength = 0; + if (responseMetaHeaders && responseMetaHeaders['Content-Length']) { + contentLength = responseMetaHeaders['Content-Length']; + } + log.end().addDefaultFields({ contentLength }); + log.end().info('responded with streamed content', { + httpCode: response.statusCode, }); } @@ -308,7 +304,6 @@ async function buildVeeamFileData(request, bucketMd, log, name) { module.exports = { _decodeURI, - // callbackify keeps backward compatibility with put.js which still uses the callback form receiveData, respondWithData, getResponseHeader, diff --git a/tests/unit/routes/veeam-routes.js b/tests/unit/routes/veeam-routes.js index 93517f130c..4d35ad8829 100644 --- a/tests/unit/routes/veeam-routes.js +++ b/tests/unit/routes/veeam-routes.js @@ -7,9 +7,6 @@ const UtilizationService = require('../../../lib/utilization/instance'); const metadata = require('../../../lib/metadata/wrapper'); const { DummyRequestLogger } = require('../helpers'); -// Helper function to give async callbacks time to execute -const giveAsyncCallbackTimeToExecute = setImmediate; - describe('Veeam routes - comprehensive unit tests', () => { let utilizationStub; let metadataStub; @@ -104,7 +101,7 @@ describe('Veeam routes - comprehensive unit tests', () => { return response; }; - it('should handle 404 error from UtilizationService and return 200', done => { + it('should handle 404 error from UtilizationService and return 200', async () => { const error404 = new Error('Not Found'); error404.response = { status: 404 }; utilizationStub.callsArgWith(4, error404); @@ -112,24 +109,21 @@ describe('Veeam routes - comprehensive unit tests', () => { const request = createRequest(); const response = createResponse(); - getVeeamFile(request, response, bucketMd, log); + await getVeeamFile(request, response, bucketMd, log); - giveAsyncCallbackTimeToExecute(() => { - assert(logWarnSpy.calledOnce, 'log.warn should have been called once'); - const warnCall = logWarnSpy.getCall(0); - assert(warnCall.args[0].includes('UtilizationService returned 404'), - 'warning message should mention 404'); - assert.strictEqual(warnCall.args[1].method, 'getVeeamFile'); - assert.strictEqual(warnCall.args[1].bucket, 'test-bucket'); + assert(logWarnSpy.calledOnce, 'log.warn should have been called once'); + const warnCall = logWarnSpy.getCall(0); + assert(warnCall.args[0].includes('UtilizationService returned 404'), + 'warning message should mention 404'); + assert.strictEqual(warnCall.args[1].method, 'getVeeamFile'); + assert.strictEqual(warnCall.args[1].bucket, 'test-bucket'); - assert(response.writeHead.calledWith(200), - 'should return 200 despite 404 from UtilizationService'); - assert(response.end.called, 'response should be ended'); - done(); - }); + assert(response.writeHead.calledWith(200), + 'should return 200 despite 404 from UtilizationService'); + assert(response.end.called, 'response should be ended'); }); - it('should handle 500 error from UtilizationService and return 500', done => { + it('should handle 500 error from UtilizationService and return 500', async () => { const error500 = new Error('Internal Server Error'); error500.response = { status: 500 }; utilizationStub.callsArgWith(4, error500); @@ -137,16 +131,13 @@ describe('Veeam routes - comprehensive unit tests', () => { const request = createRequest(); const response = createResponse(); - getVeeamFile(request, response, bucketMd, log); + await getVeeamFile(request, response, bucketMd, log); - giveAsyncCallbackTimeToExecute(() => { - assert(response.headersSent || response.write.called || response.writeHead.called, - 'should send error response for 500 errors'); - done(); - }); + assert(response.headersSent || response.write.called || response.writeHead.called, + 'should send error response for 500 errors'); }); - it('should handle connection error from UtilizationService and return 500', done => { + it('should handle connection error from UtilizationService and return 500', async () => { const errorConn = new Error('Connection refused'); errorConn.code = 'ECONNREFUSED'; utilizationStub.callsArgWith(4, errorConn); @@ -154,16 +145,13 @@ describe('Veeam routes - comprehensive unit tests', () => { const request = createRequest(); const response = createResponse(); - getVeeamFile(request, response, bucketMd, log); + await getVeeamFile(request, response, bucketMd, log); - giveAsyncCallbackTimeToExecute(() => { - assert(response.headersSent || response.write.called || response.writeHead.called, - 'should send error response for connection errors'); - done(); - }); + assert(response.headersSent || response.write.called || response.writeHead.called, + 'should send error response for connection errors'); }); - it('should successfully use metrics when UtilizationService returns data', done => { + it('should successfully use metrics when UtilizationService returns data', async () => { const metricsDate = '2026-03-26T19:00:08.996Z'; const bucketMetrics = { bytesTotal: 123456789, @@ -174,28 +162,25 @@ describe('Veeam routes - comprehensive unit tests', () => { const request = createRequest(); const response = createResponse(); - getVeeamFile(request, response, bucketMd, log); + await getVeeamFile(request, response, bucketMd, log); - giveAsyncCallbackTimeToExecute(() => { - assert(!logWarnSpy.called, 'log.warn should not have been called'); - assert(response.writeHead.calledWith(200), 'should return 200 with metrics'); - assert(utilizationStub.calledOnce, 'should call UtilizationService once'); - assert(response.end.called, 'response should be ended'); + assert(!logWarnSpy.called, 'log.warn should not have been called'); + assert(response.writeHead.calledWith(200), 'should return 200 with metrics'); + assert(utilizationStub.calledOnce, 'should call UtilizationService once'); + assert(response.end.called, 'response should be ended'); - const lastModifiedCall = response.setHeader.getCalls() - .find(call => call.args[0] === 'Last-Modified'); + const lastModifiedCall = response.setHeader.getCalls() + .find(call => call.args[0] === 'Last-Modified'); - assert(lastModifiedCall, 'Last-Modified header should be set'); - assert.strictEqual( - lastModifiedCall.args[1], - new Date(metricsDate).toUTCString(), - 'Last-Modified should use the date from bucketMetrics', - ); - done(); - }); + assert(lastModifiedCall, 'Last-Modified header should be set'); + assert.strictEqual( + lastModifiedCall.args[1], + new Date(metricsDate).toUTCString(), + 'Last-Modified should use the date from bucketMetrics', + ); }); - it('should not call UtilizationService for system.xml requests', done => { + it('should not call UtilizationService for system.xml requests', async () => { const bucketMdWithSystem = { ...bucketMd, _capabilities: { @@ -213,17 +198,14 @@ describe('Veeam routes - comprehensive unit tests', () => { const request = createRequest('.system-d26a9498-cb7c-4a87-a44a-8ae204f5ba6c/system.xml'); const response = createResponse(); - getVeeamFile(request, response, bucketMdWithSystem, log); + await getVeeamFile(request, response, bucketMdWithSystem, log); - setImmediate(() => { - assert(!utilizationStub.called, 'should not call UtilizationService for system.xml'); - assert(response.writeHead.calledWith(200), 'should return 200 for system.xml'); - assert(response.end.called, 'response should be ended'); - done(); - }); + assert(!utilizationStub.called, 'should not call UtilizationService for system.xml'); + assert(response.writeHead.calledWith(200), 'should return 200 for system.xml'); + assert(response.end.called, 'response should be ended'); }); - it('should verify the post-install scenario: 404 returns 200 with Used=0', done => { + it('should verify the post-install scenario: 404 returns 200 with Used=0', async () => { // This test reproduces the post-install scenario where scubaclient returns 404 // because no metrics are available yet const error404 = new Error('Not Found'); @@ -233,49 +215,39 @@ describe('Veeam routes - comprehensive unit tests', () => { const request = createRequest(); const response = createResponse(); - getVeeamFile(request, response, bucketMd, log); - - giveAsyncCallbackTimeToExecute(() => { - assert(logWarnSpy.calledOnce, 'should log warning for 404'); - assert(response.writeHead.calledWith(200), - 'should return 200 with static capacity data for 404'); - assert(response.end.called, 'response should be ended'); - const warnCall = logWarnSpy.getCall(0); - assert(warnCall.args[0].includes('404'), 'warning should mention 404'); + await getVeeamFile(request, response, bucketMd, log); - done(); - }); + assert(logWarnSpy.calledOnce, 'should log warning for 404'); + assert(response.writeHead.calledWith(200), + 'should return 200 with static capacity data for 404'); + assert(response.end.called, 'response should be ended'); + const warnCall = logWarnSpy.getCall(0); + assert(warnCall.args[0].includes('404'), 'warning should mention 404'); }); - it('should handle metadata.getBucket errors gracefully', done => { + it('should handle metadata.getBucket errors gracefully', async () => { const metadataError = new Error('Metadata service error'); metadataStub.callsArgWith(2, metadataError); const request = createRequest(); const response = createResponse(); - getVeeamFile(request, response, bucketMd, log); + await getVeeamFile(request, response, bucketMd, log); - giveAsyncCallbackTimeToExecute(() => { - assert(response.headersSent || response.write.called || response.writeHead.called, - 'should send response for metadata errors'); - done(); - }); + assert(response.headersSent || response.write.called || response.writeHead.called, + 'should send response for metadata errors'); }); - it('should handle tagging query parameter', done => { + it('should handle tagging query parameter', async () => { const request = createRequest(); request.query = { tagging: '' }; const response = createResponse(); - getVeeamFile(request, response, bucketMd, log); + await getVeeamFile(request, response, bucketMd, log); - giveAsyncCallbackTimeToExecute(() => { - assert(response.writeHead.calledWith(200), - 'should return 200 for tagging query'); - assert(response.end.called, 'response should be ended'); - done(); - }); + assert(response.writeHead.calledWith(200), + 'should return 200 for tagging query'); + assert(response.end.called, 'response should be ended'); }); }); @@ -352,7 +324,7 @@ describe('Veeam routes - HEAD request UtilizationService error handling', () => return response; }; - it('should handle HEAD request for system.xml without calling UtilizationService', done => { + it('should handle HEAD request for system.xml without calling UtilizationService', async () => { const bucketMdWithSystem = { ...bucketMd, _capabilities: { @@ -370,43 +342,37 @@ describe('Veeam routes - HEAD request UtilizationService error handling', () => const request = createRequest('.system-d26a9498-cb7c-4a87-a44a-8ae204f5ba6c/system.xml'); const response = createResponse(); - headVeeamFile(request, response, bucketMdWithSystem, log); + await headVeeamFile(request, response, bucketMdWithSystem, log); - giveAsyncCallbackTimeToExecute(() => { - assert(!utilizationStub.called, 'should not call UtilizationService for system.xml'); - assert(response.setHeader.called, 'should set headers'); - assert(response.end.called, 'response should be ended'); - done(); - }); + assert(!utilizationStub.called, 'should not call UtilizationService for system.xml'); + assert(response.setHeader.called, 'should set headers'); + assert(response.end.called, 'response should be ended'); }); - it('should call UtilizationService for capacity.xml and use metrics date', done => { + it('should call UtilizationService for capacity.xml and use metrics date', async () => { const metricsDate = '2026-03-26T19:00:08.996Z'; utilizationStub.callsArgWith(4, null, { bytesTotal: 123456789, date: metricsDate }); const request = createRequest('.system-d26a9498-cb7c-4a87-a44a-8ae204f5ba6c/capacity.xml'); const response = createResponse(); - headVeeamFile(request, response, bucketMd, log); + await headVeeamFile(request, response, bucketMd, log); - giveAsyncCallbackTimeToExecute(() => { - assert(utilizationStub.calledOnce, 'should call UtilizationService once'); - assert(response.setHeader.called, 'should set headers'); - assert(response.end.called, 'response should be ended'); + assert(utilizationStub.calledOnce, 'should call UtilizationService once'); + assert(response.setHeader.called, 'should set headers'); + assert(response.end.called, 'response should be ended'); - const lastModifiedCall = response.setHeader.getCalls() - .find(call => call.args[0] === 'Last-Modified'); - assert(lastModifiedCall, 'Last-Modified header should be set'); - assert.strictEqual( - lastModifiedCall.args[1], - new Date(metricsDate).toUTCString(), - 'Last-Modified should use the date from bucketMetrics', - ); - done(); - }); + const lastModifiedCall = response.setHeader.getCalls() + .find(call => call.args[0] === 'Last-Modified'); + assert(lastModifiedCall, 'Last-Modified header should be set'); + assert.strictEqual( + lastModifiedCall.args[1], + new Date(metricsDate).toUTCString(), + 'Last-Modified should use the date from bucketMetrics', + ); }); - it('should handle 404 from UtilizationService on HEAD and return 200', done => { + it('should handle 404 from UtilizationService on HEAD and return 200', async () => { const error404 = new Error('Not Found'); error404.response = { status: 404 }; utilizationStub.callsArgWith(4, error404); @@ -414,21 +380,18 @@ describe('Veeam routes - HEAD request UtilizationService error handling', () => const request = createRequest('.system-d26a9498-cb7c-4a87-a44a-8ae204f5ba6c/capacity.xml'); const response = createResponse(); - headVeeamFile(request, response, bucketMd, log); + await headVeeamFile(request, response, bucketMd, log); - giveAsyncCallbackTimeToExecute(() => { - assert(logWarnSpy.calledOnce, 'log.warn should have been called once'); - const warnCall = logWarnSpy.getCall(0); - assert(warnCall.args[0].includes('UtilizationService returned 404'), - 'warning message should mention 404'); - assert.strictEqual(warnCall.args[1].method, 'headVeeamFile'); - assert(response.setHeader.called, 'should set headers'); - assert(response.end.called, 'response should be ended'); - done(); - }); + assert(logWarnSpy.calledOnce, 'log.warn should have been called once'); + const warnCall = logWarnSpy.getCall(0); + assert(warnCall.args[0].includes('UtilizationService returned 404'), + 'warning message should mention 404'); + assert.strictEqual(warnCall.args[1].method, 'headVeeamFile'); + assert(response.setHeader.called, 'should set headers'); + assert(response.end.called, 'response should be ended'); }); - it('should handle non-404 error from UtilizationService on HEAD and return 500', done => { + it('should handle non-404 error from UtilizationService on HEAD and return 500', async () => { const error500 = new Error('Internal Server Error'); error500.response = { status: 500 }; utilizationStub.callsArgWith(4, error500); @@ -436,15 +399,12 @@ describe('Veeam routes - HEAD request UtilizationService error handling', () => const request = createRequest('.system-d26a9498-cb7c-4a87-a44a-8ae204f5ba6c/capacity.xml'); const response = createResponse(); - headVeeamFile(request, response, bucketMd, log); + await headVeeamFile(request, response, bucketMd, log); - giveAsyncCallbackTimeToExecute(() => { - assert(response.end.called, 'response should be ended'); - done(); - }); + assert(response.end.called, 'response should be ended'); }); - it('should return 404 when no VeeamSOSApi capabilities', done => { + it('should return 404 when no VeeamSOSApi capabilities', async () => { const bucketMdWithoutVeeam = { ...bucketMd, _capabilities: {}, @@ -455,13 +415,10 @@ describe('Veeam routes - HEAD request UtilizationService error handling', () => const request = createRequest(); const response = createResponse(); - headVeeamFile(request, response, bucketMdWithoutVeeam, log); + await headVeeamFile(request, response, bucketMdWithoutVeeam, log); - giveAsyncCallbackTimeToExecute(() => { - // HEAD should return 404 via headers, not body - assert(response.end.called, 'response should be ended'); - done(); - }); + // HEAD should return 404 via headers, not body + assert(response.end.called, 'response should be ended'); }); }); @@ -549,24 +506,21 @@ describe('Veeam routes - LIST request handling', () => { return response; }; - it('should list both system.xml and capacity.xml when both are present', done => { + it('should list both system.xml and capacity.xml when both are present', async () => { const metricsDate = '2026-03-26T19:00:08.996Z'; utilizationStub.callsArgWith(4, null, { bytesTotal: 123456789, date: metricsDate }); const request = createRequest(); const response = createResponse(); - listVeeamFiles(request, response, bucketMd, log); + await listVeeamFiles(request, response, bucketMd, log); - giveAsyncCallbackTimeToExecute(() => { - assert(utilizationStub.calledOnce, 'should call UtilizationService once'); - assert(response.writeHead.calledWith(200), 'should return 200'); - assert(response.end.called, 'response should be ended'); - done(); - }); + assert(utilizationStub.calledOnce, 'should call UtilizationService once'); + assert(response.writeHead.calledWith(200), 'should return 200'); + assert(response.end.called, 'response should be ended'); }); - it('should handle 404 from UtilizationService on LIST and return 200', done => { + it('should handle 404 from UtilizationService on LIST and return 200', async () => { const error404 = new Error('Not Found'); error404.response = { status: 404 }; utilizationStub.callsArgWith(4, error404); @@ -574,21 +528,18 @@ describe('Veeam routes - LIST request handling', () => { const request = createRequest(); const response = createResponse(); - listVeeamFiles(request, response, bucketMd, log); + await listVeeamFiles(request, response, bucketMd, log); - giveAsyncCallbackTimeToExecute(() => { - assert(logWarnSpy.calledOnce, 'log.warn should have been called once'); - const warnCall = logWarnSpy.getCall(0); - assert(warnCall.args[0].includes('UtilizationService returned 404'), - 'warning message should mention 404'); - assert.strictEqual(warnCall.args[1].method, 'listVeeamFiles'); - assert(response.writeHead.calledWith(200), 'should return 200 despite 404'); - assert(response.end.called, 'response should be ended'); - done(); - }); + assert(logWarnSpy.calledOnce, 'log.warn should have been called once'); + const warnCall = logWarnSpy.getCall(0); + assert(warnCall.args[0].includes('UtilizationService returned 404'), + 'warning message should mention 404'); + assert.strictEqual(warnCall.args[1].method, 'listVeeamFiles'); + assert(response.writeHead.calledWith(200), 'should return 200 despite 404'); + assert(response.end.called, 'response should be ended'); }); - it('should handle non-404 error from UtilizationService on LIST and return 500', done => { + it('should handle non-404 error from UtilizationService on LIST and return 500', async () => { const error500 = new Error('Internal Server Error'); error500.response = { status: 500 }; utilizationStub.callsArgWith(4, error500); @@ -596,80 +547,68 @@ describe('Veeam routes - LIST request handling', () => { const request = createRequest(); const response = createResponse(); - listVeeamFiles(request, response, bucketMd, log); + await listVeeamFiles(request, response, bucketMd, log); - giveAsyncCallbackTimeToExecute(() => { - assert(response.end.called, 'response should be ended'); - done(); - }); + assert(response.end.called, 'response should be ended'); }); - it('should handle versions query parameter', done => { + it('should handle versions query parameter', async () => { utilizationStub.callsArgWith(4, null, { bytesTotal: 0, date: new Date().toISOString() }); const request = createRequest({ versions: '' }); const response = createResponse(); - listVeeamFiles(request, response, bucketMd, log); + await listVeeamFiles(request, response, bucketMd, log); - giveAsyncCallbackTimeToExecute(() => { - assert(response.writeHead.calledWith(200), 'should return 200 for versions query'); - assert(response.end.called, 'response should be ended'); - done(); - }); + assert(response.writeHead.calledWith(200), 'should return 200 for versions query'); + assert(response.end.called, 'response should be ended'); }); - it('should return error for invalid query parameters', done => { + it('should return error for invalid query parameters', async () => { const request = createRequest({ 'invalid-param': 'value' }); const response = createResponse(); - listVeeamFiles(request, response, bucketMd, log); + await listVeeamFiles(request, response, bucketMd, log); - giveAsyncCallbackTimeToExecute(() => { - // Should return error for invalid query parameter - assert(response.end.called, 'response should be ended'); - done(); - }); + // Should return error for invalid query parameter + assert(response.end.called, 'response should be ended'); }); - it('should handle missing bucket metadata', done => { + it('should handle missing bucket metadata', async () => { const request = createRequest(); const response = createResponse(); - listVeeamFiles(request, response, null, log); + await listVeeamFiles(request, response, null, log); - giveAsyncCallbackTimeToExecute(() => { - assert(response.writeHead.calledWith(404), 'should return 404'); - assert(response.end.called, 'response should be ended'); - done(); - }); + assert(response.writeHead.calledWith(404), 'should return 404'); + assert(response.end.called, 'response should be ended'); }); - it('should list only available files when only SystemInfo is present, without calling UtilizationService', done => { - const bucketMdOnlySystem = { - ...bucketMd, - _capabilities: { - VeeamSOSApi: { - SystemInfo: { - ProtocolVersion: '1.0', - ModelName: 'ARTESCA', - LastModified: '2024-01-01T00:00:00.000Z', + it( + 'should list only available files when only SystemInfo is present, without calling UtilizationService', + async () => { + const bucketMdOnlySystem = { + ...bucketMd, + _capabilities: { + VeeamSOSApi: { + SystemInfo: { + ProtocolVersion: '1.0', + ModelName: 'ARTESCA', + LastModified: '2024-01-01T00:00:00.000Z', + }, }, }, - }, - }; - metadataStub.callsArgWith(2, null, bucketMdOnlySystem); + }; + metadataStub.callsArgWith(2, null, bucketMdOnlySystem); - const request = createRequest(); - const response = createResponse(); + const request = createRequest(); + const response = createResponse(); - listVeeamFiles(request, response, bucketMdOnlySystem, log); + await listVeeamFiles(request, response, bucketMdOnlySystem, log); - giveAsyncCallbackTimeToExecute(() => { assert(!utilizationStub.called, 'should not call UtilizationService without CapacityInfo'); assert(response.writeHead.calledWith(200), 'should return 200'); assert(response.end.called, 'response should be ended'); - done(); - }); - }); + }, + ); }); diff --git a/tests/unit/routes/veeam-utils.js b/tests/unit/routes/veeam-utils.js index 6466343e73..d766ec4ac0 100644 --- a/tests/unit/routes/veeam-utils.js +++ b/tests/unit/routes/veeam-utils.js @@ -34,89 +34,82 @@ describe('fetchCapacityMetrics', () => { sinon.restore(); }); - it('should call UtilizationService with the correct bucket key', done => { + it('should call UtilizationService with the correct bucket key', async () => { utilizationStub.callsArgWith(4, null, {}); - fetchCapacityMetrics(bucketMd, request, log, 'testMethod', () => { - const expectedKey = `test-bucket_${new Date('2024-01-01T00:00:00.000Z').getTime()}`; - assert.strictEqual(utilizationStub.getCall(0).args[0], 'bucket'); - assert.strictEqual(utilizationStub.getCall(0).args[1], expectedKey); - done(); - }); + await fetchCapacityMetrics(bucketMd, request, log, 'testMethod'); + + const expectedKey = `test-bucket_${new Date('2024-01-01T00:00:00.000Z').getTime()}`; + assert.strictEqual(utilizationStub.getCall(0).args[0], 'bucket'); + assert.strictEqual(utilizationStub.getCall(0).args[1], expectedKey); }); - it('should call back with metrics on success', done => { + it('should resolve with metrics on success', async () => { const bucketMetrics = { bytesTotal: 42, date: '2026-03-26T19:00:08.996Z' }; utilizationStub.callsArgWith(4, null, bucketMetrics); - fetchCapacityMetrics(bucketMd, request, log, 'testMethod', (err, metrics) => { - assert.ifError(err); - assert.strictEqual(metrics, bucketMetrics); - assert(!logWarnSpy.called); - assert(!logErrorSpy.called); - done(); - }); + const metrics = await fetchCapacityMetrics(bucketMd, request, log, 'testMethod'); + + assert.strictEqual(metrics, bucketMetrics); + assert(!logWarnSpy.called); + assert(!logErrorSpy.called); }); - it('should call back with no error and a default date on 404', done => { + it('should resolve with no error and a default date on 404', async () => { const error404 = new Error('Not Found'); error404.response = { status: 404 }; utilizationStub.callsArgWith(4, error404); - fetchCapacityMetrics(bucketMd, request, log, 'testMethod', (err, metrics) => { - assert.ifError(err); - assert(metrics && metrics.date instanceof Date, 'metrics should have a Date for date'); - assert(logWarnSpy.calledOnce); - assert(logWarnSpy.getCall(0).args[0].includes('404')); - assert.strictEqual(logWarnSpy.getCall(0).args[1].method, 'testMethod'); - assert.strictEqual(logWarnSpy.getCall(0).args[1].bucket, 'test-bucket'); - assert(!logErrorSpy.called); - done(); - }); + const metrics = await fetchCapacityMetrics(bucketMd, request, log, 'testMethod'); + + assert(metrics && metrics.date instanceof Date, 'metrics should have a Date for date'); + assert(logWarnSpy.calledOnce); + assert(logWarnSpy.getCall(0).args[0].includes('404')); + assert.strictEqual(logWarnSpy.getCall(0).args[1].method, 'testMethod'); + assert.strictEqual(logWarnSpy.getCall(0).args[1].bucket, 'test-bucket'); + assert(!logErrorSpy.called); }); - it('should also handle 404 via statusCode property', done => { + it('should also handle 404 via statusCode property', async () => { const error404 = new Error('Not Found'); error404.statusCode = 404; utilizationStub.callsArgWith(4, error404); - fetchCapacityMetrics(bucketMd, request, log, 'testMethod', (err, metrics) => { - assert.ifError(err); - assert(metrics && metrics.date instanceof Date, 'metrics should have a Date for date'); - assert(logWarnSpy.calledOnce); - done(); - }); + const metrics = await fetchCapacityMetrics(bucketMd, request, log, 'testMethod'); + + assert(metrics && metrics.date instanceof Date, 'metrics should have a Date for date'); + assert(logWarnSpy.calledOnce); }); - it('should call back with error on non-404 failures', done => { + it('should reject with error on non-404 failures', async () => { const error500 = new Error('Internal Server Error'); error500.response = { status: 500 }; utilizationStub.callsArgWith(4, error500); - fetchCapacityMetrics(bucketMd, request, log, 'testMethod', (err, metrics) => { - assert.strictEqual(err, error500); - assert.strictEqual(metrics, undefined); - assert(logErrorSpy.calledOnce); - assert.strictEqual(logErrorSpy.getCall(0).args[1].method, 'testMethod'); - assert.strictEqual(logErrorSpy.getCall(0).args[1].bucket, 'test-bucket'); - assert.strictEqual(logErrorSpy.getCall(0).args[1].statusCode, 500); - assert(!logWarnSpy.called); - done(); - }); + await assert.rejects( + fetchCapacityMetrics(bucketMd, request, log, 'testMethod'), + err => err === error500, + ); + + assert(logErrorSpy.calledOnce); + assert.strictEqual(logErrorSpy.getCall(0).args[1].method, 'testMethod'); + assert.strictEqual(logErrorSpy.getCall(0).args[1].bucket, 'test-bucket'); + assert.strictEqual(logErrorSpy.getCall(0).args[1].statusCode, 500); + assert(!logWarnSpy.called); }); - it('should call back with error on connection errors', done => { + it('should reject with error on connection errors', async () => { const connError = new Error('Connection refused'); connError.code = 'ECONNREFUSED'; utilizationStub.callsArgWith(4, connError); - fetchCapacityMetrics(bucketMd, request, log, 'testMethod', (err, metrics) => { - assert.strictEqual(err, connError); - assert.strictEqual(metrics, undefined); - assert(logErrorSpy.calledOnce); - assert.strictEqual(logErrorSpy.getCall(0).args[1].statusCode, 'ECONNREFUSED'); - done(); - }); + await assert.rejects( + fetchCapacityMetrics(bucketMd, request, log, 'testMethod'), + err => err === connError, + ); + + assert(logErrorSpy.calledOnce); + assert.strictEqual(logErrorSpy.getCall(0).args[1].statusCode, 'ECONNREFUSED'); }); }); @@ -177,90 +170,81 @@ describe('buildVeeamFileData', () => { sinon.restore(); }); - it('should return InternalError when metadata.getBucket fails', done => { + it('should reject with InternalError when metadata.getBucket fails', async () => { metadataStub.callsArgWith(2, new Error('DB error')); - buildVeeamFileData(createRequest(capacityObjectKey), bucketMd, log, 'test', (err) => { - assert(err); - assert.strictEqual(err.code, 500); - done(); - }); + await assert.rejects( + buildVeeamFileData(createRequest(capacityObjectKey), bucketMd, log, 'test'), + err => err.code === 500, + ); }); - it('should return NoSuchKey when capabilities do not include the requested file', done => { + it('should reject with NoSuchKey when capabilities do not include the requested file', async () => { metadataStub.callsArgWith(2, null, { _capabilities: {} }); - buildVeeamFileData(createRequest(capacityObjectKey), bucketMd, log, 'test', (err) => { - assert(err); - assert.strictEqual(err.code, 404); - done(); - }); + await assert.rejects( + buildVeeamFileData(createRequest(capacityObjectKey), bucketMd, log, 'test'), + err => err.code === 404, + ); }); - it('should return InternalError when fetchCapacityMetrics fails', done => { + it('should reject with InternalError when fetchCapacityMetrics fails', async () => { metadataStub.callsArgWith(2, null, bucketMd); const error500 = new Error('Internal Server Error'); error500.response = { status: 500 }; utilizationStub.callsArgWith(4, error500); - buildVeeamFileData(createRequest(capacityObjectKey), bucketMd, log, 'test', (err) => { - assert(err); - assert.strictEqual(err.code, 500); - done(); - }); + await assert.rejects( + buildVeeamFileData(createRequest(capacityObjectKey), bucketMd, log, 'test'), + err => err.code === 500, + ); }); - it('should build capacity.xml with SUR metrics date and applied Used/Available', done => { + it('should build capacity.xml with SUR metrics date and applied Used/Available', async () => { const metricsDate = new Date('2026-03-26T19:00:08.996Z'); metadataStub.callsArgWith(2, null, bucketMd); utilizationStub.callsArgWith(4, null, { date: metricsDate, bytesTotal: 100 }); - buildVeeamFileData(createRequest(capacityObjectKey), bucketMd, log, 'test', (err, result) => { - assert.ifError(err); - assert.strictEqual(result.modified, metricsDate); - assert(result.xmlContent.includes('CapacityInfo')); - assert(result.xmlContent.includes('100')); - assert(result.xmlContent.includes('')); - assert(Buffer.isBuffer(result.dataBuffer)); - assert.deepStrictEqual(result.dataBuffer, Buffer.from(result.xmlContent)); - assert.strictEqual(result.bucketData, bucketMd); - done(); - }); + const result = await buildVeeamFileData(createRequest(capacityObjectKey), bucketMd, log, 'test'); + + assert.strictEqual(result.modified, metricsDate); + assert(result.xmlContent.includes('CapacityInfo')); + assert(result.xmlContent.includes('100')); + assert(result.xmlContent.includes('')); + assert(Buffer.isBuffer(result.dataBuffer)); + assert.deepStrictEqual(result.dataBuffer, Buffer.from(result.xmlContent)); + assert.strictEqual(result.bucketData, bucketMd); }); - it('should use current date for capacity.xml when UtilizationService returns 404', done => { + it('should use current date for capacity.xml when UtilizationService returns 404', async () => { const before = Date.now(); metadataStub.callsArgWith(2, null, bucketMd); const error404 = new Error('Not Found'); error404.response = { status: 404 }; utilizationStub.callsArgWith(4, error404); - buildVeeamFileData(createRequest(capacityObjectKey), bucketMd, log, 'test', (err, result) => { - assert.ifError(err); - assert(result.modified instanceof Date); - assert(result.modified.getTime() >= before); - assert(result.xmlContent.includes('CapacityInfo')); - done(); - }); + const result = await buildVeeamFileData(createRequest(capacityObjectKey), bucketMd, log, 'test'); + + assert(result.modified instanceof Date); + assert(result.modified.getTime() >= before); + assert(result.xmlContent.includes('CapacityInfo')); }); - it('should build system.xml without calling UtilizationService', done => { + it('should build system.xml without calling UtilizationService', async () => { metadataStub.callsArgWith(2, null, bucketMdWithSystem); const before = Date.now(); - buildVeeamFileData(createRequest(systemObjectKey), bucketMdWithSystem, log, 'test', (err, result) => { - assert.ifError(err); - assert(!utilizationStub.called, 'should not call UtilizationService for system.xml'); - assert(result.modified instanceof Date); - assert(result.modified.getTime() >= before); - assert(result.xmlContent.includes('SystemInfo')); - assert(result.xmlContent.includes('ARTESCA')); - assert(Buffer.isBuffer(result.dataBuffer)); - done(); - }); + const result = await buildVeeamFileData(createRequest(systemObjectKey), bucketMdWithSystem, log, 'test'); + + assert(!utilizationStub.called, 'should not call UtilizationService for system.xml'); + assert(result.modified instanceof Date); + assert(result.modified.getTime() >= before); + assert(result.xmlContent.includes('SystemInfo')); + assert(result.xmlContent.includes('ARTESCA')); + assert(Buffer.isBuffer(result.dataBuffer)); }); - it('should not overwrite Used when it is already set', done => { + it('should not overwrite Used when it is already set', async () => { const bucketMdWithUsed = { ...bucketMd, _capabilities: { @@ -277,10 +261,8 @@ describe('buildVeeamFileData', () => { metadataStub.callsArgWith(2, null, bucketMdWithUsed); utilizationStub.callsArgWith(4, null, { date: new Date(), bytesTotal: 999 }); - buildVeeamFileData(createRequest(capacityObjectKey), bucketMdWithUsed, log, 'test', (err, result) => { - assert.ifError(err); - assert(result.xmlContent.includes('400'), 'should keep existing Used value'); - done(); - }); + const result = await buildVeeamFileData(createRequest(capacityObjectKey), bucketMdWithUsed, log, 'test'); + + assert(result.xmlContent.includes('400'), 'should keep existing Used value'); }); }); From 6f9012b5ff85f2fcbe9914ef14010994e1cef9ec Mon Sep 17 00:00:00 2001 From: DarkIsDude Date: Wed, 1 Apr 2026 14:35:47 +0200 Subject: [PATCH 08/12] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20prefer=20promisify?= =?UTF-8?q?=20over=20manual=20new=20Promise?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue: CLDSRV-878 --- lib/routes/veeam/list.js | 9 +++------ lib/routes/veeam/utils.js | 16 ++++------------ 2 files changed, 7 insertions(+), 18 deletions(-) diff --git a/lib/routes/veeam/list.js b/lib/routes/veeam/list.js index 2ff7e62b9d..bfd1b78f2a 100644 --- a/lib/routes/veeam/list.js +++ b/lib/routes/veeam/list.js @@ -6,7 +6,9 @@ const metadata = require('../../metadata/wrapper'); const { responseXMLBody } = require('arsenal/build/lib/s3routes/routesUtils'); const { respondWithData, getResponseHeader, buildHeadXML, validPath, fetchCapacityMetrics } = require('./utils'); const { processVersions, processMasterVersions } = require('../../api/bucketGet'); +const { promisify } = require('util'); +metadata.getBucketPromised = promisify(metadata.getBucket); /** * Utility function to build a standard response for the LIST route. @@ -96,12 +98,7 @@ async function listVeeamFiles(request, response, bucketMd, log) { let data; try { - data = await new Promise((resolve, reject) => - metadata.getBucket(request.bucketName, log, (err, result) => { - if (err) { return reject(err); } - return resolve(result); - }) - ); + data = await metadata.getBucket(request.bucketName, log); } catch { return responseXMLBody(errors.InternalError, null, response, log); } diff --git a/lib/routes/veeam/utils.js b/lib/routes/veeam/utils.js index 008417c9b2..b17efcff84 100644 --- a/lib/routes/veeam/utils.js +++ b/lib/routes/veeam/utils.js @@ -10,6 +10,8 @@ const UtilizationService = require('../../utilization/instance'); const metadata = require('../../metadata/wrapper'); const pipeline = promisify(streamPipeline); +const getUtilizationMetrics = promisify(UtilizationService.getUtilizationMetrics.bind(UtilizationService)); +metadata.getBucketPromised = promisify(metadata.getBucket); /** * Decodes an URI and return the result. @@ -217,12 +219,7 @@ function getFileToBuild(request, data, inlineLastModified = false) { async function fetchCapacityMetrics(bucketMd, request, log, method) { const bucketKey = `${bucketMd._name}_${new Date(bucketMd._creationDate).getTime()}`; try { - return await new Promise((resolve, reject) => - UtilizationService.getUtilizationMetrics('bucket', bucketKey, null, {}, (err, bucketMetrics) => { - if (err) { return reject(err); } - return resolve(bucketMetrics); - }) - ); + return await getUtilizationMetrics('bucket', bucketKey, null, {}); } catch (err) { const statusCode = err.response?.status || err.statusCode || err.code; if (statusCode === 404) { @@ -255,12 +252,7 @@ async function fetchCapacityMetrics(bucketMd, request, log, method) { async function buildVeeamFileData(request, bucketMd, log, name) { let data; try { - data = await new Promise((resolve, reject) => - metadata.getBucket(request.bucketName, log, (err, result) => { - if (err) { return reject(err); } - return resolve(result); - }) - ); + data = await metadata.getBucketPromised(request.bucketName, log); } catch { throw errors.InternalError; } From 78fd519dd768aeb4fd528787a8a74eefe068d90f Mon Sep 17 00:00:00 2001 From: DarkIsDude Date: Wed, 1 Apr 2026 16:18:45 +0200 Subject: [PATCH 09/12] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20remove=20useless=20a?= =?UTF-8?q?rgument=20to=20follow=20caller?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue: CLDSRV-878 --- lib/routes/veeam/get.js | 2 +- lib/routes/veeam/head.js | 2 +- lib/routes/veeam/list.js | 6 +++--- lib/routes/veeam/utils.js | 21 ++++++++------------- tests/unit/routes/veeam-routes.js | 3 --- tests/unit/routes/veeam-utils.js | 28 +++++++++++++--------------- 6 files changed, 26 insertions(+), 36 deletions(-) diff --git a/lib/routes/veeam/get.js b/lib/routes/veeam/get.js index 09c394b465..213d5c4866 100644 --- a/lib/routes/veeam/get.js +++ b/lib/routes/veeam/get.js @@ -22,7 +22,7 @@ async function getVeeamFile(request, response, bucketMd, log) { } try { - const result = await buildVeeamFileData(request, bucketMd, log, 'getVeeamFile'); + const result = await buildVeeamFileData(request, bucketMd, log); return await respondWithData(request, response, log, result.bucketData, result.xmlContent, result.modified); } catch (err) { return responseXMLBody(err, null, response, log); diff --git a/lib/routes/veeam/head.js b/lib/routes/veeam/head.js index 5c888f1030..5aeca5f78e 100644 --- a/lib/routes/veeam/head.js +++ b/lib/routes/veeam/head.js @@ -17,7 +17,7 @@ async function headVeeamFile(request, response, bucketMd, log) { } try { - const result = await buildVeeamFileData(request, bucketMd, log, 'headVeeamFile'); + const result = await buildVeeamFileData(request, bucketMd, log); return responseContentHeaders( null, {}, diff --git a/lib/routes/veeam/list.js b/lib/routes/veeam/list.js index bfd1b78f2a..083471d9bd 100644 --- a/lib/routes/veeam/list.js +++ b/lib/routes/veeam/list.js @@ -8,7 +8,7 @@ const { respondWithData, getResponseHeader, buildHeadXML, validPath, fetchCapaci const { processVersions, processMasterVersions } = require('../../api/bucketGet'); const { promisify } = require('util'); -metadata.getBucketPromised = promisify(metadata.getBucket); +const getBucket = promisify((...args) => metadata.getBucket(...args)); /** * Utility function to build a standard response for the LIST route. @@ -98,7 +98,7 @@ async function listVeeamFiles(request, response, bucketMd, log) { let data; try { - data = await metadata.getBucket(request.bucketName, log); + data = await getBucket(request.bucketName, log); } catch { return responseXMLBody(errors.InternalError, null, response, log); } @@ -106,7 +106,7 @@ async function listVeeamFiles(request, response, bucketMd, log) { let bucketMetrics; if (data._capabilities?.VeeamSOSApi?.CapacityInfo) { try { - bucketMetrics = await fetchCapacityMetrics(bucketMd, request, log, 'listVeeamFiles'); + bucketMetrics = await fetchCapacityMetrics(bucketMd, request, log); } catch { return responseXMLBody(errors.InternalError, null, response, log); } diff --git a/lib/routes/veeam/utils.js b/lib/routes/veeam/utils.js index b17efcff84..9b116b660f 100644 --- a/lib/routes/veeam/utils.js +++ b/lib/routes/veeam/utils.js @@ -10,8 +10,8 @@ const UtilizationService = require('../../utilization/instance'); const metadata = require('../../metadata/wrapper'); const pipeline = promisify(streamPipeline); -const getUtilizationMetrics = promisify(UtilizationService.getUtilizationMetrics.bind(UtilizationService)); -metadata.getBucketPromised = promisify(metadata.getBucket); +const getUtilizationMetrics = promisify((...args) => UtilizationService.getUtilizationMetrics(...args)); +const getBucket = promisify((...args) => metadata.getBucket(...args)); /** * Decodes an URI and return the result. @@ -216,7 +216,7 @@ function getFileToBuild(request, data, inlineLastModified = false) { * @returns {Promise} bucketMetrics always has at least a `date` field; * on a real 404 the date defaults to new Date() */ -async function fetchCapacityMetrics(bucketMd, request, log, method) { +async function fetchCapacityMetrics(bucketMd, request, log) { const bucketKey = `${bucketMd._name}_${new Date(bucketMd._creationDate).getTime()}`; try { return await getUtilizationMetrics('bucket', bucketKey, null, {}); @@ -224,14 +224,12 @@ async function fetchCapacityMetrics(bucketMd, request, log, method) { const statusCode = err.response?.status || err.statusCode || err.code; if (statusCode === 404) { log.warn('UtilizationService returned 404 when fetching capacity metrics', { - method, bucket: request.bucketName, error: err.message || err.code, }); return { date: new Date() }; } log.error('error fetching capacity metrics from UtilizationService', { - method, bucket: request.bucketName, error: err.message || err.code, statusCode, @@ -246,13 +244,12 @@ async function fetchCapacityMetrics(bucketMd, request, log, method) { * @param {object} request - incoming request * @param {object} bucketMd - bucket metadata from the router * @param {object} log - logger object - * @param {string} name - calling method name (for log context) * @returns {Promise} result with { xmlContent, dataBuffer, modified, bucketData } */ -async function buildVeeamFileData(request, bucketMd, log, name) { +async function buildVeeamFileData(request, bucketMd, log) { let data; try { - data = await metadata.getBucketPromised(request.bucketName, log); + data = await getBucket(request.bucketName, log); } catch { throw errors.InternalError; } @@ -266,7 +263,7 @@ async function buildVeeamFileData(request, bucketMd, log, name) { let bucketMetrics; if (!isSystemXML(request.objectKey)) { try { - bucketMetrics = await fetchCapacityMetrics(bucketMd, request, log, name); + bucketMetrics = await fetchCapacityMetrics(bucketMd, request, log); } catch { throw errors.InternalError; } @@ -275,11 +272,9 @@ async function buildVeeamFileData(request, bucketMd, log, name) { } const modified = bucketMetrics.date; - if ( - bucketMetrics.bytesTotal !== undefined + if (bucketMetrics.bytesTotal !== undefined && fileToBuild.value.CapacityInfo - && !fileToBuild.value.CapacityInfo.Used - ) { + && !fileToBuild.value.CapacityInfo.Used) { fileToBuild.value.CapacityInfo.Used = Number(bucketMetrics.bytesTotal); fileToBuild.value.CapacityInfo.Available = Number(fileToBuild.value.CapacityInfo.Capacity) - Number(bucketMetrics.bytesTotal); diff --git a/tests/unit/routes/veeam-routes.js b/tests/unit/routes/veeam-routes.js index 4d35ad8829..3b7cb73896 100644 --- a/tests/unit/routes/veeam-routes.js +++ b/tests/unit/routes/veeam-routes.js @@ -115,7 +115,6 @@ describe('Veeam routes - comprehensive unit tests', () => { const warnCall = logWarnSpy.getCall(0); assert(warnCall.args[0].includes('UtilizationService returned 404'), 'warning message should mention 404'); - assert.strictEqual(warnCall.args[1].method, 'getVeeamFile'); assert.strictEqual(warnCall.args[1].bucket, 'test-bucket'); assert(response.writeHead.calledWith(200), @@ -386,7 +385,6 @@ describe('Veeam routes - HEAD request UtilizationService error handling', () => const warnCall = logWarnSpy.getCall(0); assert(warnCall.args[0].includes('UtilizationService returned 404'), 'warning message should mention 404'); - assert.strictEqual(warnCall.args[1].method, 'headVeeamFile'); assert(response.setHeader.called, 'should set headers'); assert(response.end.called, 'response should be ended'); }); @@ -534,7 +532,6 @@ describe('Veeam routes - LIST request handling', () => { const warnCall = logWarnSpy.getCall(0); assert(warnCall.args[0].includes('UtilizationService returned 404'), 'warning message should mention 404'); - assert.strictEqual(warnCall.args[1].method, 'listVeeamFiles'); assert(response.writeHead.calledWith(200), 'should return 200 despite 404'); assert(response.end.called, 'response should be ended'); }); diff --git a/tests/unit/routes/veeam-utils.js b/tests/unit/routes/veeam-utils.js index d766ec4ac0..a76ee65f7c 100644 --- a/tests/unit/routes/veeam-utils.js +++ b/tests/unit/routes/veeam-utils.js @@ -37,7 +37,7 @@ describe('fetchCapacityMetrics', () => { it('should call UtilizationService with the correct bucket key', async () => { utilizationStub.callsArgWith(4, null, {}); - await fetchCapacityMetrics(bucketMd, request, log, 'testMethod'); + await fetchCapacityMetrics(bucketMd, request, log); const expectedKey = `test-bucket_${new Date('2024-01-01T00:00:00.000Z').getTime()}`; assert.strictEqual(utilizationStub.getCall(0).args[0], 'bucket'); @@ -48,7 +48,7 @@ describe('fetchCapacityMetrics', () => { const bucketMetrics = { bytesTotal: 42, date: '2026-03-26T19:00:08.996Z' }; utilizationStub.callsArgWith(4, null, bucketMetrics); - const metrics = await fetchCapacityMetrics(bucketMd, request, log, 'testMethod'); + const metrics = await fetchCapacityMetrics(bucketMd, request, log); assert.strictEqual(metrics, bucketMetrics); assert(!logWarnSpy.called); @@ -60,12 +60,11 @@ describe('fetchCapacityMetrics', () => { error404.response = { status: 404 }; utilizationStub.callsArgWith(4, error404); - const metrics = await fetchCapacityMetrics(bucketMd, request, log, 'testMethod'); + const metrics = await fetchCapacityMetrics(bucketMd, request, log); assert(metrics && metrics.date instanceof Date, 'metrics should have a Date for date'); assert(logWarnSpy.calledOnce); assert(logWarnSpy.getCall(0).args[0].includes('404')); - assert.strictEqual(logWarnSpy.getCall(0).args[1].method, 'testMethod'); assert.strictEqual(logWarnSpy.getCall(0).args[1].bucket, 'test-bucket'); assert(!logErrorSpy.called); }); @@ -75,7 +74,7 @@ describe('fetchCapacityMetrics', () => { error404.statusCode = 404; utilizationStub.callsArgWith(4, error404); - const metrics = await fetchCapacityMetrics(bucketMd, request, log, 'testMethod'); + const metrics = await fetchCapacityMetrics(bucketMd, request, log); assert(metrics && metrics.date instanceof Date, 'metrics should have a Date for date'); assert(logWarnSpy.calledOnce); @@ -87,12 +86,11 @@ describe('fetchCapacityMetrics', () => { utilizationStub.callsArgWith(4, error500); await assert.rejects( - fetchCapacityMetrics(bucketMd, request, log, 'testMethod'), + fetchCapacityMetrics(bucketMd, request, log), err => err === error500, ); assert(logErrorSpy.calledOnce); - assert.strictEqual(logErrorSpy.getCall(0).args[1].method, 'testMethod'); assert.strictEqual(logErrorSpy.getCall(0).args[1].bucket, 'test-bucket'); assert.strictEqual(logErrorSpy.getCall(0).args[1].statusCode, 500); assert(!logWarnSpy.called); @@ -104,7 +102,7 @@ describe('fetchCapacityMetrics', () => { utilizationStub.callsArgWith(4, connError); await assert.rejects( - fetchCapacityMetrics(bucketMd, request, log, 'testMethod'), + fetchCapacityMetrics(bucketMd, request, log), err => err === connError, ); @@ -174,7 +172,7 @@ describe('buildVeeamFileData', () => { metadataStub.callsArgWith(2, new Error('DB error')); await assert.rejects( - buildVeeamFileData(createRequest(capacityObjectKey), bucketMd, log, 'test'), + buildVeeamFileData(createRequest(capacityObjectKey), bucketMd, log), err => err.code === 500, ); }); @@ -183,7 +181,7 @@ describe('buildVeeamFileData', () => { metadataStub.callsArgWith(2, null, { _capabilities: {} }); await assert.rejects( - buildVeeamFileData(createRequest(capacityObjectKey), bucketMd, log, 'test'), + buildVeeamFileData(createRequest(capacityObjectKey), bucketMd, log), err => err.code === 404, ); }); @@ -195,7 +193,7 @@ describe('buildVeeamFileData', () => { utilizationStub.callsArgWith(4, error500); await assert.rejects( - buildVeeamFileData(createRequest(capacityObjectKey), bucketMd, log, 'test'), + buildVeeamFileData(createRequest(capacityObjectKey), bucketMd, log), err => err.code === 500, ); }); @@ -205,7 +203,7 @@ describe('buildVeeamFileData', () => { metadataStub.callsArgWith(2, null, bucketMd); utilizationStub.callsArgWith(4, null, { date: metricsDate, bytesTotal: 100 }); - const result = await buildVeeamFileData(createRequest(capacityObjectKey), bucketMd, log, 'test'); + const result = await buildVeeamFileData(createRequest(capacityObjectKey), bucketMd, log); assert.strictEqual(result.modified, metricsDate); assert(result.xmlContent.includes('CapacityInfo')); @@ -223,7 +221,7 @@ describe('buildVeeamFileData', () => { error404.response = { status: 404 }; utilizationStub.callsArgWith(4, error404); - const result = await buildVeeamFileData(createRequest(capacityObjectKey), bucketMd, log, 'test'); + const result = await buildVeeamFileData(createRequest(capacityObjectKey), bucketMd, log); assert(result.modified instanceof Date); assert(result.modified.getTime() >= before); @@ -234,7 +232,7 @@ describe('buildVeeamFileData', () => { metadataStub.callsArgWith(2, null, bucketMdWithSystem); const before = Date.now(); - const result = await buildVeeamFileData(createRequest(systemObjectKey), bucketMdWithSystem, log, 'test'); + const result = await buildVeeamFileData(createRequest(systemObjectKey), bucketMdWithSystem, log); assert(!utilizationStub.called, 'should not call UtilizationService for system.xml'); assert(result.modified instanceof Date); @@ -261,7 +259,7 @@ describe('buildVeeamFileData', () => { metadataStub.callsArgWith(2, null, bucketMdWithUsed); utilizationStub.callsArgWith(4, null, { date: new Date(), bytesTotal: 999 }); - const result = await buildVeeamFileData(createRequest(capacityObjectKey), bucketMdWithUsed, log, 'test'); + const result = await buildVeeamFileData(createRequest(capacityObjectKey), bucketMdWithUsed, log); assert(result.xmlContent.includes('400'), 'should keep existing Used value'); }); From 3f4a308004c4d548824d9d55b5e5631b05baedbf Mon Sep 17 00:00:00 2001 From: DarkIsDude Date: Thu, 2 Apr 2026 12:27:26 +0200 Subject: [PATCH 10/12] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20use=20pipeline=20ins?= =?UTF-8?q?tead=20of=20manual=20stream?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue: CLDSRV-878 --- lib/routes/veeam/utils.js | 67 ++++++++++++++++++--------------------- 1 file changed, 31 insertions(+), 36 deletions(-) diff --git a/lib/routes/veeam/utils.js b/lib/routes/veeam/utils.js index 9b116b660f..6e3e78444a 100644 --- a/lib/routes/veeam/utils.js +++ b/lib/routes/veeam/utils.js @@ -1,6 +1,6 @@ const xml2js = require('xml2js'); const { errors, errorInstances, jsutil } = require('arsenal'); -const { Readable, pipeline: streamPipeline } = require('stream'); +const { Readable, Writable, pipeline: streamPipeline } = require('stream'); const { promisify } = require('util'); const collectResponseHeaders = require('../../utilities/collectResponseHeaders'); const collectCorsHeaders = require('../../utilities/collectCorsHeaders'); @@ -31,41 +31,40 @@ function _decodeURI(uri) { * @returns {Promise} */ async function receiveData(request, log) { - // Get keycontent const { parsedContentLength } = request; const ContentLengthThreshold = 1024 * 1024; // 1MB + // Prevent memory overloads by limiting the size of the // received data. if (parsedContentLength > ContentLengthThreshold) { throw errorInstances.InvalidInput .customizeDescription(`maximum allowed content-length is ${ContentLengthThreshold} bytes`); } - return new Promise((resolve, reject) => { - const value = Buffer.alloc(parsedContentLength); + return await new Promise((resolve, reject) => { const settle = jsutil.once((err, result) => { if (err) { return reject(err); } return resolve(result); }); - const dataStream = prepareStream(request, request.streamingV4Params, log, settle); - let cursor = 0; - let exceeded = false; - dataStream.on('data', data => { - if (cursor + data.length > parsedContentLength) { - exceeded = true; - } - if (!exceeded) { - data.copy(value, cursor); - } - cursor += data.length; - }); - dataStream.on('end', () => { - if (exceeded) { - log.error('data stream exceed announced size', - { parsedContentLength, overflow: cursor }); - return settle(errors.InternalError); - } - return settle(null, value.toString()); + let totalLength = 0; + const chunks = []; + const collector = new Writable({ + write(chunk, _enc, cb) { + totalLength += chunk.length; + if (totalLength > parsedContentLength) { + log.error('data stream exceed announced size', + { parsedContentLength, overflow: totalLength }); + return cb(errors.InternalError); + } + chunks.push(chunk); + return cb(); + }, + final(cb) { + settle(null, Buffer.concat(chunks).toString()); + cb(); + }, }); + const dataStream = prepareStream(request, request.streamingV4Params, log, settle); + pipeline(dataStream, collector).catch(err => settle(err)); }); } @@ -186,21 +185,16 @@ function getFileToBuild(request, data, inlineLastModified = false) { if (inlineLastModified) { fileToBuild.LastModified = modified; - return { - value: { - [fieldName]: fileToBuild, - }, - fieldName, - }; } else { delete fileToBuild.LastModified; - return { - value: { - [fieldName]: fileToBuild, - }, - fieldName, - }; } + + return { + value: { + [fieldName]: fileToBuild, + }, + fieldName, + }; } /** @@ -212,7 +206,6 @@ function getFileToBuild(request, data, inlineLastModified = false) { * @param {object} bucketMd - bucket metadata * @param {object} request - incoming request * @param {object} log - logger object - * @param {string} method - calling method name for log context * @returns {Promise} bucketMetrics always has at least a `date` field; * on a real 404 the date defaults to new Date() */ @@ -251,6 +244,7 @@ async function buildVeeamFileData(request, bucketMd, log) { try { data = await getBucket(request.bucketName, log); } catch { + log.error('error fetching bucket metadata', { bucket: request.bucketName }); throw errors.InternalError; } @@ -265,6 +259,7 @@ async function buildVeeamFileData(request, bucketMd, log) { try { bucketMetrics = await fetchCapacityMetrics(bucketMd, request, log); } catch { + log.error('error fetching capacity metrics for bucket', { bucket: request.bucketName }); throw errors.InternalError; } } else { From 755462801dbacf83157a113d7da3cb7b07fefcc5 Mon Sep 17 00:00:00 2001 From: DarkIsDude Date: Thu, 2 Apr 2026 16:05:28 +0200 Subject: [PATCH 11/12] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20factorise=20XML=20bu?= =?UTF-8?q?ild=20at=20the=20same=20place?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue: CLDSRV-878 --- lib/routes/veeam/list.js | 12 ++---------- lib/routes/veeam/utils.js | 36 ++++++++++++++++++++++++++++-------- 2 files changed, 30 insertions(+), 18 deletions(-) diff --git a/lib/routes/veeam/list.js b/lib/routes/veeam/list.js index 083471d9bd..4abbe9cbda 100644 --- a/lib/routes/veeam/list.js +++ b/lib/routes/veeam/list.js @@ -1,14 +1,9 @@ const url = require('url'); -const xml2js = require('xml2js'); const { errors, errorInstances } = require('arsenal'); const querystring = require('querystring'); -const metadata = require('../../metadata/wrapper'); const { responseXMLBody } = require('arsenal/build/lib/s3routes/routesUtils'); -const { respondWithData, getResponseHeader, buildHeadXML, validPath, fetchCapacityMetrics } = require('./utils'); +const { respondWithData, getResponseHeader, buildXML, validPath, fetchCapacityMetrics, getBucket } = require('./utils'); const { processVersions, processMasterVersions } = require('../../api/bucketGet'); -const { promisify } = require('util'); - -const getBucket = promisify((...args) => metadata.getBucket(...args)); /** * Utility function to build a standard response for the LIST route. @@ -135,10 +130,7 @@ async function listVeeamFiles(request, response, bucketMd, log) { : file.LastModified; // eslint-disable-next-line no-param-reassign delete file.LastModified; - const builder = new xml2js.Builder({ - headless: true, - }); - const dataBuffer = Buffer.from(buildHeadXML(builder.buildObject(file))); + const dataBuffer = Buffer.from(buildXML(file)); filesToBuild.push({ ...getResponseHeader(request, data, dataBuffer, lastModified, log), diff --git a/lib/routes/veeam/utils.js b/lib/routes/veeam/utils.js index 6e3e78444a..3676669435 100644 --- a/lib/routes/veeam/utils.js +++ b/lib/routes/veeam/utils.js @@ -104,6 +104,17 @@ function getResponseHeader(request, bucket, dataBuffer, lastModified, log) { responseMetaHeaders['x-amz-request-id'] = log.getSerializedUids(); return responseMetaHeaders; } +/** + * Builds a headless XML string wrapped in the standard SOSAPI XML declaration. + * + * @param {object} obj - JS object to serialize to XML + * @returns {string} formatted XML file content + */ +function buildXML(obj) { + const builder = new xml2js.Builder({ headless: true }); + return buildHeadXML(builder.buildObject(obj)); +} + /** * Generic function to respond to user with data using streams * @@ -136,18 +147,26 @@ async function respondWithData(request, response, log, bucket, data, lastModifie } response.writeHead(200); - // Use a single-element array so the Buffer is sent as one chunk rather - // than being iterated byte-by-byte by Readable.from. - await pipeline(Readable.from([dataBuffer]), response); let contentLength = 0; if (responseMetaHeaders && responseMetaHeaders['Content-Length']) { contentLength = responseMetaHeaders['Content-Length']; } log.end().addDefaultFields({ contentLength }); - log.end().info('responded with streamed content', { - httpCode: response.statusCode, - }); + + try { + // Use a single-element array so the Buffer is sent as one chunk rather + // than being iterated byte-by-byte by Readable.from. + await pipeline(Readable.from([dataBuffer]), response); + log.end().info('responded with streamed content', { + httpCode: response.statusCode, + }); + } catch (err) { + log.end().error('error streaming response', { + httpCode: response.statusCode, + error: err.message, + }); + } } const validPath = '.system-d26a9498-cb7c-4a87-a44a-8ae204f5ba6c/'; @@ -278,8 +297,7 @@ async function buildVeeamFileData(request, bucketMd, log) { // ensuring no issue in a SOSAPI context. We should use this information. } - const builder = new xml2js.Builder({ headless: true }); - const xmlContent = buildHeadXML(builder.buildObject(fileToBuild.value)); + const xmlContent = buildXML(fileToBuild.value); const dataBuffer = Buffer.from(xmlContent); return { xmlContent, dataBuffer, modified, bucketData: data }; } @@ -290,9 +308,11 @@ module.exports = { respondWithData, getResponseHeader, buildHeadXML, + buildXML, validPath, isSystemXML, getFileToBuild, fetchCapacityMetrics, buildVeeamFileData, + getBucket, }; From dbedc1fe0ee1c4b42e88cfb93997578ed1b656f4 Mon Sep 17 00:00:00 2001 From: DarkIsDude Date: Thu, 2 Apr 2026 16:37:30 +0200 Subject: [PATCH 12/12] =?UTF-8?q?=F0=9F=94=96=20Release=209.1.14?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue: CLDSRV-878 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index f1c5b35940..33561b70b2 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@zenko/cloudserver", - "version": "9.1.13", + "version": "9.1.14", "description": "Zenko CloudServer, an open-source Node.js implementation of a server handling the Amazon S3 protocol", "main": "index.js", "engines": {