Bidding adieu to apache2-websockets

Bug #1834208 reported by Jason Boyer
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Evergreen
Medium
Unassigned
OpenSRF
Medium
Unassigned

Bug Description

Currently the OpenSRF installation instructions present https://github.com/disconnect/apache-websocket as an option for websockets. This is an issue because of the apparent abandonware status of this project (last commit Sept 22, 2012) and performance issues surrounding its use. (Bug 1774703, partially but not entirely corrected by switching to https://github.com/jchampio/apache-websocket ; last commit Jan 20, 2017.) Since there can be a bias toward choosing the first when presented multiple seemingly equally valid options, I'd like to recommend we discontinue mentioning this option at all, making the installation and use of websocketd required until or unless some better (for whatever value of "better") option presents itself.

In addition to lessening the potential troubleshooting issues for new installations, this will simplify example configurations down to "proxy or not," rather than "proxy or not and if so what *type* of proxy," and give new users fewer options to consider when they may or may not have all of the necessary information at hand.

Something else to consider outside the scope of this particular change is whether or not proxying should still be presented as an option, or just assumed.

Jason Boyer (jboyer)
Changed in opensrf:
importance: Undecided → Medium
Bill Erickson (berick)
Changed in opensrf:
status: New → Confirmed
assignee: nobody → Bill Erickson (berick)
Revision history for this message
Bill Erickson (berick) wrote :

Patch pushed:

https://git.evergreen-ils.org/?p=working/OpenSRF.git;a=shortlog;h=refs/heads/user/berick/lp1834208-purge-apache2-websockets

---

Remove code and build components for apache2-websockets. Update documentation to reflect the use of websocketd as the only supported OpenSRF websockets implementation.

Add a note to the install documentation indicating websocketd does not offer a configurable inactivity timeout, but this can be accomplished by running it behind a proxy.

Update NGINX and HAPROXY example configs to reflect the assumption that websocketd is in use and runs locally without SSL by default.

** Haproxy users should review the changes. All I did was remove the 'ssl' from the 'ws' backend configuration.

tags: added: pullrequest
Changed in opensrf:
assignee: Bill Erickson (berick) → nobody
Revision history for this message
Bill Erickson (berick) wrote :

Unsure how best to target this one. Do we need a 3.2-beta?

Revision history for this message
Jason Boyer (jboyer) wrote :

I think all of examples/apache_24 can go away; it doesn't do anything or appear anywhere other than in the apache2-websockets section. As a style suggestion, the "Configure the main Apache instance ..." section of both proxy options can also be removed and put in the Apache section dealing with mod_remoteip, additionally striking the word "main" since there's only 1 Apache instance now. (I'm not trying to be a copy editor, but if we keep going it seems not-ideal to have a succession of branches people have to track for the latest.)

As for the targeting, it seems like this would make an ideal 3.1.1 since it's so minor and many people won't have any need to install it at all.

Revision history for this message
Jason Stephenson (jstephenson) wrote :

I mostly agree with Jason Boyer's comments in #3. I think the README could use some more editing. When I get time, I may add a commit.

I'm also not sure that the change to the haproxy config is correct. I don't see any haproxy config to handle SSL for websocketd, so I think websocketd should should be configured to do its own SSL and the websocketd line in examples/haproxy should be left untouched. Hopefully, I will have a chance to verify this in a few days as I plan to setup a test cluster with haproxy in front. I want to investigate the feasibility of replacing ldirectord with haproxy in our production environment.

Revision history for this message
Jason Stephenson (jstephenson) wrote :

Preliminary testing reveals that my concerns about SSL are misplaced. HAProxy will indeed handle the SSL proxying for websocketd.

I had to make additional changes to the sample HAProxy configuration for it to work on Ubuntu 16.04. I can test this change on Debian, later, and will share a patch.

I ran into some seemingly unrelated problems with web client code on my test system, so my testing was halted at trying to register a workstation. I'll pick this up later.

Revision history for this message
Jason Stephenson (jstephenson) wrote :

I am working on a collab branch to update the HAProxy configuration and to make some of the documentation updates suggested by Jason Boyer.

Changed in opensrf:
assignee: nobody → Jason Stephenson (jstephenson)
Revision history for this message
Jason Stephenson (jstephenson) wrote :

I added the Evergreen project as affected by this because I think we should modify the examples eg_vhost.conf to use X-Forwarded-For instead of X-Real-IP.

I'm also making more changes to the nginx and haproxy sample configurations. I'll have my branches cleaned up this week, and then I'll post them so that Jason Boyer can take over the README edits.

Changed in evergreen:
assignee: nobody → Jason Stephenson (jstephenson)
importance: Undecided → Medium
status: New → Confirmed
Revision history for this message
Jason Stephenson (jstephenson) wrote :

I have pushed a collab branch with a signoff on Bill's commit and two added commits. The "first" commit removes the examples/apache_24 directory for OpenSRF since it is no longer needed. The top commit makes some adjustments to the example nginx and haproxy configurations based on IRC conversations with Jason Boyer and my own testing. The OpenSRF collab branch is collab/dyrcona/lp1834208-purge-apache2-websockets.

https://git.evergreen-ils.org/?p=working/OpenSRF.git;a=shortlog;h=refs/heads/collab/dyrcona/lp1834208-purge-apache2-websockets

Jason Boyer indicated that he would take over the README edits.

I have also pushed a branch to the working Evergreen repo that makes a small change to the example eg_vhost.conf.in file for Apache 2.4. It changes the remote IP address header from X-Real-IP to X-Forwarded-For. Both OpenSRF proxy configurations send the X-Forwarded-For header so there is no need to add an additional header. That branch is user/dyrcona/lp1834208-sync-with-opensrf.

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/dyrcona/lp1834208-sync-with-opensrf

Changed in evergreen:
assignee: Jason Stephenson (jstephenson) → nobody
Changed in opensrf:
assignee: Jason Stephenson (jstephenson) → nobody
Revision history for this message
Jason Boyer (jboyer) wrote :

So, without being able to add signoffs to a collab branch, here's a new branch with signoffs to previous commits and a new commit simplifying the README further to remove duplication.

https://git.evergreen-ils.org/?p=working/OpenSRF.git;a=shortlog;h=refs/heads/user/jboyer/lp1834208-purge-apache2-websockets / working/user/jboyer/lp1834208-purge-apache2-websockets

Revision history for this message
Ben Shum (bshum) wrote :

Targeting branches towards new release milestones. Will review and push.

Changed in opensrf:
milestone: none → 3.2-beta
Changed in evergreen:
milestone: none → 3.4-beta1
assignee: nobody → Ben Shum (bshum)
Changed in opensrf:
assignee: nobody → Ben Shum (bshum)
Revision history for this message
Ben Shum (bshum) wrote :

Pushed to Evergreen and OpenSRF master for future releases.

Changed in evergreen:
status: Confirmed → Fix Committed
Changed in opensrf:
status: Confirmed → Fix Committed
Changed in evergreen:
assignee: Ben Shum (bshum) → nobody
Changed in opensrf:
assignee: Ben Shum (bshum) → nobody
Galen Charlton (gmc)
Changed in evergreen:
status: Fix Committed → Fix Released
Galen Charlton (gmc)
Changed in opensrf:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers