Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(custom-resources): cannot set logging for state machine generated in CompleteHandler #28706

Merged
merged 51 commits into from
Apr 19, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
9ced5e8
add implementation without integ tests and index.ts
go-to-k Jan 14, 2024
8b5b49f
index.ts
go-to-k Jan 14, 2024
dcc4dc3
integ tests for dynamodb
go-to-k Jan 14, 2024
49ebbee
add integ for custom-resources/test/provider-framework/integ.provider
go-to-k Jan 14, 2024
3f978f6
aws-rds/test/integ.cluster-snapshot
go-to-k Jan 14, 2024
36a7cf5
framework-integ/test/aws-eks/test/integ.alb-controller.js.snapshot
go-to-k Jan 14, 2024
99fa1bd
aws-stepfunctions-tasks/test/emrcontainers/integ.job-submission-workflow
go-to-k Jan 14, 2024
6cee88b
aws-stepfunctions-tasks/test/emrcontainers/integ.start-job-run-aws-st…
go-to-k Jan 14, 2024
90ec02f
test/integ.eks-cluster-imported
go-to-k Jan 14, 2024
aaef07d
integ.eks-cluster-ipv6
go-to-k Jan 14, 2024
6d3a998
aws-eks/test/integ.eks-cluster-private-endpoint
go-to-k Jan 14, 2024
90d32e5
integ.eks-cluster.js.snapshot
go-to-k Jan 14, 2024
f550c6a
test/integ.eks-helm-asset
go-to-k Jan 14, 2024
472f8ab
integ.eks-inference-nodegroup
go-to-k Jan 14, 2024
de4029e
other snapshots
go-to-k Jan 14, 2024
eeb6197
integ.provider-with-waiter-state-machine
go-to-k Jan 14, 2024
a2c242e
aws-amplify-alpha/test/integ.app-asset-deployment
go-to-k Jan 14, 2024
71f1a7e
change package-lock.json
go-to-k Jan 14, 2024
99bb440
integ.cluster-snapshot.js.snapshot
go-to-k Jan 14, 2024
64ae3a9
rest snapshots
go-to-k Jan 14, 2024
fe3d1e0
docs
go-to-k Jan 14, 2024
a0820e6
Merge branch 'main' of https://github.com/go-to-k/aws-cdk into sfn-lo…
go-to-k Jan 18, 2024
61eeb14
Merge branch 'main' of https://github.com/go-to-k/aws-cdk into sfn-lo…
go-to-k Jan 20, 2024
9ca8951
Merge branch 'main' of https://github.com/go-to-k/aws-cdk into sfn-lo…
go-to-k Jan 31, 2024
cd05ed1
change snapshots
go-to-k Jan 31, 2024
43d1d9d
Merge branch 'main' into sfn-log-provider
go-to-k Jan 31, 2024
fc5c8cb
Merge branch 'main' of https://github.com/go-to-k/aws-cdk into sfn-lo…
go-to-k Feb 6, 2024
19a646d
change snapshots
go-to-k Feb 6, 2024
bbefd54
Merge branch 'main' of https://github.com/go-to-k/aws-cdk into sfn-lo…
go-to-k Feb 27, 2024
e4f5693
integ in aws-amplify-alpha
go-to-k Feb 27, 2024
0c788f0
custom-resources
go-to-k Feb 27, 2024
2aa2515
aws-dynamodb
go-to-k Feb 27, 2024
fcfb30c
Merge branch 'main' of https://github.com/go-to-k/aws-cdk into sfn-lo…
go-to-k Mar 11, 2024
9813034
aws-amplify-alpha/test/integ.app-asset-deployment
go-to-k Mar 11, 2024
abd45b2
framework-integ/test/aws-dynamodb
go-to-k Mar 11, 2024
4747d9d
aws-dynamodb snapshots
go-to-k Mar 11, 2024
68d5462
custom-resources snapshot
go-to-k Mar 11, 2024
3ffc464
aws-dynamodb
go-to-k Mar 11, 2024
7ae9cfb
aws-amplify-alpha
go-to-k Mar 11, 2024
da1d31b
custom-resources
go-to-k Mar 11, 2024
76b34e6
Merge branch 'main' into sfn-log-provider
go-to-k Mar 13, 2024
522644d
Merge branch 'main' into sfn-log-provider
go-to-k Apr 9, 2024
b3895e9
aws-eks/test/integ.eks-al2023-nodegroup
go-to-k Apr 9, 2024
d729180
Merge branch 'main' of https://github.com/go-to-k/aws-cdk into sfn-lo…
go-to-k Apr 17, 2024
8d48434
Merge branch 'main' into sfn-log-provider
go-to-k Apr 17, 2024
34136f3
add a new line
go-to-k Apr 18, 2024
d9a67ae
add a new line in tests
go-to-k Apr 18, 2024
81d420a
Merge branch 'main' into sfn-log-provider
paulhcsun Apr 18, 2024
3998877
Merge branch 'main' into sfn-log-provider
mergify[bot] Apr 18, 2024
18db57a
Merge branch 'main' into sfn-log-provider
mergify[bot] Apr 19, 2024
5621cbb
Merge branch 'main' into sfn-log-provider
mergify[bot] Apr 19, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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,20 @@ export interface ProviderProps {
* @default - AWS Lambda creates and uses an AWS managed customer master key (CMK)
*/
readonly providerFunctionEnvEncryption?: kms.IKey;

/**
* Defines what execution history events of the waiter state machine are logged and where they are logged.
*
* @default - A default log group will be created if logging for the waiter state machine is enabled.
*/
readonly waiterStateMachineLogOptions?: LogOptions;

/**
* Whether logging for the waiter state machine is disabled.
*
* @default - false
*/
readonly disableWaiterStateMachineLogging?: boolean;
}

/**
Expand Down Expand Up @@ -162,9 +176,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.waiterStateMachineLogOptions
|| props.disableWaiterStateMachineLogging !== undefined
) {
throw new Error('"queryInterval", "totalTimeout", "waiterStateMachineLogOptions", and "disableWaiterStateMachineLogging" '
+ 'can only be configured if "isCompleteHandler" is specified. '
+ 'Otherwise, they have no meaning');
}
}

this.onEventHandler = props.onEventHandler;
Expand All @@ -191,6 +213,8 @@ export class Provider extends Construct implements ICustomResourceProvider {
backoffRate: retry.backoffRate,
interval: retry.interval,
maxAttempts: retry.maxAttempts,
logOptions: props.waiterStateMachineLogOptions,
disableLogging: props.disableWaiterStateMachineLogging,
});
// 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,8 +1,39 @@
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 { ILogGroup, LogGroup } from '../../../aws-logs';
import { CfnStateMachine, LogLevel } from '../../../aws-stepfunctions';
import { Duration, Stack } from '../../../core';

/**
* Log Options for the state machine.
*/
export interface LogOptions {
/**
* The log group where the execution history events will be logged.
*
* @default - a new log group will be created
*/
readonly destination?: ILogGroup;

/**
* 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;
}

/**
* Initialization properties for the `WaiterStateMachine` construct.
*/
export interface WaiterStateMachineProps {
/**
* The main handler that notifies if the waiter to decide 'complete' or 'incomplete'.
Expand All @@ -28,17 +59,36 @@ export interface WaiterStateMachineProps {
* Backoff between attempts.
*/
readonly backoffRate: number;

/**
* Defines what execution history events are logged and where they are logged.
*
* @default - A default log group will be created if logging is enabled.
*/
readonly logOptions?: LogOptions;

/**
* Whether logging for the state machine is disabled.
*
* @default - false
*/
readonly disableLogging?: boolean;
}

/**
* A very simple StateMachine construct highly customized to the provider framework.
* This is so that this package does not need to depend on aws-stepfunctions module.
* We previously used `CfnResource` instead of `CfnStateMachine` to avoid depending
* on `aws-stepfunctions` module, but now it is okay.
*
* The state machine continuously calls the isCompleteHandler, until it succeeds or times out.
* The handler is called `maxAttempts` times with an `interval` duration and a `backoffRate` rate.
*/
export class WaiterStateMachine extends Construct {
/**
* The ARN of the state machine.
*/
public readonly stateMachineArn: string;
private readonly isCompleteHandler: IFunction;

constructor(scope: Construct, id: string, props: WaiterStateMachineProps) {
super(scope, id);
Expand Down Expand Up @@ -75,23 +125,73 @@ export class WaiterStateMachine extends Construct {
},
});

const resource = new CfnResource(this, 'Resource', {
type: 'AWS::StepFunctions::StateMachine',
properties: {
DefinitionString: definition,
RoleArn: role.roleArn,
},
this.isCompleteHandler = props.isCompleteHandler;
const resource = new CfnStateMachine(this, 'Resource', {
definitionString: definition,
roleArn: role.roleArn,
loggingConfiguration: this.renderLoggingConfiguration(role, props.logOptions, props.disableLogging),
});
resource.node.addDependency(role);

this.stateMachineArn = resource.ref;
}

/**
* Grant the given identity permissions on StartExecution of the state machine.
*/
public grantStartExecution(identity: IGrantable) {
return Grant.addToPrincipal({
grantee: identity,
actions: ['states:StartExecution'],
resourceArns: [this.stateMachineArn],
});
}

private renderLoggingConfiguration(
role: Role,
logOptions?: LogOptions,
disableLogging?: boolean,
): CfnStateMachine.LoggingConfigurationProperty | undefined {
if (disableLogging) return undefined;

// You need to specify `*` in the Resource field because CloudWatch API actions, such as
// CreateLogDelivery and DescribeLogGroups, don't support Resource types defined by Amazon
// CloudWatch Logs.
// https://docs.aws.amazon.com/step-functions/latest/dg/cw-logs.html#cloudwatch-iam-policy
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 logGroup = logOptions?.destination ?? new LogGroup(this, 'LogGroup', {
// By using the auto-generated name of the Lambda created in the `Provider` that calls this
// `WaiterStateMachine` construct, even if the `Provider` (or its parent) is deleted and then
// created again, the log group name will not duplicate previously created one with removal
// policy `RETAIN`. This is because that the Lambda will be re-created again with auto-generated name.
// The `node.addr` is also used to prevent duplicate names no matter how many times this construct
// is created in the stack. It will not duplicate if called on other stacks.
logGroupName: `/aws/vendedlogs/states/waiter-state-machine-${this.isCompleteHandler.functionName}-${this.node.addr}`,
});

return {
destinations: [{
cloudWatchLogsLogGroup: {
logGroupArn: logGroup.logGroupArn,
},
}],
includeExecutionData: logOptions?.includeExecutionData ?? false,
level: logOptions?.level ?? LogLevel.ERROR,
};
}
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import { Template } from '../../../assertions';
import { Match, Template } from '../../../assertions';
import * as ec2 from '../../../aws-ec2';
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,10 @@ test('if isComplete is specified, the isComplete framework handler is also inclu
new cr.Provider(stack, 'MyProvider', {
onEventHandler: handler,
isCompleteHandler: handler,
waiterStateMachineLogOptions: {
includeExecutionData: true,
level: LogLevel.ALL,
},
});

// THEN
Expand Down Expand Up @@ -238,10 +243,84 @@ 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('a default LoggingConfiguration will be created even if waiterStateMachineLogOptions is not specified', () => {
// 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
new cr.Provider(stack, 'MyProvider', {
onEventHandler: handler,
isCompleteHandler: handler,
});

// THEN
Template.fromStack(stack).hasResourceProperties('AWS::StepFunctions::StateMachine', {
LoggingConfiguration: {
Destinations: [
{
CloudWatchLogsLogGroup: {
LogGroupArn: {
'Fn::GetAtt': [
'MyProviderwaiterstatemachineLogGroupD43CA868',
'Arn',
],
},
},
},
],
IncludeExecutionData: false,
Level: 'ERROR',
},
});
});

test('a default LoggingConfiguration will not be created if disableWaiterStateMachineLogging 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
new cr.Provider(stack, 'MyProvider', {
onEventHandler: handler,
isCompleteHandler: handler,
disableWaiterStateMachineLogging: true,
});

// THEN
Template.fromStack(stack).hasResourceProperties('AWS::StepFunctions::StateMachine', {
LoggingConfiguration: Match.absent(),
});
});

test('fails if "queryInterval" or "totalTimeout" or "waiterStateMachineLogOptions" or "disableWaiterStateMachineLogging" are set without "isCompleteHandler"', () => {
// GIVEN
const stack = new Stack();
const handler = new lambda.Function(stack, 'MyHandler', {
Expand All @@ -254,12 +333,25 @@ 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\", \"totalTimeout\", \"waiterStateMachineLogOptions\", and \"disableWaiterStateMachineLogging\" 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\", \"totalTimeout\", \"waiterStateMachineLogOptions\", and \"disableWaiterStateMachineLogging\" can only be configured if \"isCompleteHandler\" is specified. Otherwise, they have no meaning/);

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

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

describe('retry policy', () => {
Expand Down Expand Up @@ -448,4 +540,4 @@ describe('environment encryption', () => {
},
});
});
});
});
Loading
Loading