chore(sonarqube): clean up code smells

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

@@ -79,110 +79,117 @@ export interface LambdaToS3Props {
    *
    * @default - Default props are used
    */
-   readonly loggingBucketProps?: s3.BucketProps
+  readonly loggingBucketProps?: s3.BucketProps
   /**
    * Whether to turn on Access Logs for the S3 bucket with the associated storage costs.
    * Enabling Access Logging is a best practice.
    *
    * @default - true
    */
-    readonly logS3AccessLogs?: boolean;
+  readonly logS3AccessLogs?: boolean;
 }
 
 /**
  * @summary The LambdaToS3 class.
  */
 export class LambdaToS3 extends Construct {
-    public readonly lambdaFunction: lambda.Function;
-    public readonly s3Bucket?: s3.Bucket;
-    public readonly s3LoggingBucket?: s3.Bucket;
-    public readonly vpc?: ec2.IVpc;
-    public readonly s3BucketInterface: s3.IBucket;
-
-    /**
-     * @summary Constructs a new instance of the LambdaToS3 class.
-     * @param {cdk.App} scope - represents the scope for all the resources.
-     * @param {string} id - this is a a scope-unique id.
-     * @param {LambdaToS3Props} props - user provided props for the construct.
-     * @since 0.8.0
-     * @access public
-     */
-    constructor(scope: Construct, id: string, props: LambdaToS3Props) {
-      super(scope, id);
-
-      // All our tests are based upon this behavior being on, so we're setting
-      // context here rather than assuming the client will set it
-      this.node.setContext("@aws-cdk/aws-s3:serverAccessLogsUseBucketPolicy", true);
-
-      defaults.CheckProps(props);
-
-      if (props.bucketPermissions) {
-        defaults.CheckListValues(['Delete', 'Put', 'Read', 'ReadWrite', 'Write'], props.bucketPermissions, 'bucket permission');
-      }
-
-      let bucket: s3.IBucket;
-
-      if (props.deployVpc || props.existingVpc) {
-        this.vpc = defaults.buildVpc(scope, {
-          defaultVpcProps: defaults.DefaultIsolatedVpcProps(),
-          existingVpc: props.existingVpc,
-          userVpcProps: props.vpcProps,
-          constructVpcProps: {
-            enableDnsHostnames: true,
-            enableDnsSupport: true,
-          },
-        });
-
-        defaults.AddAwsServiceEndpoint(scope, this.vpc, defaults.ServiceEndpointTypes.S3);
-      }
-
-      // Setup the Lambda function
-      this.lambdaFunction = defaults.buildLambdaFunction(this, {
-        existingLambdaObj: props.existingLambdaObj,
-        lambdaFunctionProps: props.lambdaFunctionProps,
-        vpc: this.vpc,
+  public readonly lambdaFunction: lambda.Function;
+  public readonly s3Bucket?: s3.Bucket;
+  public readonly s3LoggingBucket?: s3.Bucket;
+  public readonly vpc?: ec2.IVpc;
+  public readonly s3BucketInterface: s3.IBucket;
+
+  /**
+   * @summary Constructs a new instance of the LambdaToS3 class.
+   * @param {cdk.App} scope - represents the scope for all the resources.
+   * @param {string} id - this is a a scope-unique id.
+   * @param {LambdaToS3Props} props - user provided props for the construct.
+   * @since 0.8.0
+   * @access public
+   */
+  constructor(scope: Construct, id: string, props: LambdaToS3Props) {
+    super(scope, id);
+
+    // All our tests are based upon this behavior being on, so we're setting
+    // context here rather than assuming the client will set it
+    this.node.setContext("@aws-cdk/aws-s3:serverAccessLogsUseBucketPolicy", true);
+
+    defaults.CheckProps(props);
+
+    if (props.bucketPermissions) {
+      defaults.CheckListValues(['Delete', 'Put', 'Read', 'ReadWrite', 'Write'], props.bucketPermissions, 'bucket permission');
+    }
+
+    let bucket: s3.IBucket;
+
+    if (props.deployVpc || props.existingVpc) {
+      this.vpc = defaults.buildVpc(scope, {
+        defaultVpcProps: defaults.DefaultIsolatedVpcProps(),
+        existingVpc: props.existingVpc,
+        userVpcProps: props.vpcProps,
+        constructVpcProps: {
+          enableDnsHostnames: true,
+          enableDnsSupport: true,
+        },
+      });
+
+      defaults.AddAwsServiceEndpoint(scope, this.vpc, defaults.ServiceEndpointTypes.S3);
+    }
+
+    // Setup the Lambda function
+    this.lambdaFunction = defaults.buildLambdaFunction(this, {
+      existingLambdaObj: props.existingLambdaObj,
+      lambdaFunctionProps: props.lambdaFunctionProps,
+      vpc: this.vpc,
+    });
+
+    // Setup S3 Bucket
+    if (!props.existingBucketObj) {
+      const buildS3BucketResponse = defaults.buildS3Bucket(this, {
+        bucketProps: props.bucketProps,
+        loggingBucketProps: props.loggingBucketProps,
+        logS3AccessLogs: props.logS3AccessLogs
       });
+      this.s3Bucket = buildS3BucketResponse.bucket;
+      this.s3LoggingBucket = buildS3BucketResponse.loggingBucket;
 
-      // Setup S3 Bucket
-      if (!props.existingBucketObj) {
-        const buildS3BucketResponse = defaults.buildS3Bucket(this, {
-          bucketProps: props.bucketProps,
-          loggingBucketProps: props.loggingBucketProps,
-          logS3AccessLogs: props.logS3AccessLogs
-        });
-        this.s3Bucket = buildS3BucketResponse.bucket;
-        this.s3LoggingBucket = buildS3BucketResponse.loggingBucket;
-
-        bucket = this.s3Bucket;
-      } else {
-        bucket = props.existingBucketObj;
-      }
-
-      this.s3BucketInterface = bucket;
-
-      // Configure environment variables
-      const bucketEnvironmentVariableName = props.bucketEnvironmentVariableName || 'S3_BUCKET_NAME';
-      this.lambdaFunction.addEnvironment(bucketEnvironmentVariableName, bucket.bucketName);
-
-      // Add the requested or default bucket permissions
-      if (props.bucketPermissions) {
-        if (props.bucketPermissions.includes('Delete')) {
-          bucket.grantDelete(this.lambdaFunction.grantPrincipal);
-        }
-        if (props.bucketPermissions.includes('Put')) {
-          bucket.grantPut(this.lambdaFunction.grantPrincipal);
-        }
-        if (props.bucketPermissions.includes('Read')) {
-          bucket.grantRead(this.lambdaFunction.grantPrincipal);
-        }
-        if (props.bucketPermissions.includes('ReadWrite')) {
-          bucket.grantReadWrite(this.lambdaFunction.grantPrincipal);
-        }
-        if (props.bucketPermissions.includes('Write')) {
-          bucket.grantWrite(this.lambdaFunction.grantPrincipal);
-        }
-      } else {
-        bucket.grantReadWrite(this.lambdaFunction.grantPrincipal);
-      }
+      bucket = this.s3Bucket;
+    } else {
+      bucket = props.existingBucketObj;
+    }
+
+    this.s3BucketInterface = bucket;
+
+    // Configure environment variables
+    const bucketEnvironmentVariableName = props.bucketEnvironmentVariableName || 'S3_BUCKET_NAME';
+    this.lambdaFunction.addEnvironment(bucketEnvironmentVariableName, bucket.bucketName);
+
+    // Add the requested or default bucket permissions
+    if (props.bucketPermissions) {
+      this.AssignAppropriatePrivileges(props, bucket, this.lambdaFunction);
+    } else {
+      bucket.grantReadWrite(this.lambdaFunction.grantPrincipal);
+    }
+  }
+
+  private AssignAppropriatePrivileges(props: LambdaToS3Props, bucket: s3.IBucket, clientFunction: lambda.Function) {
+    if (!props.bucketPermissions) {
+      throw new Error('bucketPermissions required here');
+    }
+    if (props.bucketPermissions.includes('Delete')) {
+      bucket.grantDelete(clientFunction.grantPrincipal);
+    }
+    if (props.bucketPermissions.includes('Put')) {
+      bucket.grantPut(clientFunction.grantPrincipal);
+    }
+    if (props.bucketPermissions.includes('Read')) {
+      bucket.grantRead(clientFunction.grantPrincipal);
+    }
+    if (props.bucketPermissions.includes('ReadWrite')) {
+      bucket.grantReadWrite(clientFunction.grantPrincipal);
+    }
+    if (props.bucketPermissions.includes('Write')) {
+      bucket.grantWrite(clientFunction.grantPrincipal);
     }
+  }
 }

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

@@ -107,13 +107,7 @@ export class Route53ToAlb extends Construct {
 
     // NOTE: We don't call CheckAlbProps() here, because this construct creates an ALB
     // with no listener or target, so some of those checks don't apply
-    if (props?.loadBalancerProps?.vpc) {
-      throw new Error('Specify any existing VPC at the construct level, not within loadBalancerProps.');
-    }
-
-    if (props.existingLoadBalancerObj && !props.existingVpc) {
-      throw new Error('An existing ALB already exists in a VPC, so that VPC must be passed to the construct in props.existingVpc');
-    }
+    this.PropsCustomCheck(props);
 
     if (props.existingHostedZoneInterface && !props.publicApi && !props.existingVpc) {
       throw new Error('An existing Private Hosted Zone already exists in a VPC, so that VPC must be passed to the construct in props.existingVpc');
@@ -167,4 +161,14 @@ export class Route53ToAlb extends Construct {
     // Before turning off SonarQube for the line, reduce the line to it's minimum
     new r53.ARecord(this, arecordId, arecordProps); // NOSONAR
   }
+
+  private PropsCustomCheck(props: Route53ToAlbProps) {
+    if (props?.loadBalancerProps?.vpc) {
+      throw new Error('Specify any existing VPC at the construct level, not within loadBalancerProps.');
+    }
+
+    if (props.existingLoadBalancerObj && !props.existingVpc) {
+      throw new Error('An existing ALB already exists in a VPC, so that VPC must be passed to the construct in props.existingVpc');
+    }
+  }
 }

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

@@ -95,32 +95,12 @@ export class Route53ToApiGateway extends Construct {
     // Existing Public or Private Hosted Zone
     if (props.existingHostedZoneInterface) {
       this.hostedZone = props.existingHostedZoneInterface;
+      this.ExistingHostedZonePropCheck(props);
 
-      if (props.existingVpc) {
-        throw new Error('Cannot provide an existing VPC to an existing Private Hosted Zone.');
-      }
-      if (props.privateHostedZoneProps) {
-        throw new Error('Must provide either existingHostedZoneInterface or privateHostedZoneProps, but not both.');
-      }
     } else { // Creating a Private Hosted Zone
-      if (props.publicApi) {
-        throw new Error('Public APIs require an existingHostedZone be passed in the Props object.');
-      } else {
-        if (!props.privateHostedZoneProps) {
-          throw new Error('Must provide either existingHostedZoneInterface or privateHostedZoneProps.');
-        }
-        if (props.privateHostedZoneProps.vpc) {
-          throw new Error('All VPC specs must be provided at the Construct level in Route53ToApiGatewayProps.');
-        }
-        if (!props.privateHostedZoneProps.zoneName) {
-          throw new Error('Must supply zoneName for Private Hosted Zone Props.');
-        }
-        if ( !this.vpc ) {
-          throw new Error('Must specify an existingVPC for the Private Hosted Zone in the construct props.');
-        }
-        const manufacturedProps: route53.PrivateHostedZoneProps = defaults.overrideProps(props.privateHostedZoneProps, { vpc: this.vpc });
-        this.hostedZone = new route53.PrivateHostedZone(this, `${id}-zone`, manufacturedProps);
-      }
+      this.PrivateHostedZonePropsChecks(props);
+      const manufacturedProps: route53.PrivateHostedZoneProps = defaults.overrideProps(props.privateHostedZoneProps, { vpc: this.vpc });
+      this.hostedZone = new route53.PrivateHostedZone(this, `${id}-zone`, manufacturedProps);
     }
 
     // Convert IRestApi to RestApi
@@ -140,4 +120,31 @@ export class Route53ToApiGateway extends Construct {
     // Create A Record in custom domain to route traffic to API Gateway
     new route53.ARecord(this, 'CustomDomainAliasRecord', arecordProps); // NOSONAR
   }
+
+  private ExistingHostedZonePropCheck(props: Route53ToApiGatewayProps) {
+    if (props.existingVpc) {
+      throw new Error('Cannot provide an existing VPC to an existing Private Hosted Zone.');
+    }
+    if (props.privateHostedZoneProps) {
+      throw new Error('Must provide either existingHostedZoneInterface or privateHostedZoneProps, but not both.');
+    }
+  }
+
+  private PrivateHostedZonePropsChecks(props: Route53ToApiGatewayProps) {
+    if (props.publicApi) {
+      throw new Error('Public APIs require an existingHostedZone be passed in the Props object.');
+    }
+    if (!props.privateHostedZoneProps) {
+      throw new Error('Must provide either existingHostedZoneInterface or privateHostedZoneProps.');
+    }
+    if (props.privateHostedZoneProps.vpc) {
+      throw new Error('All VPC specs must be provided at the Construct level in Route53ToApiGatewayProps.');
+    }
+    if (!props.privateHostedZoneProps.zoneName) {
+      throw new Error('Must supply zoneName for Private Hosted Zone Props.');
+    }
+    if (!this.vpc) {
+      throw new Error('Must specify an existingVPC for the Private Hosted Zone in the construct props.');
+    }
+  }
 }

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

@@ -24,7 +24,6 @@ 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 {  } from "./utils";
 
 export interface BuildKendraIndexProps {
   readonly kendraIndexProps?: kendra.CfnIndexProps | any;

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

@@ -76,12 +76,7 @@ export interface BuildQueueResponse {
  * @internal This is an internal core function and should not be called directly by Solutions Constructs clients.
  */
 export function buildQueue(scope: Construct, id: string, props: BuildQueueProps): BuildQueueResponse {
-
-  if ((props.queueProps?.encryptionMasterKey || props.encryptionKey || props.encryptionKeyProps)
-  && props.enableEncryptionWithCustomerManagedKey === true) {
-    printWarning(`Ignoring enableEncryptionWithCustomerManagedKey because one of
-     queueProps.encryptionMasterKey, encryptionKey, or encryptionKeyProps was already specified`);
-  }
+  CheckEncryptionWarnings(props);
 
   // If an existingQueueObj is not specified
   if (!props.existingQueueObj) {
@@ -127,6 +122,16 @@ export function buildQueue(scope: Construct, id: string, props: BuildQueueProps)
   }
 }
 
+/**
+ * @internal This is an internal core function and should not be called directly by Solutions Constructs clients.
+ */
+function CheckEncryptionWarnings(props: BuildQueueProps) {
+  if ((props.queueProps?.encryptionMasterKey || props.encryptionKey || props.encryptionKeyProps)
+  && props.enableEncryptionWithCustomerManagedKey === true) {
+    printWarning(`Ignoring enableEncryptionWithCustomerManagedKey because one of
+     queueProps.encryptionMasterKey, encryptionKey, or encryptionKeyProps was already specified`);
+  }
+}
 export interface BuildDeadLetterQueueProps {
   /**
    * Existing instance of SQS queue object, providing both this and queueProps will cause an error.