theforeman
Fixes #36688 - Provide option to use wget for the new Register Host feature
The Register hosts page uses curl for registering hosts. This seems to be a good approach for EL based systems as most of these come with curl preinstalled. On Debian based systems this is not always the case (at least for recent versions of Debian and Ubuntu).
This feature request aims to provide an option to use wget for registering hosts as wget is as widely used as curl. By this system administrators that only have wget installed are not forced to also install curl anymore.
I have opened a PR to document this in foreman-documentation:
https://github.com/theforeman/foreman-documentation/pull/2376
Thanks @stejskalleos for your review, I finally made it to answer / adapt everything.
Please let me know if you would like to see any further changes.
When attempting to preview the 'Global Registration' template from the template edit page, I encounter the following error: There was an error rendering the Global Registration template: undefined method '#[]' for NilClass::Jail (NilClass).
Additionally, could we merge the 16 commits into a single commit?
Hey @nofaralfasi, thanks for your feedback. I fixed the template, squashed the commits and rebased my branch.
Hey @nofaralfasi, again some changes.
in the first commit I rebased my branch to solve merge conflicts in the global_registration.erb introduced by replacing apt-key for Ubuntu and Debian .
Then I've added some tests to cover my changes.
And lastly @sbernhard and I discussed moving the dicts holding the curl and wget command patterns to one place.
What I just noticed is that there is still a failing node test. I'll have a look at that.
Just ping me, in case I can / should squash the commits again. :)
Ok, something weird was happening to my local tests, but now the tests are fixed. :)
Hey there,
I rebased my branch due to merge conflicts and squashed the commits. Now the Jenkins unit tests fail. However the tests are working in my forklift. I tried both, calling bundle exec rake jenkins:unit TESTOPTS="-v" --trace and running the failing test file directly calling bundle exec rake test TEST=test/controllers/unattended_controller_test.rb.
What do you think @nofaralfasi?
I don't know why the tests are failing. Also, the wget command doesn't work for me, whereas the curl command does.
[root@rhel8-8-testing-machine ~]# set -o pipefail && wget --no-verbose -O- --no-check-certificate 'http://192.168.121.1:3000/register?download_utility=wget&location_id=2&operatingsystem_id=13&organization_id=1&update_packages=false' --header 'Authorization: Bearer eyJhbGciOiJIUzI1NiJ9.eyJ1c2VyX2lkIjo0LCJpYXQiOjE3MDU5MzQyOTksImp0aSI6IjRkOWJiYmQ4Yzg2OGUyODNlZWE3OGExZmFiMjZiNGZkZmEwNTRiZDAzMGZiOTU1NGNlNGQ2M2U3ZDc0YTEyZDciLCJleHAiOjE3MDU5NDg2OTksInNjb3BlIjoicmVnaXN0cmF0aW9uI2dsb2JhbCByZWdpc3RyYXRpb24jaG9zdCJ9.m_d_whF1xwve7KGDDDbrBRihBJMrAUpu9849ilAWd_M' | bash
2024-01-22 14:38:40 URL:http://192.168.121.1:3000/register?download_utility=wget&location_id=2&operatingsystem_id=13&organization_id=1&update_packages=false [3594] -> "-" [1]
#
# Running registration
#
http://192.168.121.1:3000/register:
2024-01-22 14:38:41 ERROR 422: Unprocessable Entity.
Hey @nofaralfasi, thanks a lot for your reviews. I'll have a look at the error.
fyi: The docs for this PR got merged yesterday.
Hey there, updated, rebased and did tests with Ubuntu 22.04 and Almalinux 8 that were successful. But I cannot tell atm what is causing the failing test. I can have a look at it again in calendar week 24, when I am back from vacation.
This PR also requires https://github.com/theforeman/foreman/pull/10179 to use wget in the host init config template.
After having a look at the failing tests again, I assume they are not related to my PR. :thinking: But I am a little bit clueless what's causing them. Any ideas, suggestions, thoughts or objections?
Hey @sbernhard, as discussed in our call last week I moved the construction of the "curl" and "wget" commands completely to helper methods. I will also have a look at unifying the two methods in one generic method for creating web requests in templates. But I have to see what the result looks like, considering all the cases and the formatting.
Hey @ofedoren and @sbernhard, I (hopefully) addressed everything that was requested and fixed the tests again (despite those often failing katello tests... :sweat_smile:). And I again did test some tests: I registered an Ubuntu host using curl and wget and the command in the register host page worked as well as the rendered global_registration template with both, curl and wget.
Is there anything left from your side or are we good to merge this? :)
I received a good suggestion by @sbernhard: Add tests for the generate_web_request method :grin: => Done
I did some other end-to-end tests with bash -x and also tested the host_init_config template with the changes from https://github.com/theforeman/foreman/pull/10179.
Thereby I noticed that https://github.com/theforeman/foreman/pull/9983 added installation of curl to the global_registration template. => Removed
In further downstream tests @dosas discovered that wget by default enables HSTS support whereas curl does not. This can lead to errors (for example) in case a repository is added to use during host registration which is secured with a self signed certificate.
To stay consistent accross curl and wget we added the --no-hsts flag to the wget commands. The --no-hsts flag has no effect on https connections.
The failed tests are not related.
Any objections to get this change in @stejskalleos / @ekohl / @ofedoren ?
Some js tests failed, could you please run npm run test -- -u to regenerate the snapshots?
Hey @ofedoren, thanks again for your review.
I left one comment above and added missing Apipie documentation for the download_utility in the registration_controller.rb :see_no_evil: