AWS: Fix stage name validation timing and allow hyphen

lib/classes/Variables.js

@@ -10,6 +10,8 @@ const fse = require('../utils/fs/fse');
 const logWarning = require('./Error').logWarning;
 const PromiseTracker = require('./PromiseTracker');
 
+const validAPIGatewayStageNamePattern = /^[-_a-zA-Z0-9]+$/;
+
 /**
  * Maintainer's notes:
  *
@@ -108,6 +110,19 @@ class Variables {
           this.populateValue(config.value, true) // populate
             .then(populated => _.assign(config, { populated })));
         return this.assignProperties(provider, prepopulations);
+      }).then(() => {
+        const stage = provider.getStage();
+        this.serverless.service.getAllFunctions().forEach(funcName => {
+          _.forEach(this.serverless.service.getAllEventsInFunction(funcName), event => {
+            if (_.has(event, 'http') && !validAPIGatewayStageNamePattern.test(stage)) {
+              throw new this.serverless.classes.Error([
+                `Invalid stage name ${stage}:`,
+                'it should contains only [-_a-zA-Z0-9] for AWS provider if http event are present',
+                'according to API Gateway limitation.',
+              ].join(' '));
+            }
+          });
+        });
       });
     }
     return BbPromise.resolve();

lib/classes/Variables.test.js

@@ -207,6 +207,113 @@ describe('Variables', () => {
         });
       });
     });
+    describe('stage name validation', () => {
+      it(`should not throw an error if http event is absent and 
+            stage contains only alphanumeric, underscore and hyphen`, () => {
+        const serverlessYml = {
+          service: 'new-service',
+          provider: {
+            name: 'aws',
+            stage: 'xyz-101_abc-123',
+          },
+          functions: {
+            first: {
+              events: [],
+            },
+          },
+        };
+        serverless.service = new serverless.classes.Service(serverless, serverlessYml);
+
+        return serverless.variables.populateService().should.be.fullfilled;
+      });
+
+      it(`should not throw an error after variable population if http event is present and
+            the populated stage contains only alphanumeric, underscore and hyphen`, () => {
+        const serverlessYml = {
+          service: 'new-service',
+          provider: {
+            name: 'aws',
+            stage: '${opt:stage, "default-stage"}',
+          },
+          functions: {
+            first: {
+              events: [
+                {
+                  http: {
+                    path: 'foo',
+                    method: 'GET',
+                  },
+                },
+              ],
+            },
+          },
+        };
+        serverless.service = new serverless.classes.Service(serverless, serverlessYml);
+
+        return serverless.variables.populateService().should.be.fullfilled;
+      });
+
+      it('should throw an error if http event is present and stage contains invalid chars', () => {
+        const serverlessYml = {
+          service: 'new-service',
+          provider: {
+            name: 'aws',
+            stage: 'my@stage',
+          },
+          functions: {
+            first: {
+              events: [
+                {
+                  http: {
+                    path: 'foo',
+                    method: 'GET',
+                  },
+                },
+              ],
+            },
+          },
+        };
+        serverless.service = new serverless.classes.Service(serverless, serverlessYml);
+
+        return serverless.variables.populateService().should.be
+          .rejectedWith(serverless.classes.Error, [
+            'Invalid stage name my@stage: it should contains only [-_a-zA-Z0-9]',
+            'for AWS provider if http event are present',
+            'according to API Gateway limitation.',
+          ].join(' '));
+      });
+
+      it(`should throw an error after variable population
+            if http event is present and stage contains hyphen`, () => {
+        const serverlessYml = {
+          service: 'new-service',
+          provider: {
+            name: 'aws',
+            stage: '${opt:stage, "default:stage"}',
+          },
+          functions: {
+            first: {
+              events: [
+                {
+                  http: {
+                    path: 'foo',
+                    method: 'GET',
+                  },
+                },
+              ],
+            },
+          },
+        };
+        serverless.service = new serverless.classes.Service(serverless, serverlessYml);
+
+        return serverless.variables.populateService().should.be
+          .rejectedWith(serverless.classes.Error, [
+            'Invalid stage name default:stage: it should contains only [-_a-zA-Z0-9]',
+            'for AWS provider if http event are present',
+            'according to API Gateway limitation.',
+          ].join(' '));
+      });
+    });
   });
 
   describe('#getProperties', () => {

lib/plugins/aws/provider/awsProvider.js

@@ -19,8 +19,6 @@ const constants = {
   providerName: 'aws',
 };
 
-const validAPIGatewayStageNamePattern = /^[a-zA-Z0-9_]+$/;
-
 PromiseQueue.configure(BbPromise.Promise);
 
 const impl = {
@@ -201,19 +199,6 @@ class AwsProvider {
         }
       }
     }
-
-    const stage = this.getStage();
-    this.serverless.service.getAllFunctions().forEach(funcName => {
-      _.forEach(this.serverless.service.getAllEventsInFunction(funcName), event => {
-        if (_.has(event, 'http') && !validAPIGatewayStageNamePattern.test(stage)) {
-          throw new Error([
-            `Invalid stage name ${stage}: `,
-            'it should contains only [a-zA-Z0-9] for AWS provider if http event are present ',
-            'since API Gateway stage name cannot contains hyphens.',
-          ].join(''));
-        }
-      });
-    });
   }
 
   /**

lib/plugins/aws/provider/awsProvider.test.js

@@ -90,51 +90,43 @@ describe('AwsProvider', () => {
       delete process.env.AWS_CLIENT_TIMEOUT;
     });
 
-    describe('validation on construction', () => {
-      it('should not throw an error if stage name contains only alphanumeric', () => {
-        const config = {
-          stage: 'configStage',
-        };
-        serverless = new Serverless(config);
-        expect(() => new AwsProvider(serverless, config)).to.not.throw(Error);
-      });
-
-      it('should not throw an error if stage contains hyphen but http events are absent', () => {
-        const config = {
-          stage: 'config-stage',
-        };
-        serverless = new Serverless(config);
-        expect(() => new AwsProvider(serverless, config)).to.not.throw(Error);
-      });
-
-      it('should throw an error if stage contains hyphen and http events are present', () => {
-        const config = {
-          stage: 'config-stage',
-        };
-        serverless = new Serverless(config);
+    describe('stage name validation', () => {
+      const stages = [
+        'myStage',
+        'my-stage',
+        'my_stage',
+        '${opt:stage, \'prod\'}',
+      ];
+      stages.forEach(stage => {
+        it(`should not throw an error before variable population 
+            even if http event is present and stage is ${stage}`, () => {
+          const config = {
+            stage,
+          };
+          serverless = new Serverless(config);
 
-        const serverlessYml = {
-          service: 'new-service',
-          provider: {
-            name: 'aws',
-            stage: 'config-stage',
-          },
-          functions: {
-            first: {
-              events: [
-                {
-                  http: {
-                    path: 'foo',
-                    method: 'GET',
+          const serverlessYml = {
+            service: 'new-service',
+            provider: {
+              name: 'aws',
+              stage,
+            },
+            functions: {
+              first: {
+                events: [
+                  {
+                    http: {
+                      path: 'foo',
+                      method: 'GET',
+                    },
                   },
-                },
-              ],
+                ],
+              },
             },
-          },
-        };
-        serverless.service = new serverless.classes.Service(serverless, serverlessYml);
-
-        expect(() => new AwsProvider(serverless, config)).to.throw(Error);
+          };
+          serverless.service = new serverless.classes.Service(serverless, serverlessYml);
+          expect(() => new AwsProvider(serverless, config)).to.not.throw(Error);
+        });
       });
     });