DataDog / datadog-cloudformation-macro

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

Read config from ENV vars before validation #88

Closed pas256 closed 1 year ago

pas256 commented 1 year ago

Challenge

We have many SAM applications deployed in many AWS accounts. As currently written, each one of our SAM templates must contain the Datadog configuration values:

AWSTemplateFormatVersion: "2010-09-09"
Transform:
  - AWS::Serverless-2016-10-31
  # Enable Datadog APM inside Lambda as a Lambda Layer
  - Name: DatadogServerless
    Parameters:
      stackName: !Ref "AWS::StackName"
      nodeLayerVersion: 87
      extensionLayerVersion: 40
      apiKeySecretArn: "arn:aws:secretsmanager:us-west-2:123456789012:secret:DdApiKeySecret-e1v5Yn7TvIPc-d1Qc4E"
      service: "my-service-name"
      env: !Ref Environment

This makes it difficult to deploy this template in an AWS account that is not 123456789012 or in a region that is not us-west-2. Further than this, both apiKeySecretArn and env don't actually change between SAM applications deployed in us-west-2:123456789012 because 123456789012 is always our dev account.

To make our SAM templates easily reusable across regions and accounts (without introducing Datadog specific parameters to every template), I would like to encapulate the apiKeySecretArn and env in the Macro Lambda as environment variables. The prerequisite to using this Macro (from following https://github.com/DataDog/datadog-cloudformation-macro/tree/master/serverless#installation) is to deploy the CloudFormation template https://datadog-cloudformation-template.s3.amazonaws.com/aws/serverless-macro/latest.yml in every account and in every region anyway, so why not make it possible to set the config at that point in time as well?

Fortunately, the code to do this is already there. env.ts is already written to use the following:

I tried this, but unfortunately, index.ts validates the config supplied to the Macro before reading the environment variables for the Lambda of the macro. Line 72 runs and validates paramters before Line 86 reads the ENV vars.

Proposal

My proposal is to modify index.ts to read the environment variables, which combines ENV and config params, before validating them.

However, since setEnvConfiguration doesn't modify config (it modifies envVariables), the validation code would need to also run against envVariables instead of config. A larger refactor than just moving the validation function call, but not impossible.

End experience

At the end of this, I could modify latest.yml to set the config via environment variables once per account, per region, like so:

Resources:
  MacroFunction:
    Type: AWS::Serverless::Function
    DependsOn: MacroFunctionZip
    Properties:
      FunctionName:
        Fn::If:
          - SetFunctionName
          - Ref: FunctionName
          - Ref: AWS::NoValue
      Description: Processes a CloudFormation template to install Datadog Lambda layers for Python and Node.js Lambda functions.
      Handler: src/index.handler
      Runtime: nodejs16.x
      CodeUri:
        Bucket: !Ref MacroFunctionZipsBucket
        Key:
          Fn::Sub:
            - "serverless-macro-${Version}.zip"
            - {
                Version:
                  !FindInMap [Constants, DatadogServerlessMacro, Version],
              }
      Environment:
        Variables:
          DD_API_KEY_SECRET_ARN: "arn:aws:secretsmanager:us-west-2:123456789012:secret:DdApiKeySecret-e1v5Yn7TvIPc-d1Qc4E"
          DD_ENV: "dev"

and my SAM's template would only be

AWSTemplateFormatVersion: "2010-09-09"
Transform:
  - AWS::Serverless-2016-10-31
  # Enable Datadog APM inside Lambda as a Lambda Layer
  - Name: DatadogServerless
    Parameters:
      stackName: !Ref "AWS::StackName"
      nodeLayerVersion: 87
      extensionLayerVersion: 40
      service: "my-service-names"

and because IAM policies support wildcards, the serverless function policy can be:

      Policies:
        - Statement:
            - Sid: GetDDAPIKey
              Effect: Allow
              Action:
                - secretsmanager:GetSecretValue
              Resource: !Sub "arn:aws:secretsmanager:${AWS::Region}:${AWS::AccountId}:secret:DdApiKeySecret-*"

... and work in every account and region where we choose to deploy the SAM app.

pas256 commented 1 year ago

I'm happy to do this refactor if you don't have any objections.

sfirrin commented 1 year ago

Hi @pas256 thanks for getting in touch and including all of those details. I just had a chance to read through and your idea makes sense to me. Thanks for offering, we would appreciate a PR