Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

YAML loading and serializing does not handle None properly #43694

Open
hgfischer opened this issue Sep 22, 2017 · 15 comments
Open

YAML loading and serializing does not handle None properly #43694

hgfischer opened this issue Sep 22, 2017 · 15 comments
Labels
Bug broken, incorrect, or confusing behavior Confirmed Salt engineer has confirmed bug/feature - often including a MCVE Core relates to code central or existential to Salt severity-low 4th level, cosemtic problems, work around exists
Milestone

Comments

@hgfischer
Copy link
Contributor

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
Copy link
Contributor

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

@gtmanfred gtmanfred added Bug broken, incorrect, or confusing behavior Core relates to code central or existential to Salt severity-low 4th level, cosemtic problems, work around exists P4 Priority 4 team-core labels Sep 22, 2017
@gtmanfred gtmanfred added this to the Approved milestone Sep 22, 2017
@hgfischer
Copy link
Contributor Author

hgfischer commented Sep 25, 2017

@gtmanfred
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link

norpol commented Oct 3, 2017

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
Copy link

stale bot commented Feb 7, 2019

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 stale bot added the stale label Feb 7, 2019
@stale stale bot closed this as completed Feb 14, 2019
@norpol
Copy link

norpol commented Feb 17, 2019

I think this is still an issue.

@rallytime
Copy link
Contributor

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

@stale
Copy link

stale bot commented Feb 18, 2019

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

@stale stale bot removed the stale label Feb 18, 2019
@garethgreenaway
Copy link
Contributor

@rallytime done.

@baby-gnu
Copy link

baby-gnu commented Jun 7, 2019

I have the same issue when using ~ like

template:
  pkg: ~

@terminalmage
Copy link
Contributor

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
Copy link

baby-gnu commented Jun 7, 2019

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:

  • The value loaded match the is none test
  • The value returned by my macro does not match is none.

Sorry for the noise, I should have missed something.

@stale
Copy link

stale bot commented Jan 8, 2020

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 stale bot added the stale label Jan 8, 2020
@waynew waynew added the Confirmed Salt engineer has confirmed bug/feature - often including a MCVE label Jan 8, 2020
@stale
Copy link

stale bot commented Jan 8, 2020

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

@stale stale bot removed the stale label Jan 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior Confirmed Salt engineer has confirmed bug/feature - often including a MCVE Core relates to code central or existential to Salt severity-low 4th level, cosemtic problems, work around exists
Projects
None yet
Development

No branches or pull requests

9 participants