Review Request 127: Add utility routine for passing stdin into subprocesses

[Stephen Gallagher]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-fedoraserver.rhcloud.com/r/127/
-----------------------------------------------------------

Review request for RoleKit Mailing List, Miloslav Trmac, Stephen Gallagher, and Thomas Woerner.

Repository: rolekit

Description
-------

Add utility routine for passing stdin into subprocesses

Diffs
-----

src/rolekit/util.py PRE-CREATION 
  src/rolekit/async.py 0f9ddaac1beb27cebdf41ca0383a62a807c4fcb6

Diff: http://reviewboard-fedoraserver.rhcloud.com/r/127/diff/

Testing
-------

Thanks,

Stephen Gallagher

[Stephen Gallagher]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-fedoraserver.rhcloud.com/r/127/#review363
-----------------------------------------------------------

src/rolekit/util.py
http://reviewboard-fedoraserver.rhcloud.com/r/127/#comment191

Per documentation, "except that data is spooled in memory until the file size exceeds max_size, _or until the file’s fileno() method is called_, at which point the contents are written to disk and operation proceeds as with TemporaryFile()".

subprocess does call fileno(), so the data will end up on disk.

I suppose it would be ideally clean to use a pipe instead and write data to it similarly to the way we collect output, but that’s also much more work…

*shrug* This works as well.

- Miloslav Trmac

On Led. 20, 2015, 6:58 odp., Stephen Gallagher wrote:
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard-fedoraserver.rhcloud.com/r/127/
>
> (Updated Led. 20, 2015, 6:58 odp.)
>
> Review request for RoleKit Mailing List, Miloslav Trmac, Stephen Gallagher, and Thomas Woerner.
>
> Repository: rolekit
> Description
>
> Add utility routine for passing stdin into subprocesses
> Diffs
>
> src/rolekit/util.py PRE-CREATION 
>   src/rolekit/async.py 0f9ddaac1beb27cebdf41ca0383a62a807c4fcb6
>
> Diff: http://reviewboard-fedoraserver.rhcloud.com/r/127/diff/
> Testing
>
> Thanks,
>
> Stephen Gallagher

[Stephen Gallagher]

> On Jan. 20, 2015, 7:41 p.m., Miloslav Trmac wrote:
> > src/rolekit/util.py, line 44
> > http://reviewboard-fedoraserver.rhcloud.com/r/127/diff/1/?file=521#file521line44
> > Per documentation, "except that data is spooled in memory until the file size exceeds max_size, _or until the file’s fileno() method is called_, at which point the contents are written to disk and operation proceeds as with TemporaryFile()".
> >
> > subprocess does call fileno(), so the data will end up on disk.
> >
> > I suppose it would be ideally clean to use a pipe instead and write data to it similarly to the way we collect output, but that’s also much more work…
> >
> > *shrug* This works as well.

Actually, I may just scrap this feature anyway. I was creating it for use with postgresql's 'createuser' command, but it turns out that just passing the password to the prompt via stdin doesn't work anyway. I'll probably have to just create the user without a password and then use an SQL command to set it securely.

- Stephen

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-fedoraserver.rhcloud.com/r/127/#review363
-----------------------------------------------------------

On Jan. 20, 2015, 5:58 p.m., Stephen Gallagher wrote:
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard-fedoraserver.rhcloud.com/r/127/
>
> (Updated Jan. 20, 2015, 5:58 p.m.)
>
> Review request for RoleKit Mailing List, Miloslav Trmac, Stephen Gallagher, and Thomas Woerner.
>
> Repository: rolekit
> Description
>
> Add utility routine for passing stdin into subprocesses
> Diffs
>
> src/rolekit/util.py PRE-CREATION 
>   src/rolekit/async.py 0f9ddaac1beb27cebdf41ca0383a62a807c4fcb6
>
> Diff: http://reviewboard-fedoraserver.rhcloud.com/r/127/diff/
> Testing
>
> Thanks,
>
> Stephen Gallagher

[Stephen Gallagher]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-fedoraserver.rhcloud.com/r/127/
-----------------------------------------------------------

(Updated Jan. 21, 2015, 3:49 p.m.)

Status
------

This change has been discarded.

Review request for RoleKit Mailing List, Miloslav Trmac, Stephen Gallagher, and Thomas Woerner.

Repository: rolekit

Description
-------

Add utility routine for passing stdin into subprocesses

Diffs
-----

src/rolekit/util.py PRE-CREATION 
  src/rolekit/async.py 0f9ddaac1beb27cebdf41ca0383a62a807c4fcb6

Diff: http://reviewboard-fedoraserver.rhcloud.com/r/127/diff/

Testing
-------

Thanks,

Stephen Gallagher

[Stephen Gallagher]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-fedoraserver.rhcloud.com/r/127/
-----------------------------------------------------------

(Updated Jan. 22, 2015, 10 p.m.)

Review request for RoleKit Mailing List, Miloslav Trmac, Stephen Gallagher, and Thomas Woerner.

Repository: rolekit

Description
-------

Add utility routine for passing stdin into subprocesses

Diffs (updated)
-----

src/rolekit/async.py 0f9ddaac1beb27cebdf41ca0383a62a807c4fcb6 
  src/rolekit/util.py f3825d748475ea837e84004129459083269d9686

Diff: http://reviewboard-fedoraserver.rhcloud.com/r/127/diff/

Testing
-------

Thanks,

Stephen Gallagher

[Stephen Gallagher]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-fedoraserver.rhcloud.com/r/127/#review376
-----------------------------------------------------------

Re-opening this request as it turns out I will be needing this capability after all in order to support the password-set child process for the database server. I've rebased it atop the UID/GID impersonation patch.

Mirek, did your last comment amount to a ShipIt? If so, please hit the button.

- Stephen Gallagher

On Jan. 22, 2015, 10 p.m., Stephen Gallagher wrote:
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard-fedoraserver.rhcloud.com/r/127/
>
> (Updated Jan. 22, 2015, 10 p.m.)
>
> Review request for RoleKit Mailing List, Miloslav Trmac, Stephen Gallagher, and Thomas Woerner.
>
> Repository: rolekit
> Description
>
> Add utility routine for passing stdin into subprocesses
> Diffs
>
> src/rolekit/async.py 0f9ddaac1beb27cebdf41ca0383a62a807c4fcb6 
>   src/rolekit/util.py f3825d748475ea837e84004129459083269d9686
>
> Diff: http://reviewboard-fedoraserver.rhcloud.com/r/127/diff/
> Testing
>
> Thanks,
>
> Stephen Gallagher

[Stephen Gallagher]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-fedoraserver.rhcloud.com/r/127/#review378
-----------------------------------------------------------

I tried to use the code with a small bash script that is simply writing the strings read from stdin to a log file. The call to the script with writing several lines to it does not return before I press Ctrl-D in the term calling the python scriptlet. Additionally there is no output in the log file.

Here is the test code:

import os
import pwd
import subprocess
from rolekit.functions import writefile
from tempfile import SpooledTemporaryFile

def temporary_stdin(io_input):
    """
    Create a temporary in-memory file object to store input for
    subprocess_future() calls. This is useful for passing passwords
    without specifying them on the command line (which would cause
    them to appear in the process table)
    """
    f = SpooledTemporaryFile()
    f.write(io_input)
    f.seek(0)

writefile("/tmp/readstdin", '#!/bin/bash\nwhile read line\ndo\necho "$line" >> /tmp/readstdin.log\ndone\n')

stdinfile = temporary_stdin("line 1\nline 2\nline 3\n")
preexec_fn = None
args = [ "/bin/bash", "/tmp/readstdin" ]

try:
    process = subprocess.Popen(args, close_fds=True,
                               stdin=stdinfile,
                               stdout=subprocess.PIPE,
                               stderr=subprocess.PIPE,
                               preexec_fn=preexec_fn)
except OSError, e:
    if e.errno is errno.EPERM:
        # Could not change users prior to executing the subprocess
        print("ERROR: Insufficient privileges to impersonate UID/GID %s/%s" %
              (uid, gid))
    raise

print process.stdout.read()

- Thomas Woerner

On Jan. 22, 2015, 10 p.m., Stephen Gallagher wrote:
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard-fedoraserver.rhcloud.com/r/127/
>
> (Updated Jan. 22, 2015, 10 p.m.)
>
> Review request for RoleKit Mailing List, Miloslav Trmac, Stephen Gallagher, and Thomas Woerner.
>
> Repository: rolekit
> Description
>
> Add utility routine for passing stdin into subprocesses
> Diffs
>
> src/rolekit/async.py 0f9ddaac1beb27cebdf41ca0383a62a807c4fcb6 
>   src/rolekit/util.py f3825d748475ea837e84004129459083269d9686
>
> Diff: http://reviewboard-fedoraserver.rhcloud.com/r/127/diff/
> Testing
>
> Thanks,
>
> Stephen Gallagher

[Stephen Gallagher]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-fedoraserver.rhcloud.com/r/127/
-----------------------------------------------------------

(Updated Feb. 17, 2015, 6:57 p.m.)

Review request for RoleKit Mailing List, Miloslav Trmac, Stephen Gallagher, and Thomas Woerner.

Changes
-------

Completely rewrote this patch to use subprocess.PIPE instead. The creation of the temporary file was overthinking the situation (it was attempting to avoid the small risk of deadlock when reading and writing stdin and stdout directly, however, since we're never doing those things at the same time, it's unnecessary).

This version of the patch also adds a test for the stdin functionality by calling 'tee' (which simply shunts stdin to stdout).

Repository: rolekit

Description
-------

Add utility routine for passing stdin into subprocesses

Diffs (updated)
-----

src/rolekit/async.py 0f9ddaac1beb27cebdf41ca0383a62a807c4fcb6 
  tests/async_test.py 12a4c12c8dbf427a6382c9bc82139bf246b08c37

Diff: http://reviewboard-fedoraserver.rhcloud.com/r/127/diff/

Testing
-------

Thanks,

Stephen Gallagher

[Stephen Gallagher]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-fedoraserver.rhcloud.com/r/127/#review379
-----------------------------------------------------------

src/rolekit/async.py
http://reviewboard-fedoraserver.rhcloud.com/r/127/#comment209

I still think this function is entirely redundant and simply saying
    preexec_fn = set_ids
    (not set_ids()) would work, but I can’t provide a tested alternative patch, so… meh.

src/rolekit/async.py
http://reviewboard-fedoraserver.rhcloud.com/r/127/#comment210

The parentheses are not idiomatic.  Not a blocker obviously ?

tests/async_test.py
http://reviewboard-fedoraserver.rhcloud.com/r/127/#comment211

This could be generalized to replace __run_bash_commands_async(with a stdin=None optional parameter), but this works just as well.

- Miloslav Trmac

On Úno. 17, 2015, 7:57 odp., Stephen Gallagher wrote:
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard-fedoraserver.rhcloud.com/r/127/
>
> (Updated Úno. 17, 2015, 7:57 odp.)
>
> Review request for RoleKit Mailing List, Miloslav Trmac, Stephen Gallagher, and Thomas Woerner.
>
> Repository: rolekit
> Description
>
> Add utility routine for passing stdin into subprocesses
> Diffs
>
> src/rolekit/async.py 0f9ddaac1beb27cebdf41ca0383a62a807c4fcb6 
>   tests/async_test.py 12a4c12c8dbf427a6382c9bc82139bf246b08c37
>
> Diff: http://reviewboard-fedoraserver.rhcloud.com/r/127/diff/
> Testing
>
> Thanks,
>
> Stephen Gallagher

[Stephen Gallagher]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-fedoraserver.rhcloud.com/r/127/#review380
-----------------------------------------------------------

Ship it!

Ship It!

- Miloslav Trmac

On Úno. 17, 2015, 7:57 odp., Stephen Gallagher wrote:
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard-fedoraserver.rhcloud.com/r/127/
>
> (Updated Úno. 17, 2015, 7:57 odp.)
>
> Review request for RoleKit Mailing List, Miloslav Trmac, Stephen Gallagher, and Thomas Woerner.
>
> Repository: rolekit
> Description
>
> Add utility routine for passing stdin into subprocesses
> Diffs
>
> src/rolekit/async.py 0f9ddaac1beb27cebdf41ca0383a62a807c4fcb6 
>   tests/async_test.py 12a4c12c8dbf427a6382c9bc82139bf246b08c37
>
> Diff: http://reviewboard-fedoraserver.rhcloud.com/r/127/diff/
> Testing
>
> Thanks,
>
> Stephen Gallagher

[Stephen Gallagher]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-fedoraserver.rhcloud.com/r/127/
-----------------------------------------------------------

(Updated Feb. 17, 2015, 7:33 p.m.)

Review request for RoleKit Mailing List, Miloslav Trmac, Stephen Gallagher, and Thomas Woerner.

Changes
-------

The previous version of the diff had some cruft mixed in from an earlier patch. This diff should be identical, but apply correctly using 'rbt patch'.

Repository: rolekit

Description
-------

Add utility routine for passing stdin into subprocesses

Diffs (updated)
-----

src/rolekit/async.py bc95d32ecebc66a602c1626de3fb72b75cadd765 
  tests/async_test.py 12a4c12c8dbf427a6382c9bc82139bf246b08c37

Diff: http://reviewboard-fedoraserver.rhcloud.com/r/127/diff/

Testing
-------

Thanks,

Stephen Gallagher

[Stephen Gallagher]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-fedoraserver.rhcloud.com/r/127/
-----------------------------------------------------------

(Updated Feb. 17, 2015, 7:35 p.m.)

Status
------

This change has been marked as submitted.

Review request for RoleKit Mailing List, Miloslav Trmac, Stephen Gallagher, and Thomas Woerner.

Repository: rolekit

Description
-------

Add utility routine for passing stdin into subprocesses

Diffs
-----

src/rolekit/async.py bc95d32ecebc66a602c1626de3fb72b75cadd765 
  tests/async_test.py 12a4c12c8dbf427a6382c9bc82139bf246b08c37

Diff: http://reviewboard-fedoraserver.rhcloud.com/r/127/diff/

Testing
-------

Thanks,

Stephen Gallagher