Overhauls to v2 code

* using inspect module instead of iteritems(self.__class__.__dict__, due
  to the fact that the later does not include attributes from parent
  classes
* added tags/when attributes to Base() class for use by all subclasses
* removed value/callable code from Attribute, as they are not used
* started moving some limited code from utils to new places in v2 tree
  (vault, yaml-parsing related defs)
* re-added ability of Block.load() to create implicit blocks from tasks
* started overhaul of Role class and role-related code

v2/ansible/parsing/__init__.py

@@ -19,19 +19,205 @@
 from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
+import json
+
+from yaml import YAMLError
+
 from ansible.errors import AnsibleError, AnsibleInternalError
+from ansible.parsing.vault import VaultLib
+from ansible.parsing.yaml import safe_load
+
+
+def process_common_errors(msg, probline, column):
+    replaced = probline.replace(" ","")
+
+    if ":{{" in replaced and "}}" in replaced:
+        msg = msg + """
+This one looks easy to fix.  YAML thought it was looking for the start of a
+hash/dictionary and was confused to see a second "{".  Most likely this was
+meant to be an ansible template evaluation instead, so we have to give the
+parser a small hint that we wanted a string instead. The solution here is to
+just quote the entire value.
+
+For instance, if the original line was:
+
+    app_path: {{ base_path }}/foo
+
+It should be written as:
+
+    app_path: "{{ base_path }}/foo"
+"""
+        return msg
+
+    elif len(probline) and len(probline) > 1 and len(probline) > column and probline[column] == ":" and probline.count(':') > 1:
+        msg = msg + """
+This one looks easy to fix.  There seems to be an extra unquoted colon in the line
+and this is confusing the parser. It was only expecting to find one free
+colon. The solution is just add some quotes around the colon, or quote the
+entire line after the first colon.
+
+For instance, if the original line was:
+
+    copy: src=file.txt dest=/path/filename:with_colon.txt
+
+It can be written as:
+
+    copy: src=file.txt dest='/path/filename:with_colon.txt'
+
+Or:
+
+    copy: 'src=file.txt dest=/path/filename:with_colon.txt'
+
+
+"""
+        return msg
+    else:
+        parts = probline.split(":")
+        if len(parts) > 1:
+            middle = parts[1].strip()
+            match = False
+            unbalanced = False
+            if middle.startswith("'") and not middle.endswith("'"):
+                match = True
+            elif middle.startswith('"') and not middle.endswith('"'):
+                match = True
+            if len(middle) > 0 and middle[0] in [ '"', "'" ] and middle[-1] in [ '"', "'" ] and probline.count("'") > 2 or probline.count('"') > 2:
+                unbalanced = True
+            if match:
+                msg = msg + """
+This one looks easy to fix.  It seems that there is a value started
+with a quote, and the YAML parser is expecting to see the line ended
+with the same kind of quote.  For instance:
+
+    when: "ok" in result.stdout
+
+Could be written as:
+
+   when: '"ok" in result.stdout'
+
+or equivalently:
+
+   when: "'ok' in result.stdout"
 
-def load(self, data):
+"""
+                return msg
 
-    if instanceof(data, file):
+            if unbalanced:
+                msg = msg + """
+We could be wrong, but this one looks like it might be an issue with
+unbalanced quotes.  If starting a value with a quote, make sure the
+line ends with the same set of quotes.  For instance this arbitrary
+example:
+
+    foo: "bad" "wolf"
+
+Could be written as:
+
+    foo: '"bad" "wolf"'
+
+"""
+                return msg
+
+    return msg
+
+def process_yaml_error(exc, data, path=None, show_content=True):
+    if hasattr(exc, 'problem_mark'):
+        mark = exc.problem_mark
+        if show_content:
+            if mark.line -1 >= 0:
+                before_probline = data.split("\n")[mark.line-1]
+            else:
+                before_probline = ''
+            probline = data.split("\n")[mark.line]
+            arrow = " " * mark.column + "^"
+            msg = """Syntax Error while loading YAML script, %s
+Note: The error may actually appear before this position: line %s, column %s
+
+%s
+%s
+%s""" % (path, mark.line + 1, mark.column + 1, before_probline, probline, arrow)
+
+            unquoted_var = None
+            if '{{' in probline and '}}' in probline:
+                if '"{{' not in probline or "'{{" not in probline:
+                    unquoted_var = True
+
+            if not unquoted_var:
+                msg = process_common_errors(msg, probline, mark.column)
+            else:
+                msg = msg + """
+We could be wrong, but this one looks like it might be an issue with
+missing quotes.  Always quote template expression brackets when they
+start a value. For instance:
+
+    with_items:
+      - {{ foo }}
+
+Should be written as:
+
+    with_items:
+      - "{{ foo }}"
+
+"""
+        else:
+            # most likely displaying a file with sensitive content,
+            # so don't show any of the actual lines of yaml just the
+            # line number itself
+            msg = """Syntax error while loading YAML script, %s
+The error appears to have been on line %s, column %s, but may actually
+be before there depending on the exact syntax problem.
+""" % (path, mark.line + 1, mark.column + 1)
+
+    else:
+        # No problem markers means we have to throw a generic
+        # "stuff messed up" type message. Sry bud.
+        if path:
+            msg = "Could not parse YAML. Check over %s again." % path
+        else:
+            msg = "Could not parse YAML."
+    raise errors.AnsibleYAMLValidationFailed(msg)
+
+
+def load_data(data):
+
+    if isinstance(data, file):
         fd = open(f)
         data = fd.read()
         fd.close()
 
-    if instanceof(data, basestring):
+    if isinstance(data, basestring):
         try:
             return json.loads(data)
         except:
             return safe_load(data)
 
     raise AnsibleInternalError("expected file or string, got %s" % type(data))
+
+def load_data_from_file(path, vault_password=None):
+    '''
+    Convert a yaml file to a data structure. 
+    Was previously 'parse_yaml_from_file()'.
+    '''
+
+    data = None
+    show_content = True
+
+    try:
+        data = open(path).read()
+    except IOError:
+        raise errors.AnsibleError("file could not read: %s" % path)
+
+    vault = VaultLib(password=vault_password)
+    if vault.is_encrypted(data):
+        # if the file is encrypted and no password was specified,
+        # the decrypt call would throw an error, but we check first
+        # since the decrypt function doesn't know the file name
+        if vault_password is None:
+            raise errors.AnsibleError("A vault password must be specified to decrypt %s" % path)
+        data = vault.decrypt(data)
+        show_content = False
+
+    try:
+        return load_data(data)
+    except YAMLError, exc:
+        process_yaml_error(exc, data, path, show_content)

v2/ansible/playbook/attribute.py

@@ -25,11 +25,7 @@ class Attribute:
 
        self.isa = isa
        self.private = private
-       self.value = None
        self.default = default
 
-    def __call__(self):
-       return self.value
-
 class FieldAttribute(Attribute):
     pass

v2/ansible/playbook/base.py

@@ -19,24 +19,39 @@
 from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
+from inspect import getmembers
 from io import FileIO
 
 from six import iteritems, string_types
 
 from ansible.playbook.attribute import Attribute, FieldAttribute
-from ansible.parsing import load as ds_load
+from ansible.parsing import load_data
 
 class Base:
 
+    _tags = FieldAttribute(isa='list')
+    _when = FieldAttribute(isa='list')
+
     def __init__(self):
 
         # each class knows attributes set upon it, see Task.py for example
         self._attributes = dict()
 
-        for (name, value) in iteritems(self.__class__.__dict__):
-            aname = name[1:]
+        for (name, value) in self._get_base_attributes().iteritems():
+            self._attributes[name] = value.default
+
+    def _get_base_attributes(self):
+        '''
+        Returns the list of attributes for this class (or any subclass thereof).
+        If the attribute name starts with an underscore, it is removed
+        '''
+        base_attributes = dict()
+        for (name, value) in getmembers(self.__class__):
             if isinstance(value, Attribute):
-                self._attributes[aname] = value.default
+               if name.startswith('_'):
+                   name = name[1:]
+               base_attributes[name] = value
+        return base_attributes
 
     def munge(self, ds):
         ''' infrequently used method to do some pre-processing of legacy terms '''
@@ -49,7 +64,7 @@ class Base:
         assert ds is not None
 
         if isinstance(ds, string_types) or isinstance(ds, FileIO):
-            ds = ds_load(ds)
+            ds = load_data(ds)
 
         # we currently don't do anything with private attributes but may
         # later decide to filter them out of 'ds' here.
@@ -57,20 +72,15 @@ class Base:
         ds = self.munge(ds)
 
         # walk all attributes in the class
-        for (name, attribute) in iteritems(self.__class__.__dict__):
-            aname = name[1:]
-
-            # process Field attributes which get loaded from the YAML
-
-            if isinstance(attribute, FieldAttribute):
+        for (name, attribute) in self._get_base_attributes().iteritems():
 
-                # copy the value over unless a _load_field method is defined
-                if aname in ds:
-                    method = getattr(self, '_load_%s' % aname, None)
-                    if method:
-                        self._attributes[aname] = method(aname, ds[aname])
-                    else:
-                        self._attributes[aname] = ds[aname]
+            # copy the value over unless a _load_field method is defined
+            if name in ds:
+                method = getattr(self, '_load_%s' % name, None)
+                if method:
+                    self._attributes[name] = method(name, ds[name])
+                else:
+                    self._attributes[name] = ds[name]
 
         # return the constructed object
         self.validate()
@@ -81,20 +91,12 @@ class Base:
         ''' validation that is done at parse time, not load time '''
 
         # walk all fields in the object
-        for (name, attribute) in self.__dict__.iteritems():
-
-            # find any field attributes
-            if isinstance(attribute, FieldAttribute):
-
-                if not name.startswith("_"):
-                    raise AnsibleError("FieldAttribute %s must start with _" % name)
+        for (name, attribute) in self._get_base_attributes().iteritems():
 
-                aname = name[1:]
-
-                # run validator only if present
-                method = getattr(self, '_validate_%s' % (prefix, aname), None)
-                if method:
-                    method(self, attribute)
+            # run validator only if present
+            method = getattr(self, '_validate_%s' % name, None)
+            if method:
+                method(self, attribute)
 
     def post_validate(self, runner_context):
         '''

v2/ansible/playbook/block.py

@@ -25,11 +25,13 @@ from ansible.playbook.attribute import Attribute, FieldAttribute
 
 class Block(Base):
 
-    # TODO: FIXME: block/rescue/always should be enough
-    _begin     = FieldAttribute(isa='list')
+    _block     = FieldAttribute(isa='list')
     _rescue    = FieldAttribute(isa='list')
-    _end       = FieldAttribute(isa='list')
-    _otherwise = FieldAttribute(isa='list')
+    _always    = FieldAttribute(isa='list')
+
+    # for future consideration? this would be functionally
+    # similar to the 'else' clause for exceptions
+    #_otherwise = FieldAttribute(isa='list')
 
     def __init__(self, role=None):
         self.role = role
@@ -45,6 +47,20 @@ class Block(Base):
         b = Block(role=role)
         return b.load_data(data)
 
+    def munge(self, ds):
+        '''
+        If a simple task is given, an implicit block for that single task
+        is created, which goes in the main portion of the block
+        '''
+        is_block = False
+        for attr in ('block', 'rescue', 'always'):
+            if attr in ds:
+                is_block = True
+                break
+        if not is_block:
+            return dict(block=ds)
+        return ds
+
     def _load_list_of_tasks(self, ds):
         assert type(ds) == list
         task_list = []
@@ -53,15 +69,16 @@ class Block(Base):
             task_list.append(t)
         return task_list
 
-    def _load_begin(self, attr, ds):
+    def _load_block(self, attr, ds):
         return self._load_list_of_tasks(ds)
 
     def _load_rescue(self, attr, ds):
         return self._load_list_of_tasks(ds)
 
-    def _load_end(self, attr, ds):
+    def _load_always(self, attr, ds):
         return self._load_list_of_tasks(ds)
 
-    def _load_otherwise(self, attr, ds):
-        return self._load_list_of_tasks(ds)
+    # not currently used
+    #def _load_otherwise(self, attr, ds):
+    #    return self._load_list_of_tasks(ds)
 

v2/test/parsing/test_general.py

@@ -20,7 +20,7 @@ from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
 from .. compat import unittest
-from ansible.parsing import load
+from ansible.parsing import load_data
 from ansible.errors import AnsibleParserError
 
 import json
@@ -68,12 +68,12 @@ class TestGeneralParsing(unittest.TestCase):
             "jkl" : 5678
         }
         """
-        output = load(input)
+        output = load_data(input)
         self.assertEqual(output['asdf'], '1234')
         self.assertEqual(output['jkl'], 5678)
 
     def parse_json_from_file(self):
-        output = load(MockFile(dict(a=1,b=2,c=3)),'json')
+        output = load_data(MockFile(dict(a=1,b=2,c=3)),'json')
         self.assertEqual(ouput, dict(a=1,b=2,c=3))
 
     def parse_yaml_from_dict(self):
@@ -81,12 +81,12 @@ class TestGeneralParsing(unittest.TestCase):
         asdf: '1234'
         jkl: 5678
         """
-        output = load(input)
+        output = load_data(input)
         self.assertEqual(output['asdf'], '1234')
         self.assertEqual(output['jkl'], 5678)
 
     def parse_yaml_from_file(self):
-        output = load(MockFile(dict(a=1,b=2,c=3),'yaml'))
+        output = load_data(MockFile(dict(a=1,b=2,c=3),'yaml'))
         self.assertEqual(output, dict(a=1,b=2,c=3))
 
     def parse_fail(self):
@@ -95,10 +95,10 @@ class TestGeneralParsing(unittest.TestCase):
             ***
                NOT VALID
         """
-        self.assertRaises(load(input), AnsibleParserError)
+        self.assertRaises(load_data(input), AnsibleParserError)
 
     def parse_fail_from_file(self):
-        self.assertRaises(load(MockFile(None,'fail')), AnsibleParserError)
+        self.assertRaises(load_data(MockFile(None,'fail')), AnsibleParserError)
 
     def parse_fail_invalid_type(self):
         self.assertRaises(3000, AnsibleParsingError)

v2/test/playbook/test_block.py

@@ -49,31 +49,39 @@ class TestBlock(unittest.TestCase):
 
     def test_load_block_simple(self):
         ds = dict(
-           begin = [],
+           block = [],
            rescue = [],
-           end = [],
-           otherwise = [],
+           always = [],
+           #otherwise = [],
         )
         b = Block.load(ds)
-        self.assertEqual(b.begin, [])
+        self.assertEqual(b.block, [])
         self.assertEqual(b.rescue, [])
-        self.assertEqual(b.end, [])
-        self.assertEqual(b.otherwise, [])
+        self.assertEqual(b.always, [])
+        # not currently used
+        #self.assertEqual(b.otherwise, [])
 
     def test_load_block_with_tasks(self):
         ds = dict(
-           begin = [dict(action='begin')],
+           block = [dict(action='block')],
            rescue = [dict(action='rescue')],
-           end = [dict(action='end')],
-           otherwise = [dict(action='otherwise')],
+           always = [dict(action='always')],
+           #otherwise = [dict(action='otherwise')],
         )
         b = Block.load(ds)
-        self.assertEqual(len(b.begin), 1)
-        assert isinstance(b.begin[0], Task)
+        self.assertEqual(len(b.block), 1)
+        assert isinstance(b.block[0], Task)
         self.assertEqual(len(b.rescue), 1)
         assert isinstance(b.rescue[0], Task)
-        self.assertEqual(len(b.end), 1)
-        assert isinstance(b.end[0], Task)
-        self.assertEqual(len(b.otherwise), 1)
-        assert isinstance(b.otherwise[0], Task)
+        self.assertEqual(len(b.always), 1)
+        assert isinstance(b.always[0], Task)
+        # not currently used
+        #self.assertEqual(len(b.otherwise), 1)
+        #assert isinstance(b.otherwise[0], Task)
+
+    def test_load_implicit_block(self):
+        ds = [dict(action='foo')]
+        b = Block.load(ds)
+        self.assertEqual(len(b.block), 1)
+        assert isinstance(b.block[0], Task)
 

v2/test/playbook/test_role.py

+# (c) 2012-2014, Michael DeHaan <michael.dehaan@gmail.com>
+#
+# This file is part of Ansible
+#
+# Ansible is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# Ansible is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
+
+# Make coding more python3-ish
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+from ansible.playbook.block import Block
+from ansible.playbook.role import Role
+from ansible.playbook.task import Task
+from .. compat import unittest
+
+class TestRole(unittest.TestCase):
+
+    def setUp(self):
+        pass
+
+    def tearDown(self):
+        pass
+
+    def test_construct_empty_block(self):
+        r = Role()
+
+    def test_role__load_list_of_blocks(self):
+        task = dict(action='test')
+        r = Role()
+        self.assertEqual(r._load_list_of_blocks([]), [])
+        res = r._load_list_of_blocks([task])
+        self.assertEqual(len(res), 1)
+        assert isinstance(res[0], Block)
+        res = r._load_list_of_blocks([task,task,task])
+        self.assertEqual(len(res), 3)
+
+    def test_load_role_simple(self):
+        pass
+
+    def test_load_role_complex(self):
+        pass