pyobjects behaving weirdly when calling python stdlib package

[grischa]

I am using pyobjects, silly me...
The following code doesn't work:

    password = ''.join(random.SystemRandom().choice(
        string.ascii_letters + string.digits) for _ in range(20))

it fails with this message:

    Data failed to compile:
----------
    Rendering SLS failed, render error: global name 'random' is not defined
Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/salt/state.py", line 2425, in render_state
    sls, rendered_sls=mods
  File "/usr/lib/python2.7/dist-packages/salt/template.py", line 84, in compile_template
    ret = render(input_data, saltenv, sls, **render_kwargs)
  File "/usr/lib/python2.7/dist-packages/salt/renderers/pyobjects.py", line 450, in render
    exec final_template in _globals, _locals
  File "<string>", line 12, in <module>
  File "<string>", line 12, in <genexpr>
NameError: global name 'random' is not defined

However, it works fine when I separate the line.

    charlist = [random.SystemRandom().choice(
        string.ascii_letters + string.digits) for _ in range(20)]
    password = ''.join(charlist)

Of course, I have import random, string at the top of the file

Does this really hand-parse python up to an arbitrary level of complexity? If yes, please document this, so people don't expect too much.

[basepi]

I don't know the answer here. The Pyobjects renderer is maintained by @borgstrom, so he may have something to add. I usually recommend using the py renderer if you want python rendering.

[borgstrom]

I will at this tonight.

On Mar 19, 2015, at 4:04 PM, Colton Myers notifications@github.com wrote:

I don't know the answer here. The Pyobjects renderer is maintained by @borgstrom, so he may have something to add. I usually recommend using the py renderer if you want python rendering.

—
Reply to this email directly or view it on GitHub.

[borgstrom]

WRT your comment:

Does this really hand-parse python up to an arbitrary level of complexity? If yes, please document this, so people don't expect too much.

We don't really hard-parse things that much, the only hand parsing we do is to allow for salt:// URI imports. Other than that we're just passing a string through exec.

The cause of this seems to be that we're passing in a globals and locals object that already contain symbols. If I run the code with empty globals & locals it works fine.

ipdb> print f_t
#!pyobjects
import random
import string
password = ''.join(random.SystemRandom().choice(
        string.ascii_letters + string.digits) for _ in range(20))

ipdb> _g = _l = {}
ipdb> six.exec_(f_t, _g, _l)
ipdb> six.exec_(f_t, _globals, _locals)
*** NameError: global name 'random' is not defined

I'll keep digging.

[borgstrom]

I've opened PR #21867 that fixes this issue. We can close this issue once the PR is merged.

Thanks for the bug report @grischa!

[borgstrom]

This is very interesting. While the fix was to remove the specific locals object I still didn't fully understand what was triggering the behavior, so I've been playing around with this.

password = ''.join(random.SystemRandom().choice(
        string.ascii_letters + string.digits) for _ in range(20))

The above code has a subtle "problem". The argument to join should be an iterable, but the list comprehension is missing the explicit list or tuple syntax to turn it into an iterable. If I add the syntax to turn the comprehension into a list then the code works under exec:

eborgstr@eborgstr-mn1 ~/Projects/salt — u:7 j:1 git:issue-21796-2015.2 (14:03:41 03.21)
#561 ❯❯❯ cat exec1.py 
#!/usr/bin/env python

code1 = '''import random, string
password = ''.join(random.SystemRandom().choice(
    string.ascii_letters + string.digits) for _ in range(20))'''

_g = {'something': lambda x: x}
_l = {}
exec(code1, _g, _l)

eborgstr@eborgstr-mn1 ~/Projects/salt — u:7 j:1 git:issue-21796-2015.2 (14:03:49 03.21)
#562 ❯❯❯ python exec1.py 
Traceback (most recent call last):
  File "exec1.py", line 9, in <module>
    exec(code1, _g, _l)
  File "<string>", line 3, in <module>
  File "<string>", line 3, in <genexpr>
NameError: global name 'random' is not defined

eborgstr@eborgstr-mn1 ~/Projects/salt — u:7 j:1 git:issue-21796-2015.2 (14:03:56 03.21)
#563 ❯❯❯ cat exec2.py 
#!/usr/bin/env python

code1 = '''import random, string
password = ''.join([random.SystemRandom().choice(
    string.ascii_letters + string.digits) for _ in range(20)])'''

_g = {'something': lambda x: x}
_l = {}
exec(code1, _g, _l)

eborgstr@eborgstr-mn1 ~/Projects/salt — u:7 j:1 git:issue-21796-2015.2 (14:04:02 03.21)
#564 ❯❯❯ python exec2.py 

However, the code succeeds without the list comprehension syntax when invoked directly using the python binary:

eborgstr@eborgstr-mn1 ~/Projects/salt — u:7 j:1 git:issue-21796-2015.2 (14:06:11 03.21)
#565 ❯❯❯ cat exec3.py 
#!/usr/bin/env python
import random, string
password = ''.join(random.SystemRandom().choice(
    string.ascii_letters + string.digits) for _ in range(20))

eborgstr@eborgstr-mn1 ~/Projects/salt — u:7 j:1 git:issue-21796-2015.2 (14:06:14 03.21)
#566 ❯❯❯ python exec3.py 

eborgstr@eborgstr-mn1 ~/Projects/salt — u:7 j:1 git:issue-21796-2015.2 (14:06:17 03.21)
#567 ❯❯❯ 

I still do not understand what specifically is happening inside the scope of the exec environment that is causing the NameError, but suffice to say it's something deep within Python's internals for how list comprehensions are handled and this wouldn't have affected the majority of Pyobjects users (when the list comprehension was separated and given the proper syntax everything worked as expected). Still it was good to better understand how exec treats globals and locals and fix an edge case.

[grischa]

Thanks for fixing this @borgstrom! And also for probing a bit further, this is quite interesting behaviour.
Indeed, I was puzzled, too, that the join worked on the un-bracketed list comprehension.
I didn't write it myself, I was lazy and used code from Stackoverflow to create a random string and it just worked. I figured join must just use its args list if there is more than one argument, but apparently the situation isn't quite as clear.