From 67e9277120e39402b32eac55ffa34a0053b6af61 Mon Sep 17 00:00:00 2001 From: kanishkkatara Date: Tue, 18 Jun 2024 12:19:20 +0530 Subject: [PATCH] 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);