globalid icon indicating copy to clipboard operation
globalid copied to clipboard

SignedGlobalID: Consider re-adding the ability to use the old serialization format

Open intrip opened this issue 1 year ago • 2 comments

https://github.com/rails/globalid/commit/12f76297c803c658ea2fc684d4336db347a90901 removed the ability to keep using the old format when serializing via SignedGlobalID. This causes the following issues:

  • Temporary errors during rolling deployments: During a rolling deployment, is possible to sign new data using the new format (new codebase) and then trigger an error when decoding the new data via the old codebase.
  • Rollbacks: When the data is encoded with the new format, it needs to be re-encoded to be read using the old format; this complicates rolling back to an old version.

I understand that "Rollbacks" can be considered a "no-issue" since the upgrade is one-way but I'm still afraid of the rolling-deployment issue. Additionally, Version "1.2.1" is the new version pointed by Rails "7.1", which makes rolling back a Rails upgrade more complicated because it forces you to upgrade GlobalID before upgrading Rails in order to not encounter the "unexpected" rollback issue.

#!/usr/bin/env ruby

require "bundler/inline"

globalid_version = ARGV[0]
$sgid = ARGV[1]

# true = install gems so this is fast on repeat invocations
gemfile(true, quiet: true) do
  source "https://rubygems.org"

  gem "globalid", globalid_version
  gem "activerecord", "~> 7.1"
  gem "sqlite3"
end

require "active_record"

# This connection will do for database-independent bug reports.
ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")

ActiveRecord::Schema.define do
  create_table "people" do |t|
    t.string "name"
  end
end
class Person < ActiveRecord::Base; end

SignedGlobalID.verifier = ActiveSupport::MessageVerifier.new("secret")
person = Person.create!(name: "John Doe")

if $sgid
  require "minitest/autorun"
  class SignedGlobalIdTest < Minitest::Test
    def test_cant_locate_new_format_sgid_with_old_version
      assert GlobalID::Locator.locate_signed($sgid), "Can't locate by SGID"
    end
  end
else
  puts SignedGlobalID.create(person, app: "test")
end
jacopo-37s-mb 3.3.0-preview2 ~ ./signed_globalid_serializarion_issue.rb 1.2.0 | tail -n 1 | xargs ./signed_globalid_serializarion_issue.rb 1.2.0
-- create_table("people")
   -> 0.0081s
Run options: --seed 25563

# Running:

.

Finished in 0.014660s, 68.2128 runs/s, 68.2128 assertions/s.

1 runs, 1 assertions, 0 failures, 0 errors, 0 skips
jacopo-37s-mb 3.3.0-preview2 ~ ./signed_globalid_serializarion_issue.rb 1.2.0 | tail -n 1 | xargs ./signed_globalid_serializarion_issue.rb 1.1.0
-- create_table("people")
   -> 0.0093s
Run options: --seed 3475

# Running:

F

Finished in 0.000923s, 1083.4238 runs/s, 1083.4238 assertions/s.

  1) Failure:
SignedGlobalIdTest#test_cant_locate_new_format_sgid_with_old_version [./signed_globalid_serializarion_issue.rb:36]:
Can't locate by SGID

1 runs, 1 assertions, 1 failures, 0 errors, 0 skips
jacopo-37s-mb 3.3.0-preview2 ~

intrip avatar Nov 08 '23 18:11 intrip

Let me know your take on this, I'll be happy to create a PR to re-add the ability to use the old format.

intrip avatar Nov 15 '23 09:11 intrip