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

feat: Add support for SQS events in Amazon.Lambda.Annotations #1758

Merged
merged 13 commits into from
Jun 17, 2024

Conversation

96malhar
Copy link
Contributor

@96malhar 96malhar commented Jun 6, 2024

Issue #, if available:
DOTNET-5526

Description of changes:
This PR adds support for SQS events in Amazon.Lambda.Annotations

  • Added new SQSEventAttribute to setup event source mapping between Lambda functions and SQS queues.
  • Refactored the source generator and added LambdaFunctionValidator class that performs all validation on the LambdaFunctionModel in a centralized location.
  • Added new DiagnosticDescriptors to indicate invalid usage of SQSEventAttribute
  • Updated CloudFormationWriter to parse the SQSEventAttribute and set up event source mapping in the serverless.template
  • Added new unit and integration tests.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@96malhar 96malhar requested a review from normj June 7, 2024 05:53
Copy link
Member

@normj normj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just have one small comment but I don't need to review it again.

Copy link
Member

@ashovlin ashovlin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a section to the README covering how to use the new attribute?

@96malhar
Copy link
Contributor Author

96malhar commented Jun 7, 2024

Could you add a section to the README covering how to use the new attribute?

Updated the Annotations README.

@ashovlin
Copy link
Member

Could code like this return a cleaner exception without the "This is a bug." language? Like we do for an invalid return type or function signature?

    [LambdaFunction]
    [SQSEvent("queueURL")]
    public async Task<SQSBatchResponse> SQSHandler(SQSEvent evnt, ILambdaContext context)
    {
        return new SQSBatchResponse();
    }
Severity	Code	Description	Project	File	Line	Suppression State	Details
Error	AWSLambda0001	This is a bug. Please run the build with detailed verbosity (dotnet build --verbosity detailed) and file a bug at https://github.com/aws/aws-lambda-dotnet with the build output and stack trace ARN is in incorrect format. ARN format is: arn:<partition>:<service>:<region>:<account-id>:<resource>   at Amazon.Lambda.Annotations.SourceGenerator.Writers.CloudFormationWriter.GetResourceNameFromArn(String arn)
   at Amazon.Lambda.Annotations.SourceGenerator.Writers.CloudFormationWriter.ProcessSqsAttribute(ILambdaFunctionSerializable lambdaFunction, SQSEventAttribute att)
   at Amazon.Lambda.Annotations.SourceGenerator.Writers.CloudFormationWriter.ProcessLambdaFunctionEventAttributes(ILambdaFunctionSerializable lambdaFunction)
   at Amazon.Lambda.Annotations.SourceGenerator.Writers.CloudFormationWriter.ApplyReport(AnnotationReport report)
   at Amazon.Lambda.Annotations.SourceGenerator.Generator.Execute(GeneratorExecutionContext context).	AnnotationSQS	C:\Users\shovlia\source\repos\AnnotationSQS\AnnotationSQS\AnnotationSQS.csproj	1	Active	

@ashovlin
Copy link
Member

I have this definition, using actual queue arns

    [LambdaFunction]
    [SQSEvent("arn:aws:sqs:us-east-1:<redacted>:shovlia-standard")]
    [SQSEvent("arn:aws:sqs:us-east-1:<redacted>:shovlia-standard-2", BatchSize = 100, Enabled = false)]
    public async Task<SQSBatchResponse> SQSHandler(SQSEvent evnt, ILambdaContext context)
    {
        return new SQSBatchResponse();
    }

It builds okay locally, but then the deployment fails with

Error getting status of change set: Failed to create CloudFormation change set: Transform AWS::Serverless-2016-10-31 failed with: Invalid Serverless Application Specification document. Number of errors found: 1. Resource with id [AnnotationSQSFunctionsSQSHandlerGeneratedshovlia-standard] is invalid. Logical ids must be alphanumeric.

@96malhar
Copy link
Contributor Author

Could code like this return a cleaner exception without the "This is a bug." language? Like we do for an invalid return type or function signature?

    [LambdaFunction]
    [SQSEvent("queueURL")]
    public async Task<SQSBatchResponse> SQSHandler(SQSEvent evnt, ILambdaContext context)
    {
        return new SQSBatchResponse();
    }
Severity	Code	Description	Project	File	Line	Suppression State	Details
Error	AWSLambda0001	This is a bug. Please run the build with detailed verbosity (dotnet build --verbosity detailed) and file a bug at https://github.com/aws/aws-lambda-dotnet with the build output and stack trace ARN is in incorrect format. ARN format is: arn:<partition>:<service>:<region>:<account-id>:<resource>   at Amazon.Lambda.Annotations.SourceGenerator.Writers.CloudFormationWriter.GetResourceNameFromArn(String arn)
   at Amazon.Lambda.Annotations.SourceGenerator.Writers.CloudFormationWriter.ProcessSqsAttribute(ILambdaFunctionSerializable lambdaFunction, SQSEventAttribute att)
   at Amazon.Lambda.Annotations.SourceGenerator.Writers.CloudFormationWriter.ProcessLambdaFunctionEventAttributes(ILambdaFunctionSerializable lambdaFunction)
   at Amazon.Lambda.Annotations.SourceGenerator.Writers.CloudFormationWriter.ApplyReport(AnnotationReport report)
   at Amazon.Lambda.Annotations.SourceGenerator.Generator.Execute(GeneratorExecutionContext context).	AnnotationSQS	C:\Users\shovlia\source\repos\AnnotationSQS\AnnotationSQS\AnnotationSQS.csproj	1	Active	

Added a check to verify that the Queue property has a valid ARN structure and returns a clear diagnostic error when the validation fails.

@96malhar
Copy link
Contributor Author

I have this definition, using actual queue arns

    [LambdaFunction]
    [SQSEvent("arn:aws:sqs:us-east-1:<redacted>:shovlia-standard")]
    [SQSEvent("arn:aws:sqs:us-east-1:<redacted>:shovlia-standard-2", BatchSize = 100, Enabled = false)]
    public async Task<SQSBatchResponse> SQSHandler(SQSEvent evnt, ILambdaContext context)
    {
        return new SQSBatchResponse();
    }

It builds okay locally, but then the deployment fails with

Error getting status of change set: Failed to create CloudFormation change set: Transform AWS::Serverless-2016-10-31 failed with: Invalid Serverless Application Specification document. Number of errors found: 1. Resource with id [AnnotationSQSFunctionsSQSHandlerGeneratedshovlia-standard] is invalid. Logical ids must be alphanumeric.

I have added a new ResourceName property in the SQSEvent attribute. It sets the CloudFormation resource name for the SQS event source mapping. By default this is set to the SQS queue name if Queue is set to an SQS queue ARN. It will sanitize the SQS queue name by stripping out non-alphanumeric characters. If Queue is set to an existing CloudFormation resource, than that is used as the default value without the "@" prefix.

Also added new unit tests that exercises this logic.

@ashovlin
Copy link
Member

Could we throw a validation error in advance if invalid settings are applied to a fifo queue?

6/12/2024 11:15:45 AM	AWS::Lambda::EventSourceMapping	MySQSLambdaFunctionshovliafifo			Resource handler returned message: "Invalid request provided: Batching window is not supported for FIFO queues (Service: Lambda, Status Code: 400, Request ID: 5df30f34-e266-44c9-b4a1-061da42df499)" (RequestToken: 539661a7-ca6f-b190-ad75-5bd099921c77, HandlerErrorCode: InvalidRequest)
[SQSEvent("arn:aws:sqs:us-east-1:<redacted>:shovlia.fifo", BatchSize = 100, MaximumBatchingWindowInSeconds = 5)]
public void SQSHandler(SQSEvent evnt, ILambdaContext context)

@96malhar
Copy link
Contributor Author

Could we throw a validation error in advance if invalid settings are applied to a fifo queue?

6/12/2024 11:15:45 AM	AWS::Lambda::EventSourceMapping	MySQSLambdaFunctionshovliafifo			Resource handler returned message: "Invalid request provided: Batching window is not supported for FIFO queues (Service: Lambda, Status Code: 400, Request ID: 5df30f34-e266-44c9-b4a1-061da42df499)" (RequestToken: 539661a7-ca6f-b190-ad75-5bd099921c77, HandlerErrorCode: InvalidRequest)
[SQSEvent("arn:aws:sqs:us-east-1:<redacted>:shovlia.fifo", BatchSize = 100, MaximumBatchingWindowInSeconds = 5)]
public void SQSHandler(SQSEvent evnt, ILambdaContext context)

Added the necessary checks for FIFO specific validations. Discussed offline and concluded that we will only perform these checks when the Queue property in the SQSEvent attribute points to an ARN that ends in .fifo.

1. Add ability to reference the Queue property from a CloudFormation parameter
2. Correctly sync event properties while preserving properties that were manually set.
3. Fix typo in error message.
@96malhar 96malhar requested review from normj and ashovlin June 14, 2024 21:52
@96malhar 96malhar merged commit f2c3c07 into dev Jun 17, 2024
6 checks passed
@96malhar 96malhar deleted the kmalhar/sqs-events branch June 17, 2024 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants