diff --git a/.rubocop.yml b/.rubocop.yml index ee6079d0..e82b7c35 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -4,13 +4,16 @@ inherit_gem: inherit_from: .rubocop_todo.yml +inherit_mode: + merge: + - Exclude + Layout/ArgumentAlignment: EnforcedStyle: with_fixed_indentation IndentationWidth: 2 Rails/SkipsModelValidations: Exclude: - - 'db/migrate/**/*' - 'db/migrate_foreman/**/*' Style/FormatStringToken: diff --git a/app/controllers/concerns/foreman_puppet_enc/extensions/hostgroups_controller_extensions.rb b/app/controllers/concerns/foreman_puppet_enc/extensions/hostgroups_controller_extensions.rb new file mode 100644 index 00000000..7a35fcae --- /dev/null +++ b/app/controllers/concerns/foreman_puppet_enc/extensions/hostgroups_controller_extensions.rb @@ -0,0 +1,38 @@ +module ForemanPuppetEnc + module Extensions + module HostgroupsControllerExtensions + extend ActiveSupport::Concern + + PUPPET_AJAX_REQUESTS = %w[environment_selected puppetclass_parameters].freeze + + included do + alias_method :ajax_request_for_puppet_hostgroup_extensions, :ajax_request + alias_method :taxonomy_scope_for_puppet_hostgroup_extensions, :taxonomy_scope + + before_action :ajax_request_for_puppet_hostgroup_extensions, only: PUPPET_AJAX_REQUESTS + before_action :taxonomy_scope_for_puppet_hostgroup_extensions, only: PUPPET_AJAX_REQUESTS + + helper ForemanPuppetEnc::HostsAndHostgroupsHelper + end + + def environment_selected + env_id = params[:environment_id] || params[:hostgroup][:environment_id] + return not_found if env_id.to_i.positive? && !(@environment = Environment.find(env_id)) + + refresh_hostgroup + @hostgroup.environment = @environment if @environment + + @hostgroup.puppetclasses = Puppetclass.where(id: params[:hostgroup][:puppetclass_ids]) + @hostgroup.config_groups = ConfigGroup.where(id: params[:hostgroup][:config_group_ids]) + render partial: 'hosts/form_puppet_enc_tab', locals: { obj: @hostgroup, resource_type: :hostgroup } + end + + def puppetclass_parameters + Taxonomy.as_taxonomy @organization, @location do + render partial: 'foreman_puppet_enc/puppetclasses/classes_parameters', + locals: { obj: refresh_hostgroup } + end + end + end + end +end diff --git a/app/controllers/concerns/foreman_puppet_enc/extensions/hosts_controller_extensions.rb b/app/controllers/concerns/foreman_puppet_enc/extensions/hosts_controller_extensions.rb index 1664b9c5..74cf1e15 100644 --- a/app/controllers/concerns/foreman_puppet_enc/extensions/hosts_controller_extensions.rb +++ b/app/controllers/concerns/foreman_puppet_enc/extensions/hosts_controller_extensions.rb @@ -24,7 +24,7 @@ module HostsControllerExtensions set_callback :set_class_variables, :after, :set_puppet_class_variables - helper ForemanPuppetEnc::HostsHelper + helper ForemanPuppetEnc::HostsAndHostgroupsHelper end def hostgroup_or_environment_selected @@ -32,7 +32,7 @@ def hostgroup_or_environment_selected set_class_variables(@host) Taxonomy.as_taxonomy @organization, @location do if @environment || @hostgroup - render partial: 'puppetclasses/class_selection', locals: { obj: @host, resource_type: :host } + render partial: 'hosts/form_puppet_enc_tab', locals: { obj: @host, resource_type: :host } else logger.info 'environment_id or hostgroup_id is required to render puppetclasses' end @@ -41,7 +41,7 @@ def hostgroup_or_environment_selected def puppetclass_parameters Taxonomy.as_taxonomy @organization, @location do - render partial: 'puppetclasses/classes_parameters', locals: { obj: refresh_host } + render partial: 'foreman_puppet_enc/puppetclasses/classes_parameters', locals: { obj: refresh_host } end end diff --git a/app/helpers/foreman_puppet_enc/hosts_and_hostgroups_helper.rb b/app/helpers/foreman_puppet_enc/hosts_and_hostgroups_helper.rb new file mode 100644 index 00000000..dfacf7c9 --- /dev/null +++ b/app/helpers/foreman_puppet_enc/hosts_and_hostgroups_helper.rb @@ -0,0 +1,84 @@ +module ForemanPuppetEnc + module HostsAndHostgroupsHelper + UI.register_host_description do + multiple_actions_provider :puppet_host_multiple_actions + end + + # TODO: remove me - prevents the puppetclass tab duplication + def puppetclasses_tab(puppetclasses_receiver) + end + + def puppet_host_multiple_actions + if ForemanPuppetEnc.extracted_from_core? + [{ action: [_('Change Environment'), foreman_puppet_enc.select_multiple_environment_hosts_path], priority: 200 }] + else + [] + end + end + + def host_puppet_environment_field(form, select_options = {}, html_options = {}) + select_options = { + include_blank: true, + disable_button: _(::HostsAndHostgroupsHelper::INHERIT_TEXT), + disable_button_enabled: inherited_by_default?(:environment_id, form.object), + user_set: user_set?(:environment_id), + }.deep_merge(select_options) + + html_options = { + data: { + host: { + id: form.object.id, + }, + }, + }.deep_merge(html_options) + + puppet_environment_field( + form, + accessible_resource(form.object, :environment), + select_options, + html_options + ) + end + + def hostgroup_puppet_environment_field(form, select_options = {}, html_options = {}) + select_options = { + include_blank: blank_or_inherit_f(form, :environment), + }.deep_merge(select_options) + + puppet_environment_field( + form, + accessible_resource(form.object, :environment), + select_options, + html_options + ) + end + + def puppet_environment_field(form, environments_choice, select_options = {}, html_options = {}) + html_options = { + onchange: 'tfm.puppetEnc.hostForm.updatePuppetclasses(this)', + help_inline: :indicator, + }.deep_merge(html_options) + + select_f( + form, + :environment_id, + environments_choice, + :id, + :to_label, + select_options, + html_options + ) + end + + def puppetclasses_with_parameters_for(obj) + classes = obj.all_puppetclasses + ids = classes.reorder(nil).pluck(:id) + class_vars = ForemanPuppetEnc::PuppetclassLookupKey.reorder(nil) + .joins(:environment_classes) + .where(EnvironmentClass.arel_table[:puppetclass_id].in(ids)) + .distinct + .pluck('environment_classes.puppetclass_id') + classes.where(id: class_vars) + end + end +end diff --git a/app/helpers/foreman_puppet_enc/hosts_helper.rb b/app/helpers/foreman_puppet_enc/hosts_helper.rb deleted file mode 100644 index df827d4f..00000000 --- a/app/helpers/foreman_puppet_enc/hosts_helper.rb +++ /dev/null @@ -1,15 +0,0 @@ -module ForemanPuppetEnc - module HostsHelper - UI.register_host_description do - multiple_actions_provider :puppet_host_multiple_actions - end - - def puppet_host_multiple_actions - if ForemanPuppetEnc.extracted_from_core? - [{ action: [_('Change Environment'), foreman_puppet_enc.select_multiple_environment_hosts_path], priority: 200 }] - else - [] - end - end - end -end diff --git a/app/views/hosts/_form_puppet_enc_tab.html.erb b/app/views/hosts/_form_puppet_enc_tab.html.erb index b0682ab9..3aebbe2a 100644 --- a/app/views/hosts/_form_puppet_enc_tab.html.erb +++ b/app/views/hosts/_form_puppet_enc_tab.html.erb @@ -1,14 +1,23 @@ -<% obj = local_assigns[pagelet.opts[:resource_type]] %> +<% resource_type ||= pagelet.opts[:resource_type] %> +<% obj ||= local_assigns[resource_type] %> +<% if resource_type == :host %> + +<% elsif resource_type == :hostgroup %> + +<% end %> <% if @environment.present? || obj.environment.present? %> <% if accessible_resource_records(:config_group).exists? %> - <%= render 'foreman_puppet_enc/config_groups/config_groups_selection', obj: obj, resource_type: pagelet.opts[:resource_type] %> + <%= render 'foreman_puppet_enc/config_groups/config_groups_selection', obj: obj, resource_type: resource_type %> <% end %> - <%= render 'foreman_puppet_enc/puppetclasses/class_selection', obj: obj, resource_type: pagelet.opts[:resource_type] %> + <%= render 'foreman_puppet_enc/puppetclasses/class_selection', obj: obj, resource_type: resource_type %> <% else %> <%= alert(class: alert_class(:info), header: _('Notice'), text: _('Please select an environment first')) %> <% end %> - +
+

<%= _('Puppet Class Parameters') %>

+ <%= render 'foreman_puppet_enc/puppetclasses/classes_parameters', obj: obj %> +
diff --git a/app/views/hosts/foreman_puppet_enc/_form_main_tab_fields.html.erb b/app/views/hosts/foreman_puppet_enc/_form_main_tab_fields.html.erb new file mode 100644 index 00000000..75f43036 --- /dev/null +++ b/app/views/hosts/foreman_puppet_enc/_form_main_tab_fields.html.erb @@ -0,0 +1 @@ +<%= public_send("#{pagelet.opts[:resource_type]}_puppet_environment_field", form) %> diff --git a/config/routes.rb b/config/routes.rb index b58e8460..89cbfb44 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -26,10 +26,19 @@ resources :hosts, only: [], controller: '/hosts' do collection do + post 'hostgroup_or_environment_selected' + post 'puppetclass_parameters' match 'select_multiple_environment', via: %i[get post] post 'update_multiple_environment' end end + + resources :hostgroups, only: [], controller: '/hostgroups' do + collection do + post 'environment_selected' + post 'puppetclass_parameters' + end + end end Foreman::Application.routes.draw do diff --git a/lib/foreman_puppet_enc/engine.rb b/lib/foreman_puppet_enc/engine.rb index 5cbcfccd..8ae3c6c0 100644 --- a/lib/foreman_puppet_enc/engine.rb +++ b/lib/foreman_puppet_enc/engine.rb @@ -26,6 +26,7 @@ class Engine < ::Rails::Engine # To stay LookupValue.include ForemanPuppetEnc::PuppetLookupValueExtensions HostsController.include ForemanPuppetEnc::Extensions::HostsControllerExtensions + HostgroupsController.include ForemanPuppetEnc::Extensions::HostgroupsControllerExtensions end rake_tasks do diff --git a/lib/foreman_puppet_enc/register.rb b/lib/foreman_puppet_enc/register.rb index bf2af488..1ecff2b5 100644 --- a/lib/foreman_puppet_enc/register.rb +++ b/lib/foreman_puppet_enc/register.rb @@ -143,17 +143,32 @@ end end + unless ForemanPuppetEnc.extracted_from_core? + Rails.application.config.after_initialize do + list = Pagelets::Manager.instance.instance_variable_get(:@pagelets)['hosts/_form'][:main_tabs] + core_pagelet = list.detect { |pagelet| pagelet.opts[:id] == :puppet_klasses } + list.delete(core_pagelet) + end + end + # extend host(group) form with puppet ENC Tab %i[host hostgroup].each do |resource_type| host_onlyif = ->(host, context) { context.send(:accessible_resource, host, :smart_proxy, :name, association: :puppet_proxy).present? } extend_page("#{resource_type}s/_form") do |context| context.add_pagelet :main_tabs, - id: :puppet_klasses, + id: :puppet_enc_tab, name: N_('Puppet ENC'), partial: 'hosts/form_puppet_enc_tab', resource_type: resource_type, priority: 100, onlyif: (host_onlyif if resource_type == :host) + + if ForemanPuppetEnc.extracted_from_core? + context.add_pagelet :main_tab_fields, + partial: 'hosts/foreman_puppet_enc/form_main_tab_fields', + resource_type: resource_type, + priority: 100 + end end end end diff --git a/test/controllers/foreman_puppet_enc/hostgroups_controller_test.rb b/test/controllers/foreman_puppet_enc/hostgroups_controller_test.rb new file mode 100644 index 00000000..c329fd50 --- /dev/null +++ b/test/controllers/foreman_puppet_enc/hostgroups_controller_test.rb @@ -0,0 +1,73 @@ +require 'test_puppet_enc_helper' + +module ForemanPuppetEnc + class HostgroupsControllerTest < ActionController::TestCase + tests ::HostgroupsController + + setup do + @routes = ForemanPuppetEnc::Engine.routes + end + + describe '#environment_selected' do + setup do + @environment = FactoryBot.create(:environment) + @puppetclass = FactoryBot.create(:puppetclass) + @hostgroup = FactoryBot.create(:hostgroup, environment: @environment) + @params = { + id: @hostgroup.id, + hostgroup: { + name: @hostgroup.name, + environment_id: '', + puppetclass_ids: [@puppetclass.id], + }, + } + end + + test 'should return the selected puppet classes on environment change' do + assert_equal 0, @hostgroup.puppetclasses.length + + post :environment_selected, params: @params, session: set_session_user, xhr: true + assert_equal(1, assigns(:hostgroup).puppetclasses.length) + assert_include assigns(:hostgroup).puppetclasses, @puppetclass + end + + context 'environment_id param is set' do + test 'it will take the hostgroup params environment_id' do + other_environment = FactoryBot.create(:environment) + @params[:hostgroup][:environment_id] = other_environment.id + + post :environment_selected, params: @params, session: set_session_user, xhr: true + assert_equal assigns(:environment), other_environment + end + end + + test 'should not escape lookup values on environment change' do + skip 'Needs fully extracted puppet' unless ForemanPuppetEnc.extracted_from_core? + hostgroup = FactoryBot.create(:hostgroup, environment: @environment, puppetclass_ids: [@puppetclass.id]) + lookup_key = FactoryBot.create(:puppetclass_lookup_key, :array, default_value: %w[a b], + override: true, + puppetclass: @puppetclass, + overrides: { "hostgroup=#{hostgroup.name}" => %w[c d] }) + lookup_value = lookup_key.lookup_values.first + FactoryBot.create(:environment_class, puppetclass: @puppetclass, environment: @environment, puppetclass_lookup_key: lookup_key) + + # sending exactly what the host form would send which is lookup_value.value_before_type_cast + lk = { 'lookup_values_attributes' => { lookup_key.id.to_s => { 'value' => lookup_value.value_before_type_cast, + 'id' => lookup_value.id, + 'lookup_key_id' => lookup_key.id, + '_destroy' => false } } } + + params = { + hostgroup_id: hostgroup.id, + hostgroup: hostgroup.attributes.merge(lk), + } + + # environment change calls puppetclass_parameters which caused the extra escaping + post :puppetclass_parameters, params: params, session: set_session_user, xhr: true + + # if this was escaped during refresh_host the value in response.body after unescapeHTML would include "[\\\"c\\\",\\\"d\\\"]" + assert_includes CGI.unescapeHTML(response.body), '["c","d"]' + end + end + end +end diff --git a/test/controllers/foreman_puppet_enc/hosts_controller_test.rb b/test/controllers/foreman_puppet_enc/hosts_controller_test.rb index 1fc2e816..1e9901a7 100644 --- a/test/controllers/foreman_puppet_enc/hosts_controller_test.rb +++ b/test/controllers/foreman_puppet_enc/hosts_controller_test.rb @@ -18,8 +18,8 @@ class HostsControllerTest < ActionController::TestCase let(:environment2) { FactoryBot.create(:environment, organizations: [org], locations: [loc]) } let(:hostgroup) { FactoryBot.create(:hostgroup, environment: environment2, organizations: [org], locations: [loc]) } let(:host_defaults) { { hostgroup: hostgroup, environment: environment1, organization: org, location: loc } } - let(:host1) { FactoryBot.create(:host, host_defaults) } - let(:host2) { FactoryBot.create(:host, host_defaults) } + let(:host1) { FactoryBot.create(:host, :with_puppetclass, host_defaults) } + let(:host2) { FactoryBot.create(:host, :with_puppetclass, host_defaults) } test 'user with edit host rights with update environments should change environments' do @request.env['HTTP_REFERER'] = '/hosts' @@ -43,5 +43,78 @@ class HostsControllerTest < ActionController::TestCase assert_equal hostgroup.environment_id, host1.reload.environment_id assert_equal hostgroup.environment_id, host2.reload.environment_id end + + describe '#edit' do + setup { @routes = Rails.application.routes } + + test 'lookup value and description should be html escaped' do + skip 'Needs complete migration to be done' unless ForemanPuppetEnc.extracted_from_core? + host = FactoryBot.create(:host, :with_puppetclass) + FactoryBot.create(:puppetclass_lookup_key, + default_value: "", + description: "", + puppetclass: host1.puppetclasses.first) + get :edit, params: { id: host.to_param }, session: set_session_user + assert_not response.body.include?('