Merge pull request #209 from miha-plesko/prevent-vapp-template-duplic…

…ates

Prevent vapp templates from being duplicated
(cherry picked from commit 901b9a1)

https://bugzilla.redhat.com/show_bug.cgi?id=1559628

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

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 = /<!-- (vappTemplate-[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}) -->/
+
   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)

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 = "<!-- #{uid} -->\n#{content}"
 
     new_result = {
       :type        => ManageIQ::Providers::Vmware::CloudManager::OrchestrationTemplate.name,

spec/fixtures/orchestration_templates/vmware_parameters_ovf.xml

@@ -1,3 +1,4 @@
+<!-- vappTemplate-05e4d68f-1a4e-40d5-9361-a121c1a67393 -->
 <?xml version="1.0" encoding="UTF-8"?>
 <ovf:Envelope xmlns:ovf="http://schemas.dmtf.org/ovf/envelope/1" xmlns:vcloud="http://www.vmware.com/vcloud/v1.5" xmlns:vssd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_VirtualSystemSettingData" xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData" xmlns:vmw="http://www.vmware.com/schema/ovf" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_VirtualSystemSettingData http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2.22.0/CIM_VirtualSystemSettingData.xsd http://www.vmware.com/schema/ovf http://www.vmware.com/schema/ovf http://schemas.dmtf.org/ovf/envelope/1 http://schemas.dmtf.org/ovf/envelope/1/dsp8023_1.1.0.xsd http://www.vmware.com/vcloud/v1.5 http://vcd-portal.vmware.local/api/v1.5/schema/master.xsd http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2.22.0/CIM_ResourceAllocationSettingData.xsd">
     <ovf:References/>

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

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
+        <!-- vappTemplate-05e4d68f-1a4e-40d5-9361-a121c1a67393 -->
+        <?xml version="1.0" encoding="UTF-8"?>
+        <envelope />
+      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
+        <!-- hi im unexpected -->
+        <?xml version="1.0" encoding="UTF-8"?>
+        <envelope />
+      CONENT
+      expect(described_class.template_ems_ref(content)).to be nil
+    end
+
+    it 'without preceeding comment' do
+      content = <<-CONENT
+        <?xml version="1.0" encoding="UTF-8"?>
+        <envelope />
+      CONENT
+      expect(described_class.template_ems_ref(content)).to be nil
+    end
+  end
 end

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