saltstack / salt

Software to automate the management and configuration of any infrastructure or application at scale. Get access to the Salt software package repository here:
https://repo.saltproject.io/
Apache License 2.0
14.13k stars 5.47k forks source link

[BUG] v3001.7 Using #!yamlex in a pillar file with salt-ssh results in some strings being quoted #60359

Open joseph-ireland opened 3 years ago

joseph-ireland commented 3 years ago

Description Strings defined in a pillar file using yamlex end up quoted when rendered in jinja to the state file.

Setup

/srv/pillar/weird_quotes.sls:

#!yamlex

variable: test

/srv/salt/weird_quotes.sls:

/{{pillar.variable}}_example:
  file.directory

Steps to Reproduce the behavior

salt-ssh <host> state.apply weird_quotes -l debug

output log ``` [DEBUG ] Reading configuration from /etc/salt/master [DEBUG ] Changed git to gitfs in master opts' fileserver_backend list [DEBUG ] Configuration file path: /etc/salt/master [WARNING ] Insecure logging configuration detected! Sensitive data may be logged. [DEBUG ] MasterEvent PUB socket URI: /var/run/salt/master/master_event_pub.ipc [DEBUG ] MasterEvent PULL socket URI: /var/run/salt/master/master_event_pull.ipc [DEBUG ] LazyLoaded flat.targets [DEBUG ] LazyLoaded jinja.render [DEBUG ] LazyLoaded yaml.render [DEBUG ] compile template: /etc/salt/roster [DEBUG ] Jinja search path: ['/var/cache/salt/master/files/base'] [DEBUG ] LazyLoaded roots.envs [DEBUG ] pygit2 gitfs_provider enabled [DEBUG ] Created gitfs object with uninitialized remotes [DEBUG ] LazyLoaded gitfs.envs [DEBUG ] Could not LazyLoad roots.init: 'roots.init' is not available. [DEBUG ] compile template: /srv/pillar/weird_quotes.sls [DEBUG ] LazyLoaded yamlex.render [DEBUG ] Results of SLS rendering: {variable: test} [PROFILE ] Time (in seconds) to render '/srv/pillar/weird_quotes.sls' using 'yamlex' renderer: 0.0006971359252929688 [DEBUG ] Finished gathering pillar data for state run [DEBUG ] LazyLoaded jinja.render [DEBUG ] LazyLoaded yaml.render [DEBUG ] Returning file list from cache: age=3 cache_time=20 /var/cache/salt/master/file_lists/roots/base.p [DEBUG ] Re-using gitfs object for process 10103 [DEBUG ] Returning file list from cache: age=3 cache_time=20 /var/cache/salt/master/file_lists/gitfs/base.p [DEBUG ] In saltenv 'base', looking at rel_path 'weird_quotes.sls' to resolve 'salt://weird_quotes.sls' [DEBUG ] In saltenv 'base', ** considering ** path '/var/cache/salt/master/files/base/weird_quotes.sls' to resolve 'salt://weird_quotes.sls' [DEBUG ] Fetching file from saltenv 'base', ** attempting ** 'salt://weird_quotes.sls' [DEBUG ] No dest file found [INFO ] Fetching file from saltenv 'base', ** done ** 'weird_quotes.sls' [DEBUG ] compile template: /var/cache/salt/master/files/base/weird_quotes.sls [DEBUG ] Jinja search path: ['/var/tmp/.root_8758df_salt/running_data/var/cache/salt/minion/files/base'] [PROFILE ] Time (in seconds) to render '/var/cache/salt/master/files/base/weird_quotes.sls' using 'jinja' renderer: 0.001580953598022461 [DEBUG ] Rendered data from file: /var/cache/salt/master/files/base/weird_quotes.sls: /"test"_example: file.directory [DEBUG ] Results of YAML rendering: OrderedDict([('/"test"_example', 'file.directory')]) [PROFILE ] Time (in seconds) to render '/var/cache/salt/master/files/base/weird_quotes.sls' using 'yaml' renderer: 0.0002658367156982422 [DEBUG ] LazyLoaded highstate.output [DEBUG ] LazyLoaded nested.output [DEBUG ] Sending event: tag = salt/job/20210610163825644822/ret/bell; data = {'jid': '20210610163825644822', 'return': {'file_|-/"test"_example_|-/"test"_example_|-directory': {'name': '/"test"_example', 'changes': {'/"test"_example': 'New Dir'}, 'result': True, 'comment': 'Directory /"test"_example updated', '__sls__': 'weird_quotes', '__run_num__': 0, 'start_time': '17:38:31.792772', 'duration': 9.567, '__id__': '/"test"_example'}}, 'retcode': 0, 'out': 'highstate', 'id': 'bell', 'fun': 'state.pkg', 'fun_args': ['/var/tmp/.root_8758df_salt/salt_state.tgz', 'test=None', 'pkg_sum=a7cabf5973425c30b7432c0ce4e617ffb20d3155250d791ef78caf894392c5e0', 'hash_type=sha256'], '_stamp': '2021-06-10T16:38:32.189607'} [DEBUG ] Closing IPCMessageClient instance bell: Name: /"test"_example - Function: file.directory - Result: Changed Started: - 17:38:31.792772 Duration: 9.567 ms Summary for bell ------------ Succeeded: 1 (changed=1) Failed: 0 ------------ Total states run: 1 Total run time: 9.567 ms ```

Expected behavior /test_example/ should be created, instead /"test"_example/ is created

Versions Report

salt --versions-report (Provided by running salt --versions-report. Please also mention any differences in master/minion versions.) ``` [jireland@bell pillar]$ salt --versions-report Salt Version: Salt: 3001.7 Dependency Versions: cffi: 1.14.0 cherrypy: unknown dateutil: 2.6.1 docker-py: 3.5.0 gitdb: Not Installed gitpython: Not Installed Jinja2: 2.8.1 libgit2: 1.0.0 M2Crypto: 0.35.2 Mako: Not Installed msgpack-pure: Not Installed msgpack-python: 0.6.2 mysql-python: Not Installed pycparser: 2.20 pycrypto: 2.6.1 pycryptodome: Not Installed pygit2: 1.2.1 Python: 3.6.8 (default, Apr 2 2020, 13:34:55) python-gnupg: Not Installed PyYAML: 3.12 PyZMQ: 17.0.0 smmap: Not Installed timelib: Not Installed Tornado: 4.5.3 ZMQ: 4.1.4 System Versions: dist: centos 7 Core locale: UTF-8 machine: x86_64 release: 3.10.0-862.9.1.el7.x86_64 system: Linux version: CentOS Linux 7 Core ```

Additional context Add any other context about the problem here.

welcome[bot] commented 3 years ago

Hi there! Welcome to the Salt Community! Thank you for making your first contribution. We have a lengthy process for issues and PRs. Someone from the Core Team will follow up as soon as possible. In the meantime, here’s some information that may help as you continue your Salt journey. Please be sure to review our Code of Conduct. Also, check out some of our community resources including:

There are lots of ways to get involved in our community. Every month, there are around a dozen opportunities to meet with other contributors and the Salt Core team and collaborate in real time. The best way to keep track is by subscribing to the Salt Community Events Calendar. If you have additional questions, email us at saltproject@vmware.com. We’re glad you’ve joined our community and look forward to doing awesome things with you!

garethgreenaway commented 3 years ago

@joseph-ireland Thanks for the report. I was able to reproduce this one and it's definitely a bug.

joseph-ireland commented 3 years ago

Looking further into this, it seems yamlex has always tried to render all strings with quotes everywhere, which seems very odd.

https://github.com/saltstack/salt/blob/master/salt/serializers/yamlex.py#L370

I'm not familiar with the code base, but I guess that this odd __str__ operator would typically be discarded due to some extra serialization step, and since salt-ssh runs the whole thing in one process, the __str__ operator sticks around.

garethgreenaway commented 3 years ago

@joseph-ireland I've verified that for this particular use case the following change does produce the expected results:

diff --git a/salt/serializers/yamlex.py b/salt/serializers/yamlex.py
index bf9b278248..fbb685e108 100644
--- a/salt/serializers/yamlex.py
+++ b/salt/serializers/yamlex.py
@@ -384,7 +384,7 @@ class SLSString(str):
     """

     def __str__(self):
-        return serialize(self, default_style='"')
+        return serialize(self)

     def __repr__(self):
         return serialize(self, default_style='"')

We just need to verify that making this change doesn't break something else.

tdobes commented 2 years ago

FYI: #47085 appears to be the same bug.