Add support for rails credentials #34
Conversation
ruby to 3.2.2 Rails to 7.1.3 and rspec to 3.12
svanhesteren
force-pushed
the
add-support-for-rails-credentials
branch
from
55ba2da
to
d7b2298
Compare
lib/eyaml/railtie.rb
Outdated
| @@ -9,7 +9,13 @@ class Railtie < Rails::Railtie | |||
| PRIVATE_KEY_ENV_VAR = "EJSON_PRIVATE_KEY" | |||
|
|
|||
| config.before_configuration do | |||
| secrets_files.each do |file| | |||
| secrets_or_credentials = if Rails.version.start_with?("7.2") || Dir.glob(Rails.root.join("config", "credentials.*")).any? | |||
There was a problem hiding this comment.
What happens if both files are present? As in - do we need to force the user to have both, or to choose one or the other? can we conditionally assign both, and let credentials take over (with a deep merge, for example)? Or we could tell the user that they need to rename the file and it will get picked up automatically? because there are Rails versions which have both (like we have now)
There was a problem hiding this comment.
Also: this check is too magicall as well, as it would pick up any, say, config.bak.rb file as an indication that the application is using credentials - while it might not be the case.
There was a problem hiding this comment.
I have added a more explicit check for both kinds. From rails 7.2 when secret files are deprecated we only look for credential files here. Before that credential files have prio over secrets, but secrets will still work of course.
lib/eyaml/railtie.rb
Outdated
| Rails.root.join("config", "secrets.#{ext}"), | ||
| Rails.root.join("config", "secrets.#{Rails.env}.#{ext}") | ||
| Rails.root.join("config", "#{secrets_or_credentials}.#{ext}"), | ||
| Rails.root.join("config", "#{secrets_or_credentials}.#{Rails.env}.#{ext}") | ||
| ] | ||
| end.flatten |
| EYAML::SUPPORTED_EXTENSIONS.map do |ext| | ||
| [ | ||
| Rails.root.join("config", "secrets.#{ext}"), | ||
| Rails.root.join("config", "secrets.#{Rails.env}.#{ext}") | ||
| Rails.root.join("config", "#{secrets_or_credentials}.#{ext}"), |
There was a problem hiding this comment.
nit: maybe hint that these files will be read and merged from the least specific to the more specific one? also, should there be, say, credentials.production.eyaml and credentials.eyml the latter will overwrite everything introduced by credentials.production.eyaml even though it is more specific (specific to an environment). So to play this safe it would be better to do the product the other way around - first "envless" variants for all extensions, then "enved" variants for all the extensions. Still a bit magical but will be more logical IMO.
| @@ -9,6 +9,10 @@ def secrets_class | |||
| ActiveSupport::OrderedOptions | |||
| end | |||
|
|
|||
| def credentials_class | |||
There was a problem hiding this comment.
we positive that on all our Rails versions the secrets/credentials thingies exposed from the config are OrderedOptions?
There was a problem hiding this comment.
I am seeing various resources online indicating that rails uses OrderedOptions for secrets/credentials dating back to rails 4 even, so I am betting on the fact that this will not cause issues. If so, I'm happy to receive an issue/pr about it. :)
So we prioritize credential files over secret files and only look for credential files from rails 7.2 onwards.
svanhesteren
force-pushed
the
add-support-for-rails-credentials
branch
from
07db351
to
51b3269
Compare
From rails 7.2 on the secrets mechanism will be deprecated in favor of rails credentials and from rails 7.1 on it will show warnings about it, so time to add support for it in here.
This includes updates to rails 7.1.3, ruby 3.2.2 and rspec 3.16.