OVF: improve ds-identify to support finding OVF iso transport.

Previously the OVF transport would not be identified except for when
config files set 'ovf_vmware_guest_customization'.  It would also
return DS_MAYBE almost always.

The change here is to add support to ds-identify for storing the
iso9660 filesystems that it finds (ISO9660_DEVS). Then the OVF check
will check that the iso9660 filesystem has ovf-env.xml on it. The least
wonderful part of this is that the check is done by 'grep' for case
insensitive ovf-env.xml.

Future improvement would be to identify VMware's OVF by label or UUID
so we could avoid the grep.

LP: #1731868

2 files changed, 161 insertions, 36 deletions

diff --git a/tests/unittests/test_ds_identify.py b/tests/unittests/test_ds_identify.py
index 7a920d42..3f1a6712 100644
--- a/tests/unittests/test_ds_identify.py
+++ b/tests/unittests/test_ds_identify.py
@@ -32,6 +32,7 @@ POLICY_FOUND_OR_MAYBE = "search,found=all,maybe=all,notfound=disabled"
 DI_DEFAULT_POLICY = "search,found=all,maybe=all,notfound=enabled"
 DI_DEFAULT_POLICY_NO_DMI = "search,found=all,maybe=all,notfound=disabled"
 DI_EC2_STRICT_ID_DEFAULT = "true"
+OVF_MATCH_STRING = 'http://schemas.dmtf.org/ovf/environment/1'
 
 SHELL_MOCK_TMPL = """\
 %(name)s() {
@@ -55,6 +56,7 @@ P_SEED_DIR = "var/lib/cloud/seed"
 P_DSID_CFG = "etc/cloud/ds-identify.cfg"
 
 MOCK_VIRT_IS_KVM = {'name': 'detect_virt', 'RET': 'kvm', 'ret': 0}
+MOCK_VIRT_IS_VMWARE = {'name': 'detect_virt', 'RET': 'vmware', 'ret': 0}
 MOCK_UNAME_IS_PPC64 = {'name': 'uname', 'out': UNAME_PPC64EL, 'ret': 0}
 
 
@@ -296,6 +298,48 @@ class TestDsIdentify(CiTestCase):
             data, RC_FOUND, dslist=['OpenStack', 'None'])
         self.assertIn("check for 'OpenStack' returned maybe", err)
 
+    def test_default_ovf_is_found(self):
+        """OVF is identified found when ovf/ovf-env.xml seed file exists."""
+        self._test_ds_found('OVF-seed')
+
+    def test_default_ovf_with_detect_virt_none_not_found(self):
+        """OVF identifies not found when detect_virt returns "none"."""
+        self._check_via_dict(
+            {'ds': 'OVF'}, rc=RC_NOT_FOUND, policy_dmi="disabled")
+
+    def test_default_ovf_returns_not_found_on_azure(self):
+        """OVF datasource won't be found as false positive on Azure."""
+        ovfonazure = copy.deepcopy(VALID_CFG['OVF'])
+        # Set azure asset tag to assert OVF content not found
+        ovfonazure['files'][P_CHASSIS_ASSET_TAG] = (
+            '7783-7084-3265-9085-8269-3286-77\n')
+        self._check_via_dict(
+            ovfonazure, RC_FOUND, dslist=['Azure', DS_NONE])
+
+    def test_ovf_on_vmware_iso_found_by_cdrom_with_ovf_schema_match(self):
+        """OVF is identified when iso9660 cdrom path contains ovf schema."""
+        self._test_ds_found('OVF')
+
+    def test_ovf_on_vmware_iso_found_when_vmware_customization(self):
+        """OVF is identified when vmware customization is enabled."""
+        self._test_ds_found('OVF-vmware-customization')
+
+    def test_ovf_on_vmware_iso_found_by_cdrom_with_matching_fs_label(self):
+        """OVF is identified when iso9660 cdrom  label has ovf-transport."""
+        ovf_cdrom_by_label = copy.deepcopy(VALID_CFG['OVF'])
+        # Unset matching cdrom ovf schema content
+        ovf_cdrom_by_label['files']['dev/sr0'] = 'No content match'
+        self._check_via_dict(
+            ovf_cdrom_by_label, rc=RC_NOT_FOUND, policy_dmi="disabled")
+
+        # Add recognized labels
+        for valid_fs_label in ['ovf-transport', 'OVF-TRANSPORT']:
+            ovf_cdrom_by_label['mocks'][0]['out'] = blkid_out([
+                {'DEVNAME': 'sr0', 'TYPE': 'iso9660',
+                 'LABEL': valid_fs_label}])
+            self._check_via_dict(
+                ovf_cdrom_by_label, rc=RC_FOUND, dslist=['OVF', DS_NONE])
+
 
 def blkid_out(disks=None):
     """Convert a list of disk dictionaries into blkid content."""
@@ -305,7 +349,9 @@ def blkid_out(disks=None):
     for disk in disks:
         if not disk["DEVNAME"].startswith("/dev/"):
             disk["DEVNAME"] = "/dev/" + disk["DEVNAME"]
-        for key in disk:
+        # devname needs to be first.
+        lines.append("%s=%s" % ("DEVNAME", disk["DEVNAME"]))
+        for key in [d for d in disk if d != "DEVNAME"]:
             lines.append("%s=%s" % (key, disk[key]))
         lines.append("")
     return '\n'.join(lines)
@@ -383,6 +429,43 @@ VALID_CFG = {
         'policy_dmi': POLICY_FOUND_ONLY,
         'policy_no_dmi': POLICY_FOUND_ONLY,
     },
+    'OVF-seed': {
+        'ds': 'OVF',
+        'files': {
+            os.path.join(P_SEED_DIR, 'ovf', 'ovf-env.xml'): 'present\n',
+        }
+    },
+    'OVF-vmware-customization': {
+        'ds': 'OVF',
+        'mocks': [
+            # Include a mockes iso9660 potential, even though content not ovf
+            {'name': 'blkid', 'ret': 0,
+             'out': blkid_out(
+                 [{'DEVNAME': 'sr0', 'TYPE': 'iso9660', 'LABEL': ''}])
+             },
+            MOCK_VIRT_IS_VMWARE,
+        ],
+        'files': {
+            'dev/sr0': 'no match',
+            # Setup vmware customization enabled
+            'usr/lib/vmware-tools/plugins/vmsvc/libdeployPkgPlugin.so': 'here',
+            'etc/cloud/cloud.cfg': 'disable_vmware_customization: false\n',
+        }
+    },
+    'OVF': {
+        'ds': 'OVF',
+        'mocks': [
+            {'name': 'blkid', 'ret': 0,
+             'out': blkid_out(
+                 [{'DEVNAME': 'vda1', 'TYPE': 'vfat', 'PARTUUID': uuid4()},
+                  {'DEVNAME': 'sr0', 'TYPE': 'iso9660', 'LABEL': ''}])
+             },
+            MOCK_VIRT_IS_VMWARE,
+        ],
+        'files': {
+            'dev/sr0': 'pretend ovf iso has ' + OVF_MATCH_STRING + '\n',
+        }
+    },
     'ConfigDrive': {
         'ds': 'ConfigDrive',
         'mocks': [
diff --git a/tools/ds-identify b/tools/ds-identify
index ee5e05a4..4c59d7bc 100755
--- a/tools/ds-identify
+++ b/tools/ds-identify
@@ -83,6 +83,7 @@ _DI_LOGGED=""
 # set DI_MAIN='noop' in environment to source this file with no main called.
 DI_MAIN=${DI_MAIN:-main}
 
+DI_BLKID_OUTPUT=""
 DI_DEFAULT_POLICY="search,found=all,maybe=all,notfound=${DI_DISABLED}"
 DI_DEFAULT_POLICY_NO_DMI="search,found=all,maybe=all,notfound=${DI_ENABLED}"
 DI_DMI_CHASSIS_ASSET_TAG=""
@@ -91,6 +92,7 @@ DI_DMI_SYS_VENDOR=""
 DI_DMI_PRODUCT_SERIAL=""
 DI_DMI_PRODUCT_UUID=""
 DI_FS_LABELS=""
+DI_ISO9660_DEVS=""
 DI_KERNEL_CMDLINE=""
 DI_VIRT=""
 DI_PID_1_PRODUCT_NAME=""
@@ -181,32 +183,43 @@ block_dev_with_label() {
     return 0
 }
 
-read_fs_labels() {
-    cached "${DI_FS_LABELS}" && return 0
+read_fs_info() {
+    cached "${DI_BLKID_OUTPUT}" && return 0
     # do not rely on links in /dev/disk which might not be present yet.
     # note that older blkid versions do not report DEVNAME in 'export' output.
-    local out="" ret=0 oifs="$IFS" line="" delim=","
-    local labels=""
     if is_container; then
         # blkid will in a container, or at least currently in lxd
         # not provide useful information.
         DI_FS_LABELS="$UNAVAILABLE:container"
-    else
-        out=$(blkid -c /dev/null -o export) || {
-            ret=$?
-            error "failed running [$ret]: blkid -c /dev/null -o export"
-            return $ret
-        }
-        IFS="$CR"
-        set -- $out
-        IFS="$oifs"
-        for line in "$@"; do
-            case "${line}" in
-                LABEL=*) labels="${labels}${line#LABEL=}${delim}";;
-            esac
-        done
-        DI_FS_LABELS="${labels%${delim}}"
+        DI_ISO9660_DEVS="$UNAVAILABLE:container"
+        return
     fi
+    local oifs="$IFS" line="" delim=","
+    local ret=0 out="" labels="" dev="" label="" ftype="" isodevs=""
+    out=$(blkid -c /dev/null -o export) || {
+        ret=$?
+        error "failed running [$ret]: blkid -c /dev/null -o export"
+        DI_FS_LABELS="$UNAVAILABLE:error"
+        DI_ISO9660_DEVS="$UNAVAILABLE:error"
+        return $ret
+    }
+    IFS="$CR"
+    set -- $out
+    IFS="$oifs"
+    for line in "$@" ""; do
+        case "${line}" in
+            DEVNAME=*) dev=${line#DEVNAME=};;
+            LABEL=*) label="${line#LABEL=}";
+                     labels="${labels}${line#LABEL=}${delim}";;
+            TYPE=*) ftype=${line#TYPE=};;
+            "") if [ "$ftype" = "iso9660" ]; then
+                    isodevs="${isodevs} ${dev}=$label"
+                fi
+                ftype=""; devname=""; label="";
+        esac
+    done
+    DI_FS_LABELS="${labels%${delim}}"
+    DI_ISO9660_DEVS="${isodevs# }"
 }
 
 cached() {
@@ -214,10 +227,6 @@ cached() {
 }
 
 
-has_cdrom() {
-    [ -e "${PATH_ROOT}/dev/cdrom" ]
-}
-
 detect_virt() {
     local virt="${UNAVAILABLE}" r="" out=""
     if [ -d /run/systemd ]; then
@@ -621,14 +630,13 @@ ovf_vmware_guest_customization() {
     [ "${DI_VIRT}" = "vmware" ] || return 1
 
     # we have to have the plugin to do vmware customization
-    local found="" pkg="" pre="/usr/lib"
+    local found="" pkg="" pre="${PATH_ROOT}/usr/lib"
     for pkg in vmware-tools open-vm-tools; do
         if [ -f "$pre/$pkg/plugins/vmsvc/libdeployPkgPlugin.so" ]; then
             found="$pkg"; break;
         fi
     done
     [ -n "$found" ] || return 1
-
     # vmware customization is disabled by default
     # (disable_vmware_customization=true). If it is set to false, then
     # user has requested customization.
@@ -644,20 +652,55 @@ ovf_vmware_guest_customization() {
     return 1
 }
 
+is_cdrom_ovf() {
+    local dev="$1" label="$2"
+    # skip devices that don't look like cdrom paths.
+    case "$dev" in
+        /dev/sr[0-9]|/dev/hd[a-z]) :;;
+        *) debug 1 "skipping iso dev $d"
+           return 1;;
+    esac
+
+    # fast path known 'OVF' labels
+    [ "$label" = "OVF-TRANSPORT" -o "$label" = "ovf-transport" ] && return 0
+
+    # explicitly skip known labels of other types. rd_rdfe is azure.
+    case "$label" in
+        config-2|rd_rdfe_stable*) return 1;;
+    esac
+
+    local idstr="http://schemas.dmtf.org/ovf/environment/1"
+    grep --quiet --ignore-case "$idstr" "${PATH_ROOT}$dev"
+}
+
 dscheck_OVF() {
-    local p=""
     check_seed_dir ovf ovf-env.xml && return "${DS_FOUND}"
 
+    [ "${DI_VIRT}" = "none" ] && return ${DS_NOT_FOUND}
+
+    # Azure provides ovf. Skip false positive by dis-allowing.
+    is_azure_chassis && return $DS_NOT_FOUND
+
+    local isodevs="${DI_ISO9660_DEVS}"
+    case "$isodevs" in
+        ""|$UNAVAILABLE:*) return ${DS_NOT_FOUND};;
+    esac
+
+    # DI_ISO9660_DEVS is <device>=label, like /dev/sr0=OVF-TRANSPORT
+    for tok in $isodevs; do
+        is_cdrom_ovf "${tok%%=*}" "${tok#*=}" && return $DS_FOUND
+    done
+
     if ovf_vmware_guest_customization; then
         return ${DS_FOUND}
     fi
 
-    has_cdrom || return ${DS_NOT_FOUND}
+    return ${DS_NOT_FOUND}
+}
 
-    # FIXME: currently just return maybe if there is a cdrom
-    # ovf iso9660 transport does not specify an fs label.
-    # better would be to check if
-    return ${DS_MAYBE}
+is_azure_chassis() {
+    local azure_chassis="7783-7084-3265-9085-8269-3286-77"
+    dmi_chassis_asset_tag_matches "${azure_chassis}"
 }
 
 dscheck_Azure() {
@@ -667,8 +710,7 @@ dscheck_Azure() {
     #  UUID="112D211272645f72" LABEL="rd_rdfe_stable.161212-1209"
     #  TYPE="udf">/dev/sr0</device>
     #
-    local azure_chassis="7783-7084-3265-9085-8269-3286-77"
-    dmi_chassis_asset_tag_matches "${azure_chassis}" && return $DS_FOUND
+    is_azure_chassis && return $DS_FOUND
     check_seed_dir azure ovf-env.xml && return ${DS_FOUND}
 
     [ "${DI_VIRT}" = "microsoft" ] || return ${DS_NOT_FOUND}
@@ -930,7 +972,7 @@ collect_info() {
     read_dmi_product_name
     read_dmi_product_serial
     read_dmi_product_uuid
-    read_fs_labels
+    read_fs_info
 }
 
 print_info() {
@@ -942,7 +984,7 @@ _print_info() {
     local n="" v="" vars=""
     vars="DMI_PRODUCT_NAME DMI_SYS_VENDOR DMI_PRODUCT_SERIAL"
     vars="$vars DMI_PRODUCT_UUID PID_1_PRODUCT_NAME DMI_CHASSIS_ASSET_TAG"
-    vars="$vars FS_LABELS KERNEL_CMDLINE VIRT"
+    vars="$vars FS_LABELS ISO9660_DEVS KERNEL_CMDLINE VIRT"
     vars="$vars UNAME_KERNEL_NAME UNAME_KERNEL_RELEASE UNAME_KERNEL_VERSION"
     vars="$vars UNAME_MACHINE UNAME_NODENAME UNAME_OPERATING_SYSTEM"
     vars="$vars DSNAME DSLIST"