feat(Implement aws-route53-alb): Implement new construct #421

biffgaut · 2021-10-05T20:14:52Z

Issue #, if available:
351

Description of changes:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

…s into Issue351 Bring up to date with main

aws-solutions-constructs-team · 2021-10-05T20:16:45Z

AWS CodeBuild CI Report

CodeBuild project: codebuildgithubautobuildPro-fkVQbXRiQi6A
Commit ID: fb3088f
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-solutions-constructs-team · 2021-10-05T22:01:01Z

AWS CodeBuild CI Report

CodeBuild project: codebuildgithubautobuildPro-fkVQbXRiQi6A
Commit ID: 223da2f
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-solutions-constructs-team · 2021-10-06T13:25:11Z

AWS CodeBuild CI Report

CodeBuild project: codebuildgithubautobuildPro-fkVQbXRiQi6A
Commit ID: 54fc793
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-solutions-constructs-team · 2021-10-06T14:05:48Z

AWS CodeBuild CI Report

CodeBuild project: codebuildgithubautobuildPro-fkVQbXRiQi6A
Commit ID: 0df6358
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-solutions-constructs-team · 2021-10-06T18:20:59Z

AWS CodeBuild CI Report

CodeBuild project: codebuildgithubautobuildPro-fkVQbXRiQi6A
Commit ID: 5a30ae1
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-solutions-constructs-team · 2021-10-06T19:30:39Z

AWS CodeBuild CI Report

CodeBuild project: codebuildgithubautobuildPro-fkVQbXRiQi6A
Commit ID: 31472a6
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-solutions-constructs-team · 2021-10-06T20:36:46Z

AWS CodeBuild CI Report

CodeBuild project: codebuildgithubautobuildPro-fkVQbXRiQi6A
Commit ID: 82783b8
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-solutions-constructs-team · 2021-10-06T21:03:24Z

AWS CodeBuild CI Report

CodeBuild project: codebuildgithubautobuildPro-fkVQbXRiQi6A
Commit ID: b89382e
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-solutions-constructs-team · 2021-10-06T21:25:29Z

AWS CodeBuild CI Report

CodeBuild project: codebuildgithubautobuildPro-fkVQbXRiQi6A
Commit ID: e984429
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

…s into Issue351 Update local.

aws-solutions-constructs-team · 2021-10-06T21:57:44Z

AWS CodeBuild CI Report

CodeBuild project: codebuildgithubautobuildPro-fkVQbXRiQi6A
Commit ID: c8578da
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-solutions-constructs-team · 2021-10-06T23:16:53Z

AWS CodeBuild CI Report

CodeBuild project: codebuildgithubautobuildPro-fkVQbXRiQi6A
Commit ID: f3bc022
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-solutions-constructs-team · 2021-10-06T23:30:16Z

AWS CodeBuild CI Report

CodeBuild project: codebuildgithubautobuildPro-fkVQbXRiQi6A
Commit ID: 5ecfaa4
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-solutions-constructs-team · 2021-10-06T23:54:07Z

AWS CodeBuild CI Report

CodeBuild project: codebuildgithubautobuildPro-fkVQbXRiQi6A
Commit ID: 40cd57a
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mickychetta

Minor typos need to be addressed but overall good construct

source/patterns/@aws-solutions-constructs/aws-route53-alb/lib/index.ts

source/patterns/@aws-solutions-constructs/aws-route53-alb/README.md

mickychetta · 2021-10-07T18:10:55Z

source/patterns/@aws-solutions-constructs/aws-route53-alb/lib/index.ts

+  public readonly loadBalancer: elb.ApplicationLoadBalancer;
+
+  /**
+   * @summary Constructs a new instance of the LambdaToStepfunctionsProps class.


Should be Route53ToAlb class

hnishar · 2021-10-08T00:48:05Z

source/patterns/@aws-solutions-constructs/aws-route53-alb/README.md

+
+| **Name**     | **Type**        | **Description** |
+|:-------------|:----------------|-----------------|
+| privateHostedZoneProps? | [route53.PrivateHostedZoneProps](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-route53.PrivateHostedZoneProps.html) | Optional custom properties for a new Private Hosted Zone. Cannot be specified for a public API. Cannot specify a VPC, it will use the VPC in existingVpc or the VPC created by the construct. Providing both this and existingHostedZoneInterfaceis an error. |


Is it missing PublicHostedZoneProps if customer chose to set publicApi to true ?

No, the construct doesn't support creating a public Hosted Zone (too many external dependencies). It will only configure a Public Hosted Zone that already exists. The thought is that externally facing DNS settings are not dynamic parts of a stack that are set up and torn down every time, but do get reconfigured to point to new internal infrastructure.

hnishar · 2021-10-09T01:49:55Z

source/patterns/@aws-solutions-constructs/aws-route53-alb/lib/index.ts

+   */
+  constructor(scope: Construct, id: string, props: Route53ToAlbProps) {
+    super(scope, id);
+    defaults.CheckProps(props);


Is the CheckProps method updated to validate Route53 props?

Not yet - CheckProps() has checks that are common to multiple constructs, all Route53 checks are unique to this construct. If/when we create another Route53 constructs, we'll identify the common checks and move them to CheckProps().

hnishar · 2021-10-09T01:53:30Z

source/patterns/@aws-solutions-constructs/aws-route53-alb/lib/index.ts

+    super(scope, id);
+    defaults.CheckProps(props);
+
+    if ((props?.logAccessLogs === false) && (props.loggingBucketProps)) {


Is there test coverage to validate this and the following scenarios?

Hmm - I'll check. I ensured 100% statement/branch coverage for the new core components, but checking this module it only had 96.8/87.1% coverage.

hnishar · 2021-10-09T02:11:56Z

source/patterns/@aws-solutions-constructs/aws-route53-alb/test/integ.deployPublicApiNewAlb.ts

+const props: Route53ToAlbProps = {
+  publicApi: true,
+  existingHostedZoneInterface: newZone,
+  existingVpc: newVpc,


Is it required to provide existingVpc for Public hosted zone?

No, a public hosted zone exists outside a VPC so we can create a new VPC or use one provided. A private hosted zone is DNS within a VPC, so it defines a VPC and which must be provided as the existingVpc.

hnishar · 2021-10-10T22:23:42Z

source/patterns/@aws-solutions-constructs/core/lib/alb-helper.ts

+export function CreateLambdaTargetGroup(
+  scope: Construct,
+  id: string,
+  lambdaFunction: lambda.Function,


The input type can be IFunction

hnishar · 2021-10-10T22:25:08Z

source/patterns/@aws-solutions-constructs/core/lib/alb-helper.ts

+  return listener;
+}
+
+export function CreateLambdaTargetGroup(


Is this a convenient helper function for customer to add lambda target? I don't see it being used from anywhere other than the test code

There's no reason a client couldn't use it, although I struggle to come up with a use case.

hnishar · 2021-10-11T11:35:37Z

source/patterns/@aws-solutions-constructs/core/lib/alb-helper.ts

+  });
+}
+
+export function AddTarget(


Are the customers required to use this utility function to add target(s) for Alb? Is this documented in README?

hnishar · 2021-10-11T11:43:52Z

source/patterns/@aws-solutions-constructs/aws-route53-alb/lib/index.ts

+        if (props.privateHostedZoneProps.vpc) {
+          throw new Error('All VPC specs must be provided at the Construct level in Route53ToAlbProps');
+        }
+        const manufacturedProps: r53.PrivateHostedZoneProps = defaults.overrideProps(props.privateHostedZoneProps, { vpc: this.vpc });


Are the default props for privateHostedZoneProps missing from core package? Do we need to enable security best practices for Route53 e.g. Logging, health checks, etc.?

https://docs.aws.amazon.com/Route53/latest/DeveloperGuide/logging-monitoring.html

I'm not sure that page is best practices, our actual Best Practices page is a little thin.

Opening an Issue to address this in a fast follow

aws-solutions-constructs-team · 2021-10-13T18:43:07Z

AWS CodeBuild CI Report

CodeBuild project: githubautobuild-for-cdk-v1
Commit ID: 9fa9ca4
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-solutions-constructs-team · 2021-10-13T18:43:11Z

AWS CodeBuild CI Report

CodeBuild project: githubautobuild-for-cdk-v2
Commit ID: 9fa9ca4
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-solutions-constructs-team · 2021-10-13T20:53:01Z

AWS CodeBuild CI Report

CodeBuild project: githubautobuild-for-cdk-v2
Commit ID: b8af255
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-solutions-constructs-team · 2021-10-13T20:54:59Z

AWS CodeBuild CI Report

CodeBuild project: githubautobuild-for-cdk-v1
Commit ID: b8af255
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-solutions-constructs-team · 2021-10-13T21:39:41Z

AWS CodeBuild CI Report

CodeBuild project: githubautobuild-for-cdk-v2
Commit ID: fa15824
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-solutions-constructs-team · 2021-10-13T21:41:58Z

AWS CodeBuild CI Report

CodeBuild project: githubautobuild-for-cdk-v1
Commit ID: fa15824
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

biffgaut added 2 commits October 5, 2021 16:02

Implement new construct

518aa49

Merge branch 'Issue351' of github.com:awslabs/aws-solutions-construct…

fb3088f

…s into Issue351 Bring up to date with main

biffgaut requested review from hayesry and hnishar as code owners October 5, 2021 20:14

biffgaut linked an issue Oct 5, 2021 that may be closed by this pull request

feat(New Construct - aws-route53-alb): Create New Construct #351

Closed

Adjust viperlight

223da2f

Fix first few cfn_nag issues

54fc793

Fix more cfn_nag issues

0df6358

cfn_nag changes

5a30ae1

Fix more cfn_nag issues

31472a6

Fix more cfn_nag issues

82783b8

Viperlight adjustments

b89382e

Merge branch 'main' into Issue351

e984429

biffgaut added 2 commits October 6, 2021 17:52

Fix more cfn_nag issues

85346df

Merge branch 'Issue351' of github.com:awslabs/aws-solutions-construct…

c8578da

…s into Issue351 Update local.

Viperlight adjustments

f3bc022

Lint trivia

5ecfaa4

Merge branch 'main' into Issue351

40cd57a

mickychetta suggested changes Oct 7, 2021

View reviewed changes

hnishar suggested changes Oct 11, 2021

View reviewed changes

biffgaut added 4 commits October 13, 2021 09:36

Address review comments

9abdefe

merge with main

802c942

Address review comments

4e1b1f5

Changes downloaded from github

9fa9ca4

viperlight edits

b8af255

hnishar approved these changes Oct 13, 2021

View reviewed changes

mickychetta approved these changes Oct 13, 2021

View reviewed changes

Merge branch 'main' into Issue351

fa15824

biffgaut merged commit afd0811 into main Oct 13, 2021

biffgaut deleted the Issue351 branch October 13, 2021 21:43

feat(Implement aws-route53-alb): Implement new construct #421

feat(Implement aws-route53-alb): Implement new construct #421

Conversation

biffgaut commented Oct 5, 2021

aws-solutions-constructs-team commented Oct 5, 2021

AWS CodeBuild CI Report

aws-solutions-constructs-team commented Oct 5, 2021

AWS CodeBuild CI Report

aws-solutions-constructs-team commented Oct 6, 2021

AWS CodeBuild CI Report

aws-solutions-constructs-team commented Oct 6, 2021

AWS CodeBuild CI Report

aws-solutions-constructs-team commented Oct 6, 2021

AWS CodeBuild CI Report

aws-solutions-constructs-team commented Oct 6, 2021

AWS CodeBuild CI Report

aws-solutions-constructs-team commented Oct 6, 2021

AWS CodeBuild CI Report

aws-solutions-constructs-team commented Oct 6, 2021

AWS CodeBuild CI Report

aws-solutions-constructs-team commented Oct 6, 2021

AWS CodeBuild CI Report

aws-solutions-constructs-team commented Oct 6, 2021

AWS CodeBuild CI Report

aws-solutions-constructs-team commented Oct 6, 2021

AWS CodeBuild CI Report

aws-solutions-constructs-team commented Oct 6, 2021

AWS CodeBuild CI Report

aws-solutions-constructs-team commented Oct 6, 2021

AWS CodeBuild CI Report

mickychetta left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

biffgaut Oct 11, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aws-solutions-constructs-team commented Oct 13, 2021

AWS CodeBuild CI Report

aws-solutions-constructs-team commented Oct 13, 2021

AWS CodeBuild CI Report

aws-solutions-constructs-team commented Oct 13, 2021

AWS CodeBuild CI Report

aws-solutions-constructs-team commented Oct 13, 2021

AWS CodeBuild CI Report

aws-solutions-constructs-team commented Oct 13, 2021

AWS CodeBuild CI Report

aws-solutions-constructs-team commented Oct 13, 2021

AWS CodeBuild CI Report

biffgaut Oct 11, 2021 •

edited

Loading