Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ def get_environment_id(env_params)

def get_environment_for(host, id)
if id == 'inherit' && host.hostgroup.present?
host.hostgroup.environment
host.hostgroup.puppet&.environment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I guess, this is a issues which is solved within this PR?

else
ForemanPuppet::Environment.find_by(id: id)
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ module Api
module V2
class HostsBulkActionsController < ::ForemanPuppet::Api::V2::PuppetBaseController
include ::Api::V2::BulkHostsExtension
before_action :find_editable_hosts, only: %i[change_puppet_proxy remove_puppet_proxy]
before_action :find_editable_hosts, only: %i[change_environment change_puppet_proxy remove_puppet_proxy]
before_action :find_environment, only: %i[change_environment]
before_action :validate_environment_taxonomies, only: %i[change_environment]
before_action :find_smart_proxy, only: %i[change_puppet_proxy]

def_param_group :bulk_params do
Expand All @@ -17,6 +19,27 @@ class HostsBulkActionsController < ::ForemanPuppet::Api::V2::PuppetBaseControlle
end
end

api :PUT, '/foreman_puppet/api/v2/hosts/bulk/change_environment', N_('Change Puppet environment')
param_group :bulk_params
param :environment_id, String, required: false, desc: N_('ID of the Puppet environment to set for the selected hosts')
def change_environment
error_hosts = ::BulkHostsManager.new(hosts: @hosts).change_puppet_environment(resolved_environment)

process_bulk_response(
error_hosts,
success_message: format(n_(
'Updated host: changed environment',
'Updated hosts: changed environment',
@hosts.count
)),
error_message: format(n_(
'Failed to change environment for %{count} host',
'Failed to change environment for %{count} hosts',
error_hosts.count
), count: error_hosts.count)
)
end

api :PUT, '/hosts/bulk/change_puppet_proxy', N_('Change Puppet (CA) Proxy')
param_group :bulk_params
param :proxy_id, :number, required: true, desc: N_('ID of the Puppet proxy to reassign the hosts to')
Expand Down Expand Up @@ -65,6 +88,14 @@ def find_editable_hosts
end

def process_bulk_puppet_proxy_response(error_hosts, success_message:, error_message:)
process_bulk_response(
error_hosts,
success_message: success_message,
error_message: error_message
)
end

def process_bulk_response(error_hosts, success_message:, error_message:)
if error_hosts.empty?
process_response(true, { message: success_message })
else
Expand Down Expand Up @@ -93,6 +124,49 @@ def ca_proxy?
Foreman::Cast.to_bool(params[:ca_proxy])
end

def find_environment
return true if environment_value == 'inherit' || environment_value.blank?

@environment = ForemanPuppet::Environment.find_by(id: environment_value)
return true if @environment.present?

render json: {
error: {
message: format(_('A Puppet environment with id %{id} could not be found.'), id: environment_value),
},
}, status: :unprocessable_entity
false
end

def validate_environment_taxonomies
return true if resolved_environment.blank? || resolved_environment == 'inherit'

invalid_host_ids = @hosts.reject do |host|
ForemanPuppet::Environment.with_taxonomy_scope(host.organization, host.location)
.exists?(id: resolved_environment.id)
end.map(&:id)

return true if invalid_host_ids.empty?

render_error(:bulk_hosts_error, status: :unprocessable_entity,
locals: {
message: _('Selected Puppet environment is not assigned to the proper organization and/or location for all hosts.'),
failed_host_ids: invalid_host_ids,
})
false
end

def environment_value
params[:environment_id]
end

def resolved_environment
return 'inherit' if environment_value == 'inherit'
return nil if environment_value.blank?

@environment
end

def proxy_type
ca_proxy? ? _('Puppet CA proxy') : _('Puppet proxy')
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,24 @@ module Extensions
module BulkHostsManager
extend ActiveSupport::Concern

def change_puppet_environment(environment)
error_hosts = []
@hosts.each do |host|
puppet = host.puppet || host.build_puppet
puppet.environment = if environment == 'inherit'
host.hostgroup&.puppet&.environment
else
environment
end
host.save(validate: false)
rescue StandardError => e
message = format(_('Failed to set Puppet environment for %{host}.'), host: host)
Foreman::Logging.exception(message, e)
error_hosts << host.id
end
error_hosts
end

def change_puppet_proxy(proxy, is_ca_proxy)
error_hosts = []
@hosts.each do |host|
Expand Down
2 changes: 2 additions & 0 deletions config/api_routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
ForemanPuppet::Engine.routes.draw do
namespace :api, defaults: { format: 'json' } do
scope '(:apiv)', module: :v2, defaults: { apiv: 'v2' }, apiv: /v1|v2/, constraints: ApiConstraints.new(version: 2, default: true) do
match 'hosts/bulk/change_environment', to: 'hosts_bulk_actions#change_environment', via: [:put]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

wouldn't it be better to move this route to the bulk action area in line 5?


constraints(id: %r{[^/]+}) do
resources :config_groups, except: %i[new edit]

Expand Down
1 change: 1 addition & 0 deletions lib/foreman_puppet/register.rb
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
p.actions << 'hosts/update_multiple_environment'
p.actions << 'hosts/select_multiple_puppet_proxy'
p.actions << 'hosts/update_multiple_puppet_proxy'
p.actions << 'foreman_puppet/api/v2/hosts_bulk_actions/change_environment'
p.actions << 'foreman_puppet/api/v2/hosts_bulk_actions/change_puppet_proxy'
p.actions << 'foreman_puppet/api/v2/hosts_bulk_actions/remove_puppet_proxy'
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,105 @@ class HostsBulkActionsControllerTest < ActionController::TestCase
organization: host.organization,
location: host.location)
end
let(:environment) { FactoryBot.create(:environment, organizations: [host.organization], locations: [host.location]) }
let(:hostgroup_environment) { FactoryBot.create(:environment, organizations: [host.organization], locations: [host.location]) }
let(:hostgroup) do
FactoryBot.create(:hostgroup, :with_puppet_enc,
environment: hostgroup_environment,
organizations: [host.organization],
locations: [host.location])
end
let(:proxy) { FactoryBot.create(:puppet_and_ca_smart_proxy, organizations: [host.organization], locations: [host.location]) }

def put_change_environment(params:, session: nil)
original_routes = @routes
@routes = ForemanPuppet::Engine.routes
put :change_environment, params: params, session: session
ensure
@routes = original_routes
end

test 'changes puppet environment for selected hosts' do
put_change_environment(params: bulk_params.merge(environment_id: environment.id))

assert_response :success
assert_equal environment.id, host2.reload.puppet.environment_id
assert_equal environment.id, host.reload.puppet.environment_id
end

test 'inherits puppet environment from hostgroups for selected hosts' do
host.update!(hostgroup: hostgroup)
host2.update!(hostgroup: hostgroup)

put_change_environment(params: bulk_params.merge(environment_id: 'inherit'))

assert_response :success
assert_equal hostgroup_environment.id, host2.reload.puppet.environment_id
assert_equal hostgroup_environment.id, host.reload.puppet.environment_id
end

test 'clears puppet environment for selected hosts' do
host.puppet.update!(environment: environment)
host2.puppet.update!(environment: environment)

put_change_environment(
params: bulk_params.merge(environment_id: nil),
session: set_session_user
)

assert_response :success
assert_nil host.reload.puppet.environment
assert_nil host2.reload.puppet.environment
end

test 'returns error when puppet environment is missing' do
missing_environment_id = 999_999

put_change_environment(
params: bulk_params.merge(environment_id: missing_environment_id),
session: set_session_user
)

assert_response :unprocessable_entity
response = JSON.parse(@response.body)
assert_equal "A Puppet environment with id #{missing_environment_id} could not be found.",
response.dig('error', 'message')
end

test 'returns error when changing puppet environment fails for some hosts' do
::BulkHostsManager.any_instance.expects(:change_puppet_environment)
.with(environment)
.returns([host2.id])

put_change_environment(
params: bulk_params.merge(environment_id: environment.id),
session: set_session_user
)

assert_response :unprocessable_entity
response = JSON.parse(@response.body)
assert_equal 'Failed to change environment for 1 host',
response.dig('error', 'message')
assert_equal [host2.id], response.dig('error', 'failed_host_ids')
end

test 'returns error when puppet environment is outside host taxonomies' do
invalid_environment = FactoryBot.create(:environment,
organizations: [host.organization],
locations: [FactoryBot.create(:location)])

put_change_environment(
params: bulk_params.merge(environment_id: invalid_environment.id),
session: set_session_user
)

assert_response :unprocessable_entity
response = JSON.parse(@response.body)
assert_equal 'Selected Puppet environment is not assigned to the proper organization and/or location for all hosts.',
response.dig('error', 'message')
assert_equal [host.id, host2.id].sort, response.dig('error', 'failed_host_ids').sort
end

test 'changes puppet proxy for selected hosts' do
put :change_puppet_proxy,
params: bulk_params.merge(proxy_id: proxy.id, ca_proxy: false)
Expand Down
40 changes: 40 additions & 0 deletions test/services/foreman_puppet/bulk_hosts_manager_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,46 @@ class BulkHostsManagerTest < ActiveSupport::TestCase
let(:hosts) { FactoryBot.create_list(:host, 2, :with_puppet_enc) }
let(:manager) { ::BulkHostsManager.new(hosts: hosts) }
let(:proxy) { FactoryBot.create(:puppet_smart_proxy) }
let(:environment) { FactoryBot.create(:environment, organizations: [hosts.first.organization], locations: [hosts.first.location]) }

test 'changes puppet environment for hosts' do
manager.change_puppet_environment(environment)

hosts.each do |host|
assert_equal environment.id, host.reload.puppet.environment_id
end
end

test 'inherits puppet environment from hostgroup' do
organization = FactoryBot.create(:organization)
location = FactoryBot.create(:location)
inherited_environment = FactoryBot.create(:environment, organizations: [organization], locations: [location])
hostgroup = FactoryBot.create(:hostgroup, :with_puppet_enc,
environment: inherited_environment,
organizations: [organization],
locations: [location])
inherited_hosts = FactoryBot.create_list(:host, 2, :with_puppet_enc,
environment: inherited_environment,
hostgroup: hostgroup,
organization: organization,
location: location)

::BulkHostsManager.new(hosts: inherited_hosts).change_puppet_environment('inherit')

inherited_hosts.each do |host|
assert_equal inherited_environment.id, host.reload.puppet.environment_id
end
end

test 'clears puppet environment when environment is nil' do
hosts.each { |host| host.puppet.update!(environment: environment) }

manager.change_puppet_environment(nil)

hosts.each do |host|
assert_nil host.reload.puppet.environment
end
end

test 'changes puppet proxy for hosts' do
manager.change_puppet_proxy(proxy, false)
Expand Down
19 changes: 18 additions & 1 deletion webpack/global_index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,15 @@ import reducers from './src/reducers';
import { registerFills } from './src/Extends/Fills';
import { registerLegacy } from './legacy';
import HostsIndexActionsBar from './src/Extends/Hosts/ActionsBar';
import BulkChangePuppetEnvironment from './src/Extends/Hosts/BulkActions/BulkChangePuppetEnvironment';
import BulkRemovePuppetEnvironment from './src/Extends/Hosts/BulkActions/BulkRemovePuppetEnvironment';
import BulkChangePuppetProxy from './src/Extends/Hosts/BulkActions/BulkChangePuppetProxy';
import BulkChangePuppetCAProxy from './src/Extends/Hosts/BulkActions/BulkChangePuppetCAProxy';
import BulkRemovePuppetProxy from './src/Extends/Hosts/BulkActions/BulkRemovePuppetProxy';
import BulkRemovePuppetCAProxy from './src/Extends/Hosts/BulkActions/BulkRemovePuppetCAProxy';

const GLOBAL_FILL_PRIORITY = 100;
const PUPPET_ENV_COLUMN_WEIGHT = 2700;

// register reducers
registerReducer('puppet', reducers);
Expand All @@ -28,7 +31,7 @@ const puppetHostsIndexColumns = [
title: __('Puppet env'),
isSorted: true,
wrapper: hostDetails => hostDetails.environment_name,
weight: 2700,
weight: PUPPET_ENV_COLUMN_WEIGHT,
},
];

Expand All @@ -45,6 +48,20 @@ addGlobalFill(
GLOBAL_FILL_PRIORITY
);

addGlobalFill(
'_all-hosts-modals',
'BulkChangePuppetEnvironment',
<BulkChangePuppetEnvironment key="bulk-change-puppet-environment" />,
GLOBAL_FILL_PRIORITY
);

addGlobalFill(
'_all-hosts-modals',
'BulkRemovePuppetEnvironment',
<BulkRemovePuppetEnvironment key="bulk-remove-puppet-environment" />,
GLOBAL_FILL_PRIORITY
);

addGlobalFill(
'_all-hosts-modals',
'BulkChangePuppetProxy',
Expand Down
20 changes: 20 additions & 0 deletions webpack/src/Extends/Hosts/ActionsBar/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,26 @@ const HostActionsBar = () => {
<Menu ouiaId="content-flyout-menu" onSelect={() => setMenuOpen(false)}>
<MenuContent>
<MenuList>
<MenuItem
itemId="bulk-change-puppet-environment-menu-item"
key="bulk-change-puppet-environment-menu-item"
onClick={() =>
handleOpenBulkModal('bulk-change-puppet-environment')
}
isDisabled={selectedCount === 0}
>
{__('Change Puppet environment')}
</MenuItem>
<MenuItem
itemId="bulk-remove-puppet-environment-menu-item"
key="bulk-remove-puppet-environment-menu-item"
onClick={() =>
handleOpenBulkModal('bulk-remove-puppet-environment')
}
isDisabled={selectedCount === 0}
>
{__('Remove Puppet environment')}
</MenuItem>
<MenuItem
itemId="bulk-change-puppet-proxy-menu-item"
key="bulk-change-puppet-proxy-menu-item"
Expand Down
Loading
Loading