ansistrano / deploy

Ansible role to deploy scripting applications like PHP, Python, Ruby, etc. in a capistrano style
https://ansistrano.com
MIT License
2.37k stars 343 forks source link

Add ansistrano_keep_releases casting to int to avoid "Unexpected templating type error" #334

Closed ewypych closed 5 years ago

ewypych commented 5 years ago

I faced the following problem when I made a Python upgrade to 3.7.4:

fatal: [host]: FAILED! => {}

MSG:

The conditional check 'ansistrano_keep_releases > 0' failed. The error was: Unexpected templating type error occurred on ({% if ansistrano_keep_releases > 0 %} True {% else %} False {% endif %}): '>' not supported between instances of 'str' and 'int'

The error appears to have been in '/path/to/playbook/roles/ansistrano.deploy/tasks/cleanup.yml': line 3, column 3, but may
be elsewhere in the file depending on the exact syntax problem.

The offending line appears to be:

# Clean up releases
- name: ANSISTRANO | Clean up releases
  ^ here

The problem was in when statement:

when: ansistrano_keep_releases > 0

ansistrano_keep_releases is treated as a string, but there we have comparison int and string. This change solves the issue.

ricardclau commented 5 years ago

Is this something specific to Python 3.7.4? What version of ansible and jinja have you in your system? Just trying to narrow it down, it cannot hurt to specifically cast to int here

ewypych commented 5 years ago

Is this something specific to Python 3.7.4?

It's a good question. :)

My current setup:

Ansible: 2.5.14
Python: 3.7.4
Jinja2: 2.10.1

I didn't test that on the newer Ansible versions, but anyway it's very interesting, that in the same file you have casting to int in one line:

4: shell: ls -1dt {{ ansistrano_releases_path }}/* | tail -n +{{ ansistrano_keep_releases | int + 1 }} | xargs rm -rf

and didn't have that one line below.

The problem doesn't exist on our Jenkins, where we have the following versions:

Ansible: 2.5.14
Python: 2.7.10
Jinja2: 2.10

Changelog for 2.10.1 Jinja2 versions says that this is the only one change:

SandboxedEnvironment securely handles str.format_map in order to prevent code execution through untrusted format strings. The sandbox already handled str.format

ricardclau commented 5 years ago

Considering what you said, this problem might be introduced with Jinja 2.10.1

But yeah, we made the casting in the that line to ensure there was always an integer, but we did not do it in the when statement ¯_(ツ)_/¯ so there is no reason to not do the same here :)