fix: event manager cyclic dep (#47)

* Refactor EventManager to prevent cyclic dependencies

- Modify EventManager.addTargetToEvent to accept a scope parameter
- Get or create event rules within the provided scope
- This ensures that event rules are created in the caller's scope
  instead of the eventManager's scope
- Add tests to verify the behavior of EventManager with different
  configurations

* update API.md

* update docs

src/control-plane/billing/billing-provider.ts

@@ -153,6 +153,6 @@ export class BillingProvider extends Construct {
     }
 
     const { handler, trigger } = this.getFunctionProps(fn, defaultEvent);
-    eventManager.addTargetToEvent(trigger, new event_targets.LambdaFunction(handler));
+    eventManager.addTargetToEvent(this, trigger, new event_targets.LambdaFunction(handler));
   }
 }

src/control-plane/control-plane.ts

@@ -88,11 +88,13 @@ export class ControlPlane extends Construct {
     this.controlPlaneAPIGatewayUrl = controlPlaneAPI.apiUrl;
 
     this.eventManager.addTargetToEvent(
+      this,
       DetailType.PROVISION_SUCCESS,
       controlPlaneAPI.tenantUpdateServiceTarget
     );
 
     this.eventManager.addTargetToEvent(
+      this,
       DetailType.DEPROVISION_SUCCESS,
       controlPlaneAPI.tenantUpdateServiceTarget
     );

src/core-app-plane/core-app-plane.ts

@@ -133,7 +133,11 @@ export class CoreApplicationPlane extends Construct {
         bashJobRunner: job,
       });
 
-      this.eventManager.addTargetToEvent(jobRunnerProps.incomingEvent, jobOrchestrator.eventTarget);
+      this.eventManager.addTargetToEvent(
+        this,
+        jobRunnerProps.incomingEvent,
+        jobOrchestrator.eventTarget
+      );
     });
   }
 }

src/utils/event-manager.ts

@@ -171,7 +171,7 @@ export interface IEventManager {
    * @param eventType The detail type of the event to add a target to.
    * @param target The target that will be added to the event.
    */
-  addTargetToEvent(eventType: DetailType, target: IRuleTarget): void;
+  addTargetToEvent(scope: Construct, eventType: DetailType, target: IRuleTarget): void;
 
   /**
    * Provides grantee the permissions to place events
@@ -218,8 +218,6 @@ export class EventManager extends Construct implements IEventManager {
    */
   public readonly busArn: string;
 
-  private readonly connectedRules: Map<DetailType, Rule> = new Map<DetailType, Rule>();
-
   constructor(scope: Construct, id: string, props?: EventManagerProps) {
     super(scope, id);
     addTemplateTag(this, 'EventManager');
@@ -275,25 +273,32 @@ export class EventManager extends Construct implements IEventManager {
   /**
    * Adds an IRuleTarget to an event.
    *
+   * @param scope The scope in which to find (or create) the Rule.
    * @param eventType The detail type of the event to add a target to.
    * @param target The target that will be added to the event.
    */
-  public addTargetToEvent(eventType: DetailType, target: IRuleTarget): void {
-    this.getOrCreateRule(eventType).addTarget(target);
+  public addTargetToEvent(scope: Construct, eventType: DetailType, target: IRuleTarget): void {
+    this.getOrCreateRule(scope, eventType).addTarget(target);
   }
 
-  private getOrCreateRule(eventType: DetailType): Rule {
-    let rule = this.connectedRules.get(eventType);
-
+  /**
+   * Returns a Rule for the given eventType in the context of a scope.
+   * A new one is created if the rule is not found in the scope.
+   *
+   * @param scope The scope in which to find (or create) the Rule.
+   * @param eventType The detail type of the event to add a target to.
+   * @returns A Rule for the given eventType in the provided scope.
+   */
+  private getOrCreateRule(scope: Construct, eventType: DetailType): Rule {
+    let rule = scope.node.tryFindChild(`${eventType}Rule`) as Rule;
     if (!rule) {
-      rule = new Rule(this, `${eventType}Rule`, {
+      rule = new Rule(scope, `${eventType}Rule`, {
         eventBus: this.eventBus,
         eventPattern: {
           source: [this.supportedEvents[eventType]],
           detailType: [eventType],
         },
       });
-      this.connectedRules.set(eventType, rule);
     }
 
     return rule;

test/core-app-plane.test.ts

@@ -173,7 +173,6 @@ describe('CoreApplicationPlane', () => {
     const template = Template.fromStack(coreApplicationPlaneStack);
 
     template.hasResourceProperties('AWS::CodeBuild::Project', {
-      // check that codebuild has the MY_TEST_ENV_VAR environment variable defined
       Environment: Match.objectLike({
         EnvironmentVariables: Match.anyValue(),
       }),

test/event-manager.test.ts

@@ -0,0 +1,195 @@
+// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+import * as cdk from 'aws-cdk-lib';
+import { Capture, Template } from 'aws-cdk-lib/assertions';
+import { Effect, PolicyDocument, PolicyStatement } from 'aws-cdk-lib/aws-iam';
+import { AwsSolutionsChecks } from 'cdk-nag';
+import { CognitoAuth, ControlPlane } from '../src/control-plane';
+import { CoreApplicationPlane } from '../src/core-app-plane';
+import { DetailType, EventManager } from '../src/utils';
+
+function testForDuplicateRulesInCoreAppPlaneStack(stack: cdk.Stack) {
+  const template = Template.fromStack(stack);
+  const eventRuleCapture = new Capture();
+
+  template.allResourcesProperties('AWS::Events::Rule', eventRuleCapture);
+  const eventRules: { [key: string]: any } = [];
+  do {
+    eventRules.push(eventRuleCapture.asObject());
+  } while (eventRuleCapture.next());
+
+  const onboardingRequestEventRules = eventRules.filter(
+    (eventRule: { EventPattern: { 'detail-type': string[] } }) => {
+      return eventRule.EventPattern['detail-type'][0] === DetailType.ONBOARDING_REQUEST;
+    }
+  );
+
+  it('should not have duplicate rules for the same event detail type', () => {
+    expect(onboardingRequestEventRules).toHaveLength(1);
+  });
+
+  it('should reuse the same rules for targets triggered by the same event detail type', () => {
+    expect(onboardingRequestEventRules[0].Targets).toHaveLength(2);
+  });
+}
+
+function testForRulesInOtherStack(stack: cdk.Stack) {
+  const template = Template.fromStack(stack);
+  const eventRuleCapture = new Capture();
+
+  const rules = template.findResources('AWS::Events::Rule', eventRuleCapture);
+  console.log(rules);
+  if (Object.keys(rules).length === 0) {
+    it('should not have rules for triggering runners in stacks that do not define them', () => {
+      expect(true).toBe(true);
+    });
+    // return as there are no rules in this stack
+    return;
+  }
+
+  // template.allResourcesProperties('AWS::Events::Rule', eventRuleCapture);
+  const eventRules: { [key: string]: any } = [];
+  do {
+    eventRules.push(eventRuleCapture.asObject());
+  } while (eventRuleCapture.next());
+
+  console.log(eventRules);
+  const onboardingRequestEventRules = eventRules.filter(
+    (eventRule: { EventPattern?: { 'detail-type': string[] } }) => {
+      return (
+        eventRule.EventPattern &&
+        eventRule.EventPattern['detail-type'][0] === DetailType.ONBOARDING_REQUEST
+      );
+    }
+  );
+
+  it('should not have rules for triggering runners in stacks that do not define them', () => {
+    expect(onboardingRequestEventRules).toHaveLength(0);
+  });
+}
+
+const samplePolicyDocument = new PolicyDocument({
+  statements: [
+    new PolicyStatement({
+      actions: ['cloudformation:CreateStack'],
+      resources: ['arn:aws:cloudformation:*:*:stack/MyStack/ExampleStack'],
+      effect: Effect.ALLOW,
+    }),
+  ],
+});
+
+const jobsUsingTheSameIncomingEvent = [
+  {
+    name: 'provisioning-1',
+    outgoingEvent: DetailType.PROVISION_SUCCESS,
+    incomingEvent: DetailType.ONBOARDING_REQUEST,
+    permissions: samplePolicyDocument,
+    script: '',
+  },
+  {
+    name: 'provisioning-2',
+    outgoingEvent: DetailType.PROVISION_SUCCESS,
+    incomingEvent: DetailType.ONBOARDING_REQUEST,
+    permissions: samplePolicyDocument,
+    script: '',
+  },
+];
+
+describe('EventManager', () => {
+  describe('using default event-manager created by control plane', () => {
+    const app = new cdk.App();
+    const controlPlaneStack = new cdk.Stack(app, 'ControlPlaneStack');
+
+    const cognitoAuth = new CognitoAuth(controlPlaneStack, 'CognitoAuth', {
+      systemAdminEmail: '[email protected]',
+    });
+
+    const controlPlane = new ControlPlane(controlPlaneStack, 'ControlPlane', {
+      auth: cognitoAuth,
+    });
+
+    const coreAppPlaneStack = new cdk.Stack(app, 'CoreApplicationPlaneStack');
+    new CoreApplicationPlane(coreAppPlaneStack, 'CoreApplicationPlane', {
+      eventManager: controlPlane.eventManager,
+      jobRunnerPropsList: jobsUsingTheSameIncomingEvent,
+    });
+
+    cdk.Aspects.of(controlPlaneStack).add(new AwsSolutionsChecks({ verbose: true }));
+    cdk.Aspects.of(coreAppPlaneStack).add(new AwsSolutionsChecks({ verbose: true }));
+
+    it('should synth without errors', () => {
+      const assembly = app.synth();
+      expect(assembly).toBeTruthy();
+    });
+
+    testForDuplicateRulesInCoreAppPlaneStack(coreAppPlaneStack);
+    testForRulesInOtherStack(controlPlaneStack);
+  });
+
+  describe('using event-manager created outside of control plane', () => {
+    const app = new cdk.App();
+    const controlPlaneStack = new cdk.Stack(app, 'ControlPlaneStack');
+
+    const eventManager = new EventManager(controlPlaneStack, 'EventManager');
+
+    const cognitoAuth = new CognitoAuth(controlPlaneStack, 'CognitoAuth', {
+      systemAdminEmail: '[email protected]',
+    });
+
+    new ControlPlane(controlPlaneStack, 'ControlPlane', {
+      auth: cognitoAuth,
+      eventManager: eventManager,
+    });
+
+    const coreAppPlaneStack = new cdk.Stack(app, 'CoreApplicationPlaneStack');
+    new CoreApplicationPlane(coreAppPlaneStack, 'CoreApplicationPlane', {
+      eventManager: eventManager,
+      jobRunnerPropsList: jobsUsingTheSameIncomingEvent,
+    });
+
+    cdk.Aspects.of(controlPlaneStack).add(new AwsSolutionsChecks({ verbose: true }));
+    cdk.Aspects.of(coreAppPlaneStack).add(new AwsSolutionsChecks({ verbose: true }));
+
+    it('should synth without errors', () => {
+      const assembly = app.synth();
+      expect(assembly).toBeTruthy();
+    });
+
+    testForDuplicateRulesInCoreAppPlaneStack(coreAppPlaneStack);
+    testForRulesInOtherStack(controlPlaneStack);
+  });
+
+  describe('using an event-manager created in a separate stack', () => {
+    const app = new cdk.App();
+    const eventManagerStack = new cdk.Stack(app, 'EventManagerStack');
+    const eventManager = new EventManager(eventManagerStack, 'EventManager');
+
+    const controlPlaneStack = new cdk.Stack(app, 'ControlPlaneStack');
+    const cognitoAuth = new CognitoAuth(controlPlaneStack, 'CognitoAuth', {
+      systemAdminEmail: '[email protected]',
+    });
+
+    new ControlPlane(controlPlaneStack, 'ControlPlane', {
+      auth: cognitoAuth,
+      eventManager: eventManager,
+    });
+
+    const coreAppPlaneStack = new cdk.Stack(app, 'CoreApplicationPlaneStack');
+    new CoreApplicationPlane(coreAppPlaneStack, 'CoreApplicationPlane', {
+      eventManager: eventManager,
+      jobRunnerPropsList: jobsUsingTheSameIncomingEvent,
+    });
+    cdk.Aspects.of(controlPlaneStack).add(new AwsSolutionsChecks({ verbose: true }));
+    cdk.Aspects.of(coreAppPlaneStack).add(new AwsSolutionsChecks({ verbose: true }));
+
+    it('should synth without errors', () => {
+      const assembly = app.synth();
+      expect(assembly).toBeTruthy();
+    });
+
+    testForDuplicateRulesInCoreAppPlaneStack(coreAppPlaneStack);
+    testForRulesInOtherStack(controlPlaneStack);
+    testForRulesInOtherStack(eventManagerStack);
+  });
+});