Skip to content

Commit

Permalink
Address Code Review Comments
Browse files Browse the repository at this point in the history
  • Loading branch information
biffgaut committed Aug 28, 2023
1 parent 1b78c31 commit 8d97cad
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,10 @@ TBD
| --- | --- | --- |
| 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.) |
| 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. Providing both these and existingKendraIndexObj is an error. |
| 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. |
| existingKendraIndexObj? | [`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. |
| existingVpc? | [`ec2.IVpc`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_ec2.IVpc.html) | An optional, existing VPC into which this pattern should be deployed. When deployed in a VPC, the Lambda function will use ENIs in the VPC to access network resources. If an existing VPC is provided, the `deployVpc` property cannot be `true`. This uses `ec2.IVpc` to allow clients to supply VPCs that exist outside the stack using the [`ec2.Vpc.fromLookup()`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_ec2.Vpc.html#static-fromwbrlookupscope-id-options) method. |
| vpcProps? | [`ec2.VpcProps`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_ec2.VpcProps.html) | Optional user provided properties to override the default properties for the new VPC. `enableDnsHostnames`, `enableDnsSupport`, `natGateways` and `subnetConfiguration` are set by the pattern, so any values for those properties supplied here will be overridden. If `deployVpc` is not `true` then this property will be ignored. |
| deployVpc? | `boolean` | Whether to create a new VPC based on `vpcProps` into which to deploy this pattern. Setting this to true will deploy the minimal, most private VPC to run the pattern:<br>\- One isolated subnet in each Availability Zone used by the CDK program<br>\- `enableDnsHostnames` and `enableDnsSupport` will both be set to true<br>If this property is `true` then `existingVpc` cannot be specified. Defaults to `false`. |
Expand All @@ -86,8 +86,7 @@ TBD
| --- | --- | --- |
| 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 {
[]`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_kendra.CfnDataSource.html) | A list of data sources created for this construct/index, each in an object that includes the role for that data source. |
| kendraDataSources | DataSourceProperties[] (this interface is defined by Solutions Constructs and described below) | A list of data sources created for this construct/index, each in an object that includes the role for that data source. |
| 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. |
Expand Down Expand Up @@ -126,7 +125,7 @@ Out of the box implementation of the Construct without any overrides will set th
## Architecture
![Architecture Diagram](/Applications/Joplin.app/Contents/Resources/app.asar/architecture.png)
![Architecture Diagram](architecture.png)
* * *
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export interface LambdaToKendraProps {
*/
readonly indexPermissions?: string[];
/**
* Existing instance of a Kendra Index. Providing both this and kendraCfnIndexProps will cause an error.
* Existing instance of a Kendra Index. Providing both this and kendraIndexProps will cause an error.
*
* @default - None
*/
Expand Down Expand Up @@ -106,8 +106,15 @@ export class LambdaToKendra extends Construct {
super(scope, id);
defaults.CheckProps(props);

if (props.deployVpc || props.existingVpc) {
if (props.kendraIndexProps && props.existingKendraIndexObj) {
throw new Error('You may not provide both kendraIndexProps and existingKendraIndexObj');
}

if (props.kendraIndexProps && props.kendraDataSourcesProps) {
throw new Error('You may not provide both kendraDataSourcesProps and existingKendraIndexObj');
}

if (props.deployVpc || props.existingVpc) {
this.vpc = defaults.buildVpc(scope, {
defaultVpcProps: defaults.DefaultIsolatedVpcProps(),
existingVpc: props.existingVpc,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -420,13 +420,16 @@ test('Launch with VPC', () => {

test('Launch with existing lambda', () => {
const stack = new cdk.Stack();
const testTimeout = 3;
const testTimeout = 17;

const functionName = 'test-name';

const existingFunction = new lambda.Function(stack, 'existing-function', {
code: lambda.Code.fromAsset(`${__dirname}/lambda`),
runtime: lambda.Runtime.NODEJS_18_X,
handler: 'index.handler',
timeout: cdk.Duration.seconds(testTimeout),
functionName: functionName
});

new LambdaToKendra(stack, 'sample', {
Expand All @@ -437,7 +440,8 @@ test('Launch with existing lambda', () => {
const template = Template.fromStack(stack);
template.resourceCountIs("AWS::Lambda::Function", 1);
template.hasResourceProperties("AWS::Lambda::Function", {
Timeout: testTimeout
Timeout: testTimeout,
FunctionName: functionName,
});
});

Expand Down

0 comments on commit 8d97cad

Please sign in to comment.