rule could not be empty string and remove online role configuration w…

…hen role is not provided in yml (alibaba#184) * fix alibaba#182: rule could not be empty string and remove online role configuration when role is not provided in yml
pythons · Apr 12, 2019 · 118edf4 · 118edf4
1 parent 2beed95
commit 118edf4
Show file tree

Hide file tree

Showing 5 changed files with 83 additions and 56 deletions.
diff --git 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);
 }
diff --git 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
@@ -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
@@ -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,21 +63,21 @@ 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');
     assert.calledWith(ram.makeRole,'aliyunfcgeneratedrole-fc',false);
     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' }
+      });
+  });
 });
diff --git 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', () => {