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.

Sign up for GitHub

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

Jump to bottom

Fixes #36688 - Provide option to use wget for the new Register Host feature #9808

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Fixes #36688 - Provide option to use wget for the new Register Host feature #9808

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions .eslintrc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@
"vnc",
"vnic",
"webpack",
"wget",
"wss",
"x86_64",
"xml",
Expand Down
3 changes: 2 additions & 1 deletion app/controllers/api/v2/registration_commands_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class RegistrationCommandsController < V2::BaseController
param :setup_insights, :bool, desc: N_("Set 'host_registration_insights' parameter for the host. If it is set to true, insights client will be installed and registered on Red Hat family operating systems")
param :setup_remote_execution, :bool, desc: N_("Set 'host_registration_remote_execution' parameter for the host. If it is set to true, SSH keys will be installed on the host")
param :jwt_expiration, :number, desc: N_("Expiration of the authorization token (in hours), 0 means 'unlimited'.")
param :insecure, :bool, desc: N_("Enable insecure argument for the initial curl")
param :insecure, :bool, desc: N_("Enable insecure argument for the initial curl/wget")
param :packages, String, desc: N_("Packages to install on the host when registered. Can be set by `host_packages` parameter, example: `pkg1 pkg2`")
param :update_packages, :bool, desc: N_("Update all packages on the host")
param :repo, String, desc: N_("DEPRECATED, use the `repo_data` param instead."), deprecated: true
Expand All @@ -25,6 +25,7 @@ class RegistrationCommandsController < V2::BaseController
param :repo, String, desc: N_("Repository URL / details, for example, for Debian OS family: 'deb http://deb.example.com/ buster 1.0', for Red Hat and SUSE OS family: 'http://yum.theforeman.org/client/latest/el8/x86_64/'")
param :repo_gpg_key_url, String, desc: N_("URL of the GPG key for the repository")
end
param :download_utility, ["curl", "wget"], desc: N_("The download utility to use during host registration")
end
def create
unless os_with_template?
Expand Down
1 change: 1 addition & 0 deletions app/controllers/api/v2/registration_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ class RegistrationController < V2::BaseController
param :repo, String, desc: N_("Repository URL / details, for example, for Debian OS family: 'deb http://deb.example.com/ buster 1.0', for Red Hat OS family: 'http://yum.theforeman.org/client/latest/el8/x86_64/'")
param :repo_gpg_key_url, String, desc: N_("URL of the GPG key for the repository")
end
param :download_utility, ["curl", "wget"], desc: N_("The download utility to use during host registration")
def global
find_global_registration

Expand Down
1 change: 1 addition & 0 deletions app/controllers/concerns/foreman/controller/registration.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ def global_registration_vars
packages: params['packages'],
update_packages: params['update_packages'],
repo_data: repo_data,
download_utility: params['download_utility'],
}

params.permit(permitted)
Expand Down
10 changes: 7 additions & 3 deletions app/controllers/concerns/foreman/controller/registration_commands.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ module Foreman::Controller::RegistrationCommands

def command
args_query = "?#{registration_args.to_query}"
"set -o pipefail && curl -sS #{insecure} '#{registration_url(@smart_proxy)}#{args_query if args_query != '?'}' #{command_headers} | bash"
"set -o pipefail && #{utility[:download_command]} #{utility[:output_pipe]} #{insecure} '#{registration_url(@smart_proxy)}#{args_query if args_query != '?'}' #{command_headers} | bash"
end

def registration_args
Expand All @@ -19,8 +19,12 @@ def registration_args
.permit!
end

def utility
Foreman.download_utilities.fetch(registration_params['download_utility'] || 'curl')
end

def insecure
registration_params['insecure'] ? '--insecure' : ''
registration_params['insecure'] ? utility[:insecure] : ''
end

def registration_url(proxy = nil)
Expand Down Expand Up @@ -65,7 +69,7 @@ def command_headers
else
invalid_expiration_error
end
"-H 'Authorization: Bearer #{User.current.jwt_token!(**jwt_args)}'"
"--header 'Authorization: Bearer #{User.current.jwt_token!(**jwt_args)}'"
end

def host_config_params
Expand Down
3 changes: 2 additions & 1 deletion app/services/foreman/renderer/configuration.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ class Configuration
:host_puppet_environment,
:host_enc,
:install_packages,
:update_packages
:update_packages,
:generate_web_request
]

DEFAULT_ALLOWED_VARIABLES = [
Expand Down
26 changes: 26 additions & 0 deletions app/services/foreman/renderer/scope/macros/helpers.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,32 @@ def truthy?(value = nil)
def falsy?(value = nil)
Foreman::Cast.to_bool(value) == false
end

apipie :method, 'Generate a web request' do
keyword :utility, String, desc: 'The utility to use for the web request ("curl" or "wget").'
keyword :url, String, desc: 'The URL for the web request.'
keyword :ssl_ca_cert, String, desc: 'The path to the SSL certificate.', default: nil
keyword :headers, Array, of: String, desc: 'Array containing the headers for the web request starting with "--header".', default: nil
keyword :params, Array, of: String, desc: 'Array containing the POST parameters for the web request.', default: nil
keyword :output_file, String, desc: 'The path were the result of the web request will be stored.', default: nil
returns String, desc: 'The web request.'
example 'generate_web_request(utility: "curl", url: "https://www.example.com/register", ssl_ca_cert: "/etc/ssl/custom_certs/ca_cert.crt", headers: ["--header \'Authorization: Bearer <my_token>\'"], params: ["host[build]=false", "host[organization_id]=1"])'
example 'generate_web_request(utility: "curl", url: "https://www.example.com/keys/client.asc", output_file: "/etc/apt/trusted.gpg.d/client1.asc")'
end
def generate_web_request(utility:, url:, ssl_ca_cert: nil, headers: nil, params: nil, output_file: nil)
utility = Foreman.download_utilities.fetch(utility || 'curl')
command = ["#{utility[:download_command]} #{url}"]
command << "#{utility[:ca_cert]} #{ssl_ca_cert}" if ssl_ca_cert
command << utility[:request_type_post] if params && utility[:request_type_post]
if output_file
command << "#{utility[:output_file]} #{output_file}"
elsif utility[:output_pipe]
command << utility[:output_pipe]
end
headers&.each { |header| command << header }
utility[:format_params].call(params).each { |param| command << param } if params
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to fix this because it generated (in case of curl):
`--data "packages=Paket1\ Paket2" which would mean it tries to install a package with the name "Paket1 Paket2" (which does not work because 2 packages are meant")

Copy link
Contributor Author

@goarsna goarsna Aug 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks Bernhard, I removed the quotes around the post parameters. This made it necessary to escape the ampersands in the wget post parameter.
In my tests installing multiple packages during host registration now worked as expected.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ofedoren Any thoughts about the solution with removing the quotes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe I should provide some additional information 😅

Post parameters have to be provided differently for curl (one --data argument for each post parameter) and wget (one --post-data argument containing all post parameters). Initially I removed the quotes of the post parameters in the Global Registration template to unify them and when formatting the post parameters for the curl or wget command respectively I added quotes around each post parameter in the case of curl and around the whole --post-data argument in the case of wget. But by this I added quotes also to those post parameters that where not quoted before (like the packages post parameter) which is causing the described problem.
While investigating the problem with installing multiple packages, I was thinking about whether it even makes sense to quote any of the post parameters as the parameters that could cause problems are already escaped. So my new approach is to not quote the parameters and escape them correctly.

command.join(" \\\n ")
end
end
end
end
Expand Down
75 changes: 38 additions & 37 deletions app/views/unattended/provisioning_templates/registration/global_registration.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ description: |
# Make sure, all command output can be parsed (e.g. from subscription-manager)
export LC_ALL=C LANG=C
<%
headers = ["-H 'Authorization: Bearer #{@auth_token}'"]
headers = ["--header 'Authorization: Bearer #{@auth_token}'"]
activation_keys = [(@hostgroup.params['kt_activation_keys'] if @hostgroup), @activation_keys].compact.join(',')
-%>

Expand All @@ -35,6 +35,7 @@ export LC_ALL=C LANG=C
<%= "\n# Ignore subman errors: [#{@ignore_subman_errors}]" unless @ignore_subman_errors.nil? -%>
<%= "\n# Lifecycle environment id: [#{@lifecycle_environment_id}]" if @lifecycle_environment_id.present? -%>
<%= "\n# Activation keys: [#{activation_keys}]" if activation_keys.present? -%>
<%= "\n# Download utility: [#{@download_utility}]" unless @download_utility.nil? -%>


if ! [ $(id -u) = 0 ]; then
Expand Down Expand Up @@ -91,11 +92,11 @@ elif [ -f /etc/debian_version ]; then
<% if repo_gpg_key_url.present? -%>
<%# "apt 1.2.35" on Ubuntu 16.04 does not support storing GPG public keys in "/etc/apt/trusted.gpg.d/" in ASCII format -%>
if [ "$(. /etc/os-release ; echo "$VERSION_ID")" = "16.04" ]; then
$PKG_MANAGER_INSTALL ca-certificates curl gnupg
curl --silent --show-error <%= shell_escape repo_gpg_key_url %> | gpg --dearmor > /etc/apt/trusted.gpg.d/client<%= index %>.gpg
$PKG_MANAGER_INSTALL ca-certificates gnupg
<%= indent(4, skip1: true) { generate_web_request(utility: @download_utility, url: shell_escape(repo_gpg_key_url)) } %> | gpg --dearmor > /etc/apt/trusted.gpg.d/client<%= index %>.gpg
else
$PKG_MANAGER_INSTALL ca-certificates curl
curl --silent --show-error --output /etc/apt/trusted.gpg.d/client<%= index %>.asc <%= shell_escape repo_gpg_key_url %>
$PKG_MANAGER_INSTALL ca-certificates
<%= indent(4, skip1: true) { generate_web_request(utility: @download_utility, url: shell_escape(repo_gpg_key_url), output_file: "/etc/apt/trusted.gpg.d/client#{index}.asc") } %>
fi
<% end -%>
apt-get update
Expand All @@ -108,22 +109,22 @@ fi
<% end -%>

register_host() {
curl --silent --show-error --cacert $SSL_CA_CERT --request POST <%= @registration_url %> \
<%= headers.join(' ') %> \
--data "host[name]=$(hostname --fqdn)" \
--data "host[build]=false" \
--data "host[managed]=false" \
<%= " --data 'host[organization_id]=#{@organization.id}' \\\n" if @organization -%>
<%= " --data 'host[location_id]=#{@location.id}' \\\n" if @location -%>
<%= " --data 'host[hostgroup_id]=#{@hostgroup.id}' \\\n" if @hostgroup -%>
<%= " --data 'host[operatingsystem_id]=#{@operatingsystem.id}' \\\n" if @operatingsystem -%>
<%= " --data host[interfaces_attributes][0][identifier]=#{shell_escape(@remote_execution_interface)} \\\n" if @remote_execution_interface.present? -%>
<%= " --data 'setup_insights=#{@setup_insights}' \\\n" unless @setup_insights.nil? -%>
<%= " --data 'setup_remote_execution=#{@setup_remote_execution}' \\\n" unless @setup_remote_execution.nil? -%>
<%= " --data remote_execution_interface=#{shell_escape(@remote_execution_interface)} \\\n" if @remote_execution_interface.present? -%>
<%= " --data packages=#{shell_escape(@packages)} \\\n" if @packages.present? -%>
<%= " --data 'update_packages=#{@update_packages}' \\\n" unless @update_packages.nil? -%>

<%
params = ["host[name]=$(hostname --fqdn)"]
params << "host[build]=false"
params << "host[managed]=false"
params << "host[organization_id]=#{@organization.id}" if @organization
params << "host[location_id]=#{@location.id}" if @location
params << "host[hostgroup_id]=#{@hostgroup.id}" if @hostgroup
params << "host[operatingsystem_id]=#{@operatingsystem.id}" if @operatingsystem
params << "host[interfaces_attributes][0][identifier]=#{shell_escape(@remote_execution_interface)}" if @remote_execution_interface.present?
params << "setup_insights=#{@setup_insights}" unless @setup_insights.nil?
params << "setup_remote_execution=#{@setup_remote_execution}" unless @setup_remote_execution.nil?
params << "remote_execution_interface=#{shell_escape(@remote_execution_interface)}" if @remote_execution_interface.present?
params << "packages=#{shell_escape(@packages)}" if @packages.present?
params << "update_packages=#{@update_packages}" unless @update_packages.nil?
-%>
<%= indent(2, skip1: true) { generate_web_request(utility: @download_utility, url: @registration_url, ssl_ca_cert: "$SSL_CA_CERT", headers: headers, params: params) } %>
}

echo "#"
Expand All @@ -132,22 +133,22 @@ echo "#"

<% if plugin_present?('katello') && activation_keys.present? -%>
register_katello_host(){
UUID=$(subscription-manager identity | grep --max-count 1 --only-matching '\([[:xdigit:]]\{8\}-[[:xdigit:]]\{4\}-[[:xdigit:]]\{4\}-[[:xdigit:]]\{4\}-[[:xdigit:]]\{12\}\)')
curl --silent --show-error --cacert $KATELLO_SERVER_CA_CERT --request POST "<%= @registration_url %>" \
<%= headers.join(' ') %> \
--data "uuid=$UUID" \
--data "host[build]=false" \
<%= " --data 'host[organization_id]=#{@organization.id}' \\\n" if @organization -%>
<%= " --data 'host[location_id]=#{@location.id}' \\\n" if @location -%>
<%= " --data 'host[hostgroup_id]=#{@hostgroup.id}' \\\n" if @hostgroup -%>
<%= " --data 'host[lifecycle_environment_id]=#{@lifecycle_environment_id}' \\\n" if @lifecycle_environment_id.present? -%>
<%= " --data 'setup_insights=#{@setup_insights}' \\\n" unless @setup_insights.nil? -%>
<%= " --data 'setup_remote_execution=#{@setup_remote_execution}' \\\n" unless @setup_remote_execution.nil? -%>
<%= " --data remote_execution_interface=#{shell_escape(@remote_execution_interface)} \\\n" if @remote_execution_interface.present? -%>
<%= " --data 'setup_remote_execution_pull=#{@setup_remote_execution_pull}' \\\n" unless @setup_remote_execution_pull.nil? -%>
<%= " --data packages=#{shell_escape(@packages)} \\\n" if @packages.present? -%>
<%= " --data 'update_packages=#{@update_packages}' \\\n" unless @update_packages.nil? -%>

<%
params = ["uuid=$UUID"]
params << "host[build]=false"
params << "host[organization_id]=#{@organization.id}" if @organization
params << "host[location_id]=#{@location.id}" if @location
params << "host[hostgroup_id]=#{@hostgroup.id}" if @hostgroup
params << "host[lifecycle_environment_id]=#{@lifecycle_environment_id}" if @lifecycle_environment_id.present?
params << "setup_insights=#{@setup_insights}" unless @setup_insights.nil?
params << "setup_remote_execution=#{@setup_remote_execution}" unless @setup_remote_execution.nil?
params << "remote_execution_interface=#{shell_escape(@remote_execution_interface)}" if @remote_execution_interface.present?
params << "setup_remote_execution_pull=#{@setup_remote_execution_pull}" unless @setup_remote_execution_pull.nil?
params << "packages=#{shell_escape(@packages)}" if @packages.present?
params << "update_packages=#{@update_packages}" unless @update_packages.nil?
-%>
UUID=$(subscription-manager identity | grep --max-count 1 --only-matching '\([[:xdigit:]]\{8\}-[[:xdigit:]]\{4\}-[[:xdigit:]]\{4\}-[[:xdigit:]]\{4\}-[[:xdigit:]]\{12\}\)')
<%= indent(2, skip1: true) { generate_web_request(utility: @download_utility, url: @registration_url, ssl_ca_cert: "$KATELLO_SERVER_CA_CERT", headers: headers, params: params) } %>
}

# Set up subscription-manager
Expand Down
21 changes: 21 additions & 0 deletions lib/foreman.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,25 @@ def self.input_types_registry
def self.settings
SettingRegistry.instance
end

def self.download_utilities
{
'curl' => {
:ca_cert => '--cacert',
:download_command => 'curl --silent --show-error',
:insecure => '--insecure',
:output_file => '--output',
:request_type_post => '--request POST',
:format_params => proc { |params| params.map { |param| "--data #{param}" } },
},
'wget' => {
:ca_cert => '--ca-certificate',
:download_command => 'wget --no-verbose --no-hsts',
:insecure => '--no-check-certificate',
:output_file => '--output-document',
:output_pipe => '--output-document -',
:format_params => proc { |params| ["--post-data #{params.join('\&')}"] },
},
}.freeze
end
end
33 changes: 30 additions & 3 deletions test/controllers/api/v2/registration_commands_controller_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ class Api::V2::RegistrationCommandsControllerTest < ActionController::TestCase
assert_response :success
response = ActiveSupport::JSON.decode(@response.body)['registration_command']

assert_includes response, "curl -sS 'http://test.host/register'"
assert_includes response, "-H 'Authorization: Bearer"
assert_includes response, "curl --silent --show-error 'http://test.host/register'"
assert_includes response, "--header 'Authorization: Bearer"
end

test 'with params' do
Expand Down Expand Up @@ -50,7 +50,7 @@ class Api::V2::RegistrationCommandsControllerTest < ActionController::TestCase
assert_response :success

response = ActiveSupport::JSON.decode(@response.body)['registration_command']
assert_includes response, "curl -sS --insecure '#{proxy.url}/register'"
assert_includes response, "curl --silent --show-error --insecure '#{proxy.url}/register'"
end

test 'os without host_init_config template' do
Expand All @@ -65,5 +65,32 @@ class Api::V2::RegistrationCommandsControllerTest < ActionController::TestCase
post :create, params: { smart_proxy_id: smart_proxies(:one).id }
assert_response :unprocessable_entity
end

test 'using wget' do
params = {
download_utility: 'wget',
}

post :create, params: params
assert_response :success

response = ActiveSupport::JSON.decode(@response.body)['registration_command']
assert_includes response, "wget --no-verbose --no-hsts --output-document - 'http://test.host/register?download_utility=wget'"
assert_includes response, "--header 'Authorization: Bearer"
end

test 'using wget with insecure option' do
params = {
download_utility: 'wget',
insecure: true,
}

post :create, params: params
assert_response :success

response = ActiveSupport::JSON.decode(@response.body)['registration_command']
assert_includes response, "wget --no-verbose --no-hsts --output-document - --no-check-certificate 'http://test.host/register?download_utility=wget'"
assert_includes response, "--header 'Authorization: Bearer"
end
end
end
2 changes: 2 additions & 0 deletions test/controllers/api/v2/registration_controller_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ class Api::V2::RegistrationControllerTest < ActionController::TestCase
location_id: taxonomies(:location1).id,
hostgroup_id: hostgroups(:common).id,
operatingsystem_id: operatingsystems(:centos5_3).id,
download_utility: 'wget',
}

get :global, params: params, session: set_session_user
Expand All @@ -38,6 +39,7 @@ class Api::V2::RegistrationControllerTest < ActionController::TestCase
assert_equal operatingsystems(:centos5_3), vars[:operatingsystem]
assert_equal users(:admin), vars[:user]
assert_equal register_url, vars[:registration_url].to_s
assert_equal 'wget', vars[:download_utility].to_s
end

test "should not pass unpermitted params to template" do
Expand Down
4 changes: 3 additions & 1 deletion test/controllers/registration_commands_controller_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ class RegistrationCommandsControllerTest < ActionController::TestCase
hostgroupId: hostgroups(:common).id,
operatingsystemId: operatingsystems(:redhat).id,
update_packages: true,
download_utility: 'wget',
}
post :create, params: params, session: set_session_user
command = JSON.parse(@response.body)['command']
Expand All @@ -46,6 +47,7 @@ class RegistrationCommandsControllerTest < ActionController::TestCase
assert_includes command, 'hostgroupId='
assert_includes command, 'operatingsystemId='
assert_includes command, 'update_packages=true'
assert_includes command, 'download_utility=wget'
end

test 'with params ignored in URL' do
Expand All @@ -61,7 +63,7 @@ class RegistrationCommandsControllerTest < ActionController::TestCase
post :create, params: params, session: set_session_user
command = JSON.parse(@response.body)['command']

assert_includes command, "curl -sS --insecure '#{proxy.url}/register"
assert_includes command, "curl --silent --show-error --insecure '#{proxy.url}/register"
refute command.include?('smart_proxy_id')
refute command.include?('insecure=true')
refute command.include?('jwt_expiration')
Expand Down
Loading
Loading