From 377bc313110192657c376d90e9dde5c3117c0ec8 Mon Sep 17 00:00:00 2001
From: willthames <will@thames.id.au>
Date: Mon, 8 Apr 2013 17:13:23 +1000
Subject: [PATCH] Prevent premature substitution of variables into tasks

As documented in #2623, early variable substitution causes when_
tests to fail and possibly other side effects.

I can see the reason for this early substitution, likely introduced
in 1dfe60a6, to allow many playbook parameters to be templated.
This is a valid goal, but the recursive nature of the utils.template
function means that it goes too far.

At this point removing tasks from the list of parameters to be
substituted seems sufficient to make my tests pass. It may be the
case that other parameters should be excluded, but I suspect not.

Adding a test case. I would prefer to analyse not just the aggregate
statistics but also whether the results are as expected - I can't
see an easy way to do that with the available callbacks at present.
---
 lib/ansible/playbook/play.py |  6 +++++-
 test/TestPlayBook.py         | 28 ++++++++++++++++++++++++++++
 test/playbook-when.yml       | 27 +++++++++++++++++++++++++++
 3 files changed, 60 insertions(+), 1 deletion(-)
 create mode 100644 test/playbook-when.yml

diff --git a/lib/ansible/playbook/play.py b/lib/ansible/playbook/play.py
index f815a016ee4..eb0cc958abe 100644
--- a/lib/ansible/playbook/play.py
+++ b/lib/ansible/playbook/play.py
@@ -65,7 +65,11 @@ class Play(object):
 
         self._update_vars_files_for_host(None)
 
-        self._ds = ds         = template.template(basedir, ds, self.vars)
+        for key in ds:
+            if key != 'tasks':
+               ds[key] = template.template(basedir, ds[key], self.vars) 
+
+        self._ds = ds         
 
         hosts = ds.get('hosts')
         if hosts is None:
diff --git a/test/TestPlayBook.py b/test/TestPlayBook.py
index c85f62262f4..0f611349927 100644
--- a/test/TestPlayBook.py
+++ b/test/TestPlayBook.py
@@ -257,3 +257,31 @@ class TestPlaybook(unittest.TestCase):
        play = ansible.playbook.Play(playbook, playbook.playbook[0], os.getcwd())
        assert play.hosts == ';'.join(('host1', 'host2', 'host3'))
 
+   def test_playbook_when(self):
+       test_callbacks = TestCallbacks()
+       playbook = ansible.playbook.PlayBook(
+           playbook=os.path.join(self.test_dir, 'playbook-when.yml'),
+           host_list='test/ansible_hosts',
+           extra_vars={ 'external' : 'xyz', 'identity': 'identity' },
+           stats=ans_callbacks.AggregateStats(),
+           callbacks=test_callbacks,
+           runner_callbacks=test_callbacks
+       )
+       actual = playbook.run()
+
+       # if different, this will output to screen
+       print "**ACTUAL**"
+       print utils.jsonify(actual, format=True)
+       expected =  {
+           "localhost": {
+               "changed": 0,
+               "failures": 0,
+               "ok": 3,
+               "skipped": 3,
+               "unreachable": 0
+           }
+       }
+       print "**EXPECTED**"
+       print utils.jsonify(expected, format=True)
+
+       assert utils.jsonify(expected, format=True) == utils.jsonify(actual,format=True)
diff --git a/test/playbook-when.yml b/test/playbook-when.yml
new file mode 100644
index 00000000000..1884cf76f4e
--- /dev/null
+++ b/test/playbook-when.yml
@@ -0,0 +1,27 @@
+---
+- hosts: all
+  connection: local
+  gather_facts: False
+
+  vars: 
+    internal: "print me"
+
+  tasks: 
+  - action: debug msg="skip me"
+    when_unset: $internal
+
+  - action: debug msg="$internal"
+    when_set: $internal
+
+  - action: debug msg="skip me"
+    when_unset: $external
+
+  - action: debug msg="$external"
+    when_set: $external
+
+  - action: debug msg="run me"
+    when_unset: $this_var_does_not_exist
+
+  - action: debug msg="skip me"
+    when_set: $this_var_does_not_exist
+