From 1b0797440beecb133e529effb5f12c3a8aa4cf4f Mon Sep 17 00:00:00 2001
From: Scott Hussey <sh8121@att.com>
Date: Fri, 4 May 2018 16:50:42 -0500
Subject: [PATCH] [411428] Bootaction pkg_list support

- Support a list of debian packages as a bootaction asset
- Add unit testing for parsing the additional bootaction information
- Add __eq__ and __hash__ for DocumentReference to allow checking
  equality and list presence

Change-Id: I0ca42baf7aae6dc2e52efd5b311d0632e069dd79
---
 docs/source/bootaction.rst                    | 16 ++++-
 drydock_provisioner/error.py                  | 13 ++++
 drydock_provisioner/objects/bootaction.py     | 62 +++++++++++++++++--
 drydock_provisioner/objects/validation.py     | 14 +++++
 drydock_provisioner/schemas/bootaction.yaml   |  8 ++-
 .../postgres/test_action_config_node_prov.py  | 16 ++---
 tests/unit/test_ingester_invalidation.py      | 41 +++++++++---
 tests/yaml_samples/deckhand_bootaction.yaml   | 26 ++++++++
 tests/yaml_samples/invalid_bootaction.yaml    | 14 +++++
 9 files changed, 184 insertions(+), 26 deletions(-)

diff --git a/docs/source/bootaction.rst b/docs/source/bootaction.rst
index e6e0b199..d7b211ce 100644
--- a/docs/source/bootaction.rst
+++ b/docs/source/bootaction.rst
@@ -54,10 +54,10 @@ The boot action framework supports assets of several types. ``type`` can be ``un
 
     - ``unit`` is a SystemD unit, such as a service, that will be saved to ``path`` and enabled via ``systemctl enable [filename]``.
     - ``file`` is simply saved to the filesystem at ``path`` and set with ``permissions``.
-    - ``pkg_list`` is a list of packages, one per line, that will be installed via apt.
+    - ``pkg_list`` is a list of packages
 
 Data assets of type ``unit`` or ``file`` will be rendered and saved as files on disk and assigned
-the ``permissions`` as sepcified. The rendering process can follow a few different paths.
+the ``permissions`` as specified. The rendering process can follow a few different paths.
 
 Referenced vs Inline Data
 -------------------------
@@ -67,6 +67,18 @@ mapping or dynamically generated by requesting them from a URL provided in ``loc
 Currently Drydock supports the schemes of ``http``, ``deckhand+http`` and
 ``promenade+http`` for referenced data.
 
+Package List
+------------
+
+For the ``pkg_list`` type, the data section is expected to be a YAML mapping
+with key: value pairs of ``package_name``: ``version`` where ``package_name`` is
+a Debian package available in one of the configured repositories and ``version``
+is a valid apt version specifier or a empty/null value. Null indicates no version
+requirement.
+
+If using a referenced data source for the package list, Drydock expects a YAML
+or JSON document returned in the above format.
+
 Pipelines
 ---------
 
diff --git a/drydock_provisioner/error.py b/drydock_provisioner/error.py
index fe353f7a..280a05c6 100644
--- a/drydock_provisioner/error.py
+++ b/drydock_provisioner/error.py
@@ -184,6 +184,19 @@ class InvalidAssetLocation(BootactionError):
     pass
 
 
+class InvalidPackageListFormat(BootactionError):
+    """
+    **Message:** *Invalid package list format.*.
+
+    **Troubleshoot: A packagelist should be valid YAML
+                    document that is a mapping with keys being
+                    Debian package names and values being version
+                    specifiers. Null values are valid and indicate no
+                    version requirement.
+    """
+    pass
+
+
 class BuildDataError(Exception):
     """
     **Message:** *Error saving build data - data_element type <data_element>
diff --git a/drydock_provisioner/objects/bootaction.py b/drydock_provisioner/objects/bootaction.py
index 2e7f1061..c9572a28 100644
--- a/drydock_provisioner/objects/bootaction.py
+++ b/drydock_provisioner/objects/bootaction.py
@@ -15,6 +15,7 @@
 import base64
 from jinja2 import Template
 import ulid2
+import yaml
 
 import oslo_versionedobjects.fields as ovo_fields
 
@@ -107,6 +108,7 @@ class BootActionAsset(base.DrydockObject):
         'path': ovo_fields.StringField(nullable=True),
         'location': ovo_fields.StringField(nullable=True),
         'data': ovo_fields.StringField(nullable=True),
+        'package_list': ovo_fields.DictOfNullableStringsField(nullable=True),
         'location_pipeline': ovo_fields.ListOfStringsField(nullable=True),
         'data_pipeline': ovo_fields.ListOfStringsField(nullable=True),
         'permissions': ovo_fields.IntegerField(nullable=True),
@@ -120,6 +122,17 @@ class BootActionAsset(base.DrydockObject):
         else:
             mode = None
 
+        ba_type = kwargs.get('type', None)
+        if ba_type == 'pkg_list':
+            if isinstance(kwargs.get('data'), dict):
+                self._extract_package_list(kwargs.pop('data'))
+            # If the data section doesn't parse as a dictionary
+            # then the package data needs to be sourced dynamically
+            # Otherwise the Bootaction is invalid
+            elif not kwargs.get('location'):
+                raise errors.InvalidPackageListFormat(
+                    "Requires a top-level mapping/object.")
+
         super().__init__(permissions=mode, **kwargs)
         self.rendered_bytes = None
 
@@ -141,15 +154,52 @@ class BootActionAsset(base.DrydockObject):
             rendered_location = self.execute_pipeline(
                 self.location, self.location_pipeline, tpl_ctx=tpl_ctx)
             data_block = self.resolve_asset_location(rendered_location)
-        else:
+            if self.type == 'pkg_list':
+                self._parse_package_list(data_block)
+        elif self.type != 'pkg_list':
             data_block = self.data.encode('utf-8')
 
-        value = self.execute_pipeline(
-            data_block, self.data_pipeline, tpl_ctx=tpl_ctx)
+        if self.type != 'pkg_list':
+            value = self.execute_pipeline(
+                data_block, self.data_pipeline, tpl_ctx=tpl_ctx)
 
-        if isinstance(value, str):
-            value = value.encode('utf-8')
-        self.rendered_bytes = value
+            if isinstance(value, str):
+                value = value.encode('utf-8')
+            self.rendered_bytes = value
+
+    def _parse_package_list(self, data):
+        """Parse data expecting a list of packages to install.
+
+        Expect data to be a bytearray reprsenting a JSON or YAML
+        document.
+
+        :param data: A bytearray of data to parse
+        """
+        try:
+            data_string = data.decode('utf-8')
+            parsed_data = yaml.safe_load(data_string)
+
+            if isinstance(parsed_data, dict):
+                self._extract_package_list(parsed_data)
+            else:
+                raise errors.InvalidPackageListFormat(
+                    "Package data should have a top-level mapping/object.")
+        except yaml.YAMLError as ex:
+            raise errors.InvalidPackageListFormat(
+                "Invalid YAML in package list: %s" % str(ex))
+
+    def _extract_package_list(self, pkg_dict):
+        """Extract package data into object model.
+
+        :param pkg_dict: a dictionary of packages to install
+        """
+        self.package_list = dict()
+        for k, v in pkg_dict.items():
+            if isinstance(k, str) and isinstance(v, str):
+                self.package_list[k] = v
+            else:
+                raise errors.InvalidPackageListFormat(
+                    "Keys and values must be strings.")
 
     def _get_template_context(self, nodename, site_design, action_id,
                               design_ref):
diff --git a/drydock_provisioner/objects/validation.py b/drydock_provisioner/objects/validation.py
index ff490f1f..2b408050 100644
--- a/drydock_provisioner/objects/validation.py
+++ b/drydock_provisioner/objects/validation.py
@@ -112,6 +112,20 @@ class DocumentReference(base.DrydockObject):
             raise errors.UnsupportedDocumentType(
                 "Document type %s not supported." % self.doc_type)
 
+    def __eq__(self, other):
+        """Override equivalence operator."""
+        if isinstance(other, DocumentReference):
+            return (self.doc_type == other.doc_type
+                    and self.doc_schema == other.doc_schema
+                    and self.doc_name == other.doc_name)
+
+        return False
+
+    def __hash__(self):
+        """Override default hashing function."""
+        return hash(
+            str(self.doc_type), str(self.doc_schema), str(self.doc_name))
+
     def to_dict(self):
         """Serialize to a dictionary for further serialization."""
         d = dict()
diff --git a/drydock_provisioner/schemas/bootaction.yaml b/drydock_provisioner/schemas/bootaction.yaml
index d35e6683..504a1df3 100644
--- a/drydock_provisioner/schemas/bootaction.yaml
+++ b/drydock_provisioner/schemas/bootaction.yaml
@@ -31,7 +31,13 @@ data:
               - 'file'
               - 'pkg_list'
           data:
-            type: 'string'
+            oneOf:
+              - type: 'string'
+              - type: 'object'
+                additionalProperties:
+                  oneOf:
+                    - type: 'string'
+                    - type: 'null'
           location_pipeline:
             type: 'array'
             items:
diff --git a/tests/integration/postgres/test_action_config_node_prov.py b/tests/integration/postgres/test_action_config_node_prov.py
index 371baaea..53e9e932 100644
--- a/tests/integration/postgres/test_action_config_node_prov.py
+++ b/tests/integration/postgres/test_action_config_node_prov.py
@@ -20,13 +20,15 @@ class TestActionConfigureNodeProvisioner(object):
     def test_create_maas_repo(selfi, mocker):
         distribution_list = ['xenial', 'xenial-updates']
 
-        repo_obj = objects.Repository(name='foo',
-                                      url='https://foo.com/repo',
-                                      repo_type='apt',
-                                      gpgkey="-----START STUFF----\nSTUFF\n-----END STUFF----\n",
-                                      distributions=distribution_list,
-                                      components=['main'])
+        repo_obj = objects.Repository(
+            name='foo',
+            url='https://foo.com/repo',
+            repo_type='apt',
+            gpgkey="-----START STUFF----\nSTUFF\n-----END STUFF----\n",
+            distributions=distribution_list,
+            components=['main'])
 
-        maas_model = ConfigureNodeProvisioner.create_maas_repo(mocker.MagicMock(), repo_obj)
+        maas_model = ConfigureNodeProvisioner.create_maas_repo(
+            mocker.MagicMock(), repo_obj)
 
         assert maas_model.distributions == ",".join(distribution_list)
diff --git a/tests/unit/test_ingester_invalidation.py b/tests/unit/test_ingester_invalidation.py
index 7cef4005..8096693c 100644
--- a/tests/unit/test_ingester_invalidation.py
+++ b/tests/unit/test_ingester_invalidation.py
@@ -12,25 +12,46 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 """Test that boot action models are properly parsed."""
+import logging
 
 from drydock_provisioner.statemgmt.state import DrydockState
 import drydock_provisioner.objects as objects
 
+LOG = logging.getLogger(__name__)
+
 
 class TestClass(object):
     def test_bootaction_parse(self, input_files, deckhand_ingester, setup):
-        objects.register_all()
+        design_status, design_data = self.parse_design(
+            "invalid_bootaction.yaml", input_files, deckhand_ingester)
 
-        input_file = input_files.join("invalid_bootaction.yaml")
+        assert design_status.status == objects.fields.ActionResult.Failure
+
+        error_msgs = [m for m in design_status.message_list if m.error]
+        assert len(error_msgs) == 3
+
+    def test_invalid_package_list(self, input_files, deckhand_ingester, setup):
+        design_status, design_data = self.parse_design(
+            "invalid_bootaction.yaml", input_files, deckhand_ingester)
+
+        assert design_status.status == objects.fields.ActionResult.Failure
+
+        pkg_list_bootaction = objects.DocumentReference(
+            doc_type=objects.fields.DocumentType.Deckhand,
+            doc_schema="drydock/BootAction/v1",
+            doc_name="invalid_pkg_list")
+        LOG.debug(design_status.to_dict())
+        pkg_list_errors = [
+            m for m in design_status.message_list
+            if (m.error and pkg_list_bootaction in m.docs)
+        ]
+
+        assert len(pkg_list_errors) == 1
+
+    def parse_design(self, filename, input_files, deckhand_ingester):
+        input_file = input_files.join(filename)
 
         design_state = DrydockState()
         design_ref = "file://%s" % str(input_file)
 
-        design_status, design_data = deckhand_ingester.ingest_data(
-            design_state=design_state, design_ref=design_ref)
-
-        assert design_status.status == objects.fields.ActionResult.Failure
-
-        print(str(design_status.to_dict()))
-        error_msgs = [m for m in design_status.message_list if m.error]
-        assert len(error_msgs) == 2
+        return deckhand_ingester.ingest_data(design_state, design_ref)
diff --git a/tests/yaml_samples/deckhand_bootaction.yaml b/tests/yaml_samples/deckhand_bootaction.yaml
index d1d51569..88a393bd 100644
--- a/tests/yaml_samples/deckhand_bootaction.yaml
+++ b/tests/yaml_samples/deckhand_bootaction.yaml
@@ -51,3 +51,29 @@ data:
         - utf8_decode
         - template
 ...
+---
+schema: 'drydock/BootAction/v1'
+metadata:
+  schema: 'metadata/Document/v1'
+  name: pkg_install
+  storagePolicy: 'cleartext'
+  labels:
+    application: 'drydock'
+data:
+  signaling: true
+  assets:
+    - path: /var/tmp/hello.sh
+      type: file
+      permissions: '555'
+      data: |-
+        IyEvYmluL2Jhc2gKCmVjaG8gJ0hlbGxvIFdvcmxkISAtZnJvbSB7eyBub2RlLmhvc3RuYW1lIH19
+        Jwo=
+      data_pipeline:
+        - base64_decode
+        - utf8_decode
+        - template
+    - type: pkg_list
+      data:
+        2ping: '3.2.1-1'
+        0xffff:
+...
diff --git a/tests/yaml_samples/invalid_bootaction.yaml b/tests/yaml_samples/invalid_bootaction.yaml
index 0394418f..c9194dde 100644
--- a/tests/yaml_samples/invalid_bootaction.yaml
+++ b/tests/yaml_samples/invalid_bootaction.yaml
@@ -28,4 +28,18 @@ data:
       data_pipeline:
         - base64_decode
         - utf8_decode
+---
+schema: 'drydock/BootAction/v1'
+metadata:
+  schema: 'metadata/Document/v1'
+  name: invalid_pkg_list
+  storagePolicy: 'cleartext'
+  labels:
+    application: 'drydock'
+data:
+  assets:
+    - type: pkg_list
+      data:
+        - pkg1
+        - pkg2
 ...