From 66ddc34a0a8c4d08c47394895e222fb6ef06e1b6 Mon Sep 17 00:00:00 2001
From: Timo Goebel <mail@timogoebel.name>
Date: Sun, 22 Apr 2018 12:48:23 +0200
Subject: [PATCH] fixes #23335 - normalize scsi attributes in rails 5

---
 .../foreman/controller/parameters/host.rb         |  7 ++++++-
 app/controllers/hosts_controller.rb               |  8 --------
 test/controllers/concerns/parameters/host_test.rb | 15 +++++++++++++--
 3 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/app/controllers/concerns/foreman/controller/parameters/host.rb b/app/controllers/concerns/foreman/controller/parameters/host.rb
index 7ff79dc4a95..dd9386fe46e 100644
--- a/app/controllers/concerns/foreman/controller/parameters/host.rb
+++ b/app/controllers/concerns/foreman/controller/parameters/host.rb
@@ -2,6 +2,7 @@ module Foreman::Controller::Parameters::Host
   extend ActiveSupport::Concern
   include Foreman::Controller::Parameters::HostBase
   include Foreman::Controller::Parameters::HostCommon
+  include Foreman::Controller::NormalizeScsiAttributes
 
   class_methods do
     def host_params_filter
@@ -31,6 +32,10 @@ def host_params_filter
   end
 
   def host_params(top_level_hash = controller_name.singularize)
-    self.class.host_params_filter.filter_params(params, parameter_filter_context, top_level_hash)
+    self.class.host_params_filter.filter_params(params, parameter_filter_context, top_level_hash).tap do |normalized|
+      if parameter_filter_context.ui? && normalized["compute_attributes"] && normalized["compute_attributes"]["scsi_controllers"]
+        normalize_scsi_attributes(normalized["compute_attributes"])
+      end
+    end
   end
 end
diff --git a/app/controllers/hosts_controller.rb b/app/controllers/hosts_controller.rb
index 079de55d480..814a0265d7f 100644
--- a/app/controllers/hosts_controller.rb
+++ b/app/controllers/hosts_controller.rb
@@ -9,7 +9,6 @@ class HostsController < ApplicationController
   include Foreman::Controller::HostFormCommon
   include Foreman::Controller::Puppet::HostsControllerExtensions
   include Foreman::Controller::CsvResponder
-  include Foreman::Controller::NormalizeScsiAttributes
   include Foreman::Controller::ConsoleCommon
 
   SEARCHABLE_ACTIONS= %w[index active errors out_of_sync pending disabled]
@@ -40,7 +39,6 @@ class HostsController < ApplicationController
   before_action :set_host_type, :only => [:update]
   before_action :find_multiple, :only => MULTIPLE_ACTIONS
   before_action :validate_power_action, :only => :update_multiple_power_state
-  before_action :normalize_vm_attributes, :only => [:create, :update, :process_taxonomy]
 
   helper :hosts, :reports, :interfaces
 
@@ -891,12 +889,6 @@ def csv_columns
     [:name, :operatingsystem, :environment, :compute_resource_or_model, :hostgroup, :last_report]
   end
 
-  def normalize_vm_attributes
-    if host_params["compute_attributes"] && host_params["compute_attributes"]["scsi_controllers"]
-      normalize_scsi_attributes(host_params["compute_attributes"])
-    end
-  end
-
   def origin_intervals_query(compare_with)
     reported_origin_interval_settings.map do |origin, interval|
       "(origin = #{origin} and last_report #{compare_with}  \"#{interval + Setting[:outofsync_interval]} minutes ago\")"
diff --git a/test/controllers/concerns/parameters/host_test.rb b/test/controllers/concerns/parameters/host_test.rb
index 54edbf535a6..5d9e26c3add 100644
--- a/test/controllers/concerns/parameters/host_test.rb
+++ b/test/controllers/concerns/parameters/host_test.rb
@@ -10,7 +10,7 @@ class HostParametersTest < ActiveSupport::TestCase
   test "passes through :compute_attributes hash untouched" do
     inner_params = {:name => 'test.example.com', :compute_attributes => {:foo => 'bar', :memory => 2}}
     expects(:params).at_least_once.returns(ActionController::Parameters.new(:host => inner_params))
-    expects(:parameter_filter_context).returns(context)
+    expects(:parameter_filter_context).at_least_once.returns(context)
     filtered = host_params
 
     assert_equal 'test.example.com', filtered['name']
@@ -20,11 +20,22 @@ class HostParametersTest < ActiveSupport::TestCase
   test "correctly passes through :interfaces_attributes :compute_attributes hash" do
     inner_params = {:name => 'test.example.com', :interfaces_attributes => [{:name => 'abc', :compute_attributes => {:type => 'awesome', :network => 'superawesome'}}]}
     expects(:params).at_least_once.returns(ActionController::Parameters.new(:host => inner_params))
-    expects(:parameter_filter_context).returns(ui_context)
+    expects(:parameter_filter_context).at_least_once.returns(ui_context)
     filtered = host_params
 
     assert_equal 'test.example.com', filtered['name']
     assert_equal 'abc', filtered['interfaces_attributes'][0][:name]
     assert_equal({'type' => 'awesome', 'network' => 'superawesome'}, filtered['interfaces_attributes'][0]['compute_attributes'].to_h)
   end
+
+  test 'normalizes json scsi attributes' do
+    inner_params = {:name => 'test.example.com', :compute_attributes => {"scsi_controllers"=>"{\"scsiControllers\":[{\"type\":\"VirtualLsiLogicController\",\"key\":1000}],\"volumes\":[{\"thin\":true,\"name\":\"Hard disk\",\"mode\":\"persistent\",\"controllerKey\":1000,\"size\":10485760,\"sizeGb\":10,\"storagePod\":\"Example-Pod\"}]}"}}
+    expects(:params).at_least_once.returns(ActionController::Parameters.new(:host => inner_params))
+    expects(:parameter_filter_context).at_least_once.returns(ui_context)
+    filtered = host_params
+
+    assert_equal 'test.example.com', filtered['name']
+    assert_equal [{"type"=>"VirtualLsiLogicController", "key"=>1000}], filtered['compute_attributes']['scsi_controllers']
+    assert_equal({"0"=>{"thin"=>true, "name"=>"Hard disk", "mode"=>"persistent", "controller_key"=>1000, "size"=>10485760, "size_gb"=>10, "storage_pod"=>"Example-Pod"}}, filtered['compute_attributes']['volumes_attributes'])
+  end
 end