-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
ESM: fix CreateESM SQS validation #12338
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
LocalStack Community integration with Pro 2 files ± 0 2 suites ±0 1h 31m 2s ⏱️ - 20m 22s Results for commit 39a7733. ± Comparison against base commit f48e6bd. This pull request removes 994 tests.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for coming up with this innovative workaround @bentsku 🧠
what a coincidenc that it worked in the first place 😅
# which is not given directly. We build out a dummy `QueueUrl` which can be parsed by SQS to return | ||
# the right value | ||
queue_name = arn["resource"].split("/")[-1] | ||
queue_url = f"http://sqs.{arn['region']}.domain/{arn['account']}/{queue_name}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting hack. I wonder how AWS does it internally because GetQueueUrl is not a required IAM permission: https://repost.aws/knowledge-center/lambda-sqs-event-source
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I really wonder too.. maybe just rebuilding the queue manually like us, but it seems… weird 😅 I don’t know if there would be any other operation that sqs:getQueueAttributes permission would give access to. Thanks for the review!
Motivation
Follow up from #12297, we did not pass the
QueueUrl
toGetQueueAttributes
but rather theQueueArn
. This actually still worked somehow because SQS took it as theQueueName
and was able to resolve it.However, when working with cross-account, this did not work because we were not able to resolve to the right account id.
I thought of doing an internal
connect_to().sqs.get_queue_url(QueueName=name, QueueOwnerAwsAccountId=queue_account)
, to get the properQueueUrl
, but if the queue did not exist, it would have raised without triggering an IAM check forGetQueueAttributes
, so the IAM parity would not be there... so I resolved to build a dummy value instead, as it is not very important and we only need SQS to be able to parse it.Upstream run: ✅ 13658265278
Changes
GetQueueAttributes
.