Add patch to aws-sdk-core to fix auth bug #432

NickLaMuro · 2018-04-17T01:13:54Z

There is a bug with the aws-sdk that prevents the API from working properly when using a proxy, and the auth for the proxy includes a special character (like a question mark: ?). This is described here:

aws/aws-sdk-ruby#1760

Since the release of the fix in aws-sdk is unknown, this currently monkey patches the fix to MIQ to allow it to work without it.

TODO

Add tests to exercise this patch, and the future fix. Unsure how to do this at the moment.

Links

Pull-request for the patch in lib: Fix to support URI encoded characters in http_proxy aws/aws-sdk-ruby#1760
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1567418

QA Steps

Add a proxy for :ec2 in the advanced config. Make sure either the :user: and/or :password: has a special character in it (? works for example):

:http_proxy:
  :ec2:
    :host: <<PROXY_HOST>>
    :port: <<PROXY_PORT>>
    :user: <<PROXY_USERNAME>>
    :password: <<PROXY_PASSWORD>>

Create a new Amazon provider, and verify that the credentials work for it.

To test in the UI, this requires ManageIQ/manageiq#17218 and ManageIQ/manageiq-ui-classic#3693 as well.

There is a bug with the aws-sdk that prevents the API from working properly when using a proxy, and the auth for the proxy includes a special character (like a question mark: '?'). This is described here: aws/aws-sdk-ruby#1760 Since the release of the fix is unknown, this currently monkey patches the fix to MIQ to allow it to work without it.

miq-bot · 2018-04-17T01:27:22Z

Checked commit NickLaMuro@14c78d7 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
3 files checked, 5 offenses detected

lib/patches/aws-sdk-core/seahorse_client_net_http_pool_patch.rb

⚠️ - Line 2, Col 28 - Lint/Void - Variable ConnectionPool used in void context.
❗ - Line 17, Col 42 - Style/ColonMethodCall - Do not use :: for method calls.
❗ - Line 18, Col 46 - Style/ColonMethodCall - Do not use :: for method calls.
❗ - Line 8, Col 27 - Style/MethodDefParentheses - Use def with parentheses when there are parameters.
❗ - Line 9, Col 1 - Layout/EmptyLinesAroundMethodBody - Extra empty line detected at method body beginning.

jrafanie · 2018-04-17T13:52:00Z

lib/patches/aws-sdk-core/seahorse_client_net_http_pool_patch.rb

@@ -0,0 +1,44 @@
+# Autoload the connection pool


I believe you mentioned this yesterday but will put it here anyway. We probably want to gate this patch for the versions that are affected and when it's fixed so we can eventually remove it.

Yeah, wanted to do this via tests, but I guess I can do it in the file as well. Is that what you were thinking?

Yeah, we can do this when the gem is released if we remember

Yeah, @blomquisg just #yolomerged before I even got a chance to make a change...

¯\_(ツ)_/¯

jrafanie · 2018-04-17T13:52:57Z

lib/patches/aws-sdk-core/seahorse_client_net_http_pool_patch.rb

+          args << http_proxy.host
+          args << http_proxy.port
+          args << (http_proxy.user && CGI::unescape(http_proxy.user))
+          args << (http_proxy.password && CGI::unescape(http_proxy.password))


I think this makes sense, monkey patch the single method instead of bringing along 2 method changes from the PR.

jrafanie · 2018-04-17T14:51:52Z

@miq-bot add_label gaprindashvili/yes

…-proxy-auth-fix Add patch to aws-sdk-core to fix auth bug (cherry picked from commit 2b2c87f) Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1568467

simaishi · 2018-04-17T15:00:05Z

Gaprindashvili backport details:

$ git log -1
commit 6b4566272944a1db4bdfe1af7919a5d9f36dd84f
Author: Greg Blomquist <[email protected]>
Date:   Tue Apr 17 16:34:25 2018 +0200

    Merge pull request #432 from NickLaMuro/monkey-patch-aws-sdk-core-for-proxy-auth-fix
    
    Add patch to aws-sdk-core to fix auth bug
    (cherry picked from commit 2b2c87f577ec429738404a0588c3ad1d8e0ed68c)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1568467

jrafanie · 2018-04-17T15:08:34Z

I did a little research into which aws versions are "fixed"

We'll need to backport to any branches that rely on aws-sdk < v1.39.0 or > 1.66.0

11:07:21 ~/Code/aws-sdk-ruby (master) (2.4.4) + git tag --contains 640297066fff98d248ba957e85858b921e25f1e1
v1.39.0
v1.40.0
v1.40.1
v1.40.2
v1.40.3
v1.41.0
v1.42.0
v1.43.0
v1.43.1
v1.43.2
v1.43.3
v1.44.0
v1.45.0
v1.46.0
v1.47.0
v1.48.0
v1.48.1
v1.49.0
v1.50.0
v1.51.0
v1.52.0
v1.53.0
v1.54.0
v1.55.0
v1.56.0
v1.57.0
v1.58.0
v1.59.0
v1.59.1
v1.60.0
v1.60.1
v1.60.2
v1.61.0
v1.62.0
v1.63.0
v1.64.0
v1.65.0
v1.66.0

NickLaMuro · 2018-04-17T15:11:21Z

@jrafanie Nice work! Though, that does assume that this code is exactly the same in those areas, so we might want to do a check on a case by case basis before we just blindly backport.

This patch really does require checking that the original codebase for this is exactly the same.

NickLaMuro · 2018-04-17T15:15:10Z

I will make a follow up PR to put in a raise statement if there is a version mismatch, which will require a follow up PR to backport this patch to fix the tests from breaking entirely as a safety measure. I think that will be a bit easier than me trying to get some tests working to exercise the code that isn't even a part of this repo.

jrafanie · 2018-04-17T15:17:42Z

We look to need this on euwe, fine, and gaprindashvili although to @NickLaMuro's point, the monkey patch might be different for the various versions of aws-sdk. I'll let @blomquisg decide what he wants to do with this information.

11:14:37 ~/Code/manageiq (euwe) (2.4.4) + git grep aws-sdk
Gemfile:gem "aws-sdk",                        "~>2.2.19",      :require => false

11:13:02 ~/Code/manageiq (fine) (2.4.4) + git grep aws-sdk
Gemfile:gem "aws-sdk",                        "~>2",           :require => false

11:13:39 ~/Code/manageiq-providers-amazon (gaprindashvili) (2.4.4) + git grep aws-sdk
manageiq-providers-amazon.gemspec:  s.add_dependency("aws-sdk", ["~>2.9.7"])

NickLaMuro · 2018-04-17T15:29:21Z

@jrafanie @blomquisg Added said PR: #433

…aws-sdk-core-for-proxy-auth-fix" This reverts commit 2b2c87f, reversing changes made to beed01f.

ManageIQ deleted a comment from miq-bot Apr 17, 2018

jrafanie reviewed Apr 17, 2018

View reviewed changes

jrafanie approved these changes Apr 17, 2018

View reviewed changes

blomquisg self-requested a review April 17, 2018 14:30

blomquisg approved these changes Apr 17, 2018

View reviewed changes

blomquisg merged commit 2b2c87f into ManageIQ:master Apr 17, 2018

miq-bot added the gaprindashvili/yes label Apr 17, 2018

simaishi added the blocker label Apr 17, 2018

simaishi added gaprindashvili/backported and removed gaprindashvili/yes labels Apr 17, 2018

NickLaMuro mentioned this pull request Apr 17, 2018

Add aws-sdk-version check to patch #433

Closed

NickLaMuro mentioned this pull request Apr 18, 2018

Add specs to confirm aws-sdk-core monkey patch is still valid #434

Merged

NickLaMuro mentioned this pull request Oct 11, 2018

update aws-sdk gem to v3 #488

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add patch to aws-sdk-core to fix auth bug #432

Add patch to aws-sdk-core to fix auth bug #432

NickLaMuro commented Apr 17, 2018

miq-bot commented Apr 17, 2018

jrafanie Apr 17, 2018

NickLaMuro Apr 17, 2018

jrafanie Apr 17, 2018

NickLaMuro Apr 17, 2018

jrafanie Apr 17, 2018

jrafanie commented Apr 17, 2018

simaishi commented Apr 17, 2018

jrafanie commented Apr 17, 2018

NickLaMuro commented Apr 17, 2018 •

edited

Loading

NickLaMuro commented Apr 17, 2018

jrafanie commented Apr 17, 2018

NickLaMuro commented Apr 17, 2018

Add patch to aws-sdk-core to fix auth bug #432

Add patch to aws-sdk-core to fix auth bug #432

Conversation

NickLaMuro commented Apr 17, 2018

TODO

Links

QA Steps

miq-bot commented Apr 17, 2018

jrafanie Apr 17, 2018

Choose a reason for hiding this comment

NickLaMuro Apr 17, 2018

Choose a reason for hiding this comment

jrafanie Apr 17, 2018

Choose a reason for hiding this comment

NickLaMuro Apr 17, 2018

Choose a reason for hiding this comment

jrafanie Apr 17, 2018

Choose a reason for hiding this comment

jrafanie commented Apr 17, 2018

simaishi commented Apr 17, 2018

jrafanie commented Apr 17, 2018

NickLaMuro commented Apr 17, 2018 • edited Loading

NickLaMuro commented Apr 17, 2018

jrafanie commented Apr 17, 2018

NickLaMuro commented Apr 17, 2018

NickLaMuro commented Apr 17, 2018 •

edited

Loading