Fix value of nginx-proxy tmpfs size

[huguesdk]

Use an int conversion in the computation of the value of
matrix_nginx_proxy_tmp_directory_size_mb, to have the integer value
multiplied by 50 instead of having the string repeated 50 times.

[spantaleev]

I wonder.. In what situation would a string value end up in matrix_nginx_proxy_proxy_matrix_federation_api_client_max_body_size_mb?

Someone explicitly messing up the data type like this:
matrix_nginx_proxy_proxy_matrix_federation_api_client_max_body_size_mb: "30"

I guess that would be pretty rare, but.. it doesn't hurt to have such defensive casting just in case.

---

We currently have a chain like this:

# Your new int cast here
matrix_nginx_proxy_tmp_directory_size_mb: "{{ (matrix_nginx_proxy_proxy_matrix_federation_api_client_max_body_size_mb | int) * 50 }}"

matrix_nginx_proxy_proxy_matrix_federation_api_client_max_body_size_mb: "{{ matrix_nginx_proxy_proxy_matrix_client_api_client_max_body_size_mb * 3 }}"

matrix_nginx_proxy_proxy_matrix_client_api_client_max_body_size_mb: 25

If someone incorrectly sets matrix_nginx_proxy_proxy_matrix_client_api_client_max_body_size_mb (e.g. "30"), we'd get an incorrect matrix_nginx_proxy_proxy_matrix_federation_api_client_max_body_size_mb ("303030"), leading us to calculate an incorrect matrix_nginx_proxy_tmp_directory_size_mb (15151500) even with the new cast.

Perhaps we need a cast there (matrix_nginx_proxy_proxy_matrix_federation_api_client_max_body_size_mb) as well, and not just at matrix_nginx_proxy_tmp_directory_size_mb?

[huguesdk]

With the current master and the default settings, /etc/systemd/system/matrix-nginx-proxy.service contains:

--tmpfs=/tmp:rw,noexec,nosuid,size=3030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030303030m \

I also wonder why this happens. It’s not clear to me when a value is considered as an integer or as a string. group_vars/matrix-servers contains:

matrix_nginx_proxy_proxy_matrix_client_api_client_max_body_size_mb: "{{ matrix_synapse_max_upload_size_mb }}"

But this is not the problem, as this value is correctly multiplied by 3 in roles/matrix-nginx-proxy/defaults/main.yml:

matrix_nginx_proxy_proxy_matrix_federation_api_client_max_body_size_mb: "{{ matrix_nginx_proxy_proxy_matrix_client_api_client_max_body_size_mb * 3 }}"

matrix_nginx_proxy_proxy_matrix_federation_api_client_max_body_size_mb has the correct value, as seen in the templates that use it. But in the same file, that value is then multiplied by 50, and this results in the incorrect value. I tried adding a cast in the definition of matrix_nginx_proxy_proxy_matrix_federation_api_client_max_body_size_mb but it doesn’t change anything.

Strange, isn’t it? I’m probably missing something here…

[spantaleev]

It looks like all variables have their type determined at YAML parsing time.

---

An integer literal is considered an integer. Example:

# matrix_nginx_proxy_proxy_matrix_client_api_client_max_body_size_mb = 25
matrix_nginx_proxy_proxy_matrix_client_api_client_max_body_size_mb: 25

---

A variable defined in terms of another one using a jinja2 template ends up being considered a string. Example:

# matrix_nginx_proxy_proxy_matrix_federation_api_client_max_body_size_mb = "75"
matrix_nginx_proxy_proxy_matrix_federation_api_client_max_body_size_mb: "{{ matrix_nginx_proxy_proxy_matrix_client_api_client_max_body_size_mb * 3 }}"

Note how the computation happens between 3 and the matrix_nginx_proxy_proxy_matrix_client_api_client_max_body_size_mb integer variable. Both are integers, so the computation is correct.

The resulting value, however, is put in a string, because matrix_nginx_proxy_proxy_matrix_federation_api_client_max_body_size_mb appears to be a string template variable.

---

And finally, where our problem lies, is when the such an "is it a integer or a string?" variable is used for a numeric computation:

# Due to matrix_nginx_proxy_proxy_matrix_federation_api_client_max_body_size_mb being a string ("75"),
# this becomes a computation between a string and a number (that is, string repetition).
matrix_nginx_proxy_tmp_directory_size_mb: "{{ matrix_nginx_proxy_proxy_matrix_federation_api_client_max_body_size_mb * 50 }}"

---

This issue is described in ansible/ansible#15249 and fixed in ansible/ansible#32738

There's this Jinja2 thing called Native Python Types, which when enabled will produce the proper types for variables coming from templates.

To enable it in Ansible (>= 2.7 required; Jinja 2.10 also required), one needs to either use jinja2_native = True in ansible.cfg or set a ANSIBLE_JINJA2_NATIVE=true environment variable. I've done the former in 3387035.

Documentation for this setting is here.

This fixes the issue for me (Ansible 2.7.10), but won't fix the issue for people on older versions.
For them, we need the workaround proposed by you here, so I'm merging it as it is.

I'm also hoping that people having Ansible 2.7+ also have a recent version of Jinja2 (>= 2.10).
Otherwise, they might experience breakage (I haven't tested, but the jinja2_native documentation leads me to believe this may be what we should expect to happen).

Thank you! 👍

[spantaleev]

I've reverted the jinja2_native change I introduced in 5be1d50, because it introduces some other regressions.

For the time being, we rely on your workaround only.

[huguesdk]

Wow, thanks for the detailed explanation!

I digged a little more into this, and it seems that the problem comes from the fact that Ansible is doing some type guessing when it can easily (as explained in ansible/ansible#9362 (comment)), but this breaks as soon as an extra reference level is added, or if an operation is done on the value. Here is a test case:

test.yaml:

- hosts: all
  gather_facts: False
  vars_files:
    - test_vars.yaml
  tasks:
    - debug:
        var: test_var_1
    - debug:
        var: test_var_2
    - debug:
        var: test_var_3
    - debug:
        var: test_var_4
    - debug:
        var: test_var_5
    - debug:
        var: test_var_6
    - debug:
        var: test_var_7
    - debug:
        var: test_var_8

test_vars.yaml:

test_var_1: 42
test_var_2: "{{ test_var_1 }}"
test_var_3: "{{ test_var_1 * 1 }}"
test_var_4: "{{ test_var_2 }}"
test_var_5: "{{ test_var_1 * 2 }}"
test_var_6: "{{ test_var_2 * 2 }}"
test_var_7: "{{ test_var_3 * 2 }}"
test_var_8: "{{ test_var_4 * 2 }}"

Run with:

ansible-playbook test.yaml -i localhost, -c local

Output:

PLAY [all] ********************************************************************************************************************************************************************************************************

TASK [debug] ******************************************************************************************************************************************************************************************************
ok: [localhost] => {
    "test_var_1": 42
}

TASK [debug] ******************************************************************************************************************************************************************************************************
ok: [localhost] => {
    "test_var_2": "42"
}

TASK [debug] ******************************************************************************************************************************************************************************************************
ok: [localhost] => {
    "test_var_3": "42"
}

TASK [debug] ******************************************************************************************************************************************************************************************************
ok: [localhost] => {
    "test_var_4": "42"
}

TASK [debug] ******************************************************************************************************************************************************************************************************
ok: [localhost] => {
    "test_var_5": "84"
}

TASK [debug] ******************************************************************************************************************************************************************************************************
ok: [localhost] => {
    "test_var_6": "84"
}

TASK [debug] ******************************************************************************************************************************************************************************************************
ok: [localhost] => {
    "test_var_7": "4242"
}

TASK [debug] ******************************************************************************************************************************************************************************************************
ok: [localhost] => {
    "test_var_8": "4242"
}

PLAY RECAP ********************************************************************************************************************************************************************************************************
localhost                  : ok=8    changed=0    unreachable=0    failed=0

Notice how test_var_2 is correctly multiplied (result in test_var_6) even if it is a string, while test_var_3 and test_var_4 which contain the same value aren’t. Nasty one!

[huguesdk]

I’m not sure whether my last explanation is clear enough. I was wondering why the first multiply works correctly:

matrix_nginx_proxy_proxy_matrix_federation_api_client_max_body_size_mb: "{{ matrix_nginx_proxy_proxy_matrix_client_api_client_max_body_size_mb * 3 }}"

even though matrix_nginx_proxy_proxy_matrix_federation_api_client_max_body_size_mb is (re)defined in group_vars/matrix-servers as:

matrix_nginx_proxy_proxy_matrix_client_api_client_max_body_size_mb: "{{ matrix_synapse_max_upload_size_mb }}"

which looks like it would make the variable a string. It is similar to test_var_2 here above. I think that this implicit behavior is nasty. Because of this, it would probably be better to always convert a variable to int explicitly when doing arithmetic operations, don’t you think?

[spantaleev]

I agree!

This casting thing is a huge gotcha that doesn't even seem mentioned on the Ansible documentation (based on what I was able to find, at least). Nasty indeed..

Because Ansible isn't our friend and people may even define variables incorrectly (e.g. "25"), we'd better cast in all places that do arithmetic operations. I've added casting for the other case as well in 0e391b5.
If you think there are others, please let me know or start a pull request!

[huguesdk]

Good! I think this is indeed good practice to shield ourselves against unexpected (and hard to detect) edge cases. Thanks!

I opened an issue in Ansible: ansible/ansible#55891 but it was closed directly, so there is apparently no plan to change this default behavior.

[spantaleev]

Thanks for opening an issue there!

It's unfortunate that they won't pursue a better (on-by-default) solution.

I'd still like to investigate enabling jinja2_native in the future. Need to try it again soon and see why it causes template generation bugs ({% if %}'s body outputted twice).