Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

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

Open
intrip opened this issue Nov 8, 2023 · 2 comments

Comments

@intrip
Copy link

intrip commented Nov 8, 2023

12f7629 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 intrip changed the title SignedGlobalID: Consider re-adding the ability to use the old serialization format SignedGlobalID: Consider re-adding the ability to use the old serialization format and make new format opt-in Nov 9, 2023
@intrip intrip changed the title SignedGlobalID: Consider re-adding the ability to use the old serialization format and make new format opt-in SignedGlobalID: Consider re-adding the ability to use the old serialization format Nov 9, 2023
@intrip
Copy link
Author

intrip commented Nov 15, 2023

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.

@rwc9u
Copy link

rwc9u commented Oct 22, 2024

This is affecting us as well, because the signed global ids are created in workers and passed along as part of a link. The links don't expire for awhile. If we upgrade the gem, the links can no longer be parsed.

To expand on this a little more, our case was an edge case where we use bootboot for upgrading rails. In this case the DEPENDENCIES_NEXT Gemfile_next.lock had version 1.2.1 and the default had 1.0.0 set. We upgraded workers first, the urls were created with the new signed global id format. Our web processes were still running on the older version of globalid, and didn't know how to handle the new format. If we had updated both web and workers at the same time we wouldn't have had an issue.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants