saltstack / salt

Software to automate the management and configuration of any infrastructure or application at scale. Install Salt from the Salt package repositories here:
https://docs.saltproject.io/salt/install-guide/en/latest/
Apache License 2.0
14.19k stars 5.48k forks source link

YAML loading and serializing does not handle None properly #43694

Open hgfischer opened 7 years ago

hgfischer commented 7 years ago

Description of Issue/Question

I am using import_yaml to load a YAML file that has some keys with empty values (Ex.: key:), then use file.serialize to write it to a YAML file again. But all keys that previously had no values, now have a None in place, which is a valid string, but not a valid YAML empty value.

This is causing problems because the program that expects an empty or decimal value is getting a string instead.

Setup

{% import_yaml "defaults.yaml" as defaults %} 

{{ sls }}~config:
  file.serialize:
    - name: /etc/config.yaml
    - show_changes: true
    - formatter: yaml
    - backup: minion
    - dataset: {{ defaults }}

Steps to Reproduce Issue

Can be verified using state.show_sls.

Versions Report

Salt Version:
           Salt: 2017.7.1

Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: 2.4.2
      docker-py: Not Installed
          gitdb: 0.6.4
      gitpython: 1.0.1
          ioflo: Not Installed
         Jinja2: 2.8
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: 1.0.3
   msgpack-pure: Not Installed
 msgpack-python: 0.4.6
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 2.7.12 (default, Nov 19 2016, 06:48:10)
   python-gnupg: Not Installed
         PyYAML: 3.11
          PyZMQ: 15.2.0
           RAET: Not Installed
          smmap: 0.9.0
        timelib: Not Installed
        Tornado: 4.2.1
            ZMQ: 4.1.4

System Versions:
           dist: Ubuntu 16.04 xenial
         locale: UTF-8
        machine: x86_64
        release: 4.4.0-96-generic
         system: Linux
        version: Ubuntu 16.04 xenial
gtmanfred commented 7 years ago

PyYaml converts empty values to None when they are put in the yaml dictionary, and I am not sure how you would differentiate between having None or empty when dumping back to a file.

I am going to mark this as a bug, but I would highly recommend using file.managed if you and values to remain empty instead of having None specified.

Thanks, Daniel

hgfischer commented 7 years ago

Hi @gtmanfred it seems this can be addressed by PyYaml with some small changes:

https://stackoverflow.com/questions/37200150/can-i-dump-blank-instead-of-null-in-yaml-pyyaml

https://stackoverflow.com/questions/30134110/how-can-i-output-blank-value-in-python-yaml-file

gtmanfred commented 7 years ago

Ok, so it is an easy solution to solve.

diff --git a/salt/serializers/yaml.py b/salt/serializers/yaml.py
index 5ebc94b..51b0758 100644
--- a/salt/serializers/yaml.py
+++ b/salt/serializers/yaml.py
@@ -98,7 +98,8 @@ Loader.add_multi_constructor(None, Loader.construct_undefined)

 class Dumper(BaseDumper):  # pylint: disable=W0232
     '''Overwrites Dumper as not for pollute legacy Dumper'''
-    pass
+    def represent_none(self, _):
+        return self.represent_scalar('tag:yaml.org,2002:null', '')

 Dumper.add_multi_representer(type(None), Dumper.represent_none)
 Dumper.add_multi_representer(str, Dumper.represent_str)

I am just not sure if it should be "solved" for everyone by default or not. It might make more sense to set it to 'null'.

The other problem is if we change this behavior, Then any files dumped from this module won't be readable by salts yaml loader.

@saltstack/team-core does anyone have any suggestions here?

Thanks, Daniel

hgfischer commented 7 years ago

If it is loaded as empty from a YAML, it should remain empty in the end, and that is the case.

One idea is to change the serializer behaviour by introducing per-format options.

norpol commented 7 years ago

I have a similar problem. import_yaml ... wrongly escapes the content. So null, becomes None, \n becomes \\n, true becomes True.

a: null
b: "null"
c: 'null
  null
  null'

to

{'a': None, 'b': 'null', 'c': 'null\\nnull\\nnull'}

This is appears when I do a file.serialize or file.managed with either dataset or contents. I've managed to ?workaround? this problem by passing the import_yaml dict through |yaml in my jinja.

Example:

 create-json:
   file.serialize:
     - name: /tmp/my.json
     - dataset: {{ myyamldict | yaml }}
     - formatter: json
stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

norpol commented 5 years ago

I think this is still an issue.

rallytime commented 5 years ago

@Ch3LL or @garethgreenaway Can you re-open this?

stale[bot] commented 5 years ago

Thank you for updating this issue. It is no longer marked as stale.

garethgreenaway commented 5 years ago

@rallytime done.

baby-gnu commented 5 years ago

I have the same issue when using ~ like

template:
  pkg: ~
terminalmage commented 5 years ago

A tilde is a null value in YAML. Are you trying to get a literal tilde? If so you'll need to quote it.

>>> import yaml
>>> yaml.safe_load('foo: ~')
{'foo': None}
>>> yaml.safe_load('foo: "~"')
{'foo': '~'}
>>>
baby-gnu commented 5 years ago

Hello @terminalmage, I want a None but today that's not what I got.

Looks like I have an issue with my macros, I import_yaml and call a macro with the value:

Sorry for the noise, I should have missed something.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

stale[bot] commented 4 years ago

Thank you for updating this issue. It is no longer marked as stale.