From 8141a03720385d7aa126e4edc3879782968b8d27 Mon Sep 17 00:00:00 2001 From: Adam Grare Date: Wed, 13 May 2020 16:18:42 -0400 Subject: [PATCH 01/12] Create the provider when creating the config_manager --- .../providers/foreman/configuration_manager.rb | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/app/models/manageiq/providers/foreman/configuration_manager.rb b/app/models/manageiq/providers/foreman/configuration_manager.rb index e7ce737..85e9d75 100644 --- a/app/models/manageiq/providers/foreman/configuration_manager.rb +++ b/app/models/manageiq/providers/foreman/configuration_manager.rb @@ -13,10 +13,14 @@ class ManageIQ::Providers::Foreman::ConfigurationManager < ManageIQ::Providers:: :authentications, :connect, :endpoints, + :url, + :url=, :verify_credentials, :with_provider_connection, :to => :provider + belongs_to :provider, :autosave => true + class << self delegate :params_for_create, :verify_credentials, @@ -31,6 +35,10 @@ def self.description @description ||= "Foreman Configuration".freeze end + def provider + super || ensure_provider + end + def image_name "foreman_configuration" end @@ -38,4 +46,14 @@ def image_name def self.display_name(number = 1) n_('Configuration Manager (Foreman)', 'Configuration Managers (Foreman)', number) end + + def ensure_provider + build_provider + + provider.configuration_manager = self + provider.name = name + provider.zone = zone + + provider + end end From b336fc1a001e375e4b81709804fcc31f49db73e0 Mon Sep 17 00:00:00 2001 From: Adam Grare Date: Thu, 14 May 2020 09:36:29 -0400 Subject: [PATCH 02/12] Dependent destroy the provider on deletion --- app/models/manageiq/providers/foreman/configuration_manager.rb | 2 +- app/models/manageiq/providers/foreman/provider.rb | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/app/models/manageiq/providers/foreman/configuration_manager.rb b/app/models/manageiq/providers/foreman/configuration_manager.rb index 85e9d75..450cc57 100644 --- a/app/models/manageiq/providers/foreman/configuration_manager.rb +++ b/app/models/manageiq/providers/foreman/configuration_manager.rb @@ -19,7 +19,7 @@ class ManageIQ::Providers::Foreman::ConfigurationManager < ManageIQ::Providers:: :with_provider_connection, :to => :provider - belongs_to :provider, :autosave => true + belongs_to :provider, :autosave => true, :dependent => :destroy class << self delegate :params_for_create, diff --git a/app/models/manageiq/providers/foreman/provider.rb b/app/models/manageiq/providers/foreman/provider.rb index fba86c4..45844df 100644 --- a/app/models/manageiq/providers/foreman/provider.rb +++ b/app/models/manageiq/providers/foreman/provider.rb @@ -158,9 +158,11 @@ def verify_credentials(auth_type = nil, options = {}) def ensure_managers build_provisioning_manager unless provisioning_manager provisioning_manager.name = "#{name} Provisioning Manager" + provisioning_manager.provider = self build_configuration_manager unless configuration_manager configuration_manager.name = "#{name} Configuration Manager" + configuration_manager.provider = self if zone_id_changed? provisioning_manager.enabled = Zone.maintenance_zone&.id != zone_id From 418cbbdd1a6a0675b6b806c31457c7c1b437c3d1 Mon Sep 17 00:00:00 2001 From: Adam Grare Date: Thu, 14 May 2020 17:11:34 -0400 Subject: [PATCH 03/12] Update provider name and zone when manager changes --- .../manageiq/providers/foreman/configuration_manager.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/app/models/manageiq/providers/foreman/configuration_manager.rb b/app/models/manageiq/providers/foreman/configuration_manager.rb index 450cc57..28a598b 100644 --- a/app/models/manageiq/providers/foreman/configuration_manager.rb +++ b/app/models/manageiq/providers/foreman/configuration_manager.rb @@ -21,6 +21,8 @@ class ManageIQ::Providers::Foreman::ConfigurationManager < ManageIQ::Providers:: belongs_to :provider, :autosave => true, :dependent => :destroy + before_save :ensure_provider_name_and_zone + class << self delegate :params_for_create, :verify_credentials, @@ -56,4 +58,9 @@ def ensure_provider provider end + + def ensure_provider_name_and_zone + provider.name = name.split(" Configuration Manager").first + provider.zone = zone + end end From f2c005834842c3b9c4a47db936b501b374de1c3a Mon Sep 17 00:00:00 2001 From: Adam Grare Date: Fri, 15 May 2020 10:16:53 -0400 Subject: [PATCH 04/12] Safeguards around naming and suffixes --- .../manageiq/providers/foreman/configuration_manager.rb | 2 +- app/models/manageiq/providers/foreman/provider.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/models/manageiq/providers/foreman/configuration_manager.rb b/app/models/manageiq/providers/foreman/configuration_manager.rb index 28a598b..7e9ba22 100644 --- a/app/models/manageiq/providers/foreman/configuration_manager.rb +++ b/app/models/manageiq/providers/foreman/configuration_manager.rb @@ -60,7 +60,7 @@ def ensure_provider end def ensure_provider_name_and_zone - provider.name = name.split(" Configuration Manager").first + provider.name = name.sub(/ Configuration Manager$/, '') provider.zone = zone end end diff --git a/app/models/manageiq/providers/foreman/provider.rb b/app/models/manageiq/providers/foreman/provider.rb index 45844df..eb726c6 100644 --- a/app/models/manageiq/providers/foreman/provider.rb +++ b/app/models/manageiq/providers/foreman/provider.rb @@ -157,11 +157,11 @@ def verify_credentials(auth_type = nil, options = {}) def ensure_managers build_provisioning_manager unless provisioning_manager - provisioning_manager.name = "#{name} Provisioning Manager" + provisioning_manager.name = "#{name} Provisioning Manager" unless name.end_with?(" Provisioning Manager") provisioning_manager.provider = self build_configuration_manager unless configuration_manager - configuration_manager.name = "#{name} Configuration Manager" + configuration_manager.name = "#{name} Configuration Manager" unless name.end_with?(" Configuration Manager") configuration_manager.provider = self if zone_id_changed? From 422dda23a82032c66d48e6064c600fa644d22f22 Mon Sep 17 00:00:00 2001 From: Adam Grare Date: Mon, 18 May 2020 10:20:46 -0400 Subject: [PATCH 05/12] Make ensure_provider* methods private --- app/models/manageiq/providers/foreman/configuration_manager.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/models/manageiq/providers/foreman/configuration_manager.rb b/app/models/manageiq/providers/foreman/configuration_manager.rb index 7e9ba22..1b32b49 100644 --- a/app/models/manageiq/providers/foreman/configuration_manager.rb +++ b/app/models/manageiq/providers/foreman/configuration_manager.rb @@ -49,6 +49,8 @@ def self.display_name(number = 1) n_('Configuration Manager (Foreman)', 'Configuration Managers (Foreman)', number) end + private + def ensure_provider build_provider From 34fcf8fbaf1c87b187e1c2410fc2f4f187666515 Mon Sep 17 00:00:00 2001 From: Adam Grare Date: Wed, 24 Jun 2020 09:09:15 -0400 Subject: [PATCH 06/12] Override name and delegate name= to provider --- .../foreman/configuration_manager.rb | 21 +++++++------------ .../manageiq/providers/foreman/provider.rb | 9 -------- .../providers/foreman/provisioning_manager.rb | 16 ++++++++++++++ 3 files changed, 23 insertions(+), 23 deletions(-) diff --git a/app/models/manageiq/providers/foreman/configuration_manager.rb b/app/models/manageiq/providers/foreman/configuration_manager.rb index 1b32b49..38967c6 100644 --- a/app/models/manageiq/providers/foreman/configuration_manager.rb +++ b/app/models/manageiq/providers/foreman/configuration_manager.rb @@ -21,8 +21,6 @@ class ManageIQ::Providers::Foreman::ConfigurationManager < ManageIQ::Providers:: belongs_to :provider, :autosave => true, :dependent => :destroy - before_save :ensure_provider_name_and_zone - class << self delegate :params_for_create, :verify_credentials, @@ -41,6 +39,12 @@ def provider super || ensure_provider end + def name + "#{provider.name} Configuration Manager" + end + + delegate :name=, :zone, :zone=, :zone_id=, :to => :provider + def image_name "foreman_configuration" end @@ -52,17 +56,6 @@ def self.display_name(number = 1) private def ensure_provider - build_provider - - provider.configuration_manager = self - provider.name = name - provider.zone = zone - - provider - end - - def ensure_provider_name_and_zone - provider.name = name.sub(/ Configuration Manager$/, '') - provider.zone = zone + build_provider.tap { |p| p.configuration_manager = self } end end diff --git a/app/models/manageiq/providers/foreman/provider.rb b/app/models/manageiq/providers/foreman/provider.rb index eb726c6..f4af648 100644 --- a/app/models/manageiq/providers/foreman/provider.rb +++ b/app/models/manageiq/providers/foreman/provider.rb @@ -157,19 +157,10 @@ def verify_credentials(auth_type = nil, options = {}) def ensure_managers build_provisioning_manager unless provisioning_manager - provisioning_manager.name = "#{name} Provisioning Manager" unless name.end_with?(" Provisioning Manager") provisioning_manager.provider = self build_configuration_manager unless configuration_manager - configuration_manager.name = "#{name} Configuration Manager" unless name.end_with?(" Configuration Manager") configuration_manager.provider = self - - if zone_id_changed? - provisioning_manager.enabled = Zone.maintenance_zone&.id != zone_id - provisioning_manager.zone_id = zone_id - configuration_manager.enabled = Zone.maintenance_zone&.id != zone_id - configuration_manager.zone_id = zone_id - end end def self.refresh_ems(provider_ids) diff --git a/app/models/manageiq/providers/foreman/provisioning_manager.rb b/app/models/manageiq/providers/foreman/provisioning_manager.rb index 9469cca..2a0923f 100644 --- a/app/models/manageiq/providers/foreman/provisioning_manager.rb +++ b/app/models/manageiq/providers/foreman/provisioning_manager.rb @@ -31,4 +31,20 @@ def self.description def self.supported_for_create? false end + + def provider + super || ensure_provider + end + + def name + "#{provider.name} Provisioning Manager" + end + + delegate :name=, :zone, :zone=, :zone_id=, :to => :provider + + private + + def ensure_provider + build_provider.tap { |p| p.provisioning_manager = self } + end end From bc4d53bb6b9013e3348a3080c027f52fe166c7f6 Mon Sep 17 00:00:00 2001 From: Adam Grare Date: Wed, 24 Jun 2020 10:16:17 -0400 Subject: [PATCH 07/12] Strip suffix when setting provider name --- app/models/manageiq/providers/foreman/provider.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/models/manageiq/providers/foreman/provider.rb b/app/models/manageiq/providers/foreman/provider.rb index f4af648..cd98ea6 100644 --- a/app/models/manageiq/providers/foreman/provider.rb +++ b/app/models/manageiq/providers/foreman/provider.rb @@ -153,6 +153,10 @@ def verify_credentials(auth_type = nil, options = {}) raise MiqException::MiqInvalidCredentialsError, err.message, err.backtrace end + def name=(n) + super(n.sub(/ (Configuration|Provisioning) Manager$/, '')) + end + private def ensure_managers From 056b78646d59d3f319eaf3c0068f44947674e89a Mon Sep 17 00:00:00 2001 From: Adam Grare Date: Wed, 24 Jun 2020 10:39:16 -0400 Subject: [PATCH 08/12] Delegate zone_id to provider --- app/models/manageiq/providers/foreman/configuration_manager.rb | 2 +- app/models/manageiq/providers/foreman/provisioning_manager.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/manageiq/providers/foreman/configuration_manager.rb b/app/models/manageiq/providers/foreman/configuration_manager.rb index 38967c6..03c713b 100644 --- a/app/models/manageiq/providers/foreman/configuration_manager.rb +++ b/app/models/manageiq/providers/foreman/configuration_manager.rb @@ -43,7 +43,7 @@ def name "#{provider.name} Configuration Manager" end - delegate :name=, :zone, :zone=, :zone_id=, :to => :provider + delegate :name=, :zone, :zone=, :zone_id, :zone_id=, :to => :provider def image_name "foreman_configuration" diff --git a/app/models/manageiq/providers/foreman/provisioning_manager.rb b/app/models/manageiq/providers/foreman/provisioning_manager.rb index 2a0923f..7176f22 100644 --- a/app/models/manageiq/providers/foreman/provisioning_manager.rb +++ b/app/models/manageiq/providers/foreman/provisioning_manager.rb @@ -40,7 +40,7 @@ def name "#{provider.name} Provisioning Manager" end - delegate :name=, :zone, :zone=, :zone_id=, :to => :provider + delegate :name=, :zone, :zone=, :zone_id, :zone_id=, :to => :provider private From 9bc939a1e67f1dc72d254fb5ab7dc551e5f60dd9 Mon Sep 17 00:00:00 2001 From: Adam Grare Date: Wed, 24 Jun 2020 13:11:32 -0400 Subject: [PATCH 09/12] Delegate endpoints=/authentications= to provider --- app/models/manageiq/providers/foreman/configuration_manager.rb | 2 ++ app/models/manageiq/providers/foreman/provisioning_manager.rb | 2 ++ 2 files changed, 4 insertions(+) diff --git a/app/models/manageiq/providers/foreman/configuration_manager.rb b/app/models/manageiq/providers/foreman/configuration_manager.rb index 03c713b..8df9c34 100644 --- a/app/models/manageiq/providers/foreman/configuration_manager.rb +++ b/app/models/manageiq/providers/foreman/configuration_manager.rb @@ -11,8 +11,10 @@ class ManageIQ::Providers::Foreman::ConfigurationManager < ManageIQ::Providers:: :authentication_status, :authentication_status_ok?, :authentications, + :authentications=, :connect, :endpoints, + :endpoints=, :url, :url=, :verify_credentials, diff --git a/app/models/manageiq/providers/foreman/provisioning_manager.rb b/app/models/manageiq/providers/foreman/provisioning_manager.rb index 7176f22..1c15b2a 100644 --- a/app/models/manageiq/providers/foreman/provisioning_manager.rb +++ b/app/models/manageiq/providers/foreman/provisioning_manager.rb @@ -5,8 +5,10 @@ class ManageIQ::Providers::Foreman::ProvisioningManager < ManageIQ::Providers::P :authentication_status, :authentication_status_ok?, :authentications, + :authentications=, :connect, :endpoints, + :endpoints=, :verify_credentials, :with_provider_connection, :to => :provider From ccb02da8aa6cc258a3c5a4f60eb40c14cccf7cfe Mon Sep 17 00:00:00 2001 From: Adam Grare Date: Wed, 24 Jun 2020 13:31:56 -0400 Subject: [PATCH 10/12] Can't dependent destroy in all directions --- app/models/manageiq/providers/foreman/provider.rb | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/app/models/manageiq/providers/foreman/provider.rb b/app/models/manageiq/providers/foreman/provider.rb index cd98ea6..a813a30 100644 --- a/app/models/manageiq/providers/foreman/provider.rb +++ b/app/models/manageiq/providers/foreman/provider.rb @@ -2,12 +2,10 @@ class ManageIQ::Providers::Foreman::Provider < ::Provider has_one :configuration_manager, :foreign_key => "provider_id", :class_name => "ManageIQ::Providers::Foreman::ConfigurationManager", - :dependent => :destroy, :autosave => true has_one :provisioning_manager, :foreign_key => "provider_id", :class_name => "ManageIQ::Providers::Foreman::ProvisioningManager", - :dependent => :destroy, :autosave => true has_many :endpoints, :as => :resource, :dependent => :destroy, :autosave => true @@ -153,8 +151,8 @@ def verify_credentials(auth_type = nil, options = {}) raise MiqException::MiqInvalidCredentialsError, err.message, err.backtrace end - def name=(n) - super(n.sub(/ (Configuration|Provisioning) Manager$/, '')) + def name=(val) + super(val.sub(/ (Configuration|Provisioning) Manager$/, '')) end private @@ -165,6 +163,11 @@ def ensure_managers build_configuration_manager unless configuration_manager configuration_manager.provider = self + + if zone_id_changed? + provisioning_manager.enabled = Zone.maintenance_zone&.id != zone_id + configuration_manager.enabled = Zone.maintenance_zone&.id != zone_id + end end def self.refresh_ems(provider_ids) From dc9feda17a21005fc5fa1c81fb923b07f725e7ea Mon Sep 17 00:00:00 2001 From: Adam Grare Date: Tue, 30 Jun 2020 08:50:44 -0400 Subject: [PATCH 11/12] Add specs for create_from_params and edit_with_params --- .../foreman/configuration_manager_spec.rb | 45 ++++++++++++++++--- 1 file changed, 40 insertions(+), 5 deletions(-) diff --git a/spec/models/manageiq/providers/foreman/configuration_manager_spec.rb b/spec/models/manageiq/providers/foreman/configuration_manager_spec.rb index 24ef530..c34d8c8 100644 --- a/spec/models/manageiq/providers/foreman/configuration_manager_spec.rb +++ b/spec/models/manageiq/providers/foreman/configuration_manager_spec.rb @@ -1,13 +1,48 @@ describe ManageIQ::Providers::Foreman::ConfigurationManager do - let(:provider) { configuration_manager.provider } - let(:configuration_manager) do - FactoryBot.build(:configuration_manager_foreman) - end - describe "#connect" do + let(:provider) { configuration_manager.provider } + let(:configuration_manager) do + FactoryBot.build(:configuration_manager_foreman) + end + it "delegates to the provider" do expect(provider).to receive(:connect) configuration_manager.connect end end + + describe ".create_from_params" do + it "delegates endpoints, zone, name to provider" do + params = {:zone => FactoryBot.create(:zone), :name => "Foreman"} + endpoints = [{"role" => "default", "url" => "https://foreman", "verify_ssl" => 0}] + authentications = [{"authtype" => "default", "userid" => "admin", "password" => "smartvm"}] + + config_manager = described_class.create_from_params(params, endpoints, authentications) + + expect(config_manager.provider.name).to eq("Foreman") + expect(config_manager.provider.endpoints.count).to eq(1) + end + end + + describe "#edit_with_params" do + let(:configuration_manager) do + FactoryBot.build(:configuration_manager_foreman, :name => "Foreman", :url => "https://localhost") + end + + it "updates the provider" do + params = {:zone => FactoryBot.create(:zone), :name => "Foreman 2"} + endpoints = [{"role" => "default", "url" => "https://foreman", "verify_ssl" => 0}] + authentications = [{"authtype" => "default", "userid" => "admin", "password" => "smartvm"}] + + provider = configuration_manager.provider + expect(provider.name).to eq("Foreman") + expect(provider.url).to eq("https://localhost") + + configuration_manager.edit_with_params(params, endpoints, authentications) + + provider.reload + expect(provider.name).to eq("Foreman 2") + expect(provider.url).to eq("https://foreman") + end + end end From cfaaa5b187d0348cc6f7b243671c3f67036d229d Mon Sep 17 00:00:00 2001 From: Adam Grare Date: Wed, 1 Jul 2020 14:07:56 -0400 Subject: [PATCH 12/12] Override create_from_params/edit_with_params to save provider This is far from ideal, but the simplest way to ensure that changes to the foreman provider get saved when edited from the API --- .../foreman/configuration_manager.rb | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/app/models/manageiq/providers/foreman/configuration_manager.rb b/app/models/manageiq/providers/foreman/configuration_manager.rb index 8df9c34..619f6c7 100644 --- a/app/models/manageiq/providers/foreman/configuration_manager.rb +++ b/app/models/manageiq/providers/foreman/configuration_manager.rb @@ -37,6 +37,33 @@ def self.description @description ||= "Foreman Configuration".freeze end + def self.create_from_params(params, endpoints, authentications) + new(params).tap do |ems| + endpoints.each { |endpoint| ems.assign_nested_endpoint(endpoint) } + authentications.each { |authentication| ems.assign_nested_authentication(authentication) } + + ems.provider.save! + ems.save! + end + end + + def edit_with_params(params, endpoints, authentications) + tap do |ems| + transaction do + # Remove endpoints/attributes that are not arriving in the arguments above + ems.endpoints.where.not(:role => nil).where.not(:role => endpoints.map { |ep| ep['role'] }).delete_all + ems.authentications.where.not(:authtype => nil).where.not(:authtype => authentications.map { |au| au['authtype'] }).delete_all + + ems.assign_attributes(params) + ems.endpoints = endpoints.map(&method(:assign_nested_endpoint)) + ems.authentications = authentications.map(&method(:assign_nested_authentication)) + + ems.provider.save! + ems.save! + end + end + end + def provider super || ensure_provider end