From 118edf44f2e03b4c5475cbae0a1c15e739cc897a Mon Sep 17 00:00:00 2001 From: coco-super <48934955+coco-super@users.noreply.github.com> Date: Fri, 12 Apr 2019 16:53:09 +0800 Subject: [PATCH] rule could not be empty string and remove online role configuration when role is not provided in yml (#184) * fix #182: rule could not be empty string and remove online role configuration when role is not provided in yml --- bin/fun-install.js | 2 +- lib/deploy/deploy-by-tpl.js | 24 ++++++------ lib/deploy/deploy-support.js | 10 ++--- test/deploy/deploy-by-tpl.test.js | 63 ++++++++++++++++++++++-------- test/deploy/deploy-support.test.js | 40 +++++++++---------- 5 files changed, 83 insertions(+), 56 deletions(-) diff --git a/bin/fun-install.js b/bin/fun-install.js index 9641143c5..1b6f7ef34 100644 --- a/bin/fun-install.js +++ b/bin/fun-install.js @@ -66,5 +66,5 @@ if (!program.args.length) { // fix windows not auto exit bug after docker operation process.exit(0); }) - .catch(handler); + .catch(handler); } \ No newline at end of file diff --git a/lib/deploy/deploy-by-tpl.js b/lib/deploy/deploy-by-tpl.js index 320d28831..76a21570f 100644 --- a/lib/deploy/deploy-by-tpl.js +++ b/lib/deploy/deploy-by-tpl.js @@ -90,10 +90,10 @@ async function deployTriggers(serviceName, functionName, events) { console.log(`\t\tWaiting for ${yellow(triggerDefinition.Type)} trigger ${triggerName} to be deployed...`); await deployTrigger(serviceName, functionName, triggerName, triggerDefinition); - + await displayTriggerInfo(serviceName, functionName, triggerName, triggerDefinition); - console.log(green(`\t\tfunction ${triggerName} deploy success`)); + console.log(green(`\t\tfunction ${triggerName} deploy success`)); } } } @@ -160,33 +160,32 @@ async function deployService(baseDir, serviceName, serviceRes) { const properties = (serviceRes.Properties || {}); const roleArn = properties.Role; const internetAccess = 'InternetAccess' in properties ? properties.InternetAccess : null; - + const policies = properties.Policies; const vpcConfig = properties.VpcConfig || {}; const logConfig = properties.LogConfig || {}; const nasConfig = properties.NasConfig || {}; - + let roleName; let createRoleIfNotExist; let role; const profile = await getProfile(); - if (roleArn) { + if (roleArn === undefined || roleArn === null) { + roleName = `aliyunfcgeneratedrole-${profile.defaultRegion}-${serviceName}`; + roleName = normalizeRoleOrPoliceName(roleName); + createRoleIfNotExist = true; + } else { try { roleName = extractFcRole(roleArn); } catch (ex) { throw new Error('The role you provided is not correct. You must provide the correct role arn.'); } - createRoleIfNotExist = false; - } else { - roleName = `aliyunfcgeneratedrole-${profile.defaultRegion}-${serviceName}`; - roleName = normalizeRoleOrPoliceName(roleName); - createRoleIfNotExist = true; } - if(roleArn || ( policies || !_.isEmpty(vpcConfig) || !_.isEmpty(logConfig))){ + if (roleArn || (policies || !_.isEmpty(vpcConfig) || !_.isEmpty(logConfig))) { // create role role = await makeRole(roleName, createRoleIfNotExist); } @@ -216,10 +215,9 @@ async function deployService(baseDir, serviceName, serviceRes) { } else if (logConfig.LogStore || logConfig.Project) { throw new Error('LogStore and Project must both exist'); } - await makeService({ serviceName, - role: ((role || {}).Role || {}).Arn, + role: ((role || {}).Role || {}).Arn || '', internetAccess, description: (serviceRes.Properties || {}).Description, logConfig, diff --git a/lib/deploy/deploy-support.js b/lib/deploy/deploy-support.js index 33486e438..3d86af33e 100644 --- a/lib/deploy/deploy-support.js +++ b/lib/deploy/deploy-support.js @@ -440,12 +440,12 @@ async function makeFunction(baseDir, { environmentVariables: addEnv(environmentVariables) }; -for (let i in params.environmentVariables) { - if(!isNaN(params.environmentVariables[i])){ - debug(`the value in environmentVariables:${params.environmentVariables[i]} cast String Done`); - params.environmentVariables[i] = params.environmentVariables[i]+ ""; + for (let i in params.environmentVariables) { + if(!isNaN(params.environmentVariables[i])){ + debug(`the value in environmentVariables:${params.environmentVariables[i]} cast String Done`); + params.environmentVariables[i] = params.environmentVariables[i]+ ''; + } } - } if (!fn) { // create params['functionName'] = functionName; diff --git a/test/deploy/deploy-by-tpl.test.js b/test/deploy/deploy-by-tpl.test.js index b88a551bb..fe6067a12 100644 --- a/test/deploy/deploy-by-tpl.test.js +++ b/test/deploy/deploy-by-tpl.test.js @@ -14,7 +14,7 @@ describe('deploy service role ',() => { beforeEach(() => { Object.keys(deploySupport).forEach(m => { - sandbox.stub(deploySupport, m).resolves({}); + sandbox.stub(deploySupport, m).resolves({}); }); Object.keys(ram).forEach(m => { @@ -63,13 +63,13 @@ describe('deploy service role ',() => { assert.calledWith(ram.makeRole,'',true); assert.notCalled(ram.makePolicy); assert.notCalled(ram.makeAndAttachPolicy); - }) + }); it('only log', async() =>{ await deploy('sls_trigger_demo'); assert.calledWith(ram.makeRole,'',true); assert.calledWith(ram.attachPolicyToRole,'AliyunFCInvocationAccess','AliyunFcGeneratedApiGatewayRole'); assert.notCalled(ram.makePolicy); - }) + }); it('only role', async() =>{ await deploy('service_role'); @@ -77,7 +77,7 @@ describe('deploy service role ',() => { assert.notCalled(ram.makeAndAttachPolicy); assert.notCalled(ram.attachPolicyToRole); assert.notCalled(ram.makePolicy); - }) + }); }); describe('deploy', () => { @@ -128,7 +128,7 @@ describe('deploy', () => { description: undefined, internetAccess: null, logConfig: {}, - role: undefined, + role: '', serviceName: 'MyService', vpcConfig: {}, nasConfig: {} @@ -157,7 +157,7 @@ describe('deploy', () => { description: 'fc test', internetAccess: null, logConfig: {}, - role: undefined, + role: '', serviceName: 'fc', vpcConfig: {}, nasConfig: {} @@ -185,7 +185,7 @@ describe('deploy', () => { description: 'java demo', internetAccess: null, logConfig: {}, - role: undefined, + role: '', serviceName: 'java', vpcConfig: {}, nasConfig: {} @@ -269,7 +269,7 @@ describe('deploy', () => { description: 'fc test', internetAccess: null, logConfig: {}, - role: undefined, + role: '', serviceName: 'fc', vpcConfig: {}, nasConfig: {}, @@ -331,7 +331,7 @@ describe('deploy', () => { description: 'Stream trigger for TableStore', internetAccess: null, logConfig: {}, - role: undefined, + role: '', serviceName: 'test-tableStore-service', vpcConfig: {}, nasConfig: {} @@ -369,7 +369,7 @@ describe('deploy', () => { description: 'sls test', internetAccess: null, logConfig: {}, - role: undefined, + role: '', serviceName: 'log-compute', vpcConfig: {}, nasConfig: {} @@ -409,7 +409,7 @@ describe('deploy', () => { description: 'rds trigger test', internetAccess: null, logConfig: {}, - role: undefined, + role: '', serviceName: 'rds-service', vpcConfig: {}, nasConfig: {} @@ -450,7 +450,7 @@ describe('deploy', () => { description: 'MnsTopic trigger test', internetAccess: null, logConfig: {}, - role: undefined, + role: '', serviceName: 'mnsTopic-service', vpcConfig: {}, nasConfig: {} @@ -490,7 +490,7 @@ describe('deploy', () => { description: 'python demo', internetAccess: null, logConfig: {}, - role: undefined, + role: '', serviceName: 'pythondemo', vpcConfig: {}, nasConfig: {} @@ -542,7 +542,7 @@ describe('deploy', () => { description: 'Module as a service', internetAccess: null, logConfig: {}, - role: undefined, + role: '', serviceName: 'maas', vpcConfig: {}, nasConfig: {} @@ -593,7 +593,7 @@ describe('deploy', () => { description: undefined, internetAccess: null, logConfig: {}, - role: undefined, + role: '', serviceName: 'MyService', vpcConfig: {}, nasConfig: {} @@ -631,7 +631,7 @@ describe('deploy', () => { description: 'wechat demo', internetAccess: null, logConfig: {}, - role: undefined, + role: '', serviceName: 'wechat', vpcConfig: {}, nasConfig: {} @@ -749,7 +749,7 @@ describe('deploy', () => { description: 'initializer demo', internetAccess: null, logConfig: {}, - role: undefined, + role: '', serviceName: 'initializerdemo', vpcConfig: {}, nasConfig: {} @@ -794,4 +794,33 @@ describe('deploy', () => { resultConfig: { failResultSample: undefined, resultSample: undefined, resultType: undefined }, }); }); + + it('deploy service role', async () => { + await deploy('service_role'); + + assert.calledWith(deploySupport.makeService, { + description: 'local invoke demo', + internetAccess: null, + logConfig: {}, + role: 'acs:ram::123:role/aliyunfcgeneratedrole-fc', + serviceName: 'localdemo', + vpcConfig: {}, + nasConfig: {} + }); + + assert.calledWith(deploySupport.makeFunction, + path.join(process.cwd(), 'examples', 'service_role'), { + codeUri: 'nodejs6', + description: 'Hello world with nodejs6!', + functionName: 'nodejs6', + handler: 'index.handler', + initializer: undefined, + memorySize: undefined, + runtime: 'nodejs6', + initializationTimeout: undefined, + serviceName: 'localdemo', + timeout: undefined, + environmentVariables: { StringTypeValue1: 123, StringTypeValue2: 'test' } + }); + }); }); \ No newline at end of file diff --git a/test/deploy/deploy-support.test.js b/test/deploy/deploy-support.test.js index 8db49aad0..b1a65b297 100644 --- a/test/deploy/deploy-support.test.js +++ b/test/deploy/deploy-support.test.js @@ -328,13 +328,13 @@ describe('Incorrect environmental variables', ()=> { }); }); - afterEach(() => { - sandbox.restore(); - restoreProcess(); + afterEach(() => { + sandbox.restore(); + restoreProcess(); }); it('should cast env value to String', async ()=> { - await deploySupport.makeFunction(path.join('examples', 'local'),{ + await deploySupport.makeFunction(path.join('examples', 'local'),{ serviceName : 'localdemo', functionName : 'nodejs6', description : 'Hello world with nodejs6!', @@ -345,33 +345,33 @@ describe('Incorrect environmental variables', ()=> { memorySize : 128, runtime :'nodejs6', codeUri : path.join('examples', 'local','nodejs6'), - environmentVariables : {"StringTypeValue1":123,"StringTypeValue2":"test"} + environmentVariables : {'StringTypeValue1':123,'StringTypeValue2':'test'} }); assert.calledWith( - FC.prototype.updateFunction, - 'localdemo', - 'nodejs6', - { - description: "Hello world with nodejs6!", - handler: "index.handler", + FC.prototype.updateFunction, + 'localdemo', + 'nodejs6', + { + description: 'Hello world with nodejs6!', + handler: 'index.handler', initializer: null, timeout: 3, initializationTimeout: 3, memorySize: 128, - runtime: "nodejs6", + runtime: 'nodejs6', code: { - zipFile: '' + zipFile: '' }, environmentVariables: { - StringTypeValue1: "123", - StringTypeValue2: "test", - LD_LIBRARY_PATH: "/code/.fun/root/usr/lib:/code/.fun/root/usr/lib/x86_64-linux-gnu:/code:/code/lib:/usr/local/lib", - PATH: "/code/.fun/root/usr/local/bin:/code/.fun/root/usr/local/sbin:/code/.fun/root/usr/bin:/code/.fun/root/usr/sbin:/code/.fun/root/sbin:/code/.fun/root/bin:/code/.fun/python/bin:/usr/local/bin:/usr/local/sbin:/usr/bin:/usr/sbin:/sbin:/bin", - PYTHONUSERBASE: "/code/.fun/python" + StringTypeValue1: '123', + StringTypeValue2: 'test', + LD_LIBRARY_PATH: '/code/.fun/root/usr/lib:/code/.fun/root/usr/lib/x86_64-linux-gnu:/code:/code/lib:/usr/local/lib', + PATH: '/code/.fun/root/usr/local/bin:/code/.fun/root/usr/local/sbin:/code/.fun/root/usr/bin:/code/.fun/root/usr/sbin:/code/.fun/root/sbin:/code/.fun/root/bin:/code/.fun/python/bin:/usr/local/bin:/usr/local/sbin:/usr/bin:/usr/sbin:/sbin:/bin', + PYTHONUSERBASE: '/code/.fun/python' } - }); - }) + }); + }); }); describe('make invocation role', () => {