Skip to content

Commit 22d3341

Browse files
authored
fix(events): cross stack rules require concrete environment (#23549)
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*
1 parent 44a4812 commit 22d3341

File tree

3 files changed

+58
-29
lines changed

3 files changed

+58
-29
lines changed

packages/@aws-cdk/aws-events/lib/rule.ts

+27-6
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { IRole, PolicyStatement, Role, ServicePrincipal } from '@aws-cdk/aws-iam';
2-
import { App, IResource, Lazy, Names, Resource, Stack, Token, TokenComparison, PhysicalName, ArnFormat } from '@aws-cdk/core';
2+
import { App, IResource, Lazy, Names, Resource, Stack, Token, TokenComparison, PhysicalName, ArnFormat, Annotations } from '@aws-cdk/core';
33
import { Node, Construct } from 'constructs';
44
import { IEventBus } from './event-bus';
55
import { EventPattern } from './event-pattern';
@@ -8,7 +8,7 @@ import { EventCommonOptions } from './on-event-options';
88
import { IRule } from './rule-ref';
99
import { Schedule } from './schedule';
1010
import { IRuleTarget } from './target';
11-
import { mergeEventPattern, renderEventPattern, sameEnvDimension } from './util';
11+
import { mergeEventPattern, renderEventPattern } from './util';
1212

1313
/**
1414
* Properties for defining an EventBridge Rule
@@ -163,7 +163,7 @@ export class Rule extends Resource implements IRule {
163163
// - forwarding rule in the source stack (target: default event bus of the receiver region)
164164
// - eventbus permissions policy (creating an extra stack)
165165
// - receiver rule in the target stack (target: the actual target)
166-
if (!sameEnvDimension(sourceAccount, targetAccount) || !sameEnvDimension(sourceRegion, targetRegion)) {
166+
if (!this.sameEnvDimension(sourceAccount, targetAccount) || !this.sameEnvDimension(sourceRegion, targetRegion)) {
167167
// cross-account and/or cross-region event - strap in, this works differently than regular events!
168168
// based on:
169169
// https://docs.aws.amazon.com/eventbridge/latest/userguide/eb-cross-account.html
@@ -332,7 +332,7 @@ export class Rule extends Resource implements IRule {
332332

333333
// For some reason, cross-region requires a Role (with `PutEvents` on the
334334
// target event bus) while cross-account doesn't
335-
const roleArn = !sameEnvDimension(targetRegion, Stack.of(this).region)
335+
const roleArn = !this.sameEnvDimension(targetRegion, Stack.of(this).region)
336336
? this.crossRegionPutEventsRole(eventBusArn).roleArn
337337
: undefined;
338338

@@ -355,7 +355,7 @@ export class Rule extends Resource implements IRule {
355355
//
356356
// For different region, no need for a policy on the target event bus (but a need
357357
// for a role).
358-
if (!sameEnvDimension(sourceAccount, targetAccount)) {
358+
if (!this.sameEnvDimension(sourceAccount, targetAccount)) {
359359
const stackId = `EventBusPolicy-${sourceAccount}-${targetRegion}-${targetAccount}`;
360360
let eventBusPolicyStack: Stack = sourceApp.node.tryFindChild(stackId) as Stack;
361361
if (!eventBusPolicyStack) {
@@ -394,7 +394,7 @@ export class Rule extends Resource implements IRule {
394394
private obtainMirrorRuleScope(targetStack: Stack, targetAccount: string, targetRegion: string): Construct {
395395
// for cross-account or cross-region events, we cannot create new components for an imported resource
396396
// because we don't have the target stack
397-
if (sameEnvDimension(targetStack.account, targetAccount) && sameEnvDimension(targetStack.region, targetRegion)) {
397+
if (this.sameEnvDimension(targetStack.account, targetAccount) && this.sameEnvDimension(targetStack.region, targetRegion)) {
398398
return targetStack;
399399
}
400400

@@ -426,6 +426,27 @@ export class Rule extends Resource implements IRule {
426426

427427
return role;
428428
}
429+
430+
431+
/**
432+
* Whether two string probably contain the same environment dimension (region or account)
433+
*
434+
* Used to compare either accounts or regions, and also returns true if one or both
435+
* are unresolved (in which case both are expected to be "current region" or "current account").
436+
*/
437+
private sameEnvDimension(dim1: string, dim2: string) {
438+
switch (Token.compareStrings(dim1, dim2)) {
439+
case TokenComparison.ONE_UNRESOLVED:
440+
Annotations.of(this).addWarning('Either the Event Rule or target has an unresolved environment. \n \
441+
If they are being used in a cross-environment setup you need to specify the environment for both.');
442+
return true;
443+
case TokenComparison.BOTH_UNRESOLVED:
444+
case TokenComparison.SAME:
445+
return true;
446+
default:
447+
return false;
448+
}
449+
}
429450
}
430451

431452
function determineRuleScope(scope: Construct, props: RuleProps): Construct {

packages/@aws-cdk/aws-events/lib/util.ts

+1-13
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import { Token, TokenComparison } from '@aws-cdk/core';
21
import { EventPattern } from './event-pattern';
32

43
/**
@@ -55,17 +54,6 @@ export function mergeEventPattern(dest: any, src: any) {
5554
}
5655
}
5756

58-
/**
59-
* Whether two string probably contain the same environment dimension (region or account)
60-
*
61-
* Used to compare either accounts or regions, and also returns true if both
62-
* are unresolved (in which case both are expted to be "current region" or "current account").
63-
* @internal
64-
*/
65-
export function sameEnvDimension(dim1: string, dim2: string) {
66-
return [TokenComparison.SAME, TokenComparison.BOTH_UNRESOLVED].includes(Token.compareStrings(dim1, dim2));
67-
}
68-
6957
/**
7058
* Transform an eventPattern object into a valid Event Rule Pattern
7159
* by changing detailType into detail-type when present.
@@ -86,4 +74,4 @@ export function renderEventPattern(eventPattern: EventPattern): any {
8674
}
8775

8876
return out;
89-
}
77+
}

packages/@aws-cdk/aws-events/test/rule.test.ts

+30-10
Original file line numberDiff line numberDiff line change
@@ -689,30 +689,50 @@ describe('rule', () => {
689689
const app = new cdk.App();
690690

691691
const sourceStack = new cdk.Stack(app, 'SourceStack');
692-
const rule = new Rule(sourceStack, 'Rule');
692+
const rule = new Rule(sourceStack, 'Rule', {
693+
eventPattern: {
694+
source: ['some-event'],
695+
},
696+
});
693697

694698
const targetAccount = '234567890123';
695699
const targetStack = new cdk.Stack(app, 'TargetStack', { env: { account: targetAccount } });
696700
const resource = new Construct(targetStack, 'Resource');
697-
698-
expect(() => {
699-
rule.addTarget(new SomeTarget('T', resource));
700-
}).toThrow(/You need to provide a concrete region/);
701+
rule.addTarget(new SomeTarget('T', resource));
702+
Template.fromStack(sourceStack).hasResourceProperties('AWS::Events::Rule', {
703+
'Targets': [
704+
{
705+
'Id': 'T',
706+
'Arn': 'ARN1',
707+
},
708+
],
709+
});
710+
Annotations.fromStack(sourceStack).hasWarning('/'+rule.node.path, Match.stringLikeRegexp('Either the Event Rule or target has an unresolved environment'));
701711
});
702712

703713
test('requires that the target stack specify a concrete account', () => {
704714
const app = new cdk.App();
705715

706716
const sourceAccount = '123456789012';
707717
const sourceStack = new cdk.Stack(app, 'SourceStack', { env: { account: sourceAccount } });
708-
const rule = new Rule(sourceStack, 'Rule');
718+
const rule = new Rule(sourceStack, 'Rule', {
719+
eventPattern: {
720+
source: ['some-event'],
721+
},
722+
});
709723

710724
const targetStack = new cdk.Stack(app, 'TargetStack');
711725
const resource = new Construct(targetStack, 'Resource');
712-
713-
expect(() => {
714-
rule.addTarget(new SomeTarget('T', resource));
715-
}).toThrow(/You need to provide a concrete account for the target stack when using cross-account or cross-region events/);
726+
rule.addTarget(new SomeTarget('T', resource));
727+
Template.fromStack(sourceStack).hasResourceProperties('AWS::Events::Rule', {
728+
'Targets': [
729+
{
730+
'Id': 'T',
731+
'Arn': 'ARN1',
732+
},
733+
],
734+
});
735+
Annotations.fromStack(sourceStack).hasWarning('/'+rule.node.path, Match.stringLikeRegexp('Either the Event Rule or target has an unresolved environment'));
716736
});
717737

718738
test('requires that the target stack specify a concrete region', () => {

0 commit comments

Comments
 (0)