-
Notifications
You must be signed in to change notification settings - Fork 68
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
Upgrade Ruby to 2.4.5 #2995
Upgrade Ruby to 2.4.5 #2995
Conversation
…` instead of `ZoTime`
config/settings.yml
Outdated
@@ -32,13 +32,13 @@ binaries: | |||
pdftk: pdftk | |||
clamdscan: /usr/bin/clamdscan | |||
|
|||
db_encryption_key: f01ff8ebd1a2b053ad697ae1f0d86adb48ebb708021e4c76c3807d37f6b4e389d5aa45ea171f2f5074222784c1ee2bb8272390d1b9517a7a6987c22733ef00b2 | |||
db_encryption_key: f01ff8ebd1a2b053ad697ae1f0d86adb |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In Ruby 2.3, OpenSSL would just truncate this key to 32 bytes.
In Ruby 2.4, OpenSSL no longer truncates, but now raises an error because it expects a 32 byte key.
Reference
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assuming this change is backward compatible with instances running Ruby 2.3 (which I think it is?), might be wise to PR & merge on it's own, preemptively to this PR upgrading ruby, to reduce risk.
This will require updates to credstash
as well (since the keys in there are too long as well), which again, is probably wise to do in advance of the ruby upgrade if we can.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it is backward compatible with Ruby 2.3.
- All my specs pass with the truncated keys
- I did manually:
- Create a db object that uses the
db_encryption_key
in a Ruby 2.3/long key environment. - Then I switched branches to the Ruby 2.4/short key environment and verified that I could retrieve the previously saved data.
- Create a db object that uses the
These specific changes are only used in local dev/testing, with the other environments values being held in credstash
, as you mentioned.
I'll open a separate PR for these truncations, and perhaps a devops engineer can truncate the values for dev
and/or staging
for now for additional manual testing.
As mentioned in the issue, this will of course require coordinating some changes with devops. Namely,
|
…headers['content-type'] in middlewares
|
@kreek @annaswims @kfrz @lihanli re-requesting review. See comment above for changes since last review. Thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nicely done. 👍
Description of change
See issue for description of changes.
Testing done
Specs
Testing planned
Manual testing, monitoring logs and Sentry in other environments
Acceptance Criteria (Definition of Done)
Unique to this PR
webmock
gemjson-schema
gemsidekiq-scheduler
gem, which updates therufus-scheduler
gemApplies to all PRs