Merge pull request #41 from MarcioMeier/feat/dont-add-permission-to-s…

…ubscribed-logs-lambda feat: don't add permission to the log subscribed lambda
flaviostutz · Oct 8, 2024 · 415c850 · 415c850
2 parents 94aa3c5 + 53621d0
commit 415c850
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 11 deletions.
diff --git a/docs/base-nodejs-lambda.md b/docs/base-nodejs-lambda.md
@@ -8,7 +8,8 @@ Based on [AWS Construct NodeJsFunction](https://docs.aws.amazon.com/cdk/api/v2/d
   - explicit private VPC configuration (see props.network)
   - source code path standardization to "[basePath]/[lambdaEventType]/[lambdaName]/index.ts" (can be overwritten by explicit props.entry)
   - custom CA support for HTTP calls (NodeJS NODE_EXTRA_CA_CERTS). See props.extraCaPubCert
-  - option to subscribe an Lambda Arn to the log group related to the Lambda function. See props.logGroupSubscriberLambdaArn
+  - option to subscribe an Lambda Arn to the log group related to the Lambda function. See props.
+    - **Be aware** that you need to handle the permissions by yourself - we don't add the lambda permission to invoke the lambda in this construct (#40)
   - adds environment STAGE to Lambda. See props.stage
 
 ### Usage

diff --git a/lib/src/lambda/lambda-base.test.ts b/lib/src/lambda/lambda-base.test.ts
@@ -117,11 +117,15 @@ describe('lambda-base', () => {
       FilterPattern: '',
     });
 
-    template.hasResourceProperties('AWS::Lambda::Permission', {
-      FunctionName: 'arn:aws:lambda:eu-west-1:012345678:function:tstLogging',
-      Action: 'lambda:InvokeFunction',
-      Principal: 'logs.eu-west-1.amazonaws.com',
-    });
+    template.resourcePropertiesCountIs(
+      'AWS::Lambda::Permission',
+      {
+        FunctionName: 'arn:aws:lambda:eu-west-1:012345678:function:tstLogging',
+        Action: 'lambda:InvokeFunction',
+        Principal: 'logs.eu-west-1.amazonaws.com',
+      },
+      0,
+    );
   });
 
   it('should allow ssm log group subscriptions', async () => {

diff --git a/lib/src/lambda/lambda-base.ts b/lib/src/lambda/lambda-base.ts
@@ -12,7 +12,6 @@ import {
   ScalableTarget,
   ServiceNamespace,
 } from 'aws-cdk-lib/aws-applicationautoscaling';
-import { ServicePrincipal } from 'aws-cdk-lib/aws-iam';
 import { RemovalPolicy } from 'aws-cdk-lib/core';
 import { StringParameter } from 'aws-cdk-lib/aws-ssm';
 
@@ -276,13 +275,15 @@ const addLogSubscriber = (
   );
   new SubscriptionFilter(nodeJsFunction, 'Subscription', {
     logGroup: nodeJsFunction.logGroup,
-    destination: new LambdaDestination(logGroupFuncSubscriber),
+    destination: new LambdaDestination(logGroupFuncSubscriber, {
+      // ? We are not adding the permission because there is a size limit for the policy document
+      // ? After a certain number of lambda permissions, the policy document will be too large and will throw an error
+      // ? The lambda invoke permission should be held in the original log group lambda stack instead
+      addPermissions: false,
+    }),
     filterPattern: FilterPattern.allEvents(),
     filterName: 'all',
   });
-  logGroupFuncSubscriber.addPermission('allow-log-subscriber', {
-    principal: new ServicePrincipal('logs.eu-west-1.amazonaws.com'),
-  });
 };
 
 /**