fix more Cognitive issues (input-validation) (#1008)

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

@@ -142,6 +142,7 @@ export class FargateToDynamoDB extends Construct {
     super(scope, id);
     defaults.CheckProps(props);
     defaults.CheckFargateProps(props);
+    defaults.CheckDynamoDBProps(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

@@ -669,4 +669,48 @@ test('test error invalid table permission', () => {
   };
 
   expect(app).toThrowError('Invalid tablePermission submitted - REED');
+});
+
+test('test that DDB input args are getting checked', () => {
+  const stack = new cdk.Stack();
+  const publicApi = false;
+  const serviceName = 'custom-name';
+  const tableName = 'custom-table-name';
+
+  const existingVpc = defaults.getTestVpc(stack, publicApi);
+
+  const createFargateServiceResponse = defaults.CreateFargateService(stack, 'test', {
+    constructVpc: existingVpc,
+    ecrRepositoryArn: defaults.fakeEcrRepoArn,
+    clientFargateServiceProps: {
+      serviceName
+    }
+  });
+
+  const existingTable = new dynamodb.Table(stack, 'MyTablet', {
+    tableName,
+    partitionKey: {
+      name: 'id',
+      type: dynamodb.AttributeType.STRING
+    }
+  });
+
+  const app = () => {
+    new FargateToDynamoDB(stack, 'test-construct', {
+      publicApi,
+      existingFargateServiceObject: createFargateServiceResponse.service,
+      existingContainerDefinitionObject: createFargateServiceResponse.containerDefinition,
+      existingVpc,
+      existingTableInterface: existingTable,
+      dynamoTableProps: {
+        tableName,
+        partitionKey: {
+          name: 'id',
+          type: dynamodb.AttributeType.STRING
+        },
+      },
+    });
+  };
+
+  expect(app).toThrowError('Error - Either provide existingTableInterface or dynamoTableProps, but not both.\n');
 });

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

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

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

@@ -496,4 +496,34 @@ test('Topic is encrypted with customer managed KMS Key when enable encryption fl
       ]
     },
   });
-});
+});
+
+test('Confirm CheckSnsProps is being called', () => {
+  // An environment with region is required to enable logging on an ALB
+  const stack = new cdk.Stack(undefined, undefined, {
+    env: { account: "123456789012", region: 'us-east-1' },
+  });
+  const publicApi = false;
+  const topicName = 'custom-topic-name';
+
+  const existingVpc = defaults.getTestVpc(stack, publicApi);
+
+  const existingTopic = new sns.Topic(stack, 'MyTopic', {
+    topicName
+  });
+
+  const app = () => {
+    new FargateToSns(stack, 'test-construct', {
+      publicApi,
+      existingVpc,
+      ecrRepositoryArn: defaults.fakeEcrRepoArn,
+      existingTopicObject: existingTopic,
+      topicProps: {
+        topicName: 'topic-name'
+      },
+    });
+  };
+
+  // Assertion
+  expect(app).toThrowError(/Error - Either provide topicProps or existingTopicObj, but not both.\n/);
+});

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

@@ -95,6 +95,7 @@ export class IotToLambdaToDynamoDB extends Construct {
   constructor(scope: Construct, id: string, props: IotToLambdaToDynamoDBProps) {
     super(scope, id);
     defaults.CheckProps(props);
+    defaults.CheckSnsProps(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-iot-lambda-dynamodb/test/iot-lambda-dynamodb.test.ts

@@ -616,3 +616,50 @@ test("Test bad call with existingVpc and deployVpc", () => {
   // Assertion
   expect(app).toThrowError();
 });
+
+// NOTE: existingTableObj was omitted from the interface for this construct,
+// so this test cannot be run. Leaving it here so it can be used if/when existingTableObj
+// is added to the interface
+//
+// test("Confirm CheckDynamoDBProps is getting called", () => {
+//   const stack = new cdk.Stack();
+//   const tableName = 'table-name';
+
+//   const existingTable = new dynamodb.Table(stack, 'MyTablet', {
+//     tableName,
+//     partitionKey: {
+//       name: 'id',
+//       type: dynamodb.AttributeType.STRING
+//     }
+//   });
+
+//   const props: IotToLambdaToDynamoDBProps = {
+//     lambdaFunctionProps: {
+//       code: lambda.Code.fromAsset(`${__dirname}/lambda`),
+//       runtime: lambda.Runtime.NODEJS_16_X,
+//       handler: 'index.handler'
+//     },
+//     iotTopicRuleProps: {
+//       topicRulePayload: {
+//         ruleDisabled: false,
+//         description: "Processing of DTC messages from the AWS Connected Vehicle Solution.",
+//         sql: "SELECT * FROM 'connectedcar/dtc/#'",
+//         actions: []
+//       }
+//     },
+//     existingTableObj: existingTable,
+//     dynamoTableProps: {
+//       tableName,
+//       partitionKey: {
+//         name: 'id',
+//         type: dynamodb.AttributeType.STRING
+//       },
+//     },
+// };
+
+//   const app = () => {
+//     new IotToLambdaToDynamoDB(stack, 'test-iot-lambda-dynamodb-stack', props);
+//   };
+
+//   expect(app).toThrowError('Error - Either provide existingTableObj or dynamoTableProps, but not both.\n');
+// });

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

@@ -91,6 +91,7 @@ export class LambdaToDynamoDB extends Construct {
   constructor(scope: Construct, id: string, props: LambdaToDynamoDBProps) {
     super(scope, id);
     defaults.CheckProps(props);
+    defaults.CheckDynamoDBProps(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-lambda-dynamodb/test/lambda-dynamodb.test.ts

@@ -802,4 +802,40 @@ test('Test bad table permission', () => {
 
   // Assertion
   expect(app).toThrowError(/Invalid table permission submitted - Reed/);
+});
+
+test('Test that CheckDynamoDBProps is getting called', () => {
+  const stack = new cdk.Stack();
+  const tableName = 'custom-table-name';
+
+  const existingTable = new dynamodb.Table(stack, 'MyTablet', {
+    tableName,
+    partitionKey: {
+      name: 'id',
+      type: dynamodb.AttributeType.STRING
+    }
+  });
+
+  const props: LambdaToDynamoDBProps = {
+    lambdaFunctionProps: {
+      code: lambda.Code.fromAsset(`${__dirname}/lambda`),
+      runtime: lambda.Runtime.NODEJS_16_X,
+      handler: 'index.handler'
+    },
+    existingTableObj: existingTable,
+    dynamoTableProps: {
+      tableName,
+      partitionKey: {
+        name: 'id',
+        type: dynamodb.AttributeType.STRING
+      },
+    },
+  };
+
+  const app = () => {
+    new LambdaToDynamoDB(stack, 'test-lambda-dynamodb-stack', props);
+  };
+
+  // Assertion
+  expect(app).toThrowError(/Error - Either provide existingTableObj or dynamoTableProps, but not both.\n/);
 });

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

@@ -114,6 +114,7 @@ export class LambdaToSns extends Construct {
     constructor(scope: Construct, id: string, props: LambdaToSnsProps) {
       super(scope, id);
       defaults.CheckProps(props);
+      defaults.CheckSnsProps(props);
 
       if (props.deployVpc || props.existingVpc) {
         this.vpc = defaults.buildVpc(scope, {

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

@@ -578,4 +578,31 @@ test('Error is thrown when conflicting VPC information is provided', () => {
   };
 
   expect(app).toThrowError('Error - Either provide an existingVpc or some combination of deployVpc and vpcProps, but not both.');
+});
+
+test('Test that CheckSnsProps is getting called', () => {
+  const stack = new Stack();
+
+  const topic = new sns.Topic(stack, 'MyTopic', {
+    topicName: "custom-topic"
+  });
+
+  const props: LambdaToSnsProps = {
+    lambdaFunctionProps: {
+      code: lambda.Code.fromAsset(`${__dirname}/lambda`),
+      runtime: lambda.Runtime.NODEJS_16_X,
+      handler: 'index.handler'
+    },
+    existingTopicObj: topic,
+    topicProps: {
+      topicName: 'topic-name'
+    },
+  };
+
+  const app = () => {
+    new LambdaToSns(stack, 'test-lambda-dynamodb-stack', props);
+  };
+
+  // Assertion
+  expect(app).toThrowError(/Error - Either provide topicProps or existingTopicObj, but not both.\n/);
 });

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

@@ -86,6 +86,7 @@ export class SnsToLambda extends Construct {
   constructor(scope: Construct, id: string, props: SnsToLambdaProps) {
     super(scope, id);
     defaults.CheckProps(props);
+    defaults.CheckSnsProps(props);
 
     // Setup the Lambda function
     this.lambdaFunction = defaults.buildLambdaFunction(this, {

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

@@ -236,3 +236,30 @@ test('Topic is encrypted with customer managed KMS Key when enable encryption fl
     },
   });
 });
+
+test('Confirm CheckSnsProps is getting called', () => {
+  const stack = new cdk.Stack();
+
+  const topic = new sns.Topic(stack, 'MyTopic', {
+    topicName: "custom-topic"
+  });
+
+  const props: SnsToLambdaProps = {
+    lambdaFunctionProps: {
+      code: lambda.Code.fromAsset(`${__dirname}/lambda`),
+      runtime: lambda.Runtime.NODEJS_16_X,
+      handler: 'index.handler'
+    },
+    existingTopicObj: topic,
+    topicProps: {
+      topicName: 'topic-name'
+    }
+  };
+
+  const app = () => {
+    new SnsToLambda(stack, 'test-sns-lambda', props);
+  };
+
+  // Assertion
+  expect(app).toThrowError(/Error - Either provide topicProps or existingTopicObj, but not both.\n/);
+});

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

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

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

@@ -132,9 +132,6 @@ test('Test getter methods', () => {
   expect(app.deadLetterQueue !== null);
 });
 
-// --------------------------------------------------------------
-// Test deployment with existing queue, and topic
-// --------------------------------------------------------------
 test('Test deployment w/ existing queue, and topic', () => {
   // Stack
   const stack = new Stack();
@@ -162,9 +159,6 @@ test('Test deployment w/ existing queue, and topic', () => {
   });
 });
 
-// --------------------------------------------------------------
-// Test deployment with imported encryption key
-// --------------------------------------------------------------
 test('Test deployment with imported encryption key', () => {
   // Stack
   const stack = new Stack();
@@ -499,4 +493,29 @@ test('Construct does not override unencrypted topic when passed in existingTopic
 
   expect(testConstruct.snsTopic).toBeDefined();
   expect(testConstruct.encryptionKey).not.toBeDefined();
+});
+
+test('Confirm that CheckSnsProps is called', () => {
+  // Stack
+  const stack = new Stack();
+  // Helper declaration
+  const topic = new sns.Topic(stack, "existing-topic-obj", {
+    topicName: 'existing-topic-obj'
+  });
+  const queue = new sqs.Queue(stack, 'existing-queue-obj', {
+    queueName: 'existing-queue-obj'
+  });
+
+  const app = () => {
+    new SnsToSqs(stack, 'sns-to-sqs-stack', {
+      existingTopicObj: topic,
+      topicProps: {
+        topicName: 'topic-name'
+      },
+      existingQueueObj: queue
+    });
+  };
+
+  // Assertion
+  expect(app).toThrowError(/Error - Either provide topicProps or existingTopicObj, but not both.\n/);
 });

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

@@ -224,16 +224,7 @@ export function CheckFargateProps(props: any) {
   let errorMessages = "";
   let errorFound = false;
 
-  if (
-    props.existingFargateServiceObject &&
-    (props.existingImageObject ||
-      props.ecrImageVersion ||
-      props.containerDefinitionProps ||
-      props.fargateTaskDefinitionProps ||
-      props.ecrRepositoryArn ||
-      props.fargateServiceProps ||
-      props.clusterProps)
-  ) {
+  if (CheckForConflictingServiceProps(props)) {
     errorFound = true;
     errorMessages +=
       "If you provide an existingFargateServiceObject, you cannot provide any props defining a new service\n";
@@ -290,6 +281,24 @@ export function CheckFargateProps(props: any) {
   }
 }
 
+/**
+ * @internal This is an internal core function and should not be called directly by Solutions Constructs clients.
+ */
+export function CheckForConflictingServiceProps(props: any): boolean {
+  if (props.existingFargateServiceObject &&
+    (props.existingImageObject ||
+      props.ecrImageVersion ||
+      props.containerDefinitionProps ||
+      props.fargateTaskDefinitionProps ||
+      props.ecrRepositoryArn ||
+      props.fargateServiceProps ||
+      props.clusterProps)
+  ) {
+    return true;
+  }
+  return false;
+}
+
 /**
  * @internal This is an internal core function and should not be called directly by Solutions Constructs clients.
  */