chore(all): reduce cognitive complexity (#1009)

* extract s3 and sqs prop checking

* removed alb bucket check from CheckS3Props

* Check VPC and S3 props

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

@@ -167,6 +167,7 @@ export class AlbToFargate extends Construct {
     defaults.CheckProps(props);
     defaults.CheckAlbProps(props);
     defaults.CheckFargateProps(props);
+    defaults.CheckVpcProps(props);
 
     // Obtain VPC for construct (existing or created)
     this.vpc = defaults.buildVpc(scope, {

source/patterns/@aws-solutions-constructs/aws-alb-fargate/test/alb-fargate.test.ts

@@ -463,3 +463,24 @@ test('Test HTTPS API with new vpc, load balancer, service and private API', () =
     ]
   });
 });
+
+test('Confirm that CheckVpcProps is called', () => {
+  const stack = new cdk.Stack(undefined, undefined, {
+    env: { account: "123456789012", region: 'us-east-1' },
+  });
+
+  const props: AlbToFargateProps = {
+    ecrRepositoryArn: defaults.fakeEcrRepoArn,
+    listenerProps: {
+      certificates: [defaults.getFakeCertificate(stack, "fake-cert")]
+    },
+    publicApi: false,
+    vpcProps: {},
+    existingVpc: defaults.getTestVpc(stack),
+  };
+  const app = () => {
+    new AlbToFargate(stack, 'new-construct', props);
+  };
+  // Assertion
+  expect(app).toThrowError('Error - Either provide an existingVpc or some combination of deployVpc and vpcProps, but not both.\n');
+});

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

@@ -123,6 +123,7 @@ export class AlbToLambda extends Construct {
     super(scope, id);
     defaults.CheckProps(props);
     defaults.CheckAlbProps(props);
+    defaults.CheckVpcProps(props);
 
     // Obtain VPC for construct (existing or created)
     this.vpc = defaults.buildVpc(scope, {

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

@@ -247,7 +247,7 @@ test("Test existing load balancer and existing lambda function", () => {
     existingLambdaObj: lambdaFunction,
     existingLoadBalancerObj: existingAlb,
     listenerProps: {
-      certificates: [ defaults.getFakeCertificate(stack, "fake-cert") ],
+      certificates: [defaults.getFakeCertificate(stack, "fake-cert")],
     },
     publicApi: true,
     existingVpc: testExistingVpc,
@@ -289,7 +289,7 @@ test('Test new load balancer and new lambda function', () => {
   });
 
   const props: AlbToLambdaProps = {
-    lambdaFunctionProps:  {
+    lambdaFunctionProps: {
       code: lambda.Code.fromAsset(`${__dirname}/lambda`),
       runtime: lambda.Runtime.NODEJS_16_X,
       handler: 'index.handler',
@@ -952,3 +952,28 @@ test('Test existingLoadBalancerObj and no existingVpc is an error', () => {
   expect(app).toThrowError(
     /An existing ALB is already in a VPC, that VPC must be provided in props.existingVpc for the rest of the construct to use./);
 });
+
+test('Confirm that CheckVpcProps is called', () => {
+  const stack = new cdk.Stack(undefined, undefined, {
+    env: { account: "123456789012", region: 'us-east-1' },
+  });
+
+  const props: AlbToLambdaProps = {
+    lambdaFunctionProps: {
+      code: lambda.Code.fromAsset(`${__dirname}/lambda`),
+      runtime: lambda.Runtime.NODEJS_16_X,
+      handler: 'index.handler'
+    },
+    listenerProps: {
+      certificates: [defaults.getFakeCertificate(stack, "fake-cert")]
+    },
+    publicApi: false,
+    vpcProps: {},
+    existingVpc: defaults.getTestVpc(stack),
+  };
+  const app = () => {
+    new AlbToLambda(stack, 'new-construct', props);
+  };
+  // Assertion
+  expect(app).toThrowError('Error - Either provide an existingVpc or some combination of deployVpc and vpcProps, but not both.\n');
+});

source/patterns/@aws-solutions-constructs/aws-apigateway-sqs/lib/index.ts

@@ -197,6 +197,7 @@ export class ApiGatewayToSqs extends Construct {
   constructor(scope: Construct, id: string, props: ApiGatewayToSqsProps) {
     super(scope, id);
     defaults.CheckProps(props);
+    defaults.CheckSqsProps(props);
 
     if (this.CheckCreateRequestProps(props)) {
       throw new Error(`The 'allowCreateOperation' property must be set to true when setting any of the following: ` +

source/patterns/@aws-solutions-constructs/aws-apigateway-sqs/test/apigateway-sqs.test.ts

@@ -12,7 +12,7 @@
  */
 
 // Imports
-import { Stack } from "aws-cdk-lib";
+import { RemovalPolicy, Stack } from "aws-cdk-lib";
 import { ApiGatewayToSqs } from '../lib';
 import * as api from "aws-cdk-lib/aws-apigateway";
 import * as kms from 'aws-cdk-lib/aws-kms';
@@ -634,4 +634,19 @@ test('Construct throws error when deleteIntegrationResponses is set and allowDel
   });
 
   expect(app).toThrowError(/The 'allowDeleteOperation' property must be set to true when setting any of the following: 'deleteRequestTemplate', 'additionalDeleteRequestTemplates', 'deleteIntegrationResponses'/);
-});
+});
+
+test('Confirm the CheckSqsProps is being called', () => {
+  const stack = new Stack();
+
+  const app = () => {
+    new ApiGatewayToSqs(stack, 'api-gateway-sqs', {
+      queueProps: {
+        removalPolicy: RemovalPolicy.DESTROY,
+      },
+      existingQueueObj: new sqs.Queue(stack, 'test', {})
+    });
+  };
+
+  expect(app).toThrowError(/Error - Either provide queueProps or existingQueueObj, but not both.\n/);
+});

source/patterns/@aws-solutions-constructs/aws-cloudfront-s3/lib/index.ts

@@ -112,6 +112,7 @@ export class CloudFrontToS3 extends Construct {
     this.node.setContext("@aws-cdk/aws-s3:serverAccessLogsUseBucketPolicy", true);
 
     defaults.CheckProps(props);
+    defaults.CheckS3Props(props);
 
     let bucket: s3.IBucket;
 

source/patterns/@aws-solutions-constructs/aws-cloudfront-s3/test/test.cloudfront-s3.test.ts

@@ -147,10 +147,7 @@ test('check properties', () => {
   expect(construct.s3Bucket !== null);
 });
 
-// --------------------------------------------------------------
-// Test bad call with existingBucket and bucketProps
-// --------------------------------------------------------------
-test("Test bad call with existingBucket and bucketProps", () => {
+test("Confirm CheckS3Props is called", () => {
   // Stack
   const stack = new cdk.Stack();
 
@@ -166,7 +163,7 @@ test("Test bad call with existingBucket and bucketProps", () => {
     });
   };
   // Assertion
-  expect(app).toThrowError();
+  expect(app).toThrowError('Error - Either provide bucketProps or existingBucketObj, but not both.\n');
 });
 
 test("Test existingBucketObj", () => {

source/patterns/@aws-solutions-constructs/aws-dynamodbstreams-lambda-elasticsearch-kibana/test/dynamodbstreams-lambda-elasticsearch-kibana.test.ts

@@ -226,4 +226,22 @@ test('Test minimal deployment with an existing isolated VPC', () => {
 
   template.resourceCountIs("AWS::EC2::VPC", 1);
   expect(construct.vpc).toBeDefined();
-});
+});
+
+test('Confirm CheckVpcProps is being called', () => {
+  const stack = new cdk.Stack();
+
+  const app = () => {
+    new DynamoDBStreamsToLambdaToElasticSearchAndKibana(stack, 'test-construct', {
+      lambdaFunctionProps: getDefaultTestLambdaProps(),
+      domainName: "test",
+      deployVpc: true,
+      vpcProps: {
+        vpcName: "existing-vpc-test"
+      },
+      existingVpc: defaults.getTestVpc(stack),
+    });
+  };
+
+  expect(app).toThrowError('Error - Either provide an existingVpc or some combination of deployVpc and vpcProps, but not both.\n');
+});

source/patterns/@aws-solutions-constructs/aws-eventbridge-kinesisfirehose-s3/test/eventbridge-kinesisfirehose-s3.test.ts

@@ -291,3 +291,23 @@ test('Supply an existing logging bucket', () => {
   template.resourceCountIs("AWS::S3::Bucket", 2);
 
 });
+
+test('Confirm CheckS3Props is being called', () => {
+  // For L4 constructs, the call is within the internal L3 constructs
+  const stack = new cdk.Stack();
+
+  const props: EventbridgeToKinesisFirehoseToS3Props = {
+    eventRuleProps: {
+      eventPattern: {
+        source: ['solutionsconstructs']
+      }
+    },
+    bucketProps: {},
+    existingBucketObj: new s3.Bucket(stack, 'test-bucket', {}),
+  };
+
+  const app = () => {
+    new EventbridgeToKinesisFirehoseToS3(stack, 'test-eventbridge-firehose', props);
+  };
+  expect(app).toThrowError('Error - Either provide bucketProps or existingBucketObj, but not both.\n');
+});

source/patterns/@aws-solutions-constructs/aws-eventbridge-sns/lib/index.ts

@@ -88,6 +88,7 @@ export class EventbridgeToSns extends Construct {
     constructor(scope: Construct, id: string, props: EventbridgeToSnsProps) {
       super(scope, id);
       defaults.CheckProps(props);
+      defaults.CheckSnsProps(props);
 
       let enableEncryptionParam = props.enableEncryptionWithCustomerManagedKey;
       if (props.enableEncryptionWithCustomerManagedKey === undefined ||

source/patterns/@aws-solutions-constructs/aws-eventbridge-sns/test/eventbridge-sns-topic.test.ts

@@ -390,4 +390,24 @@ test('Properties correctly set when unencrypted existing topic is provided', ()
 
   expect(testConstruct.snsTopic).toBeDefined();
   expect(testConstruct.encryptionKey).not.toBeDefined();
+});
+
+test('Confirm CheckSnsProps is being called', () => {
+  const stack = new cdk.Stack();
+  const existingTopicObj = new sns.Topic(stack, 'Topic', {
+    topicName: 'existing-topic-name'
+  });
+
+  const props: EventbridgeToSnsProps = {
+    existingTopicObj,
+    topicProps: {},
+    eventRuleProps: {
+      schedule: events.Schedule.rate(cdk.Duration.minutes(5))
+    }
+  };
+  const app = () => {
+    new EventbridgeToSns(stack, 'test', props);
+  };
+
+  expect(app).toThrowError("Error - Either provide topicProps or existingTopicObj, but not both.\n");
 });

source/patterns/@aws-solutions-constructs/aws-eventbridge-sqs/lib/index.ts

@@ -117,6 +117,7 @@ export class EventbridgeToSqs extends Construct {
   constructor(scope: Construct, id: string, props: EventbridgeToSqsProps) {
     super(scope, id);
     defaults.CheckProps(props);
+    defaults.CheckSqsProps(props);
 
     // Setup the dead letter queue, if applicable
     this.deadLetterQueue = defaults.buildDeadLetterQueue(this, {

source/patterns/@aws-solutions-constructs/aws-eventbridge-sqs/test/eventbridge-sqs-queue.test.ts

@@ -14,6 +14,7 @@
 import * as cdk from 'aws-cdk-lib';
 import { EventbridgeToSqs, EventbridgeToSqsProps } from '../lib';
 import * as events from "aws-cdk-lib/aws-events";
+import * as sqs from "aws-cdk-lib/aws-sqs";
 import { Template } from 'aws-cdk-lib/assertions';
 import * as defaults from '@aws-solutions-constructs/core';
 
@@ -535,4 +536,26 @@ test('Queue purging flag grants correct permissions', () => {
       }
     ]
   });
-});
+});
+
+test('check that CheckSqsProps is being called', () => {
+  const stack = new cdk.Stack();
+
+  const props: EventbridgeToSqsProps = {
+    eventRuleProps: {
+      eventPattern: {
+        source: ['solutionsconstructs']
+      }
+    },
+    eventBusProps: { eventBusName: 'test' },
+    queueProps: {
+      removalPolicy: cdk.RemovalPolicy.DESTROY,
+    },
+    existingQueueObj: new sqs.Queue(stack, 'test', {})
+  };
+
+  const app = () => {
+    new EventbridgeToSqs(stack, 'test-eventbridge-sqs', props);
+  };
+  expect(app).toThrowError("Error - Either provide queueProps or existingQueueObj, but not both.\n");
+});

source/patterns/@aws-solutions-constructs/aws-fargate-dynamodb/lib/index.ts

@@ -143,6 +143,7 @@ export class FargateToDynamoDB extends Construct {
     defaults.CheckProps(props);
     defaults.CheckFargateProps(props);
     defaults.CheckDynamoDBProps(props);
+    defaults.CheckVpcProps(props);
 
     // Other permissions for constructs are accepted as arrays, turning tablePermissions into
     // an array to use the same validation function.

source/patterns/@aws-solutions-constructs/aws-fargate-dynamodb/test/fargate-dynamodb.test.ts

@@ -14,7 +14,7 @@
 import { Template } from 'aws-cdk-lib/assertions';
 import * as defaults from '@aws-solutions-constructs/core';
 import * as cdk from "aws-cdk-lib";
-import { FargateToDynamoDB } from "../lib";
+import { FargateToDynamoDB, FargateToDynamoDBProps } from "../lib";
 import * as dynamodb from 'aws-cdk-lib/aws-dynamodb';
 import * as ecs from 'aws-cdk-lib/aws-ecs';
 import * as ec2 from 'aws-cdk-lib/aws-ec2';
@@ -713,4 +713,37 @@ test('test that DDB input args are getting checked', () => {
   };
 
   expect(app).toThrowError('Error - Either provide existingTableInterface or dynamoTableProps, but not both.\n');
-});
+});
+
+test('Confirm that CheckVpcProps was called', () => {
+  const stack = new cdk.Stack();
+  const publicApi = true;
+  const clusterName = "custom-cluster-name";
+  const containerName = "custom-container-name";
+  const serviceName = "custom-service-name";
+  const familyName = "custom-family-name";
+
+  const props: FargateToDynamoDBProps = {
+    publicApi,
+    ecrRepositoryArn: defaults.fakeEcrRepoArn,
+    clusterProps: { clusterName },
+    containerDefinitionProps: { containerName },
+    fargateTaskDefinitionProps: { family: familyName },
+    fargateServiceProps: { serviceName },
+    dynamoTableProps: {
+      tableName: 'fake-name',
+      partitionKey: {
+        name: 'id',
+        type: dynamodb.AttributeType.STRING
+      },
+    },
+    existingVpc: defaults.getTestVpc(stack),
+    vpcProps: {  },
+  };
+
+  const app = () => {
+    new FargateToDynamoDB(stack, 'test-construct', props);
+  };
+  // Assertion
+  expect(app).toThrowError('Error - Either provide an existingVpc or some combination of deployVpc and vpcProps, but not both.\n');
+});

source/patterns/@aws-solutions-constructs/aws-fargate-eventbridge/lib/index.ts

@@ -134,6 +134,7 @@ export class FargateToEventbridge extends Construct {
     super(scope, id);
     defaults.CheckProps(props);
     defaults.CheckFargateProps(props);
+    defaults.CheckVpcProps(props);
 
     this.vpc = defaults.buildVpc(scope, {
       existingVpc: props.existingVpc,

source/patterns/@aws-solutions-constructs/aws-fargate-eventbridge/test/fargate-eventbridge.test.ts

@@ -14,7 +14,7 @@
 import { Template } from 'aws-cdk-lib/assertions';
 import * as defaults from '@aws-solutions-constructs/core';
 import * as cdk from "aws-cdk-lib";
-import { FargateToEventbridge } from "../lib";
+import { FargateToEventbridge, FargateToEventbridgeProps } from "../lib";
 import * as events from 'aws-cdk-lib/aws-events';
 import * as ecs from 'aws-cdk-lib/aws-ecs';
 import * as ec2 from 'aws-cdk-lib/aws-ec2';
@@ -372,4 +372,29 @@ function createFargateConstructWithNewResources(stack: cdk.Stack, publicApi: boo
       eventBusName: 'custom-name'
     }
   });
-}
+}
+
+test('Confirm that CheckVpcProps was called', () => {
+  const stack = new cdk.Stack();
+  const publicApi = true;
+
+  const props: FargateToEventbridgeProps = {
+    publicApi,
+    ecrRepositoryArn: defaults.fakeEcrRepoArn,
+    clusterProps: { clusterName },
+    containerDefinitionProps: { containerName },
+    fargateTaskDefinitionProps: { family: familyName },
+    fargateServiceProps: { serviceName },
+    eventBusProps: {
+      eventBusName: 'custom-name'
+    },
+    existingVpc: defaults.getTestVpc(stack),
+    vpcProps: {  },
+  };
+
+  const app = () => {
+    new FargateToEventbridge(stack, 'test-construct', props);
+  };
+  // Assertion
+  expect(app).toThrowError('Error - Either provide an existingVpc or some combination of deployVpc and vpcProps, but not both.\n');
+});

source/patterns/@aws-solutions-constructs/aws-fargate-kinesisfirehose/lib/index.ts

@@ -127,6 +127,7 @@ export class FargateToKinesisFirehose extends Construct {
     super(scope, id);
     defaults.CheckProps(props);
     defaults.CheckFargateProps(props);
+    defaults.CheckVpcProps(props);
 
     if (!props.existingKinesisFirehose.deliveryStreamName) {
       throw new Error('existingKinesisFirehose must have a defined deliveryStreamName');