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

Upgrade Ruby to 2.4.5 #2995

Merged
merged 35 commits into from
Jun 3, 2019
Merged

Upgrade Ruby to 2.4.5 #2995

merged 35 commits into from
Jun 3, 2019

Conversation

jholton
Copy link
Contributor

@jholton jholton commented Apr 23, 2019

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

  • Ruby version set to 2.4.6
  • All specs pass
  • All deprecations resolved
  • update webmock gem
  • update json-schema gem
  • update sidekiq-scheduler gem, which updates the rufus-scheduler gem

Applies to all PRs

  • Appropriate logging
  • Swagger docs have been updated, if applicable
  • Provide link to originating GitHub issue, or connected to it via ZenHub
  • Does not contain any sensitive information (i.e. PII/credentials/internal URLs/etc., in logging, hardcoded, or in specs)
  • Provide which alerts would indicate a problem with this functionality (if applicable)

@va-vfs-bot va-vfs-bot temporarily deployed to jh_ruby_2.4_upgrade/master April 23, 2019 20:26 Inactive
@@ -32,13 +32,13 @@ binaries:
pdftk: pdftk
clamdscan: /usr/bin/clamdscan

db_encryption_key: f01ff8ebd1a2b053ad697ae1f0d86adb48ebb708021e4c76c3807d37f6b4e389d5aa45ea171f2f5074222784c1ee2bb8272390d1b9517a7a6987c22733ef00b2
db_encryption_key: f01ff8ebd1a2b053ad697ae1f0d86adb
Copy link
Contributor Author

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

Copy link
Contributor

@omgitsbillryan omgitsbillryan Apr 24, 2019

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.

Copy link
Contributor Author

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.

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.

@jholton jholton requested review from wyattwalter and removed request for b00klegger April 24, 2019 19:41
@jholton
Copy link
Contributor Author

jholton commented Apr 24, 2019

As mentioned in the issue, this will of course require coordinating some changes with devops. Namely,

  • devops changes to Ruby 2.4.5 for deployed environments
  • devops changes to db_encryption_key
  • verify that sidekiq-scheduler continues to schedule jobs when expected (due to update of the library).

@jholton
Copy link
Contributor Author

jholton commented May 17, 2019

  • Merging master created conflits in Gemfile.lock so, I reset it to what is current in master then re-applied my updates.
  • updated .ruby-version file to specify 2.4.5
  • updated Dockerfile to specify 2.4.5-slim-stretch
  • updated docs/setup/native.md to specify Ruby version 2.4.5
  • After rubocop changed =~ operators to .match?, had to use safe navigation operator with .match? when evaluating response_headers['content-type'] in middlewares because that value can be nil
  • change Makefile commands to use rails instead of rake
  • change syntax to set OpenSSL::PKey::RSA key to be compatible with OpenSSL 1.1

@jholton jholton changed the title Upgrade Ruby to 2.4.6 Upgrade Ruby to 2.4.5 May 17, 2019
@jholton jholton requested review from annaswims, lihanli and kfrz May 17, 2019 20:27
@jholton
Copy link
Contributor Author

jholton commented May 17, 2019

@kreek @annaswims @kfrz @lihanli re-requesting review. See comment above for changes since last review. Thanks.

@va-vfs-bot va-vfs-bot temporarily deployed to jh_ruby_2.4_upgrade/master May 17, 2019 20:30 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to jh_ruby_2.4_upgrade/master May 17, 2019 20:38 Inactive
Copy link
Contributor

@kreek kreek left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

Copy link
Contributor

@kfrz kfrz left a comment

Choose a reason for hiding this comment

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

Nicely done. 👍 :shipit:

@va-vfs-bot va-vfs-bot temporarily deployed to jh_ruby_2.4_upgrade/master May 20, 2019 15:23 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to jh_ruby_2.4_upgrade/master June 3, 2019 14:35 Inactive
@jholton jholton merged commit ef71f55 into master Jun 3, 2019
@jholton jholton deleted the jh_ruby_2.4_upgrade branch June 3, 2019 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants