feat(aws-certificatemanager): add DNSValidatedCertificate

[otterley]

Add a new class to @aws-cdk/aws-certificatemanager called
DNSValidatedCertificate. This class generates a certificate request
using AWS Certificate Manager and auto-validates the request using the
provided DNS "magic cookie" records. The user need only supply the
Domain Name of the certificate and a Route 53 Hosted Zone. A
CloudFormation Custom Resource is used along with the supplied Lambda
function to perform the operation.

---

Pull Request Checklist

• Testing
  • Unit test added (prefer not to modify an existing test, otherwise, it's probably a breaking change)
  • CLI change?: coordinate update of integration tests with team
  • cdk-init template change?: coordinated update of integration tests with team
• Docs
  • jsdocs: All public APIs documented
  • README: README and/or documentation topic updated
• Title and Description
  • Change type: title prefixed with fix, feat will appear in changelog
  • Title: use lower-case and doesn't end with a period
  • Breaking?: last paragraph: "BREAKING CHANGE: <describe what changed + link for details>"
  • Issues: Indicate issues fixed via: "Fixes #xxx" or "Closes #xxx"
• Sensitive Modules (requires 2 PR approvers)
  • IAM Policy Document (in @aws-cdk/aws-iam)
  • EC2 Security Groups and ACLs (in @aws-cdk/aws-ec2)
  • Grant APIs (only if not based on official documentation with a reference)

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.

[otterley]

IMPORTANT NOTE

It's unlikely that the included Lambda function will work properly at this time. We'll need to find a way to run npm install inside the module's subdirectory before zipping it and uploading it to S3 so that its node_modules subdirectory is properly populated.

Does CDK already have some sort of facility to do this?

[otterley]

Build is currently failing because pkglint mistakenly believes it needs to check the package.json in the Lamdba function's subdirectory. (The devDependencies it thinks are needed are not.)

[rix0rrr]

Thanks for contributing this! This is fantastic.

@sam-goodwin have you got an idea how we deal with the build issues?

[rix0rrr]

I believe this can be rewritten without an external dependency.

[otterley]

It can, but using the requests library adds clarity and makes the code easier to read vs. using Node's built-in HTTP library. I'd like to continue to use it if possible.

[rix0rrr]

Well here's the rub: we currently have no mechanism to do the 'npm install' at CDK build time, and I don't want to have people to run a manual step build anywhere in addition to requiring this library.

So either we get rid of the external dependencies and depend only on what comes with Node by default, or we cannot merge this upstream.

The source for a module that does (almost) everything we need to is here, and it doesn't have any additional dependencies:

https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-lambda-function-code.html#cfn-lambda-function-code-cfnresponsemodule

And the only thing it DOESN'T do is allow you to report a Reason back to CloudFormation (silly, but that's the way it is). I'd prefer to copy/paste and change that file into here, until we have something better.

[otterley]

@rix0rrr Addressed most of your concerns. WDYT?

[rix0rrr]

Then why not using built-in libraries?

const sha = require('crypto').createHash('sha256');
sha.update(requestId);
const token = sha.digest('hex').substr(0, 32);

[rix0rrr]

Well here's the rub: we currently have no mechanism to do the 'npm install' at CDK build time, and I don't want to have people to run a manual step build anywhere in addition to requiring this library.

So either we get rid of the external dependencies and depend only on what comes with Node by default, or we cannot merge this upstream.

The source for a module that does (almost) everything we need to is here, and it doesn't have any additional dependencies:

https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-lambda-function-code.html#cfn-lambda-function-code-cfnresponsemodule

And the only thing it DOESN'T do is allow you to report a Reason back to CloudFormation (silly, but that's the way it is). I'd prefer to copy/paste and change that file into here, until we have something better.

[sam-goodwin]

We are currently integrating with the aws-lambda-builders project to provide hooks in the CDK for performing builds of lambda functions, including running npm install and zipping up the folder as an asset.

Once we close #1435, you'll be able to instantiate a function that collaborates with the toolkit to run the isolated build you need. I'm working on this as we speak.

To give you an idea, this will look something like:

new NodeJSLambdaFunction(this, 'MyFunction', {
  path: './lambda',
  .. // etc.
});

[rix0rrr]

That would be sweet! But since building those functions will still be somewhat onerous (dependency and build-time wise), I'd still prefer if we could leave that feature mostly to user code and could keep the packages in the core libraries ready to deploy with minimal fuss.

[otterley]

@rix0rrr Updated the Lambda function to forego third-party dependencies. The only issue remaining, to my knowledge, is the test failure caused by the package.json linter.

[rix0rrr]

Hi Michael,

If you don't have any dependencies, then you don't need the package.json anymore, so I'd just get rid of it.

[otterley]

Good point @rix0rrr. I removed the files, but now the tests are failing due to incomplete coverage. Something doesn't make sense: it seems unlikely that the coverage fell by over 50% by the mere addition of a single file containing a single construct, which is actually pretty thoroughly tested.

Any thoughts on what might be happening?

[otterley]

OK, digging in a bit more (via npx nyc report --reporter=html), it's not the construct testing itself that's missing - it's the testing of the Lambda function. I'm going to dive a little deeper to see how best to satisfy the coverage analyzer.

[rix0rrr]

Ideally we'd add a unit test that mocks the SDK and AT LEAST traces a happy path through the Lambda handler. We would be setting a nice precedent for testing Lambdas as well if we did that.

[otterley]

@rix0rrr Thanks for getting the test harness working!

Any other impediments to merging?

[rix0rrr]

I guess we can, but I explicitly added jest to the top-level devDependencies as an encouragement to start using it more in the rest of the codebase. So I'd rather you get rid of it here.

[otterley]

Without it being listed here, npm run test in the Lambda's directory will not work.

There's no reason we can't have it in both places, though.