mapbox / deprecated-lambda-cfn Goto Github PK

To encrypt parameters, you have to pass -k and this is really easy to forget to do. Since this is passed through to cfn-config which actually implements this behavior it will be difficult change.

Some thoughts:

• lambda-cfn defaults to passing just -k to cfn-config and we add another option -nok or something to override that default. If you pass -k <kmsArn> it gets passed wholesale to cfn-config
  • -k with no arn defaults to the kms id alias/cloudformation, what happens if that doesn't exist?

I noticed that we're using the Node.js 6.10.2 runtime in the package.json https://github.com/mapbox/lambda-cfn/blob/14015e33f2ca0ec87bffa1866d5f61ab0f9d0ba6/package.json#L7 as well as in the Travis tests https://github.com/mapbox/lambda-cfn/blob/master/.travis.yml#L5.

The AWS Lambda Node.js 6x runtime is 6.10.3 and we say this in the lambda-cfn docs:

Lambda-cfn (or "Lambda CloudFormation") is a Node.js project that runs on Amazon Web Service's supported Node runtimes, currently Node.js v4.3.2 and v6.10.3.

This is a really minor issue. It's only a minor version difference and I don't see anything critical or important in the 6.10.3 changelog. Also, this is only to ensure consistency when developing locally and testing on Travis so that you're using the same version of Node that's in production. Even though we specified v6.10.2 in the package.json any lambda-cfn code is going to run on 6.10.3 on AWS anyway.

We should update this in lambda-cfn v2.1 or 2.0.1 and also make sure Patrol projects are using 6.10.3 as well.

/cc @zmully

It is not obvious the consequences of updating the function name specified in the function.template.js for a running function. Because this name is used within the CloudFormation template as a prefix for resources, changing the function name also changes the names these resources, and CloudFormation treats the renaming as a requests for new resources. It'll delete all old resources and create new ones, and many of the new resources will have different ARNs and URLs (like SNS topics, API Gateway URLs). This can wreak havoc if there are external dependencies configured with the old values.

With the one rule, one stack architecture, I believe it may be possible to remove the function name prefixing, and remove the requirement for a function name declaration within the template entirely. This would prevent a mistaken name change from drastically altering running stack.

When a webhook is set to apiKey: true, the key is made available via the template Outputs. Currently, when the Outputs section is added for the key, it is invalid CFN json.

We currently provide an upload.sh script to help people zip and upload code from their local machines to S3.

We could turn this into a more powerful lambda-cfn upload command to make this even easier for users. This command could also run automatically before running any lambda-cfn create or lambda-cfn update commands since it's common to forget to re-upload any changed code.

No urgency in working on this ticket - we can save this work for a later release. Leave any thoughts on this command and how it'd fit with other lambda-cfn commands in this ticket.

/cc @zmully @ianshward

lambda-cfn built-in function alarms have a very low default, very log being zero:

https://github.com/mapbox/lambda-cfn/blob/master/lib/artifacts/alarms.js#L42

This threshold should be left to the implementation of lambda-cfn to reduce the amount of alarm noise from service failures when appropriate.

When you're developing on lambda-cfn or related, and you npm link lambda-cfn into a patrol stack, rules paths cannot be resolved because the wrong application root is used.

@zmully I believe the start of the problem is here https://github.com/mapbox/lambda-cfn/blob/7e278324d7fe52973f4315b5c9fa65e91f5f0310/lib/lambda-cfn.js#L59 which uses app-root-path here https://github.com/mapbox/lambda-cfn/blob/7e278324d7fe52973f4315b5c9fa65e91f5f0310/lib/lambda-cfn.js#L5 to resolve the root of the application.

There are a few places in lambda-cfn.js that rely on app-root-path. There should be tests to ensure all these cases are handled properly, in addition to properly handling the case around an npm linked copy of lambda-cfn.

Per @zmully

The lambda-cfn-rules.js needs a --delete option to remove the ruleset if the main Crowsnest stack is deleted.

Got an alarm with this link: https://github.com/mapbox/lambda-cfn/blob/master/alarms.md#Errors

The link is broken...

cc @zmully @k-mahoney

AWS Lambda console now displays this warning in the console:

https://github.com/mapbox/lambda-cfn/blob/2dc4d70924fa50cf2369705aee7b4c4b75ddfcc6/lib/lambda-cfn.js#L1135-L1143

This method tests for indexOf > -1, so if you had myConfig and myConfigFoo there's no telling which will be found with getEnv

Quirk of API GW and CFN, since deployments are immutable, a new deployment has to be created every time the stack is updated for the latest methods to be deployed. This can be hacked around by randomizing the deployment name. #36

Currently Lambda-cfn support multiple rules, in multiple repositories in a single stack. This makes deployment and maintenance a headache as well as being a hinderance to new users. Moving to a one rule one stack architecture will simplify the lambda-cfn codebase as well as improve usability.

• support existing lambda-cfn rule functionality
• split lambda code from the lambda Cloudformation template
  • function.js: contains the actual lambda function code
  • function.template.js: Cloudformation template

Given the following example:

var lambdaCfn = require('lambda-cfn');

module.exports = lambdaCfn(
  [
    'myHandler.js',
  ],
  {
    "AWSTemplateFormatVersion": "2010-09-09",
    "Description": "my stack",
    "Resources": {
       "AutoScalingGroup": { ... }
    }
  }
);

How would I go about referencing AutoScalingGroup inside my rule configuration? Specifically, I'd like make the physical id of AutoScalingGroup available to the function. I'm currently only seeing a way to do this with a parameter.

As part of the lambda-cfn refactor to 1 rule 1 stack, a convenience function for new rules should initialize and create skeleton files to speed rule development.

Let's keep this super simple for this first go round, the command should assume you're writing a new CloudWatch Event based rule.

lambda-cfn init <rulename> should:

1. Be run from a top level parent directory, all rules will be sub directories of this directory
2. Check for a package.json in the parent directory, create it if necessary
3. Add/update lambda-cfn as dependency in the package.json
4. Check the rulename (no spaces, no underscores, anything else not legal in a Cloudformation stack name, must contain only letters, numbers, dashes and start with an alpha character. )
5. Check and create a sub directory for the rule ./<rulename>
6. Create function.js and function.template.js file skeletons (contents TBD)

If you run lambda-cfn update <stack> or lambda-cfn create <stack> and the key for the Lambda code on S3 has not been uploaded yet (either manually or as part of an automated build + deploy system) then lambda-cfn will error during the CloudFormation deploy process (and not cancel the deploy). This means a user will have gone through the CloudFormation parameter input process and deploy confirmation process only to have a failed deploy at the final step during the CloudFormation deloy. This process takes a while and can be frustrating.

To make this module a little more user friendly we could have lambda-cfn first check that the code exists on S3 after providing parameter values (in order to obtain the location on S3) but before the final CloudFormation deploy.

/cc @ianshward @zmully

To avoid parameter name collisions, parameter names should be prefixed with the name of the rule.

Right now only an Errors and NoInvocations alarm are defined for all lambdas. It will be easy to let a rule define their own alarms by specifying some properties the rule's in module.exports.config.

I'm getting the following error when I run tests using a project with lambda-cfn as a devdependency:

module.js:327
    throw err;
    ^

Error: Cannot find module '/../test/package.json'
    at Function.Module._resolveFilename (module.js:325:15)
    at Function.Module._load (module.js:276:25)
    at Module.require (module.js:353:17)
    at require (internal/module.js:12:17)
    at Object.<anonymous> (/../node_modules/lambda-cfn/lib/lambda-cfn.js:47:10)
    at Module._compile (module.js:409:26)
    at Object.Module._extensions..js (module.js:416:10)
    at Module.load (module.js:343:32)
    at Function.Module._load (module.js:300:12)
    at Module.require (module.js:353:17)
    at require (internal/module.js:12:17)

Going through the stack trace, this is caused by the following code in lambda-cfn.js at line 45:

var pkgs;
if (process.env.NODE_ENV == 'test') {
    pkgs = require(path.join(root,'test/package.json'));
} else {
    pkgs = require(path.join(root,'package.json'));
}

It seems this code is needed for lambda-cfn tests to run. But because of this, it requires test/package.json to exist to all projects using this package - which shouldn't be the case.

Rule name namespace is currently global across all rulesets. See also #6

@zmully I overlooked this yesterday - a rule definition can add additional IAM policy statements to the IAM role used by a crowsnest stack, but is not documented in the rule spec.

• Code
• Tests

You can assign this to me if you'd rather I add it to the spec.

For the 1 rule 1 stack refactor, we're moving from one repository containing multiple rules and creating a single stack for all rules to one repository with multiple rules each which gets its own stack.

For the initial command, wrap cfn-config with sane defaults to create a stack for a single rule, so

~/patrol-rules-aws/rule$ lambda-cfn deploy <environment> would run cfn-config with sane defaults, like:

cfn-config create rule-<production> function.template.js -n patrol-rules-aws -r us-east-1 -c cfn-config-${AWS_ACCOUNT_ID}-us-east-1 -k kmsKey​

It'd be useful to be able to dump the template out to the console.

If an apikey is requested for a gateway rule, a corresponding usage plan must be associated with the key. Right now, lambda-cfn does not create a plan and so any rule using API-gateway will not respond to REST api requests until one is associated. Initially, we can make this simple and just create a default plan, with no throttle and no quota. In the future these should be parameterized.

AWS Lambda now supports environment variables (docs).

As a result, we can remove streambot from this project.

/cc @ianshward @zmully @k-mahoney @rclark

For external webhooks, currently the only way to wire them into a lambda is via a service like Zapier to proxy the hook to the rule SNS topic. Instead of the extra hop and external service requirement, the webhooks could call the lambda directly via API-Gateway. There is no CFN support for API-Gateway, but because we've already a dependency on an external script to create the CWE rules, modifying it to also support API-Gateway shouldn't be difficult.

Lambda-cfn exposes two utility functions - splitOnComma and message - in https://github.com/mapbox/lambda-cfn/blob/14015e33f2ca0ec87bffa1866d5f61ab0f9d0ba6/index.js#L4-L5 that are useful when working with lambda-cfn based projects such as Patrol. For more context, you can see both of these functions being used in the function.js file for the cloudTrail rule in /patrol-rules-aws.

We should write brief docs on these functions in this repo, including why they might be useful and how to use them. Unless you're closely looking at the index.js file you wouldn't know about these functions, and it's not clear from the message() function's name what it does.

/cc @zmully @ianshward

The CWE CFN specification supports specifying both a schedule expression and an event pattern for a single CWE resource. Currently a CWE rule is generated for each CWE type.

We need to provide a helper function to generate the namespaced parameter name so it can be used as a statement reference.

See mapbox/DEPRECATED-patrol-rules-aws#80

  parameters: {
    deviceHistory: {
      Type: 'String',
      Description: 'ARN of S3 bucket for storing a list of known devices'
    }
  },
  statements: [
    {
      Effect: 'Allow',
      Action: [
        's3:GetObject',
        's3:ListBucket',
        's3:PutObject'
      ],
      Resource: { Ref: 'deviceHistory' }
    }
  ],

Ref to deviceHistory will not work in this case

cc: @zmully