Incorporate CR feedback

Author: kg-aws

packages/aws-cdk-lib/aws-ecs/lib/deployment-lifecycle-hook-target.ts

@@ -3,27 +3,6 @@ import * as iam from '../../aws-iam';
 import * as lambda from '../../aws-lambda';
 import { Fn, ValidationError } from '../../core';
 
-/**
- * Validates that input is a valid JSON object and returns it as a stringified JSON using Fn::ToJsonString
- * @param hookDetails The input to validate (must be a JSON object)
- * @returns The stringified JSON using CloudFormation's Fn::ToJsonString intrinsic function
- * @throws HookDetailsValidationError if the input is not a valid JSON object
- */
-export function stringifyHookDetails(scope: IConstruct, hookDetails: any): string {
-  // Reject arrays
-  if (Array.isArray(hookDetails)) {
-    throw new ValidationError('hookDetails must be a JSON object, got: array', scope);
-  }
-
-  // Check if it's a plain object
-  if (typeof hookDetails === 'object' && hookDetails.constructor === Object) {
-    return Fn.toJsonString(hookDetails);
-  }
-
-  // Everything else is invalid (primitives, functions, dates, etc.)
-  throw new ValidationError(`hookDetails must be a JSON object, got: ${typeof hookDetails}`, scope);
-}
-
 /**
  * Deployment lifecycle stages where hooks can be executed
  */
@@ -123,7 +102,7 @@ export interface DeploymentLifecycleLambdaTargetProps {
    *
    * @default - No custom parameters will be passed
    */
-  readonly hookDetails?: any;
+  readonly hookDetails?: { [key: string]: any };
 }
 
 /**
@@ -158,12 +137,17 @@ export class DeploymentLifecycleLambdaTarget implements IDeploymentLifecycleHook
       this.handler.grantInvoke(this._role);
     }
 
+    // Reject arrays
+    if (Array.isArray(this.props.hookDetails)) {
+      throw new ValidationError('hookDetails must be a JSON object, got: array', scope);
+    }
+
     return {
       targetArn: this.handler.functionArn,
       role: this._role,
       lifecycleStages: this.props.lifecycleStages,
       hookDetails: (this.props.hookDetails === undefined || this.props.hookDetails === null)?
-        this.props.hookDetails : stringifyHookDetails(scope, this.props.hookDetails),
+        this.props.hookDetails: Fn.toJsonString(this.props.hookDetails),
     };
   }
 }

packages/aws-cdk-lib/aws-ecs/test/deployment-lifecycle-hook-target.test.ts

@@ -5,7 +5,6 @@ import * as lambda from '../../aws-lambda';
 import * as cdk from '../../core';
 import { App, Stack } from '../../core';
 import * as ecs from '../lib';
-import { stringifyHookDetails } from '../lib/deployment-lifecycle-hook-target';
 
 describe('DeploymentLifecycleHookTarget', () => {
   let stack: cdk.Stack;
@@ -337,124 +336,5 @@ describe('DeploymentLifecycleHookTarget', () => {
         Template.fromStack(stack);
       }).toThrow(/hookDetails must be a JSON object, got: array/);
     });
-
-    test('throws error for primitive string hookDetails', () => {
-      const hookTarget = new ecs.DeploymentLifecycleLambdaTarget(lambdaFunction, 'PreScaleUpHook', {
-        lifecycleStages: [ecs.DeploymentLifecycleStage.PRE_SCALE_UP],
-        hookDetails: 'just a string',
-      });
-
-      const service = new ecs.FargateService(stack, 'FargateService', {
-        cluster,
-        taskDefinition,
-      });
-
-      service.addLifecycleHook(hookTarget);
-
-      // GIVEN & WHEN & THEN
-      expect(() => {
-        Template.fromStack(stack);
-      }).toThrow(/hookDetails must be a JSON object, got: string/);
-    });
-
-    test('throws error for primitive number hookDetails', () => {
-      const hookTarget = new ecs.DeploymentLifecycleLambdaTarget(lambdaFunction, 'PreScaleUpHook', {
-        lifecycleStages: [ecs.DeploymentLifecycleStage.PRE_SCALE_UP],
-        hookDetails: 42,
-      });
-
-      const service = new ecs.FargateService(stack, 'FargateService', {
-        cluster,
-        taskDefinition,
-      });
-
-      service.addLifecycleHook(hookTarget);
-
-      // GIVEN & WHEN & THEN
-      expect(() => {
-        Template.fromStack(stack);
-      }).toThrow(/hookDetails must be a JSON object, got: number/);
-    });
-
-    test('throws error for null hookDetails', () => {
-      const hookTarget = new ecs.DeploymentLifecycleLambdaTarget(lambdaFunction, 'PreScaleUpHook', {
-        lifecycleStages: [ecs.DeploymentLifecycleStage.PRE_SCALE_UP],
-        hookDetails: null,
-      });
-
-      const service = new ecs.FargateService(stack, 'FargateService', {
-        cluster,
-        taskDefinition,
-      });
-
-      service.addLifecycleHook(hookTarget);
-
-      // GIVEN & WHEN & THEN
-      Template.fromStack(stack).hasResourceProperties('AWS::ECS::Service', {
-        DeploymentConfiguration: {
-          LifecycleHooks: [
-            {
-              LifecycleStages: ['PRE_SCALE_UP'],
-              HookTargetArn: {
-                'Fn::GetAtt': [
-                  Match.stringLikeRegexp('TestFunction'),
-                  'Arn',
-                ],
-              },
-            },
-          ],
-        },
-      });
-    });
-
-    test('throws error for undefined hookDetails', () => {
-      const hookTarget = new ecs.DeploymentLifecycleLambdaTarget(lambdaFunction, 'PreScaleUpHook', {
-        lifecycleStages: [ecs.DeploymentLifecycleStage.PRE_SCALE_UP],
-      });
-
-      const service = new ecs.FargateService(stack, 'FargateService', {
-        cluster,
-        taskDefinition,
-      });
-
-      service.addLifecycleHook(hookTarget);
-
-      // GIVEN & WHEN & THEN
-      Template.fromStack(stack).hasResourceProperties('AWS::ECS::Service', {
-        DeploymentConfiguration: {
-          LifecycleHooks: [
-            {
-              LifecycleStages: ['PRE_SCALE_UP'],
-              HookTargetArn: {
-                'Fn::GetAtt': [
-                  Match.stringLikeRegexp('TestFunction'),
-                  'Arn',
-                ],
-              },
-            },
-          ],
-        },
-      });
-    });
-  });
-
-  describe('stringifyHookDetails function', () => {
-    test('validates and stringifies object directly', () => {
-      const objectInput = { environment: 'production', timeout: 300 };
-      expect(() => stringifyHookDetails(stack, objectInput)).not.toThrow();
-      expect(stringifyHookDetails(stack, objectInput)).toBe('{"environment":"production","timeout":300}');
-    });
-
-    test('rejects primitive string value', () => {
-      expect(() => stringifyHookDetails(stack, 'just a string')).toThrow(/hookDetails must be a JSON object, got: string/);
-    });
-
-    test('rejects primitive number value', () => {
-      expect(() => stringifyHookDetails(stack, 42)).toThrow(/hookDetails must be a JSON object, got: number/);
-    });
-
-    test('rejects primitive boolean value', () => {
-      expect(() => stringifyHookDetails(stack, true)).toThrow(/hookDetails must be a JSON object, got: boolean/);
-    });
   });
 });