[FBR] proxies: fix logic for websockets in reversepassproxy

[Tim Flink]

The last patch I did for this added some bits that didn't need to be there
and made a bad assumption about the default value for remotepath for the
reversepassproxy.conf template.

This ended up with unneccesarry complication in the ws balancers and a
unintended RewriteCond for any declared reversepassproxy that didn't
redefine remotepath.

This patch fixes the bad assumptions and removes the cruft that didn't
actually do or fix anything
---
 roles/httpd/reverseproxy/templates/reversepassproxy.conf | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/roles/httpd/reverseproxy/templates/reversepassproxy.conf
b/roles/httpd/reverseproxy/templates/reversepassproxy.conf
index 1e4afe0..1ea0b97 100644
--- a/roles/httpd/reverseproxy/templates/reversepassproxy.conf
+++ b/roles/httpd/reverseproxy/templates/reversepassproxy.conf
@@ -29,25 +29,17 @@ SSLProxyEngine On
 <Proxy "balancer://{{balancer_name}}-websocket">
   {% for member in balancer_members %}
     {% if http_not_https_yes_this_is_insecure_and_i_feel_bad %}
-    {% if remotepath is defined %}
-    BalancerMember "ws://{{ member }}{{ remotepath }}
-    {% else %}
     BalancerMember "ws://{{ member }}"
-    {% endif %}
-    {% else %}
-    {% if remotepath is defined %}
-    BalancerMember "wss://{{ member }}{{ remotepath }}
     {% else %}
     BalancerMember "wss://{{ member }}"
     {% endif %}
-    {% endif %}
   {% endfor %}
 </Proxy>
 
 RewriteEngine on
 RewriteCond %{HTTP:Upgrade} ^WebSocket$ [NC]
 RewriteCond %{HTTP:Connection} Upgrade [NC]
-{% if remotepath is defined %}
+{% if remotepath != "/" %}
 RewriteCond %{REQUEST_URI} ^{{ remotepath }}/(.)*
 {% endif %}
 RewriteRule .* "balancer://{{ balancer_name }}-websocket%{REQUEST_URI}" [P]
-- 
1.8.3.1

[FBR] proxies: fix logic for websockets in reversepassproxy (small change from first one)

[Tim Flink]

I forgot to make a change in the regexp for the RewriteCond - new patch
follows:

The last patch I did for this added some bits that didn't need to be there
and made a bad assumption about the default value for remotepath for the
reversepassproxy.conf template.

This ended up with unneccesarry complication in the ws balancers and a
unintended RewriteCond for any declared reversepassproxy that didn't
redefine remotepath.

This patch fixes the bad assumptions and removes the cruft that didn't
actually do or fix anything
---
 roles/httpd/reverseproxy/templates/reversepassproxy.conf | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/roles/httpd/reverseproxy/templates/reversepassproxy.conf
b/roles/httpd/reverseproxy/templates/reversepassproxy.conf
index 1e4afe0..93baefd 100644
--- a/roles/httpd/reverseproxy/templates/reversepassproxy.conf
+++ b/roles/httpd/reverseproxy/templates/reversepassproxy.conf
@@ -29,26 +29,18 @@ SSLProxyEngine On
 <Proxy "balancer://{{balancer_name}}-websocket">
   {% for member in balancer_members %}
     {% if http_not_https_yes_this_is_insecure_and_i_feel_bad %}
-    {% if remotepath is defined %}
-    BalancerMember "ws://{{ member }}{{ remotepath }}
-    {% else %}
     BalancerMember "ws://{{ member }}"
-    {% endif %}
-    {% else %}
-    {% if remotepath is defined %}
-    BalancerMember "wss://{{ member }}{{ remotepath }}
     {% else %}
     BalancerMember "wss://{{ member }}"
     {% endif %}
-    {% endif %}
   {% endfor %}
 </Proxy>
 
 RewriteEngine on
 RewriteCond %{HTTP:Upgrade} ^WebSocket$ [NC]
 RewriteCond %{HTTP:Connection} Upgrade [NC]
-{% if remotepath is defined %}
-RewriteCond %{REQUEST_URI} ^{{ remotepath }}/(.)*
+{% if remotepath != "/" %}
+RewriteCond %{REQUEST_URI} ^{{ remotepath }}/(.*)
 {% endif %}
 RewriteRule .* "balancer://{{ balancer_name }}-websocket%{REQUEST_URI}" [P]
 
-- 
1.8.3.1

[FBR] proxies: fix logic for websockets in reversepassproxy (small change from first one)

[Kevin Fenzi]

+1

kevin

[Tim Flink]

There was a bit of a miscommunication and a fix for this was pushed to
ansible while I was waiting for +1s on this and it's no longer needed.

I'll cleanup the template once freeze lifts since it's working now.

Tim



On Thu, 28 Mar 2019 15:07:55 -0600
Tim Flink <tflink(a)redhat.com&gt; wrote:

>  The last patch I did for this added some bits that didn't need to
> be
>  there and made a bad assumption about the default value for
>  remotepath for the reversepassproxy.conf template.
>  
>  This ended up with unneccesarry complication in the ws balancers and a
>  unintended RewriteCond for any declared reversepassproxy that didn't
>  redefine remotepath.
>  
>  This patch fixes the bad assumptions and removes the cruft that didn't
>  actually do or fix anything
>  ---
>   roles/httpd/reverseproxy/templates/reversepassproxy.conf | 10
>  +--------- 1 file changed, 1 insertion(+), 9 deletions(-)
>  
>  diff --git a/roles/httpd/reverseproxy/templates/reversepassproxy.conf
>  b/roles/httpd/reverseproxy/templates/reversepassproxy.conf index
>  1e4afe0..1ea0b97 100644 ---
>  a/roles/httpd/reverseproxy/templates/reversepassproxy.conf +++
>  b/roles/httpd/reverseproxy/templates/reversepassproxy.conf @@ -29,25
>  +29,17 @@ SSLProxyEngine On <Proxy
>  "balancer://{{balancer_name}}-websocket"> {% for member in
>  balancer_members %} {% if
>  http_not_https_yes_this_is_insecure_and_i_feel_bad %}
>  -    {% if remotepath is defined %}
>  -    BalancerMember "ws://{{ member }}{{ remotepath }}
>  -    {% else %}
>       BalancerMember "ws://{{ member }}"
>  -    {% endif %}
>  -    {% else %}
>  -    {% if remotepath is defined %}
>  -    BalancerMember "wss://{{ member }}{{ remotepath }}
>       {% else %}
>       BalancerMember "wss://{{ member }}"
>       {% endif %}
>  -    {% endif %}
>     {% endfor %}
>   </Proxy>
>   
>   RewriteEngine on
>   RewriteCond %{HTTP:Upgrade} ^WebSocket$ [NC]
>   RewriteCond %{HTTP:Connection} Upgrade [NC]
>  -{% if remotepath is defined %}
>  +{% if remotepath != "/" %}
>   RewriteCond %{REQUEST_URI} ^{{ remotepath }}/(.)*
>   {% endif %}
>   RewriteRule .*
>  "balancer://{{ balancer_name }}-websocket%{REQUEST_URI}" [P]