[BUG] var assignation ns.hourly is incorrect

Yoda-BZH commented 2 years ago

Your setup

Formula commit hash / release tag

salt 3004 + pillarstack Formula at d545fb2278b0b1c702f39d59228d0a74406aa3d2 (updated on 2022-01-28

states:
  list:
  # Logrotate
  - logrotate
  # And its jobs, if any defined
  - logrotate.jobs

Versions reports (master & minion)

salt master version:

Salt Version:
          Salt: 3004

Dependency Versions:
          cffi: Not Installed
      cherrypy: Not Installed
      dateutil: 2.7.3
     docker-py: Not Installed
         gitdb: 2.0.5
     gitpython: 2.1.11
        Jinja2: 2.10
       libgit2: Not Installed
      M2Crypto: 0.31.0
          Mako: Not Installed
       msgpack: 0.5.6
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     pycparser: Not Installed
      pycrypto: 2.6.1
  pycryptodome: 3.6.1
        pygit2: Not Installed
        Python: 3.7.3 (default, Jan 22 2021, 20:04:44)
  python-gnupg: Not Installed
        PyYAML: 3.13
         PyZMQ: 17.1.2
         smmap: 2.0.5
       timelib: Not Installed
       Tornado: 4.5.3
           ZMQ: 4.3.1

System Versions:
          dist: debian 10 buster
        locale: UTF-8
       machine: x86_64
       release: 4.19.0-18-amd64
        system: Linux
       version: Debian GNU/Linux 10 buster

salt minion version:

Salt Version:
          Salt: 3004

Dependency Versions:
          cffi: Not Installed
      cherrypy: Not Installed
      dateutil: 2.5.3
     docker-py: Not Installed
         gitdb: Not Installed
     gitpython: Not Installed
        Jinja2: 2.9.4
       libgit2: Not Installed
      M2Crypto: Not Installed
          Mako: Not Installed
       msgpack: 0.6.2
  msgpack-pure: Not Installed
  mysql-python: 1.3.7
     pycparser: Not Installed
      pycrypto: Not Installed
  pycryptodome: 3.6.1
        pygit2: Not Installed
        Python: 3.5.3 (default, Nov  4 2021, 15:29:10)
  python-gnupg: Not Installed
        PyYAML: 3.12
         PyZMQ: 17.1.2
         smmap: Not Installed
       timelib: Not Installed
       Tornado: 4.5.3
           ZMQ: 4.2.1

System Versions:
          dist: debian 9 stretch
        locale: UTF-8
       machine: x86_64
       release: 4.9.0-17-amd64
        system: Linux
       version: Debian GNU/Linux 9 stretch

Pillar / config used

Bug details

Describe the bug

when running salt-call state.test logrotate:

local:
    Data failed to compile:
----------
    Rendering SLS 'base:logrotate.config' failed: Jinja syntax error: expected token 'end of statement block', got '.'; line 10

---
[...]

{% set ns = namespace(hourly=False) %}
{% for key, value in logrotate.jobs.items() %}
{% set contents = value.get('contents', False) %}
  {% if 'hourly' in (contents or value.config) %}
    {% set ns.hourly = True %}    <======================
    {% break %}
  {% endif %}
{% endfor %}

logrotate-config:
[...]
---

Steps to reproduce the bug

use formula at d545fb2278b0b1c702f39d59228d0a74406aa3d2

Expected behaviour

no error, configuration for logorate hourly should apply if needed.

Attempts to fix the bug

If namespaces works like dict, it should be :


  {% if 'hourly' in (contents or value.config) %}
    {% do ns.update({'hourly': True}) %}
  {% endif %}

### Additional context
<!-- Add any other context about the problem here. -->

myii commented 2 years ago

@Yoda-BZH The syntax used is actually correct. The problem we're encountering here is the version of Jinja on your minion (2.9.4). This improved use of the namespace syntax was introduced in 2.10, as mentioned in the documentation.

https://jinja.palletsprojects.com/en/3.0.x/templates/#assignments

As of version 2.10 more complex use cases can be handled using namespace objects which allow propagating of changes across scopes:
{% set ns = namespace(found=false) %}
{% for item in items %}
    {% if item.check_something() %}
        {% set ns.found = true %}
    {% endif %}
    * {{ item.title }}
{% endfor %}
Found item having something: {{ ns.found }}

So the question remains: do our formulas need to workaround versions of Jinja < 2.10 or is the onus on end users to update their minions? Looking the release history, it can be seen that 2.9.4 was released on Jan 10, 2017.

CC: @mdschmitt.

Yoda-BZH commented 2 years ago

jinja 2.9.4 is provider by python3-jinja2 in debian 9, which is not yet eol.

salt is installed with debian packages provided by repo.saltproject.io/py3/debian/9/amd64/3004

myii commented 2 years ago

jinja 2.9.4 is provider by python3-jinja2 in debian 9, which is not yet eol.

salt is installed with debian packages provided by repo.saltproject.io/py3/debian/9/amd64/3004

@Yoda-BZH To get an idea of how widespread this issue is, I ran a little grep across the package versions collected by the Salt Image Builder that we use for our organisation's CI:

Docker image	Jinja2 version
`alma-08.0-3003.0-py3`	`2.10.1`
`alma-08.0-3003.1-py3`	`2.10.1`
`alma-08.0-3003.2-py3`	`2.10.1`
`alma-08.0-3003.3-py3`	`2.10.1`
`alma-08.0-3004.0-py3`	`2.10.1`
`alma-08.0-master-py3`	`3.0.3`
`alma-08.0-tiamat-py3`	`2.11.3`
`amaz-02.0-3000.9-py3`	`2.10`
`amaz-02.0-3001.7-py3`	`2.10`
`amaz-02.0-3001.8-py3`	`2.10`
`amaz-02.0-3002.6-py3`	`2.10`
`amaz-02.0-3002.7-py3`	`2.10`
`amaz-02.0-3003.0-py3`	`2.10`
`amaz-02.0-3003.1-py3`	`2.10`
`amaz-02.0-3003.2-py3`	`2.10`
`amaz-02.0-3003.3-py3`	`2.10`
`amaz-02.0-3004.0-py3`	`2.10`
`amaz-02.0-master-py3`	`3.0.3`
`amaz-02.0-tiamat-py3`	`2.11.3`
`arch-late-3000.9-py2`	`2.11.3`
`arch-late-3001.7-py3`	`2.11.3`
`arch-late-3001.8-py3`	`2.11.3`
`arch-late-3002.6-py3`	`2.11.3`
`arch-late-3002.7-py3`	`2.11.3`
`arch-late-3003.0-py3`	`2.11.3`
`arch-late-3003.1-py3`	`2.11.3`
`arch-late-3003.2-py3`	`2.11.3`
`arch-late-3003.3-py3`	`2.11.3`
`arch-late-3004.0-py3`	`3.0.3`
`arch-late-master-py3`	`3.0.3`
`cent-07.0-3000.9-py3`	`2.11.1`
`cent-07.0-3001.7-py3`	`2.11.1`
`cent-07.0-3001.8-py3`	`2.11.1`
`cent-07.0-3002.6-py3`	`2.11.1`
`cent-07.0-3002.7-py3`	`2.11.1`
`cent-07.0-3003.0-py3`	`2.11.1`
`cent-07.0-3003.1-py3`	`2.11.1`
`cent-07.0-3003.2-py3`	`2.11.1`
`cent-07.0-3003.3-py3`	`2.11.1`
`cent-07.0-3004.0-py3`	`2.11.1`
`cent-07.0-master-py3`	`3.0.3`
`cent-07.0-tiamat-py3`	`2.11.3`
`cent-08.0-3000.9-py3`	`2.10.1`
`cent-08.0-3001.7-py3`	`2.10.1`
`cent-08.0-3001.8-py3`	`2.10.1`
`cent-08.0-3002.6-py3`	`2.10.1`
`cent-08.0-3002.7-py3`	`2.10.1`
`cent-08.0-3003.0-py3`	`2.10.1`
`cent-08.0-3003.1-py3`	`2.10.1`
`cent-08.0-3003.2-py3`	`2.10.1`
`cent-08.0-3003.3-py3`	`2.10.1`
`cent-08.0-3004.0-py3`	`2.10.1`
`cent-08.0-master-py3`	`3.0.3`
`cent-08.0-tiamat-py3`	`2.11.3`
`cstr-08.0-3003.3-py3`	`2.10.1`
`cstr-08.0-3004.0-py3`	`2.10.1`
`cstr-08.0-master-py3`	`3.0.3`
`cstr-08.0-tiamat-py3`	`2.11.3`
`debi-09.0-3000.9-py3`	`2.9.4`
`debi-09.0-3001.7-py3`	`2.9.4`
`debi-09.0-3001.8-py3`	`2.9.4`
`debi-09.0-3002.6-py3`	`2.9.4`
`debi-09.0-3002.7-py3`	`2.9.4`
`debi-09.0-3003.0-py3`	`2.9.4`
`debi-09.0-3003.1-py3`	`2.9.4`
`debi-09.0-3003.2-py3`	`2.9.4`
`debi-09.0-3003.3-py3`	`2.9.4`
`debi-09.0-3004.0-py3`	`2.9.4`
`debi-09.0-master-py3`	`2.11.3`
`debi-09.0-tiamat-py3`	`2.11.3`
`debi-10.0-3000.9-py3`	`2.10`
`debi-10.0-3001.7-py3`	`2.10`
`debi-10.0-3001.8-py3`	`2.10`
`debi-10.0-3002.6-py3`	`2.10`
`debi-10.0-3002.7-py3`	`2.10`
`debi-10.0-3003.0-py3`	`2.10`
`debi-10.0-3003.1-py3`	`2.10`
`debi-10.0-3003.2-py3`	`2.10`
`debi-10.0-3003.3-py3`	`2.10`
`debi-10.0-3004.0-py3`	`2.10`
`debi-10.0-master-py3`	`3.0.3`
`debi-10.0-tiamat-py3`	`2.11.3`
`debi-11.0-3002.6-py3`	`2.11.3`
`debi-11.0-3002.7-py3`	`2.11.3`
`debi-11.0-3003.1-py3`	`2.11.3`
`debi-11.0-3003.2-py3`	`2.11.3`
`debi-11.0-3003.3-py3`	`2.11.3`
`debi-11.0-3004.0-py3`	`2.11.3`
`debi-11.0-master-py3`	`3.0.3`
`debi-11.0-tiamat-py3`	`2.11.3`
`debi-12.0-3003.2-py3`	`2.11.3`
`debi-12.0-3003.3-py3`	`2.11.3`
`debi-12.0-master-py3`	`3.0.1`
`debi-12.0-tiamat-py3`	`2.11.3`
`fbsd-11.4-3002.6-py3`	`2.11.2`
`fbsd-11.4-3003.1-py3`	`2.11.2`
`fbsd-11.4-master-py3`	`2.11.3`
`fbsd-12.2-3002.6-py3`	`2.11.2`
`fbsd-12.2-3003.1-py3`	`2.11.2`
`fbsd-12.2-master-py3`	`2.11.3`
`fbsd-13.0-3002.6-py3`	`2.11.2`
`fbsd-13.0-3003.1-py3`	`2.11.2`
`fbsd-13.0-master-py3`	`2.11.3`
`fedo-33.0-3001.7-py3`	`2.11.3`
`fedo-33.0-3001.8-py3`	`2.11.3`
`fedo-33.0-3002.6-py3`	`2.11.3`
`fedo-33.0-3002.7-py3`	`2.11.3`
`fedo-33.0-3003.0-py3`	`2.11.3`
`fedo-33.0-3003.1-py3`	`2.11.3`
`fedo-33.0-3003.2-py3`	`2.11.3`
`fedo-33.0-3003.3-py3`	`2.11.3`
`fedo-33.0-3004.0-py3`	`2.11.3`
`fedo-33.0-master-py3`	`3.0.3`
`fedo-34.0-3001.7-py3`	`2.11.3`
`fedo-34.0-3001.8-py3`	`2.11.3`
`fedo-34.0-3002.6-py3`	`2.11.3`
`fedo-34.0-3002.7-py3`	`2.11.3`
`fedo-34.0-3003.0-py3`	`2.11.3`
`fedo-34.0-3003.1-py3`	`2.11.3`
`fedo-34.0-3003.2-py3`	`2.11.3`
`fedo-34.0-3003.3-py3`	`2.11.3`
`fedo-34.0-3004.0-py3`	`2.11.3`
`fedo-34.0-master-py3`	`3.0.3`
`fedo-35.0-3001.7-py3`	`2.11.3`
`fedo-35.0-3001.8-py3`	`2.11.3`
`fedo-35.0-3002.6-py3`	`2.11.3`
`fedo-35.0-3002.7-py3`	`2.11.3`
`fedo-35.0-3003.0-py3`	`2.11.3`
`fedo-35.0-3003.1-py3`	`2.11.3`
`fedo-35.0-3003.3-py3`	`3.0.1`
`gent-late-3001.7-py3`	`2.11.3`
`gent-late-3001.8-py3`	`2.11.3`
`gent-late-3002.5-py3`	`2.11.3`
`gent-late-3002.6-py3`	`2.11.3`
`gent-late-3002.7-py3`	`2.11.3`
`gent-late-3003.0-py3`	`2.11.3`
`gent-late-3003.1-py3`	`2.11.3`
`gent-late-3003.2-py3`	`2.11.3`
`gent-late-3003.3-py3`	`2.11.3`
`gent-late-3004.0-py3`	`2.11.3`
`gent-late-master-py3`	`3.0.3`
`gent-sysd-3001.7-py3`	`2.11.3`
`gent-sysd-3001.8-py3`	`2.11.3`
`gent-sysd-3002.5-py3`	`2.11.3`
`gent-sysd-3002.6-py3`	`2.11.3`
`gent-sysd-3002.7-py3`	`2.11.3`
`gent-sysd-3003.0-py3`	`2.11.3`
`gent-sysd-3003.1-py3`	`2.11.3`
`gent-sysd-3003.2-py3`	`2.11.3`
`gent-sysd-3003.3-py3`	`2.11.3`
`gent-sysd-3004.0-py3`	`2.11.3`
`gent-sysd-master-py3`	`3.0.3`
`obsd-06.8-3001-1-py3`	`2.11.2`
`obsd-06.9-3002-6-py3`	`2.11.2`
`opsu-15.2-3000.9-py3`	`2.11.3`
`opsu-15.2-3001.7-py3`	`2.11.3`
`opsu-15.2-3001.8-py3`	`2.11.3`
`opsu-15.2-3002.2-py3`	`2.10.1`
`opsu-15.2-3002.7-py3`	`2.11.3`
`opsu-15.2-3003.1-py3`	`2.11.3`
`opsu-15.2-3003.2-py3`	`2.11.3`
`opsu-15.2-3003.3-py3`	`2.10.1`
`opsu-15.2-3004.0-py3`	`2.11.3`
`opsu-15.2-master-py3`	`3.0.3`
`opsu-15.3-3000.9-py3`	`2.11.3`
`opsu-15.3-3001.7-py3`	`2.11.3`
`opsu-15.3-3001.8-py3`	`2.11.3`
`opsu-15.3-3002.2-py3`	`2.10.1`
`opsu-15.3-3002.7-py3`	`2.11.3`
`opsu-15.3-3003.1-py3`	`2.11.3`
`opsu-15.3-3003.2-py3`	`2.11.3`
`opsu-15.3-3003.3-py3`	`2.11.3`
`opsu-15.3-3004.0-py3`	`2.10.1`
`opsu-15.3-master-py3`	`3.0.3`
`opsu-tmbl-3001.7-py3`	`2.11.3`
`opsu-tmbl-3001.8-py3`	`2.11.3`
`opsu-tmbl-3002.2-py3`	`2.11.3`
`opsu-tmbl-3002.7-py3`	`2.11.3`
`opsu-tmbl-3003.1-py3`	`2.11.3`
`opsu-tmbl-3003.2-py3`	`2.11.3`
`opsu-tmbl-3003.3-py3`	`2.11.3`
`opsu-tmbl-3004.0-py3`	`2.11.3`
`opsu-tmbl-master-py3`	`3.0.3`
`orac-07.0-3000.9-py3`	`2.11.1`
`orac-07.0-3001.7-py3`	`2.11.1`
`orac-07.0-3001.8-py3`	`2.11.1`
`orac-07.0-3002.6-py3`	`2.11.1`
`orac-07.0-3002.7-py3`	`2.11.1`
`orac-07.0-3003.0-py3`	`2.11.1`
`orac-07.0-3003.1-py3`	`2.11.1`
`orac-07.0-3003.2-py3`	`2.11.1`
`orac-07.0-3003.3-py3`	`2.11.1`
`orac-07.0-3004.0-py3`	`2.11.1`
`orac-07.0-master-py3`	`3.0.3`
`orac-07.0-tiamat-py3`	`2.11.3`
`orac-08.0-3000.9-py3`	`2.10.1`
`orac-08.0-3001.7-py3`	`2.10.1`
`orac-08.0-3001.8-py3`	`2.10.1`
`orac-08.0-3002.6-py3`	`2.10.1`
`orac-08.0-3002.7-py3`	`2.10.1`
`orac-08.0-3003.0-py3`	`2.10.1`
`orac-08.0-3003.1-py3`	`2.10.1`
`orac-08.0-3003.2-py3`	`2.10.1`
`orac-08.0-3003.3-py3`	`2.10.1`
`orac-08.0-3004.0-py3`	`2.10.1`
`orac-08.0-master-py3`	`3.0.3`
`orac-08.0-tiamat-py3`	`2.11.3`
`rock-08.0-3004.0-py3`	`2.10.1`
`rock-08.0-master-py3`	`3.0.3`
`rock-08.0-tiamat-py3`	`2.11.3`
`ubun-18.0-3000.9-py2`	`2.10`
`ubun-18.0-3000.9-py3`	`2.10`
`ubun-18.0-3001.7-py3`	`2.10`
`ubun-18.0-3001.8-py3`	`2.10`
`ubun-18.0-3002.6-py3`	`2.10`
`ubun-18.0-3002.7-py3`	`2.10`
`ubun-18.0-3003.0-py3`	`2.10`
`ubun-18.0-3003.1-py3`	`2.10`
`ubun-18.0-3003.2-py3`	`2.10`
`ubun-18.0-3003.3-py3`	`2.10`
`ubun-18.0-3004.0-py3`	`2.10`
`ubun-18.0-master-py3`	`3.0.3`
`ubun-18.0-tiamat-py3`	`2.11.3`
`ubun-20.0-3001.7-py3`	`2.10.1`
`ubun-20.0-3001.8-py3`	`2.10.1`
`ubun-20.0-3002.6-py3`	`2.10.1`
`ubun-20.0-3002.7-py3`	`2.10.1`
`ubun-20.0-3003.0-py3`	`2.10.1`
`ubun-20.0-3003.1-py3`	`2.10.1`
`ubun-20.0-3003.2-py3`	`2.10.1`
`ubun-20.0-3003.3-py3`	`2.10.1`
`ubun-20.0-3004.0-py3`	`2.10.1`
`ubun-20.0-master-py3`	`3.0.3`
`ubun-20.0-tiamat-py3`	`2.11.3`
`wind-08.1-latest-py3`	`2.10.1`
`wind-10.0-latest-py3`	`2.10.1`
`wind-2016-latest-py3`	`2.11.3`
`wind-2019-latest-py3`	`2.11.3`

So Debian 9 is the only platform which is still using Jinja2 < 2.10.
Debian 9 will hit EOL on 2022-06-30 -- almost 5 months exactly.
A strange thing is that the CI config has been updated to test this hourly functionality and the Debian 9 instance is passing fine.
@mdschmitt Any thoughts about this?

mdschmitt commented 2 years ago

I can look into why the CI is succeeding where it shouldn't be, but considering Debian 9 has a 5 year LTS cycle and we're 5 months from its end, I see very little value in shimming this. I think there's probably a lot more value (even outside of a Salt context) in the end user upgrading their systems to at least a more recent LTS Debian version.

That said, if you want to make a PR that fixes this for these next 5 months @Yoda-BZH, I'm happy to review it.

Yoda-BZH commented 2 years ago

I just checked, python3-jinja2 2.10 is available through stretch-backports.

Alternatively, Depends: python3-jinja2 (>= 2.10) could be forced on package installation/update.

I'll see to make a PR on the formula.

mdschmitt commented 1 year ago

Debian 9 is now well past its LTS cycle. I think this can be closed.

saltstack-formulas / logrotate-formula