Update fix_auth verbiage #23382

kbrock · 2025-03-18T13:58:30Z

Display the Database

I think most users expect it to change the one specified in database.yml We will have to change this one day. Until then, it is now displaying it

Message around the number of records actually changed.

We see processed, but it is not clear if anything actually happened. This is fixed

Update usage

Usage has removed [...] since we no longer handle multiple databases and it was distracting. Removed ruby since it is no longer required.

New Output

# ./tools/fix_auth.rb --help
Usage: fix_auth.rb [options] database            # to migrate from a different or lost key
       fix_auth.rb -P new_password --databaseyml # to fix database.yml
       fix_auth.rb --key                         # to generate a new certs/v2_key
  -v, --verbose           Verbose
  -d, --dry-run           Dry Run
  -h, --hostname=<s>      Database Hostname (default: localhost)
  -o, --port=<i>          Database Port (default: 5432)
  -U, --username=<s>      Database Username (default: postgres)
  -p, --password=<s>      Database Password (default: postgres)
  -P, --hardcode=<s>      Password used to replace all passwords
  -i, --invalid=<s>       Password used to replace non-decryptable passwords
  -k, --key               Generate key
  -f, --v2                ignored, available for backwards compatibility
  -r, --root=<s>          Rails Root (default: /Users/kbrock/src/manageiq)
  -y, --databaseyml       Fix database.yml
  -x, --db                Upgrade database
  -K, --legacy-key=<s>    Key used to decrypt old passwords when migrating to new key
  -a, --allow-failures    Run through all records, even with errors
  -e, --help              Show this message

# ./tools/fix_auth.rb
Error: please specify a database as an argument.
Try --help for help.

(couldn't figure out how to auto print the usage


** fix_database_passwords is executing in dry-run mode, and no actual changes will be made **
processing database: vmdb_development_fail_v2
fixing authentications.password, auth_key
0 of 8 records would change (dry run enabled)
fixing configuration_scripts.credentials
0 of 0 records would change (dry run enabled)
fixing miq_databases.session_secret_token, csrf_secret_token
0 of 1 records would change (dry run enabled)
fixing miq_ae_values.value
0 of 0 records would change (dry run enabled)
fixing miq_ae_fields.default_value
0 of 0 records would change (dry run enabled)
fixing settings_changes.value
0 of 1 records would change (dry run enabled)
fixing miq_requests.options
0 of 0 records would change (dry run enabled)
fixing miq_request_tasks.options
0 of 0 records would change (dry run enabled)

jrafanie · 2025-03-18T14:46:40Z

tools/fix_auth/auth_model.rb

@@ -127,9 +127,8 @@ def run(options = {})
            puts "processed #{processed} with #{errors} errors"
          end
        end
-        puts "#{options[:dry_run] ? "viewed" : "processed"} #{processed} records" unless options[:silent]
+        puts "#{options[:dry_run] ? "will change" : "changed"} #{records_changed} of #{processed} records" unless options[:silent]


My brain melts with the embedded double quotes within the interpolated strings. Can you use single quotes instead?

The unless is easy to miss. I know it's how it was previously so it can be changed as a followup but maybe something like this would be easier for me to grok:

unless options[:silent] puts "#{options[:dry_run] ? 'will change' : 'changed'} #{records_changed} of #{processed} records" end

changed 5 of 10 records
will change 5 of 10 records

Maybe "would change" would be better?

Should we flip the order:

5 of 10 records changed
5 of 10 records would change (dry run enabled)

jrafanie · 2025-03-18T14:48:24Z

tools/fix_auth/fix_auth.rb

@@ -67,6 +67,9 @@ def fix_database_passwords
        ActiveRecord::Base.logger = Logger.new("#{options[:root]}/log/fix_auth.log")
        ActiveRecord::Base.establish_connection(db_attributes(database))
      end
+
+      puts "database: #{options[:database]}"


Does this get lost when the remaining messages are written to the screen from the different models?

Can you post a sample of what it looks like now?

Thanks!

Yeah I think it at least needs a verb...something like

Suggested change

puts "database: #{options[:database]}"

puts "Processing database: #{options[:database]}"

I did try changing all of these to upper case, but it was harder for me to read for some reason.

I'll add "processing" and see if that helps.
Will also update the usage up top.

tools/fix_auth/cli.rb

update the usage statement to make database name more clear

- Display the Database I think most users expect it to change the one specified in database.yml We will have to change this one day. Until then, it is now displaying it - Message around the number of records actually changed. We see processed, but it is not clear if anything actually happened. This is fixed

tools/fix_auth/cli.rb

Fryguy

LGTM

kbrock · 2025-03-21T13:58:12Z

update:

added File.basename, better comments for usage
added processing
require database name when RAILS_ENV is not set (using as a way to detect the appliance / production mode - may want to compare with production - was trying to make as minimal as possible)

update:

s/Weh/When/

jrafanie · 2025-03-21T18:50:23Z

Changes look good. I'm trying it locally.

jrafanie

LGTM

% ./tools/fix_auth.rb vmdb_demo_env_03132025 -h localhost -x -i smartvm -v
** override_gem("byebug", :require=>"byebug") at /Users/joerafaniello/.bundler.d/byebug.rb:1
processing database: vmdb_demo_env_03132025
fixing authentications.password, auth_key
  3:
    password: "v2:{.../aE}" => v2:{...Q==} HARDCODED (WAS INVALID)
  2:
    password: "v2:{...g==}" => v2:{....Q==} HARDCODED (WAS INVALID)
  17:
    password: "v2:{...A==}" => v2:{.Q==} HARDCODED (WAS INVALID)
  18:
    auth_key: "v2:{...B6v}" => v2:{...Q==} HARDCODED (WAS INVALID)
  24:
    password: "v2:{...w==}" => v2:{...Q==} HARDCODED (WAS INVALID)
  28:
    password: "v2:{...w==}" => v2:{...Q==} HARDCODED (WAS INVALID)
  29:
    auth_key: "v2:{...g==}" => v2:{...Q==} HARDCODED (WAS INVALID)
  40:
    auth_key: "v2:{...g==}" => v2:{...Q==} HARDCODED (WAS INVALID)
8 of 8 records changed
fixing configuration_scripts.credentials
0 of 0 records changed
fixing miq_databases.session_secret_token, csrf_secret_token
  1:
    session_secret_token: "v2:{...4d6}" => v2:{...H/VH}
    csrf_secret_token: "v2:{...QuP}" => v2:{...gWw}
1 of 1 records changed
fixing miq_ae_values.value
0 of 0 records changed
fixing miq_ae_fields.default_value
0 of 0 records changed
fixing settings_changes.value
  24:
    value: "v2:{...g==}" => v2:{...Q==} HARDCODED (WAS INVALID)
1 of 1 records changed
fixing miq_requests.options
0 of 0 records changed
fixing miq_request_tasks.options
0 of 0 records changed

jrafanie · 2025-03-21T19:01:14Z

Nice work @kbrock, thanks!

kbrock added the documentation label Mar 18, 2025

kbrock assigned jrafanie Mar 18, 2025

kbrock requested a review from Fryguy as a code owner March 18, 2025 13:58

jrafanie reviewed Mar 18, 2025

View reviewed changes

kbrock force-pushed the v2_key_comments branch from cd99d05 to 7d41fcc Compare March 18, 2025 19:23

Fryguy reviewed Mar 20, 2025

View reviewed changes

tools/fix_auth/cli.rb Outdated Show resolved Hide resolved

kbrock added 2 commits March 20, 2025 16:00

Update usage for fix_auth

8bf3553

update the usage statement to make database name more clear

kbrock force-pushed the v2_key_comments branch from 5606c92 to 3ce218e Compare March 21, 2025 13:53

Fryguy reviewed Mar 21, 2025

View reviewed changes

tools/fix_auth/cli.rb Outdated Show resolved Hide resolved

Require database name in development

a34a0dc

kbrock force-pushed the v2_key_comments branch from 3ce218e to a34a0dc Compare March 21, 2025 13:56

Fryguy approved these changes Mar 21, 2025

View reviewed changes

jrafanie approved these changes Mar 21, 2025

View reviewed changes

jrafanie merged commit 2d184e2 into ManageIQ:master Mar 21, 2025
8 checks passed

kbrock deleted the v2_key_comments branch March 25, 2025 19:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update fix_auth verbiage #23382

Update fix_auth verbiage #23382

kbrock commented Mar 18, 2025 •

edited

Loading

jrafanie Mar 18, 2025

jrafanie Mar 18, 2025

Fryguy Mar 20, 2025

kbrock Mar 20, 2025 •

edited

Loading

Fryguy left a comment

kbrock commented Mar 21, 2025

jrafanie commented Mar 21, 2025

jrafanie left a comment

jrafanie commented Mar 21, 2025

	puts "database: #{options[:database]}"
	puts "Processing database: #{options[:database]}"

Update fix_auth verbiage #23382

Update fix_auth verbiage #23382

Conversation

kbrock commented Mar 18, 2025 • edited Loading

Display the Database

Message around the number of records actually changed.

Update usage

New Output

jrafanie Mar 18, 2025

Choose a reason for hiding this comment

jrafanie Mar 18, 2025

Choose a reason for hiding this comment

Fryguy Mar 20, 2025

Choose a reason for hiding this comment

kbrock Mar 20, 2025 • edited Loading

Choose a reason for hiding this comment

Fryguy left a comment

Choose a reason for hiding this comment

kbrock commented Mar 21, 2025

jrafanie commented Mar 21, 2025

jrafanie left a comment

Choose a reason for hiding this comment

jrafanie commented Mar 21, 2025

kbrock commented Mar 18, 2025 •

edited

Loading

kbrock Mar 20, 2025 •

edited

Loading