Skip to content
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

Fixes #37761 - use ProxyPass and upgrade=websocket where possible #1185

Merged
merged 1 commit into from
Sep 18, 2024

Conversation

evgeni
Copy link
Member

@evgeni evgeni commented Sep 18, 2024

RewriteRules need special handling of some characters (esp "?"), which
differs based on Apache version. Instead of going down that way, we can
switch to using ProxyPass as proxying is the only thing we really need
here, at least for HTTP.

For WebSockets, we need to allow protocol upgrades, which modern
(2.4.47+) Apache can do itself via "ProxyPass … upgrade=websocket".
For older Apache (esp on EL8 and Ubuntu 20.04), we keep the RewriteRules
in place. To be removed when we drop support for those targets.

Co-Authored-By: Adam Ruzicka <[email protected]>

# 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 = []
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be undef instead?

@ehelms
Copy link
Member

ehelms commented Sep 18, 2024

I tried testing this, but I think it's user error,. I just run into "Sorry" when I try to look at the web console.

@evgeni
Copy link
Member Author

evgeni commented Sep 18, 2024

can you post a screenshot with the browser console open to the network tab (and after a reload, as otherwise the tab is empty)

@ehelms
Copy link
Member

ehelms commented Sep 18, 2024

can you post a screenshot with the browser console open to the network tab (and after a reload, as otherwise the tab is empty)

image

@evgeni
Copy link
Member Author

evgeni commented Sep 18, 2024

Eh, you have no cockpit on the target machine, or no ssh set up

@ehelms
Copy link
Member

ehelms commented Sep 18, 2024

Eh, you have no cockpit on the target machine, or no ssh set up

I said user error. Cockpit is definitely set up. Let me double check SSH. Also, this error output sucks.

@evgeni
Copy link
Member Author

evgeni commented Sep 18, 2024

foreman-installer --enable-foreman-plugin-remote-execution --enable-foreman-plugin-remote-execution-cockpit --enable-foreman-proxy-plugin-remote-execution-script --foreman-proxy-plugin-remote-execution-script-cockpit-integration true --foreman-proxy-plugin-remote-execution-script-install-key true

this should make cockpit work against the foreman box

and yes, please complain to cockpit-ws, we do not own that

Copy link
Member

@ehelms ehelms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works for me, thanks @evgeni !

@evgeni evgeni changed the title use upgrade=websocket where possible Fixes #37761 - use ProxyPass and upgrade=websocket where possible Sep 18, 2024
RewriteRules need special handling of some characters (esp "?"), which
differs based on Apache version. Instead of going down that way, we can
switch to using ProxyPass as proxying is the only thing we really need
here, at least for HTTP.

For WebSockets, we need to allow protocol upgrades, which modern
(2.4.47+) Apache can do itself via "ProxyPass … upgrade=websocket".
For older Apache (esp on EL8 and Ubuntu 20.04), we keep the RewriteRules
in place. To be removed when we drop support for those targets.

Co-Authored-By: Adam Ruzicka <[email protected]>
@evgeni
Copy link
Member Author

evgeni commented Sep 18, 2024

updated commit message etc.

@evgeni evgeni marked this pull request as ready for review September 18, 2024 17:19
@evgeni evgeni added the Bug label Sep 18, 2024
@evgeni evgeni merged commit 88e8ecc into master Sep 18, 2024
19 checks passed
@evgeni evgeni deleted the better-proxy branch September 18, 2024 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants