-
Notifications
You must be signed in to change notification settings - Fork 181
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
[WIP] Use Rails credentials instead of deprecated and removed secrets #553
base: master
Are you sure you want to change the base?
[WIP] Use Rails credentials instead of deprecated and removed secrets #553
Conversation
Rails 7.1 removes access to modifying secrets as we should be moved over to rails credentials. Here we describe how to setup these rails crednetials for the purpose of recording VCR cassettes. See also: ManageIQ/manageiq-providers-autosde#253
def credentials_awesome_cloud_host | ||
@credentials_awesome_cloud_host ||= Rails.application.credentials.awesome_cloud_host || "awesome-cloud-host" | ||
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.
We didn't have a "host" in the old examples (this was modeled after a public-cloud so no hostname needed)
end | ||
|
||
def credentials_awesome_cloud_user | ||
@credentials_awesome_cloud_user ||= Rails.application.credentials.awesome_cloud_user || "awesome-cloud-user" |
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.
Should this be Rails.application.credentials.awesome_cloud&.access_key
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'm trying to generalize them. Does that make sense for a provider that uses user/password authentication? If so, I can do that.
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.
Well two things, one I think even if there was a user: "..."
it would be Rails.application.credentials.awesome_cloud.user
not Rails.application.credentials.awesome_cloud_user
and two the example here has access_key
not user
I don't think I'd want to change the "writing a provider" example to have host/user/password because that doesn't match with the rest of the example code (there's an example connect method and an example api_client, etc...)
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.
Maybe in addition to editing this specific example to work with the new credentials interface we need a "recording_vcrs.md" guide that covers how to do this more generally?
end | ||
|
||
def credentials_awesome_cloud_password | ||
@credentials_autosde_password ||= Rails.application.credentials.awesome_cloud_password || "change_me" |
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.
Rails.application.credentials.awesome_cloud&.secret_key
secrets.awesome_cloud.each do |key, val| | ||
config.define_cassette_placeholder(secrets.awesome_cloud_defaults[key]) { val } | ||
defaults = { | ||
"host_key" => credentials_awesome_cloud_host, |
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.
"host_key" => credentials_awesome_cloud_host, |
Rails 7.1 removes access to modifying secrets as we should be moved over to rails credentials. Here we describe how to setup these rails crednetials for the purpose of recording VCR cassettes.
See also: ManageIQ/manageiq-providers-autosde#253