(sns): fifo topic construct doesn't append .fifo to the topic name

[skyrpex]

The SNS FIFO topics are failing to deploy because the topic name is missing .fifo at the end.

Reproduction Steps

Create a minimal cdk app with the following SNS FIFO topic and deploy it:

import * as sns from "@aws-cdk/aws-sns";
import * as cdk from "@aws-cdk/core";

class MyStack extends cdk.Stack {
  constructor(scope: cdk.Construct, id: string, properties?: cdk.StackProps) {
    super(scope, id, properties);

    new sns.Topic(this, "MyTopic", {
      fifo: true,
    });
  }
}

const app = new cdk.App();

new MyStack(app, "MyStack");

What did you expect to happen?

The SNS topic should append .fifo to the topic name if topic: true and .fifo is not present already.

What actually happened?

The deploy failed:

9:48:48 PM | CREATE_FAILED        | AWS::SNS::Topic                 | FanOutTopic
Invalid parameter: Topic Name (Service: AmazonSNS; Status Code: 400; Error Code: InvalidParameter; Request ID: xxxxx-xxxx-xxxx-xxxxx; Proxy: null)

Environment

• CDK CLI Version : v1.83.0
• Framework Version: v1.83.0
• Node.js Version: v12.20.0
• OS : Ubuntu 20
• Language (Version): TypeScript (4.1)

Other

It works if you manually set the topicName to include .fifo at the end, like this:

new sns.Topic(this, "MyTopic", {
    fifo: true,
    topicName: "MyTopic.fifo",
});

---

This is 🐛 Bug Report

[skyrpex]

I've made the minimal reproduction more complete.

Would love to contribute with a fix but I'm not sure how to proceed. I'm looking at the SQS implementation but I don't understand the flow of the cloud formation naming in relation to topic names, physical names, etc...

[NetaNir]

If you don't pass a name, does it work? Cause CloudFormation should add it if a name is not given.

[skyrpex]

@NetaNir deliverately passing an empty topicName as follows:

new sns.Topic(this, "MyTopic", {
      fifo: true,
      topicName: "",
});

Ends up with error too:

10:38:04 AM | CREATE_FAILED        | AWS::SNS::Topic    | MyTopic
Property TopicName cannot be empty if specified.

[NetaNir]

That's not an empty name. What happens if you don't pass a name at all?

[skyrpex]

If you mean not passing topicName at all, that's in the original reproduction steps.

I just removed the comment in the reproduction steps for clarification purposes.

[NetaNir]

Thanks @skyrpex.

@rrhodes I see you have added it in 7e60d8e. The integration tests expected output suggest that this should work. Any ideas?

[rrhodes]

Hi @NetaNir, the integration test explicitly passes in topic name fooTopic.fifo, so it does not cover the case presented in this issue.

Correct me if I'm wrong, but I don't think the CDK code holds any custom logic for auto-generating a topic name if it's not provided in the props? Certainly I didn't add anything specific to FIFO.

Is it possible this is a CloudFormation bug, if its default behavior is to generate a name and we're finding this to be invalid for FIFO topics?

[NetaNir]

So currently the API is broken, if a name is not passed- the queue creation will fail. And if a name is passed but does not have the .fifo suffix - the queue creation will fail.

This is not a great customer experience and we should make it better, the minimum is to throw an error if fifo is set to true and queueName is not passed or missing the .fifo. Build time errors are always better than deploy time error.

The better experience will be to append the fifo suffix if a name is passed and to throw if a name is not passed.

[rrhodes]

Agreed, that sounds like a good step forward until the underlying problem is resolved. Happy to open a PR for this. The underlying bug is already documented in the CloudFormation roadmap

[rrhodes]

Hi @NetaNir, @MrArnoldPalmer, that's a PR opened for review now. Feedback welcome.

[github-actions]

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