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

Rails credentials read prevents Flipper from working on Rails < 5.2 #857

Open
clinejj opened this issue Mar 6, 2024 · 2 comments
Open

Comments

@clinejj
Copy link
Contributor

clinejj commented Mar 6, 2024

In #782, there was an update to read values from Rails credentials. Credentials were added in Rails 5.2, which means this throws an error when using Flipper 1.1.0+ and Rails < 5.2:

2.544 NoMethodError: undefined method `credentials' for #<Website::Application:0x0000ffff8676ff30>
2.544 /website/config/environment.rb:5:in `<top (required)>'
2.544 /usr/local/ruby-2.7.8/bin/bundle:23:in `load'
2.544 /usr/local/ruby-2.7.8/bin/bundle:23:in `<main>'
2.544 Tasks: TOP => environment
2.544 (See full trace by running task with --trace)

There isn't an explicit dependency on particular Rails versions in the gemspec, however flipper-active_record does have a requirement for Rails >= 4.2. Without getting too nitpicky on the solutions here, some possible options:

  • Set a minimum Rails version in order to use Flipper
  • Update the calls at
    ENV["FLIPPER_CLOUD_TOKEN"] ||= app.credentials.dig(:flipper, :cloud_token)
    ENV["FLIPPER_CLOUD_SYNC_SECRET"] ||= app.credentials.dig(:flipper, :cloud_sync_secret)
    to check if the credentials method exists, and if not rely on the env vars

There's also a question of whether those env var checks need to happen there, rather than inside the cloud? block since they seem to only apply to cloud users (this is not used in a cloud application FWIW).

@jnunemaker
Copy link
Collaborator

Generally I’d say we don’t support versions that are EOL by rails but checking if the app responds to credentials seems easy. Any interest in a PR?

@clinejj
Copy link
Contributor Author

clinejj commented Mar 7, 2024

Sounds good, will submit in the next week-ish

astupka added a commit to customink/flipper_rails_upgrades that referenced this issue Apr 23, 2024
astupka added a commit to customink/flipper_rails_upgrades that referenced this issue Apr 25, 2024
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

No branches or pull requests

2 participants