Do not pass in all action controller params to raw_connect

[jntullo]

Currently, validation on the queue is sending all action controller params to raw_connect, which are ending up in the log. We should not be sending all action controller params to the raw_connect method. This update makes them into a hash and excludes the unnecessary parameters.

Before:

[----] I, [2017-12-07T15:58:52.312693 #19968:2b156f45a010]  INFO -- : MIQ(MiqQueue.put) Message id: [109180],  id: [], Zone: [default], Role: [ems_operations], Server: [], Ident: [generic], Target id: [], Instance id: [], Task id: [], Command: [ManageIQ::Providers::Openstack::CloudManager.raw_connect], Timeout: [600], Priority: [100], State: [ready], Deliver On: [], Data: [], Args: ["********", <ActionController::Parameters {"utf8"=>"✓", "authenticity_token"=>"+c/4Y7PVtO==", "button"=>"validate", "cred_type"=>"default", "name"=>"name", "emstype"=>"openstack", "api_version"=>"v3", "provider_region"=>"", "keystone_v3_domain_id"=>"default", "provider_id"=>"", "zone"=>"default", "default_security_protocol"=>"non-ssl", "default_hostname"=>"107", "default_api_port"=>"0", "default_userid"=>"admin", "event_stream_selection"=>"ceilometer", "controller"=>"ems_cloud", "action"=>"create"} permitted: false>]

After:

[----] I, [2017-12-12T10:08:33.365891 #80964:3fee3c3360ac]  INFO -- : MIQ(MiqQueue.put) Message id: [10000066183456],  id: [], Zone: [default], Role: [ems_operations], Server: [], Ident: [generic], Target id: [], Instance id: [], Task id: [], Command: [ManageIQ::Providers::Openstack::CloudManager.raw_connect], Timeout: [600], Priority: [100], State: [ready], Deliver On: [], Data: [], Args: ["********", {:cred_type=>"default", :name=>"testing", :emstype=>"openstack", :api_version=>"v2", :provider_region=>"region", :provider_id=>"", :zone=>"default", :default_security_protocol=>"ssl", :default_hostname=>"test", :default_api_port=>"13000", :default_userid=>"admin", :event_stream_selection=>"ceilometer"}]

Could I get some 👀 from the OpenStack team? @aufi
@miq-bot assign @Fryguy

related to ManageIQ/manageiq#16636

[aufi]

Should be okay 👍

[Fryguy]

LGTM, but I am not an expert here. @martinpovolny @himdel Please review. I'm not sure you have a preferable method (maybe something with Strong Params directly?)

[himdel]

Looking at the code, every other case specifies exactly which params should go in - not which shouldn't.

So.. while I have no strong objections to this fix.... any chance you can make it a whitelist instead?

[jntullo]

@aufi if you don't mind adding your 👀 again - I've whitelisted the openstack params. I looked here for the ones needed

[miq-bot]

Checked commit jntullo@748a4ef with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 1 offense detected

app/controllers/mixins/ems_common_angular.rb

• ❗ - Line 141, Col 57 - Style/RedundantParentheses - Don't use parentheses around a constant.

[Fryguy]

Provider-specific code in the repo is an immediate red-flag for me. Any way to make this pluggable or somehow part of the openstack provider itself? @himdel Even though you prefer whitelist, for pluggability would blacklist be better? The UI should know what fields definitely should not go to the backend.

[himdel]

@Fryguy You're completely right, but that pretty much depends on #3097 ... in the current form, we can't really do that :(.

If you care about not passing in specific items, we can always do both whitelisting and blacklisting - but probably no point now, since it would be different per-provider anyway now.

(The point is, if we don't whitelist, they'll get random fields intended for other provider types, possibly even filled in with data if the user originally chose a different type. If we whitelist per provider, then we at least know what they need.)

[himdel]

(The hope with #3097 is that we can eventually make the provider know which fields it needs and provide it in a sane way .. but with this PR being done, that would just mean moving OPENSTACK_PARAMS over to the model .. but we first need to unravel the EmsCommon mess for that to be useful.)

[himdel]

LGTM, waiting for @aufi to get a chance to review

[martinpovolny]

@aufi: please, take a look. Merging it now, but your feedback will be addressed in a follow-up PR if you have any. Thx!

[simaishi]

Gaprindashvili backport details:

$ git log -1
commit 85bc07378daaa269152f8da0b037bb8970d93f4a
Author: Martin Povolny <[email protected]>
Date:   Sat Dec 23 12:44:24 2017 +0100

    Merge pull request #3035 from jntullo/remove_action_controller_params
    
    Do not pass in all action controller params to raw_connect
    (cherry picked from commit 0a2efb0658e6aea88caf0c93db689d593affc6b0)