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

chore(scheduler-targets-alpha): imported function as a LambdaInvoke target results in synth-time error #29615

Closed
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
7 changes: 6 additions & 1 deletion packages/@aws-cdk/aws-scheduler-targets-alpha/lib/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,12 @@ import { Token, TokenComparison } from 'aws-cdk-lib';
*
* Used to compare either accounts or regions, and also returns true if both
* are unresolved (in which case both are expted to be "current region" or "current account").
*
* Also returns true if one is unresolved (in which case we expect the unresolved dimension to match
* the resolved dimension, but it is up to the user to ensure this). Returning true here makes sure
* that we are not overly aggressive in producing a synth-time error.
*
*/
export function sameEnvDimension(dim1: string, dim2: string) {
return [TokenComparison.SAME, TokenComparison.BOTH_UNRESOLVED].includes(Token.compareStrings(dim1, dim2));
return [TokenComparison.SAME, TokenComparison.ONE_UNRESOLVED, TokenComparison.BOTH_UNRESOLVED].includes(Token.compareStrings(dim1, dim2));
Copy link
Contributor

@aaythapa aaythapa Apr 4, 2024

Choose a reason for hiding this comment

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

Correct me if I'm wrong but this essentially negates the need for the validation as, I believe now, all cases will return true.

EDIT: I see that TokenComparison.DIFFERENT can also be returned which would return false for this function. I'm still not sure if this is the best way to go about resolving this issue as it could change more things than just fixing this issue. Will consult with the team

Copy link
Author

Choose a reason for hiding this comment

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

See comment: #29615 (comment)

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { ScheduleExpression, Schedule } from '@aws-cdk/aws-scheduler-alpha';
import { App, Duration, Stack } from 'aws-cdk-lib';
import { Schedule, ScheduleExpression } from '@aws-cdk/aws-scheduler-alpha';
import { App, Duration, Fn, Stack } from 'aws-cdk-lib';
import { Template } from 'aws-cdk-lib/assertions';
import { AccountRootPrincipal, Role } from 'aws-cdk-lib/aws-iam';
import * as lambda from 'aws-cdk-lib/aws-lambda';
Expand Down Expand Up @@ -319,6 +319,31 @@ describe('schedule target', () => {
});
});

test('import lambda function from different stack in the same account', () => {
const importValueName = 'MyFunctionArn';
const functionArn = Fn.importValue(importValueName);

const importedFunc = lambda.Function.fromFunctionAttributes(stack, 'ImportedFunction', {
functionArn,
sameEnvironment: true,
});

const lambdaTarget = new LambdaInvoke(importedFunc, {});

new Schedule(stack, 'MyScheduleDummy', {
schedule: expr,
target: lambdaTarget,
});

Template.fromStack(stack).hasResource('AWS::Scheduler::Schedule', {
Properties: {
Target: {
Arn: { 'Fn::ImportValue': importValueName },
},
},
});
});

test('throws when lambda function is imported from different account', () => {
const importedFunc = lambda.Function.fromFunctionArn(stack, 'ImportedFunction', 'arn:aws:lambda:us-east-1:234567890123:function/somefunc');

Expand Down
Loading