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

Allow rails to be loaded in fix auth #17413

Merged
merged 1 commit into from
May 30, 2018
Merged

Conversation

kbrock
Copy link
Member

@kbrock kbrock commented May 11, 2018

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

Even though we don't support storing non hash/list/string value in the database, it happens. A number of customers have recently run into this issue.

This provides a --resilient option to load our environment and allow those hashes to be deserialized.

see also Fine version #17428

@jdeubel
Copy link
Member

jdeubel commented May 11, 2018

@kbrock Since making the changes not able to run fix_auth. Recieving this error message:

[root@unused vmdb]# fix_auth --help
/opt/rh/cfme-gemset/gems/activesupport-5.0.3/lib/active_support/dependencies.rb:263:in `rescue in load_dependency': /var/www/miq/vmdb/tools/fix_auth/cli.rb:28: syntax error, unexpected tSTRING_BEG, expecting keyword_end (SyntaxError)
        opt :rails     "Load rails for those stuborn ...
                        ^
/var/www/miq/vmdb/tools/fix_auth/cli.rb:28: syntax error, unexpected ',', expecting keyword_end
... for those stuborn migrations", :type => :boolean, :short =>...
...                               ^
/var/www/miq/vmdb/tools/fix_auth/cli.rb:28: Can't assign to nil
...ype => :boolean, :short => nil, :default => false
...                               ^
/var/www/miq/vmdb/tools/fix_auth/cli.rb:28: syntax error, unexpected =>, expecting &. or :: or '[' or '.'
...ean, :short => nil, :default => false
...                               ^
	from /opt/rh/cfme-gemset/gems/activesupport-5.0.3/lib/active_support/dependencies.rb:256:in `load_dependency'
	from /opt/rh/cfme-gemset/gems/activesupport-5.0.3/lib/active_support/dependencies.rb:293:in `require'
	from tools/fix_auth.rb:23:in `<main>'

@kbrock kbrock changed the title allow rails to be loaded in fix auth [wip] allow rails to be loaded in fix auth May 14, 2018
@kbrock kbrock added the wip label May 14, 2018
@kbrock
Copy link
Member Author

kbrock commented May 14, 2018

Also of note

The require statement throws the following exception:

/var/www/miq/vmdb/config/environments/patches/database_configuration.rb:29:in `database_configuration': undefined method `decrypt_passwords!' for Vmdb::Settings:Class (NoMethodError)

@kbrock kbrock force-pushed the fix_auth_fixup branch 2 times, most recently from eeb7aad to 60126b7 Compare May 15, 2018 20:56
@kbrock
Copy link
Member Author

kbrock commented May 15, 2018

I'll kick again. unsure about the cops (invalid syntax) - that has already been fixed.
the spec failures look unrelated

@kbrock kbrock changed the title [wip] allow rails to be loaded in fix auth Allow rails to be loaded in fix auth May 15, 2018
@miq-bot miq-bot removed the wip label May 15, 2018
@kbrock kbrock force-pushed the fix_auth_fixup branch 8 times, most recently from c73ffcd to d2090f6 Compare May 18, 2018 20:06
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
@miq-bot
Copy link
Member

miq-bot commented May 21, 2018

Checked commit kbrock@88faab3 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
6 files checked, 2 offenses detected

tools/fix_auth/auth_model.rb

Copy link
Member

@carbonin carbonin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. @kbrock does this need to go back to gaprindashvili? (Only asking because you made a PR to fine)

Also is there a BZ associated with this change?

@carbonin carbonin self-assigned this May 30, 2018
@carbonin carbonin merged commit c174d0b into ManageIQ:master May 30, 2018
@carbonin carbonin added this to the Sprint 87 Ending Jun 4, 2018 milestone May 30, 2018
@@ -69,6 +69,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__)
Copy link
Member

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!

@kbrock I don't know that it matters but was curious since all the other tools load config/environment (since that's what I always do).

@jrafanie
Copy link
Member

@kbrock I need this to go back to at least gaprindashvili for #17410 otherwise, that PR will need to be manually backported to address conflicts

@kbrock
Copy link
Member Author

kbrock commented Oct 29, 2018

@simaishi Is it possible to backport this? to G?

@simaishi
Copy link
Contributor

@kbrock We need this in G for #17410 or something else?

@kbrock
Copy link
Member Author

kbrock commented Nov 3, 2018

@gmcculloug @mkanoor Do you all have a BZ that you would like referenced by this PR?
This was to help to get around automate objects serialized in the requests tables

@gmcculloug
Copy link
Member

@tinaafitz Can you help find a BZ for this? I know we've had BZs related to this but could use some help identifying them.

@tinaafitz
Copy link
Member

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.

8 participants