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 #30507: Use enabled hook helpers for Katello and Foreman #546

Merged
merged 1 commit into from
Aug 27, 2020
Merged

Fixes #30507: Use enabled hook helpers for Katello and Foreman #546

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
6 changes: 5 additions & 1 deletion hooks/boot/01-kafo-hook-extensions.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ def local_foreman_db?
end

def local_candlepin_db?
module_enabled?('katello') && param_value('katello', 'candlepin_manage_db')
candlepin_enabled? && param_value('katello', 'candlepin_manage_db')
end

def local_postgresql?
Expand All @@ -73,6 +73,10 @@ def local_redis?
(foreman_server? && !param_value('foreman', 'jobs_sidekiq_redis_url')) || pulpcore_enabled? || devel_scenario?
end

def candlepin_enabled?
katello_enabled?
end

def pulp_enabled?
module_enabled?('foreman_proxy_plugin_pulp') && (param_value('foreman_proxy_plugin_pulp', 'pulpnode_enabled') || param_value('foreman_proxy_plugin_pulp', 'enabled'))
end
Expand Down
4 changes: 2 additions & 2 deletions hooks/post/99-post_install_message.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@
new_install_message(@kafo) if new_install?
end

if module_enabled?('katello')
if katello_enabled?
certs_generate_command_message
end

if module_enabled?('katello_devel')
if devel_scenario?
dev_server_success_message(@kafo)
dev_new_install_message(@kafo) if new_install?
end
Expand Down
2 changes: 1 addition & 1 deletion hooks/pre/10-reset_data.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ def reset
stop_services
start_services(['postgresql']) if local_postgresql?
empty_db_in_postgresql('foreman') if foreman_server?
reset_candlepin if module_enabled?('katello')
reset_candlepin if candlepin_enabled?
reset_pulp if pulp_enabled?
empty_db_in_postgresql('pulpcore') if pulpcore_enabled?
end
Expand Down
2 changes: 1 addition & 1 deletion katello/hooks/post/31-cdn_setting.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
if !app_value(:noop) && module_enabled?('katello')
if !app_value(:noop) && katello_enabled?
param = get_custom_config('cdn_ssl_version')
if param
logger.info 'cdn_ssl_version param found, migrating to a Katello setting'
Expand Down
2 changes: 1 addition & 1 deletion katello/hooks/pre/15-check_java.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def error_exit(message, code)
kafo.class.exit code
end

if module_enabled?('katello')
if candlepin_enabled?
java_version_string = `/usr/bin/java -version 2>&1`
java_version = java_version_string.split("\n")[0].split('"')[1]
Copy link
Member

Choose a reason for hiding this comment

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

On an unrelated note: I think this check itself should ensure JDK 1.8 is used since 1.7 is too old.


Expand Down
2 changes: 1 addition & 1 deletion katello/hooks/pre/20-certs_update.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def mark_for_update(cert_name, hostname = nil)
cert_file = param('certs', 'server_cert').value
key_file = param('certs', 'server_key').value

if app_value('certs_update_server_ca') && !module_enabled?('katello')
if app_value('certs_update_server_ca') && !katello_enabled?
fail_and_exit("--certs-update-server-ca needs to be used with katello", 101)
end
Comment on lines +21 to 23
Copy link
Member

Choose a reason for hiding this comment

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

This part should really be a pre_commit hook IMHO. In pre the answers are already saved to disk. You should fail on invalid user input in pre.

It should also check if the certs module is enabled, rather than katello, right? It checks if the certs params are passed in and I think that server_ca_cert can also be used on a content proxy.

Copy link
Member Author

Choose a reason for hiding this comment

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

File an issue if you don't mind, touching our certs code requires a Friday :P


Expand Down
3 changes: 1 addition & 2 deletions katello/hooks/pre_commit/14-cdn_setting.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
if module_enabled?('katello') &&
@kafo.config.answers['katello'].is_a?(Hash) &&
if @kafo.config.answers['katello'].is_a?(Hash) &&
@kafo.config.answers['katello'].key?('cdn_ssl_version')
# Using @kafo.config.answers because param() goes through PuppetModule. At
# this point the parameter doesn't exist in the module on disk so param()
Expand Down