-
Notifications
You must be signed in to change notification settings - Fork 898
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
Add a shared vcr spec helper for access to secrets #23292
base: master
Are you sure you want to change the base?
Conversation
We need a way to sanitize actual secrets with fake and consistent values so tests are reliable. This mechanism must allow us to provide overide values with real secrets when we're actually recording new or updated cassettes. For now, we're reusing the existing mechanism of specifying the secrets: * relative to the current directory - we run specs for plugins from the plugin's directory * config/secrets.yml - actual secrets * config/secrets.defaults.yml - defaults for sanitizing real values with consistent repeatable replacements Plugins can use this mechanism by adding: Object.include Spec::Shared::CassetteSecretsHelper to their spec_helper.rb. You can then access these values via helper methods with interfaces like below: default_vcr_secret_by_key_path(:vmware_infra, :hostname) vcr_secret_by_key_path(:vmware_tanzu, :hostname) Each plugin should define their own: config/secrets.default.yml, which can look something like this: ``` --- :vmware_cloud: :host: vmwarecloudhost :userid: VMWARE_CLOUD_USERID :password: VMWARE_CLOUD_PASSWORD :vmware_infra: :hostname: HOSTNAME :vmware_tanzu: :hostname: vmware-tanzu-hostname :userid: VMWARE_TANZU_USERID :password: VMWARE_TANZU_PASSWORD ``` Developers can then copy this file to config/secrets.yml and provide actual values for each of the keys. When recording, the cassettes will use the real values for connecting to environments, but store the resulting placeholder using the fake value for that key from the defaults.
DEFAULT_VCR_SECRETS_PATH = Pathname.new(Dir.pwd).join("config/secrets.defaults.yml") | ||
VCR_SECRETS_PATH = Pathname.new(Dir.pwd).join("config/secrets.yml") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this might be a little cleaner, just in case Dir.pwd isn't what we think it is?
DEFAULT_VCR_SECRETS_PATH = Pathname.new(Dir.pwd).join("config/secrets.defaults.yml") | |
VCR_SECRETS_PATH = Pathname.new(Dir.pwd).join("config/secrets.yml") | |
if defined?(ENGINE_ROOT) | |
DEFAULT_VCR_SECRETS_PATH = Pathname.new(ENGINE_ROOT).join("config/secrets.defaults.yml") | |
VCR_SECRETS_PATH = Rails.root.join("config/secrets.yml") | |
end |
Since this is at the constant level, that's really all we can do, unless we also want to allow Rails.root
and allow core to have secrets too. Otherwise, if we move this into a method instead, and load it on-demand, then we could do something like:
DEFAULT_VCR_SECRETS_PATH = Pathname.new(Dir.pwd).join("config/secrets.defaults.yml") | |
VCR_SECRETS_PATH = Pathname.new(Dir.pwd).join("config/secrets.yml") | |
def default_vcr_secrets_path | |
raise "Cassette secrets are only expected to be used from plugins" unless defined?(ENGINE_ROOT) | |
Pathname.new(ENGINE_ROOT).join("config/secrets.defaults.yml") | |
end | |
def vcr_secrets_path | |
raise "Cassette secrets are only expected to be used from plugins" unless defined?(ENGINE_ROOT) | |
Rails.root.join("config/secrets.yml") | |
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
defined?(ENGINE_ROOT)
basically means "we are running this from a plugin" as opposed to "we are running in core"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I thought I tried ENGINE_ROOT. I'll take another look.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer the 'extract a method' style as we change it if it needs to be handled in core.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, I remember the hurdle I hit. ENGINE_ROOT is not defined. I could find it but it's the same as what I did:
(byebug) Vmdb::Plugins.paths.detect {|_,path| File.expand_path(path) == File.expand_path(Dir.pwd)}.last
"/Users/joerafaniello/Code/manageiq-providers-vmware"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ENGINE_ROOT is not defined.
yeah, that's why you do defined?(ENGINE_ROOT)
. If we want to support core also, you just do
root = defined?(ENGINE_ROOT) ? Pathname.new(ENGINE_ROOT) : Rails.root
Then you go from there. I don't like Dir.pwd
, because it assumes you are running tests from the root directory, and that's not always the case. For example, it's not wrong to do cd spec; be rspec ./models/foo_spec.rb
. If you use a root object though, then you can really be anywhere in the directory tree.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, but that doesn't really help. If ENGINE_ROOT isn't defined in the plugin context as I'd still need to look at Dir.pwd
to determine the plugin's directory since Rails.root
is not the location of the engine:
joerafaniello@MacBookPro manageiq-providers-vmware % bin/rails c
...
Loading development environment (Rails 7.0.8.6)
irb(main):001> Rails.root
=> #<Pathname:/Users/joerafaniello/Code/manageiq>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm really confused - ENGINE_ROOT is the path to the plugin and will always be available when running from a plugin.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
~/dev/manageiq-providers-vmware $ be rails c
irb(main):001> ENGINE_ROOT
=> "/Users/jfrey/dev/manageiq-providers-vmware"
irb(main):002> Rails.root
=> #<Pathname:/Users/jfrey/dev/manageiq>
irb(main):003>
~/dev/manageiq $ be rails c
irb(main):001> defined?(ENGINE_ROOT)
=> nil
irb(main):004> Rails.root
=> #<Pathname:/Users/jfrey/dev/manageiq>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll have to check when I have time. It was undefined in the context of the plugin at test time.
e316215
to
69cd2d0
Compare
Note: rails secrets are deprecated and some features are actually removed in rails 7.1. This PR allows us to use plain text files for use for dev environments with an easy mechanism for sanitizing the real credentials with fake values to be used as placeholders in the vcr cassettes.
Here's one provider using this feature: ManageIQ/manageiq-providers-vmware#928
We need a way to sanitize actual secrets with fake and consistent values so tests are reliable. This mechanism must allow us to provide overide values with real secrets when we're actually recording new or updated cassettes.
For now, we're reusing the existing mechanism of specifying the secrets:
Plugins can use this mechanism by adding:
Object.include Spec::Shared::CassetteSecretsHelper to their spec_helper.rb.
You can then access these values via helper methods with interfaces like below:
default_vcr_secret_by_key_path(:vmware_infra, :hostname) vcr_secret_by_key_path(:vmware_tanzu, :hostname)
Each plugin should define their own:
config/secrets.default.yml, which can look something like this:
Developers can then copy this file to config/secrets.yml and provide actual values for each of the keys.
When recording, the cassettes will use the real values for connecting to environments, but store the resulting placeholder using the fake value for that key from the defaults.