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

(aws-events): Cross Stack Rule Requires Concrete Account #18405

Closed
jatsrt opened this issue Jan 13, 2022 · 20 comments · Fixed by #23549
Closed

(aws-events): Cross Stack Rule Requires Concrete Account #18405

jatsrt opened this issue Jan 13, 2022 · 20 comments · Fixed by #23549
Assignees
Labels
@aws-cdk/aws-events Related to CloudWatch Events bug This issue is a bug. p1

Comments

@jatsrt
Copy link

jatsrt commented Jan 13, 2022

What is the problem?

When setting up a rule against a lambda defined in another stack that is in the same account and region. CDK is now returning an error:

"You need to provide a concrete account for the target stack when using cross-account or cross-region events"

This is for any CDK version > 2.3.0

Reproduction Steps

Create and export the ARN of a lambda function from one stack. Add a policy to the function that allows "lambda:InvokeFunction". In another stack use "Function.fromFunctionArn"

Example:

const importedFunction = lambda.Function.fromFunctionArn(this, "ImportedFunction", cdk.Fn.importValue("OtherStack-ExportedFunctionARN"));

new events.Rule(this, "Rule", {
      schedule: events.Schedule.expression("cron(0 * * * ? *)"),
      targets: [
        new targets.LambdaFunction(importedFunction, {}),
      ],
    });

What did you expect to happen?

Expect the rule to be created and the rule is able to execute the remote function.

What actually happened?

/cdk/node_modules/aws-cdk-lib/aws-events/lib/rule.ts:134
          throw new Error('You need to provide a concrete account for the target stack when using cross-account or cross-region events');

CDK CLI Version

2.7.0

Framework Version

2.7.0

Node.js Version

16.13.1

OS

OSX M1

Language

Typescript

Language Version

4.5

Other information

No response

@jatsrt jatsrt added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jan 13, 2022
@github-actions github-actions bot added the @aws-cdk/aws-events Related to CloudWatch Events label Jan 13, 2022
@ryparker ryparker added guidance Question that needs advice or information. and removed bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jan 13, 2022
@ryparker
Copy link
Contributor

ryparker commented Jan 13, 2022

Her @jatsrt 👋🏻

This is intentional. Importing a resource using a method such as .fromFuncitonArn() will only work in stacks that are defined with an explicit account and region in their env property. If the AWS CDK attempts to look up an Amazon Lambda from an environment-agnostic stack, the CLI does not know which environment to query to find the Lambda.

See How to define environment in CDK
More on importing resources

@ryparker ryparker added @aws-cdk/aws-events Related to CloudWatch Events package/tools Related to AWS CDK Tools or CLI and removed @aws-cdk/aws-events Related to CloudWatch Events labels Jan 13, 2022
@glc-gplassard
Copy link

glc-gplassard commented Jan 14, 2022

Hello,

as discussed in #18255, this used to work when using cdk.ScopedAws (in cdk 1.138.0). CDK didn't need to know the region & account when using the cdk.ScopedAws().{region, accountId} because it uses the Ref AWS::Region & Ref AWS::AccountId from cloudformation

     Targets:
        - Arn:
            Fn::Join:
              - ""
              - - "arn:aws:lambda:"
                - Ref: AWS::Region
                - ":"
                - Ref: AWS::AccountId
                - :function:functionName
          Id: Target0

I created a reproducer stack (when using with codepipeline since it's my use case but the issue seems to be the same) : https://github.com/cbm-gplassard/cdk-lambda-events-reproducer

Maybe the cdk.ScopedAws().{region, accountId} should not trigger the cross-region / cross-account error ? And the error when specifying hardcoded regions / account could suggest to instead use cdk.ScopedAws if the user doesn't want to use cross-region / cross-account ?

@jatsrt
Copy link
Author

jatsrt commented Jan 14, 2022

@ryparker the top level env that is being passed in with the scope to the lookup has both account and region defined in it in this scenario. So, I think (again worked previously) this should work as it does have an account to base the lookup on.

@jatsrt
Copy link
Author

jatsrt commented Jan 14, 2022

This is what was generated by CDK <= 2.3.0, which for a same account, same region would be correct and scoped right.

        "Targets": [
          {
            "Arn": {
              "Fn::ImportValue": "OtherStack-ExportedFunctionARN"
            },
            "Id": "Target0"
          }
        ]

@jatsrt
Copy link
Author

jatsrt commented Jan 19, 2022

@ryparker and @cbm-gplassard Ae we sure we are talking about the same issue.
From my testing this is a regression that appeared after 2.3. With multiple stacks, the account and region defined in each stack, this is not not working where it was before.

@glc-gplassard
Copy link

Hello @jatsrt ,

indeed you're right we don't have exactly the same use case as if I understand correctly

  • you pass the entire ARN in a stack to which you pass the account & region
  • I construct the ARN dynamically using ScodedAws in a stack in which I don't pass the region & account

But both use cases trigger the same error message and might be cause by the same issue. And as you said both use cases used to work with previous versions and should continue to work IMHO

@peterwoodworth peterwoodworth assigned ryparker and unassigned rix0rrr Jan 22, 2022
@Kruspe
Copy link
Contributor

Kruspe commented Jan 24, 2022

I see the same issue @jatsrt is describing. Through the change from #18255 the account and region are trying to be imported from the lambda arn which is not known yet, since it is being imported.

Not sure if this is something we can fix in the events section here.
In my opinion it would make more sense to check if the lambda arn is a token and then use the env property instead.

@markilott
Copy link

markilott commented Feb 2, 2022

We are blocked with the same issue as @jatsrt but with a slightly different config:

const functionArn = Fn.importValue('StackName:exportedFncArn');
const importedtFnc = lambda.Function.fromFunctionAttributes(this, 'importedtFnc', { functionArn, sameEnvironment: true });
new targets.LambdaFunction(importedtFnc, {});

Account and region are passed in with the scope so are known at synth time (Stack is part of a Stage).

We are also using the sameEnvironment flag here to allow for permission changes - could the same flag disable this cross-account error check?

As above it was working in v2.3

@markilott
Copy link

Workaround:
Exporting stack:

new CfnOutput(this, 'exportedFncName', {
       description: 'Name of the exported Function',
       exportName: 'exportedFncName',
       value: exportedFnc.functionName,
});

Importing stack:

const functionName = Fn.importValue('exportedFncName');
const functionArn = `arn:aws:lambda:${this.region}:${this.account}:function:${functionName}`;
const importedtFnc = lambda.Function.fromFunctionAttributes(this, 'importedtFnc', { functionArn, sameEnvironment: true });
new targets.LambdaFunction(importedtFnc, {});

@peterwoodworth
Copy link
Contributor

Hey all, thanks for the discussion here.

I can reproduce this issue that's been described here. I can confirm that this functionality was working prior to v2.4.0, and is still broken on the current version.

Interestingly enough, I've found that the issue only occurs if you specify the environment on your stack.

@peterwoodworth peterwoodworth added bug This issue is a bug. and removed guidance Question that needs advice or information. package/tools Related to AWS CDK Tools or CLI labels Feb 7, 2022
@peterwoodworth
Copy link
Contributor

peterwoodworth commented Feb 7, 2022

I believe adding TokenComparison.ONE_UNRESOLVED here would fix the problem, but that might break some of the protection the assertions offer.

return [TokenComparison.SAME, TokenComparison.BOTH_UNRESOLVED].includes(Token.compareStrings(dim1, dim2));

@jamiepmullan
Copy link
Contributor

jamiepmullan commented Feb 28, 2022

Hey @peterwoodworth - are there any plans to fix this? Really blocked by this at the moment

@stephenwiebe
Copy link
Contributor

@peterwoodworth I'm blocked by this too, unfortunately. I tried the workaround suggested by @markilott (using Function.fromFunctionAttributes instead of Function.fromFunctionArn), but still got the same error.

@rix0rrr
Copy link
Contributor

rix0rrr commented Mar 31, 2022

You can know the Lambda Function is definitely in the same account, but CDK can't know that because all you're giving it is an ARN that it can't look into (Fn.importValue()). It could be anything and anywhere, but in only a subset of cases can we successfully trigger that rule.

lambda.Function.fromFunctionArn(this, 'Fn', fn.importValue('FunctionArn'));
//                         ^^^^^               ^^^^^^^^^^^^
//                      the combination of these is the problem

The solution is to use Function.fromFunctionName(), which (due to lack of place to put an account and region) is forced to assume the Function must be in the same account and region as the Stack itself.

If all you have is an ARN, parse it first to get the function name out (dropping the account and region on the floor, telling CDK not to worry about it):

    const functionArn = Fn.importValue('FunctionArn');
    const functionName = Arn.split(functionArn, ArnFormat.COLON_RESOURCE_NAME).resourceName;
    const fn = lambda.Function.fromFunctionName(this, 'Fn', functionName!);

In case the resource you are importing only has a fromResourceArn method (and not a fromResourceName method) like a StepFunctions StateMachine, you need to take apart the ARN and put it together again using the stack's region and account:

const stateMachineArn = Fn.importValue('StateMachineArn');
const stackLocalArn = stack.formatArn({
  ...stack.splitArn(stateMachineArn,ArnFormat.COLON_RESOURCE_NAME),
  account: stack.account,
  region: stack.region,
});

const stateMachine = sfn.StateMachine.fromStateMachineArn(stack, 'SM', stackLocalArn);

@gravitylow
Copy link

Why is there not just an easy way to override the environment for any event target? Seems silly to have to mess around with all this ARN parsing logic when typically the environment is known and can just be injected manually.

@rix0rrr
Copy link
Contributor

rix0rrr commented Apr 4, 2022

@gravitylow would you rather have:

 const fn = lambda.Function.fromFunctionArn(this, 'Fn', Fn.importValue('FunctionArn'), {
  env: {
    account: Stack.of(this).account,
    region: Stack.of(this).region,
  },
});

?

@gravitylow
Copy link

Actually in my case I'm dealing with a stepFunction having this issue, so I'd rather be able to override it at the IRuleTarget level, for instance:

            new Rule(this, `RunTask-${value}`, {
                ruleName: `RunTask-${value}`,
                schedule: Schedule.rate(Duration.minutes(tasksToInterval[value])),
                targets: [new SfnStateMachine(this.stateMachine, {
                    input: RuleTargetInput.fromObject({"inputTaskType": value, "inputTaskId": null}),
                    role: asyncRole,
                    deadLetterQueue: dlq,
                    env: { account: Stack.of(this).account, region: Stack.of(this).region }
                })],
            });

@rix0rrr rix0rrr self-assigned this May 4, 2022
@rix0rrr
Copy link
Contributor

rix0rrr commented Jun 1, 2022

I am convinced now that treating a "floating" region and account as being equal to the "current" region and account is the most desirable behavior. While the current way is technically correct it is also very inconvenient in many common use cases. The use case we are optimizing for (cross-env targets) is likely to be much more rare.

@peterpham
Copy link

I upgrade from cdk v1.45 to cdk 1.168 and is getting the same error when importing state machine of the same stack

eventRule.addTarget(
      new eventTargets.SfnStateMachine(
        sfn.StateMachine.fromStateMachineArn(this, 'ApprovalWorkflowSF' , workflow.attrArn)
      , {
        input: events.RuleTargetInput.fromEventPath("$.detail")
      })
    );

@mergify mergify bot closed this as completed in #23549 Jan 4, 2023
mergify bot pushed a commit that referenced this issue Jan 4, 2023
When a rule and a rule target are in different environments (account and/or region) we have to perform some extra setup to get the cross environment stuff working. Previously if one of the environments was not defined then we assumed that we were working in a cross environment fashion and we threw an error message. It is probably a much more common scenario for both the stacks to be deployed to the same environment if one of the environments is not defined.

This PR updates the logic to make that assumption and to provide a warning to the user that if they _are_ working in a cross environment setup they need to provide the environment.

fix #18405


----

### All Submissions:

* [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Construct Runtime Dependencies:

* [ ] This PR adds new construct runtime dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-construct-runtime-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

github-actions bot commented Jan 4, 2023

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-events Related to CloudWatch Events bug This issue is a bug. p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.