Fixes #32885: Remove puppet from foreman::user_groups if present #690
Conversation
| @@ -0,0 +1,3 @@ | |||
| if answers['foreman'].is_a?(Hash) | |||
| answers['foreman']['user_groups'].delete('puppet') if answers['foreman']['user_groups'].include?('puppet') | |||
There was a problem hiding this comment.
Can we make this the same as the Foreman migration? That should be safer.
| @@ -23,7 +23,6 @@ foreman: | |||
| server_ssl_chain: /etc/pki/katello/certs/katello-server-ca.crt | |||
| server_ssl_crl: "" | |||
| server_ssl_key: /etc/pki/katello/private/katello-apache.key | |||
| user_groups: [] | |||
There was a problem hiding this comment.
I figured this does not need to be here anymore.
config/foreman.migrations/20210625142707_dynamic_puppet-in-foreman-groups.rb
Outdated
Show resolved
Hide resolved
|
Around whether to remove the migration file or not: a) I don't know what happens when you remove a migration that previously existed This was my rationale for leaving the file and clearing the content. |
Luckily https://github.com/theforeman/kafo/blob/master/lib/kafo/migrations.rb is quite easy to follow. Most crucial is this line. It essentially is a list of (file)names that have already been applied and should be skipped. As such, it should be safe to remove the file. You should only be careful not to change/reuse migration names, but since a timestamp is included that shouldn't happen in practice.
On the other hand, then you can know what they followed. The real migration or not. With this it'll be impossible to determine. Given that, I'm in favor of removing the migration rather than keeping a dummy. |
The change theforeman/puppet-foreman#938 introduced dynamically determining if the puppet group needed to be added to the foreman user. This no longer needs to be set explicitly on the foreman::user_groups parameter thus the migration cleans it up if present.
The change theforeman/puppet-foreman#938
introduced dynamically determining if the puppet group needed to
be added to the foreman user. This no longer needs to be set
explicitly on the foreman::user_groups parameter thus the migration
cleans it up if present.