DataDog / serverless-plugin-datadog

Serverless plugin to automagically instrument your Lambda functions with Datadog
Apache License 2.0
96 stars 49 forks source link

Update above 5.55.0 breaks step function deployment with #517

Open schammah opened 3 months ago

schammah commented 3 months ago

Expected Behavior

Step function deploys correction with datadog serverless plugin 5.70.0 (i deploy step function with "serverless-step-functions": "^3.21.0" plugin.

Like In version 5.55.0 there was no problem deploying my step functions with serverless After trying to update to latest 5.70.0, looks like some schema validation was inserted and it fails to deploy my step functions with a specific syntax we use in our payloads

Actual Behavior

cloudformation reports an error and fails to deploy my step function with serverless (which always worked)

Steps to Reproduce the Problem

  1. Define a step function json with payload syntax as follow
     "delete integrations": {
      "Type": "Task",
      "Resource": "arn:aws:states:::lambda:invoke",
      "Parameters": {
        "FunctionName": "function:integration_scheduler:$LATEST",
        "Payload": {
          "action": "service/delete_customer",
          "payload": {
            "customer.$": "$.customer"
          }
        }
      },

All these $. and .$ are suddenly not passing schema validation with the latest versions of the plugin

Specifications

Stacktrace

Resource handler returned message: "Invalid State Machine Definition: 'SCHEMA_VALIDATION_FAILED: Cannot have both the field 'Payload.$' and the field 'Payload' at the same time at /States/Delete Athena Files/Parameters, SCHEMA_VALIDATION_FAILED: Cannot have both the field 'Payload.$' and the field 'Payload' at the same time at /States/Delete FS Files/Parameters, SCHEMA_VALIDATION_FAILED: Cannot have both the field 'Payload.$' and the field 'Payload' at the same time at /States/drop RDS app DB/Parameters, SCHEMA_VALIDATION_FAILED: Cannot have both the field 'Payload.$' and the field 'Payload' at the same time at /States/delete integrations/Parameters, SCHEMA_VALIDATION_FAILED: Cannot have both the field 'Payload.$' and the field 'Payload' at the same time at /States/delete ml/Parameters' (Service: AWSStepFunctions; Status Code: 400; Error Code: InvalidDefinition; Request ID: afeb3da8-d6bc-4330-8f8c-81fc7cb4af08; Proxy: null)" (RequestToken: 56028ebf-66dd-7570-e166-68fedcff5fa9, HandlerErrorCode: InvalidRequest)
agocs commented 3 months ago

Hi @schammah, thanks for creating this issue. The Datadog plugin is adding a Payload.$ field to the Task Parameters. It looks like we're not correctly handling the case when the customer already has a Payload field. We'll get this fixed.

schammah commented 2 months ago

hi @agocs was wondering if there was any ETA for this fix? as we need it to upgrade our plugin (needed to get latest versions + latest security patches)

agocs commented 2 months ago

@lym953 have you had a chance to look at this? This is SVLS-5561, correct? Are you able to provide a timeline to @schammah ?

lym953 commented 2 months ago

Hi @schammah, I'm working on loosening the constraint. We will add support for Lambda functions that have a Payload field, as long as the Payload doesn't contain Execution or State:

"Lambda Invoke": {
  ...
  "Parameters": {
    "Payload.$": {
      "action": "service/delete_customer", // ok
      "Execution": "xxx,                   // not ok
      "State": "xxx,                       // not ok
    },
  },

I'm targeting to finish this by the end of September.

schammah commented 2 months ago

hi @lym953 , thank you very much, looks like a solid solution. if possible would appreciate if we could get this fix earliest as this would greatly assist us in our current security audits.

lym953 commented 2 months ago

@schammah Thanks for the request. I will keep in mind to prioritize supporting this for Serverless Plugin.

lym953 commented 1 month ago

@schammah Relevant PRs have all been merged. I'm waiting for permissions to release a new version. Hopefully I can release it this Tuesday.

schammah commented 1 month ago

awesome, thanks for the update @lym953

lym953 commented 1 month ago

@schammah We just released v5.71.0. Could you try it out?

schammah commented 1 month ago

@lym953 tested a deployment in dev, its now successful working and not broken. thank you for the release.