State machine definition validation using asl-validator #90

ChristopheBougere · 2017-12-09T12:09:05Z

Hi !

I have added a validation command for the state machine definition. All the logic is handled in asl-validator package.

It can help saving a lot of time avoiding failed deployment because of a wrong state machine definition,

You can use it like this: sls validate-state-machines [--name <stepfunctionname>].

I also plan to add it in the deploy command, but I'd like it to happen at the beginning (before packaging everything). Let me know what you think about it, and if you have some thoughts about when exactly running validation.

Some output examples:

Valid definition

$ serverless validate-state-machine -vServerless: Validating state machines definition
Serverless: ✓ yourParallelMachine definition is valid

State machine does not exists

$ serverless validate-state-machine -n yourParallelMachines
Serverless: Validating state machines definition
 
  Serverless Error ---------------------------------------
 
  stateMachine "yourParallelMachines" doesn't exist in this Service

Invalid defintion (missing StartAt property)

$ serverless validate-state-machine
Serverless: Validating state machines definition
Serverless: ✕ yourParallelMachine definition is invalid:
[{"keyword":"required","dataPath":"","schemaPath":"#/required","params":{"missingProperty":"StartAt"},"message":"should have required property 'StartAt'"}]
 
  Serverless Error ---------------------------------------
 
  At least one of your state machine definition is invalid.

…sing asl-validator

coveralls · 2017-12-09T12:11:34Z

Coverage decreased (-0.2%) to 99.793% when pulling cc205f2 on ChristopheBougere:validate-state-machine-2 into 7873412 on horike37:master.

ChristopheBougere · 2017-12-09T14:20:44Z

Tests in Node 4.4 and 5.5 are failing however I don't think it is related to this PR (otherwise error message is not really helpful...)
Tests and lint pass in Node 6.2

Stack trace


SyntaxError: Unexpected token {

    at exports.runInThisContext (vm.js:53:16)

    at Module._compile (module.js:373:25)

    at Object.Module._extensions.(anonymous function) [as .js] (/home/travis/build/horike37/serverless-step-functions/node_modules/istanbul/lib/hook.js:107:24)

    at Module.load (module.js:343:32)

    at Function.Module._load (module.js:300:12)

    at Module.require (module.js:353:17)

    at require (internal/module.js:12:17)

    at Object.<anonymous> (/home/travis/build/horike37/serverless-step-functions/lib/deploy/events/apiGateway/cors.test.js:6:33)

    at Module._compile (module.js:409:26)

    at Object.Module._extensions..js (module.js:416:10)

    at Object.Module._extensions.(anonymous function) [as .js] (/home/travis/build/horike37/serverless-step-functions/node_modules/istanbul/lib/hook.js:109:37)

    at Module.load (module.js:343:32)

    at Function.Module._load (module.js:300:12)

    at Module.require (module.js:353:17)

    at require (internal/module.js:12:17)

    at /home/travis/build/horike37/serverless-step-functions/node_modules/mocha/lib/mocha.js:231:27

    at Array.forEach (native)

    at Mocha.loadFiles (/home/travis/build/horike37/serverless-step-functions/node_modules/mocha/lib/mocha.js:228:14)

    at Mocha.run (/home/travis/build/horike37/serverless-step-functions/node_modules/mocha/lib/mocha.js:514:10)

    at Object.<anonymous> (/home/travis/build/horike37/serverless-step-functions/node_modules/mocha/bin/_mocha:480:18)

    at Module._compile (module.js:409:26)

    at Object.Module._extensions..js (module.js:416:10)

    at Object.Module._extensions.(anonymous function) [as .js] (/home/travis/build/horike37/serverless-step-functions/node_modules/istanbul/lib/hook.js:109:37)

    at Module.load (module.js:343:32)

    at Function.Module._load (module.js:300:12)

    at Function.Module.runMain (module.js:441:10)

    at runFn (/home/travis/build/horike37/serverless-step-functions/node_modules/istanbul/lib/command/common/run-with-cover.js:122:16)

    at /home/travis/build/horike37/serverless-step-functions/node_modules/istanbul/lib/command/common/run-with-cover.js:251:17

    at /home/travis/build/horike37/serverless-step-functions/node_modules/istanbul/lib/util/file-matcher.js:68:16

    at /home/travis/build/horike37/serverless-step-functions/node_modules/istanbul/node_modules/async/lib/async.js:52:16

    at /home/travis/build/horike37/serverless-step-functions/node_modules/istanbul/node_modules/async/lib/async.js:361:13

    at /home/travis/build/horike37/serverless-step-functions/node_modules/istanbul/node_modules/async/lib/async.js:52:16

    at done (/home/travis/build/horike37/serverless-step-functions/node_modules/istanbul/node_modules/async/lib/async.js:246:17)

    at /home/travis/build/horike37/serverless-step-functions/node_modules/istanbul/node_modules/async/lib/async.js:44:16

    at /home/travis/build/horike37/serverless-step-functions/node_modules/istanbul/node_modules/async/lib/async.js:358:17

    at LOOP (fs.js:1530:14)

    at nextTickCallbackWith0Args (node.js:420:9)

    at process._tickCallback (node.js:349:13)

Let me know if I'm wrong

horike37 · 2017-12-12T05:09:12Z

Thank you for sending PR @ChristopheBougere 👍
That'a great functionality 💯 It definitely makes easier to validate statemachines!!

However, I'm not sure whether validate-state-machine is the best name for the option.
IMO, it would be good to serverless stepf validate or serverless validate stepf since I feel validate-state-machine is too long name a little bit. What do you think?

coveralls · 2017-12-12T09:18:58Z

Coverage decreased (-0.2%) to 99.793% when pulling 980aeff on ChristopheBougere:validate-state-machine-2 into 7873412 on horike37:master.

ChristopheBougere · 2017-12-12T09:25:35Z

@horike37 Yep I first wanted to avoid the ambiguity between step function and state machine, but I think serverless validate stepf will be good :)
Thanks for your feedback, I pushed the change. Do you have an idea for the CI build ? 😕

horike37 · 2017-12-12T17:20:47Z

@ChristopheBougere

Do you have an idea for the CI build ?

I found the cause. asl-validator does not support node v4 and v5.
Personally, there is not a problem to abandon support v4 and v5 for the plugin. However, it would be breaking change for some people who use such an old environment.

Any ideas for a workaround?

ChristopheBougere · 2017-12-12T17:42:44Z

@horike37 Thanks, I thought asl-validator was compatible with node 4. However, only the API part is compatible, not the CLI part (see https://github.com/airware/asl-validator/blob/master/bin/asl-validator.js#L6).
I will update it to be compatible and let you know :)

horike37 · 2017-12-12T17:48:16Z

I will update it to be compatible and let you know :)

Sounds great 😄
Let me know if you need any helps or questions.

coveralls · 2017-12-12T18:02:09Z

Coverage decreased (-0.2%) to 99.793% when pulling d4041d7 on ChristopheBougere:validate-state-machine-2 into 7873412 on horike37:master.

coveralls · 2017-12-12T18:11:56Z

Coverage decreased (-0.2%) to 99.793% when pulling 507aaf0 on ChristopheBougere:validate-state-machine-2 into 7873412 on horike37:master.

ChristopheBougere · 2017-12-12T18:22:50Z

@horike37 Tests are passing now :)

horike37 · 2017-12-13T01:18:02Z

Awesome @ChristopheBougere!
Great to see working fine even old node version.

Then I tested it locally but an invalid error was detected on even right setting 🤔
Here is serverless.yml I tested. sls validate stepf was failed but sls deploy was successful.

Any ideas why?

service: step-func

provider:
  name: aws
  runtime: nodejs6.10

functions:
  hello:
    handler: handler.hello

stepFunctions:
  stateMachines:
    statemachine1:
      name: ${self:service}-${opt:stage}-statemachine1
      events:
        - http:
            path: /hello
            method: post
      definition:
        Comment: "A Hello World example of the Amazon States Language using an AWS Lambda Function"
        StartAt: HelloWorld1
        States:
          HelloWorld1:
            Type: Task
            Resource: arn:aws:lambda:#{AWS::Region}:#{AWS::AccountId}:function:${self:service}-${opt:stage}-hello
            End: true


plugins:
  - serverless-step-functions
  - serverless-pseudo-parameters

ChristopheBougere · 2017-12-13T08:56:23Z

@horike37 This looks like an incompatibility with serverless-pseudo-parameters. asl-validator is validating task Resource ARN too with a regex, and the pseudo parameters are not replaced I guess.
I'll have a closer look, thanks for catching this !

ChristopheBougere · 2017-12-13T15:01:32Z

https://github.com/svdgraaf/serverless-pseudo-parameters/blob/master/lib/index.js#L12
In serverless-pseudo-parameters the hook for replacement is only done for before:deploy:deploy, while we would also need before:validate:stepf:validate.

By the way, I think that sls invoke stepf currently work because you are not using the state machine definition when invoking, but only the step function name. I am right?

horike37 · 2017-12-14T04:30:17Z

Thank you for taking a look into it @ChristopheBougere 😄
That being said, We need to send a PR which includes a fix which the replacement is also done for before:validate:stepf:validate?

I think that sls invoke stepf currently work because you are not using the state machine definition >when invoking, but only the step function name. I am right?

Yup, you are right.
sls invoke stepf invokes a deployed statemachine, so it doesn't read definition statement in serverless.yml.

ChristopheBougere · 2017-12-14T16:27:21Z

I think this is the only way to keep the ARN validation with serverless-pseudo-parameters yes.
It's too bad that pseudo parameters aren't natively supported in serverless :/

theburningmonk · 2019-08-08T22:04:52Z

@ChristopheBougere now that we don't need pseudo-parameters anymore, maybe it's time to bring this back?

ChristopheBougere · 2019-08-09T07:42:57Z

Good catch @theburningmonk, I'll rebase and give a try soon

ChristopheBougere · 2019-08-16T13:02:24Z

@theburningmonk @horike37
I opened a new PR for this with intrinsic function support and a different behaviour: #245 🎉
Sorry for the delay, I know some people have been waiting a while for this.

Christophe Bougère added 2 commits December 9, 2017 12:59

Adding state machines validation (validate-state-machines command), u…

b8bb7e9

…sing asl-validator

unit tests for state machines validation

cc205f2

renaming command to sls validate stepf

980aeff

update asl-validator version to work with node 4

d4041d7

making index.js works with node 4

507aaf0

ChristopheBougere mentioned this pull request Feb 21, 2018

Discussion about Offline execution #105

Open

jasonkuhrt mentioned this pull request May 20, 2018

SCHEMA_VALIDATION_FAILED #120

Closed

gmunguia mentioned this pull request Mar 27, 2019

GetAtt intrinsic function support? #178

Closed

ChristopheBougere mentioned this pull request Aug 16, 2019

feat: asl validation #245

Merged

horike37 closed this Aug 19, 2019

State machine definition validation using asl-validator #90

State machine definition validation using asl-validator #90

Uh oh!

Conversation

ChristopheBougere commented Dec 9, 2017

Valid definition

State machine does not exists

Invalid defintion (missing StartAt property)

Uh oh!

coveralls commented Dec 9, 2017

Uh oh!

ChristopheBougere commented Dec 9, 2017

Uh oh!

horike37 commented Dec 12, 2017

Uh oh!

coveralls commented Dec 12, 2017

Uh oh!

ChristopheBougere commented Dec 12, 2017

Uh oh!

horike37 commented Dec 12, 2017

Uh oh!

ChristopheBougere commented Dec 12, 2017

Uh oh!

horike37 commented Dec 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Dec 12, 2017

Uh oh!

coveralls commented Dec 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ChristopheBougere commented Dec 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

horike37 commented Dec 13, 2017

Uh oh!

ChristopheBougere commented Dec 13, 2017

Uh oh!

ChristopheBougere commented Dec 13, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

horike37 commented Dec 14, 2017

Uh oh!

ChristopheBougere commented Dec 14, 2017

Uh oh!

theburningmonk commented Aug 8, 2019

Uh oh!

ChristopheBougere commented Aug 9, 2019

Uh oh!

ChristopheBougere commented Aug 16, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

horike37 commented Dec 12, 2017 •

edited

Loading

coveralls commented Dec 12, 2017 •

edited

Loading

ChristopheBougere commented Dec 12, 2017 •

edited

Loading

ChristopheBougere commented Dec 13, 2017 •

edited

Loading