themarshallproject / klaxon

Klaxon enables reporters and editors to monitor scores of sites on the web for newsworthy changes.
https://newsklaxon.org
MIT License
646 stars 199 forks source link

Clean house on Docker implementation #642

Closed rdmurphy closed 1 year ago

rdmurphy commented 1 year ago

This PR basically declares Docker bankruptcy — what we currently have here in Klaxon is an amalgamation of many different (and sometimes conflicting) approaches to Docker that I think has reached a point where it's not meaningfully maintainable.

I think we should take the approach going forward that if you wanna set up a Docker system for Klaxon, it's best that you fork and take it from there. There's no hope it'll remain current and maintained in the main repo.

However! Rails 7.1 is going to have Dockerfile support out of the box, and the reference implementation exists as a gem called dockerfile-rails, which supports Rails 7.0. I've pulled this gem in and have used it to replace the old Dockerfile. Other than some minor tweaks to account for some stuff we expect to run on load it matches what that outputs by default.

Pay special attention to

I tweaked some stuff with the cookies to allow Klaxon to work again in non-HTTP scenarios. Setting force_ssl to true will handle setting SameSite: None and Secure: true for all cookies, so this will still be possible. I believe the now default value of Lax for SameSite is otherwise good enough, and I left HttpOnly: true.

Also — I made the text black. Time for Klaxon to be accessible. 😶

How to test

I guess you could make sure the Dockerfile builds? I've done this extensively on my end, so I'd vote to just trust me on this.