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.16k stars 5.48k forks source link

jinja_trim_blocks and jinja_lstrip_blocks Seem to Break Formulas #35398

Open bnied opened 8 years ago

bnied commented 8 years ago

Description of Issue/Question

When using our in-house Salt tree, jinja_trim_blocks and jinja_lstrip_blocks both work without issue. However, if I then enable a GitFS-backed Saltstack formula, things start breaking spectacularly.

For example, when trying to use the users-formula, if you have jinja_trim_blocks enabled:

local:
    - Rendering SLS 'base:users' failed: sequence entries are not allowed here; line 2

      ---
      # vim: sts=2 ts=2 sw=2 et ai
      include:  - users.sudo    <======================

      users_new-user_adm_group:
        group.present:
          - name: new-group
      users_new-user_staff_group:
      [...]
      ---

(please note that the dash in the user and group aren't present in the actual user or group)

Alternatively, when jinja_lstrip_blocks is enabled:

local:
    - Rendering SLS 'base:users' failed: while parsing a block mapping
        in "<unicode string>", line 6, column 1:
          include:
          ^
      expected <block end>, but found '-'
        in "<unicode string>", line 51, column 1:
          - uid: 1111
          ^

Removing these two options allow the Salt Formulas to work normally.

Setup

jinja_trim_blocks: True
jinja_lstrip_blocks: True
gitfs_remotes:
  - https://github.com/saltstack-formulas/hostsfile-formula.git
  - https://github.com/saltstack-formulas/users-formula.git

Steps to Reproduce Issue

Run Salt after configuring as noted above.

Versions Report

Salt Version:
           Salt: 2016.3.2

Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: 1.5
          gitdb: 0.5.4
      gitpython: 0.3.2 RC1
          ioflo: Not Installed
         Jinja2: 2.7.2
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: 0.9.1
   msgpack-pure: Not Installed
 msgpack-python: 0.3.0
   mysql-python: 1.2.3
      pycparser: Not Installed
       pycrypto: 2.6.1
         pygit2: Not Installed
         Python: 2.7.6 (default, Jun 22 2015, 17:58:13)
   python-gnupg: 0.3.6
         PyYAML: 3.10
          PyZMQ: 14.0.1
           RAET: Not Installed
          smmap: 0.8.2
        timelib: Not Installed
        Tornado: 4.2.1
            ZMQ: 4.0.5

System Versions:
           dist: Ubuntu 14.04 trusty
        machine: x86_64
        release: 3.13.0-91-generic
         system: Linux
        version: Ubuntu 14.04 trusty

Master and all minions are running the same version.

Ch3LL commented 8 years ago

Just to verify its not the sls files in combination with the jinja settings, can you try cloning that repo to your local file_roots and see if it works then versus the gitfs?

I know you state that your local state files work fine but I want to make sure whether the users-formulas states are the issue or gitfs. Thanks

bnied commented 8 years ago

Can do. I'll try it and report back.

bnied commented 8 years ago

So, I tried the following:

In both cases, I get the same errors as above. So GitFS and python-git do not appear to be at fault.

bnied commented 8 years ago

Is there any other information you need?

bnied commented 8 years ago

ping?

bnied commented 8 years ago

@Ch3LL do you need any other info for this issue? Would this be better-suited to be opened against the saltstack-formulas org?

Ch3LL commented 8 years ago

@bnied okay I believe what is going on is related to how jinja handles - in combination with jinja_trim_blocks and jinja_lstrip_blocks. So in the users formula you can see the user of - often for example:

{%- if used_sudo or used_googleauth or used_user_files %}
include:
{%- if used_sudo %}

Accordign to the jinja docs here:

You can also strip whitespace in templates by hand. If you add a minus sign (-) to the start or end of a block (e.g. a For tag), a comment, or a variable expression, the whitespaces before or after that block will be removed.

This whitespace includes newlines

And in combination with those two jinja settings. according to the docs those do the following:

With both trim_blocks and lstrip_blocks enabled, you can put block tags on their own lines, and the entire block line will be removed when rendered, preserving the whitespace of the contents

so I'm thinking this might be expected behavior, but can you please test one thing for me.

Can you clone that repo down and then remove the - in those jinja blocks and see if that resolves the issue?

bnied commented 8 years ago

Can do! I'll try this either today or this weekend, and have feedback back to you by Monday.

wwalker commented 7 years ago

I'm seeing the same problem. We have in our environment set jinja_trim_blocks and jinja_lstrip_blocks to true. At this point it would be very painful to turn them off. I am hoping that there is a way to turn them off on a per file basis so that I can take advantage of the users formula without having to re do all of our templates, etc. that are dependent on that jinja behavior.

stale[bot] commented 6 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.

myii commented 5 years ago

I believe this issue needs to be reopened.

Here is another example of it breaking formulas (the issue and the PR that resolved it):

Please also note my comment regarding the fix, since it resulted in other side effects.

Ch3LL commented 5 years ago

as per my most recent comment i believe this is expected behavior and not an issue with salt, but expected behavior with how jinja is rendered with these options.

myii commented 5 years ago

@Ch3LL Thanks for your response. I agree, from a Jinja perspective, that this is expected behaviour. However, the Jinja doesn't respect the surrounding YAML, which needs to be kept sane via. Salt. Whether the master has been configured to use jinja_trim_blocks or not, entire SaltStack formulas must not be compromised or rendered useless. Let me be more specific and include a comparison to Ansible.

@wwalker above:

... I am hoping that there is a way to turn them off on a per file basis so that I can take advantage of the users formula without having to re do all of our templates, etc. that are dependent on that jinja behavior.

Ansible has this feature as shown in the quotes below.

https://github.com/ansible/ansible/issues/10725#issuecomment-113369067:

you can easily add this using template overrides, add #jinja2: lstrip_blocks: True to the top of the template

https://github.com/ansible/ansible/issues/10725#issuecomment-249494236:

This worked for me:

#jinja2: trim_blocks: "true", lstrip_blocks: "false"

You have to surround values by "

I've tried all sorts of incantations of this to see if it is already supported by SaltStack but that doesn't seem to be the case. It would be really helpful if it was, so that SaltStack formulas can explicitly state the settings to use, so that the rendering is consistent, whether jinja_trim_blocks has been set or not, etc. Hence the request for this issue to be reopened.

Ch3LL commented 5 years ago

are you using version >=2018.3.0?

I re-looked at the documentation for these options and they have been deprecated as you can see here:

https://docs.saltstack.com/en/latest/ref/configuration/master.html#jinja-trim-blocks

Deprecated since version 2018.3.0: Replaced by jinja_env and jinja_sls_env

and here is the docs for jinja_env: https://docs.saltstack.com/en/latest/ref/configuration/master.html#jinja-env and jinja_sls_env: https://docs.saltstack.com/en/latest/ref/configuration/master.html#std:conf_master-jinja_sls_env

I'm guessing you might be on the updated version of salt. Do those options work for you?

myii commented 5 years ago

@Ch3LL Thanks for your feedback. Actually, I had noticed that and have already tried the newly introduced method, using the example mentioned in the issue I linked to earlier (https://github.com/saltstack-formulas/postgres-formula/issues/241):

jinja_env:
  trim_blocks: True
  lstrip_blocks: True

The problem remains as expected, since what's really required is something like Ansible's per-file override (as mentioned above). As formula writers, we cannot guarantee the configuration of the salt masters out there. With the per-file override, we will be able to explicitly state how the Jinja renderer should handle whitespace for that specific file.

Ch3LL commented 5 years ago

thanks for follow up. I will go ahead and re-open this as a feature request

stale[bot] commented 5 years ago

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

myii commented 5 years ago

@Ch3LL Thanks, appreciate it.

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.

myii commented 4 years ago

Still a problem.

stale[bot] commented 4 years ago

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

ggiesen commented 3 years ago

I'd love to see a per-file override as well so that we can do something like:

#jinja2: lstrip_blocks: True, trim_blocks: True

Managing whitespace for network device configs without it is painful, and I can't enable:

jinja_env:
  trim_blocks: True
  lstrip_blocks: True

globally because it would break a dozen or more formulas I'm using.

Lillecarl commented 1 year ago

https://github.com/ansible/ansible/blob/devel/lib/ansible/template/__init__.py#L921 this is how ansible implements this, sadly they're gpl3 and we're apache2 so we can't use their code. Eyes only.

A solution would be nice though, this can't be a global setting.

Lillecarl commented 1 year ago

I've just implemented a version of the ansible per-file settings thingy like this: https://pastebin.com/XuT2sSHL It's my first go and it works.

Since the Ansible code is of an incompatible license I figured I'd reimplement it as easily as possible. What we do is:

  1. Look if first line contains #jinja2: (let's assume it does in this flow here)
  2. Remove #jinja2: from the first line
  3. Parse the first line as JSON
  4. Feed it into some options mangler helper function that was already there
  5. Profit.

The patch is based off 3005.1, since that's what's building in my Nix infrastructure at the moment. I'll look into upstreaming "this" (something like this) :)

danel1 commented 1 year ago

@Ch3LL It's been seven years and no progress :( Please, this would be really useful..

ggiesen commented 1 year ago

To be fair, there is some progress. There's an outstanding PR that needs a little love and then it should be merge ready. I'll see if I can spend some time on it in a couple weeks.