Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

json-ify environment values #61295

Closed
wants to merge 2 commits into from

Conversation

@jborean93
Copy link
Contributor

commented Aug 25, 2019

SUMMARY

If an environment value (of an env key) set on a task is a dict or a list then the actual env value may not be what's expected. This PR converts these values to a json to ensure consistency across shell plugins.

Fixes #60956

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

environment

OTHER

Using the following playbook as an example

---
- hosts: '2019'
  gather_facts: no
  vars:
    some_var: some_value
    bla:
      foo: bar
      aaa: '{{ some_var }}'
      zzz: baz
  tasks:
  - win_shell: $env:testvar
    environment:
      testvar: '{{ item }}'
    loop:
    - '{{ bla }}'
    - [ '{{ bla }}' ]

  - shell: env | grep testvar
    environment:
      testvar: '{{ item }}'
    loop:
    - '{{ bla }}'
    - [ '{{ bla }}' ]
    delegate_to: localhost

Here is what happens before the PR

TASK [win_shell] ****************************************************************************************
task path: /home/jborean/dev/ansible-tester/adhoc.yml:11
Using module file /home/jborean/dev/ansible/lib/ansible/modules/windows/win_shell.ps1
Pipelining is enabled.
<server2019.domain.local> ESTABLISH WINRM CONNECTION FOR USER: vagrant-domain@DOMAIN.LOCAL on PORT 5985 TO server2019.domain.local
EXEC (via pipeline wrapper)
changed: [2019] => (item={'foo': 'bar', 'aaa': 'some_value', 'zzz': 'baz'}) => {
    "ansible_loop_var": "item",                                                                          
    "changed": true,                                                                                                                                                                                                   "cmd": "$env:testvar",                                                                               
    "delta": "0:00:00.344241",                                                                           
    "end": "2019-08-25 09:23:51.109701",                                                                                                                                                                           
    "item": {                                                                                                                                                                                                              "aaa": "some_value",
        "foo": "bar",                                                                                                                                                                                                      "zzz": "baz"                                                                                                                                                                                                   },            
    "rc": 0,                                                                                                                                                                                                       
    "start": "2019-08-25 09:23:50.765459",                                                               
    "stderr": "",              
    "stderr_lines": [],
    "stdout": "System.Collections.Hashtable\r\n",
    "stdout_lines": [         
        "System.Collections.Hashtable"  
    ]              
}                       
Using module file /home/jborean/dev/ansible/lib/ansible/modules/windows/win_shell.ps1
Pipelining is enabled.          
EXEC (via pipeline wrapper)
changed: [2019] => (item=[{'foo': 'bar', 'aaa': 'some_value', 'zzz': 'baz'}]) => {
    "ansible_loop_var": "item",
    "changed": true,           
    "cmd": "$env:testvar",  
    "delta": "0:00:00.327646",
    "end": "2019-08-25 09:23:52.640482",
    "item": [                        
        {               
            "aaa": "some_value",
            "foo": "bar",
            "zzz": "baz"
        }
    ],                          
    "rc": 0,             
    "start": "2019-08-25 09:23:52.312835",
    "stderr": "",
    "stderr_lines": [],
    "stdout": "System.Object[]\r\n",
    "stdout_lines": [                     
        "System.Object[]"
    ]                  
}                                                                                                        
                                                    
TASK [shell] ******************************************************************************************** 
task path: /home/jborean/dev/ansible-tester/adhoc.yml:18
<localhost> ESTABLISH LOCAL CONNECTION FOR USER: jborean
<localhost> EXEC /bin/sh -c 'echo ~jborean && sleep 0'
<localhost> EXEC /bin/sh -c '( umask 77 && mkdir -p "` echo /home/jborean/.ansible/tmp/ansible-tmp-1566768232.8760302-15286391486797 `" && echo ansible-tmp-1566768232.8760302-15286391486797="` echo /home/jborean
/.ansible/tmp/ansible-tmp-1566768232.8760302-15286391486797 `" ) && sleep 0'
Using module file /home/jborean/dev/ansible/lib/ansible/modules/commands/command.py                                                                                                                                
<localhost> PUT /home/jborean/.ansible/tmp/ansible-local-7891ckdydgjc/tmp35yo5qda TO /home/jborean/.ansible/tmp/ansible-tmp-1566768232.8760302-15286391486797/AnsiballZ_command.py
<localhost> EXEC /bin/sh -c 'chmod u+x /home/jborean/.ansible/tmp/ansible-tmp-1566768232.8760302-15286391486797/ /home/jborean/.ansible/tmp/ansible-tmp-1566768232.8760302-15286391486797/AnsiballZ_command.py && s
leep 0'                                                                                                  
<localhost> EXEC /bin/sh -c 'testvar='"'"'{'"'"'"'"'"'"'"'"'foo'"'"'"'"'"'"'"'"': '"'"'"'"'"'"'"'"'bar'"'"'"'"'"'"'"'"', '"'"'"'"'"'"'"'"'aaa'"'"'"'"'"'"'"'"': '"'"'"'"'"'"'"'"'some_value'"'"'"'"'"'"'"'"', '"'"'
"'"'"'"'"'"'zzz'"'"'"'"'"'"'"'"': '"'"'"'"'"'"'"'"'baz'"'"'"'"'"'"'"'"'}'"'"' /home/jborean/venvs/ansible-py37/bin/python /home/jborean/.ansible/tmp/ansible-tmp-1566768232.8760302-15286391486797/AnsiballZ_comman
d.py && sleep 0'
<localhost> EXEC /bin/sh -c 'rm -f -r /home/jborean/.ansible/tmp/ansible-tmp-1566768232.8760302-15286391486797/ > /dev/null 2>&1 && sleep 0'
changed: [2019 -> localhost] => (item={'foo': 'bar', 'aaa': 'some_value', 'zzz': 'baz'}) => {
    "ansible_loop_var": "item",
    "changed": true,
    "cmd": "env | grep testvar",
    "delta": "0:00:00.003535",
    "end": "2019-08-26 07:23:53.076507",
    "invocation": {
        "module_args": {
            "_raw_params": "env | grep testvar",
            "_uses_shell": true,
            "argv": null,
            "chdir": null,
            "creates": null,
            "executable": null,
            "removes": null,
            "stdin": null,
            "stdin_add_newline": true,
            "strip_empty_ends": true,
            "warn": true
        }
    },
    "item": {
        "aaa": "some_value",
        "foo": "bar",
        "zzz": "baz"
    },
    "rc": 0,
    "start": "2019-08-26 07:23:53.072972",
    "stderr": "",
    "stderr_lines": [],
    "stdout": "testvar={'foo': 'bar', 'aaa': 'some_value', 'zzz': 'baz'}",
    "stdout_lines": [
        "testvar={'foo': 'bar', 'aaa': 'some_value', 'zzz': 'baz'}"
    ]
}
<localhost> EXEC /bin/sh -c 'echo ~jborean && sleep 0'
<localhost> EXEC /bin/sh -c '( umask 77 && mkdir -p "` echo /home/jborean/.ansible/tmp/ansible-tmp-1566768233.0985906-61227789404856 `" && echo ansible-tmp-1566768233.0985906-61227789404856="` echo /home/jborean/.ansible/tmp/ansible-tmp-1566768233.0985906-61227789404856 `" ) && sleep 0'
Using module file /home/jborean/dev/ansible/lib/ansible/modules/commands/command.py
<localhost> PUT /home/jborean/.ansible/tmp/ansible-local-7891ckdydgjc/tmpp0nn6ko1 TO /home/jborean/.ansible/tmp/ansible-tmp-1566768233.0985906-61227789404856/AnsiballZ_command.py
<localhost> EXEC /bin/sh -c 'chmod u+x /home/jborean/.ansible/tmp/ansible-tmp-1566768233.0985906-61227789404856/ /home/jborean/.ansible/tmp/ansible-tmp-1566768233.0985906-61227789404856/AnsiballZ_command.py && sleep 0'
<localhost> EXEC /bin/sh -c 'testvar='"'"'[{'"'"'"'"'"'"'"'"'foo'"'"'"'"'"'"'"'"': '"'"'"'"'"'"'"'"'bar'"'"'"'"'"'"'"'"', '"'"'"'"'"'"'"'"'aaa'"'"'"'"'"'"'"'"': '"'"'"'"'"'"'"'"'some_value'"'"'"'"'"'"'"'"', '"'"'"'"'"'"'"'"'zzz'"'"'"'"'"'"'"'"': '"'"'"'"'"'"'"'"'baz'"'"'"'"'"'"'"'"'}]'"'"' /home/jborean/venvs/ansible-py37/bin/python /home/jborean/.ansible/tmp/ansible-tmp-1566768233.0985906-61227789404856/AnsiballZ_command.py && sleep 0'
<localhost> EXEC /bin/sh -c 'rm -f -r /home/jborean/.ansible/tmp/ansible-tmp-1566768233.0985906-61227789404856/ > /dev/null 2>&1 && sleep 0'
changed: [2019 -> localhost] => (item=[{'foo': 'bar', 'aaa': 'some_value', 'zzz': 'baz'}]) => {
    "ansible_loop_var": "item",
    "changed": true,
    "cmd": "env | grep testvar",
    "delta": "0:00:00.003162",
    "end": "2019-08-26 07:23:53.233212",
    "invocation": {
        "module_args": {
            "_raw_params": "env | grep testvar",
            "_uses_shell": true,
            "argv": null,
            "chdir": null,
            "creates": null,
            "executable": null,
            "removes": null,
            "stdin": null,
            "stdin_add_newline": true,
            "strip_empty_ends": true,
            "warn": true
        }
    },
    "item": [
        {
            "aaa": "some_value",
            "foo": "bar",
            "zzz": "baz"
        }
    ],
    "rc": 0,
    "start": "2019-08-26 07:23:53.230050",
    "stderr": "",
    "stderr_lines": [],
    "stdout": "testvar=[{'foo': 'bar', 'aaa': 'some_value', 'zzz': 'baz'}]",
    "stdout_lines": [
        "testvar=[{'foo': 'bar', 'aaa': 'some_value', 'zzz': 'baz'}]"
    ]
}

Here is what happens after the PR

TASK [win_shell] ****************************************************************************************
task path: /home/jborean/dev/ansible-tester/adhoc.yml:11                              
Using module file /home/jborean/dev/ansible/lib/ansible/modules/windows/win_shell.ps1
Pipelining is enabled.                                                                                   
<server2019.domain.local> ESTABLISH WINRM CONNECTION FOR USER: vagrant-domain@DOMAIN.LOCAL on PORT 5985 TO server2019.domain.local
EXEC (via pipeline wrapper)                         
changed: [2019] => (item={'foo': 'bar', 'aaa': 'some_value', 'zzz': 'baz'}) => {
    "ansible_loop_var": "item",                                                                                                                                                                                    
    "changed": true,                                                                                     
    "cmd": "$env:testvar",                                                                               
    "delta": "0:00:00.329618",                                                                                                                                                                                     
    "end": "2019-08-25 09:23:06.157097",                                                                                                                                                                           
    "item": {                                       
        "aaa": "some_value",                                                                                                                                                                                       
        "foo": "bar",                               
        "zzz": "baz"                                                                                                                                                                                               
    },                                                                                                   
    "rc": 0,                                        
    "start": "2019-08-25 09:23:05.827478",          
    "stderr": "",                                   
    "stderr_lines": [],                             
    "stdout": "{\"foo\": \"bar\", \"aaa\": \"some_value\", \"zzz\": \"baz\"}\r\n",
    "stdout_lines": [                               
        "{\"foo\": \"bar\", \"aaa\": \"some_value\", \"zzz\": \"baz\"}"
    ]                                               
}                                                   
Using module file /home/jborean/dev/ansible/lib/ansible/modules/windows/win_shell.ps1
Pipelining is enabled.                              
EXEC (via pipeline wrapper) 
changed: [2019] => (item=[{'foo': 'bar', 'aaa': 'some_value', 'zzz': 'baz'}]) => {
    "ansible_loop_var": "item",
    "changed": true,                                
    "cmd": "$env:testvar",            
    "delta": "0:00:00.313245",       
    "end": "2019-08-25 09:23:07.688085",            
    "item": [                                       
        {                                           
            "aaa": "some_value",                    
            "foo": "bar",                           
            "zzz": "baz"                            
        }                                           
    ],                                              
    "rc": 0,                                        
    "start": "2019-08-25 09:23:07.374839",          
    "stderr": "",                                   
    "stderr_lines": [],                             
    "stdout": "[{\"foo\": \"bar\", \"aaa\": \"some_value\", \"zzz\": \"baz\"}]\r\n",
    "stdout_lines": [                               
        "[{\"foo\": \"bar\", \"aaa\": \"some_value\", \"zzz\": \"baz\"}]"               
    ]                                               
}                                                                                                        
                                                    
TASK [shell] ********************************************************************************************
task path: /home/jborean/dev/ansible-tester/adhoc.yml:18
<localhost> ESTABLISH LOCAL CONNECTION FOR USER: jborean
<localhost> EXEC /bin/sh -c 'echo ~jborean && sleep 0'
<localhost> EXEC /bin/sh -c '( umask 77 && mkdir -p "` echo /home/jborean/.ansible/tmp/ansible-tmp-1566768187.9035008-78083565786418 `" && echo ansible-tmp-1566768187.9035008-78083565786418="` echo /home/jborean
/.ansible/tmp/ansible-tmp-1566768187.9035008-78083565786418 `" ) && sleep 0'                                                                                                                                       
Using module file /home/jborean/dev/ansible/lib/ansible/modules/commands/command.py
<localhost> PUT /home/jborean/.ansible/tmp/ansible-local-7389gdide8_w/tmpbbqxe6w7 TO /home/jborean/.ansible/tmp/ansible-tmp-1566768187.9035008-78083565786418/AnsiballZ_command.py
<localhost> EXEC /bin/sh -c 'chmod u+x /home/jborean/.ansible/tmp/ansible-tmp-1566768187.9035008-78083565786418/ /home/jborean/.ansible/tmp/ansible-tmp-1566768187.9035008-78083565786418/AnsiballZ_command.py && s
leep 0'                                             
<localhost> EXEC /bin/sh -c 'testvar='"'"'{"foo": "bar", "aaa": "some_value", "zzz": "baz"}'"'"' /home/jborean/venvs/ansible-py37/bin/python /home/jborean/.ansible/tmp/ansible-tmp-1566768187.9035008-780835657864
18/AnsiballZ_command.py && sleep 0'                 
<localhost> EXEC /bin/sh -c 'rm -f -r /home/jborean/.ansible/tmp/ansible-tmp-1566768187.9035008-78083565786418/ > /dev/null 2>&1 && sleep 0'
changed: [2019 -> localhost] => (item={'foo': 'bar', 'aaa': 'some_value', 'zzz': 'baz'}) => {
    "ansible_loop_var": "item",
    "changed": true,
    "cmd": "env | grep testvar",
    "delta": "0:00:00.003325",
    "end": "2019-08-26 07:23:08.105863",
    "invocation": {
        "module_args": {
            "_raw_params": "env | grep testvar",
            "_uses_shell": true,
            "argv": null,
            "chdir": null,
            "creates": null,
            "executable": null,
            "removes": null,
            "stdin": null,
            "stdin_add_newline": true,
            "strip_empty_ends": true,
            "warn": true
        }
    },
    "item": {
        "aaa": "some_value",
        "foo": "bar",
        "zzz": "baz"
    },
    "rc": 0,
    "start": "2019-08-26 07:23:08.102538",
    "stderr": "",
    "stderr_lines": [],
    "stdout": "testvar={\"foo\": \"bar\", \"aaa\": \"some_value\", \"zzz\": \"baz\"}",
    "stdout_lines": [
        "testvar={\"foo\": \"bar\", \"aaa\": \"some_value\", \"zzz\": \"baz\"}"
    ]
}
<localhost> EXEC /bin/sh -c 'echo ~jborean && sleep 0'
<localhost> EXEC /bin/sh -c '( umask 77 && mkdir -p "` echo /home/jborean/.ansible/tmp/ansible-tmp-1566768188.1285756-66597294527812 `" && echo ansible-tmp-1566768188.1285756-66597294527812="` echo /home/jborean
/.ansible/tmp/ansible-tmp-1566768188.1285756-66597294527812 `" ) && sleep 0'
Using module file /home/jborean/dev/ansible/lib/ansible/modules/commands/command.py
<localhost> PUT /home/jborean/.ansible/tmp/ansible-local-7389gdide8_w/tmp6vs7lh90 TO /home/jborean/.ansible/tmp/ansible-tmp-1566768188.1285756-66597294527812/AnsiballZ_command.py
<localhost> EXEC /bin/sh -c 'chmod u+x /home/jborean/.ansible/tmp/ansible-tmp-1566768188.1285756-66597294527812/ /home/jborean/.ansible/tmp/ansible-tmp-1566768188.1285756-66597294527812/AnsiballZ_command.py && s
leep 0'
<localhost> EXEC /bin/sh -c 'testvar='"'"'[{"foo": "bar", "aaa": "some_value", "zzz": "baz"}]'"'"' /home/jborean/venvs/ansible-py37/bin/python /home/jborean/.ansible/tmp/ansible-tmp-1566768188.1285756-6659729452
7812/AnsiballZ_command.py && sleep 0'
<localhost> EXEC /bin/sh -c 'rm -f -r /home/jborean/.ansible/tmp/ansible-tmp-1566768188.1285756-66597294527812/ > /dev/null 2>&1 && sleep 0'
changed: [2019 -> localhost] => (item=[{'foo': 'bar', 'aaa': 'some_value', 'zzz': 'baz'}]) => {
    "ansible_loop_var": "item",
    "changed": true,
    "cmd": "env | grep testvar",
    "delta": "0:00:00.003017",
    "end": "2019-08-26 07:23:08.258902",
    "invocation": {
        "module_args": {
            "_raw_params": "env | grep testvar",
            "_uses_shell": true,
            "argv": null,
            "chdir": null,
            "creates": null,
            "executable": null,
            "removes": null,
            "stdin": null,
            "stdin_add_newline": true,
            "strip_empty_ends": true,
            "warn": true
        }
    },
    "item": [
        {
            "aaa": "some_value",
            "foo": "bar",
            "zzz": "baz"
        }
    ],
    "rc": 0,
    "start": "2019-08-26 07:23:08.255885",
    "stderr": "",
    "stderr_lines": [],
    "stdout": "testvar=[{\"foo\": \"bar\", \"aaa\": \"some_value\", \"zzz\": \"baz\"}]",
    "stdout_lines": [
        "testvar=[{\"foo\": \"bar\", \"aaa\": \"some_value\", \"zzz\": \"baz\"}]"
    ]
}

We can see that before PowerShell is completely useless as the raw object being sent to the host is an actual dict/list and .NET is not helpful in their ToString implementations. On Python it is slightly more useful but the values aren't fully JSON compliant with their single quotes instead of double quotes. With this PR now both platforms align with each other and the JSON being set as the env var in Python uses double quotes.

An alternative is to set the key inside an actual string like;

environment:
  testvar: '"{{ my_dict }}"'`

But this causes the env var to be testvar="{"key": "value"}" where the value is double quoted potentially causing more issues with parsing it.

@ansibot

This comment has been minimized.

@ansibot ansibot added core_review and removed needs_revision labels Aug 25, 2019

@jamescassell
Copy link
Contributor

left a comment

shipit

Makes sense to me

# Ensure any env values that are dicts are converted to JSON
# https://github.com/ansible/ansible/issues/60956
for env_key, env_val in temp_environment.items():
if isinstance(env_val, dict) or isinstance(env_val, list):

This comment has been minimized.

Copy link
@bcoca

bcoca Aug 26, 2019

Member

use Mapping and Sequence classes instead as you might get an AnsibleSequence or similar object that does not inherit directly from 'list`

This comment has been minimized.

Copy link
@sivel

sivel Aug 26, 2019

Member

the is_sequence function would be preferred, to make sure it isn't picking up strings too.

# https://github.com/ansible/ansible/issues/60956
for env_key, env_val in temp_environment.items():
if isinstance(env_val, dict) or isinstance(env_val, list):
temp_environment[env_key] = json.dumps(env_val)

This comment has been minimized.

Copy link
@bcoca

bcoca Aug 26, 2019

Member

while this might make sense for dictionaries, since shell doesn't really support them, for lists it might not as they are supported by shells

@bcoca

This comment has been minimized.

Copy link
Member

commented Aug 26, 2019

I'm not sure if this should be a JSON transform, seems more like an error to me. Users should be able to |to_json the value of an env var if that is what they really want.

@jborean93

This comment has been minimized.

Copy link
Contributor Author

commented Aug 26, 2019

Going by @bcoca's comments above and it looks like we would want to error out if the type is a sequence type instead of assuming the user wants JSON. The original issue is only a problem if we use this pattern;

- set_fact:
    json_str: '{{ my_dict | to_json }}'

- shell: env | grep testenv
  environment:
    testenv: '{{ json_str }}'

Ansible's magic eval of values here is causing the string json_str to be parsed as a dict as it starts with {. If you were to enable DEFAULT_JINJA2_NATIVE then the above work fine as the Ansible magic no longer applies. If you cannot enable that config option then omitting the set_fact altogether and using to_json in the env section is also an alternative:

- shell: env | grep testenv
  environment:
    testenv: '{{ my_dict | to_json }}'

Because there are options today I propose we change this PR to error out instead of converting to json is a sequence type is encountered. I'll talk about this at the public agenda a bit more.

@ansibot ansibot added the stale_ci label Sep 3, 2019

@jborean93 jborean93 closed this Sep 12, 2019

@jborean93 jborean93 deleted the jborean93:environment-dict branch Sep 12, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.