getlift / lift

Expanding Serverless Framework beyond functions using the AWS CDK
MIT License
913 stars 113 forks source link

Feat/queue custom name #235

Closed neovasili closed 2 years ago

neovasili commented 2 years ago

This PR is a proposal to add support for #121 feature request just adding a name property to the construct configuration like follows:

constructs:
  messages-1:
    type: queue
    encryption: 'kmsManaged'
    name: queue-with-messages-type-1-${sls:stage}
    worker:
      timeout: 30
      name: lambda-for-handling-messages-1-${sls:stage}
...
  messages-2:
    type: queue
    encryption: 'kmsManaged'
    name: queue-with-messages-type-2-${sls:stage}
    worker:
      timeout: 30
      name: lambda-for-handling-messages-2-${sls:stage}
...

As stated in the related issue, in some use cases is useful to have the ability to set the queue name for compliance or usability reasons. I've already made this change using a local copy of the plugin for internal purposes so I'm improving and sharing it.

The PR also includes a queue name validation according to AWS API specs wether the queue custom name is specified or not.

Resolves #121

mnapoli commented 2 years ago

How about using extensions to set the queue name? https://github.com/getlift/lift/blob/master/docs/queue.md#extensions

That avoids adding another option, and so far it seems like an advanced use case as the original issue doesn't seem to get an overwhelming number of upvotes or comments.

neovasili commented 2 years ago

How about using extensions to set the queue name? https://github.com/getlift/lift/blob/master/docs/queue.md#extensions

That avoids adding another option, and so far it seems like an advanced use case as the original issue doesn't seem to get an overwhelming number of upvotes or comments.

Thanks for your response @mnapoli

However, looks like the extensions is not considering the DLQ queue name, very likely (didn't dig into it) because queue name is managed differently than extensions properties in the plugin and the nature of the queue construct that adds a DLQ by default (which is fine):

image

As I already mentioned, this is something I had already implemented, and that's why I shared it

Tested with version 1.19.0.

t-richard commented 2 years ago

AFAICT you can also override the dlq name by doing something like this

constructs:
  test:
    type: queue
    ...
    extensions:
      dlq:
        Properties:
          QueueName: my-awesome-queue-dlq
neovasili commented 2 years ago

Awesome! didn't know it thanks for your clarification

Closing this one then 😄