[Vmware] Fix for ovf templates with prefix (#5448)

* [Vmware] Fix for ovf templates with prefix

* Support multiple hardware versions

api/src/main/java/com/cloud/agent/api/storage/OVFHelper.java

@@ -115,6 +115,20 @@ public class OVFHelper {
         return null;
     }
 
+    /**
+     * Check if there are elements matching the tag name, otherwise check prepending the prefix
+     */
+    public NodeList getElementsByTagNameAndPrefix(Document doc, String name, String prefix) {
+        if (doc == null) {
+            return null;
+        }
+        NodeList elementsByTagName = doc.getElementsByTagName(name);
+        if (elementsByTagName.getLength() > 0) {
+            return elementsByTagName;
+        }
+        return doc.getElementsByTagName(String.format("%s:%s", prefix, name));
+    }
+
     /**
      * Check if the attribute is present on the element, otherwise check preprending ':'
      */
@@ -179,7 +193,7 @@ public class OVFHelper {
             return props;
         }
         int propertyIndex = 0;
-        NodeList productSections = doc.getElementsByTagName("ProductSection");
+        NodeList productSections = getElementsByTagNameAndPrefix(doc, "ProductSection", "ovf");
         if (productSections != null) {
             String lastCategoryFound = null;
             for (int i = 0; i < productSections.getLength(); i++) {
@@ -193,10 +207,12 @@ public class OVFHelper {
                     if (child == null) {
                         continue;
                     }
-                    if (child.getNodeName().equalsIgnoreCase("Category")) {
+                    if (child.getNodeName().equalsIgnoreCase("Category") ||
+                            child.getNodeName().equalsIgnoreCase("ovf:Category")) {
                         lastCategoryFound = child.getTextContent();
                         s_logger.info("Category found " + lastCategoryFound);
-                    } else if (child.getNodeName().equalsIgnoreCase("Property")) {
+                    } else if (child.getNodeName().equalsIgnoreCase("Property") ||
+                            child.getNodeName().equalsIgnoreCase("ovf:Property")) {
                         OVFPropertyTO prop = createOVFPropertyFromNode(child, propertyIndex, lastCategoryFound);
                         if (prop != null && prop.isUserConfigurable()) {
                             props.add(prop);
@@ -398,7 +414,7 @@ public class OVFHelper {
     }
 
     protected List<OVFFile> extractFilesFromOvfDocumentTree(File ovfFile, Document doc) {
-        NodeList files = doc.getElementsByTagName("File");
+        NodeList files = getElementsByTagNameAndPrefix(doc, "File", "ovf");
         ArrayList<OVFFile> vf = new ArrayList<>();
         boolean toggle = true;
         for (int j = 0; j < files.getLength(); j++) {
@@ -514,9 +530,9 @@ public class OVFHelper {
     public void rewriteOVFFileForSingleDisk(final String origOvfFilePath, final String newOvfFilePath, final String diskName) {
         final Document doc = getDocumentFromFile(origOvfFilePath);
 
-        NodeList disks = doc.getElementsByTagName("Disk");
+        NodeList disks = getElementsByTagNameAndPrefix(doc, "Disk", "ovf");
-        NodeList files = doc.getElementsByTagName("File");
+        NodeList files = getElementsByTagNameAndPrefix(doc, "File", "ovf");
-        NodeList items = doc.getElementsByTagName("Item");
+        NodeList items = getElementsByTagNameAndPrefix(doc, "Item", "ovf");
         String keepfile = null;
         List<Element> toremove = new ArrayList<>();
         for (int j = 0; j < files.getLength(); j++) {
@@ -677,7 +693,7 @@ public class OVFHelper {
 
     private void checkForOnlyOneSystemNode(Document doc) throws InternalErrorException {
         // get hardware VirtualSystem, for now we support only one of those
-        NodeList systemElements = doc.getElementsByTagName("VirtualSystem");
+        NodeList systemElements = getElementsByTagNameAndPrefix(doc, "VirtualSystem", "ovf");
         if (systemElements.getLength() != 1) {
             String msg = "found " + systemElements.getLength() + " system definitions in OVA, can only handle exactly one.";
             s_logger.warn(msg);
@@ -686,7 +702,7 @@ public class OVFHelper {
     }
 
     private Map<String, OVFNetworkTO> getNetworksFromDocumentTree(Document doc) {
-        NodeList networkElements = doc.getElementsByTagName("Network");
+        NodeList networkElements = getElementsByTagNameAndPrefix(doc,"Network", "ovf");
         Map<String, OVFNetworkTO> nets = new HashMap<>();
         for (int i = 0; i < networkElements.getLength(); i++) {
 
@@ -746,7 +762,7 @@ public class OVFHelper {
     private String getMinimumHardwareVersionFromDocumentTree(Document doc) {
         String version = null;
         if (doc != null) {
-            NodeList systemNodeList = doc.getElementsByTagName("System");
+            NodeList systemNodeList = getElementsByTagNameAndPrefix(doc, "System", "ovf");
             if (systemNodeList.getLength() != 0) {
                 Node systemItem = systemNodeList.item(0);
                 String hardwareVersions = getChildNodeValue(systemItem, "VirtualSystemType");
@@ -766,7 +782,7 @@ public class OVFHelper {
         if (doc == null) {
             return options;
         }
-        NodeList deploymentOptionSection = doc.getElementsByTagName("DeploymentOptionSection");
+        NodeList deploymentOptionSection = getElementsByTagNameAndPrefix(doc,"DeploymentOptionSection", "ovf");
         if (deploymentOptionSection.getLength() == 0) {
             return options;
         }
@@ -775,7 +791,7 @@ public class OVFHelper {
         int index = 0;
         for (int i = 0; i < childNodes.getLength(); i++) {
             Node node = childNodes.item(i);
-            if (node != null && node.getNodeName().equals("Configuration")) {
+            if (node != null && (node.getNodeName().equals("Configuration") || node.getNodeName().equals("ovf:Configuration"))) {
                 Element configuration = (Element) node;
                 String configurationId = getNodeAttribute(configuration,"ovf","id");
                 String description = getChildNodeValue(configuration, "Description");
@@ -793,7 +809,7 @@ public class OVFHelper {
         if (doc == null) {
             return items;
         }
-        NodeList hardwareSection = doc.getElementsByTagName("VirtualHardwareSection");
+        NodeList hardwareSection = getElementsByTagNameAndPrefix(doc, "VirtualHardwareSection","ovf");
         if (hardwareSection.getLength() == 0) {
             return items;
         }
@@ -801,7 +817,7 @@ public class OVFHelper {
         NodeList childNodes = hardwareSectionNode.getChildNodes();
         for (int i = 0; i < childNodes.getLength(); i++) {
             Node node = childNodes.item(i);
-            if (node != null && node.getNodeName().equals("Item")) {
+            if (node != null && (node.getNodeName().equals("Item") || node.getNodeName().equals("ovf:Item"))) {
                 Element configuration = (Element) node;
                 String configurationIds = getNodeAttribute(configuration, "ovf", "configuration");
                 String allocationUnits = getChildNodeValue(configuration, "AllocationUnits");
@@ -869,7 +885,7 @@ public class OVFHelper {
         if (doc == null) {
             return eulas;
         }
-        NodeList eulaSections = doc.getElementsByTagName("EulaSection");
+        NodeList eulaSections = getElementsByTagNameAndPrefix(doc, "EulaSection", "ovf");
         int eulaIndex = 0;
         if (eulaSections.getLength() > 0) {
             for (int index = 0; index < eulaSections.getLength(); index++) {
@@ -905,7 +921,7 @@ public class OVFHelper {
         if (doc == null) {
             return null;
         }
-        NodeList guesOsList = doc.getElementsByTagName("OperatingSystemSection");
+        NodeList guesOsList = getElementsByTagNameAndPrefix(doc, "OperatingSystemSection", "ovf");
         if (guesOsList.getLength() == 0) {
             return null;
         }

core/src/main/java/com/cloud/storage/template/OVAProcessor.java

@@ -243,7 +243,7 @@ public class OVAProcessor extends AdapterBase implements Processor {
         try {
             Document ovfDoc = null;
             ovfDoc = DocumentBuilderFactory.newInstance().newDocumentBuilder().parse(new File(ovfFileName));
-            NodeList diskElements = ovfDoc.getElementsByTagName("Disk");
+            NodeList diskElements = new OVFHelper().getElementsByTagNameAndPrefix(ovfDoc, "Disk", "ovf");
             for (int i = 0; i < diskElements.getLength(); i++) {
                 Element disk = (Element)diskElements.item(i);
                 long diskSize = Long.parseLong(disk.getAttribute("ovf:capacity"));
@@ -259,40 +259,6 @@ public class OVAProcessor extends AdapterBase implements Processor {
         }
     }
 
-    public Pair<Long, Long> getDiskDetails(String ovfFilePath, String diskName) throws InternalErrorException {
-        long virtualSize = 0;
-        long fileSize = 0;
-        String fileId = null;
-        try {
-            Document ovfDoc = null;
-            ovfDoc = DocumentBuilderFactory.newInstance().newDocumentBuilder().parse(new File(ovfFilePath));
-            NodeList disks = ovfDoc.getElementsByTagName("Disk");
-            NodeList files = ovfDoc.getElementsByTagName("File");
-            for (int j = 0; j < files.getLength(); j++) {
-                Element file = (Element)files.item(j);
-                if (file.getAttribute("ovf:href").equals(diskName)) {
-                    fileSize = Long.parseLong(file.getAttribute("ovf:size"));
-                    fileId = file.getAttribute("ovf:id");
-                    break;
-                }
-            }
-            for (int i = 0; i < disks.getLength(); i++) {
-                Element disk = (Element)disks.item(i);
-                if (disk.getAttribute("ovf:fileRef").equals(fileId)) {
-                    virtualSize = Long.parseLong(disk.getAttribute("ovf:capacity"));
-                    String allocationUnits = disk.getAttribute("ovf:capacityAllocationUnits");
-                    virtualSize = OVFHelper.getDiskVirtualSize(virtualSize, allocationUnits, ovfFilePath);
-                    break;
-                }
-            }
-            return new Pair<Long, Long>(virtualSize, fileSize);
-        } catch (InternalErrorException | IOException | NumberFormatException | ParserConfigurationException | SAXException e) {
-            String msg = "getDiskDetails: Unable to parse OVF XML document " + ovfFilePath + " to get the virtual disk " + diskName + " size due to " + e;
-            LOGGER.error(msg);
-            throw new InternalErrorException(msg);
-        }
-    }
-
     private String getOVFFilePath(String srcOVAFileName) {
         File file = new File(srcOVAFileName);
         assert (_storage != null);

engine/storage/src/main/java/org/apache/cloudstack/storage/image/deployasis/DeployAsIsHelperImpl.java

@@ -256,8 +256,7 @@ public class DeployAsIsHelperImpl implements DeployAsIsHelper {
     /**
      * Minimum VMware hosts supported version is 6.0
      */
-    protected String getMinimumSupportedHypervisorVersionForHardwareVersion(String hardwareVersion) {
+    protected String mapHardwareVersionToHypervisorVersion(String hardwareVersion) {
-        // From https://kb.vmware.com/s/article/1003746 and https://kb.vmware.com/s/article/2007240
         String hypervisorVersion = "default";
         if (StringUtils.isBlank(hardwareVersion)) {
             return hypervisorVersion;
@@ -278,6 +277,34 @@ public class DeployAsIsHelperImpl implements DeployAsIsHelper {
         return hypervisorVersion;
     }
 
+    /**
+     * Retrieve the minimal hypervisor version for a single or multiple hardware version(s) in the parameter
+     */
+    protected String getMinimumSupportedHypervisorVersionForHardwareVersion(String hardwareVersion) {
+        // From https://kb.vmware.com/s/article/1003746 and https://kb.vmware.com/s/article/2007240
+        String hypervisorVersion = "default";
+        if (StringUtils.isBlank(hardwareVersion)) {
+            return hypervisorVersion;
+        }
+        if (hardwareVersion.contains(" ")) {
+            String[] versions = hardwareVersion.split(" ");
+            String minVersion = null;
+            for (String version : versions) {
+                String hvVersion = mapHardwareVersionToHypervisorVersion(version);
+                if (minVersion == null) {
+                    minVersion = hvVersion;
+                } else if (hvVersion.equalsIgnoreCase("default")) {
+                    return minVersion;
+                } else if (hvVersion.compareTo(minVersion) < 0) {
+                    minVersion = hvVersion;
+                }
+            }
+            return minVersion;
+        } else {
+            return mapHardwareVersionToHypervisorVersion(hardwareVersion);
+        }
+    }
+
     @Override
     public Map<String, String> getVirtualMachineDeployAsIsProperties(VirtualMachineProfile vm) {
         Map<String, String> map = new HashMap<>();

engine/storage/src/test/java/org/apache/cloudstack/storage/image/deployasis/DeployAsIsHelperImplTest.java

@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.cloudstack.storage.image.deployasis;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class DeployAsIsHelperImplTest {
+
+    private DeployAsIsHelperImpl deployAsIsHelper = new DeployAsIsHelperImpl();
+
+    private static final String singleHardwareVersionDefinition = "vmx-13";
+    private static final String multipleHardwareVersionDefinition = "vmx-10 vmx-11 vmx-13";
+    private static final String multipleHardwareVersionDefinitionUnordered = "vmx-13 vmx-8 vmx-10";
+
+    @Test
+    public void testGetMinimumSupportedHypervisorVersionForHardwareVersion() {
+        String vmwareVersion = deployAsIsHelper.getMinimumSupportedHypervisorVersionForHardwareVersion(singleHardwareVersionDefinition);
+        Assert.assertEquals("6.5", vmwareVersion);
+    }
+
+    @Test
+    public void testGetMinimumSupportedHypervisorVersionForMultipleHardwareVersion() {
+        String vmwareVersion = deployAsIsHelper.getMinimumSupportedHypervisorVersionForHardwareVersion(multipleHardwareVersionDefinition);
+        Assert.assertEquals("6.0", vmwareVersion);
+    }
+
+    @Test
+    public void testGetMinimumSupportedHypervisorVersionForMultipleUnorderedHardwareVersion() {
+        String vmwareVersion = deployAsIsHelper.getMinimumSupportedHypervisorVersionForHardwareVersion(multipleHardwareVersionDefinitionUnordered);
+        Assert.assertEquals("6.0", vmwareVersion);
+    }
+}