From 732fbf47ca0043d38535afc3c45b191d73b474c6 Mon Sep 17 00:00:00 2001 From: kanishkkatara Date: Thu, 23 May 2024 15:33:24 +0530 Subject: [PATCH 01/20] chore: added credentials to javascript transformations --- src/types/index.ts | 7 +++++++ src/util/customTransformer-v1.js | 2 ++ src/util/customTransformer.js | 2 ++ src/util/customTransformerFactory.js | 10 ++++++++-- src/util/ivmFactory.js | 19 ++++++++++++++++--- 5 files changed, 35 insertions(+), 5 deletions(-) diff --git a/src/types/index.ts b/src/types/index.ts index 68dfe3870d..25b9beae01 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -137,12 +137,19 @@ type UserTransformationLibrary = { VersionID: string; }; +type Credential = { + Key: string; + Value: string; + isPublic: boolean; +}; + type ProcessorTransformationRequest = { request?: object; message: object; metadata: Metadata; destination: Destination; libraries?: UserTransformationLibrary[]; + credentials?: Credential[]; }; type RouterTransformationRequestData = { diff --git a/src/util/customTransformer-v1.js b/src/util/customTransformer-v1.js index 7e854a3714..1b3e270f52 100644 --- a/src/util/customTransformer-v1.js +++ b/src/util/customTransformer-v1.js @@ -51,6 +51,7 @@ function calculateMsFromIvmTime(value) { async function userTransformHandlerV1( events, userTransformation, + credentials, libraryVersionIds, testMode = false, ) { @@ -62,6 +63,7 @@ async function userTransformHandlerV1( userTransformation.code, libraryVersionIds, userTransformation.versionId, + credentials, userTransformation.secrets || {}, testMode, ); diff --git a/src/util/customTransformer.js b/src/util/customTransformer.js index a87c12dd6e..8094119ef9 100644 --- a/src/util/customTransformer.js +++ b/src/util/customTransformer.js @@ -285,12 +285,14 @@ async function userTransformHandler( events.forEach((ev) => { eventsMetadata[ev.message.messageId] = ev.metadata; }); + const credentials = events[0].credentials; let userTransformedEvents = []; let result; if (res.codeVersion && res.codeVersion === '1') { result = await UserTransformHandlerFactory(res).runUserTransfrom( events, + credentials, testMode, libraryVersionIDs, ); diff --git a/src/util/customTransformerFactory.js b/src/util/customTransformerFactory.js index ee53531946..f6bbbc492c 100644 --- a/src/util/customTransformerFactory.js +++ b/src/util/customTransformerFactory.js @@ -13,14 +13,20 @@ const UserTransformHandlerFactory = (userTransformation) => { } }, - runUserTransfrom: async (events, testMode, libraryVersionIds) => { + runUserTransfrom: async (events, credentials, testMode, libraryVersionIds) => { switch (userTransformation.language) { case 'pythonfaas': case 'python': return runOpenFaasUserTransform(events, userTransformation, libraryVersionIds, testMode); default: - return userTransformHandlerV1(events, userTransformation, libraryVersionIds, testMode); + return userTransformHandlerV1( + events, + userTransformation, + credentials, + libraryVersionIds, + testMode, + ); } }, }; diff --git a/src/util/ivmFactory.js b/src/util/ivmFactory.js index 9a6419295d..62da76295d 100644 --- a/src/util/ivmFactory.js +++ b/src/util/ivmFactory.js @@ -30,7 +30,7 @@ async function loadModule(isolateInternal, contextInternal, moduleName, moduleCo return module; } -async function createIvm(code, libraryVersionIds, versionId, secrets, testMode) { +async function createIvm(code, libraryVersionIds, versionId, credentials, secrets, testMode) { const createIvmStartTime = new Date(); const logs = []; const libraries = await Promise.all( @@ -243,6 +243,11 @@ async function createIvm(code, libraryVersionIds, versionId, secrets, testMode) }), ); + await jail.set('_credentials', function (...args) { + if (args.length == 0 || !credentials || !credentials[args[0]]) return 'ERROR'; + return credentials[args[0]]; + }); + await jail.set('_rsSecrets', function (...args) { if (args.length == 0 || !secrets || !secrets[args[0]]) return 'ERROR'; return secrets[args[0]]; @@ -321,6 +326,14 @@ async function createIvm(code, libraryVersionIds, versionId, secrets, testMode) ]); }; + let credentials = _credentials; + delete _credentials; + global.credentials = function(...args) { + return credentials([ + ...args.map(arg => new ivm.ExternalCopy(arg).copyInto()) + ]); + }; + return new ivm.Reference(function forwardMainPromise( fnRef, resolve, @@ -411,10 +424,10 @@ async function compileUserLibrary(code) { return evaluateModule(isolate, context, code); } -async function getFactory(code, libraryVersionIds, versionId, secrets, testMode) { +async function getFactory(code, libraryVersionIds, versionId, credentials, secrets, testMode) { const factory = { create: async () => { - return createIvm(code, libraryVersionIds, versionId, secrets, testMode); + return createIvm(code, libraryVersionIds, versionId, credentials, secrets, testMode); }, destroy: async (client) => { client.fnRef.release(); From e6a3dbb711bfb6567baa43f42dc33b6be6db62cd Mon Sep 17 00:00:00 2001 From: kanishkkatara Date: Thu, 23 May 2024 18:53:23 +0530 Subject: [PATCH 02/20] chore: added tests --- ...user_transformation_input_credentials.json | 57 +++++++++++++++++++ test/__tests__/user_transformation.test.js | 28 +++++++++ 2 files changed, 85 insertions(+) create mode 100644 test/__tests__/data/user_transformation_input_credentials.json diff --git a/test/__tests__/data/user_transformation_input_credentials.json b/test/__tests__/data/user_transformation_input_credentials.json new file mode 100644 index 0000000000..370938a909 --- /dev/null +++ b/test/__tests__/data/user_transformation_input_credentials.json @@ -0,0 +1,57 @@ +[ + { + "message": { + "channel": "web", + "context": { + "app": { + "build": "1.0.0", + "name": "RudderLabs JavaScript SDK", + "namespace": "com.rudderlabs.javascript", + "version": "1.0.0" + }, + "traits": { + "email": "test@rudderstack.com", + "anonymousId": "12345" + }, + "library": { + "name": "RudderLabs JavaScript SDK", + "version": "1.0.0" + }, + "userAgent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/77.0.3865.90 Safari/537.36", + "locale": "en-US", + "ip": "0.0.0.0", + "os": { + "name": "", + "version": "" + }, + "screen": { + "density": 2 + } + }, + "type": "track", + "messageId": "ec5481b6-a926-4d2e-b293-0b3a77c4d3be", + "originalTimestamp": "2019-10-14T11:15:18.300Z", + "anonymousId": "00000000000000000000000000", + "userId": "12345", + "event": "test track event GA3", + "properties": { + "user_actual_role": "system_admin, system_user", + "user_actual_id": 12345 + }, + "integrations": { + "All": true + }, + "sentAt": "2019-10-14T11:15:53.296Z" + }, + "destination": { + "Config": { + "trackingID": "UA-149602794-1" + }, + "Enabled": true + }, + "credentials": { + "key1": "value1", + "key2": "value2" + } + } +] diff --git a/test/__tests__/user_transformation.test.js b/test/__tests__/user_transformation.test.js index 924bf4f791..de6a587d1e 100644 --- a/test/__tests__/user_transformation.test.js +++ b/test/__tests__/user_transformation.test.js @@ -243,6 +243,34 @@ describe("User transformation", () => { expect(output[0].transformedEvent.dummy_key).toEqual(secrets.dummy_key); }); + it(`Simple ${name} Test with credentials for codeVersion 1`, async () => { + const versionId = randomID(); + + const inputData = require(`./data/${integration}_input_credentials.json`); + + const respBody = { + versionId: versionId, + codeVersion: "1", + name, + code: ` + export function transformEvent(event, metadata) { + event.credentialValue = credentials("key1"); + return event; + } + ` + }; + fetch.mockResolvedValue({ + status: 200, + json: jest.fn().mockResolvedValue(respBody) + }); + + const output = await userTransformHandler(inputData, versionId, []); + expect(fetch).toHaveBeenCalledWith( + `https://api.rudderlabs.com/transformation/getByVersionId?versionId=${versionId}` + ); + expect(output[0].transformedEvent.credentialValue).toEqual("value1"); + }); + it(`Simple async ${name} FetchV2 Test for V0 transformation`, async () => { const versionId = randomID(); const inputData = require(`./data/${integration}_input.json`); From a7e4ba4f5826e835560b31d22e20447d0696d7bd Mon Sep 17 00:00:00 2001 From: kanishkkatara Date: Fri, 24 May 2024 11:48:05 +0530 Subject: [PATCH 03/20] chore: change in credentials structure --- src/util/customTransformer.js | 6 ++++-- .../user_transformation_input_credentials.json | 16 ++++++++++++---- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/util/customTransformer.js b/src/util/customTransformer.js index 8094119ef9..2573b82e6a 100644 --- a/src/util/customTransformer.js +++ b/src/util/customTransformer.js @@ -285,8 +285,10 @@ async function userTransformHandler( events.forEach((ev) => { eventsMetadata[ev.message.messageId] = ev.metadata; }); - const credentials = events[0].credentials; - + const credentials = {}; + events[0]?.credentials?.forEach((cred) => { + credentials[cred.key] = cred.value; + }); let userTransformedEvents = []; let result; if (res.codeVersion && res.codeVersion === '1') { diff --git a/test/__tests__/data/user_transformation_input_credentials.json b/test/__tests__/data/user_transformation_input_credentials.json index 370938a909..49b224e560 100644 --- a/test/__tests__/data/user_transformation_input_credentials.json +++ b/test/__tests__/data/user_transformation_input_credentials.json @@ -49,9 +49,17 @@ }, "Enabled": true }, - "credentials": { - "key1": "value1", - "key2": "value2" - } + "credentials": [ + { + "key": "key1", + "value": "value1", + "isPublic": true + }, + { + "key": "key2", + "value": "value2", + "isPublic": true + } + ] } ] From 5111ce9dee2ca4362ea398d4d9805bab7070c8f7 Mon Sep 17 00:00:00 2001 From: kanishkkatara Date: Fri, 31 May 2024 01:07:57 +0530 Subject: [PATCH 04/20] chore: changes in test transform API --- src/controllers/userTransform.ts | 3 ++- src/services/userTransform.ts | 3 ++- src/util/customTransformer.js | 17 +++++++++---- test/__tests__/user_transformation.test.js | 5 ++++ .../user_transformation_errors.test.js | 8 +++++++ .../user_transformation_fetch.test.js | 24 +++++++++---------- 6 files changed, 41 insertions(+), 19 deletions(-) diff --git a/src/controllers/userTransform.ts b/src/controllers/userTransform.ts index 0e288c6f04..33e1507832 100644 --- a/src/controllers/userTransform.ts +++ b/src/controllers/userTransform.ts @@ -33,11 +33,12 @@ export class UserTransformController { '(User transform - router:/transformation/test ):: Request to transformer', ctx.request.body, ); - const { events, trRevCode, libraryVersionIDs = [] } = ctx.request.body as any; + const { events, trRevCode, libraryVersionIDs = [], credentials } = ctx.request.body as any; const response = await UserTransformService.testTransformRoutine( events, trRevCode, libraryVersionIDs, + credentials, ); ctx.body = response.body; ControllerUtility.postProcess(ctx, response.status); diff --git a/src/services/userTransform.ts b/src/services/userTransform.ts index 18c47ddc83..a587973deb 100644 --- a/src/services/userTransform.ts +++ b/src/services/userTransform.ts @@ -192,7 +192,7 @@ export class UserTransformService { } as UserTransformationServiceResponse; } - public static async testTransformRoutine(events, trRevCode, libraryVersionIDs) { + public static async testTransformRoutine(events, trRevCode, libraryVersionIDs, credentials) { const response: FixMe = {}; try { if (!trRevCode || !trRevCode.code || !trRevCode.codeVersion) { @@ -210,6 +210,7 @@ export class UserTransformService { trRevCode.versionId, libraryVersionIDs, trRevCode, + credentials, true, ); logger.debug(`[CT] Test Output Events: ${JSON.stringify(response.body.transformedEvents)}`); diff --git a/src/util/customTransformer.js b/src/util/customTransformer.js index 2573b82e6a..c0082e2c63 100644 --- a/src/util/customTransformer.js +++ b/src/util/customTransformer.js @@ -273,6 +273,7 @@ async function userTransformHandler( versionId, libraryVersionIDs, trRevCode = {}, + credentials = {}, testMode = false, ) { if (versionId) { @@ -285,16 +286,22 @@ async function userTransformHandler( events.forEach((ev) => { eventsMetadata[ev.message.messageId] = ev.metadata; }); - const credentials = {}; - events[0]?.credentials?.forEach((cred) => { - credentials[cred.key] = cred.value; - }); + const credentialsMap = {}; + if (testMode === false) { + events[0]?.credentials?.forEach((cred) => { + credentialsMap[cred.key] = cred.value; + }); + } else { + credentials?.forEach((cred) => { + credentialsMap[cred.key] = cred.value; + }); + } let userTransformedEvents = []; let result; if (res.codeVersion && res.codeVersion === '1') { result = await UserTransformHandlerFactory(res).runUserTransfrom( events, - credentials, + credentialsMap, testMode, libraryVersionIDs, ); diff --git a/test/__tests__/user_transformation.test.js b/test/__tests__/user_transformation.test.js index de6a587d1e..5706f926d2 100644 --- a/test/__tests__/user_transformation.test.js +++ b/test/__tests__/user_transformation.test.js @@ -1081,6 +1081,7 @@ describe("User transformation", () => { trRevCode.versionId, [libraryVersionId], trRevCode, + null, true ); @@ -1117,6 +1118,7 @@ describe("User transformation", () => { trRevCode.versionId, [], trRevCode, + null, true ); expect(output).toEqual(expectedData); @@ -1536,6 +1538,7 @@ describe("Python transformations", () => { trRevCode.versionId, [], trRevCode, + null, true ); expect(output).toEqual(outputData); @@ -1581,6 +1584,7 @@ describe("Python transformations", () => { trRevCode.versionId, Object.values(importNameLibraryVersionIdsMap), trRevCode, + null, true ); expect(output).toEqual(outputData); @@ -1622,6 +1626,7 @@ describe("Python transformations", () => { trRevCode.versionId, Object.values(importNameLibraryVersionIdsMap), trRevCode, + null, true ); expect(output).toEqual(outputData); diff --git a/test/__tests__/user_transformation_errors.test.js b/test/__tests__/user_transformation_errors.test.js index c2a99ce09d..eb171a1e4d 100644 --- a/test/__tests__/user_transformation_errors.test.js +++ b/test/__tests__/user_transformation_errors.test.js @@ -47,6 +47,7 @@ describe("JS Transformation Error Tests", () => { versionId, [], trRevCode, + null, true, ); @@ -65,6 +66,7 @@ describe("JS Transformation Error Tests", () => { versionId, [], trRevCode, + null, true, ); @@ -102,6 +104,7 @@ describe("JS Transformation Error Tests", () => { versionId, [libVid], trRevCode, + null, true, ); @@ -140,6 +143,7 @@ describe("JS Transformation Error Tests", () => { versionId, [libVid], trRevCode, + null, true, ); @@ -160,6 +164,7 @@ describe("JS Transformation Error Tests", () => { versionId, [], trRevCode, + null, true, ); @@ -181,6 +186,7 @@ describe("JS Transformation Error Tests", () => { versionId, [], trRevCode, + null, true, ); @@ -218,6 +224,7 @@ describe("JS Transformation Error Tests", () => { versionId, [libVid], trRevCode, + null, true, ); @@ -256,6 +263,7 @@ describe("JS Transformation Error Tests", () => { versionId, [libVid], trRevCode, + null, true, ); diff --git a/test/__tests__/user_transformation_fetch.test.js b/test/__tests__/user_transformation_fetch.test.js index e37b42ea1e..edbac04cf3 100644 --- a/test/__tests__/user_transformation_fetch.test.js +++ b/test/__tests__/user_transformation_fetch.test.js @@ -43,7 +43,7 @@ describe("User transformation fetch tests", () => { ` }; - const output = await userTransformHandler(inputData, versionId, [], trRevCode, true); + const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true); expect(output).toEqual({ logs: [], transformedEvents: expectedData.map(ev => (ev.transformedEvent)) @@ -67,7 +67,7 @@ describe("User transformation fetch tests", () => { }; const errMsg = "ERROR"; - const output = await userTransformHandler(inputData, versionId, [], trRevCode, true); + const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true); expect(mockResolver).toHaveBeenCalledTimes(0); output.transformedEvents.forEach(ev => { @@ -92,7 +92,7 @@ describe("User transformation fetch tests", () => { const errMsg = "ERROR"; mockResolver.mockResolvedValue([ '127.0.0.1' ]); - const output = await userTransformHandler(inputData, versionId, [], trRevCode, true); + const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true); expect(mockResolver).toHaveBeenCalledTimes(inputData.length); expect(mockResolver).toHaveBeenCalledWith('abc.xyz.com'); @@ -122,7 +122,7 @@ describe("User transformation fetch tests", () => { }; const errMsg = "invalid url, localhost requests are not allowed"; - const output = await userTransformHandler(inputData, versionId, [], trRevCode, true); + const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true); expect(mockResolver).toHaveBeenCalledTimes(0); output.transformedEvents.forEach(ev => { @@ -152,7 +152,7 @@ describe("User transformation fetch tests", () => { const errMsg = "request to https://abc.xyz.com/dummyUrl failed, reason: Invalid IP address: unable to resolve IP address for abc.xyz.com"; mockResolver.mockRejectedValue('invalid host'); - const output = await userTransformHandler(inputData, versionId, [], trRevCode, true); + const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true); expect(mockResolver).toHaveBeenCalledTimes(inputData.length); expect(mockResolver).toHaveBeenCalledWith('abc.xyz.com'); @@ -182,7 +182,7 @@ describe("User transformation fetch tests", () => { }; const errMsg = "invalid protocol, only http and https are supported"; - const output = await userTransformHandler(inputData, versionId, [], trRevCode, true); + const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true); output.transformedEvents.forEach(ev => { expect(ev.errMsg).toEqual(errMsg); @@ -204,7 +204,7 @@ describe("User transformation fetch tests", () => { versionId, }; - const output = await userTransformHandler(inputData, versionId, [], trRevCode, true); + const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true); expect(output).toEqual({ logs: [], transformedEvents: expectedData.map(ev => (ev.transformedEvent)) @@ -226,7 +226,7 @@ describe("User transformation fetch tests", () => { }; const errMsg = "ERROR"; - const output = await userTransformHandler(inputData, versionId, [], trRevCode, true); + const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true); expect(mockResolver).toHaveBeenCalledTimes(0); output.transformedEvents.forEach(ev => { @@ -250,7 +250,7 @@ describe("User transformation fetch tests", () => { const errMsg = "ERROR"; mockResolver.mockRejectedValue('invalid host'); - const output = await userTransformHandler(inputData, versionId, [], trRevCode, true); + const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true); expect(mockResolver).toHaveBeenCalledTimes(inputData.length); expect(mockResolver).toHaveBeenCalledWith('abc.xyz.com'); @@ -279,7 +279,7 @@ describe("User transformation fetch tests", () => { }; const errMsg = "invalid url, localhost requests are not allowed"; - const output = await userTransformHandler(inputData, versionId, [], trRevCode, true); + const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true); expect(mockResolver).toHaveBeenCalledTimes(0); output.transformedEvents.forEach(ev => { @@ -308,7 +308,7 @@ describe("User transformation fetch tests", () => { const errMsg = "request to https://abc.xyz.com/dummyUrl failed, reason: Invalid IP address: cannot use 127.0.0.1 as IP address"; mockResolver.mockResolvedValue(['3.122.122.122', '127.0.0.1']); - const output = await userTransformHandler(inputData, versionId, [], trRevCode, true); + const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true); expect(mockResolver).toHaveBeenCalledTimes(inputData.length); expect(mockResolver).toHaveBeenCalledWith('abc.xyz.com'); @@ -337,7 +337,7 @@ describe("User transformation fetch tests", () => { }; const errMsg = "fetch url is required"; - const output = await userTransformHandler(inputData, versionId, [], trRevCode, true); + const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true); output.transformedEvents.forEach(ev => { expect(ev.errMsg).toEqual(errMsg); From c280d81307a3c5da02f728961d7eeeabb79a7e39 Mon Sep 17 00:00:00 2001 From: kanishkkatara Date: Fri, 31 May 2024 01:53:25 +0530 Subject: [PATCH 05/20] fix: credential param --- src/util/customTransformer.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/customTransformer.js b/src/util/customTransformer.js index c0082e2c63..9b60cc163f 100644 --- a/src/util/customTransformer.js +++ b/src/util/customTransformer.js @@ -273,7 +273,7 @@ async function userTransformHandler( versionId, libraryVersionIDs, trRevCode = {}, - credentials = {}, + credentials = [], testMode = false, ) { if (versionId) { From 31379645c33b9dc75cd839849952297fa233f2cb Mon Sep 17 00:00:00 2001 From: kanishkkatara Date: Fri, 31 May 2024 16:07:19 +0530 Subject: [PATCH 06/20] fix: test --- src/legacy/router.js | 1 + test/__tests__/user_transformation.integration.test.js | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/legacy/router.js b/src/legacy/router.js index 9dd83b5988..90b6b05693 100644 --- a/src/legacy/router.js +++ b/src/legacy/router.js @@ -675,6 +675,7 @@ if (transformerTestModeEnabled) { trRevCode.versionId, libraryVersionIDs, trRevCode, + null, true, ); logger.debug(`[CT] Test Output Events: ${JSON.stringify(res.transformedEvents)}`); diff --git a/test/__tests__/user_transformation.integration.test.js b/test/__tests__/user_transformation.integration.test.js index b0b8fc9cae..1e2fafeb73 100644 --- a/test/__tests__/user_transformation.integration.test.js +++ b/test/__tests__/user_transformation.integration.test.js @@ -159,13 +159,14 @@ describe("Function invocation & creation tests", () => { versionId, [], trRevCode, + null, true ); expect(response).toEqual(outputEvents); // Test with language python; should return same output trRevCode = contructTrRevCode(versionId, 'python'); - response = await userTransformHandler(inputEvents, versionId, [], trRevCode, true); + response = await userTransformHandler(inputEvents, versionId, [], trRevCode, null, true); expect(response).toEqual(outputEvents); }); @@ -184,7 +185,7 @@ describe("Function invocation & creation tests", () => { }); await expect(async () => { - await userTransformHandler(inputEvents, respBody.versionId, []); + await userTransformHandler(inputEvents, respBody.versionId, null, []); }).rejects.toThrow(RetryRequestError); // If function is not found, it will be created From 7eb537c91f40ee4802f14a088a249ebed903e7b5 Mon Sep 17 00:00:00 2001 From: kanishkkatara Date: Mon, 3 Jun 2024 14:21:02 +0530 Subject: [PATCH 07/20] fix: credential error handling --- src/types/index.ts | 3 ++- src/util/ivmFactory.js | 7 ++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/types/index.ts b/src/types/index.ts index 25b9beae01..93eb9bb6ba 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -138,9 +138,10 @@ type UserTransformationLibrary = { }; type Credential = { + ID: string; Key: string; Value: string; - isPublic: boolean; + IsSecret: boolean; }; type ProcessorTransformationRequest = { diff --git a/src/util/ivmFactory.js b/src/util/ivmFactory.js index 62da76295d..e2692697a7 100644 --- a/src/util/ivmFactory.js +++ b/src/util/ivmFactory.js @@ -244,7 +244,12 @@ async function createIvm(code, libraryVersionIds, versionId, credentials, secret ); await jail.set('_credentials', function (...args) { - if (args.length == 0 || !credentials || !credentials[args[0]]) return 'ERROR'; + if (args[0].length === 0) { + throw new Error('Invalid credentials key'); + } + if (!credentials) { + throw new Error('Credentials not available'); + } return credentials[args[0]]; }); From a57da897c0b1206415954134cf0f855c7ec6c27f Mon Sep 17 00:00:00 2001 From: kanishkkatara Date: Mon, 3 Jun 2024 14:39:23 +0530 Subject: [PATCH 08/20] chore: added tests --- test/__tests__/user_transformation.test.js | 58 ++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/test/__tests__/user_transformation.test.js b/test/__tests__/user_transformation.test.js index 5706f926d2..9c9fc55f35 100644 --- a/test/__tests__/user_transformation.test.js +++ b/test/__tests__/user_transformation.test.js @@ -271,6 +271,64 @@ describe("User transformation", () => { expect(output[0].transformedEvent.credentialValue).toEqual("value1"); }); + it(`Simple ${name} Test with credentials without arguements for codeVersion 1`, async () => { + const versionId = randomID(); + + const inputData = require(`./data/${integration}_input_credentials.json`); + + const respBody = { + versionId: versionId, + codeVersion: "1", + name, + code: ` + export function transformEvent(event, metadata) { + event.credentialValue = credentials(); + return event; + } + ` + }; + fetch.mockResolvedValue({ + status: 200, + json: jest.fn().mockResolvedValue(respBody) + }); + + const output = await userTransformHandler(inputData, versionId, []); + + expect(fetch).toHaveBeenCalledWith( + `https://api.rudderlabs.com/transformation/getByVersionId?versionId=${versionId}` + ); + expect(output[0].error).toMatch(/Invalid credentials key/); + }); + + it(`Simple ${name} Test with credentials without value for codeVersion 1`, async () => { + const versionId = randomID(); + + const inputData = require(`./data/${integration}_input_credentials.json`); + + const respBody = { + versionId: versionId, + codeVersion: "1", + name, + code: ` + export function transformEvent(event, metadata) { + event.credentialValue = credentials('key3'); + return event; + } + ` + }; + fetch.mockResolvedValue({ + status: 200, + json: jest.fn().mockResolvedValue(respBody) + }); + + const output = await userTransformHandler(inputData, versionId, []); + + expect(fetch).toHaveBeenCalledWith( + `https://api.rudderlabs.com/transformation/getByVersionId?versionId=${versionId}` + ); + expect(output[0].transformedEvent.credentialValue).toEqual(undefined); + }); + it(`Simple async ${name} FetchV2 Test for V0 transformation`, async () => { const versionId = randomID(); const inputData = require(`./data/${integration}_input.json`); From 1fd068b8e6e4ccda40c3e6ee65add5a5785367d2 Mon Sep 17 00:00:00 2001 From: kanishkkatara Date: Mon, 3 Jun 2024 20:53:11 +0530 Subject: [PATCH 09/20] chore: added errors in credentials --- src/util/ivmFactory.js | 20 +- test/__tests__/user_transformation.test.js | 232 +++++++++++++-------- 2 files changed, 156 insertions(+), 96 deletions(-) diff --git a/src/util/ivmFactory.js b/src/util/ivmFactory.js index e2692697a7..9e1207f844 100644 --- a/src/util/ivmFactory.js +++ b/src/util/ivmFactory.js @@ -243,14 +243,14 @@ async function createIvm(code, libraryVersionIds, versionId, credentials, secret }), ); - await jail.set('_credentials', function (...args) { - if (args[0].length === 0) { - throw new Error('Invalid credentials key'); + await jail.set('_credential', function (key) { + if (_.isNil(credentials) || !_.isObject(credentials)) { + throw new Error('Credentials in incorrect format'); } - if (!credentials) { - throw new Error('Credentials not available'); + if (_.isNil(key[0]) || !_.isString(key[0])) { + throw new Error('Key should be a string'); } - return credentials[args[0]]; + return credentials[key]; }); await jail.set('_rsSecrets', function (...args) { @@ -331,10 +331,10 @@ async function createIvm(code, libraryVersionIds, versionId, credentials, secret ]); }; - let credentials = _credentials; - delete _credentials; - global.credentials = function(...args) { - return credentials([ + let credential = _credential; + delete _credential; + global.credential = function(...args) { + return credential([ ...args.map(arg => new ivm.ExternalCopy(arg).copyInto()) ]); }; diff --git a/test/__tests__/user_transformation.test.js b/test/__tests__/user_transformation.test.js index 9c9fc55f35..893b60e799 100644 --- a/test/__tests__/user_transformation.test.js +++ b/test/__tests__/user_transformation.test.js @@ -243,92 +243,6 @@ describe("User transformation", () => { expect(output[0].transformedEvent.dummy_key).toEqual(secrets.dummy_key); }); - it(`Simple ${name} Test with credentials for codeVersion 1`, async () => { - const versionId = randomID(); - - const inputData = require(`./data/${integration}_input_credentials.json`); - - const respBody = { - versionId: versionId, - codeVersion: "1", - name, - code: ` - export function transformEvent(event, metadata) { - event.credentialValue = credentials("key1"); - return event; - } - ` - }; - fetch.mockResolvedValue({ - status: 200, - json: jest.fn().mockResolvedValue(respBody) - }); - - const output = await userTransformHandler(inputData, versionId, []); - expect(fetch).toHaveBeenCalledWith( - `https://api.rudderlabs.com/transformation/getByVersionId?versionId=${versionId}` - ); - expect(output[0].transformedEvent.credentialValue).toEqual("value1"); - }); - - it(`Simple ${name} Test with credentials without arguements for codeVersion 1`, async () => { - const versionId = randomID(); - - const inputData = require(`./data/${integration}_input_credentials.json`); - - const respBody = { - versionId: versionId, - codeVersion: "1", - name, - code: ` - export function transformEvent(event, metadata) { - event.credentialValue = credentials(); - return event; - } - ` - }; - fetch.mockResolvedValue({ - status: 200, - json: jest.fn().mockResolvedValue(respBody) - }); - - const output = await userTransformHandler(inputData, versionId, []); - - expect(fetch).toHaveBeenCalledWith( - `https://api.rudderlabs.com/transformation/getByVersionId?versionId=${versionId}` - ); - expect(output[0].error).toMatch(/Invalid credentials key/); - }); - - it(`Simple ${name} Test with credentials without value for codeVersion 1`, async () => { - const versionId = randomID(); - - const inputData = require(`./data/${integration}_input_credentials.json`); - - const respBody = { - versionId: versionId, - codeVersion: "1", - name, - code: ` - export function transformEvent(event, metadata) { - event.credentialValue = credentials('key3'); - return event; - } - ` - }; - fetch.mockResolvedValue({ - status: 200, - json: jest.fn().mockResolvedValue(respBody) - }); - - const output = await userTransformHandler(inputData, versionId, []); - - expect(fetch).toHaveBeenCalledWith( - `https://api.rudderlabs.com/transformation/getByVersionId?versionId=${versionId}` - ); - expect(output[0].transformedEvent.credentialValue).toEqual(undefined); - }); - it(`Simple async ${name} FetchV2 Test for V0 transformation`, async () => { const versionId = randomID(); const inputData = require(`./data/${integration}_input.json`); @@ -1181,6 +1095,152 @@ describe("User transformation", () => { ); expect(output).toEqual(expectedData); }); + + describe("Credentials", () => { + it(`Simple ${name} Test with credentials for codeVersion 1`, async () => { + const versionId = randomID(); + + const inputData = require(`./data/${integration}_input_credentials.json`); + + const respBody = { + versionId: versionId, + codeVersion: "1", + name, + code: ` + export function transformEvent(event, metadata) { + event.credentialValue = credential("key1"); + return event; + } + ` + }; + fetch.mockResolvedValue({ + status: 200, + json: jest.fn().mockResolvedValue(respBody) + }); + + const output = await userTransformHandler(inputData, versionId, []); + expect(fetch).toHaveBeenCalledWith( + `https://api.rudderlabs.com/transformation/getByVersionId?versionId=${versionId}` + ); + expect(output[0].transformedEvent.credentialValue).toEqual("value1"); + }); + + it(`Simple ${name} Test with credentials without key for codeVersion 1`, async () => { + const versionId = randomID(); + + const inputData = require(`./data/${integration}_input_credentials.json`); + + const respBody = { + versionId: versionId, + codeVersion: "1", + name, + code: ` + export function transformEvent(event, metadata) { + event.credentialValue = credential(); + return event; + } + ` + }; + fetch.mockResolvedValue({ + status: 200, + json: jest.fn().mockResolvedValue(respBody) + }); + + const output = await userTransformHandler(inputData, versionId, []); + + expect(fetch).toHaveBeenCalledWith( + `https://api.rudderlabs.com/transformation/getByVersionId?versionId=${versionId}` + ); + expect(output[0].error).toMatch(/Key should be a string/); + }); + + it(`Simple ${name} Test with credentials with non string key for codeVersion 1`, async () => { + const versionId = randomID(); + + const inputData = require(`./data/${integration}_input_credentials.json`); + + const respBody = { + versionId: versionId, + codeVersion: "1", + name, + code: ` + export function transformEvent(event, metadata) { + event.credentialValue = credential(1); + return event; + } + ` + }; + fetch.mockResolvedValue({ + status: 200, + json: jest.fn().mockResolvedValue(respBody) + }); + + const output = await userTransformHandler(inputData, versionId, []); + + expect(fetch).toHaveBeenCalledWith( + `https://api.rudderlabs.com/transformation/getByVersionId?versionId=${versionId}` + ); + expect(output[0].error).toMatch(/Key should be a string/); + }); + + it(`Simple ${name} Test with credentials without value for codeVersion 1`, async () => { + const versionId = randomID(); + + const inputData = require(`./data/${integration}_input_credentials.json`); + + const respBody = { + versionId: versionId, + codeVersion: "1", + name, + code: ` + export function transformEvent(event, metadata) { + event.credentialValue = credential('key3'); + return event; + } + ` + }; + fetch.mockResolvedValue({ + status: 200, + json: jest.fn().mockResolvedValue(respBody) + }); + + const output = await userTransformHandler(inputData, versionId, []); + + expect(fetch).toHaveBeenCalledWith( + `https://api.rudderlabs.com/transformation/getByVersionId?versionId=${versionId}` + ); + expect(output[0].transformedEvent.credentialValue).toEqual(undefined); + }); + + it(`Simple ${name} Test without credentials for codeVersion 1`, async () => { + const versionId = randomID(); + + const inputData = require(`./data/${integration}_input.json`); + + const respBody = { + versionId: versionId, + codeVersion: "1", + name, + code: ` + export function transformEvent(event, metadata) { + event.credentialValue = credential('key1'); + return event; + } + ` + }; + fetch.mockResolvedValue({ + status: 200, + json: jest.fn().mockResolvedValue(respBody) + }); + + const output = await userTransformHandler(inputData, versionId, []); + + expect(fetch).toHaveBeenCalledWith( + `https://api.rudderlabs.com/transformation/getByVersionId?versionId=${versionId}` + ); + expect(output[0].transformedEvent.credentialValue).toEqual(undefined); + }); + }); }); // Running timeout tests From 8808508d74ce6f5762bd95792f37b9c9eb068b55 Mon Sep 17 00:00:00 2001 From: kanishkkatara Date: Tue, 4 Jun 2024 19:08:38 +0530 Subject: [PATCH 10/20] chore: added credentials at event level for python transformations --- src/util/customTransformer.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/util/customTransformer.js b/src/util/customTransformer.js index 9b60cc163f..b3d7214a51 100644 --- a/src/util/customTransformer.js +++ b/src/util/customTransformer.js @@ -8,6 +8,7 @@ const { parserForImport } = require('./parser'); const stats = require('./stats'); const { fetchWithDnsWrapper } = require('./utils'); const { getMetadata, getTransformationMetadata } = require('../v0/util'); +const _ = require('lodash'); const ISOLATE_VM_MEMORY = parseInt(process.env.ISOLATE_VM_MEMORY || '128', 10); const GEOLOCATION_TIMEOUT_IN_MS = parseInt(process.env.GEOLOCATION_TIMEOUT_IN_MS || '1000', 10); @@ -295,6 +296,11 @@ async function userTransformHandler( credentials?.forEach((cred) => { credentialsMap[cred.key] = cred.value; }); + events.forEach((ev) => { + if (_.isNil(ev?.credentials)) { + ev.credentials = credentials; + } + }); } let userTransformedEvents = []; let result; From 546a9379eba6d44e5ed2ed4451e14229dbce82fd Mon Sep 17 00:00:00 2001 From: kanishkkatara Date: Wed, 12 Jun 2024 10:57:35 +0530 Subject: [PATCH 11/20] chore: added check for multiple arguments --- src/util/ivmFactory.js | 2 +- test/__tests__/user_transformation.test.js | 29 ++++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/src/util/ivmFactory.js b/src/util/ivmFactory.js index 9e1207f844..50bf4caa49 100644 --- a/src/util/ivmFactory.js +++ b/src/util/ivmFactory.js @@ -247,7 +247,7 @@ async function createIvm(code, libraryVersionIds, versionId, credentials, secret if (_.isNil(credentials) || !_.isObject(credentials)) { throw new Error('Credentials in incorrect format'); } - if (_.isNil(key[0]) || !_.isString(key[0])) { + if (key.length > 1 || _.isNil(key[0]) || !_.isString(key[0])) { throw new Error('Key should be a string'); } return credentials[key]; diff --git a/test/__tests__/user_transformation.test.js b/test/__tests__/user_transformation.test.js index 31a07b6716..710b21e6d6 100644 --- a/test/__tests__/user_transformation.test.js +++ b/test/__tests__/user_transformation.test.js @@ -1174,6 +1174,35 @@ describe("User transformation", () => { expect(output[0].error).toMatch(/Key should be a string/); }); + it(`Simple ${name} Test with credentials with multiple arguements for codeVersion 1`, async () => { + const versionId = randomID(); + + const inputData = require(`./data/${integration}_input_credentials.json`); + + const respBody = { + versionId: versionId, + codeVersion: "1", + name, + code: ` + export function transformEvent(event, metadata) { + event.credentialValue = credential('arg1', 'arg2'); + return event; + } + ` + }; + fetch.mockResolvedValue({ + status: 200, + json: jest.fn().mockResolvedValue(respBody) + }); + + const output = await userTransformHandler(inputData, versionId, []); + + expect(fetch).toHaveBeenCalledWith( + `https://api.rudderlabs.com/transformation/getByVersionId?versionId=${versionId}` + ); + expect(output[0].error).toMatch(/Key should be a string/); + }); + it(`Simple ${name} Test with credentials with non string key for codeVersion 1`, async () => { const versionId = randomID(); From bb94c81c90610e2d27413a054834137e87cf7161 Mon Sep 17 00:00:00 2001 From: kanishkkatara Date: Wed, 12 Jun 2024 12:23:09 +0530 Subject: [PATCH 12/20] chore: change in lodash import --- src/util/customTransformer.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/util/customTransformer.js b/src/util/customTransformer.js index fcc71b82a3..3b3a3539f3 100644 --- a/src/util/customTransformer.js +++ b/src/util/customTransformer.js @@ -8,8 +8,7 @@ const { parserForImport } = require('./parser'); const stats = require('./stats'); const { fetchWithDnsWrapper } = require('./utils'); const { getMetadata, getTransformationMetadata } = require('../v0/util'); -const _ = require('lodash'); - +const { isNil } = require('lodash'); const ISOLATE_VM_MEMORY = parseInt(process.env.ISOLATE_VM_MEMORY || '128', 10); const GEOLOCATION_TIMEOUT_IN_MS = parseInt(process.env.GEOLOCATION_TIMEOUT_IN_MS || '1000', 10); @@ -298,7 +297,7 @@ async function userTransformHandler( credentialsMap[cred.key] = cred.value; }); events.forEach((ev) => { - if (_.isNil(ev?.credentials)) { + if (isNil(ev?.credentials)) { ev.credentials = credentials; } }); From a80d403d0b88259c837564f9f0cd1b7969c5d4ca Mon Sep 17 00:00:00 2001 From: kanishkkatara Date: Mon, 17 Jun 2024 18:50:54 +0530 Subject: [PATCH 13/20] chore: changes in transformation error handling --- src/util/ivmFactory.js | 11 +++++++---- src/util/prometheus.js | 6 ++++++ test/__tests__/user_transformation.test.js | 8 ++++---- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/src/util/ivmFactory.js b/src/util/ivmFactory.js index 50bf4caa49..9bb5606b6f 100644 --- a/src/util/ivmFactory.js +++ b/src/util/ivmFactory.js @@ -243,12 +243,15 @@ async function createIvm(code, libraryVersionIds, versionId, credentials, secret }), ); - await jail.set('_credential', function (key) { + await jail.set('_credential', function (...args) { if (_.isNil(credentials) || !_.isObject(credentials)) { - throw new Error('Credentials in incorrect format'); + logger.error('Error fetching credentials map'); + stats.increment('credential_error', { versionId }); + return undefined; } - if (key.length > 1 || _.isNil(key[0]) || !_.isString(key[0])) { - throw new Error('Key should be a string'); + const key = args[0][0]; + if (_.isNil(key)) { + throw new Error('Key should be valid and defined'); } return credentials[key]; }); diff --git a/src/util/prometheus.js b/src/util/prometheus.js index bc4c6f2eb9..e37fe249ae 100644 --- a/src/util/prometheus.js +++ b/src/util/prometheus.js @@ -569,6 +569,12 @@ class Prometheus { 'module', ], }, + { + name: 'credentials_error', + help: 'Error in fetching credentials count', + type: 'counter', + labelNames: ['versionId'], + }, // Gauges { diff --git a/test/__tests__/user_transformation.test.js b/test/__tests__/user_transformation.test.js index 710b21e6d6..e3c6122c0c 100644 --- a/test/__tests__/user_transformation.test.js +++ b/test/__tests__/user_transformation.test.js @@ -1171,7 +1171,7 @@ describe("User transformation", () => { expect(fetch).toHaveBeenCalledWith( `https://api.rudderlabs.com/transformation/getByVersionId?versionId=${versionId}` ); - expect(output[0].error).toMatch(/Key should be a string/); + expect(output[0].error).toMatch(/Key should be valid and defined/); }); it(`Simple ${name} Test with credentials with multiple arguements for codeVersion 1`, async () => { @@ -1185,7 +1185,7 @@ describe("User transformation", () => { name, code: ` export function transformEvent(event, metadata) { - event.credentialValue = credential('arg1', 'arg2'); + event.credentialValue = credential('key1', 'key2'); return event; } ` @@ -1200,7 +1200,7 @@ describe("User transformation", () => { expect(fetch).toHaveBeenCalledWith( `https://api.rudderlabs.com/transformation/getByVersionId?versionId=${versionId}` ); - expect(output[0].error).toMatch(/Key should be a string/); + expect(output[0].transformedEvent.credentialValue).toEqual("value1"); }); it(`Simple ${name} Test with credentials with non string key for codeVersion 1`, async () => { @@ -1229,7 +1229,7 @@ describe("User transformation", () => { expect(fetch).toHaveBeenCalledWith( `https://api.rudderlabs.com/transformation/getByVersionId?versionId=${versionId}` ); - expect(output[0].error).toMatch(/Key should be a string/); + expect(output[0].transformedEvent.credentialValue).toEqual(undefined); }); it(`Simple ${name} Test with credentials without value for codeVersion 1`, async () => { From 67e9277120e39402b32eac55ffa34a0053b6af61 Mon Sep 17 00:00:00 2001 From: kanishkkatara Date: Tue, 18 Jun 2024 12:19:20 +0530 Subject: [PATCH 14/20] fix: fixes in javascript transformation --- src/util/customTransformer-v1.js | 2 + src/util/customTransformer.js | 10 ++-- src/util/ivmFactory.js | 51 ++++++++++++++----- src/util/prometheus.js | 4 +- ...user_transformation_input_credentials.json | 14 ++--- .../user_transformation.integration.test.js | 6 +-- test/__tests__/user_transformation.test.js | 24 +++++---- .../user_transformation_fetch.test.js | 24 ++++----- 8 files changed, 85 insertions(+), 50 deletions(-) diff --git a/src/util/customTransformer-v1.js b/src/util/customTransformer-v1.js index 4d846687b5..ea6dec7759 100644 --- a/src/util/customTransformer-v1.js +++ b/src/util/customTransformer-v1.js @@ -63,6 +63,8 @@ async function userTransformHandlerV1( userTransformation.code, libraryVersionIds, userTransformation.versionId, + userTransformation.id, + userTransformation.workspaceId || events[0].metadata?.workspaceId, credentials, userTransformation.secrets || {}, testMode, diff --git a/src/util/customTransformer.js b/src/util/customTransformer.js index 3b3a3539f3..5107d7e29a 100644 --- a/src/util/customTransformer.js +++ b/src/util/customTransformer.js @@ -289,15 +289,15 @@ async function userTransformHandler( }); const credentialsMap = {}; if (testMode === false) { - events[0]?.credentials?.forEach((cred) => { - credentialsMap[cred.key] = cred.value; + (events[0]?.credentials || []).forEach((cred) => { + credentialsMap[cred.Key] = cred.Value; }); } else { - credentials?.forEach((cred) => { - credentialsMap[cred.key] = cred.value; + (credentials || []).forEach((cred) => { + credentialsMap[cred.Key] = cred.Value; }); events.forEach((ev) => { - if (isNil(ev?.credentials)) { + if (isNil(ev.credentials)) { ev.credentials = credentials; } }); diff --git a/src/util/ivmFactory.js b/src/util/ivmFactory.js index 9bb5606b6f..216c6d15c6 100644 --- a/src/util/ivmFactory.js +++ b/src/util/ivmFactory.js @@ -30,7 +30,16 @@ async function loadModule(isolateInternal, contextInternal, moduleName, moduleCo return module; } -async function createIvm(code, libraryVersionIds, versionId, credentials, secrets, testMode) { +async function createIvm( + code, + libraryVersionIds, + versionId, + transformationId, + workspaceId, + credentials, + secrets, + testMode, +) { const createIvmStartTime = new Date(); const logs = []; const libraries = await Promise.all( @@ -243,16 +252,12 @@ async function createIvm(code, libraryVersionIds, versionId, credentials, secret }), ); - await jail.set('_credential', function (...args) { + await jail.set('_credential', function (key) { if (_.isNil(credentials) || !_.isObject(credentials)) { - logger.error('Error fetching credentials map'); - stats.increment('credential_error', { versionId }); + logger.error('Error fetching credentials map', versionId); + stats.increment('credential_error', { transformationId, workspaceId }); return undefined; } - const key = args[0][0]; - if (_.isNil(key)) { - throw new Error('Key should be valid and defined'); - } return credentials[key]; }); @@ -337,9 +342,11 @@ async function createIvm(code, libraryVersionIds, versionId, credentials, secret let credential = _credential; delete _credential; global.credential = function(...args) { - return credential([ - ...args.map(arg => new ivm.ExternalCopy(arg).copyInto()) - ]); + const key = args[0]; + if (key === null || key === undefined) { + throw new Error('Key should be valid and defined'+ JSON.stringify(args)); + } + return credential(new ivm.ExternalCopy(key).copyInto()); }; return new ivm.Reference(function forwardMainPromise( @@ -432,10 +439,28 @@ async function compileUserLibrary(code) { return evaluateModule(isolate, context, code); } -async function getFactory(code, libraryVersionIds, versionId, credentials, secrets, testMode) { +async function getFactory( + code, + libraryVersionIds, + transformationId, + workspaceId, + versionId, + credentials, + secrets, + testMode, +) { const factory = { create: async () => { - return createIvm(code, libraryVersionIds, versionId, credentials, secrets, testMode); + return createIvm( + code, + libraryVersionIds, + versionId, + transformationId, + workspaceId, + credentials, + secrets, + testMode, + ); }, destroy: async (client) => { client.fnRef.release(); diff --git a/src/util/prometheus.js b/src/util/prometheus.js index e37fe249ae..3b9a9d5d44 100644 --- a/src/util/prometheus.js +++ b/src/util/prometheus.js @@ -570,10 +570,10 @@ class Prometheus { ], }, { - name: 'credentials_error', + name: 'credential_error', help: 'Error in fetching credentials count', type: 'counter', - labelNames: ['versionId'], + labelNames: ['transformationId', 'workspaceId'], }, // Gauges diff --git a/test/__tests__/data/user_transformation_input_credentials.json b/test/__tests__/data/user_transformation_input_credentials.json index 49b224e560..bd2593f56d 100644 --- a/test/__tests__/data/user_transformation_input_credentials.json +++ b/test/__tests__/data/user_transformation_input_credentials.json @@ -51,14 +51,16 @@ }, "credentials": [ { - "key": "key1", - "value": "value1", - "isPublic": true + "Id": "id1", + "Key": "key1", + "Value": "value1", + "IsSecret": false }, { - "key": "key2", - "value": "value2", - "isPublic": true + "Id": "id2", + "Key": "key2", + "Value": "value2", + "IsSecret": true } ] } diff --git a/test/__tests__/user_transformation.integration.test.js b/test/__tests__/user_transformation.integration.test.js index 1e2fafeb73..712e151a45 100644 --- a/test/__tests__/user_transformation.integration.test.js +++ b/test/__tests__/user_transformation.integration.test.js @@ -159,14 +159,14 @@ describe("Function invocation & creation tests", () => { versionId, [], trRevCode, - null, + [], true ); expect(response).toEqual(outputEvents); // Test with language python; should return same output trRevCode = contructTrRevCode(versionId, 'python'); - response = await userTransformHandler(inputEvents, versionId, [], trRevCode, null, true); + response = await userTransformHandler(inputEvents, versionId, [], trRevCode, [], true); expect(response).toEqual(outputEvents); }); @@ -185,7 +185,7 @@ describe("Function invocation & creation tests", () => { }); await expect(async () => { - await userTransformHandler(inputEvents, respBody.versionId, null, []); + await userTransformHandler(inputEvents, respBody.versionId, []); }).rejects.toThrow(RetryRequestError); // If function is not found, it will be created diff --git a/test/__tests__/user_transformation.test.js b/test/__tests__/user_transformation.test.js index e3c6122c0c..1abece8f0c 100644 --- a/test/__tests__/user_transformation.test.js +++ b/test/__tests__/user_transformation.test.js @@ -1073,7 +1073,7 @@ describe("User transformation", () => { trRevCode.versionId, [libraryVersionId], trRevCode, - null, + [], true ); @@ -1110,7 +1110,7 @@ describe("User transformation", () => { trRevCode.versionId, [], trRevCode, - null, + [], true ); expect(output).toEqual(expectedData); @@ -1128,7 +1128,7 @@ describe("User transformation", () => { name, code: ` export function transformEvent(event, metadata) { - event.credentialValue = credential("key1"); + event.credentialValue = credential('key1'); return event; } ` @@ -1174,7 +1174,7 @@ describe("User transformation", () => { expect(output[0].error).toMatch(/Key should be valid and defined/); }); - it(`Simple ${name} Test with credentials with multiple arguements for codeVersion 1`, async () => { + it(`Simple ${name} Test with credentials with multiple arguments for codeVersion 1`, async () => { const versionId = randomID(); const inputData = require(`./data/${integration}_input_credentials.json`); @@ -1214,7 +1214,10 @@ describe("User transformation", () => { name, code: ` export function transformEvent(event, metadata) { - event.credentialValue = credential(1); + event.credentialValueForNumkey = credential(1); + event.credentialValueForBoolkey = credential(true); + event.credentialValueForArraykey = credential([]); + event.credentialValueForObjkey = credential({}); return event; } ` @@ -1229,7 +1232,10 @@ describe("User transformation", () => { expect(fetch).toHaveBeenCalledWith( `https://api.rudderlabs.com/transformation/getByVersionId?versionId=${versionId}` ); - expect(output[0].transformedEvent.credentialValue).toEqual(undefined); + expect(output[0].transformedEvent.credentialValueForNumkey).toEqual(undefined); + expect(output[0].transformedEvent.credentialValueForBoolkey).toEqual(undefined); + expect(output[0].transformedEvent.credentialValueForArraykey).toEqual(undefined); + expect(output[0].transformedEvent.credentialValueForObjkey).toEqual(undefined); }); it(`Simple ${name} Test with credentials without value for codeVersion 1`, async () => { @@ -1707,7 +1713,7 @@ describe("Python transformations", () => { trRevCode.versionId, [], trRevCode, - null, + [], true ); expect(output).toEqual(outputData); @@ -1753,7 +1759,7 @@ describe("Python transformations", () => { trRevCode.versionId, Object.values(importNameLibraryVersionIdsMap), trRevCode, - null, + [], true ); expect(output).toEqual(outputData); @@ -1795,7 +1801,7 @@ describe("Python transformations", () => { trRevCode.versionId, Object.values(importNameLibraryVersionIdsMap), trRevCode, - null, + [], true ); expect(output).toEqual(outputData); diff --git a/test/__tests__/user_transformation_fetch.test.js b/test/__tests__/user_transformation_fetch.test.js index edbac04cf3..5bf9d54935 100644 --- a/test/__tests__/user_transformation_fetch.test.js +++ b/test/__tests__/user_transformation_fetch.test.js @@ -43,7 +43,7 @@ describe("User transformation fetch tests", () => { ` }; - const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true); + const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true); expect(output).toEqual({ logs: [], transformedEvents: expectedData.map(ev => (ev.transformedEvent)) @@ -67,7 +67,7 @@ describe("User transformation fetch tests", () => { }; const errMsg = "ERROR"; - const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true); + const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true); expect(mockResolver).toHaveBeenCalledTimes(0); output.transformedEvents.forEach(ev => { @@ -92,7 +92,7 @@ describe("User transformation fetch tests", () => { const errMsg = "ERROR"; mockResolver.mockResolvedValue([ '127.0.0.1' ]); - const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true); + const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true); expect(mockResolver).toHaveBeenCalledTimes(inputData.length); expect(mockResolver).toHaveBeenCalledWith('abc.xyz.com'); @@ -122,7 +122,7 @@ describe("User transformation fetch tests", () => { }; const errMsg = "invalid url, localhost requests are not allowed"; - const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true); + const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true); expect(mockResolver).toHaveBeenCalledTimes(0); output.transformedEvents.forEach(ev => { @@ -152,7 +152,7 @@ describe("User transformation fetch tests", () => { const errMsg = "request to https://abc.xyz.com/dummyUrl failed, reason: Invalid IP address: unable to resolve IP address for abc.xyz.com"; mockResolver.mockRejectedValue('invalid host'); - const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true); + const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true); expect(mockResolver).toHaveBeenCalledTimes(inputData.length); expect(mockResolver).toHaveBeenCalledWith('abc.xyz.com'); @@ -182,7 +182,7 @@ describe("User transformation fetch tests", () => { }; const errMsg = "invalid protocol, only http and https are supported"; - const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true); + const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true); output.transformedEvents.forEach(ev => { expect(ev.errMsg).toEqual(errMsg); @@ -204,7 +204,7 @@ describe("User transformation fetch tests", () => { versionId, }; - const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true); + const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true); expect(output).toEqual({ logs: [], transformedEvents: expectedData.map(ev => (ev.transformedEvent)) @@ -226,7 +226,7 @@ describe("User transformation fetch tests", () => { }; const errMsg = "ERROR"; - const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true); + const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true); expect(mockResolver).toHaveBeenCalledTimes(0); output.transformedEvents.forEach(ev => { @@ -250,7 +250,7 @@ describe("User transformation fetch tests", () => { const errMsg = "ERROR"; mockResolver.mockRejectedValue('invalid host'); - const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true); + const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true); expect(mockResolver).toHaveBeenCalledTimes(inputData.length); expect(mockResolver).toHaveBeenCalledWith('abc.xyz.com'); @@ -279,7 +279,7 @@ describe("User transformation fetch tests", () => { }; const errMsg = "invalid url, localhost requests are not allowed"; - const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true); + const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true); expect(mockResolver).toHaveBeenCalledTimes(0); output.transformedEvents.forEach(ev => { @@ -308,7 +308,7 @@ describe("User transformation fetch tests", () => { const errMsg = "request to https://abc.xyz.com/dummyUrl failed, reason: Invalid IP address: cannot use 127.0.0.1 as IP address"; mockResolver.mockResolvedValue(['3.122.122.122', '127.0.0.1']); - const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true); + const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true); expect(mockResolver).toHaveBeenCalledTimes(inputData.length); expect(mockResolver).toHaveBeenCalledWith('abc.xyz.com'); @@ -337,7 +337,7 @@ describe("User transformation fetch tests", () => { }; const errMsg = "fetch url is required"; - const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true); + const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true); output.transformedEvents.forEach(ev => { expect(ev.errMsg).toEqual(errMsg); From e7a59f977ff29e53f7b6e7cc39f7f0654aeac120 Mon Sep 17 00:00:00 2001 From: kanishkkatara Date: Tue, 18 Jun 2024 13:09:57 +0530 Subject: [PATCH 15/20] chore: added tests for transform batch and code version 0 --- test/__tests__/user_transformation.test.js | 91 ++++++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/test/__tests__/user_transformation.test.js b/test/__tests__/user_transformation.test.js index 1abece8f0c..e94df4f4df 100644 --- a/test/__tests__/user_transformation.test.js +++ b/test/__tests__/user_transformation.test.js @@ -1295,6 +1295,97 @@ describe("User transformation", () => { ); expect(output[0].transformedEvent.credentialValue).toEqual(undefined); }); + + it(`Simple ${name} Batch Test with credentials for codeVersion 1`, async () => { + const versionId = randomID(); + + const inputData = require(`./data/${integration}_input_credentials.json`); + + const respBody = { + versionId: versionId, + codeVersion: "1", + name, + code: ` + export function transformBatch(events, metadata) { + events.forEach((event) => { + event.credentialValue1 = credential("key1"); + event.credentialValue2 = credential("key3"); + }); + return events; + } + ` + }; + fetch.mockResolvedValue({ + status: 200, + json: jest.fn().mockResolvedValue(respBody) + }); + + const output = await userTransformHandler(inputData, versionId, []); + expect(fetch).toHaveBeenCalledWith( + `https://api.rudderlabs.com/transformation/getByVersionId?versionId=${versionId}` + ); + expect(output[0].transformedEvent.credentialValue1).toEqual("value1"); + expect(output[0].transformedEvent.credentialValue2).toEqual(undefined); + }); + + it(`Simple ${name} Batch Test with credentials without key for codeVersion 1`, async () => { + const versionId = randomID(); + + const inputData = require(`./data/${integration}_input_credentials.json`); + + const respBody = { + versionId: versionId, + codeVersion: "1", + name, + code: ` + export function transformBatch(events, metadata) { + events.forEach((event) => { + event.credentialValue = credential(); + }); + return events; + } + ` + }; + fetch.mockResolvedValue({ + status: 200, + json: jest.fn().mockResolvedValue(respBody) + }); + + const output = await userTransformHandler(inputData, versionId, []); + expect(fetch).toHaveBeenCalledWith( + `https://api.rudderlabs.com/transformation/getByVersionId?versionId=${versionId}` + ); + expect(output[0].error).toMatch(/Key should be valid and defined/); + }); + + it(`Simple ${name} Test with credentials for codeVersion 0`, async () => { + const versionId = randomID(); + + const inputData = require(`./data/${integration}_input_credentials.json`); + + const respBody = { + versionId: versionId, + codeVersion: "0", + name, + code: ` + function transform(events) { + events.forEach((event) => { + event.credentialValue = credential + }); + return events; + } + ` + }; + fetch.mockResolvedValue({ + status: 200, + json: jest.fn().mockResolvedValue(respBody) + }); + try { + await userTransformHandler(inputData, versionId, []); + } catch (e) { + expect(e).toEqual('credential is not defined'); + } + }); }); }); From 46cd3413af065b76e4f89fb5b2720cbf6213e51b Mon Sep 17 00:00:00 2001 From: kanishkkatara Date: Tue, 18 Jun 2024 13:39:57 +0530 Subject: [PATCH 16/20] fix: pr comments --- src/controllers/userTransform.ts | 2 +- src/util/customTransformer-v1.js | 2 +- src/util/ivmFactory.js | 8 ++++---- src/util/prometheus.js | 2 +- test/__tests__/user_transformation.test.js | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/controllers/userTransform.ts b/src/controllers/userTransform.ts index 33e1507832..8f6e3447a1 100644 --- a/src/controllers/userTransform.ts +++ b/src/controllers/userTransform.ts @@ -33,7 +33,7 @@ export class UserTransformController { '(User transform - router:/transformation/test ):: Request to transformer', ctx.request.body, ); - const { events, trRevCode, libraryVersionIDs = [], credentials } = ctx.request.body as any; + const { events, trRevCode, libraryVersionIDs = [], credentials = [] } = ctx.request.body as any; const response = await UserTransformService.testTransformRoutine( events, trRevCode, diff --git a/src/util/customTransformer-v1.js b/src/util/customTransformer-v1.js index ea6dec7759..e0212831ca 100644 --- a/src/util/customTransformer-v1.js +++ b/src/util/customTransformer-v1.js @@ -64,7 +64,7 @@ async function userTransformHandlerV1( libraryVersionIds, userTransformation.versionId, userTransformation.id, - userTransformation.workspaceId || events[0].metadata?.workspaceId, + userTransformation.workspaceId, credentials, userTransformation.secrets || {}, testMode, diff --git a/src/util/ivmFactory.js b/src/util/ivmFactory.js index 216c6d15c6..b31c87c049 100644 --- a/src/util/ivmFactory.js +++ b/src/util/ivmFactory.js @@ -1,6 +1,6 @@ const ivm = require('isolated-vm'); const fetch = require('node-fetch'); -const _ = require('lodash'); +const { isNil, isObject, camelCase } = require('lodash'); const { getLibraryCodeV1, getRudderLibByImportName } = require('./customTransforrmationsStore-v1'); const { extractStackTraceUptoLastSubstringMatch } = require('./utils'); @@ -60,7 +60,7 @@ async function createIvm( // TODO: Check if this should this be && libraries.forEach((library) => { - const libHandleName = _.camelCase(library.name); + const libHandleName = camelCase(library.name); if (extractedLibraries.includes(libHandleName)) { librariesMap[libHandleName] = library.code; } @@ -253,9 +253,9 @@ async function createIvm( ); await jail.set('_credential', function (key) { - if (_.isNil(credentials) || !_.isObject(credentials)) { + if (isNil(credentials) || !isObject(credentials)) { logger.error('Error fetching credentials map', versionId); - stats.increment('credential_error', { transformationId, workspaceId }); + stats.increment('credential_errors', { transformationId, workspaceId }); return undefined; } return credentials[key]; diff --git a/src/util/prometheus.js b/src/util/prometheus.js index 3b9a9d5d44..0f8a1f4e0c 100644 --- a/src/util/prometheus.js +++ b/src/util/prometheus.js @@ -570,7 +570,7 @@ class Prometheus { ], }, { - name: 'credential_error', + name: 'credential_errors', help: 'Error in fetching credentials count', type: 'counter', labelNames: ['transformationId', 'workspaceId'], diff --git a/test/__tests__/user_transformation.test.js b/test/__tests__/user_transformation.test.js index e94df4f4df..200895f480 100644 --- a/test/__tests__/user_transformation.test.js +++ b/test/__tests__/user_transformation.test.js @@ -1370,7 +1370,7 @@ describe("User transformation", () => { code: ` function transform(events) { events.forEach((event) => { - event.credentialValue = credential + event.credentialValue = credential('key1'); }); return events; } From 819f508b017f988e302b6e2579cb3da4030dcfa0 Mon Sep 17 00:00:00 2001 From: kanishkkatara Date: Tue, 18 Jun 2024 13:49:08 +0530 Subject: [PATCH 17/20] fix: undefined tests --- test/__tests__/user_transformation.test.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/__tests__/user_transformation.test.js b/test/__tests__/user_transformation.test.js index 200895f480..574a5417ca 100644 --- a/test/__tests__/user_transformation.test.js +++ b/test/__tests__/user_transformation.test.js @@ -1232,10 +1232,10 @@ describe("User transformation", () => { expect(fetch).toHaveBeenCalledWith( `https://api.rudderlabs.com/transformation/getByVersionId?versionId=${versionId}` ); - expect(output[0].transformedEvent.credentialValueForNumkey).toEqual(undefined); - expect(output[0].transformedEvent.credentialValueForBoolkey).toEqual(undefined); - expect(output[0].transformedEvent.credentialValueForArraykey).toEqual(undefined); - expect(output[0].transformedEvent.credentialValueForObjkey).toEqual(undefined); + expect(output[0].transformedEvent.credentialValueForNumkey).toBeUndefined(); + expect(output[0].transformedEvent.credentialValueForBoolkey).toBeUndefined(); + expect(output[0].transformedEvent.credentialValueForArraykey).toBeUndefined(); + expect(output[0].transformedEvent.credentialValueForObjkey).toBeUndefined(); }); it(`Simple ${name} Test with credentials without value for codeVersion 1`, async () => { @@ -1264,7 +1264,7 @@ describe("User transformation", () => { expect(fetch).toHaveBeenCalledWith( `https://api.rudderlabs.com/transformation/getByVersionId?versionId=${versionId}` ); - expect(output[0].transformedEvent.credentialValue).toEqual(undefined); + expect(output[0].transformedEvent.credentialValue).toBeUndefined(); }); it(`Simple ${name} Test without credentials for codeVersion 1`, async () => { @@ -1293,7 +1293,7 @@ describe("User transformation", () => { expect(fetch).toHaveBeenCalledWith( `https://api.rudderlabs.com/transformation/getByVersionId?versionId=${versionId}` ); - expect(output[0].transformedEvent.credentialValue).toEqual(undefined); + expect(output[0].transformedEvent.credentialValue).toBeUndefined(); }); it(`Simple ${name} Batch Test with credentials for codeVersion 1`, async () => { @@ -1325,7 +1325,7 @@ describe("User transformation", () => { `https://api.rudderlabs.com/transformation/getByVersionId?versionId=${versionId}` ); expect(output[0].transformedEvent.credentialValue1).toEqual("value1"); - expect(output[0].transformedEvent.credentialValue2).toEqual(undefined); + expect(output[0].transformedEvent.credentialValue2).toBeUndefined(); }); it(`Simple ${name} Batch Test with credentials without key for codeVersion 1`, async () => { From e6c50980a12960a419f5c03e120739eea642767a Mon Sep 17 00:00:00 2001 From: kanishkkatara Date: Tue, 18 Jun 2024 17:57:00 +0530 Subject: [PATCH 18/20] fix: credentials payload fix --- src/types/index.ts | 8 ++++---- src/util/customTransformer.js | 4 ++-- .../user_transformation_input_credentials.json | 16 ++++++++-------- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/types/index.ts b/src/types/index.ts index 6297d49c42..150758363e 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -140,10 +140,10 @@ type UserTransformationLibrary = { }; type Credential = { - ID: string; - Key: string; - Value: string; - IsSecret: boolean; + id: string; + key: string; + value: string; + isSecret: boolean; }; type ProcessorTransformationRequest = { diff --git a/src/util/customTransformer.js b/src/util/customTransformer.js index 5107d7e29a..a7b52b004c 100644 --- a/src/util/customTransformer.js +++ b/src/util/customTransformer.js @@ -290,11 +290,11 @@ async function userTransformHandler( const credentialsMap = {}; if (testMode === false) { (events[0]?.credentials || []).forEach((cred) => { - credentialsMap[cred.Key] = cred.Value; + credentialsMap[cred.key] = cred.value; }); } else { (credentials || []).forEach((cred) => { - credentialsMap[cred.Key] = cred.Value; + credentialsMap[cred.key] = cred.value; }); events.forEach((ev) => { if (isNil(ev.credentials)) { diff --git a/test/__tests__/data/user_transformation_input_credentials.json b/test/__tests__/data/user_transformation_input_credentials.json index bd2593f56d..d210a3715c 100644 --- a/test/__tests__/data/user_transformation_input_credentials.json +++ b/test/__tests__/data/user_transformation_input_credentials.json @@ -51,16 +51,16 @@ }, "credentials": [ { - "Id": "id1", - "Key": "key1", - "Value": "value1", - "IsSecret": false + "id": "id1", + "key": "key1", + "value": "value1", + "isSecret": false }, { - "Id": "id2", - "Key": "key2", - "Value": "value2", - "IsSecret": true + "id": "id2", + "key": "key2", + "value": "value2", + "isSecret": true } ] } From 3db810c857017318c1979965ce653eca3b3fdadf Mon Sep 17 00:00:00 2001 From: kanishkkatara Date: Wed, 19 Jun 2024 14:29:37 +0530 Subject: [PATCH 19/20] fix: tests names, credentialsMap refactor --- src/legacy/router.js | 1 - src/services/userTransform.ts | 14 +- src/util/customTransformer-v1.js | 7 +- src/util/customTransformer.js | 18 --- src/util/customTransformerFactory.js | 10 +- src/util/ivmFactory.js | 4 +- src/util/prometheus.js | 2 +- .../user_transformation.integration.test.js | 2 +- test/__tests__/user_transformation.test.js | 132 +++++++++--------- .../user_transformation_errors.test.js | 8 -- .../user_transformation_fetch.test.js | 24 ++-- 11 files changed, 100 insertions(+), 122 deletions(-) diff --git a/src/legacy/router.js b/src/legacy/router.js index b2c50660f4..afc8c1a797 100644 --- a/src/legacy/router.js +++ b/src/legacy/router.js @@ -678,7 +678,6 @@ if (transformerTestModeEnabled) { trRevCode.versionId, libraryVersionIDs, trRevCode, - null, true, ); logger.debug(`[CT] Test Output Events: ${JSON.stringify(res.transformedEvents)}`); diff --git a/src/services/userTransform.ts b/src/services/userTransform.ts index e03aebc290..83e0c807d6 100644 --- a/src/services/userTransform.ts +++ b/src/services/userTransform.ts @@ -1,5 +1,6 @@ import groupBy from 'lodash/groupBy'; import isEmpty from 'lodash/isEmpty'; +import { isNil } from 'lodash'; import { userTransformHandler } from '../routerUtils'; import { UserTransformationLibrary, @@ -213,15 +214,24 @@ export class UserTransformService { throw new Error('Invalid request. Missing events'); } + const updatedEvents = events.map((ev) => { + if (isNil(ev.credentials)) { + return { + ...ev, + credentials, + }; + } + return ev; + }); + logger.debug(`[CT] Test Input Events: ${JSON.stringify(events)}`); // eslint-disable-next-line no-param-reassign trRevCode.versionId = 'testVersionId'; response.body = await userTransformHandler()( - events, + updatedEvents, trRevCode.versionId, libraryVersionIDs, trRevCode, - credentials, true, ); logger.debug(`[CT] Test Output Events: ${JSON.stringify(response.body.transformedEvents)}`); diff --git a/src/util/customTransformer-v1.js b/src/util/customTransformer-v1.js index e0212831ca..b1865dee6b 100644 --- a/src/util/customTransformer-v1.js +++ b/src/util/customTransformer-v1.js @@ -51,7 +51,6 @@ function calculateMsFromIvmTime(value) { async function userTransformHandlerV1( events, userTransformation, - credentials, libraryVersionIds, testMode = false, ) { @@ -59,13 +58,17 @@ async function userTransformHandlerV1( return { transformedEvents: events }; } + const credentialsMap = {}; + (events[0]?.credentials || []).forEach((cred) => { + credentialsMap[cred.key] = cred.value; + }); const isolatevmFactory = await getFactory( userTransformation.code, libraryVersionIds, userTransformation.versionId, userTransformation.id, userTransformation.workspaceId, - credentials, + credentialsMap, userTransformation.secrets || {}, testMode, ); diff --git a/src/util/customTransformer.js b/src/util/customTransformer.js index a7b52b004c..787ce04d63 100644 --- a/src/util/customTransformer.js +++ b/src/util/customTransformer.js @@ -8,7 +8,6 @@ const { parserForImport } = require('./parser'); const stats = require('./stats'); const { fetchWithDnsWrapper } = require('./utils'); const { getMetadata, getTransformationMetadata } = require('../v0/util'); -const { isNil } = require('lodash'); const ISOLATE_VM_MEMORY = parseInt(process.env.ISOLATE_VM_MEMORY || '128', 10); const GEOLOCATION_TIMEOUT_IN_MS = parseInt(process.env.GEOLOCATION_TIMEOUT_IN_MS || '1000', 10); @@ -274,7 +273,6 @@ async function userTransformHandler( versionId, libraryVersionIDs, trRevCode = {}, - credentials = [], testMode = false, ) { if (versionId) { @@ -287,27 +285,11 @@ async function userTransformHandler( events.forEach((ev) => { eventsMetadata[ev.message.messageId] = ev.metadata; }); - const credentialsMap = {}; - if (testMode === false) { - (events[0]?.credentials || []).forEach((cred) => { - credentialsMap[cred.key] = cred.value; - }); - } else { - (credentials || []).forEach((cred) => { - credentialsMap[cred.key] = cred.value; - }); - events.forEach((ev) => { - if (isNil(ev.credentials)) { - ev.credentials = credentials; - } - }); - } let userTransformedEvents = []; let result; if (res.codeVersion && res.codeVersion === '1') { result = await UserTransformHandlerFactory(res).runUserTransfrom( events, - credentialsMap, testMode, libraryVersionIDs, ); diff --git a/src/util/customTransformerFactory.js b/src/util/customTransformerFactory.js index f6bbbc492c..ee53531946 100644 --- a/src/util/customTransformerFactory.js +++ b/src/util/customTransformerFactory.js @@ -13,20 +13,14 @@ const UserTransformHandlerFactory = (userTransformation) => { } }, - runUserTransfrom: async (events, credentials, testMode, libraryVersionIds) => { + runUserTransfrom: async (events, testMode, libraryVersionIds) => { switch (userTransformation.language) { case 'pythonfaas': case 'python': return runOpenFaasUserTransform(events, userTransformation, libraryVersionIds, testMode); default: - return userTransformHandlerV1( - events, - userTransformation, - credentials, - libraryVersionIds, - testMode, - ); + return userTransformHandlerV1(events, userTransformation, libraryVersionIds, testMode); } }, }; diff --git a/src/util/ivmFactory.js b/src/util/ivmFactory.js index b31c87c049..84eb52f0c8 100644 --- a/src/util/ivmFactory.js +++ b/src/util/ivmFactory.js @@ -255,7 +255,7 @@ async function createIvm( await jail.set('_credential', function (key) { if (isNil(credentials) || !isObject(credentials)) { logger.error('Error fetching credentials map', versionId); - stats.increment('credential_errors', { transformationId, workspaceId }); + stats.increment('credential_error_total', { transformationId, workspaceId }); return undefined; } return credentials[key]; @@ -344,7 +344,7 @@ async function createIvm( global.credential = function(...args) { const key = args[0]; if (key === null || key === undefined) { - throw new Error('Key should be valid and defined'+ JSON.stringify(args)); + throw new TypeError('Key should be valid and defined'); } return credential(new ivm.ExternalCopy(key).copyInto()); }; diff --git a/src/util/prometheus.js b/src/util/prometheus.js index 0f8a1f4e0c..d8a2a0cb04 100644 --- a/src/util/prometheus.js +++ b/src/util/prometheus.js @@ -570,7 +570,7 @@ class Prometheus { ], }, { - name: 'credential_errors', + name: 'credential_error_total', help: 'Error in fetching credentials count', type: 'counter', labelNames: ['transformationId', 'workspaceId'], diff --git a/test/__tests__/user_transformation.integration.test.js b/test/__tests__/user_transformation.integration.test.js index 712e151a45..6598b9fa0d 100644 --- a/test/__tests__/user_transformation.integration.test.js +++ b/test/__tests__/user_transformation.integration.test.js @@ -166,7 +166,7 @@ describe("Function invocation & creation tests", () => { // Test with language python; should return same output trRevCode = contructTrRevCode(versionId, 'python'); - response = await userTransformHandler(inputEvents, versionId, [], trRevCode, [], true); + response = await userTransformHandler(inputEvents, versionId, [], trRevCode, true); expect(response).toEqual(outputEvents); }); diff --git a/test/__tests__/user_transformation.test.js b/test/__tests__/user_transformation.test.js index 574a5417ca..24ed1ae1ff 100644 --- a/test/__tests__/user_transformation.test.js +++ b/test/__tests__/user_transformation.test.js @@ -1073,7 +1073,6 @@ describe("User transformation", () => { trRevCode.versionId, [libraryVersionId], trRevCode, - [], true ); @@ -1110,14 +1109,13 @@ describe("User transformation", () => { trRevCode.versionId, [], trRevCode, - [], true ); expect(output).toEqual(expectedData); }); - describe("Credentials", () => { - it(`Simple ${name} Test with credentials for codeVersion 1`, async () => { + describe("UserTransformation With Credentials for code version 1", () => { + it(`successfully executes transformation with credential lookup with valid key`, async () => { const versionId = randomID(); const inputData = require(`./data/${integration}_input_credentials.json`); @@ -1145,7 +1143,7 @@ describe("User transformation", () => { expect(output[0].transformedEvent.credentialValue).toEqual("value1"); }); - it(`Simple ${name} Test with credentials without key for codeVersion 1`, async () => { + it(`throws TypeError if the key provided for credential lookup is null or undefined`, async () => { const versionId = randomID(); const inputData = require(`./data/${integration}_input_credentials.json`); @@ -1174,7 +1172,7 @@ describe("User transformation", () => { expect(output[0].error).toMatch(/Key should be valid and defined/); }); - it(`Simple ${name} Test with credentials with multiple arguments for codeVersion 1`, async () => { + it(`allows user to pass multiple arguments to functions and performs lookup with first key passed`, async () => { const versionId = randomID(); const inputData = require(`./data/${integration}_input_credentials.json`); @@ -1203,7 +1201,7 @@ describe("User transformation", () => { expect(output[0].transformedEvent.credentialValue).toEqual("value1"); }); - it(`Simple ${name} Test with credentials with non string key for codeVersion 1`, async () => { + it(`allows user to pass valid key of any type and performs lookup accordingly`, async () => { const versionId = randomID(); const inputData = require(`./data/${integration}_input_credentials.json`); @@ -1238,7 +1236,7 @@ describe("User transformation", () => { expect(output[0].transformedEvent.credentialValueForObjkey).toBeUndefined(); }); - it(`Simple ${name} Test with credentials without value for codeVersion 1`, async () => { + it(`returns undefined when the key doesn't match any credential lookup`, async () => { const versionId = randomID(); const inputData = require(`./data/${integration}_input_credentials.json`); @@ -1267,7 +1265,7 @@ describe("User transformation", () => { expect(output[0].transformedEvent.credentialValue).toBeUndefined(); }); - it(`Simple ${name} Test without credentials for codeVersion 1`, async () => { + it(`returns undefined when the credentials are not passed in the request`, async () => { const versionId = randomID(); const inputData = require(`./data/${integration}_input.json`); @@ -1296,69 +1294,72 @@ describe("User transformation", () => { expect(output[0].transformedEvent.credentialValue).toBeUndefined(); }); - it(`Simple ${name} Batch Test with credentials for codeVersion 1`, async () => { - const versionId = randomID(); - - const inputData = require(`./data/${integration}_input_credentials.json`); + describe('Batch UserTransformation with Credentials', () => { + it(`successfully executes transformation with credential lookup with valid key`, async () => { + const versionId = randomID(); + + const inputData = require(`./data/${integration}_input_credentials.json`); + + const respBody = { + versionId: versionId, + codeVersion: "1", + name, + code: ` + export function transformBatch(events, metadata) { + events.forEach((event) => { + event.credentialValue1 = credential("key1"); + event.credentialValue2 = credential("key3"); + }); + return events; + } + ` + }; + fetch.mockResolvedValue({ + status: 200, + json: jest.fn().mockResolvedValue(respBody) + }); - const respBody = { - versionId: versionId, - codeVersion: "1", - name, - code: ` - export function transformBatch(events, metadata) { - events.forEach((event) => { - event.credentialValue1 = credential("key1"); - event.credentialValue2 = credential("key3"); - }); - return events; - } - ` - }; - fetch.mockResolvedValue({ - status: 200, - json: jest.fn().mockResolvedValue(respBody) + const output = await userTransformHandler(inputData, versionId, []); + expect(fetch).toHaveBeenCalledWith( + `https://api.rudderlabs.com/transformation/getByVersionId?versionId=${versionId}` + ); + expect(output[0].transformedEvent.credentialValue1).toEqual("value1"); + expect(output[0].transformedEvent.credentialValue2).toBeUndefined(); }); - const output = await userTransformHandler(inputData, versionId, []); - expect(fetch).toHaveBeenCalledWith( - `https://api.rudderlabs.com/transformation/getByVersionId?versionId=${versionId}` - ); - expect(output[0].transformedEvent.credentialValue1).toEqual("value1"); - expect(output[0].transformedEvent.credentialValue2).toBeUndefined(); - }); - - it(`Simple ${name} Batch Test with credentials without key for codeVersion 1`, async () => { - const versionId = randomID(); - - const inputData = require(`./data/${integration}_input_credentials.json`); + it(`throws TypeError if the key provided for credential lookup is null or undefined`, async () => { + const versionId = randomID(); + + const inputData = require(`./data/${integration}_input_credentials.json`); + + const respBody = { + versionId: versionId, + codeVersion: "1", + name, + code: ` + export function transformBatch(events, metadata) { + events.forEach((event) => { + event.credentialValue = credential(); + }); + return events; + } + ` + }; + fetch.mockResolvedValue({ + status: 200, + json: jest.fn().mockResolvedValue(respBody) + }); - const respBody = { - versionId: versionId, - codeVersion: "1", - name, - code: ` - export function transformBatch(events, metadata) { - events.forEach((event) => { - event.credentialValue = credential(); - }); - return events; - } - ` - }; - fetch.mockResolvedValue({ - status: 200, - json: jest.fn().mockResolvedValue(respBody) + const output = await userTransformHandler(inputData, versionId, []); + expect(fetch).toHaveBeenCalledWith( + `https://api.rudderlabs.com/transformation/getByVersionId?versionId=${versionId}` + ); + expect(output[0].error).toMatch(/Key should be valid and defined/); }); - const output = await userTransformHandler(inputData, versionId, []); - expect(fetch).toHaveBeenCalledWith( - `https://api.rudderlabs.com/transformation/getByVersionId?versionId=${versionId}` - ); - expect(output[0].error).toMatch(/Key should be valid and defined/); }); - it(`Simple ${name} Test with credentials for codeVersion 0`, async () => { + it(`throws error when credentials function is used with code version 0`, async () => { const versionId = randomID(); const inputData = require(`./data/${integration}_input_credentials.json`); @@ -1804,7 +1805,6 @@ describe("Python transformations", () => { trRevCode.versionId, [], trRevCode, - [], true ); expect(output).toEqual(outputData); @@ -1850,7 +1850,6 @@ describe("Python transformations", () => { trRevCode.versionId, Object.values(importNameLibraryVersionIdsMap), trRevCode, - [], true ); expect(output).toEqual(outputData); @@ -1892,7 +1891,6 @@ describe("Python transformations", () => { trRevCode.versionId, Object.values(importNameLibraryVersionIdsMap), trRevCode, - [], true ); expect(output).toEqual(outputData); diff --git a/test/__tests__/user_transformation_errors.test.js b/test/__tests__/user_transformation_errors.test.js index eb171a1e4d..c2a99ce09d 100644 --- a/test/__tests__/user_transformation_errors.test.js +++ b/test/__tests__/user_transformation_errors.test.js @@ -47,7 +47,6 @@ describe("JS Transformation Error Tests", () => { versionId, [], trRevCode, - null, true, ); @@ -66,7 +65,6 @@ describe("JS Transformation Error Tests", () => { versionId, [], trRevCode, - null, true, ); @@ -104,7 +102,6 @@ describe("JS Transformation Error Tests", () => { versionId, [libVid], trRevCode, - null, true, ); @@ -143,7 +140,6 @@ describe("JS Transformation Error Tests", () => { versionId, [libVid], trRevCode, - null, true, ); @@ -164,7 +160,6 @@ describe("JS Transformation Error Tests", () => { versionId, [], trRevCode, - null, true, ); @@ -186,7 +181,6 @@ describe("JS Transformation Error Tests", () => { versionId, [], trRevCode, - null, true, ); @@ -224,7 +218,6 @@ describe("JS Transformation Error Tests", () => { versionId, [libVid], trRevCode, - null, true, ); @@ -263,7 +256,6 @@ describe("JS Transformation Error Tests", () => { versionId, [libVid], trRevCode, - null, true, ); diff --git a/test/__tests__/user_transformation_fetch.test.js b/test/__tests__/user_transformation_fetch.test.js index 5bf9d54935..e37b42ea1e 100644 --- a/test/__tests__/user_transformation_fetch.test.js +++ b/test/__tests__/user_transformation_fetch.test.js @@ -43,7 +43,7 @@ describe("User transformation fetch tests", () => { ` }; - const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true); + const output = await userTransformHandler(inputData, versionId, [], trRevCode, true); expect(output).toEqual({ logs: [], transformedEvents: expectedData.map(ev => (ev.transformedEvent)) @@ -67,7 +67,7 @@ describe("User transformation fetch tests", () => { }; const errMsg = "ERROR"; - const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true); + const output = await userTransformHandler(inputData, versionId, [], trRevCode, true); expect(mockResolver).toHaveBeenCalledTimes(0); output.transformedEvents.forEach(ev => { @@ -92,7 +92,7 @@ describe("User transformation fetch tests", () => { const errMsg = "ERROR"; mockResolver.mockResolvedValue([ '127.0.0.1' ]); - const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true); + const output = await userTransformHandler(inputData, versionId, [], trRevCode, true); expect(mockResolver).toHaveBeenCalledTimes(inputData.length); expect(mockResolver).toHaveBeenCalledWith('abc.xyz.com'); @@ -122,7 +122,7 @@ describe("User transformation fetch tests", () => { }; const errMsg = "invalid url, localhost requests are not allowed"; - const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true); + const output = await userTransformHandler(inputData, versionId, [], trRevCode, true); expect(mockResolver).toHaveBeenCalledTimes(0); output.transformedEvents.forEach(ev => { @@ -152,7 +152,7 @@ describe("User transformation fetch tests", () => { const errMsg = "request to https://abc.xyz.com/dummyUrl failed, reason: Invalid IP address: unable to resolve IP address for abc.xyz.com"; mockResolver.mockRejectedValue('invalid host'); - const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true); + const output = await userTransformHandler(inputData, versionId, [], trRevCode, true); expect(mockResolver).toHaveBeenCalledTimes(inputData.length); expect(mockResolver).toHaveBeenCalledWith('abc.xyz.com'); @@ -182,7 +182,7 @@ describe("User transformation fetch tests", () => { }; const errMsg = "invalid protocol, only http and https are supported"; - const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true); + const output = await userTransformHandler(inputData, versionId, [], trRevCode, true); output.transformedEvents.forEach(ev => { expect(ev.errMsg).toEqual(errMsg); @@ -204,7 +204,7 @@ describe("User transformation fetch tests", () => { versionId, }; - const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true); + const output = await userTransformHandler(inputData, versionId, [], trRevCode, true); expect(output).toEqual({ logs: [], transformedEvents: expectedData.map(ev => (ev.transformedEvent)) @@ -226,7 +226,7 @@ describe("User transformation fetch tests", () => { }; const errMsg = "ERROR"; - const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true); + const output = await userTransformHandler(inputData, versionId, [], trRevCode, true); expect(mockResolver).toHaveBeenCalledTimes(0); output.transformedEvents.forEach(ev => { @@ -250,7 +250,7 @@ describe("User transformation fetch tests", () => { const errMsg = "ERROR"; mockResolver.mockRejectedValue('invalid host'); - const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true); + const output = await userTransformHandler(inputData, versionId, [], trRevCode, true); expect(mockResolver).toHaveBeenCalledTimes(inputData.length); expect(mockResolver).toHaveBeenCalledWith('abc.xyz.com'); @@ -279,7 +279,7 @@ describe("User transformation fetch tests", () => { }; const errMsg = "invalid url, localhost requests are not allowed"; - const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true); + const output = await userTransformHandler(inputData, versionId, [], trRevCode, true); expect(mockResolver).toHaveBeenCalledTimes(0); output.transformedEvents.forEach(ev => { @@ -308,7 +308,7 @@ describe("User transformation fetch tests", () => { const errMsg = "request to https://abc.xyz.com/dummyUrl failed, reason: Invalid IP address: cannot use 127.0.0.1 as IP address"; mockResolver.mockResolvedValue(['3.122.122.122', '127.0.0.1']); - const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true); + const output = await userTransformHandler(inputData, versionId, [], trRevCode, true); expect(mockResolver).toHaveBeenCalledTimes(inputData.length); expect(mockResolver).toHaveBeenCalledWith('abc.xyz.com'); @@ -337,7 +337,7 @@ describe("User transformation fetch tests", () => { }; const errMsg = "fetch url is required"; - const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true); + const output = await userTransformHandler(inputData, versionId, [], trRevCode, true); output.transformedEvents.forEach(ev => { expect(ev.errMsg).toEqual(errMsg); From 7b0ea8040213b4bf7b73306743bd1414dc8e5b46 Mon Sep 17 00:00:00 2001 From: kanishkkatara Date: Wed, 19 Jun 2024 16:14:02 +0530 Subject: [PATCH 20/20] refactor: change in credentials function validation code --- src/util/ivmFactory.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/util/ivmFactory.js b/src/util/ivmFactory.js index 84eb52f0c8..44beff01dc 100644 --- a/src/util/ivmFactory.js +++ b/src/util/ivmFactory.js @@ -254,10 +254,15 @@ async function createIvm( await jail.set('_credential', function (key) { if (isNil(credentials) || !isObject(credentials)) { - logger.error('Error fetching credentials map', versionId); + logger.error( + `Error fetching credentials map for transformationID: ${transformationId} and workspaceId: ${workspaceId}`, + ); stats.increment('credential_error_total', { transformationId, workspaceId }); return undefined; } + if (key === null || key === undefined) { + throw new TypeError('Key should be valid and defined'); + } return credentials[key]; }); @@ -343,9 +348,6 @@ async function createIvm( delete _credential; global.credential = function(...args) { const key = args[0]; - if (key === null || key === undefined) { - throw new TypeError('Key should be valid and defined'); - } return credential(new ivm.ExternalCopy(key).copyInto()); };