Skip to content

Commit

Permalink
fix(custom-resources): StateMachine for isCompleteHandler in provider…
Browse files Browse the repository at this point in the history
… cannot set logging
  • Loading branch information
go-to-k committed Sep 27, 2023
1 parent ab52bb5 commit c7d5edf
Show file tree
Hide file tree
Showing 5 changed files with 322 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import * as rds from 'aws-cdk-lib/aws-rds';
import { ClusterInstance } from 'aws-cdk-lib/aws-rds';
import { IntegTest } from '@aws-cdk/integ-tests-alpha';
import { STANDARD_NODEJS_RUNTIME } from '../../config';
import { RetentionDays } from 'aws-cdk-lib/aws-logs';

class TestStack extends Stack {
constructor(scope: Construct, id: string, props?: StackProps) {
Expand Down Expand Up @@ -107,6 +108,10 @@ class Snapshoter extends Construct {
const provider = new cr.Provider(this, 'SnapshotProvider', {
onEventHandler,
isCompleteHandler,
logOptionsForWaiterStateMachine: {
logRetention: RetentionDays.ONE_DAY,
},
disableLoggingForWaiterStateMachine: false,
});

const customResource = new CustomResource(this, 'Snapshot', {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import * as path from 'path';
import { Construct } from 'constructs';
import * as consts from './runtime/consts';
import { calculateRetryPolicy } from './util';
import { WaiterStateMachine } from './waiter-state-machine';
import { LogOptions, WaiterStateMachine } from './waiter-state-machine';
import { CustomResourceProviderConfig, ICustomResourceProvider } from '../../../aws-cloudformation';
import * as ec2 from '../../../aws-ec2';
import * as iam from '../../../aws-iam';
Expand Down Expand Up @@ -126,6 +126,25 @@ export interface ProviderProps {
* @default - AWS Lambda creates and uses an AWS managed customer master key (CMK)
*/
readonly providerFunctionEnvEncryption?: kms.IKey;

/**
* Log options for `WaiterStateMachine` with `isCompleteHandler` in the provider.
*
* This must not be used if `isCompleteHandler` is not specified or
* `disableLoggingForWaiterStateMachine` is true.
*
* @default - no log options
*/
readonly logOptionsForWaiterStateMachine?: LogOptions;

/**
* Disable logging for `WaiterStateMachine` with `isCompleteHandler` in the provider.
*
* This must not be used if `isCompleteHandler` is not specified.
*
* @default false
*/
readonly disableLoggingForWaiterStateMachine?: boolean;
}

/**
Expand Down Expand Up @@ -162,9 +181,17 @@ export class Provider extends Construct implements ICustomResourceProvider {
constructor(scope: Construct, id: string, props: ProviderProps) {
super(scope, id);

if (!props.isCompleteHandler && (props.queryInterval || props.totalTimeout)) {
throw new Error('"queryInterval" and "totalTimeout" can only be configured if "isCompleteHandler" is specified. '
+ 'Otherwise, they have no meaning');
if (!props.isCompleteHandler) {
if (
props.queryInterval
|| props.totalTimeout
|| props.logOptionsForWaiterStateMachine
|| props.disableLoggingForWaiterStateMachine !== undefined
) {
throw new Error('"queryInterval" and "totalTimeout" and "logOptionsForWaiterStateMachine" and "disableLoggingForWaiterStateMachine" '
+ 'can only be configured if "isCompleteHandler" is specified. '
+ 'Otherwise, they have no meaning');
}
}

this.onEventHandler = props.onEventHandler;
Expand All @@ -181,6 +208,10 @@ export class Provider extends Construct implements ICustomResourceProvider {
const onEventFunction = this.createFunction(consts.FRAMEWORK_ON_EVENT_HANDLER_NAME, props.providerFunctionName);

if (this.isCompleteHandler) {
if (props.disableLoggingForWaiterStateMachine && props.logOptionsForWaiterStateMachine) {
throw new Error('logOptionsForWaiterStateMachine must not be used if disableLoggingForWaiterStateMachine is true');
}

const isCompleteFunction = this.createFunction(consts.FRAMEWORK_IS_COMPLETE_HANDLER_NAME);
const timeoutFunction = this.createFunction(consts.FRAMEWORK_ON_TIMEOUT_HANDLER_NAME);

Expand All @@ -191,6 +222,8 @@ export class Provider extends Construct implements ICustomResourceProvider {
backoffRate: retry.backoffRate,
interval: retry.interval,
maxAttempts: retry.maxAttempts,
logOptions: !props.disableLoggingForWaiterStateMachine ? props.logOptionsForWaiterStateMachine : undefined,
disableLogging: props.disableLoggingForWaiterStateMachine,
});
// the on-event entrypoint is going to start the execution of the waiter
onEventFunction.addEnvironment(consts.WAITER_STATE_MACHINE_ARN_ENV, waiterStateMachine.stateMachineArn);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,34 @@
import { Construct } from 'constructs';
import { Grant, IGrantable, Role, ServicePrincipal } from '../../../aws-iam';
import { Grant, IGrantable, PolicyStatement, Role, ServicePrincipal } from '../../../aws-iam';
import { IFunction } from '../../../aws-lambda';
import { CfnResource, Duration, Stack } from '../../../core';
import { LogGroup, RetentionDays } from '../../../aws-logs';
import { LogLevel } from '../../../aws-stepfunctions';

export interface LogOptions {
/**
* Determines whether execution data is included in your log.
*
* @default false
*/
readonly includeExecutionData?: boolean;

/**
* Defines which category of execution history events are logged.
*
* @default ERROR
*/
readonly level?: LogLevel;

/**
* The number of days framework log events are kept in CloudWatch Logs. When
* updating this property, unsetting it doesn't remove the log retention policy.
* To remove the retention policy, set the value to `INFINITE`.
*
* @default logs.RetentionDays.INFINITE
*/
readonly logRetention?: RetentionDays;
}

export interface WaiterStateMachineProps {
/**
Expand All @@ -28,6 +55,22 @@ export interface WaiterStateMachineProps {
* Backoff between attempts.
*/
readonly backoffRate: number;

/**
* Options for StateMachine logging.
*
* If `disableLogging` is true, this property is ignored.
*
* @default - no logOptions
*/
readonly logOptions?: LogOptions;

/**
* Disable StateMachine logging.
*
* @default false
*/
readonly disableLogging?: boolean;
}

/**
Expand All @@ -49,6 +92,32 @@ export class WaiterStateMachine extends Construct {
props.isCompleteHandler.grantInvoke(role);
props.timeoutHandler.grantInvoke(role);

let logGroup: LogGroup | undefined;
if (props.disableLogging) {
if (props.logOptions) {
throw new Error('logOptions must not be used if disableLogging is true');
}
} else {
logGroup = new LogGroup(this, 'LogGroup', {
retention: props.logOptions?.logRetention,
});
role.addToPrincipalPolicy(new PolicyStatement({
actions: [
'logs:CreateLogDelivery',
'logs:CreateLogStream',
'logs:GetLogDelivery',
'logs:UpdateLogDelivery',
'logs:DeleteLogDelivery',
'logs:ListLogDeliveries',
'logs:PutLogEvents',
'logs:PutResourcePolicy',
'logs:DescribeResourcePolicies',
'logs:DescribeLogGroups',
],
resources: ['*'],
}));
}

const definition = Stack.of(this).toJsonString({
StartAt: 'framework-isComplete-task',
States: {
Expand All @@ -75,11 +144,24 @@ export class WaiterStateMachine extends Construct {
},
});

const logOptions = logGroup ? {
LoggingConfiguration: {
Destinations: [{
CloudWatchLogsLogGroup: {
LogGroupArn: logGroup.logGroupArn,
},
}],
},
IncludeExecutionData: props.logOptions?.includeExecutionData ?? false,
Level: props.logOptions?.level ?? LogLevel.ERROR,
} : undefined;

const resource = new CfnResource(this, 'Resource', {
type: 'AWS::StepFunctions::StateMachine',
properties: {
DefinitionString: definition,
RoleArn: role.roleArn,
...logOptions,
},
});
resource.node.addDependency(role);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import * as iam from '../../../aws-iam';
import * as kms from '../../../aws-kms';
import * as lambda from '../../../aws-lambda';
import * as logs from '../../../aws-logs';
import { LogLevel } from '../../../aws-stepfunctions';
import { Duration, Stack } from '../../../core';
import * as cr from '../../lib';
import * as util from '../../lib/provider-framework/util';
Expand Down Expand Up @@ -180,6 +181,12 @@ test('if isComplete is specified, the isComplete framework handler is also inclu
new cr.Provider(stack, 'MyProvider', {
onEventHandler: handler,
isCompleteHandler: handler,
logOptionsForWaiterStateMachine: {
includeExecutionData: true,
level: LogLevel.ALL,
logRetention: logs.RetentionDays.ONE_DAY,
},
disableLoggingForWaiterStateMachine: false,
});

// THEN
Expand Down Expand Up @@ -238,10 +245,51 @@ test('if isComplete is specified, the isComplete framework handler is also inclu
],
],
},
LoggingConfiguration: {
Destinations: [
{
CloudWatchLogsLogGroup: {
LogGroupArn: {
'Fn::GetAtt': [
'MyProviderwaiterstatemachineLogGroupD43CA868',
'Arn',
],
},
},
},
],
},
IncludeExecutionData: true,
Level: 'ALL',
});
});

test('fails if "queryInterval" and/or "totalTimeout" are set without "isCompleteHandler"', () => {
test('fails if logOptionsForWaiterStateMachine is specified and disableLoggingForWaiterStateMachine is true', () => {
// GIVEN
const stack = new Stack();
const handler = new lambda.Function(stack, 'MyHandler', {
code: new lambda.InlineCode('foo'),
handler: 'index.onEvent',
runtime: lambda.Runtime.NODEJS_LATEST,
});

// WHEN
// THEN
expect(() => {
new cr.Provider(stack, 'MyProvider', {
onEventHandler: handler,
isCompleteHandler: handler,
logOptionsForWaiterStateMachine: {
includeExecutionData: true,
level: LogLevel.ALL,
logRetention: logs.RetentionDays.ONE_DAY,
},
disableLoggingForWaiterStateMachine: true,
});
}).toThrow(/logOptionsForWaiterStateMachine must not be used if disableLoggingForWaiterStateMachine is true/);
});

test('fails if "queryInterval" or "totalTimeout" or "logOptionsForWaiterStateMachine" or "disableLoggingForWaiterStateMachine" are set without "isCompleteHandler"', () => {
// GIVEN
const stack = new Stack();
const handler = new lambda.Function(stack, 'MyHandler', {
Expand All @@ -254,12 +302,26 @@ test('fails if "queryInterval" and/or "totalTimeout" are set without "isComplete
expect(() => new cr.Provider(stack, 'provider1', {
onEventHandler: handler,
queryInterval: Duration.seconds(10),
})).toThrow(/\"queryInterval\" and \"totalTimeout\" can only be configured if \"isCompleteHandler\" is specified. Otherwise, they have no meaning/);
})).toThrow(/\"queryInterval\" and \"totalTimeout\" and \"logOptionsForWaiterStateMachine\" and \"disableLoggingForWaiterStateMachine\" can only be configured if \"isCompleteHandler\" is specified. Otherwise, they have no meaning/);

expect(() => new cr.Provider(stack, 'provider2', {
onEventHandler: handler,
totalTimeout: Duration.seconds(100),
})).toThrow(/\"queryInterval\" and \"totalTimeout\" can only be configured if \"isCompleteHandler\" is specified. Otherwise, they have no meaning/);
})).toThrow(/\"queryInterval\" and \"totalTimeout\" and \"logOptionsForWaiterStateMachine\" and \"disableLoggingForWaiterStateMachine\" can only be configured if \"isCompleteHandler\" is specified. Otherwise, they have no meaning/);

expect(() => new cr.Provider(stack, 'provider3', {
onEventHandler: handler,
logOptionsForWaiterStateMachine: {
includeExecutionData: true,
level: LogLevel.ALL,
logRetention: logs.RetentionDays.ONE_DAY,
},
})).toThrow(/\"queryInterval\" and \"totalTimeout\" and \"logOptionsForWaiterStateMachine\" and \"disableLoggingForWaiterStateMachine\" can only be configured if \"isCompleteHandler\" is specified. Otherwise, they have no meaning/);

expect(() => new cr.Provider(stack, 'provider4', {
onEventHandler: handler,
disableLoggingForWaiterStateMachine: false,
})).toThrow(/\"queryInterval\" and \"totalTimeout\" and \"logOptionsForWaiterStateMachine\" and \"disableLoggingForWaiterStateMachine\" can only be configured if \"isCompleteHandler\" is specified. Otherwise, they have no meaning/);
});

describe('retry policy', () => {
Expand Down
Loading

0 comments on commit c7d5edf

Please sign in to comment.