feat(aws-lambda-opensearch): created new construct #818

mickychetta · 2022-09-29T19:16:17Z

Issue #817 , if available:

Description of changes:

Created new construct aws-lambda-opensearch
Added OpenSearch helper and default files
Added unit and integ tests for construct

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

fixes #817

aws-solutions-constructs-team · 2022-09-29T19:22:47Z

AWS CodeBuild CI Report

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

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

source/patterns/@aws-solutions-constructs/aws-lambda-opensearch/README.md

aws-solutions-constructs-team · 2022-09-29T22:12:08Z

AWS CodeBuild CI Report

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

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

aws-solutions-constructs-team · 2022-09-29T22:20:04Z

AWS CodeBuild CI Report

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

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

biffgaut

I think there are 2 comments requiring changes - one about the service role and one about the comment on Cognito Domain name.

source/patterns/@aws-solutions-constructs/aws-lambda-opensearch/README.md

aws-solutions-constructs-team · 2022-09-30T17:08:57Z

AWS CodeBuild CI Report

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

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

aws-solutions-constructs-team · 2022-10-04T17:10:57Z

AWS CodeBuild CI Report

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

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

aws-solutions-constructs-team · 2022-10-04T18:01:40Z

AWS CodeBuild CI Report

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

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

biffgaut · 2022-10-04T20:22:29Z

source/patterns/@aws-solutions-constructs/aws-lambda-opensearch/lib/index.ts

+
+  /**
+   * @summary Constructs a new instance of the LambdaToOpenSearch class.
+   * @param {cdk.App} scope - represents the scope for all the resources.


I realize this is just copied from other Constructs, but can you think of a reason this says cdk.App instead of Construct? The type below is clearly Construct...
Should we change this in every construct?

Yes, we should change this in every construct

biffgaut · 2022-10-04T20:39:20Z

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

@@ -120,3 +120,18 @@ export function setupCognitoForElasticSearch(scope: Construct, domainName: strin

  return cognitoAuthorizedRole;
 }
+
+export function setupOpenSearchCognito(scope: Construct, domainName: string):


Any reason why you didn't follow the same naming pattern as setupCognitoForElasticSearch?

It's fine if you have a reason, the ElasticSearch stuff will probably not live forever, but if we're changing the naming pattern we should have a reason.

No particular reason, changed to setupCognitoForOpenSearch

source/patterns/@aws-solutions-constructs/aws-lambda-opensearch/lib/index.ts

biffgaut · 2022-10-04T20:54:33Z

source/patterns/@aws-solutions-constructs/aws-lambda-opensearch/test/lambda/index.js

+var creds = new AWS.EnvironmentCredentials('AWS');
+var endpoint = new AWS.Endpoint(openSearchDomain.endpoint);
+
+function postDocumentToOpenSearch(doc, context) {


Have you launched this and confirm that data ends up in OpenSearch when run?

Yep, data is shown in dashboards

source/patterns/@aws-solutions-constructs/aws-lambda-opensearch/test/lambda/index.js

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

biffgaut · 2022-10-04T21:44:16Z

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

+
+const MAXIMUM_AZS_IN_OPENSEARCH_DOMAIN = 3;
+
+export interface BuildOpenSearchProps {


Wait a minute - if this exists, why were the props defined as any in index.ts?

BuildOpenSearchProps interface are readonly, we want to to add securityGroupIds if a VPC is present

biffgaut · 2022-10-04T21:45:09Z

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

+  // ClusterStatus.red maximum is >= 1 for 1 minute, 1 consecutive time
+  alarms.push(new cloudwatch.Alarm(scope, 'StatusRedAlarm', {
+    metric: new cloudwatch.Metric({
+      namespace: 'AWS/ES',


Why still ES instead of OS?

OpenSearch kept ES prefix instead of creating new OS

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

aws-solutions-constructs-team · 2022-10-05T00:22:53Z

AWS CodeBuild CI Report

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

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

biffgaut

One change requested

source/patterns/@aws-solutions-constructs/aws-lambda-opensearch/lib/index.ts

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

biffgaut

Couple comments, one question

biffgaut · 2022-10-10T18:19:55Z

source/patterns/@aws-solutions-constructs/aws-lambda-opensearch/test/integ.no-arguments.ts

+  handler: 'index.handler'
+};
+
+const openSearchDomain = 'solutions-constructs-domain';


Any reason why we don't specify a cognito domain like we do in the Elasticsearch version of this test?

In the integ.no-arguments.ts for aws-lambda-opensearch, I used only the required arguments to deploy the stack. Specifying both OpenSearch and Cognito domain separately is handled in integ.domain-arguments.ts.

biffgaut · 2022-10-10T18:25:54Z

source/patterns/@aws-solutions-constructs/aws-lambda-opensearch/test/lambda-opensearch.test.ts

+  return new LambdaToOpenSearch(stack, 'test-lambda-opensearch-stack', props);
+}
+
+function getDefaultTestLambdaProps(): lambda.FunctionProps {


Don't need to change this, but it could have been a constant to reduce complexity a little (most compile languages would probably make this optimization at compile time):

const defaultTestLambdaProps: lambda.FunctionProps = { code: lambda.Code.fromAsset(`${__dirname}/lambda`), runtime: lambda.Runtime.NODEJS_14_X, handler: 'index.handler', };

When using this approach, I get an Asset resource already exists in another stack:
Asset is already associated with another stack 'Default'. Create a new Code instance for every stack.

Using a function will create a unique reference id for the Code/Asset resource in each CloudFormation stack:
When the AWS CDK deploys an app that references assets (either directly by the app code or through a library), the AWS CDK CLI first prepares and publishes the assets to an Amazon S3 bucket or Amazon ECR repository, which was created during bootstrapping. Only then are the resources defined in the stack deployed.

Interesting - thanks, I learned something there!

source/patterns/@aws-solutions-constructs/aws-lambda-opensearch/test/lambda-opensearch.test.ts

aws-solutions-constructs-team · 2022-10-11T17:46:22Z

AWS CodeBuild CI Report

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

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

biffgaut

Approved with 1 comment

biffgaut · 2022-10-11T19:21:21Z

source/patterns/@aws-solutions-constructs/aws-lambda-opensearch/test/lambda-opensearch.test.ts

+  return new LambdaToOpenSearch(stack, 'test-lambda-opensearch-stack', props);
+}
+
+function getDefaultTestLambdaProps(): lambda.FunctionProps {


Interesting - thanks, I learned something there!

mickychetta added 2 commits September 29, 2022 19:13

created README

d569972

fixed pattern prop name

8097acd

mickychetta requested review from biffgaut, hayesry and mobri2a as code owners September 29, 2022 19:16

mickychetta self-assigned this Sep 29, 2022

mickychetta requested a review from emcfins as a code owner September 29, 2022 19:16

biffgaut changed the title ~~feat(aws-lambda-opensearch): Created README~~ feat(aws-lambda-opensearch): Created new construct Sep 29, 2022

biffgaut requested changes Sep 29, 2022

View reviewed changes

fixed service name in README

e60d4fb

updated prop description

620a2ee

biffgaut requested changes Sep 30, 2022

View reviewed changes

source/patterns/@aws-solutions-constructs/aws-lambda-opensearch/README.md Outdated Show resolved Hide resolved

removed legacy elasticsearch role description

fcbfc34

biffgaut previously approved these changes Sep 30, 2022

View reviewed changes

created new construct

1d01293

mickychetta dismissed biffgaut’s stale review via 1d01293 October 4, 2022 17:00

fixed prop description

d926fd2

biffgaut requested changes Oct 4, 2022

View reviewed changes

removed kibana text in opensearch construct

cdfc289

biffgaut changed the title ~~feat(aws-lambda-opensearch): Created new construct~~ feat(aws-lambda-opensearch): created new construct Oct 10, 2022

biffgaut requested changes Oct 10, 2022

View reviewed changes

source/patterns/@aws-solutions-constructs/aws-lambda-opensearch/lib/index.ts Outdated Show resolved Hide resolved

source/patterns/@aws-solutions-constructs/core/lib/vpc-helper.ts Outdated Show resolved Hide resolved

biffgaut reviewed Oct 10, 2022

View reviewed changes

added type safety to opensearch prop

64df182

biffgaut approved these changes Oct 11, 2022

View reviewed changes

biffgaut merged commit f31f59d into awslabs:main Oct 11, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(aws-lambda-opensearch): created new construct #818

feat(aws-lambda-opensearch): created new construct #818

mickychetta commented Sep 29, 2022 •

edited

Loading

aws-solutions-constructs-team commented Sep 29, 2022

aws-solutions-constructs-team commented Sep 29, 2022

aws-solutions-constructs-team commented Sep 29, 2022

biffgaut left a comment

aws-solutions-constructs-team commented Sep 30, 2022

aws-solutions-constructs-team commented Oct 4, 2022

aws-solutions-constructs-team commented Oct 4, 2022

biffgaut Oct 4, 2022

mickychetta Oct 4, 2022

biffgaut Oct 4, 2022

mickychetta Oct 4, 2022

biffgaut Oct 4, 2022

mickychetta Oct 4, 2022

biffgaut Oct 4, 2022

mickychetta Oct 4, 2022

biffgaut Oct 4, 2022

mickychetta Oct 4, 2022

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

biffgaut left a comment

biffgaut left a comment

biffgaut Oct 10, 2022

mickychetta Oct 11, 2022

biffgaut Oct 10, 2022

mickychetta Oct 11, 2022

biffgaut Oct 11, 2022

aws-solutions-constructs-team commented Oct 11, 2022

biffgaut left a comment

biffgaut Oct 11, 2022


		const MAXIMUM_AZS_IN_OPENSEARCH_DOMAIN = 3;

		export interface BuildOpenSearchProps {

feat(aws-lambda-opensearch): created new construct #818

feat(aws-lambda-opensearch): created new construct #818

Conversation

mickychetta commented Sep 29, 2022 • edited Loading

aws-solutions-constructs-team commented Sep 29, 2022

AWS CodeBuild CI Report

aws-solutions-constructs-team commented Sep 29, 2022

AWS CodeBuild CI Report

aws-solutions-constructs-team commented Sep 29, 2022

AWS CodeBuild CI Report

biffgaut left a comment

Choose a reason for hiding this comment

aws-solutions-constructs-team commented Sep 30, 2022

AWS CodeBuild CI Report

aws-solutions-constructs-team commented Oct 4, 2022

AWS CodeBuild CI Report

aws-solutions-constructs-team commented Oct 4, 2022

AWS CodeBuild CI Report

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 5, 2022

AWS CodeBuild CI Report

biffgaut left a comment

Choose a reason for hiding this comment

biffgaut 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aws-solutions-constructs-team commented Oct 11, 2022

AWS CodeBuild CI Report

biffgaut left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mickychetta commented Sep 29, 2022 •

edited

Loading