agiledigital/serverless-sns-sqs-lambda

feat: Ensure name uniqueness

Closed this issue · 3 comments

Background

Recently stumbled upon a two cascading issue regarding the name config:

  1. SQS name too long
  2. Non unique names will cause one of the lambda subscribing to the wrong queue

The most sensible way to make a name unique in a project is to use the function name itself as the event name, but because the physical ID of the queue is using the format of ${service}-${stage}-${function}${snsSqs.name}Queue, it essentially duplicated the function name twice. e.g. if the function is called MyFunc and I use snsSqs.name: MyFunc to make it project-unique, the resulting queue name will be myapp-dev-MyFuncMyFuncQueue.

For the sake of clarity, one of our resulting queue name exceeded the 80 character limit looks like this:

"shopify-api-000000-FnShopCustomerCheckoutProcessFnShopCustomerCheckoutProcessQueue.fifo"

I tried avoiding it by using the same name name: Event1 in different functions, but that led to problem 2 above. For example,

functions:
  Fn1:
    events:
      - snsSqs:
          name: Event1
  Fn2:
    events:
      - snsSqs:
          name: Event1

Fn1 and Fn2 may randomly subscribe to the same SQS queue generated from this plugin, this usually happens during a stack update action but the result is random and it's based on how CloudFormation's internal priorities to choose which resources to work on.

Proposed Solutions

  1. Prepend the queue logical ID with function name

    Because problem 2 above should be caused by having multiple CloudFormation stack resources having the same logical ID, prepending with the function name should make it intrinsically unique at function level.

  2. Add a global uniqueness check during before:deploy:deploy.

    This blocks unintentionally subscribed even if function prefixes are not suffice. e.g. Foo + BarEvent and FooBar + Event

  3. Do something when queue names are exceeding the 80 character limit during before:deploy:deploy, either

    1. throw, or
    2. add a new option (e.g. HashName: true for SHA1) to make the queue name a constant width

Thanks for the detailed issue write up @vicary

I'm surprised that there are duplicated CloudFormation logical IDs, I would have thought that CloudFormation would have just thrown an error if it detected duplicate logical IDs before it even started updating the Stack. Ensuring that the logical IDs are unique is definitely something we should fix ASAP.

I'm leaning towards just having the plugin throw an error if the function names are too long. I wonder if we can just omit physical names and get AWS to generate them from the logical IDs. That has worked for me in the past but maybe that doesn't work with every situation.

We will also have to be careful not to break existing stacks, we might need to have another major version bump.

I'll try and have a look this week.
Do you have any thoughts @robinMcA or @dbalmain ?

For later reference: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/resources-section-structure.html#resources-section-structure-resource-fields

@NoxHarmonium I have a gut feeling that the colliding logical IDs maybe simply overridden because resources.Resources is an object in serverless.yml. During queue generation, the plugin may simply throw when the target key (i.e. Logical ID) already exist.

// For Fn1
Resources.Event1 = {
  Type: "AWS::SQS::Queue",
  QueueName: "myapp-dev-Fn1Event1Queue",
  // ...
};

// For Fn2, it may throw because `Resources.Event1` already exists.
Resources.Event1 = {
  Type: "AWS::SQS::Queue",
  QueueName: "myapp-dev-Fn2Event1Queue",
  // ...
};

Hi @vicary, sorry for the delay.
I've opened up a PR #552
Let me know what you think.