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.

Sign up for GitHub

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

Jump to bottom

fix(scheduler-targets-alpha): imported lambda function as schedule target throws synth error #31837

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

fix(scheduler-targets-alpha): imported lambda function as schedule target throws synth error #31837

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ba14f9a
remove lambda target same env check
samson-keung Oct 20, 2024
8c05610
fix import name
samson-keung Oct 21, 2024
22a8253
test integ test
samson-keung Oct 21, 2024
16f6ce9
fix integ test assertion
samson-keung Oct 21, 2024
0e5df38
update integ test to test imported lambda function
samson-keung Oct 21, 2024
cbb0c4c
Merge branch 'main' into issue-29284-schedule-lambda-target
samson-keung Oct 21, 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
23 changes: 6 additions & 17 deletions packages/@aws-cdk/aws-scheduler-targets-alpha/lib/lambda-invoke.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,34 +1,23 @@
import { ISchedule, IScheduleTarget } from '@aws-cdk/aws-scheduler-alpha';
import { Names } from 'aws-cdk-lib';
import { IRole } from 'aws-cdk-lib/aws-iam';
import * as lambda from 'aws-cdk-lib/aws-lambda';
import { ScheduleTargetBase, ScheduleTargetBaseProps } from './target';
import { sameEnvDimension } from './util';

/**
* Use an AWS Lambda function as a target for AWS EventBridge Scheduler.
*/
export class LambdaInvoke extends ScheduleTargetBase implements IScheduleTarget {
private readonly func: lambda.IFunction;

constructor(
private readonly func: lambda.IFunction,
private readonly props: ScheduleTargetBaseProps,
func: lambda.IFunction,
props: ScheduleTargetBaseProps,
) {
super(props, func.functionArn);
this.func = func;
GavinZZ marked this conversation as resolved.
Show resolved Hide resolved
}

protected addTargetActionToRole(schedule: ISchedule, role: IRole): void {
if (!sameEnvDimension(this.func.env.region, schedule.env.region)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this would fix the problem mentioned in the issue, I don't think this is a good fix in general since it removes check even for functions and scheduler with known account & regions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is correct. The reasoning to remove the check is explained here.

throw new Error(`Cannot assign function in region ${this.func.env.region} to the schedule ${Names.nodeUniqueId(schedule.node)} in region ${schedule.env.region}. Both the schedule and the function must be in the same region.`);
}

if (!sameEnvDimension(this.func.env.account, schedule.env.account)) {
throw new Error(`Cannot assign function in account ${this.func.env.account} to the schedule ${Names.nodeUniqueId(schedule.node)} in account ${schedule.env.region}. Both the schedule and the function must be in the same account.`);
}

if (this.props.role && !sameEnvDimension(this.props.role.env.account, this.func.env.account)) {
throw new Error(`Cannot grant permission to execution role in account ${this.props.role.env.account} to invoke target ${Names.nodeUniqueId(this.func.node)} in account ${this.func.env.account}. Both the target and the execution role must be in the same account.`);
}

protected addTargetActionToRole(_schedule: ISchedule, role: IRole): void {
this.func.grantInvoke(role);
}
}
Loading