passing options

Archives Ansible Project
1

In looking at how ssh passes in options and how ansible execs them I
think we need to consider something different for passing options to
modules.

I think we should have the runner create a file for the module options
xfer the file to the minion(s)
and then pass that file as the only argument to the module.

I think this is important b/c otherwise we are going to struggle
passing in args like:

state=installed pkg="yum >= 3.2"

or if we enabled a command module that allowed the shell to run then we
can do partial shell redirects in there, too.

thoughts?

-sv

2

Hmm, let’s see if we can probably avoid this.

Seeing RPM can’t (usually) downgrade, it seems sufficient to say “pkg=yum-3.2” for the syntax in that case where I want a specific version. I usually don’t want X or higher in my infra if I am spec’ing a version, I want everything to be homogenous.

For more involved things (should that be needed), the following would still work fine with naive parsing, provided “>” was escaped:

pkg=yum>3.2

But in that case, it seems that I’d most likely just want the latest version and I’m managing the packages by managing my yum repo and yum config, right? AFAIK, what Puppet supports is pkg=Y and pkg=latest, but there’s not a X>=Y. Folks mostly just snapshot their repos.

Another reason to avoid it is if you are writing modules in bash, it’s easy to print JSON via echo, but it’s
hard to parse JSON. Supporting bash modules is a major design goal.

Anyway, we have to keep the ansible command line and playbook invocations the same as they are now and I need to know how any changes will complicate things for bash-written modules that need to parse arguments.

I want to make sure all the solutions are covered, but I think we might be ok as we are now.

Does that sort of make sense?

–Michael

3

Hmm, let's see if we can probably avoid this.

Seeing RPM can't (usually) downgrade, it seems sufficient to say
"pkg=yum-3.2" for the syntax in that case where I want a specific
version. I usually don't want X or higher in my infra if I am
spec'ing a version, I want everything to be homogenous.

For more involved things (should that be needed), the following would
still work fine with naive parsing, provided ">" was escaped:

pkg=yum>3.2

It's not just about the yum/rpm versions - it is also about shell
commands and redirects.

if I want to grep somefile | cut | wc -l

and get that back on all hosts - I cannot do that if we pass them in as
arguments on the exec'd commandline to ssh. b/c that will kill the
arguments immediately.

But in that case, it seems that I'd most likely just want the latest
version and I'm managing the packages by managing my yum repo and yum
config, right? AFAIK, what Puppet supports is pkg=Y and pkg=latest,
but there's not a X>=Y. Folks mostly just snapshot their repos.

Another reason to avoid it is if you are writing modules in bash,
it's easy to print JSON via echo, but it's hard to parse JSON.
Supporting bash modules is a major design goal.

Why would you need to PARSE json?

I'm not suggesting parsing it from the module.

I'm saying this

if I run:

ansible -m command -a "ls -la /path/to/someplace | wc -l"

that on the remote side we do this

sftp command_module
sftp argfile containing "ls -la /path/to/someplace | wc -l"
exec command_module argfile

the command module reads the argfile for it's arguments.

Then we do not have to worry about double escaping or having shell
escapes captured.

It also means if any argument someone would want to any module happens
to include a > or a | then it will just work - no extra effort or
contortions needed.

-sv

4

BTW, do you think it would be possible for the command module to analyze the input string and decide
when to use shell=true to subprocess?

It could probably also auto-escape shell-looking things on the runner side if necessary, if that is what it took
to feed them to command. We could choose to not do this for other module names.

I do want to support shell-like things in the command module but other modules probably don’t have to do it.

–Michael

5

supporting shell MIGHT be possible but it changes how you hand
arguments off and it changes what the primary argument to the
subprocess call looks like (specifically a string vs a list).

But here's the most important bit to focus on:
As it is ansible, right now, will not support any module with an
argument that has a >, & or a | anywhere in it. And because of how
things are being exec'd they would need to be double-escaped.

once to get past the ssh exec and again to get past the subprocess exec
(in the case of a command or shell module)
-sv

6

Forgot to reply all.

Transferring a non-JSON file seems totally reasonable.

We’ll have to change all of the modules and test them though. Easy while the count is small.

Send me a patch?

–Michael

7

Yeah, I think we need to do this for future proofing, plus I like this solution better than overcomplicating
escape/unescape for just the command module.

8

okay - I sent a pull request for a patch which handles this. I think I
made it handle all the cases nicely.

Also - I tested a 'shell' module which sets shell=True to the
subprocess.Popen() call.

When I do that I can successfully run shell commands like:

ansible '*' -k -m shell -a "echo yay && echo boo"

and

ansible '*' -k -m shell -a "ls -la /etc/ | wc -l"

Now - those ONLY work with the shell=True for... obvious reasons.

But I'm also able to do things like:

ansible '*' -k -m yum -a "state=installed pkg=\'yum >= 3.3\'"

Which is sub-optimal - but at least it is possible.

I think I may rewrite the argument parsing for the yum module so it
does fewer dodgy things in the presence of odd package specifications.

-sv

9

Ok, thanks, I commented on the pull request. It will be very useful to have shell options.

https://github.com/mpdehaan/ansible/pull/92

One thought I have is that rather than the modules having dual behavior (inline+args file) it may be better if
we supply a scripts/test_module script that does the magic, that way we’re getting more full coverage
when testing the module directly. This also keeps the module source shorter, which is an important point of
showing how easy they are to write. We would not ship the test_module script but it could be used in development.

Another thought – Rather than duplicating the module source in ‘shell’, wouldn’t it be better to maybe teach runner
to know that “shell” meant to deploy the command module and that the command module gets
executed in some slightly ‘special’ way such that it knows to use the shell behavior? That
way there’s one less module file to maintain. I didn’t see the commit for ‘shell’ but suspect that module
is only minimally different. That would be good. There is already some larger magic around ‘copy’ and ‘template’
in runner, this feels like smaller magic. We could just add a special option to the arguments that it would know
how to read and then strip off before running the command.

I can look at getting setup/async & async_wrapper operational with this tonight, as well
as doing some more module testing.

Generally “make tests” is pretty good for exercising all of these and should be run
when making module changes. I need to add some easy (i.e. non-side effect) tests for
the yum module if the distribution is Red Hat (aka if redhat-release exists or whatever). Probably
will just run list-available to make sure syntax of the module is still ok and some basics. It is good to make sure
new modules generally update the test suite.

Thinking out loud, Is there a decent way to write the args file remotely without using the local tempfile?
I would think you could use the ssh libraries and open up cat with a pipe. Might be nice, as so far we
don’t create temp files on the local box.

We could make a method in connection called put_file_string or equivalent? I may look
into this as well.

In any event, I’ll merge this to my private branch tonight and explore some things.

This is a very good feature.

(OT – I’m overdue for some more extensive refactoring in Runner to remove some very procedural artifacts
of me building it the first time. Should make things easier/nicer to maintain)

–Michael

10

All modules updated to work this way.

End users should not detect any new behavior. If you do, let me know.

–Michael