Skip to content

QueueUrls plugin incorrectly inferring region from custom URL #2696

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

Closed
paul110 opened this issue Apr 26, 2022 · 6 comments · Fixed by #2697
Closed

QueueUrls plugin incorrectly inferring region from custom URL #2696

paul110 opened this issue Apr 26, 2022 · 6 comments · Fixed by #2697
Labels
bug This issue is a bug. needs-triage This issue or PR still needs to be triaged.

Comments

@paul110
Copy link

paul110 commented Apr 26, 2022

Describe the bug

We have a custom localstack endpoint for our SQS service "localstack-sqs" and the queue names look like this: https://localstack-sqs.example.dev/queue/example_queue_name. When the QueueUrls plugin runs update_region on that URL, it incorrectly ends up overriding the region to example.

Expected Behavior

I think the expected behaviour should be that if sqs isn't a clear delimiter within the URL and is just part of the subdomain (localstack-sqs in this example), then it shouldn't assume that the region is right after sqs.. It should instead skip updating the region altogether.

Current Behavior

We discovered this because

sqs_client.get_queue_attributes(queue_url: "https://localstack-sqs.example.dev/queue/example_queue_name", attribute_names: %w[QueueArn])

was coming back with

Aws::SQS::Errors::Http500Error: Aws::SQS::Errors::Http500Error
from /Users/paulpintilie/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/aws-sdk-core-3.130.0/lib/seahorse/client/plugins/raise_response_errors.rb:17:in `call'

even though sqs_client.list_queues is successfully listing the queues correctly.

It turns out that when queue_url is specified, the QueueUrls::Handler#update_region method will override the region with the section of the URL that comes immediately after sqs. which in our case wasn't a region, but instead was the domain.

Reproduction Steps

require 'aws-sdk-sqs'

sqs_client = Aws::SQS::Client.new
puts "Client region before request: #{sqs_client.config.region} "

begin
  sqs_client.get_queue_attributes(
    queue_url: "https://localstack-sqs.example.dev/queue/example_queue_name",
    attribute_names: %w[QueueArn]
  )
rescue => e
  puts "Request error context region : #{e.context.config.region}"
end

Prints the following:

Client region before request: eu-west-1
Request error context region : example

Possible Solution

Maybe we can ensure that we only assume the region is after sqs if sqs is either the 1st/2nd component of the URL (vpce-x-y.sqs.us-east-1.vpce.amazonaws.com or sqs.us-east-1.vpce.amazonaws.com)

Happy to open a PR for it if this sounds like a good idea and am also open to other ways of addressing it.

Additional Information/Context

Related PRs:

Related issues:

Gem name ('aws-sdk', 'aws-sdk-resources' or service gems like 'aws-sdk-s3') and its version

aws-sdk-sqs

Environment details (Version of Ruby, OS environment)

ruby 2.7.4

@paul110 paul110 added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Apr 26, 2022
@mullermp
Copy link
Contributor

Thanks for opening an issue. I've looked at this code recently and can agree that it's fragile. I think a workaround might be to create an SQS client using your custom :endpoint option (and not pass queue url). This assumes that you are not calling any other operations with that same client. Are you able to use that for now?

@mullermp
Copy link
Contributor

mullermp commented Apr 26, 2022

We currently parse the region like this:

          # take the first component after service delimiter
          # https://sqs.us-east-1.amazonaws.com/1234567890/demo
          # https://vpce-x-y.sqs.us-east-1.vpce.amazonaws.com/1234567890/demo
          def parse_region(url)
            parts = url.split('sqs.')
            parts[1].split('.').first if parts.size > 1
          end

Another workaround might be to rename your queue url to avoid this limitation - perhaps removing "sqs" from "localstack-sqs"? or rename to "sqs-localstack"

@paul110
Copy link
Author

paul110 commented Apr 28, 2022

Hi @mullermp. Thanks for the quick reply! I think if possible we would prefer not to have to change the name of the local service localstack-sqs as that would require for us to change quite a few other things along with it.

Regarding your first workaround suggestion about setting an :endpoint option and not pass the queue url, we are already using a custom :endpoint for our client.

[16] > sqs_client.config.endpoint.to_s
=> "https://localstack-sqs.example.dev"

I am not 100% sure what you mean by "not passing the queue url" though. The :queue_url attribute seems to be required for the get_queue_attributes method.

@github-actions
Copy link

github-actions bot commented May 2, 2022

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

@mullermp
Copy link
Contributor

mullermp commented May 2, 2022

I've pushed a fix to handle this. See #2697.

@paul110
Copy link
Author

paul110 commented May 3, 2022

@mullermp thank you! much appreciated 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. needs-triage This issue or PR still needs to be triaged.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants