From 772a49cfb5c177451d71f31f199be0cc66f19cdb Mon Sep 17 00:00:00 2001 From: Adam Grare Date: Thu, 22 Mar 2018 14:39:05 -0400 Subject: [PATCH] Merge pull request #209 from miha-plesko/prevent-vapp-template-duplicates Prevent vapp templates from being duplicated (cherry picked from commit 901b9a10c9aad7a0f95b29313cbd76bb815ca00a) https://bugzilla.redhat.com/show_bug.cgi?id=1559628 --- .../cloud_manager/orchestration_template.rb | 9 ++++++ .../vmware/cloud_manager/ovf_template.rb | 11 ++++++++ .../vmware/cloud_manager/refresh_parser.rb | 2 ++ .../vmware_parameters_ovf.xml | 1 + .../orchestration_template_spec.rb | 5 ++++ .../vmware/cloud_manager/ovf_template_spec.rb | 28 +++++++++++++++++++ .../vmware/cloud_manager/refresher_spec.rb | 2 +- 7 files changed, 57 insertions(+), 1 deletion(-) diff --git a/app/models/manageiq/providers/vmware/cloud_manager/orchestration_template.rb b/app/models/manageiq/providers/vmware/cloud_manager/orchestration_template.rb index 35b1c70de..feb598d69 100644 --- a/app/models/manageiq/providers/vmware/cloud_manager/orchestration_template.rb +++ b/app/models/manageiq/providers/vmware/cloud_manager/orchestration_template.rb @@ -298,4 +298,13 @@ def ip_constraint :description => 'IP address' ) end + + # Override md5 calculation on vapp templates because XML elements ordering is not guaranteed. + # We observed annoying fact that vCloud returns randomly shuffled XML content for very same + # vapp template, therefore MD5 content differs. For this reason we need to override md5 calculation + # to return unique identifier instead actual checksum. Luckily, vapp templates are not modifyable on + # vCloud dashboard, so we don't really need the checksum like other providers. + def self.calc_md5(text) + ManageIQ::Providers::Vmware::CloudManager::OvfTemplate.template_ems_ref(text) if text + end end diff --git a/app/models/manageiq/providers/vmware/cloud_manager/ovf_template.rb b/app/models/manageiq/providers/vmware/cloud_manager/ovf_template.rb index 304982719..ecea54de4 100644 --- a/app/models/manageiq/providers/vmware/cloud_manager/ovf_template.rb +++ b/app/models/manageiq/providers/vmware/cloud_manager/ovf_template.rb @@ -7,6 +7,8 @@ class OvfParseError < StandardError; end OvfVappNetwork = Struct.new(:name, :mode, :subnets) OvfSubnet = Struct.new(:gateway, :netmask, :dns1, :dns2) + RESERVED_LINE_REGEX = // + def initialize(ovf_string) @vms = [] @vapp_networks = [] @@ -31,6 +33,15 @@ def vapp_net_name_from_idx(vapp_net_idx) @vapp_networks[vapp_net_idx].name if @vapp_networks[vapp_net_idx] end + # Extract ems_ref of the template from the very first line of the XML. This line is supposed to + # be put there by refresh parser. + def self.template_ems_ref(ovf_string) + return unless ovf_string + if (reserved_line = ovf_string.lines.first) && (param_match = reserved_line.match(RESERVED_LINE_REGEX)) + param_match.captures.first + end + end + private def parse(ovf_string) diff --git a/app/models/manageiq/providers/vmware/cloud_manager/refresh_parser.rb b/app/models/manageiq/providers/vmware/cloud_manager/refresh_parser.rb index 8541393dd..40b9c5a25 100644 --- a/app/models/manageiq/providers/vmware/cloud_manager/refresh_parser.rb +++ b/app/models/manageiq/providers/vmware/cloud_manager/refresh_parser.rb @@ -203,6 +203,8 @@ def parse_vapp_template(vapp_template) # The content of the template is the OVF specification of the vApp template content = @connection.get_vapp_template_ovf_descriptor(uid).body + # Prepend comment containing template uid which is then used istead of MD5 checksum. + content = "\n#{content}" new_result = { :type => ManageIQ::Providers::Vmware::CloudManager::OrchestrationTemplate.name, diff --git a/spec/fixtures/orchestration_templates/vmware_parameters_ovf.xml b/spec/fixtures/orchestration_templates/vmware_parameters_ovf.xml index 2aab7fb1c..077aad03a 100644 --- a/spec/fixtures/orchestration_templates/vmware_parameters_ovf.xml +++ b/spec/fixtures/orchestration_templates/vmware_parameters_ovf.xml @@ -1,3 +1,4 @@ + diff --git a/spec/models/manageiq/providers/vmware/cloud_manager/orchestration_template_spec.rb b/spec/models/manageiq/providers/vmware/cloud_manager/orchestration_template_spec.rb index 6c9497c75..f87ee192e 100644 --- a/spec/models/manageiq/providers/vmware/cloud_manager/orchestration_template_spec.rb +++ b/spec/models/manageiq/providers/vmware/cloud_manager/orchestration_template_spec.rb @@ -46,6 +46,11 @@ expect(ovf_doc).not_to be(nil) expect(ovf_doc.root.name).not_to be("Envelope") end + + it "contains template ems_ref" do + ems_ref = described_class.calc_md5(valid_template.content) + expect(ems_ref).to eq('vappTemplate-05e4d68f-1a4e-40d5-9361-a121c1a67393') + end end context "orchestration template" do diff --git a/spec/models/manageiq/providers/vmware/cloud_manager/ovf_template_spec.rb b/spec/models/manageiq/providers/vmware/cloud_manager/ovf_template_spec.rb index 490f937c5..7d6d68203 100644 --- a/spec/models/manageiq/providers/vmware/cloud_manager/ovf_template_spec.rb +++ b/spec/models/manageiq/providers/vmware/cloud_manager/ovf_template_spec.rb @@ -82,4 +82,32 @@ ) end end + + describe '#template_ems_ref' do + it 'with expected preceeding comment' do + content = <<-CONENT + + + + CONENT + expect(described_class.template_ems_ref(content)).to eq('vappTemplate-05e4d68f-1a4e-40d5-9361-a121c1a67393') + end + + it 'with unexpected preceeding comment' do + content = <<-CONENT + + + + CONENT + expect(described_class.template_ems_ref(content)).to be nil + end + + it 'without preceeding comment' do + content = <<-CONENT + + + CONENT + expect(described_class.template_ems_ref(content)).to be nil + end + end end diff --git a/spec/models/manageiq/providers/vmware/cloud_manager/refresher_spec.rb b/spec/models/manageiq/providers/vmware/cloud_manager/refresher_spec.rb index 77e846c2e..5abf82377 100644 --- a/spec/models/manageiq/providers/vmware/cloud_manager/refresher_spec.rb +++ b/spec/models/manageiq/providers/vmware/cloud_manager/refresher_spec.rb @@ -347,7 +347,7 @@ def assert_specific_orchestration_template expect(@template.ems_id).to eq(@ems.id) expect(@template.content.include?('ovf:Envelope')).to be_truthy - expect(@template.md5).to eq('729bfcafe52065bdee376931efe104d9') + expect(@template.md5).to eq('vappTemplate-a19bdc8f-88fa-4dd6-8436-486590353ed5') end def assert_specific_vm_with_snapshot