serverless-operations / serverless-step-functions

AWS Step Functions plugin for Serverless Framework ⚡️
Other
1.03k stars 206 forks source link

Validate fails when state resource uses pseudo parameters from the serverless-pseudo-parameters plugin #291

Open kalevalp opened 4 years ago

kalevalp commented 4 years ago

This is a Bug Report

Description

Validation fails when a resource in the state machine uses serverless-pseudo-parameters pseudo parameters.

This is the deployment:

service: scheduler-api

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

provider:
  name: aws
  runtime: nodejs10.x
  stage: dev
  region: eu-west-2

functions:
  checkairtable:
    handler: check.main

stepFunctions:
  validate: true
  stateMachines:
    checkAirtableSM:
      name: AirtableChecker
      definition:
        Comment: "Checks for posts that are complete and ready to be scheduled for posting in airtable and schedules them."
        StartAt: CheckAirtable
        States:
          CheckAirtable:
            Type: Task
            Resource: 'arn:aws:lambda:#{AWS::Region}:#{AWS::AccountId}:function:${self:service}-${self:provider.stage}-checkairtable'
            End: true

This is the error message:

  Serverless Error ---------------------------------------

  ✕ State machine "checkAirtableSM" definition is invalid:
  [{"keyword":"additionalProperties","dataPath":".States['CheckAirtable']","schemaPath":"#/additionalProperties","params":{"additionalProperty":"Resource"},"message":"should NOT have additional properties"},{"keyword":"additionalProperties","dataPath":".States['CheckAirtable']","schemaPath":"http://asl-validator.cloud/fail#/additionalProperties","params":{"additionalProperty":"Resource"},"message":"should NOT have additional properties"},{"keyword":"additionalProperties","dataPath":".States['CheckAirtable']","schemaPath":"#/additionalProperties","params":{"additionalProperty":"Resource"},"message":"should NOT have additional properties"},{"keyword":"additionalProperties","dataPath":".States['CheckAirtable']","schemaPath":"http://asl-validator.cloud/pass#/additionalProperties","params":{"additionalProperty":"Resource"},"message":"should NOT have additional properties"},{"keyword":"additionalProperties","dataPath":".States['CheckAirtable']","schemaPath":"http://asl-validator.cloud/succeed#/additionalProperties","params":{"additionalProperty":"Resource"},"message":"should NOT have additional properties"},{"keyword":"pattern","dataPath":".States['CheckAirtable'].Resource","schemaPath":"http://asl-validator.cloud/task#/properties/Resource/oneOf/0/pattern","params":{"pattern":"^arn:aws:([a-z]|-)+:([a-z]|[0-9]|-)*:[0-9]*:([a-z]|-)+:[a-zA-Z0-9-_.]+(:(\\$LATEST|[a-zA-Z0-9-_]+))?$"},"message":"should match pattern \"^arn:aws:([a-z]|-)+:([a-z]|[0-9]|-)*:[0-9]*:([a-z]|-)+:[a-zA-Z0-9-_.]+(:(\\$LATEST|[a-zA-Z0-9-_]+))?$\""},{"keyword":"type","dataPath":".States['CheckAirtable'].Resource","schemaPath":"http://asl-validator.cloud/task#/properties/Resource/oneOf/1/type","params":{"type":"object"},"message":"should be object"},{"keyword":"oneOf","dataPath":".States['CheckAirtable'].Resource","schemaPath":"http://asl-validator.cloud/task#/properties/Resource/oneOf","params":{"passingSchemas":null},"message":"should match exactly one schema in oneOf"},{"keyword":"additionalProperties","dataPath":".States['CheckAirtable']","schemaPath":"http://asl-validator.cloud/wait#/additionalProperties","params":{"additionalProperty":"Resource"},"message":"should NOT have additional properties"},{"keyword":"additionalProperties","dataPath":".States['CheckAirtable']","schemaPath":"#/additionalProperties","params":{"additionalProperty":"Resource"},"message":"should NOT have additional properties"},{"keyword":"oneOf","dataPath":".States['CheckAirtable']","schemaPath":"#/oneOf","params":{"passingSchemas":null},"message":"should match exactly one schema in oneOf"}]

Env info:

  Your Environment Information ---------------------------
     Operating System:          linux
     Node Version:              10.16.3
     Framework Version:         1.59.3
     Plugin Version:            3.2.5
     SDK Version:               2.2.1
     Components Core Version:   1.1.2
     Components CLI Version:    1.4.0

Plugin versions are:

serverless-pseudo-parameters: 2.5.0
serverless-step-functions: 2.15.0
theburningmonk commented 4 years ago

@kalevalp you don't need pseudo-params in this case, you can reference the checkairtable function like this:

CheckAirtable:
  Type: Task
  Resource: !GetAtt checkairtable.Arn
  End: true

normally you have to use the logical ID for the function (which in this case would have been CheckairtableLambdaFunction, but we made a change in this plugin to let you use the local function names in the definition section.

kalevalp commented 4 years ago

Sure, but it still seems like a bug. At least in that the error is hard to figure out from the error message. Also, considering that this use case of pseudo parameters appears in the serverless-pseudo-parameters, I expect more people to encounter it.

I opened this issue after investigating the bug in this SO question.

Also, apparently there is already an open issue for serverless-pseudo-parameters on the same bug.

theburningmonk commented 4 years ago

The problem is with the asl-validator which is not aware of the #{} syntax. And arguably it has no reason to, since it's a library for validating SFN state machine definitions and is not specific to the Serverless framework.

Also, as a side, we (as in, this serverless-step-functions plugin) actually translate those #{} pseudo parameters ourselves so the serverless-pseudo-parameters plugin doesn't even kick in. This is because there's an incompatibility with the serverless-pseudo-parameters plugin (see #252) in the way it tries to insert a Fn::Sub but we also need to use Fn::Sub ourselves too.

aslafy-z commented 4 years ago

https://github.com/serverless-operations/serverless-step-functions/pull/264 fixed fixed the incompatibility but still, validation is done before replacement, which makes it fail. Moving the replacement before the validation should fix this.