search

node-ts / bus

A typescript based enterprise service bus framework based on enterprise integration patterns
https://bus.node-ts.com/
MIT License
270 stars 26 forks source link

feat(bus-sqs): use aws-sdk v3, allow passing ARNs #199

Open shellscape opened 1 year ago

shellscape commented 1 year ago

This PR proposes the following:

This update has the advantage of producing smaller bundles, as v3 is tree-shakable, as well as automatic discovery of AWS environment and credentials data (a built-in with v3). This PR also makes this package much smaller as the latest v2 aws-sdk weighs in at a massive 77mb https://packagephobia.com/result?p=aws-sdk.

I had opened #198 to discuss, but as we're in immediate need on our end, I figured it wouldn't hurt to open a PR as well.

adenhertog commented 1 year ago

hi @shellscape - thanks for the PR! just letting you know I'm reviewing this now. The proposal's really smart and I'll merge this through if there are no issues

adenhertog commented 1 year ago

hey @shellscape the changes look great! I've left a couple of nit-picky comments, but the structure is all fine.

One small blocker is the sqs-transport.integration.ts is out of sync with these changes. If you're comfortable updating that then feel free, otherwise I'll wait to merge it back and update the tests there.

shellscape commented 1 year ago

hey @adenhertog thanks for taking a look. yes, integration tests didn't work out of the box on a fresh clone, and docs didn't seem to address the appropriate setup for getting them to run, so in the spirit of providing code for aws-sdk v3 to the community, we opened this PR as is.

since I opened this up we've come to find out that much of the mechanisms of the default sqs bus require blanket permissions in the running context to both SQS and SNS, and assume that those permissions are available (e.g. sns:CreateTopic and sqs:CreateQueue, among others). while useful in a RabbitMQ scenario (and others) this is counter-intuitive for AWS and not the case for most business environments, as these permissions are restricted to IAM roles that have deployment capabilities. as well, SNS isn't required to interact with and send/receive messages to a queue and is only really useful in fan-out scenarios.

as a result, we've mostly ripped out the SNS functionality and the automatic creation of queues. that ends up being quite the divergence from the transports in this repo. you can view our changes along those lines here if you're curious https://github.com/Moment-Wealth/bus/tree/%40superadvisor. given the assumptions about AWS that were made in the exisiting sqs bus and the large divergence of the code in our branch, we're not going to continue work on master so please feel free to close this PR or use it as a reference for adding v3 support.

adenhertog commented 1 year ago

hey @shellscape thanks for the debrief and for raising this MR to demonstrate the aws-sdk-v3 changes. I'll still get that part merged into master as it's certainly useful. I have had to reapply the changes in a separate branch as the repo has changed a bit in the meantime.

It's been a month since you commented, but if you still remember the context I'd be keen to learn a bit more about two of your points:

much of the mechanisms of the default sqs bus require blanket permissions in the running context to both SQS and SNS

To confirm, this is saying that the IAM policies needed to bootstrap the app is different to those needed to run the app (ie: creation/subscription of queues vs publishing & deleting messages). I do agree here and the current approach is more of a short-cut.

Would something like a db-migration style approach fit your use-case here? eg: the build server can have the bootstrap IAM policy and then run a cli-command to create/subscribe queues; meaning the runtime policy can be more restrictive.

SNS isn't required to interact with and send/receive messages to a queue and is only really useful in fan-out scenarios

This approach was mainly to decouple the sender from having knowledge of the receiver. For a more general library like this I can't see much benefit to dropping the SNS support, but I'm guessing you have a more optimised use case. Keen to learn what this is if you can share :)