Fixes #22249 - Handle use_autosignfile in migrations

[ekohl]

No description provided.

[theforeman-bot]

Issues: #22249

[stbenjam]

Looks good, just need to fix the rubocop issue

[ekohl]

Updated. Note I didn't get to test this yet. Forgot we can't take cherry picks in the installer itself with forklift, but since it's a straight copy (with rubocop fixes now) from foreman-installer I'm assuming it should work.

[stbenjam]

For migrations I usually drop them into an existing install and run foreman-installer --migrations-only to test.

We have tests here that make sure migrations don't alter the answers file, so answer files need updating whenever we add a migration. Although, this migration doesn't really work with that paradigm, because it'll set the dir param based on the OS.

Why is the default false for use_autosignfile in puppet-foreman_proxy? And even then, why do we need a migration to set the autosign directory? The puppet params.pp looks like it should already pick the right value.

[ekohl]

I should have just used https://projects.theforeman.org/issues/21856 rather than creating a new issue. Going to try it on the pipeline box now.

[stbenjam]

I see from theforeman/puppet-foreman_proxy@8d6880b why we need the migration to turn on $use_autosignfile, but I don't understand why we have to set the $autosignfile itself in a migration, the params.pp should pick the right value. Am I missing something?

[ekohl]

After looking into it: the default in params.pp is false. So it ends up being disabled while 1.16 needs it to be true. This code wouldn't actually deliver it to our users that already upgraded to 1.16.

[ekohl]

I've submitted theforeman/puppet-foreman_proxy#401 to use modern defaults and will update this PR.

[stbenjam]

No, I get that, please re-read the above. Totally fine with setting $use_autosign to true. You need to also set that in the answers.

However, there are two params. You are setting the value of the location of the file too, I don't get why. It's not needed, and it's incompatible with how Katello handles migrations. Katello requires migrations already be run against the answers, so they match, but the autosign path is dependent on OS. It's also not needed, params.pp handles it.

[stbenjam]

(Look at the current test failures - it's not because of rubocop)

[ekohl]

A user could have overridden the puppetdir value and this maintains their setting in the new format. Say a user wants to continue using /etc/puppet even with puppet 4, this makes that possible.

[stbenjam]

Erhm, ok, maybe that's fine, the if answers['foreman_proxy'].key?('puppetdir') won't be there on first run of the installer, so to get the tests to pass all you need to do is add use_autosign: true to the answers

[ekohl]

Updated to update the default answer files and included katello-devel too.

[stbenjam]

LGTM

[ekohl]

I am concerned about users who already upgraded to 1.16 and are now stuck with a broken install.

[stbenjam]

You could change the migration to always flip use_autosign to true. It'd only be run once.

Of course, it'd enable autosign again if users explicitly disabled it in the past, I think. Is there some param we could look at that existed pre-1.16?

[ekohl]

Like this?

[stbenjam]

Looks the same, I think you can get rid of && !answers['foreman_proxy']['use_autosignfile']?

[ekohl]

Updated

[stbenjam]

Looks good