aws-codestarnotifications: missing dependency on target Topic's TopicPolicy

[HansFalkenberg-Visma]

Describe the bug

When deploying a stack with an SNS Topic and NotificationRule for a CodePipeline, it appears the NotificationRule will verify that it is able to send messages to the Topic and could fail if the TopicPolicy has not been deployed yet.

My TopicPolicy and NotifactionRule were being deployed simultaneously by CloudFormation and the NotificationRule deployment failed with error

Resource handler returned message: "Invalid request provided: AWS::CodeStarNotifications::NotificationRule"

This seems to be caused by addTarget in https://github.com/aws/aws-cdk/blob/main/packages/aws-cdk-lib/aws-codestarnotifications/lib/notification-rule.ts not adding a dependency to the Topic's policy.

Expected Behavior

NotificationRule is deployed successfully.

Current Behavior

A race condition caused the NotificationRule deployment to fail if the TopicPolicy happens to not have been deployed yet.

Reproduction Steps

Cannot be consistently reproduced.

Possible Solution

In addTarget, for each NotificationRule target that is a Topic that is created in the same stack, add a dependency on the topic's policy.

Workaround, explicitly add the dependency:

const topic = new Topic(...);
const rule = new NotificationRule(..., ..., {
  ...,
  targets: [topic]
});
// 'Policy': https://github.com/aws/aws-cdk/blob/v2.132.1/packages/aws-cdk-lib/aws-sns/lib/topic-base.ts#L135
rule.node.addDependency(topic.node.findChild('Policy'));

Additional Information/Context

No response

CDK CLI Version

2.132.1 (build 9df7dd3)

Framework Version

No response

Node.js Version

v20.10.0

OS

Windows 10 Version 22H2

Language

TypeScript

Language Version

No response

Other information

No response

[pahud]

Yes the notification rule would implicitly depend on the topic but not the topic policy. Under the hood the topic and topic policy would be provisioned in parallel and there would be a chance topic policy is not provisioned yet.

Yep, the workaround is the best solution. We could note this in the doc to help the community. Are you interested to submit a PR for that?

[github-actions]

This issue has not received a response in a while. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

[HansFalkenberg-Visma]

The given workaround might be the best solution for a workaround, but surely an actual solution would be to have CDK generate the dependency automatically?

I don't have the capacity to create a PR for this, sorry.

[pahud]

No worries. We welcome any PR to improve this.