-
Notifications
You must be signed in to change notification settings - Fork 62
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
EB SQS Docker IP check overhaul to correct 403 issue (use remote IP + remote addr) #156
Conversation
0679cc4
to
9dd7708
Compare
I'm not sure if the unreleased changes to the old code would have worked/changed anything, but this seems to solve the issue in reality on Al2023. |
@@ -96,23 +96,13 @@ def in_docker_container_with_cgroup2? | |||
File.exist?('/proc/self/mountinfo') && File.read('/proc/self/mountinfo') =~ %r{/docker/containers/} | |||
end | |||
|
|||
def default_gw_ips | |||
default_gw_ips = ['172.17.0.1'] |
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.
This just seems to resolve in giving me two entries of 172.17.0.1
on the AL2023 test, which seems legit. Maybe the issue is the non-remote ip/addr usage? IIRC when I ran this, the debug logger had the IP as something else. I can add more debug statements to prove what's going on (this is very weird).
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.
D, [2024-11-05T17:13:44.713584 #16] DEBUG -- : aws-sdk-rails middleware detected call from Elastic Beanstalk SQS Daemon.
D, [2024-11-05T17:13:44.713881 #16] DEBUG -- : SQSD request detected from IP: 127.0.0.1.
D, [2024-11-05T17:13:44.713926 #16] DEBUG -- : SQSD request detected from Remote IP: 127.0.0.1.
Maybe it needs to be remote_addr 🤯 . Trying again
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.
if this is the root cause—checking request.ip
does not work—then I wonder if using the OG code is fine, it just needs to check against remote_ip and remote_addr instead. I assume it would since it seemed like it was calculating properly.
I'll make those changes once I confirm it really must be remote_addr
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.
D, [2024-11-05T17:41:38.678283 #16] DEBUG -- : SQSD request detected from Remote ADDR: 172.17.0.1.
HA! There it is.
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.
Updated code!
Thanks. I'll take a look. Sorry for the delays. I'm trying to set up an elastic beanstalk rails website but the step by step guide that is published doesn't produce something that works. |
f173271
to
d184469
Compare
describe EbsSqsActiveJobMiddleware do | ||
# Simple mock Rack app that always returns 200 | ||
let(:mock_rack_app) { ->(_) { [200, { 'Content-Type' => 'text/plain' }, ['OK']] } } | ||
describe Aws::Rails::EbsSqsActiveJobMiddleware do |
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.
massively re-did the test suite, let me know if I missed any cases 🤯 or it could be simplified
d184469
to
5f6a4d3
Compare
@mullermp no worries, I just figured out the actual root cause and pushed up proper fixing. What you'd really need is both a ruby-rails EB stack and a Docker EB stack that happens to run rails 🤯 |
@mullermp I haven't tested this round of updates in reality, but it was the remote_addr lack of checking that seemed to be the core issue, based on debugging |
I've finally managed to get this setup and tested - looks like the change is working! Thank you for all the debugging work on this! |
Issue #, if available:
#154
Description of changes:
Taken code from https://github.com/active-elastic-job/active-elastic-job/blob/master/lib/active_elastic_job/rack/sqs_message_consumer.rb to resolve issue—seems to be the old IP check in this gem doesn't work with AL2023?Issue was that we needed to check
remote_addr