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

Set secret_key_base early enough for rails processes #23351

Conversation

jrafanie
Copy link
Member

@jrafanie jrafanie commented Feb 24, 2025

We don't use secret_key_base like vanilla rails. We implemented secrets through MiqPassword before any of this existed in rails. Our only things currently going through rails secrets perhaps is the provider vcr cassette information.

Rails 7.1 asserts secret_key_base is now set and set during rails boot fairly early.

So, now, we can't only set this for puma based workers, but all rails processes. We also, must move this logic to be done earlier during boot. It was found that after eager load allows us to leverage autoload and access the database, both are required in order to fetch the secrets from the database.

Move over the logic for rails console so it can use a dummy secret key base.

Move the tests to a vmdb/initializer test.

Co-Authored-By: Jason Frey fryguy9@gmail.com

@jrafanie
Copy link
Member Author

@miq-bot cross-repo-test /all

miq-bot pushed a commit to ManageIQ/manageiq-cross_repo-tests that referenced this pull request Feb 24, 2025
@jrafanie jrafanie force-pushed the set-secret-token-globally-earlier-after-eager-load-hook branch from 365fa19 to 839fc1a Compare February 24, 2025 17:50
@agrare
Copy link
Member

agrare commented Feb 24, 2025

@jrafanie this method is still called but not defined

$ ruby lib/workers/bin/run_single_worker.rb MiqUiWorker

[----] E, [2025-02-24T13:20:15.923516#180210:8818] ERROR -- evm: MIQ(MiqWorker::Runner) ID [235] PID [180210] GUID [40b2c639-3200-4989-8fa0-5fe48bec3ad6] An unhandled error has occurred: undefined method `configure_secret_token' for class MiqUiWorker
/home/grare/adam/.gem/ruby/3.3.0/gems/activerecord-7.1.5.1/lib/active_record/dynamic_matchers.rb:22:in `method_missing'
/home/grare/adam/src/manageiq/manageiq/app/models/mixins/miq_web_server_runner_mixin.rb:15:in `run'
/home/grare/adam/src/manageiq/manageiq/app/models/miq_worker/runner.rb:103:in `start_rails_worker'
/home/grare/adam/src/manageiq/manageiq/app/models/miq_worker/runner.rb:98:in `start'
lib/workers/bin/run_single_worker.rb:128:in `<main>'


=> 16:     worker.class.configure_secret_token
   17:     start_rails_server(worker.rails_server_options)
   18: 
   19:     # when puma exits allow the heartbeat thread to exit cleanly using #do_exit
   20:     worker_thread.join
(byebug) worker.class.method(:configure_secret_token)
*** NameError Exception: undefined method `configure_secret_token' for class `#<Class:MiqUiWorker(id: integer, guid: string, status: string, started_on: datetime, stopped_on: datetime, last_heartbeat: datetime, pid: integer, queue_name: string, type: string, percent_memory: float, percent_cpu: float, cpu_time: float, os_priority: integer, memory_usage: decimal, memory_size: decimal, uri: string, miq_server_id: integer, sql_spid: integer, proportional_set_size: decimal, unique_set_size: decimal, system_uid: string, href_slug: string, region_number: integer, region_description: string, friendly_name: string, uri_or_queue_name: string)>'

Looks like just a couple more ✂️ 🔥 😜

@jrafanie jrafanie force-pushed the set-secret-token-globally-earlier-after-eager-load-hook branch from 839fc1a to 1d3dfee Compare February 24, 2025 18:22
We don't use secret_key_base like vanilla rails. We implemented secrets through
MiqPassword before any of this existed in rails.  Our only things currently going
through rails secrets perhaps is the provider vcr cassette information.

Rails 7.1 asserts secret_key_base is now set and set during rails boot fairly early.

So, now, we can't only set this for puma based workers, but all rails processes.
We also, must move this logic to be done earlier during boot.  It was found that
after eager load allows us to leverage autoload and access the database, both
are required in order to fetch the secrets from the database.

Move over the logic for rails console so it can use a dummy secret key base.

Move the tests to a vmdb/initializer test.

Co-Authored-By: Jason Frey <fryguy9@gmail.com>
@jrafanie jrafanie force-pushed the set-secret-token-globally-earlier-after-eager-load-hook branch from 1d3dfee to e6d6cd7 Compare February 24, 2025 18:24
@jrafanie
Copy link
Member Author

Looks like just a couple more ✂️ 🔥 😜

thanks... more 🔴 deletions

@agrare
Copy link
Member

agrare commented Feb 24, 2025

I was able to re-record a VCR cassette with this patch applied

@agrare agrare merged commit 9213a9d into ManageIQ:master Feb 24, 2025
8 checks passed
jrafanie added a commit to jrafanie/manageiq-rpm_build that referenced this pull request Feb 24, 2025
See: ManageIQ/manageiq#23351

We no longer need to do the change in ManageIQ#545 because it is set in core
for all rails processes in the new pull request above.
jrafanie added a commit to jrafanie/manageiq-rpm_build that referenced this pull request Feb 24, 2025
See: ManageIQ/manageiq#23351

We no longer need to do the change in ManageIQ#545 because it is set in core
for all rails processes in the new pull request above.
@jrafanie jrafanie deleted the set-secret-token-globally-earlier-after-eager-load-hook branch February 24, 2025 18:58
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.

2 participants