chore(all): address sonarqube code smells

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

@@ -205,23 +205,19 @@ export class ApiGatewayToDynamoDB extends Construct {
     super(scope, id);
     defaults.CheckProps(props);
 
-    if ((props.createRequestTemplate || props.additionalCreateRequestTemplates || props.createIntegrationResponses)
-        && props.allowCreateOperation !== true) {
+    if (this.CheckCreateRequestProps(props)) {
       throw new Error(`The 'allowCreateOperation' property must be set to true when setting any of the following: ` +
         `'createRequestTemplate', 'additionalCreateRequestTemplates', 'createIntegrationResponses'`);
     }
-    if ((props.readRequestTemplate || props.additionalReadRequestTemplates || props.readIntegrationResponses)
-        && props.allowReadOperation === false) {
+    if (this.CheckReadRequestProps(props)) {
       throw new Error(`The 'allowReadOperation' property must be set to true or undefined when setting any of the following: ` +
         `'readRequestTemplate', 'additionalReadRequestTemplates', 'readIntegrationResponses'`);
     }
-    if ((props.updateRequestTemplate || props.additionalUpdateRequestTemplates || props.updateIntegrationResponses)
-        && props.allowUpdateOperation !== true) {
+    if (this.CheckUpdateRequestProps(props)) {
       throw new Error(`The 'allowUpdateOperation' property must be set to true when setting any of the following: ` +
         `'updateRequestTemplate', 'additionalUpdateRequestTemplates', 'updateIntegrationResponses'`);
     }
-    if ((props.deleteRequestTemplate || props.additionalDeleteRequestTemplates || props.deleteIntegrationResponses)
-        && props.allowDeleteOperation !== true) {
+    if (this.CheckDeleteRequestProps(props)) {
       throw new Error(`The 'allowDeleteOperation' property must be set to true when setting any of the following: ` +
         `'deleteRequestTemplate', 'additionalDeleteRequestTemplates', 'deleteIntegrationResponses'`);
     }
@@ -261,8 +257,9 @@ export class ApiGatewayToDynamoDB extends Construct {
 
     // Setup API Gateway Method
     // Create
-    if (props.allowCreateOperation && props.allowCreateOperation === true && props.createRequestTemplate) {
-      const createRequestTemplate = props.createRequestTemplate.replace("${Table}", this.dynamoTable.tableName);
+    if (this.ImplementCreateOperation(props)) {
+      // ImplementCreateOperation has confirmed that createRequestTemplate exists)
+      const createRequestTemplate = props.createRequestTemplate!.replace("${Table}", this.dynamoTable.tableName);
       this.addActionToPolicy("dynamodb:PutItem");
       defaults.addProxyMethodToApiResource({
         service: "dynamodb",
@@ -276,7 +273,7 @@ export class ApiGatewayToDynamoDB extends Construct {
       });
     }
     // Read
-    if (props.allowReadOperation === undefined || props.allowReadOperation === true) {
+    if (this.ImplementReaOperation(props)) {
       const readRequestTemplate = props.readRequestTemplate ??
         `{ \
           "TableName": "${this.dynamoTable.tableName}", \
@@ -301,8 +298,9 @@ export class ApiGatewayToDynamoDB extends Construct {
       });
     }
     // Update
-    if (props.allowUpdateOperation && props.allowUpdateOperation === true && props.updateRequestTemplate) {
-      const updateRequestTemplate = props.updateRequestTemplate.replace("${Table}", this.dynamoTable.tableName);
+    if (this.ImplementUpdateOperation(props)) {
+      // ImplementUpdateOperation confirmed the existence of updateRequestTemplate
+      const updateRequestTemplate = props.updateRequestTemplate!.replace("${Table}", this.dynamoTable.tableName);
       this.addActionToPolicy("dynamodb:UpdateItem");
       defaults.addProxyMethodToApiResource({
         service: "dynamodb",
@@ -316,7 +314,7 @@ export class ApiGatewayToDynamoDB extends Construct {
       });
     }
     // Delete
-    if (props.allowDeleteOperation && props.allowDeleteOperation === true) {
+    if (this.ImplementDeleteOperation(props)) {
       const deleteRequestTemplate = props.deleteRequestTemplate ??
         `{ \
           "TableName": "${this.dynamoTable.tableName}", \
@@ -342,6 +340,64 @@ export class ApiGatewayToDynamoDB extends Construct {
     }
   }
 
+  private CheckReadRequestProps(props: ApiGatewayToDynamoDBProps): boolean {
+    if ((props.readRequestTemplate || props.additionalReadRequestTemplates || props.readIntegrationResponses)
+        && props.allowReadOperation === false) {
+      return true;
+    }
+    return false;
+  }
+  private CheckUpdateRequestProps(props: ApiGatewayToDynamoDBProps): boolean {
+    if ((props.updateRequestTemplate || props.additionalUpdateRequestTemplates || props.updateIntegrationResponses)
+        && props.allowUpdateOperation !== true) {
+      return true;
+    }
+    return false;
+  }
+  private CheckDeleteRequestProps(props: ApiGatewayToDynamoDBProps): boolean {
+    if ((props.deleteRequestTemplate || props.additionalDeleteRequestTemplates || props.deleteIntegrationResponses)
+        && props.allowDeleteOperation !== true)  {
+      return true;
+    }
+    return false;
+  }
+
+  private CheckCreateRequestProps(props: ApiGatewayToDynamoDBProps): boolean {
+    if ((props.createRequestTemplate || props.additionalCreateRequestTemplates || props.createIntegrationResponses)
+        && props.allowCreateOperation !== true) {
+      return true;
+    }
+    return false;
+  }
+
+  private ImplementCreateOperation(props: ApiGatewayToDynamoDBProps): boolean {
+    if (props.allowCreateOperation && props.allowCreateOperation === true && props.createRequestTemplate) {
+      return true;
+    }
+    return false;
+  }
+
+  private ImplementReaOperation(props: ApiGatewayToDynamoDBProps): boolean {
+    if (props.allowReadOperation === undefined || props.allowReadOperation === true) {
+      return true;
+    }
+    return false;
+  }
+
+  private ImplementUpdateOperation(props: ApiGatewayToDynamoDBProps): boolean {
+    if (props.allowUpdateOperation && props.allowUpdateOperation === true && props.updateRequestTemplate) {
+      return true;
+    }
+    return false;
+  }
+
+  private ImplementDeleteOperation(props: ApiGatewayToDynamoDBProps): boolean {
+    if (props.allowDeleteOperation && props.allowDeleteOperation === true) {
+      return true;
+    }
+    return false;
+  }
+
   private addActionToPolicy(action: string) {
     this.apiGatewayRole.addToPolicy(new iam.PolicyStatement({
       resources: [

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

@@ -93,6 +93,25 @@ test('check properties with no CW Alarms ', () => {
   expect(construct.elasticsearchRole).toBeDefined();
 });
 
+test('Check that TLS 1.2 is the default', () => {
+  const stack = new cdk.Stack();
+
+  const props: LambdaToElasticSearchAndKibanaProps = {
+    lambdaFunctionProps: getDefaultTestLambdaProps(),
+    domainName: 'test-domain',
+    createCloudWatchAlarms: false
+  };
+
+  new LambdaToElasticSearchAndKibana(stack, 'test-lambda-opensearch-stack', props);
+  const template = Template.fromStack(stack);
+  template.hasResourceProperties("AWS::Elasticsearch::Domain", {
+    DomainEndpointOptions: {
+      EnforceHTTPS: true,
+      TLSSecurityPolicy: "Policy-Min-TLS-1-2-2019-07"
+    },
+  });
+});
+
 test('check lambda function custom environment variable', () => {
   const stack = new cdk.Stack();
   const props: LambdaToElasticSearchAndKibanaProps = {

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

@@ -69,6 +69,25 @@ test('Check properties with no CloudWatch alarms ', () => {
   expect(construct.openSearchRole).toBeDefined();
 });
 
+test('Check that TLS 1.2 is the default', () => {
+  const stack = new cdk.Stack();
+
+  const props: LambdaToOpenSearchProps = {
+    lambdaFunctionProps: getDefaultTestLambdaProps(),
+    openSearchDomainName: 'test-domain',
+    createCloudWatchAlarms: false
+  };
+
+  new LambdaToOpenSearch(stack, 'test-lambda-opensearch-stack', props);
+  const template = Template.fromStack(stack);
+  template.hasResourceProperties("AWS::OpenSearchService::Domain", {
+    DomainEndpointOptions: {
+      EnforceHTTPS: true,
+      TLSSecurityPolicy: "Policy-Min-TLS-1-2-2019-07"
+    },
+  });
+});
+
 test('Check for an existing Lambda object', () => {
   const stack = new cdk.Stack();
 

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

@@ -67,7 +67,8 @@ export class WafwebaclToAlb extends Construct {
     };
 
     // Setup the Web ACL Association
-    new waf.CfnWebACLAssociation(scope, `${id}-WebACLAssociation`, aclProps);
+    // Before turning off SonarQube for the line, reduce the line to it's minimum
+    new waf.CfnWebACLAssociation(scope, `${id}-WebACLAssociation`, aclProps); // NOSONAR
 
     this.loadBalancer = props.existingLoadBalancerObj;
   }

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

@@ -112,6 +112,9 @@ export function AddListener(
       protocol: "HTTPS",
     };
 
+    // NOSONAR: (typescript:S5332)
+    // This listener is explicitly created to redirect non TLS connections
+    // The lack of SSL/TLS is intentional
     const httpListener = new elb.ApplicationListener(
       scope,
       `${id}-redirect`,

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

@@ -83,7 +83,10 @@ export function buildElasticSearch(scope: Construct, props: BuildElasticSearchPr
   const defaultCfnDomainProps = DefaultCfnDomainProps(props.domainName, cognitoKibanaConfigureRole, props);
   const finalCfnDomainProps = consolidateProps(defaultCfnDomainProps, props.clientDomainProps, constructDrivenProps);
 
-  const esDomain = new elasticsearch.CfnDomain(scope, `ElasticsearchDomain`, finalCfnDomainProps);
+  // tlsSecurityPolicy is set in DefaultCfnDomainProps() - it is the
+  // default behavior, but Sonarqube cannot follow the program flow to confirm this.
+  // This is confirmed by the 'Check that TLS 1.2 is the default' test in aws-lambda-elasticsearch
+  const esDomain = new elasticsearch.CfnDomain(scope, `ElasticsearchDomain`, finalCfnDomainProps); // NOSONAR
 
   addCfnSuppressRules(esDomain, [
     {

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

@@ -18,13 +18,13 @@
 
 import * as kendra from 'aws-cdk-lib/aws-kendra';
 import * as iam from 'aws-cdk-lib/aws-iam';
-import { addCfnSuppressRules, consolidateProps } from "./utils";
+import { addCfnSuppressRules, consolidateProps, generatePhysicalName, overrideProps } from "./utils";
 import { Aws } from 'aws-cdk-lib';
 
 // Note: To ensure CDKv2 compatibility, keep the import statement for Construct separate
 import { Construct } from 'constructs';
 import { DefaultKendraIndexProps } from './kendra-defaults';
-import { generatePhysicalName, overrideProps } from "./utils";
+import {  } from "./utils";
 
 export interface BuildKendraIndexProps {
   readonly kendraIndexProps?: kendra.CfnIndexProps | any;

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

@@ -82,7 +82,10 @@ export function buildOpenSearch(scope: Construct, props: BuildOpenSearchProps):
   const defaultCfnDomainProps = DefaultOpenSearchCfnDomainProps(props.openSearchDomainName, cognitoDashboardConfigureRole, props);
   const finalCfnDomainProps = consolidateProps(defaultCfnDomainProps, props.clientDomainProps, constructDrivenProps);
 
-  const opensearchDomain = new opensearch.CfnDomain(scope, `OpenSearchDomain`, finalCfnDomainProps);
+  // tlsSecurityPolicy is set in DefaultOpenSearchCfnDomainProps() - it is the
+  // default behavior, but Sonarqube cannot follow the program flow to confirm this.
+  // This is confirmed by the 'Check that TLS 1.2 is the default' test in aws-lambda-opensearch
+  const opensearchDomain = new opensearch.CfnDomain(scope, `OpenSearchDomain`, finalCfnDomainProps); // NOSONAR
 
   addCfnSuppressRules(opensearchDomain, [
     {

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

@@ -61,7 +61,19 @@ export function createLoggingBucket(scope: Construct,
   const combinedBucketProps = consolidateProps(DefaultS3Props(), loggingBucketProps);
 
   // Create the Logging Bucket
-  const loggingBucket: s3.Bucket = new s3.Bucket(scope, bucketId, combinedBucketProps);
+  // NOSONAR  (typescript:S6281)
+  // Block Public Access is set by DefaultS3Props, but Sonarqube can't detect it
+  // It is verified by 's3 bucket with default props' in the unit tests
+  // NOSONAR (typescript:S6245)
+  // Encryption is turned on in the default properties that Sonarqube doesn't see
+  // Verified by unit test 's3 bucket with default props'
+  // NOSONAR (typescript:S6249)
+  // enforceSSL  is turned on in the default properties that Sonarqube doesn't see
+  // Verified by unit test 's3 bucket with default props'
+  // NOSONAR (typescript:typescript:S6249)
+  // versioning is turned on in the default properties that Sonarqube doesn't see
+  // Verified by unit test 's3 bucket with default props'
+  const loggingBucket: s3.Bucket = new s3.Bucket(scope, bucketId, combinedBucketProps); // NOSONAR
 
   // Extract the CfnBucket from the loggingBucket
   const loggingBucketResource = loggingBucket.node.findChild('Resource') as s3.CfnBucket;
@@ -94,6 +106,18 @@ export function createAlbLoggingBucket(scope: Construct,
   const combinedBucketProps = consolidateProps(DefaultS3Props(), loggingBucketProps);
 
   // Create the Logging Bucket
+  // NOSONAR (typescript:S6281)
+  // Block Public Access is set by DefaultS3Props, but Sonarqube can't detect it
+  // It is verified by 's3 bucket with default props' in the unit tests
+  // NOSONAR (typescript:S6245)
+  // Encryption is turned on in the default properties that Sonarqube doesn't see
+  // Verified by unit test 's3 bucket with default props'
+  // NOSONAR (typescript:S6249)
+  // enforceSSL  is turned on in the default properties that Sonarqube doesn't see
+  // Verified by unit test 's3 bucket with default props'
+  // NOSONAR (typescript:typescript:S6249)
+  // versioning is turned on in the default properties that Sonarqube doesn't see
+  // Verified by unit test 's3 bucket with default props'
   const loggingBucket: s3.Bucket = new s3.Bucket(scope, bucketId, combinedBucketProps);
 
   // Extract the CfnBucket from the loggingBucket
@@ -163,6 +187,18 @@ export function buildS3Bucket(scope: Construct,
 
   const combinedBucketProps = consolidateProps(defaultBucketProps, props.bucketProps);
 
+  // NOSONAR (typescript:S6281) - Block Public Access is set by DefaultS3Props,
+  // but Sonarqube can't detect it
+  // It is verified by 's3 bucket with default props' in the unit tests
+  // NOSONAR (typescript:S6245)
+  // Encryption is turned on in the default properties that Sonarqube doesn't see
+  // Verified by unit test 's3 bucket with default props'
+  // NOSONAR (typescript:S6249)
+  // enforceSSL  is turned on in the default properties that Sonarqube doesn't see
+  // Verified by unit test 's3 bucket with default props'
+  // NOSONAR (typescript:typescript:S6249)
+  // versioning is turned on in the default properties that Sonarqube doesn't see
+  // Verified by unit test 's3 bucket with default props'
   const s3Bucket: s3.Bucket = new s3.Bucket(scope, resolvedBucketId, combinedBucketProps );
 
   return { bucket: s3Bucket, loggingBucket };

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

@@ -295,11 +295,15 @@ export function buildSagemakerNotebook(
     sagemakerNotebookProps = consolidateProps(sagemakerNotebookProps, props.sagemakerNotebookProps);
 
     // Create the notebook
+    // NOSONAR: (typescript:S6319)
+    // keyID is created above in the if (props.sagemakerNotebookProps?.kmsKeyId === undefined)
+    // block. It is then passed to DefaultSagemakerNotebookProps()
+    // This behavior is validated in unit test
     const sagemakerInstance: sagemaker.CfnNotebookInstance = new sagemaker.CfnNotebookInstance(
       scope,
       'SagemakerNotebook',
       sagemakerNotebookProps
-    );
+    ); // NOSONAR
     if (vpcInstance) {
       return { notebook: sagemakerInstance, vpc: vpcInstance, securityGroup };
     } else {

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

@@ -160,7 +160,10 @@ export function buildTopic(scope: Construct, props: BuildTopicProps): BuildTopic
     }
 
     // Create the SNS Topic
-    const topic: sns.Topic = new sns.Topic(scope, 'SnsTopic', snsTopicProps);
+    // NOSONAR (typescript:S6327) - The masterKey is set in the if statement above, SONAR is
+    // not catching it. Behavior is confirmed in the
+    // 'Test deployment with no properties using AWS Managed KMS Key' unit test
+    const topic: sns.Topic = new sns.Topic(scope, 'SnsTopic', snsTopicProps); // NOSONAR
 
     applySecureTopicPolicy(topic);
 

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

@@ -111,7 +111,11 @@ export function buildQueue(scope: Construct, id: string, props: BuildQueueProps)
       queueProps.encryptionMasterKey = buildEncryptionKey(scope, props.encryptionKeyProps);
     }
 
-    const queue = new sqs.Queue(scope, id, queueProps);
+    // NOSONAR (typescript:S6330)
+    // encryption is set to QueueEncryption.KMS_MANAGED by default in DefaultQueueProps, but
+    // Sonarqube can't parse the code well enough to see this. Encryption is confirmed by
+    // the 'Test deployment without imported encryption key' unit test
+    const queue = new sqs.Queue(scope, id, queueProps); // NOSONAR
 
     applySecureQueuePolicy(queue);
 

source/patterns/@aws-solutions-constructs/core/test/sagemaker-helper.test.ts

@@ -81,6 +81,27 @@ test('Test deployment w/ existing VPC', () => {
   });
 });
 
+test('Test default values encrypt notebook', () => {
+  // Stack
+  const stack = new Stack();
+  const sagemakerRole = new iam.Role(stack, 'SagemakerRole', {
+    assumedBy: new iam.ServicePrincipal('sagemaker.amazonaws.com'),
+  });
+
+  // Build Sagemaker Notebook Instance
+  defaults.buildSagemakerNotebook(stack, {
+    role: sagemakerRole,
+    deployInsideVpc: false,
+  });
+  // Assertion
+  const template = Template.fromStack(stack);
+  template.hasResourceProperties('AWS::SageMaker::NotebookInstance', {
+    KmsKeyId: {
+      Ref: "EncryptionKey1B843E66"
+    },
+  });
+});
+
 test('Test deployment w/ override', () => {
   // Stack
   const stack = new Stack();