Fixes #36940 - Add necessary migration for host_config tftp directory

[sbernhard]

The migration is necessary as part of the puppet_foreman_proxy change in https://github.com/theforeman/puppet-foreman_proxy/pull/821

Belongs to the SecureBoot story: https://github.com/theforeman/foreman/pull/9864

[ehelms]

A migration spec for this would be great to have. Example: https://github.com/theforeman/foreman-installer/blob/develop/spec/migrations/20240328125552_reset_puppetserver_ciphers_spec.rb

[goarsna]

FYI: @nofaralfasi and I had a discussion last week that resulted in some changes we want to implement on the Smart Proxy side which are summarized in the Smart Proxy PR. These changes also will affect this PR. Further updates will follow soonish / in the upcoming weeks (as soon as I have time to work on this again).

[goarsna]

One outcome of the above mentioned discussion was that the Bootloader Universe should not be configurable. Instead it will be a fixed folder inside the TFTP root that is created automatically during installation / upgrade. The other agreement was, that the Host Config directory will be written with a dash. Both have now been applied to this PR.

[nofaralfasi]

/packit build

[packit-as-a-service[bot]]

Account nofaralfasi has no write access nor is author of PR!

[ehelms]

/packit build

[ehelms]

I think this will need a rebase to pass tests.

[goarsna]

Why are there multiple migrations, one adding host-config and another adding bootloader-universe?

There is no specific reason for that despite that the directories are independent of each other from a file system POV and that at the point I added the migration for the bootloader-universe directory to the migrations it looked cleaner to me to add it in a separate migration. But on the other hand the directories belong to each other as the one does not make sense without the other.

My current POV: We sould merge the migrations into one.

What do you think about it @nofaralfasi?

[nofaralfasi]

My current POV: We sould merge the migrations into one.

What do you think about it @nofaralfasi?

I also think that we should merge them into one.

[nofaralfasi]

Hi, when testing this PR, we're getting the following error: /usr/share/gems/gems/kafo-7.5.1/lib/kafo/puppet_command.rb:53:in format_command': wrong number of arguments (given 2, expected 1) (ArgumentError). Any idea why this is happening?

[goarsna]

Hi, when testing this PR, we're getting the following error: /usr/share/gems/gems/kafo-7.5.1/lib/kafo/puppet_command.rb:53:in format_command': wrong number of arguments (given 2, expected 1) (ArgumentError). Any idea why this is happening?

Hey Nofar, that is strange... I do not encounter this error (or any other errors) on my test system I just rolled out. But I have to mention that my test setup is based on Foreman 3.11 with the changes from the Secure Boot PRs applied on top. When exactly does the error occur? Could you share the complete log output?

[nofaralfasi]

Hey Nofar, that is strange... I do not encounter this error (or any other errors) on my test system I just rolled out. But I have to mention that my test setup is based on Foreman 3.11 with the changes from the Secure Boot PRs applied on top. When exactly does the error occur? Could you share the complete log output?

When we run the foreman-installer command:

# foreman-installer
/usr/share/gems/gems/kafo-7.5.1/lib/kafo/puppet_command.rb:53:in `format_command': wrong number of arguments (given 2, expected 1) (ArgumentError)
	from /usr/share/foreman-installer/hooks/boot/01-kafo-hook-extensions.rb:132:in `execute_command'
	from /usr/share/foreman-installer/hooks/boot/01-kafo-hook-extensions.rb:124:in `execute'
	from /usr/share/foreman-installer/hooks/boot/03-foreman-maintain-extensions.rb:4:in `foreman_maintain'
	from /usr/share/foreman-installer/hooks/boot/09-version_locking.rb:2:in `package_lock_feature?'
	from /usr/share/foreman-installer/hooks/boot/09-version_locking.rb:5:in `block (4 levels) in load'
	from /usr/share/gems/gems/kafo-7.5.1/lib/kafo/hooking.rb:36:in `instance_eval'
	from /usr/share/gems/gems/kafo-7.5.1/lib/kafo/hooking.rb:36:in `block (4 levels) in load'
	from /usr/share/gems/gems/kafo-7.5.1/lib/kafo/hook_context.rb:19:in `instance_eval'
	from /usr/share/gems/gems/kafo-7.5.1/lib/kafo/hook_context.rb:19:in `execute'
	from /usr/share/gems/gems/kafo-7.5.1/lib/kafo/hooking.rb:67:in `block in execute'
	from /usr/share/gems/gems/kafo-7.5.1/lib/kafo/hooking.rb:65:in `each'
	from /usr/share/gems/gems/kafo-7.5.1/lib/kafo/hooking.rb:65:in `execute'
	from /usr/share/gems/gems/kafo-7.5.1/lib/kafo/kafo_configure.rb:144:in `initialize'
	from /usr/share/gems/gems/clamp-1.3.2/lib/clamp/command.rb:140:in `new'
	from /usr/share/gems/gems/clamp-1.3.2/lib/clamp/command.rb:140:in `run'
	from /usr/share/gems/gems/kafo-7.5.1/lib/kafo/kafo_configure.rb:54:in `run'
	from /usr/sbin/foreman-installer:8:in `<main>'

[goarsna]

Thanks for the log! I assume the error is not related to the changes in the PR. But I do not know what is causing the error... :/

[goarsna]

@nofar Rebased as discussed in DM. Maybe this solves the errors you encounter :)

[goarsna]

I squashed the migrations as discussed :)

[stejskalleos]

https://github.com/theforeman/smart-proxy/pull/877 and https://github.com/theforeman/foreman/pull/9864 have been merged

[nofaralfasi]

theforeman/puppet-foreman_proxy#821 has been merged. Could we have this included as well?

[goarsna]

:see_no_evil: Is there a face palm emoji on GitHub? :sweat_smile: Fixed it

[goarsna]

Thanks a lot @ekohl for merging this! And thanks to everyone who was involved in getting this done :tada: