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

Using multiple SubscriptionFilter fails #4951

Closed
MariusRumpf opened this issue Nov 11, 2019 · 1 comment · Fixed by #4975
Closed

Using multiple SubscriptionFilter fails #4951

MariusRumpf opened this issue Nov 11, 2019 · 1 comment · Fixed by #4975
Assignees
Labels
@aws-cdk/aws-logs Related to Amazon CloudWatch Logs bug This issue is a bug. p1

Comments

@MariusRumpf
Copy link

MariusRumpf commented Nov 11, 2019

When creating multiple instances of a aws-log SubscriptionFilter for aws-lambda Function an error occurs. This is not the case using only a single instance. Same error occurs when using the .addSubscriptionFilter() method on the aws-log LogGroup directly.

Reproduction Steps

import { Stack, Construct, StackProps} from '@aws-cdk/core';
import { Function, Runtime, Code } from '@aws-cdk/aws-lambda';
import { LogGroup, SubscriptionFilter, FilterPattern } from '@aws-cdk/aws-logs';
import { LambdaDestination } from '@aws-cdk/aws-logs-destinations';

export class LoggingStack extends Stack {
  constructor(scope: Construct, id: string, props?: StackProps) {
    super(scope, id, props);

    const handler = new Function(this, 'LoggingLambda', {
      runtime: Runtime.PYTHON_2_7,
      code: Code.asset('lambda'),
      handler: 'logging.lambda_handler',
    });

    const logGroupIr = new LogGroup(this, 'IrLogGroup', {
      logGroupName: '/aws/lambda/logging-1'
    });

    const logGroupUpload = new LogGroup(this, 'UploadLogGroup', {
      logGroupName: '/aws/lambda/logging-2'
    });

    const lambdaLogDest = new LambdaDestination(handler);

    new SubscriptionFilter(this, 'IrSubscription', {
      logGroup: logGroupIr,
      destination: lambdaLogDest,
      filterPattern: FilterPattern.allEvents(),
    });

    new SubscriptionFilter(this, 'UploadSubscription', {
      logGroup: logGroupUpload,
      destination: lambdaLogDest,
      filterPattern: FilterPattern.allEvents(),
    });
  }
}

Error Log

There is already a Construct with name 'InvokedByCloudWatchLogs'

Environment

  • CLI Version : 1.15.0 (build bdbe3aa)
  • Framework Version: 1.15.0
  • OS : Mac
  • Language : Typescript

Other

According to this code the permission might be attempted to be created multiple times.


This is 🐛 Bug Report

@MariusRumpf MariusRumpf added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Nov 11, 2019
@SomayaB SomayaB added the @aws-cdk/aws-logs Related to Amazon CloudWatch Logs label Nov 11, 2019
@rix0rrr rix0rrr added p1 and removed needs-triage This issue or PR still needs to be triaged. labels Nov 12, 2019
@rix0rrr
Copy link
Contributor

rix0rrr commented Nov 12, 2019

Yep, you are right. Thanks for reporting. As a workaround to get yourself unstuck, you can make your own implementation of LambdaDestination which does not have the issue.

import iam = require('@aws-cdk/aws-iam');
import lambda = require('@aws-cdk/aws-lambda');
import logs = require('@aws-cdk/aws-logs');
import { Construct } from '@aws-cdk/core';

/**
 * Use a Lamda Function as the destination for a log subscription
 */
export class LambdaDestination implements logs.ILogSubscriptionDestination {
  constructor(private readonly fn: lambda.IFunction) {
  }

  public bind(scope: Construct, logGroup: logs.ILogGroup): logs.LogSubscriptionDestinationConfig {
    const arn = logGroup.logGroupArn;

    this.fn.addPermission('CanInvokeLambda', {
      principal: new iam.ServicePrincipal(`logs.amazonaws.com`),
      sourceArn: arn,
      // Using SubScription Filter as scope is okay, since every Subscription Filter has only
      // one destination.
      scope
    });
    return { arn: this.fn.functionArn };
  }
}```

rix0rrr added a commit that referenced this issue Nov 12, 2019
We used to scope the permission for calling it from CloudWatch Logs
under the Lambda. However, when it was used multiple times there
would be a name conflict.

Instead, scope the Permission under the SubscriptionFilter.

Fixes #4951.
@mergify mergify bot closed this as completed in #4975 Nov 12, 2019
mergify bot pushed a commit that referenced this issue Nov 12, 2019
…4975)

We used to scope the permission for calling it from CloudWatch Logs
under the Lambda. However, when it was used multiple times there
would be a name conflict.

Instead, scope the Permission under the SubscriptionFilter.

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

Successfully merging a pull request may close this issue.

3 participants