cloud-init-dev team mailing list archive

[Scott Moser <ssmoser2+ubuntu@xxxxxxxxx>]

Scott Moser has proposed merging ~smoser/cloud-init:fix/1849640-adjust-yaml-usage into cloud-init:master.

Commit message:
Fix usages of yaml, and move yaml_dump to safeyaml.dumps.

Here we replace uses of the pyyaml module directly with functions
provided by cloudinit.safeyaml.  Also, change/move
  cloudinit.util.yaml_dumps
to
  cloudinit.safeyaml.dumps

LP: #1849640

Requested reviews:
  cloud-init Commiters (cloud-init-dev)

For more details, see:
https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/374679

see commit message
-- 
Your team cloud-init Commiters is requested to review the proposed merge of ~smoser/cloud-init:fix/1849640-adjust-yaml-usage into cloud-init:master.

diff --git a/cloudinit/cmd/devel/net_convert.py b/cloudinit/cmd/devel/net_convert.py
index 1ad7e0b..9b76830 100755
--- a/cloudinit/cmd/devel/net_convert.py
+++ b/cloudinit/cmd/devel/net_convert.py
@@ -5,13 +5,12 @@ import argparse
 import json
 import os
 import sys
-import yaml
 
 from cloudinit.sources.helpers import openstack
 from cloudinit.sources import DataSourceAzure as azure
 from cloudinit.sources import DataSourceOVF as ovf
 
-from cloudinit import distros
+from cloudinit import distros, safeyaml
 from cloudinit.net import eni, netplan, network_state, sysconfig
 from cloudinit import log
 
@@ -78,13 +77,12 @@ def handle_args(name, args):
     if args.kind == "eni":
         pre_ns = eni.convert_eni_data(net_data)
     elif args.kind == "yaml":
-        pre_ns = yaml.load(net_data)
+        pre_ns = safeyaml.load(net_data)
         if 'network' in pre_ns:
             pre_ns = pre_ns.get('network')
         if args.debug:
             sys.stderr.write('\n'.join(
-                ["Input YAML",
-                 yaml.dump(pre_ns, default_flow_style=False, indent=4), ""]))
+                ["Input YAML", safeyaml.dumps(pre_ns), ""]))
     elif args.kind == 'network_data.json':
         pre_ns = openstack.convert_net_json(
             json.loads(net_data), known_macs=known_macs)
@@ -100,9 +98,8 @@ def handle_args(name, args):
                            "input data")
 
     if args.debug:
-        sys.stderr.write('\n'.join([
-            "", "Internal State",
-            yaml.dump(ns, default_flow_style=False, indent=4), ""]))
+        sys.stderr.write('\n'.join(
+            ["", "Internal State", safeyaml.dumps(ns), ""]))
     distro_cls = distros.fetch(args.distro)
     distro = distro_cls(args.distro, {}, None)
     config = {}
diff --git a/cloudinit/cmd/tests/test_main.py b/cloudinit/cmd/tests/test_main.py
index a1e534f..57b8fdf 100644
--- a/cloudinit/cmd/tests/test_main.py
+++ b/cloudinit/cmd/tests/test_main.py
@@ -6,8 +6,9 @@ import os
 from six import StringIO
 
 from cloudinit.cmd import main
+from cloudinit import safeyaml
 from cloudinit.util import (
-    ensure_dir, load_file, write_file, yaml_dumps)
+    ensure_dir, load_file, write_file)
 from cloudinit.tests.helpers import (
     FilesystemMockingTestCase, wrap_and_call)
 
@@ -39,7 +40,7 @@ class TestMain(FilesystemMockingTestCase):
             ],
             'cloud_init_modules': ['write-files', 'runcmd'],
         }
-        cloud_cfg = yaml_dumps(self.cfg)
+        cloud_cfg = safeyaml.dumps(self.cfg)
         ensure_dir(os.path.join(self.new_root, 'etc', 'cloud'))
         self.cloud_cfg_file = os.path.join(
             self.new_root, 'etc', 'cloud', 'cloud.cfg')
@@ -113,7 +114,7 @@ class TestMain(FilesystemMockingTestCase):
         """When local-hostname metadata is present, call cc_set_hostname."""
         self.cfg['datasource'] = {
             'None': {'metadata': {'local-hostname': 'md-hostname'}}}
-        cloud_cfg = yaml_dumps(self.cfg)
+        cloud_cfg = safeyaml.dumps(self.cfg)
         write_file(self.cloud_cfg_file, cloud_cfg)
         cmdargs = myargs(
             debug=False, files=None, force=False, local=False, reporter=None,
diff --git a/cloudinit/config/cc_debug.py b/cloudinit/config/cc_debug.py
index 0a039eb..610dbc8 100644
--- a/cloudinit/config/cc_debug.py
+++ b/cloudinit/config/cc_debug.py
@@ -33,6 +33,7 @@ from six import StringIO
 
 from cloudinit import type_utils
 from cloudinit import util
+from cloudinit import safeyaml
 
 SKIP_KEYS = frozenset(['log_cfgs'])
 
@@ -49,7 +50,7 @@ def _make_header(text):
 
 
 def _dumps(obj):
-    text = util.yaml_dumps(obj, explicit_start=False, explicit_end=False)
+    text = safeyaml.dumps(obj, explicit_start=False, explicit_end=False)
     return text.rstrip()
 
 
diff --git a/cloudinit/config/cc_salt_minion.py b/cloudinit/config/cc_salt_minion.py
index d6a21d7..cd9cb0b 100644
--- a/cloudinit/config/cc_salt_minion.py
+++ b/cloudinit/config/cc_salt_minion.py
@@ -45,7 +45,7 @@ specify them with ``pkg_name``, ``service_name`` and ``config_dir``.
 
 import os
 
-from cloudinit import util
+from cloudinit import safeyaml, util
 
 # Note: see https://docs.saltstack.com/en/latest/topics/installation/
 # Note: see https://docs.saltstack.com/en/latest/ref/configuration/
@@ -97,13 +97,13 @@ def handle(name, cfg, cloud, log, _args):
     if 'conf' in s_cfg:
         # Add all sections from the conf object to minion config file
         minion_config = os.path.join(const.conf_dir, 'minion')
-        minion_data = util.yaml_dumps(s_cfg.get('conf'))
+        minion_data = safeyaml.dumps(s_cfg.get('conf'))
         util.write_file(minion_config, minion_data)
 
     if 'grains' in s_cfg:
         # add grains to /etc/salt/grains
         grains_config = os.path.join(const.conf_dir, 'grains')
-        grains_data = util.yaml_dumps(s_cfg.get('grains'))
+        grains_data = safeyaml.dumps(s_cfg.get('grains'))
         util.write_file(grains_config, grains_data)
 
     # ... copy the key pair if specified
diff --git a/cloudinit/config/cc_snappy.py b/cloudinit/config/cc_snappy.py
index 15bee2d..b94cd04 100644
--- a/cloudinit/config/cc_snappy.py
+++ b/cloudinit/config/cc_snappy.py
@@ -68,6 +68,7 @@ is ``auto``. Options are:
 from cloudinit import log as logging
 from cloudinit.settings import PER_INSTANCE
 from cloudinit import temp_utils
+from cloudinit import safeyaml
 from cloudinit import util
 
 import glob
@@ -188,7 +189,7 @@ def render_snap_op(op, name, path=None, cfgfile=None, config=None):
             # Note, however, we do not touch config files on disk.
             nested_cfg = {'config': {shortname: config}}
             (fd, cfg_tmpf) = temp_utils.mkstemp()
-            os.write(fd, util.yaml_dumps(nested_cfg).encode())
+            os.write(fd, safeyaml.dumps(nested_cfg).encode())
             os.close(fd)
             cfgfile = cfg_tmpf
 
diff --git a/cloudinit/handlers/cloud_config.py b/cloudinit/handlers/cloud_config.py
index 99bf0e6..2a30736 100644
--- a/cloudinit/handlers/cloud_config.py
+++ b/cloudinit/handlers/cloud_config.py
@@ -14,6 +14,7 @@ from cloudinit import handlers
 from cloudinit import log as logging
 from cloudinit import mergers
 from cloudinit import util
+from cloudinit import safeyaml
 
 from cloudinit.settings import (PER_ALWAYS)
 
@@ -75,7 +76,7 @@ class CloudConfigPartHandler(handlers.Handler):
                 '',
             ]
             lines.extend(file_lines)
-            lines.append(util.yaml_dumps(self.cloud_buf))
+            lines.append(safeyaml.dumps(self.cloud_buf))
         else:
             lines = []
         util.write_file(self.cloud_fn, "\n".join(lines), 0o600)
diff --git a/cloudinit/net/netplan.py b/cloudinit/net/netplan.py
index e54a34e..54be122 100644
--- a/cloudinit/net/netplan.py
+++ b/cloudinit/net/netplan.py
@@ -8,6 +8,7 @@ from .network_state import subnet_is_ipv6, NET_CONFIG_TO_V2
 
 from cloudinit import log as logging
 from cloudinit import util
+from cloudinit import safeyaml
 from cloudinit.net import SYS_CLASS_NET, get_devicelist
 
 KNOWN_SNAPD_CONFIG = b"""\
@@ -235,9 +236,9 @@ class Renderer(renderer.Renderer):
         # if content already in netplan format, pass it back
         if network_state.version == 2:
             LOG.debug('V2 to V2 passthrough')
-            return util.yaml_dumps({'network': network_state.config},
-                                   explicit_start=False,
-                                   explicit_end=False)
+            return safeyaml.dumps({'network': network_state.config},
+                                  explicit_start=False,
+                                  explicit_end=False)
 
         ethernets = {}
         wifis = {}
@@ -359,10 +360,10 @@ class Renderer(renderer.Renderer):
         # workaround yaml dictionary key sorting when dumping
         def _render_section(name, section):
             if section:
-                dump = util.yaml_dumps({name: section},
-                                       explicit_start=False,
-                                       explicit_end=False,
-                                       noalias=True)
+                dump = safeyaml.dumps({name: section},
+                                      explicit_start=False,
+                                      explicit_end=False,
+                                      noalias=True)
                 txt = util.indent(dump, ' ' * 4)
                 return [txt]
             return []
diff --git a/cloudinit/net/network_state.py b/cloudinit/net/network_state.py
index c0c415d..b485f3d 100644
--- a/cloudinit/net/network_state.py
+++ b/cloudinit/net/network_state.py
@@ -12,6 +12,7 @@ import struct
 
 import six
 
+from cloudinit import safeyaml
 from cloudinit import util
 
 LOG = logging.getLogger(__name__)
@@ -253,7 +254,7 @@ class NetworkStateInterpreter(object):
             'config': self._config,
             'network_state': self._network_state,
         }
-        return util.yaml_dumps(state)
+        return safeyaml.dumps(state)
 
     def load(self, state):
         if 'version' not in state:
@@ -272,7 +273,7 @@ class NetworkStateInterpreter(object):
             setattr(self, key, state[key])
 
     def dump_network_state(self):
-        return util.yaml_dumps(self._network_state)
+        return safeyaml.dumps(self._network_state)
 
     def as_dict(self):
         return {'version': self._version, 'config': self._config}
diff --git a/cloudinit/safeyaml.py b/cloudinit/safeyaml.py
index 3bd5e03..9c53845 100644
--- a/cloudinit/safeyaml.py
+++ b/cloudinit/safeyaml.py
@@ -27,4 +27,17 @@ class NoAliasSafeDumper(yaml.dumper.SafeDumper):
 def load(blob):
     return(yaml.load(blob, Loader=_CustomSafeLoader))
 
+
+def dumps(obj, explicit_start=True, explicit_end=True, noalias=False):
+    """Return data in nicely formatted yaml."""
+
+    return yaml.dump(obj,
+                     line_break="\n",
+                     indent=4,
+                     explicit_start=explicit_start,
+                     explicit_end=explicit_end,
+                     default_flow_style=False,
+                     Dumper=(NoAliasSafeDumper
+                             if noalias else yaml.dumper.Dumper))
+
 # vi: ts=4 expandtab
diff --git a/cloudinit/util.py b/cloudinit/util.py
index 0d338ca..bb42b01 100644
--- a/cloudinit/util.py
+++ b/cloudinit/util.py
@@ -1629,19 +1629,6 @@ def json_dumps(data):
         raise
 
 
-def yaml_dumps(obj, explicit_start=True, explicit_end=True, noalias=False):
-    """Return data in nicely formatted yaml."""
-
-    return yaml.dump(obj,
-                     line_break="\n",
-                     indent=4,
-                     explicit_start=explicit_start,
-                     explicit_end=explicit_end,
-                     default_flow_style=False,
-                     Dumper=(safeyaml.NoAliasSafeDumper
-                             if noalias else yaml.dumper.Dumper))
-
-
 def ensure_dir(path, mode=None):
     if not os.path.isdir(path):
         # Make the dir and adjust the mode
diff --git a/tests/unittests/test_data.py b/tests/unittests/test_data.py
index 3efe7ad..22cf8f2 100644
--- a/tests/unittests/test_data.py
+++ b/tests/unittests/test_data.py
@@ -27,6 +27,7 @@ from cloudinit.settings import (PER_INSTANCE)
 from cloudinit import sources
 from cloudinit import stages
 from cloudinit import user_data as ud
+from cloudinit import safeyaml
 from cloudinit import util
 
 from cloudinit.tests import helpers
@@ -502,7 +503,7 @@ c: 4
         data = [{'content': '#cloud-config\npassword: gocubs\n'},
                 {'content': '#cloud-config\nlocale: chicago\n'},
                 {'content': non_decodable}]
-        message = b'#cloud-config-archive\n' + util.yaml_dumps(data).encode()
+        message = b'#cloud-config-archive\n' + safeyaml.dumps(data).encode()
 
         self.reRoot()
         ci = stages.Init()
diff --git a/tests/unittests/test_runs/test_merge_run.py b/tests/unittests/test_runs/test_merge_run.py
index d1ac494..ff27a28 100644
--- a/tests/unittests/test_runs/test_merge_run.py
+++ b/tests/unittests/test_runs/test_merge_run.py
@@ -7,6 +7,7 @@ import tempfile
 from cloudinit.tests import helpers
 
 from cloudinit.settings import PER_INSTANCE
+from cloudinit import safeyaml
 from cloudinit import stages
 from cloudinit import util
 
@@ -26,7 +27,7 @@ class TestMergeRun(helpers.FilesystemMockingTestCase):
             'system_info': {'paths': {'run_dir': new_root}}
         }
         ud = helpers.readResource('user_data.1.txt')
-        cloud_cfg = util.yaml_dumps(cfg)
+        cloud_cfg = safeyaml.dumps(cfg)
         util.ensure_dir(os.path.join(new_root, 'etc', 'cloud'))
         util.write_file(os.path.join(new_root, 'etc',
                                      'cloud', 'cloud.cfg'), cloud_cfg)
diff --git a/tests/unittests/test_runs/test_simple_run.py b/tests/unittests/test_runs/test_simple_run.py
index d67c422..cb3aae6 100644
--- a/tests/unittests/test_runs/test_simple_run.py
+++ b/tests/unittests/test_runs/test_simple_run.py
@@ -5,6 +5,7 @@ import os
 
 
 from cloudinit.settings import PER_INSTANCE
+from cloudinit import safeyaml
 from cloudinit import stages
 from cloudinit.tests import helpers
 from cloudinit import util
@@ -34,7 +35,7 @@ class TestSimpleRun(helpers.FilesystemMockingTestCase):
             ],
             'cloud_init_modules': ['write-files', 'spacewalk', 'runcmd'],
         }
-        cloud_cfg = util.yaml_dumps(self.cfg)
+        cloud_cfg = safeyaml.dumps(self.cfg)
         util.ensure_dir(os.path.join(self.new_root, 'etc', 'cloud'))
         util.write_file(os.path.join(self.new_root, 'etc',
                                      'cloud', 'cloud.cfg'), cloud_cfg)
@@ -130,7 +131,7 @@ class TestSimpleRun(helpers.FilesystemMockingTestCase):
         # re-write cloud.cfg with unverified_modules override
         cfg = copy.deepcopy(self.cfg)
         cfg['unverified_modules'] = ['spacewalk']  # Would have skipped
-        cloud_cfg = util.yaml_dumps(cfg)
+        cloud_cfg = safeyaml.dumps(cfg)
         util.ensure_dir(os.path.join(self.new_root, 'etc', 'cloud'))
         util.write_file(os.path.join(self.new_root, 'etc',
                                      'cloud', 'cloud.cfg'), cloud_cfg)
@@ -159,7 +160,7 @@ class TestSimpleRun(helpers.FilesystemMockingTestCase):
         cfg = copy.deepcopy(self.cfg)
         # Represent empty configuration in /etc/cloud/cloud.cfg
         cfg['cloud_init_modules'] = None
-        cloud_cfg = util.yaml_dumps(cfg)
+        cloud_cfg = safeyaml.dumps(cfg)
         util.ensure_dir(os.path.join(self.new_root, 'etc', 'cloud'))
         util.write_file(os.path.join(self.new_root, 'etc',
                                      'cloud', 'cloud.cfg'), cloud_cfg)