chore(kinesisfirehose-destinations): refactor logging to combine logG…

…roup and logging properties into loggingConfig

packages/@aws-cdk/aws-kinesisfirehose-alpha/README.md

@@ -183,16 +183,16 @@ Kinesis Data Firehose will send logs to CloudWatch when data transformation or d
 delivery fails. The CDK will enable logging by default and create a CloudWatch LogGroup
 and LogStream for your Delivery Stream.
 
-When you create a destination, you can specify a log group. In this log group, The CDK
-will create log streams where log events will be sent:
+When creating a destination, you can provide a `LoggingConfig`, which can either be an `EnableLogging` or `DisableLogging` instance.
+If you use `EnableLogging`, you can specify a log group where the CDK will create log streams to capture and store log events. For example:
 
 ```ts
 import * as logs from 'aws-cdk-lib/aws-logs';
 
 const logGroup = new logs.LogGroup(this, 'Log Group');
 declare const bucket: s3.Bucket;
 const destination = new destinations.S3Bucket(bucket, {
-  logGroup: logGroup,
+  loggingConfig: new destinations.EnableLogging(logGroup),
 });
 
 new firehose.DeliveryStream(this, 'Delivery Stream', {
@@ -205,7 +205,7 @@ Logging can also be disabled:
 ```ts
 declare const bucket: s3.Bucket;
 const destination = new destinations.S3Bucket(bucket, {
-  logging: false,
+  loggingConfig: new destinations.DisableLogging(),
 });
 new firehose.DeliveryStream(this, 'Delivery Stream', {
   destinations: [destination],

packages/@aws-cdk/aws-kinesisfirehose-destinations-alpha/lib/common.ts

@@ -4,6 +4,7 @@ import * as kms from 'aws-cdk-lib/aws-kms';
 import * as logs from 'aws-cdk-lib/aws-logs';
 import * as s3 from 'aws-cdk-lib/aws-s3';
 import * as cdk from 'aws-cdk-lib/core';
+import { LoggingConfig } from './logging-config';
 
 /**
  * Possible compression options Kinesis Data Firehose can use to compress data on delivery.
@@ -62,20 +63,12 @@ export enum BackupMode {
  */
 interface DestinationLoggingProps {
   /**
-   * If true, log errors when data transformation or data delivery fails.
+   * Configuration that determines whether to log errors during data transformation or delivery failures,
+   * and specifies the CloudWatch log group for storing error logs.
    *
-   * If `logGroup` is provided, this will be implicitly set to `true`.
-   *
-   * @default true - errors are logged.
-   */
-  readonly logging?: boolean;
-
-  /**
-   * The CloudWatch log group where log streams will be created to hold error logs.
-   *
-   * @default - if `logging` is set to `true`, a log group will be created for you.
+   * @default - errors will be logged and a log group will be created for you.
    */
-  readonly logGroup?: logs.ILogGroup;
+  readonly loggingConfig?: LoggingConfig;
 }
 
 /**

packages/@aws-cdk/aws-kinesisfirehose-destinations-alpha/lib/index.ts

@@ -1,2 +1,3 @@
 export * from './common';
 export * from './s3-bucket';
+export * from './logging-config';

packages/@aws-cdk/aws-kinesisfirehose-destinations-alpha/lib/logging-config.ts

@@ -0,0 +1,58 @@
+import * as logs from 'aws-cdk-lib/aws-logs';
+
+/**
+ * Represents the logging configuration for error logs.
+ * 
+ * This class defines whether logging is enabled or disabled and holds
+ * the CloudWatch log group where error logs are stored if logging is enabled.
+ * 
+ * Subclasses must implement whether logging is enabled (`EnableLogging`) 
+ * or disabled (`DisableLogging`).
+ */
+export abstract class LoggingConfig {
+    /**
+     * If true, log errors when data transformation or data delivery fails.
+     *
+     * `true` when using `EnableLogging`, `false` when using `DisableLogging`.
+     */
+    public abstract logging: boolean;
+
+    /**
+     * The CloudWatch log group where log streams will be created to hold error logs.
+     *
+     * @default - if `logging` is set to `true`, a log group will be created for you.
+     */
+    public logGroup?: logs.ILogGroup;
+}
+
+/**
+ * Enables logging for error logs with an optional custom CloudWatch log group.
+ * 
+ * When this class is used, logging is enabled (`logging: true`) and 
+ * you can optionally provide a CloudWatch log group for storing the error logs.
+ * 
+ * If no log group is provided, a default one will be created automatically.
+ */
+export class EnableLogging extends LoggingConfig {
+    public logging: boolean = true;
+
+    constructor(logGroup?: logs.ILogGroup) {
+      super();
+      this.logGroup = logGroup;
+    }
+  }
+
+/**
+ * Disables logging for error logs.
+ * 
+ * When this class is used, logging is disabled (`logging: false`) 
+ * and no CloudWatch log group can be specified.
+ */
+export class DisableLogging extends LoggingConfig {
+    public logging: boolean = false;
+
+    constructor() {
+      super();
+      // No logGroup should be allowed when logging is disabled
+    }
+  }

packages/@aws-cdk/aws-kinesisfirehose-destinations-alpha/lib/private/helpers.ts

@@ -7,23 +7,16 @@ import * as s3 from 'aws-cdk-lib/aws-s3';
 import * as cdk from 'aws-cdk-lib/core';
 import { Construct, IDependable, Node } from 'constructs';
 import { DestinationS3BackupProps } from '../common';
+import { LoggingConfig } from '../logging-config';
 
 export interface DestinationLoggingProps {
   /**
-   * If true, log errors when data transformation or data delivery fails.
+   * Configuration that determines whether to log errors during data transformation or delivery failures,
+   * and specifies the CloudWatch log group for storing error logs.
    *
-   * If `logGroup` is provided, this will be implicitly set to `true`.
-   *
-   * @default true - errors are logged.
-   */
-  readonly logging?: boolean;
-
-  /**
-   * The CloudWatch log group where log streams will be created to hold error logs.
-   *
-   * @default - if `logging` is set to `true`, a log group will be created for you.
+   * @default - errors will be logged and a log group will be created for you.
    */
-  readonly logGroup?: logs.ILogGroup;
+  readonly loggingConfig?: LoggingConfig;
 
   /**
    * The IAM role associated with this destination.
@@ -60,11 +53,8 @@ export interface DestinationBackupConfig extends ConfigWithDependables {
 }
 
 export function createLoggingOptions(scope: Construct, props: DestinationLoggingProps): DestinationLoggingConfig | undefined {
-  if (props.logging === false && props.logGroup) {
-    throw new Error('logging cannot be set to false when logGroup is provided');
-  }
-  if (props.logging !== false || props.logGroup) {
-    const logGroup = props.logGroup ?? Node.of(scope).tryFindChild('LogGroup') as logs.ILogGroup ?? new logs.LogGroup(scope, 'LogGroup');
+  if (props.loggingConfig?.logging !== false || props.loggingConfig?.logGroup) {
+    const logGroup = props.loggingConfig?.logGroup ?? Node.of(scope).tryFindChild('LogGroup') as logs.ILogGroup ?? new logs.LogGroup(scope, 'LogGroup');
     const logGroupGrant = logGroup.grantWrite(props.role);
     return {
       loggingOptions: {
@@ -152,8 +142,7 @@ export function createBackupConfig(scope: Construct, role: iam.IRole, props?: De
   const bucketGrant = bucket.grantReadWrite(role);
 
   const { loggingOptions, dependables: loggingDependables } = createLoggingOptions(scope, {
-    logging: props.logging,
-    logGroup: props.logGroup,
+    loggingConfig: props.loggingConfig,
     role,
     streamId: 'S3Backup',
   }) ?? {};

packages/@aws-cdk/aws-kinesisfirehose-destinations-alpha/lib/s3-bucket.ts

@@ -29,8 +29,7 @@ export class S3Bucket implements firehose.IDestination {
     const bucketGrant = this.bucket.grantReadWrite(role);
 
     const { loggingOptions, dependables: loggingDependables } = createLoggingOptions(scope, {
-      logging: this.props.logging,
-      logGroup: this.props.logGroup,
+      loggingConfig: this.props.loggingConfig,
       role,
       streamId: 'S3Destination',
     }) ?? {};

packages/@aws-cdk/aws-kinesisfirehose-destinations-alpha/test/integ.s3-bucket.lit.ts

@@ -46,8 +46,7 @@ const backupKey = new kms.Key(stack, 'BackupKey', {
 
 new firehose.DeliveryStream(stack, 'Delivery Stream', {
   destinations: [new destinations.S3Bucket(bucket, {
-    logging: true,
-    logGroup: logGroup,
+    loggingConfig: new destinations.EnableLogging(logGroup),
     processor: processor,
     compression: destinations.Compression.GZIP,
     dataOutputPrefix: 'regularPrefix',

packages/@aws-cdk/aws-kinesisfirehose-destinations-alpha/test/s3-bucket.test.ts

@@ -107,7 +107,7 @@ describe('S3 destination', () => {
 
   it('bucket and log group grants are depended on by delivery stream', () => {
     const logGroup = logs.LogGroup.fromLogGroupName(stack, 'Log Group', 'evergreen');
-    const destination = new firehosedestinations.S3Bucket(bucket, { role: destinationRole, logGroup });
+    const destination = new firehosedestinations.S3Bucket(bucket, { role: destinationRole, loggingConfig: new firehosedestinations.EnableLogging(logGroup) });
     new firehose.DeliveryStream(stack, 'DeliveryStream', {
       destinations: [destination],
     });
@@ -171,7 +171,7 @@ describe('S3 destination', () => {
 
     it('does not create resources or configuration if disabled', () => {
       new firehose.DeliveryStream(stack, 'DeliveryStream', {
-        destinations: [new firehosedestinations.S3Bucket(bucket, { logging: false })],
+        destinations: [new firehosedestinations.S3Bucket(bucket, { loggingConfig: new firehosedestinations.DisableLogging() })],
       });
 
       Template.fromStack(stack).resourceCountIs('AWS::Logs::LogGroup', 0);
@@ -186,7 +186,7 @@ describe('S3 destination', () => {
       const logGroup = new logs.LogGroup(stack, 'Log Group');
 
       new firehose.DeliveryStream(stack, 'DeliveryStream', {
-        destinations: [new firehosedestinations.S3Bucket(bucket, { logGroup })],
+        destinations: [new firehosedestinations.S3Bucket(bucket, { loggingConfig: new firehosedestinations.EnableLogging(logGroup) })],
       });
 
       Template.fromStack(stack).resourceCountIs('AWS::Logs::LogGroup', 1);
@@ -200,19 +200,19 @@ describe('S3 destination', () => {
       });
     });
 
-    it('throws error if logging disabled but log group provided', () => {
-      const destination = new firehosedestinations.S3Bucket(bucket, { logging: false, logGroup: new logs.LogGroup(stack, 'Log Group') });
+    // it('throws error if logging disabled but log group provided', () => {
+    //   const destination = new firehosedestinations.S3Bucket(bucket, { logging: false, logGroup: new logs.LogGroup(stack, 'Log Group') });
 
-      expect(() => new firehose.DeliveryStream(stack, 'DeliveryStream', {
-        destinations: [destination],
-      })).toThrowError('logging cannot be set to false when logGroup is provided');
-    });
+    //   expect(() => new firehose.DeliveryStream(stack, 'DeliveryStream', {
+    //     destinations: [destination],
+    //   })).toThrowError('logging cannot be set to false when logGroup is provided');
+    // });
 
     it('grants log group write permissions to destination role', () => {
       const logGroup = new logs.LogGroup(stack, 'Log Group');
 
       new firehose.DeliveryStream(stack, 'DeliveryStream', {
-        destinations: [new firehosedestinations.S3Bucket(bucket, { logGroup, role: destinationRole })],
+        destinations: [new firehosedestinations.S3Bucket(bucket, { loggingConfig: new firehosedestinations.EnableLogging(logGroup), role: destinationRole })],
       });
 
       Template.fromStack(stack).hasResourceProperties('AWS::IAM::Policy', {
@@ -572,8 +572,9 @@ describe('S3 destination', () => {
           bufferingInterval: cdk.Duration.minutes(1),
           compression: firehosedestinations.Compression.ZIP,
           encryptionKey: key,
-          logging: true,
-          logGroup: logGroup,
+          // logging: true,
+          // logGroup: logGroup,
+          loggingConfig: new firehosedestinations.EnableLogging(logGroup),
         },
       });
       new firehose.DeliveryStream(stack, 'DeliveryStream', {