use upgrade=websocket where possible

theforeman · Sep 18, 2024 · 5b6ff1d · 5b6ff1d
1 parent d5d4921
commit 5b6ff1d
Show file tree

Hide file tree

Showing 6 changed files with 59 additions and 24 deletions.
diff --git a/manifests/config/apache.pp b/manifests/config/apache.pp
@@ -167,8 +167,9 @@
   if $suburi {
     $custom_fragment = undef
   } else {
-    # mod_env and mod_expires are required by configuration in _assets.conf.erb
+    # mod_env, mod_expires and mod_rewrite are required by configuration in _assets.conf.erb
     include apache::mod::env
+    include apache::mod::rewrite
     # apache::mod::expires pulls in a config file we don't want, like apache::default_mods
     # It uses ensure_resource to be compatible with both $apache::default_mods set to true and false
     include apache
@@ -182,8 +183,23 @@
     order   => '03',
   }
 
-  include apache::mod::proxy_wstunnel
-  $websockets_backend = regsubst($_proxy_backend, 'http://', 'ws://')
+  # mod_proxy supports "ProxyPass ... upgrade=websocket" since 2.4.47
+  # EL8: 2.4.37 / EL9: 2.4.62 / Debian11: 2.4.62 / Ubuntu20.04: 2.4.41 / Ubuntu22.04: 2.4.52
+  $proxy_upgrade_websocket = !($facts['os']['family'] == 'RedHat' and $facts['os']['release']['major'] == '8') and !($facts['os']['name'] == 'Ubuntu' and $facts['os']['release']['major'] == '20.04')
+  if $proxy_upgrade_websocket {
+    $vhost_rewrites = []
+    $_proxy_params = $proxy_params + { 'upgrade' => 'websocket' }
+  } else {
+    include apache::mod::proxy_wstunnel
+    $websockets_backend = regsubst($_proxy_backend, 'http://', 'ws://')
+    $websockets_rewrite = {
+      'comment'      => 'Upgrade Websocket connections',
+      'rewrite_cond' => '%{HTTP:Upgrade} =websocket [NC]',
+      'rewrite_rule' => "/(.*) ${websockets_backend}\$1 [P,L]",
+    }
+    $vhost_rewrites = [$websockets_rewrite]
+    $_proxy_params = $proxy_params
+  }
 
   $vhost_http_request_headers = [
     'set X_FORWARDED_PROTO "http"',
@@ -209,15 +225,9 @@
       'no_proxy_uris' => $_proxy_no_proxy_uris,
       'path'          => pick($suburi, '/'),
       'url'           => $_proxy_backend,
-      'params'        => $proxy_params,
+      'params'        => $_proxy_params,
     },
-    'rewrites'            => [
-      {
-        'comment'      => 'Upgrade Websocket connections',
-        'rewrite_cond' => '%{HTTP:Upgrade} =websocket [NC]',
-        'rewrite_rule' => "/(.*) ${websockets_backend}\$1 [P,L]",
-      },
-    ],
+    'rewrites'            => $vhost_rewrites,
   }
 
   $vhost_https_request_headers = [

diff --git a/manifests/plugin/remote_execution/cockpit.pp b/manifests/plugin/remote_execution/cockpit.pp
@@ -72,11 +72,16 @@
       require        => Class['foreman::database'],
     }
   } else {
-    include apache::mod::rewrite
-    include apache::mod::proxy_wstunnel
     include apache::mod::proxy_http
+    if $foreman::config::apache::proxy_upgrade_websocket {
+      $_apache_template = 'cockpit-apache-ssl.conf.erb'
+    } else {
+      include apache::mod::rewrite
+      include apache::mod::proxy_wstunnel
+      $_apache_template = 'cockpit-apache-ssl-rewrite.conf.erb'
+    }
     foreman::config::apache::fragment { 'cockpit':
-      ssl_content => template('foreman/cockpit-apache-ssl.conf.erb'),
+      ssl_content => template("foreman/${_apache_template}"),
     }
 
     foreman_config_entry { 'remote_execution_cockpit_url':

diff --git a/spec/classes/foreman_config_apache_spec.rb b/spec/classes/foreman_config_apache_spec.rb
@@ -22,8 +22,11 @@
         should contain_apache__mod('expires')
         should contain_class('apache::mod::proxy')
         should contain_class('apache::mod::proxy_http')
-        should contain_class('apache::mod::proxy_wstunnel')
         should contain_class('apache::mod::rewrite')
+        if (facts[:os]['family'] == 'RedHat' && facts[:os]['release']['major'] == '8') ||
+            (facts[:os]['name'] == 'Ubuntu' && facts[:os]['release']['major'] == '20.04')
+          should contain_class('apache::mod::proxy_wstunnel')
+        end
       end
 
       it 'does not manage the docroot' do
@@ -109,8 +112,11 @@ class { 'apache':
           should contain_apache__mod('expires')
           should contain_class('apache::mod::proxy')
           should contain_class('apache::mod::proxy_http')
-          should contain_class('apache::mod::proxy_wstunnel')
           should contain_class('apache::mod::rewrite')
+          if (facts[:os]['family'] == 'RedHat' && facts[:os]['release']['major'] == '8') ||
+              (facts[:os]['name'] == 'Ubuntu' && facts[:os]['release']['major'] == '20.04')
+            should contain_class('apache::mod::proxy_wstunnel')
+          end
         end
       end
 

diff --git a/spec/classes/plugin/remote_execution_cockpit_spec.rb b/spec/classes/plugin/remote_execution_cockpit_spec.rb
@@ -70,8 +70,16 @@ class {'foreman':
           is_expected.to contain_foreman__config__apache__fragment('cockpit')
             .without_content
             .with_ssl_content(%r{^<Location /webcon>$})
-            .with_ssl_content(%r{^  RewriteRule /webcon/\(\.\*\)           ws://127\.0\.0\.1:19090/webcon/\$1 \[P\]$})
-            .with_ssl_content(%r{^  RewriteRule /webcon/\(\.\*\)           http://127\.0\.0\.1:19090/webcon/\$1 \[P\]$})
+          if (facts[:os]['family'] == 'RedHat' && facts[:os]['release']['major'] == '8') ||
+              (facts[:os]['name'] == 'Ubuntu' && facts[:os]['release']['major'] == '20.04')
+            is_expected.to contain_foreman__config__apache__fragment('cockpit')
+              .with_ssl_content(%r{^  RewriteRule /webcon/\(\.\*\)           ws://127\.0\.0\.1:19090/webcon/\$1 \[P\]$})
+              .with_ssl_content(%r{^  ProxyPass http://127\.0\.0\.1:19090/webcon$})
+          else
+            is_expected.to contain_foreman__config__apache__fragment('cockpit')
+              .without_ssl_content(%r{RewriteRule})
+              .with_ssl_content(%r{^  ProxyPass http://127\.0\.0\.1:19090/webcon upgrade=websock$})
+          end
         end
       end
 

diff --git a/templates/cockpit-apache-ssl-rewrite.conf.erb b/templates/cockpit-apache-ssl-rewrite.conf.erb
@@ -0,0 +1,11 @@
+### File managed with puppet ###
+
+<Location <%= @cockpit_path %>>
+  ProxyPreserveHost On
+
+  RewriteEngine On
+  RewriteCond %{HTTP:Upgrade} =websocket [NC]
+  RewriteRule <%= @cockpit_path %>/(.*)           ws://<%= @cockpit_host %>:<%= @cockpit_port %><%= @cockpit_path %>/$1 [P]
+
+  ProxyPass http://<%= @cockpit_host %>:<%= @cockpit_port %><%= @cockpit_path %>
+</Location>
diff --git a/templates/cockpit-apache-ssl.conf.erb b/templates/cockpit-apache-ssl.conf.erb
@@ -2,10 +2,5 @@
 
 <Location <%= @cockpit_path %>>
   ProxyPreserveHost On
-
-  RewriteEngine On
-  RewriteCond %{HTTP:Upgrade} =websocket [NC]
-  RewriteRule <%= @cockpit_path %>/(.*)           ws://<%= @cockpit_host %>:<%= @cockpit_port %><%= @cockpit_path %>/$1 [P]
-  RewriteCond %{HTTP:Upgrade} !=websocket [NC]
-  RewriteRule <%= @cockpit_path %>/(.*)           http://<%= @cockpit_host %>:<%= @cockpit_port %><%= @cockpit_path %>/$1 [P]
+  ProxyPass http://<%= @cockpit_host %>:<%= @cockpit_port %><%= @cockpit_path %> upgrade=websocket
 </Location>