-
Notifications
You must be signed in to change notification settings - Fork 900
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
[Fine] Fixauth handles serialized objects #17428
Conversation
to access MiqPassword from a manageiq-gems-pending
tools/fix_auth/cli.rb
Outdated
@@ -24,6 +24,7 @@ def parse(args, env = {}) | |||
opt :databaseyml, "Rewrite database.yml", :type => :boolean, :short => "y", :default => false | |||
opt :db, "Upgrade database", :type => :boolean, :short => 'x', :default => false | |||
opt :legacy_key, "Legacy Key", :type => :string, :short => "K" | |||
opt :resilient, "Run through all records, even with errors", :type => :boolean, :short => nil, :default => false |
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 something like pg_restore's exit on error, but default it to true so it won't blindly continue on
-e
--exit-on-error
https://www.postgresql.org/docs/9.6/static/app-pgrestore.html
@jrafanie so if |
Good point. I don't like how trollop does |
f38b546
to
118034a
Compare
done |
tools/fix_auth/auth_config_model.rb
Outdated
puts "potentially bad yaml:" | ||
puts old_value | ||
unless options[:allow_failures] | ||
puts "potentially bad yaml:" |
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.
Would STDERR.puts be better?
tools/fix_auth/auth_model.rb
Outdated
rescue | ||
errors += 1 | ||
unless options[:allow_failures] | ||
puts "unable to fix #{r.class.table_name}:#{r.id}" if !options[:silent] |
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.
Same with STDERR
@@ -89,6 +93,7 @@ def run | |||
|
|||
generate_password if options[:key] | |||
fix_database_yml if options[:databaseyml] | |||
load_rails if options[:allow_failures] |
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 don't get why you load rails only when you allow failures
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.
@Fryguy we have never loaded rails before. As long as the database.yml is fixed (fix_database_yml
) I guess we could always load rails.
It certainly would mask some issues but would simplify this process (typically performed in a critical window).
fe4895a
to
e25e943
Compare
When objects are serialized into yaml blobs in tables, we need to load our whole environment to handle the deserialization This typically happens with miq_requests.options - add --allow-failures flag - add note where error was found - require standard rails when allow failures - continue despite errors when allow failures - display status, counts, and # errors - change order of fixes - reduce columns brought back - sending error output to STDERR - only bring back request/task records that have v2 encoded passwords - drop tests for v0 encoded passwords
Checked commits kbrock/manageiq@f044013~...dc11ba7 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 tools/fix_auth/auth_model.rb
|
@@ -75,6 +75,10 @@ def fix_database_yml | |||
FixDatabaseYml.run({:hardcode => options[:password]}.merge(run_options)) | |||
end | |||
|
|||
def load_rails | |||
require File.expand_path("../../../config/application.rb", __FILE__) |
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.
Don't you want to run the initializers? Typically, tools in the tools directory require config/environment to load rails, which is the same as above ^ but it also calls Vmdb::Application.initialize!
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 move this comment to the master PR
closing - this is too late to be backported to |
allow rails to be loaded in fix auth
When objects are serialized into yaml blobs in tables,
we need to load our whole environment to handle the deserialization
This typically happens with miq_requests.options
--allow-failures
flagrelated to #17413
Performance implication: For one customer, it is taking a multi hour process down to 3 minutes