DataDog / datadog-ci

Use Datadog from your CI.
https://datadoghq.com
Apache License 2.0
129 stars 55 forks source link

Skip sfn->sfn context injection if custom Payload is used without $ #1451

Closed lym953 closed 2 months ago

lym953 commented 2 months ago

Problem

If a Lambda function in a State Machine has a custom Payload defined using "Payload": ..., e.g.,

    "Step Functions StartExecution": {
      "Type": "Task",
      "Resource": "arn:aws:states:::states:startExecution",
      "Parameters": {
        "StateMachineArn": "xxx",
        "Input": {
          "CONTEXT": "Context!"
        }
      },
      "End": true
    }

then context injection for the Step Function will fail with the error:

[Error] Invalid State Machine Definition: 'SCHEMA_VALIDATION_FAILED: Cannot have both the field 'CONTEXT.$' and the field 'CONTEXT' at the same time at /States/Step Functions StartExecution/Parameters'. Failed to inject context into lambda functions' payload of arn:aws:states:sa-east-1:425362996713:stateMachine:YimingTestExpressStateMachine2

This is because our instrumentation code will add a field "CONTEXT.$": ..., which duplicates with "CONTEXT": .... Right now we only check for custom "CONTEXT.$": ... (with $, i.e. using JSONPath) but don't check for custom "CONTEXT": ....

What

Also skip context injection when a custom "CONTEXT": ... field is used (without $)

Testing

Automated testing

Pass the added test, which would fail without the changes on helpers.ts

Manual testing

Steps:

  1. change the CONTEXT field of a Lambda function to what's shown in "Problem" section
  2. run datadog-ci stepfunctions instrument

Result: a warning is printed as expected

image

Notes

  1. Thanks @agocs for bringing this up in https://github.com/DataDog/datadog-ci/pull/1443#pullrequestreview-2293937693
  2. The behavior will be changed soon: we will soon allow custom CONTEXT field as long as it doesn't contain Execution or State field inside. But I want to first merge this PR to make the existing behavior more clear, so the changes in future PRs will be more clear so the PRs will be easier to review.

Review checklist

datadog-datadog-prod-us1[bot] commented 2 months ago

Datadog Report

Branch report: yiming.luo/fix-step-func-4 Commit report: 998b0be Test service: datadog-ci-tests

:white_check_mark: 0 Failed, 376 Passed, 0 Skipped, 1m 27.58s Total duration (2m 0.1s time saved)