From ba515fc24d309a40f5552b514b59381777d8ad0e Mon Sep 17 00:00:00 2001

From: Eduardo Otubo <otubo@redhat.com>

Date: Tue, 10 Mar 2020 16:04:18 +0100

Subject: [PATCH] azure: avoid

Message-id: <20200310160418.887-1-otubo@redhat.com>

Patchwork-id: 94221

O-Subject: [RHEL-8.1.0/RHEL-7.8.z/RHEL-7.7.z cloud-init PATCH] azure: avoid re-running cloud-init when instance-id is byte-swapped (#84)

Bugzilla: 1810112

RH-Acked-by: Vitaly Kuznetsov <vkuznets@redhat.com>

RH-Acked-by: Mohammed Gamal <mgamal@redhat.com>

commit 129b1c4ea250619bd7caed7aaffacc796b0139f2

Author: AOhassan <37305877+AOhassan@users.noreply.github.com>

Date:   Thu Dec 12 13:51:42 2019 -0800

    azure: avoid re-running cloud-init when instance-id is byte-swapped (#84)

    Azure stores the instance ID with an incorrect byte ordering for the

    first three hyphen delimited parts. This results in invalid

    is_new_instance checks forcing Azure datasource to recrawl the metadata

    service.

    When persisting instance-id from the metadata service, swap the

    instance-id string byte order such that it is consistent with

    that returned by dmi information. Check whether the instance-id

    string is a byte-swapped match when determining correctly whether

    the Azure platform instance-id has actually changed.

Signed-off-by: Eduardo Otubo <otubo@redhat.com>

Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>

---

 cloudinit/sources/DataSourceAzure.py               | 16 ++++++++++---

 cloudinit/sources/helpers/azure.py                 | 27 ++++++++++++++++++++++

 tests/unittests/test_datasource/test_azure.py      | 24 ++++++++++++++++---

 .../unittests/test_datasource/test_azure_helper.py | 19 +++++++++++++++

 4 files changed, 80 insertions(+), 6 deletions(-)

diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py

index 5baf8da..66bbe5e 100755

--- a/cloudinit/sources/DataSourceAzure.py

+++ b/cloudinit/sources/DataSourceAzure.py

@@ -28,7 +28,8 @@ from cloudinit.reporting import events

 from cloudinit.sources.helpers.azure import (azure_ds_reporter,

                                              azure_ds_telemetry_reporter,

-                                             get_metadata_from_fabric)

+                                             get_metadata_from_fabric,

+                                             is_byte_swapped)

 LOG = logging.getLogger(__name__)

@@ -458,8 +459,7 @@ class DataSourceAzure(sources.DataSource):

         seed = _get_random_seed()

         if seed:

             crawled_data['metadata']['random_seed'] = seed

-        crawled_data['metadata']['instance-id'] = util.read_dmi_data(

-            'system-uuid')

+        crawled_data['metadata']['instance-id'] = self._iid()

         if perform_reprovision:

             LOG.info("Reporting ready to Azure after getting ReprovisionData")

@@ -530,6 +530,16 @@ class DataSourceAzure(sources.DataSource):

         # quickly (local check only) if self.instance_id is still valid

         return sources.instance_id_matches_system_uuid(self.get_instance_id())

+    def _iid(self, previous=None):

+        prev_iid_path = os.path.join(

+            self.paths.get_cpath('data'), 'instance-id')

+        iid = util.read_dmi_data('system-uuid')

+        if os.path.exists(prev_iid_path):

+            previous = util.load_file(prev_iid_path).strip()

+            if is_byte_swapped(previous, iid):

+                return previous

+        return iid

+

     @azure_ds_telemetry_reporter

     def setup(self, is_new_instance):

         if self._negotiated is False:

diff --git a/cloudinit/sources/helpers/azure.py b/cloudinit/sources/helpers/azure.py

index 82c4c8c..c2a57cc 100755

--- a/cloudinit/sources/helpers/azure.py

+++ b/cloudinit/sources/helpers/azure.py

@@ -7,6 +7,7 @@ import re

 import socket

 import struct

 import time

+import textwrap

 from cloudinit.net import dhcp

 from cloudinit import stages

@@ -40,6 +41,32 @@ def azure_ds_telemetry_reporter(func):

     return impl

+def is_byte_swapped(previous_id, current_id):

+    """

+    Azure stores the instance ID with an incorrect byte ordering for the

+    first parts. This corrects the byte order such that it is consistent with

+    that returned by the metadata service.

+    """

+    if previous_id == current_id:

+        return False

+

+    def swap_bytestring(s, width=2):

+        dd = [byte for byte in textwrap.wrap(s, 2)]

+        dd.reverse()

+        return ''.join(dd)

+

+    parts = current_id.split('-')

+    swapped_id = '-'.join([

+            swap_bytestring(parts[0]),

+            swap_bytestring(parts[1]),

+            swap_bytestring(parts[2]),

+            parts[3],

+            parts[4]

+        ])

+

+    return previous_id == swapped_id

+

+

 @contextmanager

 def cd(newdir):

     prevdir = os.getcwd()

diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py

index bc8b42c..1fb0565 100644

--- a/tests/unittests/test_datasource/test_azure.py

+++ b/tests/unittests/test_datasource/test_azure.py

@@ -314,7 +314,7 @@ scbus-1 on xpt0 bus 0

             'public-keys': [],

         })

-        self.instance_id = 'test-instance-id'

+        self.instance_id = 'D0DF4C54-4ECB-4A4B-9954-5BDF3ED5C3B8'

         def _dmi_mocks(key):

             if key == 'system-uuid':

@@ -511,7 +511,7 @@ fdescfs            /dev/fd          fdescfs rw              0 0

                       'subnet': [{'address': '10.0.0.0', 'prefix': '24'}]},

                 'ipv6': {'ipAddress': []},

                 'macAddress': '000D3A047598'}]}},

-            'instance-id': 'test-instance-id',

+            'instance-id': 'D0DF4C54-4ECB-4A4B-9954-5BDF3ED5C3B8',

             'local-hostname': u'myhost',

             'random_seed': 'wild'}

@@ -881,6 +881,24 @@ fdescfs            /dev/fd          fdescfs rw              0 0

         self.assertTrue(ret)

         self.assertEqual('value', dsrc.metadata['test'])

+    def test_instance_id_endianness(self):

+        """Return the previous iid when dmi uuid is the byteswapped iid."""

+        ds = self._get_ds({'ovfcontent': construct_valid_ovf_env()})

+        # byte-swapped previous

+        write_file(

+            os.path.join(self.paths.cloud_dir, 'data', 'instance-id'),

+            '544CDFD0-CB4E-4B4A-9954-5BDF3ED5C3B8')

+        ds.get_data()

+        self.assertEqual(

+            '544CDFD0-CB4E-4B4A-9954-5BDF3ED5C3B8', ds.metadata['instance-id'])

+        # not byte-swapped previous

+        write_file(

+            os.path.join(self.paths.cloud_dir, 'data', 'instance-id'),

+            '644CDFD0-CB4E-4B4A-9954-5BDF3ED5C3B8')

+        ds.get_data()

+        self.assertEqual(

+            'D0DF4C54-4ECB-4A4B-9954-5BDF3ED5C3B8', ds.metadata['instance-id'])

+

     def test_instance_id_from_dmidecode_used(self):

         ds = self._get_ds({'ovfcontent': construct_valid_ovf_env()})

         ds.get_data()

@@ -1080,7 +1098,7 @@ class TestAzureBounce(CiTestCase):

         def _dmi_mocks(key):

             if key == 'system-uuid':

-                return 'test-instance-id'

+                return 'D0DF4C54-4ECB-4A4B-9954-5BDF3ED5C3B8'

             elif key == 'chassis-asset-tag':

                 return '7783-7084-3265-9085-8269-3286-77'

             raise RuntimeError('should not get here')

diff --git a/tests/unittests/test_datasource/test_azure_helper.py b/tests/unittests/test_datasource/test_azure_helper.py

index bd006ab..7ad5cc1 100644

--- a/tests/unittests/test_datasource/test_azure_helper.py

+++ b/tests/unittests/test_datasource/test_azure_helper.py

@@ -170,6 +170,25 @@ class TestGoalStateParsing(CiTestCase):

         goal_state = self._get_goal_state(instance_id=instance_id)

         self.assertEqual(instance_id, goal_state.instance_id)

+    def test_instance_id_byte_swap(self):

+        """Return true when previous_iid is byteswapped current_iid"""

+        previous_iid = "D0DF4C54-4ECB-4A4B-9954-5BDF3ED5C3B8"

+        current_iid = "544CDFD0-CB4E-4B4A-9954-5BDF3ED5C3B8"

+        self.assertTrue(

+            azure_helper.is_byte_swapped(previous_iid, current_iid))

+

+    def test_instance_id_no_byte_swap_same_instance_id(self):

+        previous_iid = "D0DF4C54-4ECB-4A4B-9954-5BDF3ED5C3B8"

+        current_iid = "D0DF4C54-4ECB-4A4B-9954-5BDF3ED5C3B8"

+        self.assertFalse(

+            azure_helper.is_byte_swapped(previous_iid, current_iid))

+

+    def test_instance_id_no_byte_swap_diff_instance_id(self):

+        previous_iid = "D0DF4C54-4ECB-4A4B-9954-5BDF3ED5C3B8"

+        current_iid = "G0DF4C54-4ECB-4A4B-9954-5BDF3ED5C3B8"

+        self.assertFalse(

+            azure_helper.is_byte_swapped(previous_iid, current_iid))

+

     def test_certificates_xml_parsed_and_fetched_correctly(self):

         http_client = mock.MagicMock()

         certificates_url = 'TestCertificatesUrl'

-- 

1.8.3.1