Salt: Salt 2018.3 wrongly parses ": ", "[" and "]" symbols

[Martin819]

Description of Issue/Question

Running Salt 2018.3.2 I'm experiencing an issue with parsing pillar containing these symbols ": " (colon followed by space), "[" and "]". This issue only exists in the 2018.3 version of Salt. In the 2017.7 version, it passes just fine.

Setup

Running Salt 2018.3.2

Pillar:

linux:
  system:
    enabled: true
    repo:
      ubuntu:
        architectures: amd64
        default: true
        key: |
          -----BEGIN PGP PUBLIC KEY BLOCK-----

          Version: GnuPG v1


          mQGiBEFEnz8RBAC7LstGsKD7McXZgd58oN68KquARLBl6rjA2vdhwl77KkPPOr3O

          ...

          Twx6DKLF+3rF5nf1F3Q=

          =PBAe

        refresh_db: true
        source: "deb [arch=amd64] http://mirror.domain.com/2018.7.0/ubuntu/ xenial main restricted universe"

SLS file:

...
{%- for name, repo in system.repo.items() %}
    {%- if repo.get('default', False) %}
        {%- do default_repos.update({name: repo}) %}
    {%- endif %}
{%- endif %}

{%- if default_repos|length > 0 and grains.os_family == 'Debian' %}

default_repo_list:
  file.managed:
    - name: /etc/apt/sources.list
    - source: salt://linux/files/sources.list
    - template: jinja
    - user: root
    - group: root
    - mode: 0644
    {%- if system.purge_repos|default(False) %}
    - replace: True
    {%- endif %}
    - defaults:
        default_repos: {{ default_repos }}

  {%- endif %}
...

sources.list file:

{%- for name, repo in default_repos.items() %}
# Repository {{ name }}
{{ repo.source }}
{%- endfor %}

Steps to Reproduce Issue

When running with the pillar above, I'm getting the following error:

       [CRITICAL] Rendering SLS 'base:linux.system.repo' failed: while parsing a flow mapping
         in "<unicode string>", line 43, column 36:
            default_repos: {u'ubuntu': {u'architectures': u'amd64', u'd ... 
                                       ^
       expected ',' or '}', but got ':'
         in "<unicode string>", line 43, column 140:
            ... PUBLIC KEY BLOCK-----\n\nVersion: GnuPG v1\n\n\nmQGiBEFEnz8RBAC7 ... 
                                         ^

I have tried to wrap the multi-line value with double quotes, but it didn't help.

After removing the colon ":" symbol, the parsing error moves to the next symbol - see bellow

       [CRITICAL] Rendering SLS 'base:linux.system.repo' failed: while parsing a flow mapping
         in "<unicode string>", line 43, column 36:
            default_repos: {u'ubuntu': {u'architectures': u'amd64', u'd ... 
                                       ^
       expected ',' or '}', but got '['
         in "<unicode string>", line 43, column 302:
            ... esh_db': True, u'source': u'deb [arch=amd64] http://mirror.domai ... 
                                         ^

After removing the "[" symbol, the error moves once more to:

       [CRITICAL] Rendering SLS 'base:linux.system.repo' failed: while parsing a flow mapping
         in "<unicode string>", line 43, column 36:
            default_repos: {u'ubuntu': {u'architectures': u'amd64', u'd ... 
                                       ^
       expected ',' or '}', but got ']'
         in "<unicode string>", line 43, column 312:
            ... rue, u'source': u'deb arch=amd64] http://mirror.domain.com/201 ... 
                                         ^

Finally, after removing the "]" symbol, the run passes successfully.

Versions Report

                  Salt Version:
                             Salt: 2018.3.2

                  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, Dec  4 2017, 14:50:18)
                     python-gnupg: 0.3.8
                           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: ANSI_X3.4-1968
                          machine: x86_64
                          release: 4.13.0-46-generic
                           system: Linux
                          version: Ubuntu 16.04 xenial

[vitaliyf]

I was just building a test case for similar/same error we are seeing.

testcase.yaml

key1: "value1"
key2: "value2: with a colon"

testcase.txt

List:
{%- for key, value in testcast_yaml | dictsort %}
{{ key }}: {{ value }}
{%- endfor %}

Everything:
{{ testcast_yaml }}

testcase.sls

{% import_yaml "testcase.yaml" as testcase_yaml %}

/tmp/testcase.txt:
  file.managed:
    - source: salt://testcase.txt
    - template: jinja
    - context:
        testcast_yaml: {{ testcase_yaml }}

And applying this results in:

$ sudo salt-call state.show_sls testcase

/var/cache/salt/minion/extmods/grains/system.py:18: DeprecationWarning: Use of 'salt.utils.fopen' detected. This function has been moved to 'salt.utils.files.fopen' as of Salt 2018.3.0. This warning will be removed in Salt Neon.
[INFO    ] Loading fresh modules for state activity
[INFO    ] Fetching file from saltenv 'base', ** done ** 'testcase.yaml'
[CRITICAL] Rendering SLS 'base:testcase' failed: while parsing a flow mapping
  in "<unicode string>", line 8, column 24:
            testcast_yaml: {u'key2': u'value2: with a colon ...
                           ^
expected ',' or '}', but got ':'
  in "<unicode string>", line 8, column 42:
     ... estcast_yaml: {u'key2': u'value2: with a colon', u'key1': u'value1'}

This used to work in 2017.x and only broke with upgrade to 2018.3.x

There are ways to get around it:

• Remove colon inside value2 in my .yaml file
• Add | yaml (or | json) to testcast_yaml: {{ testcase_yaml | yaml }}

I think I understand why 2nd workaround is necessary, but is this documented somewhere as backwards-incompatible change in 2018.3? We have quite a few places where we pass context: like this, so would need to go through and fix them all.

[Ch3LL]

looks like i'm able to replicate this and i bisected it to 9ee5b417f775ea307f508459b037514cdf67021f

which is this PR https://github.com/saltstack/salt/pull/45406

so it seems like this was due to our unicode work in 2018.3

[Ch3LL]

i also tested this on the head of 2018.3 and 2018.3.3 and it still fails

[vitaliyf]

Similar to #48977

[terminalmage]

This is the same as https://github.com/saltstack/salt/issues/47001. Dumping a data structure using Jinja calls repr() on that data structure and thus produces invalid YAML, due to the fact that 2018.3 and newer uses unicode types for strings.

You need to pass it through a jinja filter to get the strings into a YAML-ready format. I ported the tojson filter from Jinja 2.9 to Salt for the 2018.3.3 release, but there apparently already exists in the Salt codebase a similar filter called simply json which does mostly the same thing. So using the following should work:

{{ default_repos | json }}

[vitaliyf]

@terminalmage I understand why, but this seems like a pretty bad backwards-incompatible change. In my example above, after I add | json (or | yaml) in .sls, I end up with:

Everything:
{u'key1': u'value1', u'key2': u'value2: with a colon'}

in my managed /tmp/testcase.txt file. I totally understand why, just didn't expect to need to audit all our code for this during a Salt upgrade.

[Martin819]

I agree that it is bad backwards compatible change, but moreover there's nothing mentioned in the release notes.

[terminalmage]

Yes, I'm working on getting the release notes for all 2018.3 releases updated with a warning about this.

We honestly should have had this documented in the past, since dumping a data structure using Jinja does not guarantee YAML-ready output. It was incorrect usage before 2018.3.0 was released, it was just incorrect usage that didn't cause an error (until now).

[terminalmage]

Here's the documentation updates for the release notes: https://github.com/saltstack/salt/pull/49731/files

[AndreiPashkin]

I'd agree, that's pretty confusing.

[terminalmage]

As this change in behavior has been documented, I will close this.