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

Added Support to Amazon.Lambda.Annotations For Sqs (SQSEvent.SQSMessage) #1203

Open
wants to merge 154 commits into
base: master
Choose a base branch
from

Conversation

hounddog22030
Copy link
Contributor

@hounddog22030 hounddog22030 commented Jun 14, 2022

Description of changes:

Added support to add Event properties and values for a new attribute (SqsMessageAttribute) to template file in a similar manner to RestApiAttribute and HttpApiAttribute.

Added attribute SqsMessageAttribute that supports all AWS::Serverless::Function Event properties of type 'SQS' and all AWS::SQS::Queue properties .

Added functionality to write the AWS::SQS::Queue to the template using values from the attributes.

Added a sample class (Messaging.cs) that demonstrates using the SqsMessageAttribute in its two use cases:

  • Wires up a preexisting or manually defined AWS::SQS::Queue to the Events property of AWS::Serverless::Function.
  • Adds a AWS::SQS::Queue and wires it up to the Events property AWS::Serverless::Function.

Added a property to AWS::Serverless::Function metadata in the template named SyncedResources that works in very similar vain to SyncedEvents.

Added tests cases following same patterns as HttpApiAttribute and RestApiAttribute in class CloudFormationJsonWriterTests.

Added test cases to check the bounds of all the properties in SqsMessageAttribute.

This is a large change and there are specific places where I had a hard time following the patterns of RestApiAttribute and HttpApiAttribute. Otherwise, I tried to followed those patterns very closely. Those three places are:

  • [assembly: InternalsVisibleTo] in SqsMessageAttribute.cs - there is a TODO that documents this.
  • field _processedResources in CloudFormationJsonWriter.cs - this is probably not a big deal, but let me know.
    * I added a <VersionPrefix> for our build process in order to be able to push to a nuget repo with a unique version number. (I figured out how to get around this, it has been removed).

I made several attempts to go thru and remove any formatting issues, but a few small ones remain. So, hopefully the review will be straightforward.

I have updated the readme.md with description of the attribute and provided two examples. All public properties are documented in ISqsMessage with <summary>.

This was originally created off of the annotations branch, but master was merged in.

The commit comments will probably be nonsensical as I do a lot of iterating (over 100 commits).

Thanks for reviewing.

This code will produce a template similar to:

{
  "AWSTemplateFormatVersion": "2010-09-09",
  "Transform": "AWS::Serverless-2016-10-31",
  "Resources": {
    "TestServerlessAppMessagingMessageHandlerForPreExistingQueueGenerated": {
      "Type": "AWS::Serverless::Function",
      "Metadata": {
        "Tool": "Amazon.Lambda.Annotations",
        "SyncedEvents": [
          "TestServerlessAppMessagingMessageHandlerForPreExistingQueueGeneratedappdeployblueLAVETRYB3JKXSomeQueueName"
        ]
      },
      "Properties": {
        "Runtime": "dotnet6",
        "CodeUri": ".",
        "MemorySize": 256,
        "Timeout": 30,
        "Policies": [
          "AWSLambdaBasicExecutionRole"
        ],
        "PackageType": "Zip",
        "Handler": "TestProject::TestServerlessApp.Messaging_MessageHandlerForPreExistingQueue_Generated::MessageHandlerForPreExistingQueue",
        "Events": {
          "TestServerlessAppMessagingMessageHandlerForPreExistingQueueGeneratedappdeployblueLAVETRYB3JKXSomeQueueName": {
            "Type": "SQS",
            "Properties": {
              "BatchSize": 11,
              "Queue": "arn:aws:sqs:us-east-1:968993296699:app-deploy-blue-LAVETRYB3JKX-SomeQueueName"
            }
          }
        }
      }
    },
    "TestServerlessAppMessagingMessageHandlerForNewQueueGenerated": {
      "Type": "AWS::Serverless::Function",
      "Metadata": {
        "Tool": "Amazon.Lambda.Annotations",
        "SyncedEvents": [
          "TestServerlessAppMessagingMessageHandlerForNewQueueGeneratedQueueForMessageHandlerForNewQueue"
        ],
        "SyncedResources": [
          "QueueForMessageHandlerForNewQueue"
        ]
      },
      "Properties": {
        "Runtime": "dotnet6",
        "CodeUri": ".",
        "MemorySize": 256,
        "Timeout": 30,
        "Policies": [
          "AWSLambdaBasicExecutionRole"
        ],
        "PackageType": "Zip",
        "Handler": "TestProject::TestServerlessApp.Messaging_MessageHandlerForNewQueue_Generated::MessageHandlerForNewQueue",
        "Events": {
          "TestServerlessAppMessagingMessageHandlerForNewQueueGeneratedQueueForMessageHandlerForNewQueue": {
            "Type": "SQS",
            "Properties": {
              "BatchSize": 12,
              "Queue": {
                "Fn::GetAtt": [
                  "QueueForMessageHandlerForNewQueue",
                  "Arn"
                ]
              },
              "FilterCriteria": {
                "Filters": [
                  {
                    "Pattern": "Filter1"
                  },
                  {
                    "Pattern": "Filter2"
                  }
                ]
              },
              "MaximumBatchingWindowInSeconds": 31
            }
          }
        }
      }
    },
    "QueueForMessageHandlerForNewQueue": {
      "Type": "AWS::SQS::Queue",
      "Metadata": {
        "Tool": "Amazon.Lambda.Annotations"
      },
      "Properties": {
        "ContentBasedDeduplication": true,
        "DeduplicationScope": "queue",
        "DelaySeconds": 5,
        "FifoQueue": true,
        "FifoThroughputLimit": "perQueue",
        "KmsDataKeyReusePeriodSeconds": 299,
        "KmsMasterKeyId": "alias/aws/sqs",
        "MaximumMessageSize": 1024,
        "MessageRetentionPeriod": 60,
        "QueueName": "thisismyqueuename.fifo",
        "ReceiveMessageWaitTimeSeconds": 5,
        "RedriveAllowPolicy": {
          "redrivePermission": "denyAll"
        },
        "RedrivePolicy": {
          "deadLetterTargetArn": "arn:somewhere",
          "maxReceiveCount": 5
        },
        "Tags": [
          {
            "Key": "keyname1",
            "Value": "value1"
          },
          {
            "Key": "keyname2",
            "Value": "value2"
          }
        ],
        "VisibilityTimeout": 100
      }
    }
  }
}

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

@normj
Copy link
Member

normj commented Jul 27, 2022

Hi @hounddog22030, sorry we haven't gotten back to you sooner. We have been busy with other projects and are now refocusing on this project.

I like what you are doing with syncing the CF template, although I still need dig in more. Do you have any thoughts about simplifying the parameters into the Lambda function? Or handling the return SQSBatchResponse? I wonder if the library should take care of the boiler plate of handling which messages were successfully processed.

@hounddog22030
Copy link
Contributor Author

Do you have any thoughts about simplifying the parameters into the Lambda function?

I can take a look @ that.

Or handling the return SQSBatchResponse? <

I haven't thought of that. Honestly, I've never even used that. What are your thoughts?

I wonder if the library should take care of the boiler plate of handling which messages were successfully processed. <

Please elaborate a bit more on that.

Thanks.

@hounddog22030
Copy link
Contributor Author

hounddog22030 commented Jul 27, 2022

Do you have any thoughts about simplifying the parameters into the Lambda function?

It has been a while since I worked on this and just took a look @ it. I thought you were asking about all of the properties on SqsMessageAttribute?

Looking @ that, I don't know how to simplify that; it's literally everything from AWS::SQS:Queue.

For those, there are two examples, one for an existing queue and one to create a new queue. The existing queue example is pretty clean:

[SqsMessage(EventQueueARN = "arn:aws:sqs:us-east-1:968993296699:app-deploy-blue-LAVETRYB3JKX-SomeQueueName", EventBatchSize = 11)]

Other than that, the one that creates a new queue, almost everything is optional with defaults.

For example:

[SqsMessage(FifoQueue = true)]

Is that what you were asking about to simplify or were you asking about something entirely different?

@hounddog22030
Copy link
Contributor Author

@normj - any thoughts on the above?

@normj
Copy link
Member

normj commented Aug 4, 2022

@hounddog22030 I'm on vacation right now, but don't want you to think I'm ignoring your question. I'll catch back up on the PR when I'm back from vacation.

@hounddog22030
Copy link
Contributor Author

@hounddog22030 I'm on vacation right now, but don't want you to think I'm ignoring your question. I'll catch back up on the PR when I'm back from vacation.

np. enjoy your well deserved vacay

@hounddog22030
Copy link
Contributor Author

@normj - any chance of looking @ this soon? thanks

@ashishdhingra ashishdhingra added the feature-request A feature should be added or improved. label Oct 11, 2022
@ashishdhingra ashishdhingra added p2 This is a standard priority issue queued annotations labels Sep 1, 2023
@danieliu456
Copy link

@hounddog22030 You are the best ! I really appreciate your hard work with this feature! Why this is taking so long to merge or at least provide constructive feedback? @normj.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
annotations feature-request A feature should be added or improved. p2 This is a standard priority issue queued
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants