DataDog / datadog-cloudformation-macro

CloudFormation Macros by Datadog
Apache License 2.0
14 stars 22 forks source link

IAM Policy Requirement for `enableXRayTracing` #69

Closed BlissfulDarkness closed 1 year ago

BlissfulDarkness commented 2 years ago

Expected Behavior

Enabling X-Ray Tracing should not require a defined IAM Resource in the template, if an IAM role is being specifically provided that already has the needed permissions.

For the enableXRayTracking and enableDDTracing options, a third value of "inherit" or "ignore" would resolve this, since all the required configuration can be done via Globals.Function template specifications.

Actual Behavior

If the DataDog Macro is given enableXRayTracing: true provided, it will fail to transform due to requiring an IAM Resource for that function.

This behavior also blocks the way this could be done on an older version of the Macro, where you could configure the behavior by providing the required information as part of the Global section of the template.

Globals:
  Function:
    Runtime: nodejs14.x
    MemorySize: 384
    Timeout: 60
    VpcConfig:
      SecurityGroupIds: !Split [",", !Ref SecurityGroupIds]
      SubnetIds: !Split [",", !Ref SubnetIds]
    Tracing: Active
    Layers:
      - !Sub "arn:aws:lambda:${AWS::Region}:580247275435:layer:LambdaInsightsExtension:14"
    AutoPublishAlias: !Ref StageName
    DeploymentPreference:
      Type: AllAtOnce
    Tags:
      env: !Ref StageName
      environment: !Ref StageName
      service: "bootworks"
      version: !Ref Version
    Environment:
      Variables:
        SERVICE_ENV: !Ref StageName
        DD_ENV: !Ref StageName
        DD_SERVICE: "bootworks"
        DD_VERSION: !Ref Version
        DD_MERGE_XRAY_TRACES: "true"
        DD_TRACE_ENABLED: "true"
        AWS_XRAY_CONTEXT_MISSING: "LOG_ERROR"
        AWS_NODEJS_CONNECTION_REUSE_ENABLED: 1

Steps to Reproduce the Problem

  1. Configure enableXRayTracing: true for the DataDog Macro
  2. Define a Function that uses an existing IAM Role by ARN
  3. Stack creation fails due to transform requirement

Specifications

For the Datadog Serverless Macro:

Stacktrace

Error: Failed to create changeset for the stack: bootworks-api-v0-james, ex: Waiter ChangeSetCreateComplete failed: Waiter encountered a terminal failure state: For expression "Status" we matched expected path: "FAILED" Status: FAILED. Reason: Transform 552520131273::DatadogServerless failed with: No AWS::IAM::Role resource was found for the function bwHandleScanEvents when adding xray tracing policies
tianchu commented 2 years ago

@hindmanj Thanks for flagging the issue!

Can you help confirm if I understood the issue correctly? When you set enableXRayTracing to true, datadog macro tries to add the required X-Ray permissions to the IAM role. The findIamRole assumes that the IAM role is also defined in the same template. But in your use case, you have the IAM role predefined somewhere else (not as part of the template), therefore the macro fails because it cannot find the IAM role and add required permissions?

If my understanding is correct, I think the better fix instead of https://github.com/DataDog/datadog-cloudformation-macro/pull/70 is:

  1. refactor https://github.com/DataDog/datadog-cloudformation-macro/blob/21047dffdc4c1bc1094031c5b109f7e8e29f2dc8/serverless/src/tracing.ts#L101-L117 to its own function (say addXrayPolicies)
  2. if findIamRole returns null (i.e., the role is defined somewhere else), instead of throw an error and stop (https://github.com/DataDog/datadog-cloudformation-macro/blob/21047dffdc4c1bc1094031c5b109f7e8e29f2dc8/serverless/src/tracing.ts#L95), the macro should only log an error, and continue processing without calling the addXrayPolicies function (assumes the permissions are already on the given role).

What do you think?

astuyve commented 1 year ago

Closing as there's been no update here. Please feel free to re-open if you want us to take another look!