I'm reopening this one and removing my previous comment - I think we do need a way to make variables if undefined or False or string False (no, 'false', etc) be removed from the argument list.

I do need to make sure it works with non complex args.

I'm thinking we may also just call this "omit". On me to test.

Thanks!

default(null None)?​

Okay, just implemented the special variable omit, both for complex_args and module_args. In my use case, that would be

- name: generic-users | Make sure all groups are present
  group >
    name: "{{ item.name }}"
    system: "{{ item.system|default('no') }}"
    gid: "{{ item.gid|default(omit) }}"
    state: present
  with_items: genericusers_groups

I am thinking some possible threats to this omit variable. One possible attack would be if attacker can manage to obtain the omit string, he can actually omit arguments by injecting the string to the target variable. However, my bet is the chance attacker can get the omit place holder is pretty rare. And even so, every time ansible runs, it generates a different omit place holder, if we save the token accidentally in a file or what, attacker can only use it if he can grab that token and inject that variable into the same running ansible environment. If he can manage to do that, then he can just probably control the process directly. So I would say it's impossible or impractical to do so.

If we want a even higher level of security, we can regenerate the place holder token every time before we run a module. But since os.urandom needs to collect entropy from system, it's actually kinda expensive to generate a token (not really a big deal in most cases, but as far as I know if there is no enough entropy, seems os.urandom will be blocked). Although we can figure a cheap but secure way to generate the unpredictable token, I think that would be an overkill.

Personally, from security standing point, I would say current approach is good enough. But still, it's good to be reviewed by peers. Please let me know if you guys have any concern about current approach.

Hi @victorlin,

Thanks for this - I like the "omit" variable a lot more than the filter and it works well with defaults or also if conditionals, so this is a win.

On reviewing the above, I think there are two things we should do.

(A) generate the omit token pre-fork in the constructor to Runner and use os.urandom() to avoid the blocking. This will be a very low-cost call, but it also assures the token changes each time. I don't think that's especially important as the token won't be made available to remote nodes anyway so there's no good way to guess it. If someone is using a host variable in a playbook, they could use incorrect values, and this only adds another "wrong" value choice. For instance if you say "ask foo for the answer to question bar and use it as a boolean value as input to module baz", giving the wrong answer is the same as leaving the argument off in terms of severity.

(B) It appears the serialize_args function added duplicates some functionality in the rest of ansible, and we've been (especially as of late) very careful with parsing before.

(C) We'd probably want a unit test if possible for the function that handled omitting arguments, showing this used in both the key=value and "complex" (hash) forms.

Depending on your availability we may take a crack at this one in the next few days as we'd like this in 1.7 -- so just advising that this may result in some duplicated efforts should we hit that time we want to attack this, which may be very soon.

In all, I'm very excited by this solution to the problem. Thanks very much for working on it!

James points out that os.random won't block, so that's not a concern.

/dev/random raw, yes :)

There's also some question we may wish to hold this for the 1.8 development branch, due to the idea that we want to keep things as hashes as long as possible there, which may also make this easier. If we can implement it easily in 1.7 we will.

I am not pretty familiar with many implementation details, or the plan for future (not sure what hashes mean), in the mean time, I also have no much time to keep working on further improvements. So I will stop here.

Merged

Hi!

This has been merged in, and will also be included in the next major release. Since there were several changes to some of the underpinnings, GitHub isn't detecting the merge properly so I'm closing this manually.

If you or anyone else has any further questions, please let us know by stopping by one of the two mailing lists, as appropriate:

• https://groups.google.com/forum/#!forum/ansible-project - for user questions, tips, and tricks
• https://groups.google.com/forum/#!forum/ansible-devel - for strategy, future planning, and questions about writing code

Because this project is very active, we're unlikely to see comments made on closed tickets, but the mailing list is a great way to ask questions, or post if you don't think this particular issue is resolved.

Thank you!