feat(new construct): aws-lambda-kendra #989
Conversation
biffgaut
requested review from
hayesry,
mobri2a,
emcfins,
mickychetta and
georgebearden
as code owners
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
| | --- | --- | --- | | ||
| | existingLambdaObj? | [`lambda.Function`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_lambda.Function.html) | Existing instance of Lambda Function object, providing both this and `lambdaFunctionProps` will cause an error. | | ||
| | lambdaFunctionProps? | [`lambda.FunctionProps`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_lambda.FunctionProps.html) | User provided props to override the default props for the Lambda function. | | ||
| | kendraIndexProps? | [`kendra.CfnIndexProps`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_kendra.CfnIndex.html) | Optional user provided props to override the default props for the Kendra index. **Is this required?** | |
There was a problem hiding this comment.
Questioning the **Is this required?** comment at the end of of the kendraIndexProps?
| | existingLambdaObj? | [`lambda.Function`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_lambda.Function.html) | Existing instance of Lambda Function object, providing both this and `lambdaFunctionProps` will cause an error. | | ||
| | lambdaFunctionProps? | [`lambda.FunctionProps`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_lambda.FunctionProps.html) | User provided props to override the default props for the Lambda function. | | ||
| | kendraIndexProps? | [`kendra.CfnIndexProps`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_kendra.CfnIndex.html) | Optional user provided props to override the default props for the Kendra index. **Is this required?** | | ||
| | kendraDataSourcesProps | [`CfnDataSourceProps[]`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_kendra.CfnDataSource.html) | A list of data sources that will provide data to the Kendra index. *?At least 1 must be specified*. We will do majority of processing for some data sources (S3 crawler initially), but for others the props must be complete (e.g. proper roleArn, etc.) | |
There was a problem hiding this comment.
Is the *? a special markdown syntax?
| | kendraIndexProps? | [`kendra.CfnIndexProps`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_kendra.CfnIndex.html) | Optional user provided props to override the default props for the Kendra index. **Is this required?** | | ||
| | kendraDataSourcesProps | [`CfnDataSourceProps[]`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_kendra.CfnDataSource.html) | A list of data sources that will provide data to the Kendra index. *?At least 1 must be specified*. We will do majority of processing for some data sources (S3 crawler initially), but for others the props must be complete (e.g. proper roleArn, etc.) | | ||
| | indexPermissions? | `string[]` | Optional - index permissions to grant to the Lambda function. One or more of the following may be specified: `Read`, `SubmitFeedback` and `Write`. Default is `["Read", "SubmitFeedback"]`. Read is all the operations IAM defines as Read and List. SubmitFeedback is only the SubmitFeedback action. Write is all the operations IAM defines as Write and Tag. This functionality may be overridden by providing a specific role arn in lambdaFunctionProps | | ||
| | existingKendraIndex | [`kendra.cfnIndex`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_kendra.CfnIndex.html) | An existing Kendra index to which the Lambda function will be granted access. Supplying along with kendraIndexProps or kendraDataSourceProps will throw an error. | |
There was a problem hiding this comment.
In the index.ts file, I believe this property is called existingKendraIndexObj and is optional, so we should update the README to reflect it.
| | --- | --- | --- | | ||
| | lambdaFunction | [`lambda.Function`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_lambda.Function.html) | Returns an instance of `lambda.Function` managed by the construct | | ||
| | kendraIndex | [`kendra.cfnIndex`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_kendra.CfnIndex.html) | Returns an instance of `kendra.cfnIndex` managed by the construct | | ||
| | kendraDataSources | [`interface DataSourceProperties { |
There was a problem hiding this comment.
The type here seems a bit off, should the interface declaration have a closing }?
| | lambdaRole | [`iam.Role`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_iam.Role.html) | The role assumed by the Lambda function | | ||
| | vpc? | [`ec2.IVpc`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_ec2.IVpc.html) | Returns an interface on the VPC used by the pattern (if any). This may be a VPC created by the pattern or the VPC supplied to the pattern constructor. | | ||
|
|
||
| interface DataSourceProperties { |
There was a problem hiding this comment.
Ok, I see this is the declaration of the kendraDataSources property above. Maybe clean up the type above or tell them to look here for the details.
|
|
||
| ## Architecture | ||
|
|
||
|  |
There was a problem hiding this comment.
I don't see an actual architecture.png commited in this PR, and not sure about the URL that is specified.
|
|
||
| const testBucket = defaults.CreateScrapBucket(stack); | ||
|
|
||
| new LambdaToKendra(stack, 'minimal-arguments', { |
There was a problem hiding this comment.
I love how clean the interface is to use this construct, nice!
| const sourceProps: kendra.CfnDataSource.WebCrawlerConfigurationProperty = { | ||
| urls: { | ||
| seedUrlConfiguration: { | ||
| seedUrls: ["https://aws.amazon.com"] | ||
| } | ||
| }, | ||
| crawlDepth: 1, | ||
| }; | ||
|
|
||
| const webCrawlerSource = { | ||
| name: "web-source", | ||
| roleArn: existingIamRole.roleArn, | ||
| type: "WEBCRAWLER", | ||
| dataSourceConfiguration: { | ||
| webCrawlerConfiguration: sourceProps | ||
| } | ||
| }; |
There was a problem hiding this comment.
This is a nice example of how to crawl.
| }); | ||
|
|
||
| const template = Template.fromStack(stack); | ||
| template.resourceCountIs("AWS::Lambda::Function", 1); |
There was a problem hiding this comment.
Can we add a Name to the existing lambda function, and then assert on that. As is, this test doesn't necessarily prove that a new lambda function wasn't created, since the lambda default timeout is 3 seconds if I recall correctly.
| */ | ||
| readonly indexPermissions?: string[]; | ||
| /** | ||
| * Existing instance of a Kendra Index. Providing both this and kendraCfnIndexProps will cause an error. |
There was a problem hiding this comment.
I may have missed it, but after reviewing the PR, I don't see where we do the error checking if both existingKendraIndexObj and kendraCfnIndexProps is done? Also, I believe the property is actually called kendraIndexProps
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Issue #, if available:
N/A
Description of changes:
Create the new aws-lambda-kendra construct
The original PR (#983 was open too long and got stale, so the code has be added to this newer PR
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.