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

Use Rails.application.credentials instead of deprecated secrets #23254

Conversation

jrafanie
Copy link
Member

@jrafanie jrafanie commented Nov 1, 2024

Fixes:
DEPRECATION WARNING: Rails.application.secrets is deprecated in favor of Rails.application.credentials and will be removed in Rails 7.2.

Extracted from #23225

Fixes:
DEPRECATION WARNING: `Rails.application.secrets` is deprecated in favor of `Rails.application.credentials` and will be removed in Rails 7.2.
@jrafanie jrafanie mentioned this pull request Nov 1, 2024
41 tasks
@agrare
Copy link
Member

agrare commented Nov 3, 2024

@kbrock
Copy link
Member

kbrock commented Nov 4, 2024

Yea, I see that Rails.application.secrets is accessed in most providers.
Can we add TODOs for each provider here? (Want me to add the TODOs?

@jrafanie
Copy link
Member Author

jrafanie commented Nov 4, 2024

@jrafanie I assume we'll have to do this in all of the providers as well?
E.g.
https://github.com/ManageIQ/manageiq-providers-vmware/blob/b070f714d1bb4d887479b0de120ca6d1d87d2d48/spec/spec_helper.rb#L21

Thanks, I did see a warning from one of the plugins in core, it might have been the hmc provider. Yes, I expect we'll have to do this throughout the plugins. At this point, I'm trying to get core to work and then I'll move to the plugins.

Yea, I see that Rails.application.secrets is accessed in most providers. Can we add TODOs for each provider here? (Want me to add the TODOs?

Thanks, I'll add it to the the main issue.

@agrare agrare self-assigned this Nov 4, 2024
@agrare agrare merged commit dca9857 into ManageIQ:master Nov 4, 2024
12 checks passed
@Fryguy
Copy link
Member

Fryguy commented Nov 4, 2024

It's all over the place in the providers. Additionally, I think it's in the generator as well

@jrafanie
Copy link
Member Author

jrafanie commented Nov 4, 2024

Thanks, I'll add it to the the main issue.

I added it to #23200

@jrafanie jrafanie deleted the use-application-credentials-instead-of-secrets branch November 4, 2024 17:29
@Fryguy
Copy link
Member

Fryguy commented Nov 4, 2024

@jrafanie I updated the link in #23200 to be broader.

jrafanie added a commit to jrafanie/manageiq that referenced this pull request Nov 6, 2024
Partial Revert of ManageIQ#23254

Enhanced the test to check both the app config secret_key_base and app
secret_key_base as there's caches in both.  These test changes make the
test fail if we do not clear the cached secrets on Rails 7.0.
jrafanie added a commit to jrafanie/manageiq that referenced this pull request Nov 6, 2024
Partial Revert of ManageIQ#23254

Enhanced the test to check both the app config secret_key_base and app
secret_key_base as there's caches in both.  These test changes make the
test fail if we do not clear the cached secrets on Rails 7.0.
jrafanie added a commit to jrafanie/manageiq that referenced this pull request Nov 12, 2024
We're switching to rails credentials to keep current with rails 7.1 and the future.
Note, we're assuming we won't want to commit and share encrypted credentials. If we want
to share them, such as for recording cassettes, the comments describe how to switch to only
ignoring the plain text encryption key files.

Followup to:  ManageIQ#23254
Required for: ManageIQ/manageiq-providers-autosde#253
jrafanie added a commit to jrafanie/manageiq that referenced this pull request Nov 13, 2024
We're switching to rails credentials to keep current with rails 7.1 and the future.
Note, we're assuming we won't want to commit and share encrypted credentials. If we want
to share them, such as for recording cassettes, the comments describe how to switch to only
ignoring the plain text encryption key files.

Followup to:  ManageIQ#23254
Required for: ManageIQ/manageiq-providers-autosde#253
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants