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.

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

Add Global params for host obfuscation and migrate off settings #921

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ class UploadsSettingsController < ::ApplicationController
def index
render json: {
autoUploadEnabled: Setting[:allow_auto_inventory_upload],
hostObfuscationEnabled: Setting[:obfuscate_inventory_hostnames],
ipsObfuscationEnabled: Setting[:obfuscate_inventory_ips],
excludePackagesEnabled: Setting[:exclude_installed_packages],
allowAutoInsightsMismatchDelete: Setting[:allow_auto_insights_mismatch_delete],
CloudConnectorStatus: ForemanInventoryUpload::UploadsSettingsController.cloud_connector_status,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
class ConvertObfuscationSettingsToParams < ActiveRecord::Migration[6.1]
def change
# Create the common parameters if they don't exist and set the default values to false
CommonParameter.find_or_create_by!(name: 'obfuscate_inventory_hostnames', value: false)
CommonParameter.find_or_create_by!(name: 'obfuscate_inventory_ips', value: false)
Comment on lines +4 to +5
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
CommonParameter.find_or_create_by!(name: 'obfuscate_inventory_hostnames', value: false)
CommonParameter.find_or_create_by!(name: 'obfuscate_inventory_ips', value: false)
hostname_setting = CommonParameter.find_or_create_by!(name: 'obfuscate_inventory_hostnames', value: false)
ip_setting = CommonParameter.find_or_create_by!(name: 'obfuscate_inventory_ips', value: false)

You can skip the find_by later in the code


# Copy the settings to the common parameters and remove the settings from the database
# rubocop:disable Style/GuardClause
if Setting.find_by(name: 'obfuscate_inventory_hostnames')&.value
hostname_setting = CommonParameter.find_by(name: 'obfuscate_inventory_hostnames')
hostname_setting.update!(value: Setting.find_by(name: 'obfuscate_inventory_hostnames')&.value)
Setting.find_by(name: 'obfuscate_inventory_hostnames').destroy_all
end

if Setting.find_by(name: 'obfuscate_inventory_ips')&.value
ip_setting = CommonParameter.find_by(name: 'obfuscate_inventory_ips')
ip_setting.update!(value: Setting.find_by(name: 'obfuscate_inventory_ips')&.value)
Setting.find_by(name: 'obfuscate_inventory_ips').destroy_all
end
# rubocop:enable Style/GuardClause
end
end
6 changes: 3 additions & 3 deletions lib/foreman_inventory_upload/generators/fact_helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ def obfuscate_hostname?(host)
insights_client_setting = ActiveModel::Type::Boolean.new.cast(insights_client_setting)
return insights_client_setting unless insights_client_setting.nil?

Setting[:obfuscate_inventory_hostnames]
CommonParameter.find_by(name: 'obfuscate_inventory_hostnames')&.value
Copy link
Member

Choose a reason for hiding this comment

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

You should look for a value on the host, not just the common parameter, similar to

param_value = host.host_param(InsightsCloud.enable_client_param)

end

def fqdn(host)
Expand All @@ -79,7 +79,7 @@ def obfuscate_ips?(host)
insights_client_setting = ActiveModel::Type::Boolean.new.cast(insights_client_setting)
return insights_client_setting unless insights_client_setting.nil?

Setting[:obfuscate_inventory_ips]
CommonParameter.find_by(name: 'obfuscate_inventory_ips')&.value
Copy link
Member

Choose a reason for hiding this comment

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

If it's a parameter, you have to look up the value on the host object itself:

param_value = host.host_param(InsightsCloud.enable_client_param)

end

def host_ips(host)
Expand Down Expand Up @@ -114,7 +114,7 @@ def hostname_match
foreman_hostname = ForemanRhCloud.foreman_host&.name
if bash_hostname == foreman_hostname
fqdn(ForemanRhCloud.foreman_host)
elsif Setting[:obfuscate_inventory_hostnames]
elsif CommonParameter.find_by(name: 'obfuscate_inventory_hostnames')&.value
Copy link
Member

Choose a reason for hiding this comment

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

If it's a parameter, you have to look up the value on the host object itself:

param_value = host.host_param(InsightsCloud.enable_client_param)

obfuscate_fqdn(bash_hostname)
else
bash_hostname
Expand Down
2 changes: 0 additions & 2 deletions lib/foreman_rh_cloud/engine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,6 @@ def self.register_scheduled_task(task_class, cronline)
setting('allow_auto_inventory_upload', type: :boolean, description: N_('Enable automatic upload of your host inventory to the Red Hat cloud'), default: true, full_name: N_('Automatic inventory upload'))
setting('allow_auto_insights_sync', type: :boolean, description: N_('Enable automatic synchronization of Insights recommendations from the Red Hat cloud'), default: false, full_name: N_('Synchronize recommendations Automatically'))
setting('allow_auto_insights_mismatch_delete', type: :boolean, description: N_('Enable automatic deletion of mismatched host records from the Red Hat cloud'), default: false, full_name: N_('Automatic mismatch deletion'))
setting('obfuscate_inventory_hostnames', type: :boolean, description: N_('Obfuscate host names sent to the Red Hat cloud'), default: false, full_name: N_('Obfuscate host names'))
setting('obfuscate_inventory_ips', type: :boolean, description: N_('Obfuscate ipv4 addresses sent to the Red Hat cloud'), default: false, full_name: N_('Obfuscate host ipv4 addresses'))
setting('exclude_installed_packages', type: :boolean, description: N_('Exclude installed packages from being uploaded to the Red Hat cloud'), default: false, full_name: N_("Exclude installed Packages"))
setting('include_parameter_tags', type: :boolean, description: N_('Should import include parameter tags from Foreman?'), default: false, full_name: N_('Include parameters in insights-client reports'))
setting('rhc_instance_id', type: :string, description: N_('RHC daemon id'), default: nil, full_name: N_('ID of the RHC(Yggdrasil) daemon'))
Expand Down
4 changes: 2 additions & 2 deletions test/unit/metadata_generator_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ class MetadataGeneratorTest < ActiveSupport::TestCase
end

test 'generates an empty report with hidden hostname and ip' do
Setting[:obfuscate_inventory_hostnames] = true
Setting[:obfuscate_inventory_ips] = true
CommonParameter.find_or_create_by!(name: 'obfuscate_inventory_ips', value: true)
Copy link
Member

Choose a reason for hiding this comment

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

If the parameter exists, I think this method will not reset its value.

CommonParameter.find_or_create_by!(name: 'obfuscate_inventory_hostnames', value: true)
@host = FactoryBot.create(:host, :managed)
ForemanRhCloud.expects(:foreman_host_name).returns(@host.name)
generator = ForemanInventoryUpload::Generators::Metadata.new
Expand Down
4 changes: 2 additions & 2 deletions test/unit/slice_generator_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ def fact_names
end

test 'generates obfuscated ip_address fields without inisghts-client' do
Setting[:obfuscate_inventory_ips] = true
CommonParameter.find_or_create_by!(name: 'obfuscate_inventory_ips', value: true)

@host.interfaces << FactoryBot.build(:nic_managed)
batch = Host.where(id: @host.id).in_batches.first
Expand Down Expand Up @@ -261,7 +261,7 @@ def fact_names
end

test 'obfuscates fqdn when setting set' do
Setting[:obfuscate_inventory_hostnames] = true
CommonParameter.find_or_create_by!(name: 'obfuscate_inventory_hostnames', value: true)

batch = Host.where(id: @host.id).in_batches.first
generator = create_generator(batch)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,6 @@ export const settingsDict = {
'Enable automatic upload of your hosts inventory to the Red Hat cloud'
),
},
hostObfuscationEnabled: {
name: 'obfuscate_inventory_hostnames',
label: __('Obfuscate host names'),
tooltip: __('Obfuscate host names sent to the Red Hat cloud'),
},
ipsObfuscationEnabled: {
name: 'obfuscate_inventory_ips',
label: __('Obfuscate host ipv4 addresses'),
tooltip: __('Obfuscate ipv4 addresses sent to the Red Hat cloud'),
},
excludePackagesEnabled: {
name: 'exclude_installed_packages',
label: __('Exclude installed Packages'),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,6 @@ exports[`InventorySettings rendering render without Props 1`] = `
key="autoUploadEnabled"
setting="autoUploadEnabled"
/>
<AdvancedSetting
key="hostObfuscationEnabled"
setting="hostObfuscationEnabled"
/>
<AdvancedSetting
key="ipsObfuscationEnabled"
setting="ipsObfuscationEnabled"
/>
<AdvancedSetting
key="excludePackagesEnabled"
setting="excludePackagesEnabled"
Expand Down
Loading