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

(lambda): Support SNS Topics as DLQs #16246

Closed
kornicameister opened this issue Aug 26, 2021 · 4 comments · Fixed by #18546
Closed

(lambda): Support SNS Topics as DLQs #16246

kornicameister opened this issue Aug 26, 2021 · 4 comments · Fixed by #18546
Labels
@aws-cdk/aws-lambda Related to AWS Lambda effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2

Comments

@kornicameister
Copy link
Contributor

aws_lambda.Function does not support having sns.Topic as a target dead letter queue. It is incorrect as lambda does allow to set topic as a target.
Such information can be found here as well as can be seen in attached screenshot.

image

Reproduction Steps

#!/usr/bin/env python3
import os

from aws_cdk import (
    core as cdk,
    aws_lambda as fn,
    aws_sns as sns,
)


class BugTestStack(cdk.Stack):
    def __init__(self, scope, construct_id, **kwargs) -> None:
        super().__init__(scope, construct_id, **kwargs)

        dlq = sns.Topic(self, 'dlq')
        fun = fn.Function(
            self,
            'lambdaFn',
            code=fn.Code.from_inline('def handler(event, context): ...'),
            runtime=fn.Runtime.PYTHON_3_8,
            handler='index.handler',
            dead_letter_queue=dlq,
        )


app = cdk.App()
BugTestStack(app, "BugTestStack")

app.synth()

What did you expect to happen?

Expected lambda to deploy correctly and have topic configured as DQL.

What actually happened?

$ cdk deploy

This deployment will make potentially sensitive changes according to your current security approval level (--require-approval broadening).
Please confirm you intend to make the following modifications:

IAM Statement Changes
┌───┬──────────────────────┬────────┬──────────────────────┬──────────────────────┬───────────┐
│   │ Resource             │ Effect │ Action               │ Principal            │ Condition │
├───┼──────────────────────┼────────┼──────────────────────┼──────────────────────┼───────────┤
│ + │                      │ Allow  │ sqs:SendMessage      │ AWS:${lambdaFn/Servi │           │
│   │                      │        │                      │ ceRole}              │           │
├───┼──────────────────────┼────────┼──────────────────────┼──────────────────────┼───────────┤
│ + │ ${lambdaFn/ServiceRo │ Allow  │ sts:AssumeRole       │ Service:lambda.amazo │           │
│   │ le.Arn}              │        │                      │ naws.com             │           │
└───┴──────────────────────┴────────┴──────────────────────┴──────────────────────┴───────────┘
IAM Policy Changes
┌───┬─────────────────────────┬───────────────────────────────────────────────────────────────┐
│   │ Resource                │ Managed Policy ARN                                            │
├───┼─────────────────────────┼───────────────────────────────────────────────────────────────┤
│ + │ ${lambdaFn/ServiceRole} │ arn:${AWS::Partition}:iam::aws:policy/service-role/AWSLambdaB │
│   │                         │ asicExecutionRole                                             │
└───┴─────────────────────────┴───────────────────────────────────────────────────────────────┘
(NOTE: There may be security-related changes not in this list. See https://github.com/aws/aws-cdk/issues/1299)

Do you wish to deploy these changes (y/n)? y
BugTestStack: deploying...
BugTestStack: creating CloudFormation changeset...
10:01:05 PM | CREATE_FAILED        | AWS::IAM::Policy      | lambdaFnServiceRoleDefaultPolicyFB
1DE846
Policy statement must contain resources. (Service: AmazonIdentityManagement; Status Code: 400;
Error Code: MalformedPolicyDocument; Request ID: 76d252b1-e30d-473f-8c16-6cab739f2a46; Proxy: n
ull)

	new Policy (/private/var/folders/23/m0xp04h13tl7ht3bx69jgfvc0000gn/T/jsii-kernel-H52uSA/node_molowing resource(s) failed to create: [lambdaFnServiceRoleDefaultPolicyFB1DE846]. Rollbac
dules/@aws-cdk/aws-iam/lib/policy.js:56:26)
	\_ Role.addToPrincipalPolicy (/private/var/folders/23/m0xp04h13tl7ht3bx69jgfvc0000gn/T/jsii-ker
nel-H52uSA/node_modules/@aws-cdk/aws-iam/lib/role.js:209:34)
	\_ Function.addToRolePolicy (/private/var/folders/23/m0xp04h13tl7ht3bx69jgfvc0000gn/T/jsii-kern
el-H52uSA/node_modules/@aws-cdk/aws-lambda/lib/function-base.js:60:19)
	\_ Function.buildDeadLetterQueue (/private/var/folders/23/m0xp04h13tl7ht3bx69jgfvc0000gn/T/jsii
-kernel-H52uSA/node_modules/@aws-cdk/aws-lambda/lib/function.js:594:14)
	\_ new Function (/private/var/folders/23/m0xp04h13tl7ht3bx69jgfvc0000gn/T/jsii-kernel-H52uSA/no
de_modules/@aws-cdk/aws-lambda/lib/function.js:128:37)
	\_ /private/var/folders/23/m0xp04h13tl7ht3bx69jgfvc0000gn/T/tmpkb42qs7s/lib/program.js:8154:58
	\_ Kernel._wrapSandboxCode (/private/var/folders/23/m0xp04h13tl7ht3bx69jgfvc0000gn/T/tmpkb42qs7
s/lib/program.js:8582:24)
	\_ Kernel._create (/private/var/folders/23/m0xp04h13tl7ht3bx69jgfvc0000gn/T/tmpkb42qs7s/lib/pro
gram.js:8154:34)
	\_ Kernel.create (/private/var/folders/23/m0xp04h13tl7ht3bx69jgfvc0000gn/T/tmpkb42qs7s/lib/prog
ram.js:7895:29)
	\_ KernelHost.processRequest (/private/var/folders/23/m0xp04h13tl7ht3bx69jgfvc0000gn/T/tmpkb42q
s7s/lib/program.js:9479:36)
	\_ KernelHost.run (/private/var/folders/23/m0xp04h13tl7ht3bx69jgfvc0000gn/T/tmpkb42qs7s/lib/pro
gram.js:9442:22)
	\_ Immediate._onImmediate (/private/var/folders/23/m0xp04h13tl7ht3bx69jgfvc0000gn/T/tmpkb42qs7s
/lib/program.js:9443:46)
	\_ processImmediate (internal/timers.js:464:21)


 ❌  BugTestStack failed: Error: The stack named BugTestStack failed creation, it may need to be manually deleted from the AWS console: ROLLBACK_COMPLETE
    at Object.waitForStackDeploy (/Users/tomasztrebski/dev/dotfiles/dependencies/nodenv/versions/14.17.4/lib/node_modules/aws-cdk/lib/api/util/cloudformation.ts:305:11)
    at processTicksAndRejections (internal/process/task_queues.js:95:5)
    at Object.deployStack (/Users/tomasztrebski/dev/dotfiles/dependencies/nodenv/versions/14.17.4/lib/node_modules/aws-cdk/lib/api/deploy-stack.ts:294:26)
    at CdkToolkit.deploy (/Users/tomasztrebski/dev/dotfiles/dependencies/nodenv/versions/14.17.4/lib/node_modules/aws-cdk/lib/cdk-toolkit.ts:184:24)
    at initCommandLine (/Users/tomasztrebski/dev/dotfiles/dependencies/nodenv/versions/14.17.4/lib/node_modules/aws-cdk/bin/cdk.ts:213:9)
The stack named BugTestStack failed creation, it may need to be manually deleted from the AWS console: ROLLBACK_COMPLETE

Environment

  • CDK CLI Version : 1.118.0
  • Framework Version: ?
  • OS : MacOS BigSur
  • **Language (Version): Python 3.9.6 **

Other

I believe it makes sense to enable that for the sake of use cases where someone wishes to receive Slack or Email notification that one the lambda failed. Having only SQS as an option forces to write additional lambda to simply send an email.


This is 🐛 Bug Report

@kornicameister kornicameister added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Aug 26, 2021
@kornicameister kornicameister changed the title (aws_cdk.aws_lambda): short issue description (aws_cdk.aws_lambda): Unable to set SNS as dead letter queue for lambda Aug 26, 2021
@github-actions github-actions bot added @aws-cdk/aws-lambda Related to AWS Lambda @aws-cdk/aws-sns Related to Amazon Simple Notification Service labels Aug 26, 2021
@kornicameister
Copy link
Contributor Author

kornicameister commented Aug 26, 2021

FYI. That can be achieved with a little bit of AwsCustomResource like so:

dlq: sns.Topic = ...  # defined elsewhere

dlq.grant_publish(function)

set_dlq_action = custom.AwsSdkCall(
    service='Lambda',
    action='updateFunctionConfiguration',
    parameters={
        'FunctionName': function.function_name,
        'DeadLetterConfig': {
            'TargetArn': dlq.topic_arn,
        },
    },
    output_paths=['DeadLetterConfig.TargetArn'],
    physical_resource_id=custom.PhysicalResourceId.of(construct_id),
)

custom.AwsCustomResource(
    self,
    construct_id,
    on_create=set_dlq_action,
    on_update=set_dlq_action,
    on_delete=custom.AwsSdkCall(
        service='Lambda',
        action='updateFunctionConfiguration',
        parameters={
            'FunctionName': function.function_name,
            'DeadLetterConfig': {
                'TargetArn': None,
            },
        },
        output_paths=['DeadLetterConfig.TargetArn'],
    ),
    policy=custom.AwsCustomResourcePolicy.from_sdk_calls(
        resources=custom.AwsCustomResourcePolicy.ANY_RESOURCE,
    ),
)

@jumic
Copy link
Contributor

jumic commented Sep 2, 2021

Probably using escape hatches instead of custom resources is a better solution for the workaround until it is supported.

dlq = sns.Topic(self, 'dlq')
fun = fn.Function(
    self,
    'lambdaFn',
    code=fn.Code.from_inline('def handler(event, context): ...'),
    runtime=fn.Runtime.PYTHON_3_8,
    handler='index.handler',
)

# add dead letter config using escape hatches
cfn_function = fun.node.default_child
cfn_function.dead_letter_config = {
    "targetArn": dlq.topic_arn
}

As proposed by @kornicameister, the permission has to be granted (I didn't test this part).

dlq.grant_publish(fun)

@kornicameister
Copy link
Contributor Author

@jumic permission is needed, when I was dealing with custom resource, it failed on stacking up because of that. I think lambda detects whether is has permission to write to a topic/queue or not.

Anyway, thx for suggesting escape hatches! much much cleaner.
Will have to change that sometime soon.

@njlynch njlynch changed the title (aws_cdk.aws_lambda): Unable to set SNS as dead letter queue for lambda (lambda): Support SNS Topics as DLQs Sep 10, 2021
@njlynch njlynch added effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p1 and removed bug This issue is a bug. @aws-cdk/aws-sns Related to Amazon Simple Notification Service needs-triage This issue or PR still needs to be triaged. labels Sep 10, 2021
@njlynch njlynch removed their assignment Sep 10, 2021
@nija-at nija-at removed their assignment Sep 23, 2021
@nija-at nija-at added p2 effort/small Small work item – less than a day of effort and removed p1 effort/medium Medium work item – several days of effort labels Sep 27, 2021
kornicameister added a commit to kornicameister/aws-cdk that referenced this issue Jan 19, 2022
Adds possibility of using sns.Topic as a deadLetterQueue
for a lambda function as described in `AWS::Lambda::Function`
documentation.

closes: aws#16246
@mergify mergify bot closed this as completed in #18546 Feb 17, 2022
mergify bot pushed a commit that referenced this issue Feb 17, 2022
Adds possibility of using sns.Topic as a deadLetterQueue
for a lambda function as described in `AWS::Lambda::Function`
documentation.

closes: #16246

----

*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

⚠️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.

TikiTDO pushed a commit to TikiTDO/aws-cdk that referenced this issue Feb 21, 2022
Adds possibility of using sns.Topic as a deadLetterQueue
for a lambda function as described in `AWS::Lambda::Function`
documentation.

closes: aws#16246

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-lambda Related to AWS Lambda effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants